bug#59381: Should xref--marker-ring be per-window?

bug#59381: Should xref--marker-ring be per-window?

[Ackerley Tng]

Hi,

While using xref (xref-find-definitions and xref-find-references, etc)
in separate windows, I hit M-, to pop the marker stack expecting that
each window should have a separate stack.

However, it seems that the entire emacs instance shares a single
xref--marker ring.

Could we have separate xref--marker-rings per window, or some
configuration option to enable that?

I have some patches that I could submit, if that helps!

Thanks,

Ackerley

bug#59381: Should xref--marker-ring be per-window?

[Juri Linkov]

> While using xref (xref-find-definitions and xref-find-references, etc)
> in separate windows, I hit M-, to pop the marker stack expecting that
> each window should have a separate stack.
>
> However, it seems that the entire emacs instance shares a single
> xref--marker ring.
>
> Could we have separate xref--marker-rings per window, or some
> configuration option to enable that?

Ideally, making an existing variable window-local should be as easy
as making it buffer-local, e.g.:

  (make-variable-buffer-local 'xref--history)
  ->
  (make-variable-window-local 'xref--history)

But we are not here so far.

bug#59381: Should xref--marker-ring be per-window?

[Eli Zaretskii]

> Cc: 59381@debbugs.gnu.org
> From: Juri Linkov <juri@linkov.net>
> Date: Sat, 19 Nov 2022 20:53:46 +0200
> 
> > While using xref (xref-find-definitions and xref-find-references, etc)
> > in separate windows, I hit M-, to pop the marker stack expecting that
> > each window should have a separate stack.
> >
> > However, it seems that the entire emacs instance shares a single
> > xref--marker ring.
> >
> > Could we have separate xref--marker-rings per window, or some
> > configuration option to enable that?
> 
> Ideally, making an existing variable window-local should be as easy
> as making it buffer-local, e.g.:
> 
>   (make-variable-buffer-local 'xref--history)
>   ->
>   (make-variable-window-local 'xref--history)
> 
> But we are not here so far.

I don't think it's that simple.  Windows are much more ephemeral than buffers;
for example, "C-x 2" followed by "C-x 1" or "C-x 0" deletes one of the windows.
What do we want to happen with the "window-local" xref stack in that case?

My guess is that the OP wanted to have control on when M-. pushes locations to
the last used stack or begin a new stack.  Because only the user knows when
M-. begins a new series of searches.  So I think it is better to offer a
separate command for exercising just such control.

bug#59381: Should xref--marker-ring be per-window?

[Ackerley Tng]

[-- Attachment #1: Type: text/plain, Size: 1492 bytes --]

What if we copy the whole stackv from the old window whenever a new window
is opened?

On Sat, Nov 19, 2022, 11:53 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > Cc: 59381@debbugs.gnu.org
> > From: Juri Linkov <juri@linkov.net>
> > Date: Sat, 19 Nov 2022 20:53:46 +0200
> >
> > > While using xref (xref-find-definitions and xref-find-references, etc)
> > > in separate windows, I hit M-, to pop the marker stack expecting that
> > > each window should have a separate stack.
> > >
> > > However, it seems that the entire emacs instance shares a single
> > > xref--marker ring.
> > >
> > > Could we have separate xref--marker-rings per window, or some
> > > configuration option to enable that?
> >
> > Ideally, making an existing variable window-local should be as easy
> > as making it buffer-local, e.g.:
> >
> >   (make-variable-buffer-local 'xref--history)
> >   ->
> >   (make-variable-window-local 'xref--history)
> >
> > But we are not here so far.
>
> I don't think it's that simple.  Windows are much more ephemeral than
> buffers;
> for example, "C-x 2" followed by "C-x 1" or "C-x 0" deletes one of the
> windows.
> What do we want to happen with the "window-local" xref stack in that case?
>
> My guess is that the OP wanted to have control on when M-. pushes
> locations to
> the last used stack or begin a new stack.  Because only the user knows when
> M-. begins a new series of searches.  So I think it is better to offer a
> separate command for exercising just such control.
>

[-- Attachment #2: Type: text/html, Size: 2165 bytes --]

bug#59381: Should xref--marker-ring be per-window?

[Dmitry Gutov]

On 19.11.2022 21:53, Eli Zaretskii wrote:
>> Ideally, making an existing variable window-local should be as easy
>> as making it buffer-local, e.g.:
>>
>>    (make-variable-buffer-local 'xref--history)
>>    ->
>>    (make-variable-window-local 'xref--history)
>>
>> But we are not here so far.
> I don't think it's that simple.  Windows are much more ephemeral than buffers;
> for example, "C-x 2" followed by "C-x 1" or "C-x 0" deletes one of the windows.
> What do we want to happen with the "window-local" xref stack in that case?

Poof. Not sure what else we could do.

> My guess is that the OP wanted to have control on when M-. pushes locations to
> the last used stack or begin a new stack.  Because only the user knows when
> M-. begins a new series of searches.  So I think it is better to offer a
> separate command for exercising just such control.

As previously mentioned in 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38797#8, I personally find 
it perfectly usable to always use window-local stacks.

But maybe it will be helpful for you to elaborate: what the workflow 
would look like. Would it be a parallel set of commands, or simply a 
command to... do what?

In my workflow, a new stack is more or less created implicitly by 
splitting a window, and discarded by deleting one. The older stacks can 
get forgotten, but while the locations are fresh in mind, this behavior 
feels logical: it *feels* that I did that chain of navigations in one 
window, and another in the other one. And I can jump back and forward in 
each one in parallel.

I suppose it doesn't work as well when commands pop new windows a lot, 
but luckily M-. doesn't do that too often.

bug#59381: Should xref--marker-ring be per-window?

[Eli Zaretskii]

> From: Ackerley Tng <ackerleytng@gmail.com>
> Date: Sat, 19 Nov 2022 14:01:52 -0800
> Cc: Juri Linkov <juri@linkov.net>, 59381@debbugs.gnu.org
> 
> What if we copy the whole stackv from the old window whenever a new window is opened?

What will happen with that if you switch to another window which displays
the same file, or delete the window where the stack is kept?

And please note that results of creation and deletion of windows are not
always predictable from the user POV.  E.g., when you type "C-x 2", do you
always know which of the two windows will keep the ID of the original single
window?

bug#59381: Should xref--marker-ring be per-window?

[Eli Zaretskii]

> Date: Sun, 20 Nov 2022 04:52:38 +0200
> Cc: ackerleytng@gmail.com, 59381@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> > My guess is that the OP wanted to have control on when M-. pushes locations to
> > the last used stack or begin a new stack.  Because only the user knows when
> > M-. begins a new series of searches.  So I think it is better to offer a
> > separate command for exercising just such control.
> 
> As previously mentioned in 
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38797#8, I personally find 
> it perfectly usable to always use window-local stacks.
> 
> But maybe it will be helpful for you to elaborate: what the workflow 
> would look like. Would it be a parallel set of commands, or simply a 
> command to... do what?

I just did that, above: add a command that starts a new "stack".  All the
rest is unchanged.

> In my workflow, a new stack is more or less created implicitly by 
> splitting a window, and discarded by deleting one.

So you always ever have a given buffer displayed in a single window?  Does
it ever happen to you that you need to work on one portion of a file while
looking at its another portion? or work on one file while look at another
file in a sibling window?  If you ever need to do these, and both windows
show files that belong to the same "editing activity", why would the stack
be local to a window?  That would effectively designate a single window as
the only one where M-. and M-, will do what you expect, no?

> The older stacks can get forgotten, but while the locations are fresh in
> mind, this behavior feels logical: it *feels* that I did that chain of
> navigations in one window, and another in the other one. And I can jump
> back and forward in each one in parallel.

But not if you switch windows?

> I suppose it doesn't work as well when commands pop new windows a lot, 
> but luckily M-. doesn't do that too often.

In your experience, maybe.  In Emacs we have macros like FOO_BAR that call
functions named foo_bar, and then M-. always pops up a new window.  Likewise
with macros or data structures that have several different definitions
depending on the window-system backend (X, w32, NS, etc.).

The use cases I described don't work well with window-local stacks.  So if
an explicit command as I envisioned is deemed an annoyance, perhaps a user
option which will allow one or the other workflow is in order?

bug#59381: Should xref--marker-ring be per-window?

[Dmitry Gutov]

On 20.11.2022 09:09, Eli Zaretskii wrote:
>> From: Ackerley Tng<ackerleytng@gmail.com>
>> Date: Sat, 19 Nov 2022 14:01:52 -0800
>> Cc: Juri Linkov<juri@linkov.net>,59381@debbugs.gnu.org
>>
>> What if we copy the whole stackv from the old window whenever a new window is opened?
> What will happen with that if you switch to another window which displays
> the same file, or delete the window where the stack is kept?
> 
> And please note that results of creation and deletion of windows are not
> always predictable from the user POV.  E.g., when you type "C-x 2", do you
> always know which of the two windows will keep the ID of the original single
> window?

If the stack is copied, isn't that a non-issue? Both windows get the 
same history, and then their identities will be tired to the positions 
on the screen, that's how the user will recognize them.

And FWIW, in my personal config the stack isn't even copied. Somehow 
that works out fine, with one small (but potentially significant) caveat 
that in my config 'C-x 2' and 'C-x 3' always select the new window. 
Making it obvious which of the windows in new, and thus isn't expected 
to have existing history.

bug#59381: Should xref--marker-ring be per-window?

[Eli Zaretskii]

> Date: Sun, 20 Nov 2022 19:00:49 +0200
> Cc: 59381@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 20.11.2022 09:09, Eli Zaretskii wrote:
> >> From: Ackerley Tng<ackerleytng@gmail.com>
> >> Date: Sat, 19 Nov 2022 14:01:52 -0800
> >> Cc: Juri Linkov<juri@linkov.net>,59381@debbugs.gnu.org
> >>
> >> What if we copy the whole stackv from the old window whenever a new window is opened?
> > What will happen with that if you switch to another window which displays
> > the same file, or delete the window where the stack is kept?
> > 
> > And please note that results of creation and deletion of windows are not
> > always predictable from the user POV.  E.g., when you type "C-x 2", do you
> > always know which of the two windows will keep the ID of the original single
> > window?
> 
> If the stack is copied, isn't that a non-issue?

It is, provided that copying is indeed what the user wants.  But how can we
know?  A new window can be completely unrelated.

And I'm more worried by window deletion than by creation.

> And FWIW, in my personal config the stack isn't even copied. Somehow 
> that works out fine, with one small (but potentially significant) caveat 
> that in my config 'C-x 2' and 'C-x 3' always select the new window. 
> Making it obvious which of the windows in new, and thus isn't expected 
> to have existing history.

I don't see how this could work reliably enough.  Maybe I'm missing
something.

bug#59381: Should xref--marker-ring be per-window?

[Ackerley Tng]

On Sun, Nov 20, 2022 at 9:32 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > Date: Sun, 20 Nov 2022 19:00:49 +0200
> > Cc: 59381@debbugs.gnu.org, juri@linkov.net
> > From: Dmitry Gutov <dgutov@yandex.ru>
> >
> > On 20.11.2022 09:09, Eli Zaretskii wrote:
> > >> From: Ackerley Tng<ackerleytng@gmail.com>
> > >> Date: Sat, 19 Nov 2022 14:01:52 -0800
> > >> Cc: Juri Linkov<juri@linkov.net>,59381@debbugs.gnu.org
> > >>
> > >> What if we copy the whole stackv from the old window whenever a new window is opened?
> > > What will happen with that if you switch to another window which displays
> > > the same file, or delete the window where the stack is kept?
> > >
> > > And please note that results of creation and deletion of windows are not
> > > always predictable from the user POV.  E.g., when you type "C-x 2", do you
> > > always know which of the two windows will keep the ID of the original single
> > > window?
> >
> > If the stack is copied, isn't that a non-issue?
>
> It is, provided that copying is indeed what the user wants.  But how can we
> know?  A new window can be completely unrelated.
>
> And I'm more worried by window deletion than by creation.

I think Eli brought up a good point about splitting windows. Here's
the situation that is a little sticky.

1. User builds the stack up to buffer A in window 1
2. User creates a new window, so now we have buffer A in window 1 and
buffer A in window 2
3. User builds up the stack further in window 2, AND we have the
feature to navigate forwards as eli brought up, in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38797#8
4. In window 2, user navigates backwards so that now both window 1 and
2 are showing buffer A
5. User might close window 2, either confused about which stack was in
window 1 and window 2, or assuming that window 1 and 2 have the same
stacks.

Closing window 2 would be bad since the user would lose the go-forward
history. However, I feel that if the window position on screen remains
consistent, the user would be able to remember where they were
navigating, and I think they would close the right window. I feel that
for most cases this wouldn't be a problem.

I think Eli is right in saying that we will never know when the user
wants to fork a navigation stack. My issue with an explicit command is
that I would probably forget to initiate a new stack when I need to,
and by time I realize it, it would be too late to start/copy the
stack.

Also, in this new explicit command, the user will have to specify the
window to copy the stack from, and the window to copy the stack to,
which is quite a lot of steps, which would get in the way of wanting
to navigate code.

What does everyone think if we do the baseline of just creating a new
stack per-window (no copying) and then waiting to see if we get
feedback once people start using it?

I'm new to development of emacs itself, since in the debbugs link the
bug was closed and the feature pushed, are we expecting this feature
in a future version of emacs?

bug#59381: Should xref--marker-ring be per-window?

[Eli Zaretskii]

> From: Ackerley Tng <ackerleytng@gmail.com>
> Date: Sun, 20 Nov 2022 10:11:30 -0800
> Cc: Dmitry Gutov <dgutov@yandex.ru>, 59381@debbugs.gnu.org, juri@linkov.net
> 
> What does everyone think if we do the baseline of just creating a new
> stack per-window (no copying) and then waiting to see if we get
> feedback once people start using it?

If this is optional and can be disabled, I don't think I mind.

bug#59381: Should xref--marker-ring be per-window?

[Dmitry Gutov]

On 20.11.2022 20:22, Eli Zaretskii wrote:
>> From: Ackerley Tng<ackerleytng@gmail.com>
>> Date: Sun, 20 Nov 2022 10:11:30 -0800
>> Cc: Dmitry Gutov<dgutov@yandex.ru>,59381@debbugs.gnu.org,juri@linkov.net
>>
>> What does everyone think if we do the baseline of just creating a new
>> stack per-window (no copying) and then waiting to see if we get
>> feedback once people start using it?
> If this is optional and can be disabled, I don't think I mind.

Definitely optional, we should provide different "storages" for this. 
Global (the current behavior), per-window, per-frame, maybe others.

bug#59381: Should xref--marker-ring be per-window?

[Dmitry Gutov]

On 20.11.2022 09:59, Eli Zaretskii wrote:

>> But maybe it will be helpful for you to elaborate: what the workflow
>> would look like. Would it be a parallel set of commands, or simply a
>> command to... do what?
> 
> I just did that, above: add a command that starts a new "stack".  All the
> rest is unchanged.

What would happen with the current stack, though? Does it get "stashed" 
at the top of "stack of stacks", switching the current global stack 
entirely? Until we decide to "pop" to the previous stack, that is 
(another new command).

Or does it apply to the current window? What about the windows split 
from it? What about older windows we decide to pop-to-buffer to from one 
of the new windows?

You make some good points down below, I just don't see how a new command 
is going to solve the raised issues entirely.

>> In my workflow, a new stack is more or less created implicitly by
>> splitting a window, and discarded by deleting one.
> 
> So you always ever have a given buffer displayed in a single window?

Not necessarily, no. If it's a big file, I can have two parallel 
"investigations" going on in two different window on it. Using two 
different navigation stacks. That's a feature.

> Does
> it ever happen to you that you need to work on one portion of a file while
> looking at its another portion? or work on one file while look at another
> file in a sibling window?

Sure.

> If you ever need to do these, and both windows
> show files that belong to the same "editing activity", why would the stack
> be local to a window?  That would effectively designate a single window as
> the only one where M-. and M-, will do what you expect, no?

More-or-less-ish.

>> The older stacks can get forgotten, but while the locations are fresh in
>> mind, this behavior feels logical: it *feels* that I did that chain of
>> navigations in one window, and another in the other one. And I can jump
>> back and forward in each one in parallel.
> 
> But not if you switch windows?

Yup.

>> I suppose it doesn't work as well when commands pop new windows a lot,
>> but luckily M-. doesn't do that too often.
> 
> In your experience, maybe.  In Emacs we have macros like FOO_BAR that call
> functions named foo_bar, and then M-. always pops up a new window.  Likewise
> with macros or data structures that have several different definitions
> depending on the window-system backend (X, w32, NS, etc.).

Whether M-. pop a new window, or you use project-find-regexp, we usually 
make sure that after you navigate to a location, it's displayed in the 
same window the search was made in. Unless the user called something 
like xref-find-definitions-other-window, naturally.

So it's generally possible to stay within the same window most of the time.

> The use cases I described don't work well with window-local stacks.  So if
> an explicit command as I envisioned is deemed an annoyance, perhaps a user
> option which will allow one or the other workflow is in order?

Sure, it should be optional. I'd like to ensure that the new workflow 
can be helpful for as many users as possible nevertheless.

And you make good points: Emacs often makes you go from a window to a 
window, reusing older windows as well. So I'm not sure how to solve that 
better: searching the window hierarchy won't help.

So it could be some propagation mechanism working when windows are split 
or buffers get displayed, which would nevertheless leave a window when 
the user pressed 'q', for instance. Reverting the window to its previous 
stack, let's say. And as for separate command, using it explicitly by 
itself is easy to forget, but it perhaps could be added to some other 
commands by the user (via before-advice or etc), to mark the beginning 
of each stack.

This is a very rough idea. There's nobody to work on it in the near 
future, I'm afraid, so adding an optional change in behavior to use 
window-local storage is probably the best way forward. To get feedback, 
as Ackerley said.

bug#59381: Should xref--marker-ring be per-window?

[Juri Linkov]

>>> What does everyone think if we do the baseline of just creating a new
>>> stack per-window (no copying) and then waiting to see if we get
>>> feedback once people start using it?
>> If this is optional and can be disabled, I don't think I mind.
>
> Definitely optional, we should provide different "storages" for
> this. Global (the current behavior), per-window, per-frame, maybe others.

Also per-project (maybe even by default).

PS: this whole subject of different xref stacks reminds me
trying to make sense of the different next-error stacks ;-)

bug#59381: Should xref--marker-ring be per-window?

[Eli Zaretskii]

> Date: Mon, 21 Nov 2022 01:17:02 +0200
> Cc: 59381@debbugs.gnu.org, ackerleytng@gmail.com, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 20.11.2022 09:59, Eli Zaretskii wrote:
> 
> >> But maybe it will be helpful for you to elaborate: what the workflow
> >> would look like. Would it be a parallel set of commands, or simply a
> >> command to... do what?
> > 
> > I just did that, above: add a command that starts a new "stack".  All the
> > rest is unchanged.
> 
> What would happen with the current stack, though?

It's discarded, as no longer needed.

> Or does it apply to the current window? What about the windows split 
> from it? What about older windows we decide to pop-to-buffer to from one 
> of the new windows?

In my mental picture, the stack is not specific to a window, like it is
today.

> >> In my workflow, a new stack is more or less created implicitly by
> >> splitting a window, and discarded by deleting one.
> > 
> > So you always ever have a given buffer displayed in a single window?
> 
> Not necessarily, no. If it's a big file, I can have two parallel 
> "investigations" going on in two different window on it. Using two 
> different navigation stacks. That's a feature.

It's a feature if you indeed want a separate stack in each window.  What if
you want the same stack in all of those windows?

> Whether M-. pop a new window, or you use project-find-regexp, we usually 
> make sure that after you navigate to a location, it's displayed in the 
> same window the search was made in. Unless the user called something 
> like xref-find-definitions-other-window, naturally.
> 
> So it's generally possible to stay within the same window most of the time.

Not if I split that one window because I want to look at something else as
well.

> And you make good points: Emacs often makes you go from a window to a 
> window, reusing older windows as well. So I'm not sure how to solve that 
> better: searching the window hierarchy won't help.
> 
> So it could be some propagation mechanism working when windows are split 
> or buffers get displayed, which would nevertheless leave a window when 
> the user pressed 'q', for instance. Reverting the window to its previous 
> stack, let's say. And as for separate command, using it explicitly by 
> itself is easy to forget, but it perhaps could be added to some other 
> commands by the user (via before-advice or etc), to mark the beginning 
> of each stack.
> 
> This is a very rough idea. There's nobody to work on it in the near 
> future, I'm afraid, so adding an optional change in behavior to use 
> window-local storage is probably the best way forward. To get feedback, 
> as Ackerley said.

When this becomes practical, we could try it and see if enough people like
it.

bug#59381: Should xref--marker-ring be per-window?

[Ackerley Tng]

[-- Attachment #1: Type: text/plain, Size: 3112 bytes --]

Here's a patch for review!

I made 'window-local the default storage so that we would hopefully
get more feedback, do let me know if I should leave the default as
'global.

On Mon, Nov 21, 2022 at 5:14 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > Date: Mon, 21 Nov 2022 01:17:02 +0200
> > Cc: 59381@debbugs.gnu.org, ackerleytng@gmail.com, juri@linkov.net
> > From: Dmitry Gutov <dgutov@yandex.ru>
> >
> > On 20.11.2022 09:59, Eli Zaretskii wrote:
> >
> > >> But maybe it will be helpful for you to elaborate: what the workflow
> > >> would look like. Would it be a parallel set of commands, or simply a
> > >> command to... do what?
> > >
> > > I just did that, above: add a command that starts a new "stack".  All the
> > > rest is unchanged.
> >
> > What would happen with the current stack, though?
>
> It's discarded, as no longer needed.
>
> > Or does it apply to the current window? What about the windows split
> > from it? What about older windows we decide to pop-to-buffer to from one
> > of the new windows?
>
> In my mental picture, the stack is not specific to a window, like it is
> today.
>
> > >> In my workflow, a new stack is more or less created implicitly by
> > >> splitting a window, and discarded by deleting one.
> > >
> > > So you always ever have a given buffer displayed in a single window?
> >
> > Not necessarily, no. If it's a big file, I can have two parallel
> > "investigations" going on in two different window on it. Using two
> > different navigation stacks. That's a feature.
>
> It's a feature if you indeed want a separate stack in each window.  What if
> you want the same stack in all of those windows?
>
> > Whether M-. pop a new window, or you use project-find-regexp, we usually
> > make sure that after you navigate to a location, it's displayed in the
> > same window the search was made in. Unless the user called something
> > like xref-find-definitions-other-window, naturally.
> >
> > So it's generally possible to stay within the same window most of the time.
>
> Not if I split that one window because I want to look at something else as
> well.
>
> > And you make good points: Emacs often makes you go from a window to a
> > window, reusing older windows as well. So I'm not sure how to solve that
> > better: searching the window hierarchy won't help.
> >
> > So it could be some propagation mechanism working when windows are split
> > or buffers get displayed, which would nevertheless leave a window when
> > the user pressed 'q', for instance. Reverting the window to its previous
> > stack, let's say. And as for separate command, using it explicitly by
> > itself is easy to forget, but it perhaps could be added to some other
> > commands by the user (via before-advice or etc), to mark the beginning
> > of each stack.
> >
> > This is a very rough idea. There's nobody to work on it in the near
> > future, I'm afraid, so adding an optional change in behavior to use
> > window-local storage is probably the best way forward. To get feedback,
> > as Ackerley said.
>
> When this becomes practical, we could try it and see if enough people like
> it.

[-- Attachment #2: 0001-Add-support-for-window-local-xref-history.patch --]
[-- Type: text/x-patch, Size: 6361 bytes --]

From 3297a0ff016394dbb775caeb194d15c754a238dc Mon Sep 17 00:00:00 2001
From: Ackerley Tng <ackerleytng@google.com>
Date: Mon, 21 Nov 2022 18:38:03 -0800
Subject: [PATCH] Add support for window-local xref history

---
 lisp/progmodes/xref.el | 106 ++++++++++++++++++++++++++++-------------
 1 file changed, 74 insertions(+), 32 deletions(-)

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 89a090ae93..122bf55541 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -427,32 +427,71 @@ xref-auto-jump-to-first-xref
   :version "28.1"
   :package-version '(xref . "1.2.0"))
 
+(defcustom xref-storage-type 'window-local
+  "How xref history is stored.
+
+Before Emacs 29.1, xref history is stored at the global level, so
+the same history is used across the entire Emacs instance.
+
+In Emacs 29.1 the new default is to have one xref history per
+window, which allows you to navigate code independently in
+different windows.
+
+A new xref history is created for every new window."
+  :type '(choice
+          (const :tag "Per-window" window-local)
+          (const :tag "Global history for Emacs instance" global))
+  :version "29.1"
+  :package-version '(xref . "1.5.1"))
+
 (make-obsolete-variable 'xref--marker-ring 'xref--history "29.1")
 
 (defun xref-set-marker-ring-length (_var _val)
   (declare (obsolete nil "29.1"))
   nil)
 
-(defvar xref--history (cons nil nil)
+(defun xref--make-xref-history ()
+  "Return a new xref history."
+  (cons nil nil))
+
+(defvar xref--history (xref--make-xref-history)
   "(BACKWARD-STACK . FORWARD-STACK) of markers to visited Xref locations.")
 
+(defun xref--get-or-create-window-xref-history ()
+  "Return the xref history for the selected window.
+
+Create an xref history and return it if it did not already exist."
+  (let ((w (selected-window)))
+    (if-let ((r (window-parameter w 'xref--window-xref-history))) r
+      (let ((h (xref--make-xref-history)))
+        (set-window-parameter w 'xref--window-xref-history h)))))
+
+(defun xref--get-history ()
+  "Return xref history based on `xref-storage-type'."
+  (cl-case xref-storage-type
+    (window-local (xref--get-or-create-window-xref-history))
+    (global xref--history)))
+
 (defun xref--push-backward (m)
   "Push marker M onto the backward history stack."
-  (unless (equal m (caar xref--history))
-    (push m (car xref--history))))
+  (let ((history (xref--get-history)))
+    (unless (equal m (caar history))
+      (push m (car history)))))
 
 (defun xref--push-forward (m)
   "Push marker M onto the forward history stack."
-  (unless (equal m (cadr xref--history))
-    (push m (cdr xref--history))))
+  (let ((history (xref--get-history)))
+    (unless (equal m (cadr history))
+      (push m (cdr history)))))
 
 (defun xref-push-marker-stack (&optional m)
   "Add point M (defaults to `point-marker') to the marker stack.
 The future stack is erased."
   (xref--push-backward (or m (point-marker)))
-  (dolist (mk (cdr xref--history))
-    (set-marker mk nil nil))
-  (setcdr xref--history nil))
+  (let ((history (xref--get-history)))
+    (dolist (mk (cdr history))
+      (set-marker mk nil nil))
+    (setcdr history nil)))
 
 ;;;###autoload
 (define-obsolete-function-alias 'xref-pop-marker-stack #'xref-go-back "29.1")
@@ -462,29 +501,31 @@ xref-go-back
   "Go back to the previous position in xref history.
 To undo, use \\[xref-go-forward]."
   (interactive)
-  (if (null (car xref--history))
-      (user-error "At start of xref history")
-    (let ((marker (pop (car xref--history))))
-      (xref--push-forward (point-marker))
-      (switch-to-buffer (or (marker-buffer marker)
-                            (user-error "The marked buffer has been deleted")))
-      (goto-char (marker-position marker))
-      (set-marker marker nil nil)
-      (run-hooks 'xref-after-return-hook))))
+  (let ((history (xref--get-history)))
+    (if (null (car history))
+        (user-error "At start of xref history")
+      (let ((marker (pop (car history))))
+        (xref--push-forward (point-marker))
+        (switch-to-buffer (or (marker-buffer marker)
+                              (user-error "The marked buffer has been deleted")))
+        (goto-char (marker-position marker))
+        (set-marker marker nil nil)
+        (run-hooks 'xref-after-return-hook)))))
 
 ;;;###autoload
 (defun xref-go-forward ()
   "Got to the point where a previous \\[xref-go-back] was invoked."
   (interactive)
-  (if (null (cdr xref--history))
-      (user-error "At end of xref history")
-    (let ((marker (pop (cdr xref--history))))
-      (xref--push-backward (point-marker))
-      (switch-to-buffer (or (marker-buffer marker)
-                            (user-error "The marked buffer has been deleted")))
-      (goto-char (marker-position marker))
-      (set-marker marker nil nil)
-      (run-hooks 'xref-after-return-hook))))
+  (let ((history (xref--get-history)))
+    (if (null (cdr history))
+        (user-error "At end of xref history")
+      (let ((marker (pop (cdr history))))
+        (xref--push-backward (point-marker))
+        (switch-to-buffer (or (marker-buffer marker)
+                              (user-error "The marked buffer has been deleted")))
+        (goto-char (marker-position marker))
+        (set-marker marker nil nil)
+        (run-hooks 'xref-after-return-hook)))))
 
 (define-obsolete-variable-alias
   'xref--current-item
@@ -510,22 +551,23 @@ xref-pulse-momentarily
 ;; etags.el needs this
 (defun xref-clear-marker-stack ()
   "Discard all markers from the xref history."
-  (dolist (l (list (car xref--history) (cdr xref--history)))
-    (dolist (m l)
-      (set-marker m nil nil)))
-  (setq xref--history (cons nil nil))
+  (let ((history (xref--get-history)))
+    (dolist (l (list (car history) (cdr history)))
+      (dolist (m l)
+        (set-marker m nil nil)))
+    (setq history (cons nil nil)))
   nil)
 
 ;;;###autoload
 (defun xref-marker-stack-empty-p ()
   "Whether the xref back-history is empty."
-  (null (car xref--history)))
+  (null (car (xref--get-history))))
 ;; FIXME: rename this to `xref-back-history-empty-p'.
 
 ;;;###autoload
 (defun xref-forward-history-empty-p ()
   "Whether the xref forward-history is empty."
-  (null (cdr xref--history)))
+  (null (cdr (xref--get-history))))
 \f
 
 (defun xref--goto-char (pos)
-- 
2.38.1

bug#59381: Should xref--marker-ring be per-window?

[Dmitry Gutov]

On 21/11/22 09:42, Juri Linkov wrote:
>>>> What does everyone think if we do the baseline of just creating a new
>>>> stack per-window (no copying) and then waiting to see if we get
>>>> feedback once people start using it?
>>> If this is optional and can be disabled, I don't think I mind.
>>
>> Definitely optional, we should provide different "storages" for
>> this. Global (the current behavior), per-window, per-frame, maybe others.
> 
> Also per-project (maybe even by default).

I don't know whether I like that as default: some navigations could jump 
across projects, it would be weird to break the stacks along those 
boundaries.

> PS: this whole subject of different xref stacks reminds me
> trying to make sense of the different next-error stacks ;-)

Definitely. ;-)

bug#59381: Should xref--marker-ring be per-window?

[Dmitry Gutov]

On 21/11/22 15:14, Eli Zaretskii wrote:
>> Date: Mon, 21 Nov 2022 01:17:02 +0200
>> Cc: 59381@debbugs.gnu.org, ackerleytng@gmail.com, juri@linkov.net
>> From: Dmitry Gutov <dgutov@yandex.ru>
>>
>> On 20.11.2022 09:59, Eli Zaretskii wrote:
>>
>>>> But maybe it will be helpful for you to elaborate: what the workflow
>>>> would look like. Would it be a parallel set of commands, or simply a
>>>> command to... do what?
>>>
>>> I just did that, above: add a command that starts a new "stack".  All the
>>> rest is unchanged.
>>
>> What would happen with the current stack, though?
> 
> It's discarded, as no longer needed.

That sounds odd. The idea regarding windows is about keeping multiple 
stacks at the same time, not about discarding information.

>> Or does it apply to the current window? What about the windows split
>> from it? What about older windows we decide to pop-to-buffer to from one
>> of the new windows?
> 
> In my mental picture, the stack is not specific to a window, like it is
> today.
> 
>>>> In my workflow, a new stack is more or less created implicitly by
>>>> splitting a window, and discarded by deleting one.
>>>
>>> So you always ever have a given buffer displayed in a single window?
>>
>> Not necessarily, no. If it's a big file, I can have two parallel
>> "investigations" going on in two different window on it. Using two
>> different navigation stacks. That's a feature.
> 
> It's a feature if you indeed want a separate stack in each window.  What if
> you want the same stack in all of those windows?

Maybe you never do? Or if you really do, that would require some 
additional manual management (through new commands, I suppose).

>> Whether M-. pop a new window, or you use project-find-regexp, we usually
>> make sure that after you navigate to a location, it's displayed in the
>> same window the search was made in. Unless the user called something
>> like xref-find-definitions-other-window, naturally.
>>
>> So it's generally possible to stay within the same window most of the time.
> 
> Not if I split that one window because I want to look at something else as
> well.

In my book that's starting a new line of thought, where it's okay to 
create a new stack. The old one is still around.

>> And you make good points: Emacs often makes you go from a window to a
>> window, reusing older windows as well. So I'm not sure how to solve that
>> better: searching the window hierarchy won't help.
>>
>> So it could be some propagation mechanism working when windows are split
>> or buffers get displayed, which would nevertheless leave a window when
>> the user pressed 'q', for instance. Reverting the window to its previous
>> stack, let's say. And as for separate command, using it explicitly by
>> itself is easy to forget, but it perhaps could be added to some other
>> commands by the user (via before-advice or etc), to mark the beginning
>> of each stack.
>>
>> This is a very rough idea. There's nobody to work on it in the near
>> future, I'm afraid, so adding an optional change in behavior to use
>> window-local storage is probably the best way forward. To get feedback,
>> as Ackerley said.
> 
> When this becomes practical, we could try it and see if enough people like
> it.

I don't know if it's practical or not, but it requires some additional 
design for sure. Maybe someday.

bug#59381: Should xref--marker-ring be per-window?

[Dmitry Gutov]

On 22/11/22 04:46, Ackerley Tng wrote:
> Here's a patch for review!

It's reasonable, but what if we turn xref-storage-type into a variable 
that gets set to a function? One that knows how to retrieve and set the 
value. E.g.:

(defcustom xref-storage-type 'xref-window-local-history
   ...)

(defun xref-window-local-history (&optional new-value)
   (let ((w (selected-window)))
     (if new-value
         (set-window-parameter w 'xref--history new-value)
       (or (window-parameter w 'xref--history)
           (cons nil nil)))))

(defun xref-global-history (&optional new-value)
   (if new-value
       (setq xref--history new-value)
     xref--history))

Then it will be trivial to extend with new storage mechanisms.

> I made 'window-local the default storage so that we would hopefully
> get more feedback, do let me know if I should leave the default as
> 'global.

That's not how we introduce changes here, with rare exceptions.

window-local storage is going to be disruptive (I'm fairly sure), so 
let's keep the current behavior as default.

bug#59381: Should xref--marker-ring be per-window?

[Eli Zaretskii]

> Date: Thu, 24 Nov 2022 05:19:22 +0200
> Cc: 59381@debbugs.gnu.org, ackerleytng@gmail.com, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> >>>> But maybe it will be helpful for you to elaborate: what the workflow
> >>>> would look like. Would it be a parallel set of commands, or simply a
> >>>> command to... do what?
> >>>
> >>> I just did that, above: add a command that starts a new "stack".  All the
> >>> rest is unchanged.
> >>
> >> What would happen with the current stack, though?
> > 
> > It's discarded, as no longer needed.
> 
> That sounds odd. The idea regarding windows is about keeping multiple 
> stacks at the same time, not about discarding information.

My idea is not about windows, though.  It's about a workflow that resembles
searches: you keep searching for the same or similar strings as long as you
are interested in a particular string/regexp; as long as you do that, using
"C-s C-s" to repeat search, perhaps with minor edits of the search string,
is what you want.  Then, when you want another search, you discard the
previous search string and start with a completely new one.

> >>> So you always ever have a given buffer displayed in a single window?
> >>
> >> Not necessarily, no. If it's a big file, I can have two parallel
> >> "investigations" going on in two different window on it. Using two
> >> different navigation stacks. That's a feature.
> > 
> > It's a feature if you indeed want a separate stack in each window.  What if
> > you want the same stack in all of those windows?
> 
> Maybe you never do? Or if you really do, that would require some 
> additional manual management (through new commands, I suppose).

I do that sometimes, not to rarely to remember it as a feature.  That's why
I suggested an explicit command, because I don't think Emacs can guess my
intentions in this case.

bug#59381: Should xref--marker-ring be per-window?

[Dmitry Gutov]

On 24/11/22 05:28, Dmitry Gutov wrote:
> (defcustom xref-storage-type 'xref-window-local-history

Or rather:

(defcustom xref-history-storage 'xref-window-local-history

bug#59381: Should xref--marker-ring be per-window?

[Ackerley Tng]

[-- Attachment #1: Type: text/plain, Size: 340 bytes --]

> (defcustom xref-history-storage 'xref-window-local-history

Here's an updated patch!

On Thu, Nov 24, 2022 at 6:17 AM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 24/11/22 05:28, Dmitry Gutov wrote:
> > (defcustom xref-storage-type 'xref-window-local-history
>
> Or rather:
>
> (defcustom xref-history-storage 'xref-window-local-history

[-- Attachment #2: 0001-Add-support-for-window-local-xref-history.patch --]
[-- Type: text/x-patch, Size: 6669 bytes --]

From 4c89685dc6d32389d7f2a9053670a17511b83b29 Mon Sep 17 00:00:00 2001
From: Ackerley Tng <ackerleytng@google.com>
Date: Mon, 21 Nov 2022 18:38:03 -0800
Subject: [PATCH] Add support for window-local xref history

---
 lisp/progmodes/xref.el | 115 +++++++++++++++++++++++++++++------------
 1 file changed, 83 insertions(+), 32 deletions(-)

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index e220367a21..6f11d6f4d4 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -429,32 +429,80 @@ xref-auto-jump-to-first-xref
   :version "28.1"
   :package-version '(xref . "1.2.0"))
 
+(defcustom xref-history-storage #'xref-window-local-history
+  "Function that returns xref history.
+
+The following functions are predefined:
+
+- `xref-global-history'
+    Return a single, global history used across the entire Emacs
+    instance.
+- `xref-window-local-history'
+    Return different xref histories, one per window.  Allows you
+    to navigate code independently in different windows.  A new
+    xref history is created for every new window."
+  :type '(radio
+          (function-item :tag "Per-window" xref-window-local-history)
+          (function-item :tag "Global history for Emacs instance" xref-global-history)
+          (function :tag "Other"))
+  :version "29.1"
+  :package-version '(xref . "1.5.1"))
+
 (make-obsolete-variable 'xref--marker-ring 'xref--history "29.1")
 
 (defun xref-set-marker-ring-length (_var _val)
   (declare (obsolete nil "29.1"))
   nil)
 
-(defvar xref--history (cons nil nil)
+(defun xref--make-xref-history ()
+  "Return a new xref history."
+  (cons nil nil))
+
+(defvar xref--history (xref--make-xref-history)
   "(BACKWARD-STACK . FORWARD-STACK) of markers to visited Xref locations.")
 
+(defun xref-global-history (&optional new-value)
+  "Return the xref history global to this Emacs instance.
+
+Override existing value with NEW-VALUE if NEW-VALUE is set."
+  (if new-value
+      (setq xref--history new-value)
+    xref--history))
+
+(defun xref-window-local-history (&optional new-value)
+  "Return window-local xref history.
+
+Override existing value with NEW-VALUE if NEW-VALUE is set."
+  (let ((w (selected-window)))
+    (if new-value
+        (set-window-parameter w 'xref--history new-value)
+      (or (window-parameter w 'xref--history)
+          (set-window-parameter w 'xref--history (xref--make-xref-history))))))
+
+(defun xref--get-history ()
+  "Return xref history using xref-history-storage."
+  (funcall xref-history-storage))
+
 (defun xref--push-backward (m)
   "Push marker M onto the backward history stack."
-  (unless (equal m (caar xref--history))
-    (push m (car xref--history))))
+  (let ((history (xref--get-history)))
+    (unless (equal m (caar history))
+      (push m (car history)))))
 
 (defun xref--push-forward (m)
   "Push marker M onto the forward history stack."
-  (unless (equal m (cadr xref--history))
-    (push m (cdr xref--history))))
+  (let ((history (xref--get-history)))
+    (unless (equal m (cadr history))
+      (push m (cdr history)))))
 
 (defun xref-push-marker-stack (&optional m)
   "Add point M (defaults to `point-marker') to the marker stack.
 The future stack is erased."
   (xref--push-backward (or m (point-marker)))
-  (dolist (mk (cdr xref--history))
-    (set-marker mk nil nil))
-  (setcdr xref--history nil))
+  (let ((history (xref--get-history)))
+    (dolist (mk (cdr history))
+      (set-marker mk nil nil))
+    (setcdr history nil)))
 
 ;;;###autoload
 (define-obsolete-function-alias 'xref-pop-marker-stack #'xref-go-back "29.1")
@@ -464,29 +512,31 @@ xref-go-back
   "Go back to the previous position in xref history.
 To undo, use \\[xref-go-forward]."
   (interactive)
-  (if (null (car xref--history))
-      (user-error "At start of xref history")
-    (let ((marker (pop (car xref--history))))
-      (xref--push-forward (point-marker))
-      (switch-to-buffer (or (marker-buffer marker)
-                            (user-error "The marked buffer has been deleted")))
-      (goto-char (marker-position marker))
-      (set-marker marker nil nil)
-      (run-hooks 'xref-after-return-hook))))
+  (let ((history (xref--get-history)))
+    (if (null (car history))
+        (user-error "At start of xref history")
+      (let ((marker (pop (car history))))
+        (xref--push-forward (point-marker))
+        (switch-to-buffer (or (marker-buffer marker)
+                              (user-error "The marked buffer has been deleted")))
+        (goto-char (marker-position marker))
+        (set-marker marker nil nil)
+        (run-hooks 'xref-after-return-hook)))))
 
 ;;;###autoload
 (defun xref-go-forward ()
   "Got to the point where a previous \\[xref-go-back] was invoked."
   (interactive)
-  (if (null (cdr xref--history))
-      (user-error "At end of xref history")
-    (let ((marker (pop (cdr xref--history))))
-      (xref--push-backward (point-marker))
-      (switch-to-buffer (or (marker-buffer marker)
-                            (user-error "The marked buffer has been deleted")))
-      (goto-char (marker-position marker))
-      (set-marker marker nil nil)
-      (run-hooks 'xref-after-return-hook))))
+  (let ((history (xref--get-history)))
+    (if (null (cdr history))
+        (user-error "At end of xref history")
+      (let ((marker (pop (cdr history))))
+        (xref--push-backward (point-marker))
+        (switch-to-buffer (or (marker-buffer marker)
+                              (user-error "The marked buffer has been deleted")))
+        (goto-char (marker-position marker))
+        (set-marker marker nil nil)
+        (run-hooks 'xref-after-return-hook)))))
 
 (define-obsolete-variable-alias
   'xref--current-item
@@ -512,22 +562,23 @@ xref-pulse-momentarily
 ;; etags.el needs this
 (defun xref-clear-marker-stack ()
   "Discard all markers from the xref history."
-  (dolist (l (list (car xref--history) (cdr xref--history)))
-    (dolist (m l)
-      (set-marker m nil nil)))
-  (setq xref--history (cons nil nil))
+  (let ((history (xref--get-history)))
+    (dolist (l (list (car history) (cdr history)))
+      (dolist (m l)
+        (set-marker m nil nil)))
+    (setq history (cons nil nil)))
   nil)
 
 ;;;###autoload
 (defun xref-marker-stack-empty-p ()
   "Whether the xref back-history is empty."
-  (null (car xref--history)))
+  (null (car (xref--get-history))))
 ;; FIXME: rename this to `xref-back-history-empty-p'.
 
 ;;;###autoload
 (defun xref-forward-history-empty-p ()
   "Whether the xref forward-history is empty."
-  (null (cdr xref--history)))
+  (null (cdr (xref--get-history))))
 \f
 
 (defun xref--goto-char (pos)
-- 
2.38.1

bug#59381: Should xref--marker-ring be per-window?

[Dmitry Gutov]

On 25/11/22 01:42, Ackerley Tng wrote:
>> (defcustom xref-history-storage 'xref-window-local-history
> Here's an updated patch!

Nice. :-) Seems good to go.

How would you like to sign the copyright assignment papers for Emacs?

Regarding the patch, I was a tad surprised that there is no use for the 
"setter" function of xref-history-store, but it makes sense given that 
the value cons serves as a pointer to the data structure which we modify 
in-place.

Perhaps we should keep the setter option in the signature anyway, for 
someone to be able to reset the history. Or save and restore it. Hm.

bug#59381: Should xref--marker-ring be per-window?

[Ackerley Tng]

I think we should keep the setter option in the signature too! Perhaps
someone would want to copy/transfer the history from storage to
storage.

I made the commit with my @google.com email, I believe Google has
already signed an agreement with the FSF for all staff. Is that okay?

On Thu, Nov 24, 2022 at 3:59 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 25/11/22 01:42, Ackerley Tng wrote:
> >> (defcustom xref-history-storage 'xref-window-local-history
> > Here's an updated patch!
>
> Nice. :-) Seems good to go.
>
> How would you like to sign the copyright assignment papers for Emacs?
>
> Regarding the patch, I was a tad surprised that there is no use for the
> "setter" function of xref-history-store, but it makes sense given that
> the value cons serves as a pointer to the data structure which we modify
> in-place.
>
> Perhaps we should keep the setter option in the signature anyway, for
> someone to be able to reset the history. Or save and restore it. Hm.

bug#59381: Should xref--marker-ring be per-window?

[Dmitry Gutov]

On 25/11/22 02:28, Ackerley Tng wrote:
> I think we should keep the setter option in the signature too! Perhaps
> someone would want to copy/transfer the history from storage to
> storage.

Deal!

> I made the commit with my @google.com email, I believe Google has
> already signed an agreement with the FSF for all staff. Is that okay?

Ah yep, that probably works.

I've pushed the patch with minor updates in 65f35b7f6f4 and bumped xref 
version to 1.6.0.

Also see the commit message for an example of how we write them, for 
your next contribution. Thanks!