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

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.

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

[Robert Pluim]

charles@aurox.ch (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

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 <rpluim@gmail.com>
> Cc: 31027@debbugs.gnu.org
> Mail-Copies-To: never
> Gmane-Reply-To-List: yes
> Date: Tue, 03 Apr 2018 09:15:33 +0200
> Content-Type: text/plain
> 
> charles@aurox.ch (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.

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

[Dmitry Gutov]

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.

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

[Charles A. Roelli]

> From: Dmitry Gutov <dgutov@yandex.ru>
> 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?

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.

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

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 4 Apr 2018 21:57:49 +0300
> Cc: 31027@debbugs.gnu.org
> 
>  > 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.

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

[Juri Linkov]

>>> 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.

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.

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

[Charles A. Roelli]

> From: Dmitry Gutov <dgutov@yandex.ru>
> 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))))))

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.

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

[Juri Linkov]

>>> 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.

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

[Charles A. Roelli]

> Cc: 31027@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> 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.