emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* 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/)]
@ 2016-07-29 18:16 talwrii talwrii
  2016-07-30 22:26 ` Nicolas Goaziou
  0 siblings, 1 reply; 2+ messages in thread
From: talwrii talwrii @ 2016-07-29 18:16 UTC (permalink / raw)
  To: emacs-orgmode


[-- Attachment #1.1: Type: text/plain, Size: 69 bytes --]

Org mode file complete with code to reproduce the issue is attached.

[-- Attachment #1.2: Type: text/html, Size: 94 bytes --]

[-- Attachment #2: map-tree-repro.org --]
[-- Type: application/octet-stream, Size: 4281 bytes --]

* 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` like so:

(defun fixed-org-map-tree (fun)
  "Call FUN for every heading underneath the current one."
  (org-back-to-heading 't)
  (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 can confirm that this gives us the expected results

#+begin_src emacs-lisp :results raw
(defun fixed-org-map-tree (fun)
  "Call FUN for every heading underneath the current one."
  (org-back-to-heading 't)
  (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)))))

(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)
(fixed-org-map-tree (lambda () (add-to-list 'accum (point))))
accum
)

#+end_src

#+RESULTS:
(33 20 10)

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.







* Backtrace
This is a behavioral bug, there is not backtrace.

^ permalink raw reply	[flat|nested] 2+ messages in thread

* 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/)]
  2016-07-29 18:16 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/)] talwrii talwrii
@ 2016-07-30 22:26 ` Nicolas Goaziou
  0 siblings, 0 replies; 2+ messages in thread
From: Nicolas Goaziou @ 2016-07-30 22:26 UTC (permalink / raw)
  To: talwrii talwrii; +Cc: emacs-orgmode

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2016-07-30 22:26 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-29 18:16 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/)] talwrii talwrii
2016-07-30 22:26 ` Nicolas Goaziou

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).