Re: Bug: TAB cycling and narrowing to subtree [8.2.6 (release_8.2.6-903-ga1b4d9 @ /home/youngfrog/sources/org-mode/lisp/)]

[Nicolas Richard <theonewiththeevillook@yahoo.fr>]

Hello Bastien,

Bastien <bzg@gnu.org> writes:
> Nicolas Richard <theonewiththeevillook@yahoo.fr> writes:
>> If, instead, you first hit TAB (to open the subtree), then narrow to subtree
>> and hit TAB again to fold, you get "* love is" followed by
>> org-ellipsis followed by 'd' on the same line. The 'd' shouldn't be there.
>
> That's an annoying and known glitch -- I don't have time for this
> right now, but it is definitely in my radar.
>
> Maybe getting rid of (defadvice outline-end-of-subtree ...) in org.el
> is one way to start cleaning up that stuff.  If you can have a look,
> that'd be good.  Otherwise I will in the coming weeks.

I love the idea of cleaning, but that might require some knowledge of
the codebase that I don't have.

e.g. why does the advice exist in the first place ? IIUC,
org-end-of-subtree exists for speed reasons. Could we move the advice to
within org-end-of-subtree itself, i.e. something like:

(defun org-end-of-subtree (...)
   (if (derived-mode-p 'org-mode)
       (progn existing code)
     (outline-end-of-subtree)))

and then only call *that* function from within org.el. Code that wants
to be org-aware could use that function, code that doesn't need the
speedup in org mode shouldn't use it.

It might be more trouble than what it solves.

Another question is why does the advice use (unless (eobp)
(backward-char 1)) ? Couldn't that be moved to org-end-of-subtree itself
? Isn't that even superfluous given the 'to-heading' argument (which
could be set to nil) ? Does the test suite cover that part of org ?

Anyway, looking at the functions that get called, I spotted an easy fix.
WDYT ?

--- a/lisp/org.el
+++ b/lisp/org.el
@@ -6729,7 +6729,10 @@ in special contexts.
 	    (setq has-children (org-list-has-child-p (point) struct)))
 	(org-back-to-heading)
 	(setq eoh (save-excursion (outline-end-of-heading) (point)))
-	(setq eos (save-excursion (1- (org-end-of-subtree t t))))
+	(setq eos (save-excursion
+		    (org-end-of-subtree t t)
+		    (when (bolp) (backward-char))
+		    (point)))
 	(setq has-children
 	      (or (save-excursion
 		    (let ((level (funcall outline-level)))


-- 
Nico.