Re: Bug: org-map-tree behaves strangely if first heading before point is hidden [8.3.5 (8.3.5-elpa @ /home/tom/.emacs.d/elpa/org-20160725/)]

[Nicolas Goaziou <mail@nicolasgoaziou.fr>]

Hello,

talwrii talwrii <talwrii@gmail.com> writes:

> Org mode file complete with code to reproduce the issue is attached.
>
> * Folded
> ** Hidden
> *** Children
> *** More children
>
> ** Other child
>
>
> * Bug description
> When calling org-map-tree on a hidden tree, the results are different from
> those if the tree were visible: this is not as expected.
>
> * Repro
> The following code samples consistently reproduce this behaviour.
> Run then with `C-c C-c` when the cursor is over the code block.
>
> The first folds up the first tree in this document and then calls `org-map-tree` with the point at "Hidden";
> the second does the same but expands the first tree.
>
> I consider the case with the expanded tree to retrun the correct result.
> ** Folded
> #+begin_src emacs-lisp :results raw
> (save-excursion
> ; Expand first tree
> (goto-char 0)
> (while (not (equal org-cycle-subtree-status 'folded))
>   (org-cycle))
>
> ; Run map over `Hidden`
> (goto-char 10)
> (setq accum nil)
> (org-map-tree (lambda () (add-to-list 'accum (point))))
> accum
> )
>
> #+end_src
> #+RESULTS:
> (52 33 20 10 1)
> ** Expanded
> #+begin_src emacs-lisp :results raw
> (save-excursion 
>
> (goto-char 0)
> (setq accum nil)
>
> (while (not (equal org-cycle-subtree-status 'children))
>   (message (format "%S" org-cycle-subtree-status))
>   (org-cycle))
>
> (goto-char 10)
>
> (org-map-tree (lambda () (add-to-list 'accum (point))))
> accum
> )
> #+end_src
>
> #+RESULTS:
> (33 20 10)
>
> * Likely cause and fix
>
> Looking at `org-map-tree`:
>
> (defun org-map-tree (fun)
>   "Call FUN for every heading underneath the current one."
>   (org-back-to-heading)
>   (message (format "org-map-tree start %S" (point)))
>   (let ((level (funcall outline-level)))
>     (save-excursion
>       (funcall fun)
>       (while (and (progn
>                     (prog1
>                       (outline-next-heading)
>                       (message (format "org-map-tree outline-ext-point %S" (point)))
>                       )
> 		    (> (funcall outline-level) level))
> 		  (not (eobp)))
> 	(funcall fun)))))
>
> We see that it calls `org-back-to-heading`; this skips back to the first visible ancestor heading.
>
> `org-back-to-heading` has the optional argument `invisible-ok` which causes it to skip back to the first
> ancestor heading, whether it is visible or not.
>
> (defun org-back-to-heading (&optional invisible-ok)
>   "Call `outline-back-to-heading', but provide a better error message."
>   (condition-case nil
>       (outline-back-to-heading invisible-ok)
>     (error (error "Before first headline at position %d in buffer %s"
> 		  (point) (current-buffer)))))
>
> I think that `org-map-subtree` should call `org-back-to-heading` with
> the `invisible-ok` argument set to `'t`

That's correct.

> However, this is changes behavior, and people may be relying upon this behaviour in their scripts... though it would
> be rather strange to have your code behave different based on whether subheadings are visible or not. So perhaps
> we should just make this change.

This is not a big issue because you can test within the function if the
headline is collapsed or not.

Anyway, just to be safe, I've changed it in master.

Thank you.

Regards,

-- 
Nicolas Goaziou