bug#28407: 26.0.50; xref should use imenu

[Visuwesh]

[திங்கள் மே 30, 2022] Dmitry Gutov wrote:

> Hi Visuwesh,
>

Hi Dmitry!

> On 16.05.2022 09:59, Visuwesh wrote:
>> [ஞாயிறு மே 15, 2022] Visuwesh wrote:
>> 
>>> [திங்கள் செப்டம்பர் 11, 2017] Dmitry Gutov wrote:
>>>
>>>> On 9/10/17 7:23 PM, Tom Tromey wrote:
>>>>> It would be nice if imenu were a back end for xref.
>>>>> Then M-. could also use symbols found by imenu.
>>>>> A further wrinkle on this would be if xref, project, and imenu
>>>>> worked
>>>>> together, so that M-. would automatically know to look at imenu results
>>>>> for other buffers in the same project.
>>>>
>>>> Agreed. It could be a nice default for when no tags table is currently
>>>> visited.
>>>
>>> I tried to write a general imenu backend in attached file (extracted
>>> from my init.el) but I hit quite a few roadblocks,
>>>
>>>    1. I activate the imenu backend iff there are no tags table defined
>>>       for the buffer but this means that one cannot use the imenu
>>>       backend to jump to definitions for symbols that TAGS do not know
>>>       of currently.  I can think of two ways to solve this problem,
>>>
>>>       (a) Check if the symbol is in TAGS table.
>>>       (b) Modify the etags backend so that the user can say "I have no
>>>       TAGS table for this file/project/whatever."
>>>
>>>       (a) is definitely not clean, and (b) sounds feasible but similar
>>>       situation can also exist with other backends (like elisp).
>>>
>>>       I'm lost on how to solve this problem.
>>>
>>>    2. I have not defined all the methods and the completion-table does
>>>       not handle the nested case of the index alist.  AFAIU from `(elisp)
>>>       Programmed Completion', completion "ends" when `try-completion'
>>>       returns t but I seem to be mistaken.  I have to rewrite
>>>       completion-table to be like `imenu--completion-buffer' but I don't
>>>       know how to pull that off.
>>>
>>>    3. `imenu-xref--in-alist' is mostly a 1-1 copy of `imenu--in-alist'
>>>       with the only difference being my function returns all matches of
>>>       the symbol instead of just the first one.  This should be easy
>>>       enough to fix by adding an optional argument INCLUDE-ALL to
>>>       `imenu--in-alist'.
>>>
>>> I'm testing in python-mode with the following settings,
>>>
>>>      (setq imenu-name-lookup-function (lambda (symbol item) 
>>> (string-prefix-p symbol item))
>>>            python-imenu-format-parent-item-jump-label-function (lambda (_ 
>>> name) name))
>> I solved (2) by using an affixation function.  I did (3) as well,
>> and
>> I'm attaching my work as a patch against imenu.el.
>
> (1) sounds reasonable because the reference might easily be in another
> file. If you wanted to add extra customizations (for the user to be
> able to indicated things like "use imenu for xref in these files"),
> maybe add it to this backend's options. 

Do we really need something like this?  Can we not let the user handle
it themselves by adding/removing the imenu backend in .dir-locals.el?

Also what's the right way to check if a TAGS table is defined for the
current buffer?  I currently have,

    (or tags-table-files tags-file-name)

> As long as it comes before etags in xref-backend-functions, it should
> have all the power. Another possible direction for its development
> would be to always try to combine the locations coming from both etags
> and imenu (in the current file).

Yes, this sounds attractive but then we would be limiting ourselves to
etags.  IMO, xref trying another backend when one fails to produce a
match would be a better solution in the long term.
[ I, for one, would like to have imenu and elisp backends working at the
  same time.  ]

> I would leave that to a later revision, though. Some testing for
> performance regressions in large projects would be nice too.

Indeed.  Unfortunately, I don't have any large projects on hand.  All
the testing I did was in a small python script of mine.

> (2) Could you try to explain the problem that you were solving here?
> affixation-function is normally about how the completions look (I
> think). Would 'completion-table-with-predicate' help? Or maybe you
> just need to pre-process the nested structure into a "flat" completion
> table first.

I did the pre-processing but I hit a block wrt how the
xref-backend-definitions method worked.  To illustrate, in the test file
that I had I have a function that goes like

    def do1():
        ...
        
        def checkforlink1():
        ...

        def checkforlink2():
        ...

        def sortlinks():
        ...
            def weight():
            ...

        def checkforlink():
        ...

and the imenu--index-alist for this part looks like

    ("do1 (def)"
      ("do1" . #<marker at 2413 in rdscrape.py>)
      ("checkforlink1 (def)" . #<marker at 2850 in rdscrape.py>)
      ("checkforlink2 (def)" . #<marker at 3363 in rdscrape.py>)
      ("sortlinks (def)"
       ("sortlinks" . #<marker at 3721 in rdscrape.py>)
       ("weight (def)" . #<marker at 3747 in rdscrape.py>))
      ("checkforlink (def)" . #<marker at 4121 in rdscrape.py>))

I wrote the flatten function so that it would produce completion
candidates like

"do1:do1" "do1:checkforlink1 (def)" "do1:checkforlink2 (def)" ... and so
on.

But the xref-backend-definitions method can only handle the last part of
it (i.e., "do1" "checkforlink1 (def)").  Since I didn't feel like
parsing this "path-tree" (for a lack of a better word) would be
appropriate for xref-backend-definitions, I slapped this "path-tree" as
cosmetics in the affixation-function instead.  Hopefully, this makes
sense.

But perhaps handling this "path-tree" in xref-backend-definitions would
not hurt after all.  I can spin this up if you think this is better
moving forward.

Some other questions follow:

    1. I was thinking about adding a buffer-local function for the
       identifier-at-point instead of hard-coding (thing-at-point 'symbol)
       to let major-modes like org-mode and auctex-mode behave more 
       smartly. WDYT?

    2. When implementing the apropos method for the backend, should we
       let pattern match against the whole "path-tree" or just the last
       part of it?