[PATCH] lisp/org-compat.el: Allow using imenu to visit non-leaf headlines

[PATCH] lisp/org-compat.el: Allow using imenu to visit non-leaf headlines

[Morgan Smith]

With a file like this:

* headline 1
** headline 2

We currently produce an imenu tree that looks like this:

'(("headline 1" ("headline 2" . marker-2)))

imenu has no clue where "headline 1" is located and thus the user
can't navigate to it.  With this patch installed imenu knows where
non-leaf headlines are as the tree will now look like this:

'(("headline 1" . marker-1)
  ("headline 1" ("headline 2" . marker-2)))

Quirks:

With the default `imenu-flatten' value of nil, it is still impossible
to visit non-leaf headlines and no change is perceived.

Setting `imenu-flatten' to 'group works as expected with the quirk
that top level headlines don't end up in the group.

Ex:
* Headline 1
Group is "*"
Setting the group to "Headline 1" somehow might be nice but would
require upstream changes in imenu.
** Headline 2
Group is "Headline 1"
*** Headline 3
Group is "Headline 1:Headline 2"

Everything seems to work as expected when `imenu-flatten' is set to
'prefix or 'annotation.

* lisp/org-compat.el (org-imenu-get-tree): Add the current headline to
the tree as a simple item even if it isn't a leaf.
---
 lisp/org-compat.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/org-compat.el b/lisp/org-compat.el
index d6620f962..a1152186d 100644
--- a/lisp/org-compat.el
+++ b/lisp/org-compat.el
@@ -1447,8 +1447,8 @@ This also applied for speedbar access."
 	   (let* ((m (point-marker))
 		  (item (propertize headline 'org-imenu-marker m 'org-imenu t)))
 	     (push m org-imenu-markers)
-	     (if (>= level last-level)
-		 (push (cons item m) (aref subs level))
+             (push (cons item m) (aref subs level))
+             (unless (>= level last-level)
 	       (push (cons item
 			   (cl-mapcan #'identity (cl-subseq subs (1+ level))))
 		     (aref subs level))
-- 
2.45.1

Re: [PATCH] lisp/org-compat.el: Allow using imenu to visit non-leaf headlines

[Ihor Radchenko]

Morgan Smith <Morgan.J.Smith@outlook.com> writes:

> With a file like this:
>
> * headline 1
> ** headline 2
>
> We currently produce an imenu tree that looks like this:
>
> '(("headline 1" ("headline 2" . marker-2)))
>
> imenu has no clue where "headline 1" is located and thus the user
> can't navigate to it.  With this patch installed imenu knows where
> non-leaf headlines are as the tree will now look like this:
>
> '(("headline 1" . marker-1)
>   ("headline 1" ("headline 2" . marker-2)))

Thanks!
This makes sense.

> Quirks:
>
> With the default `imenu-flatten' value of nil, it is still impossible
> to visit non-leaf headlines and no change is perceived.

But no regressions, right? Especially in older Emacs versions with
`imenu-flatten' not yet available.

> Setting `imenu-flatten' to 'group works as expected with the quirk
> that top level headlines don't end up in the group.

We may add a top-level group, can't we?

> Ex:
> * Headline 1
> Group is "*"
> Setting the group to "Headline 1" somehow might be nice but would
> require upstream changes in imenu.
> ** Headline 2
> Group is "Headline 1"
> *** Headline 3
> Group is "Headline 1:Headline 2"
>
> Everything seems to work as expected when `imenu-flatten' is set to
> 'prefix or 'annotation.

We may also consider changing the default value of `imenu-flatten' in
Org buffers to non-nil. But what would be the best default?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] lisp/org-compat.el: Allow using imenu to visit non-leaf headlines

[Morgan Smith]

Ihor Radchenko <yantar92@posteo.net> writes:

> Morgan Smith <Morgan.J.Smith@outlook.com> writes:
>
>> Quirks:
>>
>> With the default `imenu-flatten' value of nil, it is still impossible
>> to visit non-leaf headlines and no change is perceived.
>
> But no regressions, right? Especially in older Emacs versions with
> `imenu-flatten' not yet available.
>

No regressions that I can tell from my testing.  Both emacs 29.3 (before
imenu-flatten) and emacs-master show no change whatsoever (from what I
can tell) when imenu-flatten is nil.

>> Setting `imenu-flatten' to 'group works as expected with the quirk
>> that top level headlines don't end up in the group.
>
> We may add a top-level group, can't we?
>

This does not seem to be a feature.  You can confirm this yourself by
reading the comment in `imenu--flatten-index-alist' that says "PREFIX is
for internal use only".

> We may also consider changing the default value of `imenu-flatten' in
> Org buffers to non-nil.

In my opinion this is not a good idea.  While the UI of imenu is better
when `imenu-flatten' is set, that's not a decision we should make for
users.

The inability for imenu to visit non-leaf nodes with the default UI is
probably something that should be fixed in emacs core.

> But what would be the best default?

IMO that would be 'prefix.  Which is equivalent to setting it to `t'.

Re: [PATCH] lisp/org-compat.el: Allow using imenu to visit non-leaf headlines

[Ihor Radchenko]

Morgan Smith <morgan.j.smith@outlook.com> writes:

>>> With the default `imenu-flatten' value of nil, it is still impossible
>>> to visit non-leaf headlines and no change is perceived.
>>
>> But no regressions, right? Especially in older Emacs versions with
>> `imenu-flatten' not yet available.
>>
>
> No regressions that I can tell from my testing.  Both emacs 29.3 (before
> imenu-flatten) and emacs-master show no change whatsoever (from what I
> can tell) when imenu-flatten is nil.

Applied, onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=b7b188e23

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>