GNU bug report logs - #32510
xref-find-definitions should return file names, too

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 32510 in the body.
You can then email your comments to 32510 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Brenta <ludovic <at> ludovic-brenta.org>
To: <bug-gnu-emacs <at> gnu.org>
Subject: xref-find-definitions should return file names, too
Date: Thu, 23 Aug 2018 16:32:21 +0100

Package: emacs
Version: 26.1
Severity: wishlist

Hello,

It would be nice if xref-find-definitions returned files in
addition to language-specific "definitions".  For example:

M-. foo-bar.adb RET

should open the file foo-bar.adb (wherever it is in the
potentially complex directory structure of the project) and
leave point at the beginning of the file.

This is a feature of find-tag but find-tag is now deprecated
in favor of xref-find-definitions; so this feature is missing
and xref-find-definitions is not yet a complete replacement for
find-tag.

Thanks for consideration.

-- 
Ludovic Brenta.

Message #8 received at 32510 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Ludovic Brenta <ludovic <at> ludovic-brenta.org>
Cc: 32510 <at> debbugs.gnu.org
Subject: Re: bug#32510: xref-find-definitions should return file names, too
Date: Sat, 13 Jul 2019 04:50:34 +0200

Ludovic Brenta <ludovic <at> ludovic-brenta.org> writes:

> It would be nice if xref-find-definitions returned files in
> addition to language-specific "definitions".  For example:
>
> M-. foo-bar.adb RET
>
> should open the file foo-bar.adb (wherever it is in the
> potentially complex directory structure of the project) and
> leave point at the beginning of the file.
>
> This is a feature of find-tag but find-tag is now deprecated
> in favor of xref-find-definitions; so this feature is missing
> and xref-find-definitions is not yet a complete replacement for
> find-tag.

Hm...  it seems to me that a command like that seems to belong more in
something like project.el than in xref, which has a different scope.  So
I'm closing this bug report; if somebody else disagrees, please reopen.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #15 received at 32510 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Brenta <ludovic <at> ludovic-brenta.org>
To: 32510 <at> debbugs.gnu.org, control <at> debbugs.gnu.org
Subject: Re: bug#32510 acknowledged by developer (control message for bug
 #32510)
Date: Sat, 13 Jul 2019 21:34:26 +0200

reopen 32510
thanks

Please to not close this bug so summarily.  Quoting the doc-string of
find-tag:

This function is obsolete since 25.1;
use ‘xref-find-definitions’ instead.

This bug report states that a useful functionality of find-tag is *not*
provided by its official replacement, xref-find-definitions.  This is a
regression.  Just because you think this missing functionality should be
provided elsewhere is not a good reason to close this bug without
providing any solution.

-- 
Ludovic Brenta.

Message #20 received at 32510 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Ludovic Brenta <ludovic <at> ludovic-brenta.org>, 32510 <at> debbugs.gnu.org,
 control <at> debbugs.gnu.org
Subject: RE: bug#32510: acknowledged by developer (control message for bug
 #32510)
Date: Sat, 13 Jul 2019 16:25:49 -0700 (PDT)

> reopen 32510
> thanks
> 
> Please [d]o not close this bug so summarily.  Quoting the doc-string of
> find-tag:
> 
> This function is obsolete since 25.1;
> use ‘xref-find-definitions’ instead.
> 
> This bug report states that a useful functionality of find-tag is *not*
> provided by its official replacement, xref-find-definitions.  This is a
> regression.  Just because you think this missing functionality should be
> provided elsewhere is not a good reason to close this bug without
> providing any solution.

+1

And I don't think that `find-tag' should be deprecated
(obsolete).  Its "replacement" is simply a different
command, with some things in common and some things
different.  Both have their uses; each can be preferred
by some users for some things.

(I also don't think that the default key binding of
`find-tag' should have been hijacked for its
"replacement", but that's a different and lesser problem.)

Message #23 received at 32510 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ludovic Brenta <ludovic <at> ludovic-brenta.org>
Cc: 32510 <at> debbugs.gnu.org
Subject: Re: bug#32510: acknowledged by developer (control message for bug
 #32510)
Date: Sun, 14 Jul 2019 08:21:23 +0300

> From: Ludovic Brenta <ludovic <at> ludovic-brenta.org>
> Date: Sat, 13 Jul 2019 21:34:26 +0200
> 
> This bug report states that a useful functionality of find-tag is *not*
> provided by its official replacement, xref-find-definitions.  This is a
> regression.  Just because you think this missing functionality should be
> provided elsewhere is not a good reason to close this bug without
> providing any solution.

With the patch below, you should be able to have what you want if you
add tag-partial-file-name-match-p to the list in
etags-xref-find-definitions-tag-order.  Please try this patch and see
if it works for you.

Thanks.

diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index 7bf5753..b092c63 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -2070,13 +2070,16 @@ etags--xref-find-definitions
               (beginning-of-line)
               (pcase-let* ((tag-info (etags-snarf-tag))
                            (`(,hint ,line . _) tag-info))
-                (unless (eq hint t) ; hint==t if we are in a filename line
+                (unless (and (eq hint t) ; we are in a filename line
+                             (not (eq order-fun
+                                      'tag-partial-file-name-match-p)))
                   (let* ((file (file-of-tag))
                          (mark-key (cons file line)))
                     (unless (gethash mark-key marks)
                       (let ((loc (xref-make-etags-location
                                   tag-info (expand-file-name file))))
-                        (push (xref-make hint loc) xrefs)
+                        (push (xref-make (if (eq hint t) pattern hint) loc)
+                               xrefs)
                         (puthash mark-key t marks)))))))))))
     (nreverse xrefs)))
 

Message #26 received at 32510 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Brenta <ludovic <at> ludovic-brenta.org>
To: control <at> debbugs.gnu.org
Subject: Tags: wontfix -> patch
Date: Mon, 15 Jul 2019 15:54:25 +0200

tags 32510 -wontfix patch
thanks

-- 
Ludovic Brenta.

Message #31 received at 32510 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Brenta <ludovic <at> ludovic-brenta.org>
To: 32510 <at> debbugs.gnu.org
Subject: bug#32510 xref-find-definitions should return file names, too
Date: Thu, 18 Jul 2019 16:53:42 +0200

tags 32510 - wontfix
thanks

I can confirm that the patch by Eli Zaretskii works, with a
difference compared to find-tag: find-tag opens the first
file whose name matches the searched string whereas
xref-find-definitions opens a new buffer with all matches,
forcing the user to use many keystrokes (or worse: reach
for the mouse :)) to choose a match.

I suppose this change of behavior is intentional, consistent
with all other cross-references, and only affects ergonomy;
the patch more importantly restores the functionality that
was previously lost.

-- 
Ludovic Brenta.

Message #36 received at 32510 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ludovic Brenta <ludovic <at> ludovic-brenta.org>
Cc: 32510 <at> debbugs.gnu.org
Subject: Re: bug#32510: xref-find-definitions should return file names, too
Date: Thu, 18 Jul 2019 18:16:23 +0300

> Date: Thu, 18 Jul 2019 16:53:42 +0200
> From: Ludovic Brenta <ludovic <at> ludovic-brenta.org>
> 
> I can confirm that the patch by Eli Zaretskii works, with a
> difference compared to find-tag: find-tag opens the first
> file whose name matches the searched string whereas
> xref-find-definitions opens a new buffer with all matches,
> forcing the user to use many keystrokes (or worse: reach
> for the mouse :)) to choose a match.

That's not what happened to me after the patch.  For me, M-. just
visited the one file whose name I typed.

Can you show the exact sequence of commands you typed, preferably
using the Emacs sources and corresponding TAGS tables and file names
as the basis, so that I could repeat it here?

> I suppose this change of behavior is intentional, consistent
> with all other cross-references, and only affects ergonomy;
> the patch more importantly restores the functionality that
> was previously lost.

I cannot tell whether it's intentional until I see the behavior you
describe.  What I can say is that if there's only one match, xref goes
there automatically and immediately, but if there are several
candidate matches, xref shows them and allows you to select the one(s)
you want.  The xref behavior is better when the match you want is not
one of the first few, because find-tag required you to continuously
type "C-u M-." in that case, and moreover do that blindly, since you
had no idea how far away is your match.  With xref you can select the
match you are after without iterating through all the previous ones.

However, I would expect the user to type the full file name in this
use case, since that's what this feature is about: finding a file
given its name.  In that case, both commands behave almost
identically.

Dmitry, any comments on the patch?  I admit I didn't study in detail
the role of the PATTERN slot of the object generated by the function
where I proposed to make the change, so perhaps I'm missing some use
case where the patch will not DTRT?

Thanks.

Message #39 received at 32510 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Brenta <ludovic <at> ludovic-brenta.org>
To: 32510 <at> debbugs.gnu.org
Subject: Re: bug#32510: xref-find-definitions should return file names, too
Date: Thu, 18 Jul 2019 17:54:59 +0200

Le 2019-07-18 17:16, Eli Zaretskii a écrit :
> [...] if there's only one match, xref goes
> there automatically and immediately, but if there are several
> candidate matches, xref shows them and allows you to select the one(s)
> you want.

Yes, this is exactly what happens.  We have thousands of source files
in our tree and most have names longer than 20 characters.  Our normal
usage pattern is to use partial matching.  Also your patch uses
tag-partial-file-name-match-p, not tag-full-file-name-match-p, so
it's not surprising that it should do partial matching with possibly
more than one match :)

With etags we were used to using "C-u M-." a couple times too, or
start over with a longer substring of the file name we wanted.

I'm not complaining about this new behavior; it will just take a
little getting used to.  Personally I like the fact that M-g M-n
works with the *xref* buffer like it does in a *compilation* buffer.

-- 
Ludovic Brenta.

Message #42 received at 32510 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ludovic Brenta <ludovic <at> ludovic-brenta.org>
Cc: 32510 <at> debbugs.gnu.org
Subject: Re: bug#32510: xref-find-definitions should return file names, too
Date: Thu, 18 Jul 2019 19:07:50 +0300

> Date: Thu, 18 Jul 2019 17:54:59 +0200
> From: Ludovic Brenta <ludovic <at> ludovic-brenta.org>
> 
> Yes, this is exactly what happens.  We have thousands of source files
> in our tree and most have names longer than 20 characters.  Our normal
> usage pattern is to use partial matching.  Also your patch uses
> tag-partial-file-name-match-p, not tag-full-file-name-match-p, so
> it's not surprising that it should do partial matching with possibly
> more than one match :)
> 
> With etags we were used to using "C-u M-." a couple times too, or
> start over with a longer substring of the file name we wanted.
> 
> I'm not complaining about this new behavior; it will just take a
> little getting used to.  Personally I like the fact that M-g M-n
> works with the *xref* buffer like it does in a *compilation* buffer.

OK, so I hope Dmitry will approve the change.

Thanks.

Message #45 received at 32510 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Ludovic Brenta <ludovic <at> ludovic-brenta.org>, Eli Zaretskii <eliz <at> gnu.org>
Cc: "32510 <at> debbugs.gnu.org" <32510 <at> debbugs.gnu.org>
Subject: Re: bug#32510: xref-find-definitions should return file names, too
Date: Sat, 20 Jul 2019 01:23:32 +0300

[Message part 1 (text/html, inline)]

Message #48 received at 32510 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 32510 <at> debbugs.gnu.org, ludovic <at> ludovic-brenta.org
Subject: Re: bug#32510: xref-find-definitions should return file names, too
Date: Sat, 20 Jul 2019 10:17:38 +0300

> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Cc: "32510 <at> debbugs.gnu.org" <32510 <at> debbugs.gnu.org>
> Date: Sat, 20 Jul 2019 01:23:32 +0300
> 
> Sorry, I'm on a vacation in the next several days, and away from my computer, so I can't test it.
> 
> But the idea behind the patch seems sound, and if it works fine for you (in particular, with partial file name
> inputs), it's probably good.

Thanks.  I prefer to wait for you to review the code when you have
time.

Message #51 received at 32510 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>, Ludovic Brenta <ludovic <at> ludovic-brenta.org>
Cc: 32510 <at> debbugs.gnu.org
Subject: Re: bug#32510: acknowledged by developer (control message for bug
 #32510)
Date: Tue, 30 Jul 2019 03:06:18 +0300

On 14.07.2019 8:21, Eli Zaretskii wrote:
>> From: Ludovic Brenta <ludovic <at> ludovic-brenta.org>
>> Date: Sat, 13 Jul 2019 21:34:26 +0200
>>
>> This bug report states that a useful functionality of find-tag is *not*
>> provided by its official replacement, xref-find-definitions.  This is a
>> regression.  Just because you think this missing functionality should be
>> provided elsewhere is not a good reason to close this bug without
>> providing any solution.
> 
> With the patch below, you should be able to have what you want if you
> add tag-partial-file-name-match-p to the list in

Finally got around to reviewing it...

> diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
> index 7bf5753..b092c63 100644
> --- a/lisp/progmodes/etags.el
> +++ b/lisp/progmodes/etags.el
> @@ -2070,13 +2070,16 @@ etags--xref-find-definitions
>                 (beginning-of-line)
>                 (pcase-let* ((tag-info (etags-snarf-tag))
>                              (`(,hint ,line . _) tag-info))
> -                (unless (eq hint t) ; hint==t if we are in a filename line
> +                (unless (and (eq hint t) ; we are in a filename line
> +                             (not (eq order-fun
> +                                      'tag-partial-file-name-match-p)))

First, I was thinking we shouldn't check for the exact order-fun value 
(because others could be used, too) and replace it with something like

                              (save-excursion
                               (forward-line 0)
                               (forward-char -2)
                               (not (looking-at "\f\n")))

But then, I'm not sure why that check is there in the first place (the 
order functions make sure not to match the wrong like). Maybe because 
the code inside couldn't handle hint=t? So if it does now, the (unless 
...) conditional can be removed.

>                     (let* ((file (file-of-tag))
>                            (mark-key (cons file line)))
>                       (unless (gethash mark-key marks)
>                         (let ((loc (xref-make-etags-location
>                                     tag-info (expand-file-name file))))
> -                        (push (xref-make hint loc) xrefs)
> +                        (push (xref-make (if (eq hint t) pattern hint) loc)
> +                               xrefs)

I'm not sure using pattern as a hint works well. How about we say 
something like "(file name match)" instead? Or you could pick a better 
wording. The full proposed patch is below.

I see that it doesn't work exactly perfectly, e.g. moving point within 
the quotes in

  #include "composite.h"

and pressing M-. brings up three matches (composite.c, composite.h and 
composite.el), whereas only one of them is correct, but find-tag 
probably has the same problem anyway. Maybe CC Mode should set up 
find-tag-default-function to return the full file name when inside 
#include statements.

diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index 7bf575340e..a052ad2ce5 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -2070,14 +2070,15 @@ etags--xref-find-definitions
               (beginning-of-line)
               (pcase-let* ((tag-info (etags-snarf-tag))
                            (`(,hint ,line . _) tag-info))
-                (unless (eq hint t) ; hint==t if we are in a filename line
-                  (let* ((file (file-of-tag))
-                         (mark-key (cons file line)))
-                    (unless (gethash mark-key marks)
-                      (let ((loc (xref-make-etags-location
-                                  tag-info (expand-file-name file))))
-                        (push (xref-make hint loc) xrefs)
-                        (puthash mark-key t marks)))))))))))
+                (let* ((file (file-of-tag))
+                       (mark-key (cons file line)))
+                  (unless (gethash mark-key marks)
+                    (let ((loc (xref-make-etags-location
+                                tag-info (expand-file-name file))))
+                      (push (xref-make (if (eq hint t) "(filename 
match)" hint)
+                                       loc)
+                            xrefs)
+                      (puthash mark-key t marks))))))))))
     (nreverse xrefs)))

 (defclass xref-etags-location (xref-location)

Message #54 received at 32510 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>, Ludovic Brenta <ludovic <at> ludovic-brenta.org>
Cc: 32510 <at> debbugs.gnu.org
Subject: Re: bug#32510: acknowledged by developer (control message for bug
 #32510)
Date: Tue, 30 Jul 2019 17:00:28 +0300

On 30.07.2019 3:06, Dmitry Gutov wrote:
> The full proposed patch is below.

I've pushed that change now to master. Please try it out.

Message #59 received at 32510-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 32510-done <at> debbugs.gnu.org, ludovic <at> ludovic-brenta.org
Subject: Re: bug#32510: acknowledged by developer (control message for bug
 #32510)
Date: Sat, 03 Aug 2019 13:00:36 +0300

> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Cc: 32510 <at> debbugs.gnu.org
> Date: Tue, 30 Jul 2019 17:00:28 +0300
> 
> On 30.07.2019 3:06, Dmitry Gutov wrote:
> > The full proposed patch is below.
> 
> I've pushed that change now to master. Please try it out.

Thanks, it LGTM, so I'm closing the bug.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.