bug#31027: 27.0.50; xref, tags-location-ring equivalent

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#31027: 27.0.50; xref, tags-location-ring equivalent

Charles A. Roelli
tags-location-ring seems to have no replacement in xref.el.

from etags.el:
(defvar tags-location-ring (make-ring xref-marker-ring-length)
  "Ring of markers which are locations visited by \\[find-tag].
Pop back to the last location with \\[negative-argument] \\[find-tag].")

We should add a "xref-location-ring" (or similar) that stores the
locations visited by "xref-find-definitions", and we can allow jumping
to them with C-u - M-., in the same way as "find-tag" does.



Reply | Threaded
Open this post in threaded view
|

bug#31027: 27.0.50; xref, tags-location-ring equivalent

Robert Pluim
[hidden email] (Charles A. Roelli) writes:

> tags-location-ring seems to have no replacement in xref.el.
>
> from etags.el:
> (defvar tags-location-ring (make-ring xref-marker-ring-length)
>   "Ring of markers which are locations visited by \\[find-tag].
> Pop back to the last location with \\[negative-argument] \\[find-tag].")
>
> We should add a "xref-location-ring" (or similar) that stores the
> locations visited by "xref-find-definitions", and we can allow jumping
> to them with C-u - M-., in the same way as "find-tag" does.

xref has a marker stack. The following is bound to "M-," by default.

(defun xref-pop-marker-stack ()
  "Pop back to where \\[xref-find-definitions] was last invoked."
  (interactive)

Robert



Reply | Threaded
Open this post in threaded view
|

bug#31027: 27.0.50; xref, tags-location-ring equivalent

Charles A. Roelli
> Authentication-Results: sinyavsky.aurox.ch (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
> From: Robert Pluim <[hidden email]>
> Cc: [hidden email]
> Mail-Copies-To: never
> Gmane-Reply-To-List: yes
> Date: Tue, 03 Apr 2018 09:15:33 +0200
> Content-Type: text/plain
>
> [hidden email] (Charles A. Roelli) writes:
>
> > tags-location-ring seems to have no replacement in xref.el.
> >
> > from etags.el:
> > (defvar tags-location-ring (make-ring xref-marker-ring-length)
> >   "Ring of markers which are locations visited by \\[find-tag].
> > Pop back to the last location with \\[negative-argument] \\[find-tag].")
> >
> > We should add a "xref-location-ring" (or similar) that stores the
> > locations visited by "xref-find-definitions", and we can allow jumping
> > to them with C-u - M-., in the same way as "find-tag" does.
>
> xref has a marker stack. The following is bound to "M-," by default.
>
> (defun xref-pop-marker-stack ()
>   "Pop back to where \\[xref-find-definitions] was last invoked."
>   (interactive)

The xref marker stack (in xref--marker-ring) is a different beast: it
stores the list of places where xref-find-definitions was invoked.  I
suggest having another marker stack that tracks the list of places
jumped to by xref-find-definitions.



Reply | Threaded
Open this post in threaded view
|

bug#31027: 27.0.50; xref, tags-location-ring equivalent

Dmitry Gutov
In reply to this post by Charles A. Roelli
On 4/2/18 9:06 PM, Charles A. Roelli wrote:

> tags-location-ring seems to have no replacement in xref.el.
>
> from etags.el:
> (defvar tags-location-ring (make-ring xref-marker-ring-length)
>    "Ring of markers which are locations visited by \\[find-tag].
> Pop back to the last location with \\[negative-argument] \\[find-tag].")
>
> We should add a "xref-location-ring" (or similar) that stores the
> locations visited by "xref-find-definitions", and we can allow jumping
> to them with C-u - M-., in the same way as "find-tag" does.

What about 'M-x previous error'? Or 'C-u - M-x next-error', to mirror
your example.



Reply | Threaded
Open this post in threaded view
|

bug#31027: 27.0.50; xref, tags-location-ring equivalent

Charles A. Roelli
> From: Dmitry Gutov <[hidden email]>
> Date: Tue, 3 Apr 2018 23:32:26 +0300
>
> On 4/2/18 9:06 PM, Charles A. Roelli wrote:
> > tags-location-ring seems to have no replacement in xref.el.
> >
> > from etags.el:
> > (defvar tags-location-ring (make-ring xref-marker-ring-length)
> >    "Ring of markers which are locations visited by \\[find-tag].
> > Pop back to the last location with \\[negative-argument] \\[find-tag].")
> >
> > We should add a "xref-location-ring" (or similar) that stores the
> > locations visited by "xref-find-definitions", and we can allow jumping
> > to them with C-u - M-., in the same way as "find-tag" does.
>
> What about 'M-x previous error'? Or 'C-u - M-x next-error', to mirror
> your example.

That's useful, but also different in nature and scope to a global
ring, which would not use a local variable (`next-error-function') to
navigate to other "errors".  An "xref-location-ring" would be simpler.

By the way, I didn't know previous-error/next-error worked with xref.
Is that documented in the manual?



Reply | Threaded
Open this post in threaded view
|

bug#31027: 27.0.50; xref, tags-location-ring equivalent

Dmitry Gutov
On 4/4/18 9:37 PM, Charles A. Roelli wrote:

>> What about 'M-x previous error'? Or 'C-u - M-x next-error', to mirror
>> your example.
>
> That's useful, but also different in nature and scope to a global
> ring, which would not use a local variable (`next-error-function') to

Whether next-error-function is going to be local or not, is subject to
discussion, see https://debbugs.gnu.org/cgi/bugreport.cgi?bug=20489 and
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=30674.

> navigate to other "errors".  An "xref-location-ring" would be simpler.

What's simpler about that? You'd need some new commands to use it as
well, right?

 > By the way, I didn't know previous-error/next-error worked with xref.
 > Is that documented in the manual?

Not sure.



Reply | Threaded
Open this post in threaded view
|

bug#31027: 27.0.50; xref, tags-location-ring equivalent

Eli Zaretskii
> From: Dmitry Gutov <[hidden email]>
> Date: Wed, 4 Apr 2018 21:57:49 +0300
> Cc: [hidden email]
>
>  > By the way, I didn't know previous-error/next-error worked with xref.
>  > Is that documented in the manual?
>
> Not sure.

It is not documented, because this feature is still in flux.



Reply | Threaded
Open this post in threaded view
|

bug#31027: 27.0.50; xref, tags-location-ring equivalent

Juri Linkov-2
In reply to this post by Dmitry Gutov
>>> What about 'M-x previous error'? Or 'C-u - M-x next-error', to mirror
>>> your example.
>>
>> That's useful, but also different in nature and scope to a global
>> ring, which would not use a local variable (`next-error-function') to
>
> Whether next-error-function is going to be local or not, is subject to
> discussion, see https://debbugs.gnu.org/cgi/bugreport.cgi?bug=20489 and
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=30674.

I have a wish to close these overgrown bugreports and redesign this feature
from scratch ;)

>> navigate to other "errors".  An "xref-location-ring" would be simpler.
>
> What's simpler about that? You'd need some new commands to use it as
> well, right?

Is the idea to use a ring of next-error capable buffers?
So that the next-error command in the current buffer
will return a list of all potentially next-error capable buffers
and allow the user to select the required one.



Reply | Threaded
Open this post in threaded view
|

bug#31027: 27.0.50; xref, tags-location-ring equivalent

Dmitry Gutov
On 4/4/18 11:59 PM, Juri Linkov wrote:

>> Whether next-error-function is going to be local or not, is subject to
>> discussion, see https://debbugs.gnu.org/cgi/bugreport.cgi?bug=20489 and
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=30674.
>
> I have a wish to close these overgrown bugreports and redesign this feature
> from scratch ;)

The newer one is not so overgrown yet. I do wish we revert the part
where next-error-function is buffer local, though, and then you could
design this possibility as a user option. We discussed the approaches,
but it comes out pretty complex no matter the way you look at it.

>>> navigate to other "errors".  An "xref-location-ring" would be simpler.
>>
>> What's simpler about that? You'd need some new commands to use it as
>> well, right?
>
> Is the idea to use a ring of next-error capable buffers?
> So that the next-error command in the current buffer
> will return a list of all potentially next-error capable buffers
> and allow the user to select the required one.

Umm, I don't think the request is anything so ambitious.

Charles has been asking for a ring to store the navigation locations
visited by xref only.



Reply | Threaded
Open this post in threaded view
|

bug#31027: 27.0.50; xref, tags-location-ring equivalent

Charles A. Roelli
> From: Dmitry Gutov <[hidden email]>
> Date: Thu, 5 Apr 2018 01:14:14 +0300
>
> >> What's simpler about that? You'd need some new commands to use it as
> >> well, right?
> >
> > Is the idea to use a ring of next-error capable buffers?
> > So that the next-error command in the current buffer
> > will return a list of all potentially next-error capable buffers
> > and allow the user to select the required one.
>
> Umm, I don't think the request is anything so ambitious.
>
> Charles has been asking for a ring to store the navigation locations
> visited by xref only.

Exactly, this feature request is only about xref.  Nevertheless, the
idea of a "ring of next-error capable buffers" does sound like it
could be useful in its own right -- if anybody wants to open up a bug
for that, feel free.

Anyway, I suggest we follow the etags implementation of
"tags-location-ring".  This is all there is to it, in simplified
terms:

(defun find-tag-noselect ...
  ...
  (if (eq '- PREFIX-ARG)
        ;; Pop back to a previous location.
        (if (ring-empty-p tags-location-ring)
            (user-error "No previous tag locations")
          (let ((marker (ring-remove tags-location-ring 0)))
            (prog1
                ;; Move to the saved location.
                (set-buffer (or (marker-buffer marker)
                                (error "The marked buffer has been deleted")))
              (goto-char (marker-position marker))
              ;; Kill that marker so it doesn't slow down editing.
              (set-marker marker nil nil))))
    ;; Else, we jump to wherever we wanted to go, and record and add a
    ;; marker to tags-location-ring.
    (let ((marker (make-marker)))
      (with-current-buffer
          (find-tag-in-order ...)
        (set-marker marker (point))
        (run-hooks 'local-find-tag-hook)
        (ring-insert tags-location-ring marker)
        (current-buffer))))))



Reply | Threaded
Open this post in threaded view
|

bug#31027: 27.0.50; xref, tags-location-ring equivalent

Dmitry Gutov
On 4/5/18 9:56 PM, Charles A. Roelli wrote:
> Anyway, I suggest we follow the etags implementation of
> "tags-location-ring".  This is all there is to it, in simplified
> terms:

You mean, supporting a negative argument to xref-find-definitions? As
well as successive invocations that move between matches?

Not sure we really want to do that: after all, the xref interface
already provides the means to select among the locations. And we have
previous/next-error

More opinions welcome, of course.



Reply | Threaded
Open this post in threaded view
|

bug#31027: 27.0.50; xref, tags-location-ring equivalent

Juri Linkov-2
In reply to this post by Dmitry Gutov
>>> Whether next-error-function is going to be local or not, is subject to
>>> discussion, see https://debbugs.gnu.org/cgi/bugreport.cgi?bug=20489 and
>>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=30674.
>>
>> I have a wish to close these overgrown bugreports and redesign this feature
>> from scratch ;)
>
> The newer one is not so overgrown yet. I do wish we revert the part where
> next-error-function is buffer local, though, and then you could design this
> possibility as a user option. We discussed the approaches, but it comes out
> pretty complex no matter the way you look at it.

I'll post a request for comments to emacs-devel shortly.



Reply | Threaded
Open this post in threaded view
|

bug#31027: 27.0.50; xref, tags-location-ring equivalent

Charles A. Roelli
In reply to this post by Dmitry Gutov
> Cc: [hidden email], [hidden email]
> From: Dmitry Gutov <[hidden email]>
> Date: Fri, 6 Apr 2018 00:05:01 +0300
>
> On 4/5/18 9:56 PM, Charles A. Roelli wrote:
> > Anyway, I suggest we follow the etags implementation of
> > "tags-location-ring".  This is all there is to it, in simplified
> > terms:
>
> You mean, supporting a negative argument to xref-find-definitions?

Yes.

>     As
> well as successive invocations that move between matches?

Successive invocations wouldn't be handled specially, I think; they
would just show entries further back in the location ring.

> Not sure we really want to do that: after all, the xref interface
> already provides the means to select among the locations.

Can you clarify what you mean by this?  How can I use xref to navigate
among the N previous locations that have been jumped to?  If that
functionality already exists, then this discussion is moot.

>    And we have
> previous/next-error

It's difficult to say whether "next-error" can do what this request is
asking for, when "next-error" and its relation to xref have yet to be
defined.  From what I can see, it looks like a different ball game.
For example, the doc of "next-error-function" says it's the

  Function to use to find the next error in the current buffer.

But this request is asking for a way to navigate among the locations
jumped to in any buffer, not just one.  Additionally, there's no
guarantee that the "next' or "previous error" in the current buffer
would happen to coincide with the previous location jumped to
globally.  I don't see how those two things relate to each other,
since an "error" can be one of many different things: a compiler
error, a search result, a changed part of a buffer, or with the xref
package, a symbol definition.  The scope of "errors" is very broad.