See attached 3 patches. They address all comments and should be fine to apply. That being said, I did uncover some interesting things.
>From what I can tell, sparse trees and org-occur are supposed to refer to the same thing but actually refer to two different things. I'll use the names that org-fold gives these two things: "occur-tree" and "tags-tree". The function `org-occur' creates an occur-tree. The function `org-sparse-tree' can create either an occur-tree (by calling `org-occur') or a tags-tree depending on user input. The two sparse tree's I personally tend to use are "[m]atch" and "[p]roperty" which are both tags-tree's. `org-scan-tags' is what creates the tags-tree and seems to have most of the logic of `org-occur' although much more spread out. There are some important differences though. Features missing from `org-scan-tags' that `org-occur' has - Optionally allow keeping the previous search to stack searches - Run `org-occur' hooks - Remove highlights after change Ihor Radchenko <[email protected]> writes: > Morgan Smith <[email protected]> writes: > >> While I was looking at it I also removed an incorrect sentence from the >> docstring of `org-highlight-sparse-tree-matches'. The highlights >> automatically >> disapper on edit for org-occur matches but not for sparse trees > > Maybe we should instead add this feature to sparse tree matches? Ya we probably should. >> + (buffer-substring-no-properties (point) (pos-eol)))) > > Do note that `pos-eol' is not available in Emacs 28 Fixed. >> Subject: [PATCH 2/2] Fix `next-error' behavior with sparse trees >> >> * lisp/org.el (org-highlight-sparse-tree-matches): Adjust docstring. > > Ideally, we should limit commits to a single change. Changing the > docstring appears to be irrelevant to fixing next-error behavior. It > would better be placed in a separate commit. Fixed. >> (org-scan-tags): Set `next-error-last-buffer' when adding >> highlighting. >> * testing/lisp/test-org.el (test-org/sparse-tree-next-error): Show >> fixed behavior. Remove todo. > > Please also add Reported-by: and link back to this email thread. See > https://orgmode.org/worg/org-contribute.html#org252f961 Fixed. >> @@ -11570,7 +11569,8 @@ org-scan-tags >> (and org-highlight-sparse-tree-matches >> (org-get-heading) (match-end 0) >> (org-highlight-new-match >> - (match-beginning 1) (match-end 1))) >> + (match-beginning 1) (match-end 1)) >> + (setq next-error-last-buffer (current-buffer))) > > This way, your change will only take effect when > `org-highlight-sparse-tree-matches' is non-nil. When it is nil, the bug > will still remain. Fixed. > Also, what about `org-occur' itself? Won't it suffer from the same bug? Fixed.
>From 11d64fba79698f47fae2c80634b14060d95cc64e Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Tue, 7 Oct 2025 14:42:12 -0400 Subject: [PATCH 1/3] test-org/sparse-tree-next-error: New test * testing/lisp/test-org.el (test-org/sparse-tree-next-error): New test. --- testing/lisp/test-org.el | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index 88c083def..88f0c2534 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -8016,6 +8016,43 @@ test-org/match-sparse-tree (search-forward "H2") (org-invisible-p2)))) +(ert-deftest test-org/sparse-tree-next-error () + "Test calling `next-error' on sparse trees." + :expected-result :failed + (org-test-with-temp-text "* H :tag:\n** H1 :tag:\n** H2 :tag:\n" + (org-match-sparse-tree nil "tag") + (should + (string-equal + "* H :tag:" + (buffer-substring-no-properties (point) (line-end-position)))) + (next-error) + (should + (string-equal + "** H1 :tag:" + (buffer-substring-no-properties (point) (line-end-position)))) + (next-error) + (should + (string-equal + "** H2 :tag:" + (buffer-substring-no-properties (point) (line-end-position)))) + ;; Now switch to another buffer and try again leaving the first one intact + (org-test-with-temp-text "* 2H :tag:\n** 2H1 :tag:\n** 2H2 :tag:\n" + (org-match-sparse-tree nil "tag") + (should + (string-equal + "* 2H :tag:" + (buffer-substring-no-properties (point) (line-end-position)))) + (next-error) + (should + (string-equal + "** 2H1 :tag:" + (buffer-substring-no-properties (point) (line-end-position)))) + (next-error) + (should + (string-equal + "** 2H2 :tag:" + (buffer-substring-no-properties (point) (line-end-position))))))) + (ert-deftest test-org/occur () "Test `org-occur' specifications." ;; Count number of matches. base-commit: df5628041dd2317f458e2903bc61fb985849c328 -- 2.51.0
>From c1778386ecc0b3c60ef957009344c5704546a651 Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Tue, 7 Oct 2025 19:13:11 -0400 Subject: [PATCH 2/3] Fix `next-error' behavior with sparse trees * lisp/org.el (org-occur): Set `next-error-last-buffer'. (org-scan-tags): Set `next-error-last-buffer' for sparse-tree's. * testing/lisp/test-org.el (test-org/sparse-tree-next-error): Test passes. Reported-by: "Peter Lutz" <[email protected]> Link: https://list.orgmode.org/[email protected]/ --- lisp/org.el | 10 +++++++--- testing/lisp/test-org.el | 1 - 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lisp/org.el b/lisp/org.el index 6b8d02b87..9df2cb837 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -11250,6 +11250,8 @@ org-occur nil 'local)) (unless org-sparse-tree-open-archived-trees (org-fold-hide-archived-subtrees (point-min) (point-max))) + (when org-highlight-sparse-tree-matches + (setq next-error-last-buffer (current-buffer))) (run-hooks 'org-occur-hook) (when (called-interactively-p 'interactive) (message "%d match(es) for regexp %s" cnt regexp)) @@ -11627,9 +11629,11 @@ org-scan-tags :next-re heading-re :fail-re heading-re :narrow t)) - (when (and (eq action 'sparse-tree) - (not org-sparse-tree-open-archived-trees)) - (org-fold-hide-archived-subtrees (point-min) (point-max))) + (when (eq action 'sparse-tree) + (unless org-sparse-tree-open-archived-trees + (org-fold-hide-archived-subtrees (point-min) (point-max))) + (when org-highlight-sparse-tree-matches + (setq next-error-last-buffer (current-buffer)))) (nreverse rtn))) (defun org-remove-uninherited-tags (tags) diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index 88f0c2534..9873b45f6 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -8018,7 +8018,6 @@ test-org/match-sparse-tree (ert-deftest test-org/sparse-tree-next-error () "Test calling `next-error' on sparse trees." - :expected-result :failed (org-test-with-temp-text "* H :tag:\n** H1 :tag:\n** H2 :tag:\n" (org-match-sparse-tree nil "tag") (should -- 2.51.0
>From e5d96a5e7963b24b1f4c15fafddbca3d1623d1fc Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Thu, 30 Oct 2025 17:27:04 -0400 Subject: [PATCH 3/3] Document that sparse tree highlights are used in `next-error' * lisp/org.el (org-highlight-sparse-tree-matches): Document that variable must be non-nil to traverse the sparse tree using `next-error'. --- lisp/org.el | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lisp/org.el b/lisp/org.el index 9df2cb837..ed56c46f3 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -1783,7 +1783,8 @@ org-sparse-trees (defcustom org-highlight-sparse-tree-matches t "Non-nil means highlight all matches that define a sparse tree. The highlights will automatically disappear the next time the buffer is -changed by an edit command." +changed by an edit command. +Must be non-nil to traverse the sparse tree using `next-error'." :group 'org-sparse-trees :type 'boolean) -- 2.51.0
