bug#38797: 27.0.50; Feature request: provide the opposite of xref-pop-marker-stack

[Dmitry Gutov]

On 20.10.2021 22:12, Mattias Engdegård wrote:
> As usual my message was lost. Here's the patch at least.

Thanks, Mattias.

Some comments on the latest version of the patch:

- The removal of

  (defvaralias 'find-tag-marker-ring 'xref--marker-ring)

and the subsequent initialization of the aliased variable to a new ring 
will likely break any real code code which is still depending on that 
variable. Because now it is nil by default, and not a ring. Not sure 
what's the best solution, perhaps it's just time to remove it.

- The call

  (make-obsolete-variable 'xref-marker-ring-length nil "29.1")

is similarly pointless because the variable definition has been deleted. 
Declaring it obsolete does not help the fact that it is unbound now.

It could be made into an alias for the newly introduced 
tags-location-ring-length, I suppose.

Or we hold off on removing it, declare it obsolete (with some 
explanation), but keep using its alias in etags.el. That would mean 
dropping all changes in the etags.el part of the patch. But probably 
adding some 'with-suppressed-warinings' wrappers in there.

- Regarding the names of the commands, some alternatives:

* xref-rewind-history? It's a play on "fast forward". A bit cryptic, 
perhaps.
* xref-go-back and xref-go-forward? xref--history is fine the name of 
the variable, but the purpose of the commands seems obvious enough even 
without "history" in them.

No pressure. Just in case you like any of these better.

- The slots in the docstring of xref--history (should we call it 
xref--location-history?) could be called (BACK-STACK . FORWARD-STACK).

- xref-set-marker-ring-length's fate is changed accordingly to what we 
choose to do with xref-marker-ring-length.

- The FIXME about proposed xref--back-history-empty-p doesn't make much 
sense if the introduced function name is going to be private (we don't 
need private functions that aren't used in the package itself). 
xref-back-history-empty-p and xref-forward-history-empty-p could work. 
But we can keep xref-marker-stack-empty-p around, pending feature 
requests. Maybe add a boolean optional FORWARD argument. Or rename it to 
xref-history-empty-p, for instance (with the optional FORWARD argument).