GNU bug report logs - #68958
[PATCH] Support bookmarking Xref results buffers

Previous Next

To reply to this bug, email your comments to 68958 AT debbugs.gnu.org.

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

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

From: Eshel Yaron <me <at> eshelyaron.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Support bookmarking Xref results buffers
Date: Tue, 06 Feb 2024 21:17:45 +0100

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

Tags: patch

Hello Dmitry, All,

This patch adds support for bookmarking "*xref*" buffers and restoring
them later, even across Emacs sessions.

To make this happen, we need to propagate some more information to the
"*xref*" buffer (and any other Xref fronted).  We do this, without
breaking compatibility, by setting a new variable from inside the xrefs
fetcher function.  The frontend can examine this variable to learn all
about the source of the fetched xrefs after invoking the fetcher.
Namely, the "*xref*" buffer uses this information to create bookmarks.

WDYT?

[0001-Support-bookmarking-Xref-results-buffers.patch (text/patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: dmitry <at> gutov.dev, 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Wed, 07 Feb 2024 14:25:36 +0200

> Cc: Dmitry Gutov <dmitry <at> gutov.dev>
> Date: Tue, 06 Feb 2024 21:17:45 +0100
> From:  Eshel Yaron via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> This patch adds support for bookmarking "*xref*" buffers and restoring
> them later, even across Emacs sessions.
> 
> To make this happen, we need to propagate some more information to the
> "*xref*" buffer (and any other Xref fronted).  We do this, without
> breaking compatibility, by setting a new variable from inside the xrefs
> fetcher function.  The frontend can examine this variable to learn all
> about the source of the fetched xrefs after invoking the fetcher.
> Namely, the "*xref*" buffer uses this information to create bookmarks.

Thanks.  Frankly, I'm surprised we need such a complex changeset for
supporting such a simple extension, but I'll let Dmitry judge that.

> --- a/doc/emacs/maintaining.texi
> +++ b/doc/emacs/maintaining.texi
> @@ -2466,6 +2466,10 @@ Xref Commands
>  @kbd{C-n}, and @kbd{C-p} are available for moving around the buffer
>  without displaying the references.
>  
> +You can also bookmark the @file{*xref*} buffer with @kbd{C-x r m} and
> +restore it from the same state later by jumping to that bookmark with
> +@kbd{C-x r b}.  @xref{Bookmarks}.

Since "C-x r m" and "C-x r b" are bookmark commands, they should not
be described here; instead, its description in "Bookmarks" should
mention any special features related to the Xref buffers (not that I
see what is there to mention, but maybe I'm missing something).  If
you think this capability is worth mentioning in the "Xref Commands"
node, you should do it in passage, like

  You can bookmark and restore your place in @file{*xref*} buffers,
  see @ref{Bookmarks}.

> +** New Xref generic functions for recording and restoring context.
> +Xref backends can now implement the generic function
> +'xref-backend-context' to change how Xref records the context used for
> +fetching cross-references when bookmarking Xref results for later use.
> +In addition, the new generic function 'xref-backend-restore' lets
> +backends change how Xref then restores this context.

I'm not sure this is for NEWS.  Either expand the documentation, place
it in the ELisp manual, and just mention the function's name in NEWS,
or simply don't mention it in NEWS.

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

From: Eshel Yaron <me <at> eshelyaron.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: dmitry <at> gutov.dev, 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Wed, 07 Feb 2024 18:04:24 +0100

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

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Cc: Dmitry Gutov <dmitry <at> gutov.dev>
>> Date: Tue, 06 Feb 2024 21:17:45 +0100
>> From:  Eshel Yaron via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>>
>> This patch adds support for bookmarking "*xref*" buffers and restoring
>> them later, even across Emacs sessions.
>>
>> To make this happen, we need to propagate some more information to the
>> "*xref*" buffer (and any other Xref fronted).  We do this, without
>> breaking compatibility, by setting a new variable from inside the xrefs
>> fetcher function.  The frontend can examine this variable to learn all
>> about the source of the fetched xrefs after invoking the fetcher.
>> Namely, the "*xref*" buffer uses this information to create bookmarks.
>
> Thanks.  Frankly, I'm surprised we need such a complex changeset for
> supporting such a simple extension, but I'll let Dmitry judge that.

Thanks for taking a look.

It's a simple extension, but unless I'm missing something in the code,
the Xref infrastructure needs these extra tweaks to accommodate for it.

>> --- a/doc/emacs/maintaining.texi
>> +++ b/doc/emacs/maintaining.texi
>> @@ -2466,6 +2466,10 @@ Xref Commands
>>  @kbd{C-n}, and @kbd{C-p} are available for moving around the buffer
>>  without displaying the references.
>>
>> +You can also bookmark the @file{*xref*} buffer with @kbd{C-x r m} and
>> +restore it from the same state later by jumping to that bookmark with
>> +@kbd{C-x r b}.  @xref{Bookmarks}.
>
> Since "C-x r m" and "C-x r b" are bookmark commands, they should not
> be described here; instead, its description in "Bookmarks" should
> mention any special features related to the Xref buffers (not that I
> see what is there to mention, but maybe I'm missing something).  If
> you think this capability is worth mentioning in the "Xref Commands"
> node, you should do it in passage, like
>
>   You can bookmark and restore your place in @file{*xref*} buffers,
>   see @ref{Bookmarks}.

That makes sense.  I updated the text accordingly in the attached patch.

>> +** New Xref generic functions for recording and restoring context.
>> +Xref backends can now implement the generic function
>> +'xref-backend-context' to change how Xref records the context used for
>> +fetching cross-references when bookmarking Xref results for later use.
>> +In addition, the new generic function 'xref-backend-restore' lets
>> +backends change how Xref then restores this context.
>
> I'm not sure this is for NEWS.  Either expand the documentation, place
> it in the ELisp manual, and just mention the function's name in NEWS,
> or simply don't mention it in NEWS.

All right, I removed this bit.

Here's the updated patch (v2):

[v2-0001-Support-bookmarking-Xref-results-buffers.patch (text/x-patch, attachment)]

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

From: Juri Linkov <juri <at> linkov.net>
To: 68958 <at> debbugs.gnu.org
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, Eshel Yaron <me <at> eshelyaron.com>
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Wed, 07 Feb 2024 19:25:48 +0200

> This patch adds support for bookmarking "*xref*" buffers and restoring
> them later, even across Emacs sessions.

Shouldn't 'revert-buffer-function' in the xref buffer
be sufficient to reconstruct the buffer contents?

Usually modes set buffer-local 'revert-buffer-function'
to a lambda that reruns the top function with previous
arguments.  But it seems xref.el doesn't set it.

I once tried to use 'revert-buffer-function' to restore
xref buffers from the desktop, but abandoned the idea.
Not because xref.el doesn't set 'revert-buffer-function'.
But because it would take too much time to restore
the desktop while it will rerun all saved xref buffers.

OTOH, saving an xref bookmark makes more sense.
And probably your patch will help to implement
'revert-buffer-function' for xref as well.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>, 68958 <at> debbugs.gnu.org
Cc: Eshel Yaron <me <at> eshelyaron.com>
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Sun, 11 Feb 2024 05:21:25 +0200

On 07/02/2024 19:25, Juri Linkov wrote:
>> This patch adds support for bookmarking "*xref*" buffers and restoring
>> them later, even across Emacs sessions.
> 
> Shouldn't 'revert-buffer-function' in the xref buffer
> be sufficient to reconstruct the buffer contents?
> 
> Usually modes set buffer-local 'revert-buffer-function'
> to a lambda that reruns the top function with previous
> arguments.  But it seems xref.el doesn't set it.

I can't find my original cause for not using this variable here, so if 
replacing the command xref-revert-buffer with setting 
revert-buffer-function makes things easier for someone, I'm all for it.

> I once tried to use 'revert-buffer-function' to restore
> xref buffers from the desktop, but abandoned the idea.
> Not because xref.el doesn't set 'revert-buffer-function'.
> But because it would take too much time to restore
> the desktop while it will rerun all saved xref buffers.
> 
> OTOH, saving an xref bookmark makes more sense.
> And probably your patch will help to implement
> 'revert-buffer-function' for xref as well.

I don't think bookmark would save the whole buffer contents. Would it? 
Otherwise, re-running the search(es) seems inevitable.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>, Eshel Yaron <me <at> eshelyaron.com>
Cc: 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Sun, 11 Feb 2024 05:27:27 +0200

On 07/02/2024 14:25, Eli Zaretskii wrote:
>> To make this happen, we need to propagate some more information to the
>> "*xref*" buffer (and any other Xref fronted).  We do this, without
>> breaking compatibility, by setting a new variable from inside the xrefs
>> fetcher function.  The frontend can examine this variable to learn all
>> about the source of the fetched xrefs after invoking the fetcher.
>> Namely, the "*xref*" buffer uses this information to create bookmarks.
> Thanks.  Frankly, I'm surprised we need such a complex changeset for
> supporting such a simple extension, but I'll let Dmitry judge that.

A lot of changes seem to stem from the desire to add detailed info into 
the bookmarks's name (including the identifier being searched, the 
search type, and the xref backend in use).

At the moment our code doesn't save all of those separately, just 
combines them in xref--fetcher.

So whether the patch has to be complex would depend on whether we really 
need to have bookmark names look exactly like proposed. Though I'd 
rewrite it a little even in that case.

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

From: Eshel Yaron <me <at> eshelyaron.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Sun, 11 Feb 2024 07:18:56 +0100

Hi,

Dmitry Gutov <dmitry <at> gutov.dev> writes:

> On 07/02/2024 14:25, Eli Zaretskii wrote:
>>> To make this happen, we need to propagate some more information to the
>>> "*xref*" buffer (and any other Xref fronted).  We do this, without
>>> breaking compatibility, by setting a new variable from inside the xrefs
>>> fetcher function.  The frontend can examine this variable to learn all
>>> about the source of the fetched xrefs after invoking the fetcher.
>>> Namely, the "*xref*" buffer uses this information to create bookmarks.
>> Thanks.  Frankly, I'm surprised we need such a complex changeset for
>> supporting such a simple extension, but I'll let Dmitry judge that.
>
> A lot of changes seem to stem from the desire to add detailed info
> into the bookmarks's name (including the identifier being searched,
> the search type, and the xref backend in use).

That's not the case.  The changes are all meant to facilitate creating
bookmarks that you can restore at a later session.  AFAICT the backend,
identifier and search type (kind) are a required for that, no?  We use
them to suggest a meaningful bookmark name, but that's just a bonus.

> At the moment our code doesn't save all of those separately, just
> combines them in xref--fetcher.
>
> So whether the patch has to be complex would depend on whether we
> really need to have bookmark names look exactly like proposed. Though
> I'd rewrite it a little even in that case.

Again, the name of the bookmark is really not the focus here.  We can't
persist the value of xref--fetcher, since it's an anonymous function, so
we get all the info needed to /recreate/ that function to the frontend.
If there's another (simpler?) way to provide this feature, please do tell.


Best,

Eshel

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

From: Eshel Yaron <me <at> eshelyaron.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Sun, 11 Feb 2024 12:13:40 +0100

I wrote:

> Dmitry Gutov <dmitry <at> gutov.dev> writes:
>
>> A lot of changes seem to stem from the desire to add detailed info
>> into the bookmarks's name (including the identifier being searched,
>> the search type, and the xref backend in use).
>
> That's not the case.  The changes are all meant to facilitate creating
> bookmarks that you can restore at a later session.  AFAICT the backend,
> identifier and search type (kind) are a required for that, no?  We use
> them to suggest a meaningful bookmark name, but that's just a bonus.
>
>> At the moment our code doesn't save all of those separately, just
>> combines them in xref--fetcher.
>>
>> So whether the patch has to be complex would depend on whether we
>> really need to have bookmark names look exactly like proposed. Though
>> I'd rewrite it a little even in that case.
>
> Again, the name of the bookmark is really not the focus here.  We can't
> persist the value of xref--fetcher, since it's an anonymous function, so
> we get all the info needed to /recreate/ that function to the frontend.
> If there's another (simpler?) way to provide this feature, please do tell.

Perhaps I can explain the reasoning behind this patch a little better:

The goal is to be able to persistently save (bookmark) your state in an
*xref* results buffer, perhaps as part of a long refactoring effort for
some code base, and restore it later, perhaps in another Emacs session.

Basically we want the following to work:

1. Use M-? to get a list of references in the *xref* buffer.
2. Do something with some of them, but not all.
3. Hit C-x r m to bookmark your position.
4. Quit the *xref* buffer, and close Emacs.
5. Open Emacs again, type C-x r b and select a bookmark to get an *xref* buffer
   corresponding to the same search as before, with point on the same
   reference that you where on when you created the bookmark, if possible.

Crucially, looking at step 3, we need to have the data needed to create
such a persistent bookmark available in the *xref* buffer, long after
the execution of the command that created this buffer.  So, what data do
we need?  IIUC, to replicate the saved search we need to invoke the same
backend, with the same type of search, for the same input.  Since Xref
backends may (and do) use the position of point and other buffer context
to guide the search, we want to preserve and restore that extra context
as well.

So in this patch, we add the new xref-fetcher-alist variable that allows
the fetcher function to communicate all this information to the
frontend, so when we create the *xref* buffer we can store it in
buffer-local variables, and then use them to create a bookmark record
when the user types C-x r m.  This record includes all of the data
needed to perform the same Xref search in a new Emacs session, and in
most cases to get back to same position.  We also let the backend
override what extra context exactly gets saved and restored, and how.

Hope this makes it clearer :)

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Sun, 11 Feb 2024 17:34:57 +0200

On 11/02/2024 08:18, Eshel Yaron wrote:

> Again, the name of the bookmark is really not the focus here.  We can't
> persist the value of xref--fetcher, since it's an anonymous function, so
> we get all the info needed to /recreate/ that function to the frontend.
> If there's another (simpler?) way to provide this feature, please do tell.

All right, that's a good point.

Could we really not persist an anonymous function, though? It can be 
printed and, I suppose, evaluated. At least in theory, whatever links it 
has to containing lexical contexts, should be possible to "detach" when 
writing the literal to disk, to be read later.

The issue with doing this at the level of xref--create-fetcher, is that 
the addition becomes specific to the Xref searches only (find 
definitions/references), and the more generic Xref UI infrastructure 
remains unsupported (such as 'M-x project-find-regexp' or whatever calls 
to xref-show-xrefs exist in third-party packages) -- so those Xref 
buffers would remain not bookmark-able, or they will each require 
specialized code like the one you proposed here.

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

From: Eshel Yaron <me <at> eshelyaron.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Sun, 11 Feb 2024 18:21:02 +0100

Dmitry Gutov <dmitry <at> gutov.dev> writes:

> On 11/02/2024 08:18, Eshel Yaron wrote:
>
>> Again, the name of the bookmark is really not the focus here.  We can't
>> persist the value of xref--fetcher, since it's an anonymous function, so
>> we get all the info needed to /recreate/ that function to the frontend.
>> If there's another (simpler?) way to provide this feature, please do tell.
>
> All right, that's a good point.
>
> Could we really not persist an anonymous function, though? It can be
> printed and, I suppose, evaluated. At least in theory, whatever links
> it has to containing lexical contexts, should be possible to "detach"
> when writing the literal to disk, to be read later.

I'm not sure.  It certainly cant' work for arbitrary function objects
(say, if you create a function with 'make_function' in a module).
>
> The issue with doing this at the level of xref--create-fetcher, is
> that the addition becomes specific to the Xref searches only (find
> definitions/references), and the more generic Xref UI infrastructure
> remains unsupported (such as 'M-x project-find-regexp' or whatever
> calls to xref-show-xrefs exist in third-party packages) -- so those
> Xref buffers would remain not bookmark-able, or they will each require
> specialized code like the one you proposed here.

Yes, but such callers of xref-show-xrefs can implement the same API to
have the corresponding *xref* buffer bookmark-able.  Namely, arrange for
the fetcher function to populate xref-fetcher-alist with relevant data.
Indeed, I plan to work on doing that for project-find-regexp next.  If
we had a solution that doesn't require any work from third party to
benefit from this new feature, that'd be even better, of course.  But
since the current API to Xref frontends accepts any fetcher function,
I'm not sure that's possible...

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Eshel Yaron <me <at> eshelyaron.com>, 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Sun, 11 Feb 2024 19:37:59 +0200

>> I once tried to use 'revert-buffer-function' to restore
>> xref buffers from the desktop, but abandoned the idea.
>> Not because xref.el doesn't set 'revert-buffer-function'.
>> But because it would take too much time to restore
>> the desktop while it will rerun all saved xref buffers.
>> OTOH, saving an xref bookmark makes more sense.
>> And probably your patch will help to implement
>> 'revert-buffer-function' for xref as well.
>
> I don't think bookmark would save the whole buffer contents. Would it?
> Otherwise, re-running the search(es) seems inevitable.

Indeed, saving the buffer contents of transient buffers
either to the bookmark or to the desktop makes no sense.
So re-running the command is inevitable.

But then I can't imagine how would it be possible
to move point to its saved location when initially
a restored buffer is empty before re-running the command.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eshel Yaron <me <at> eshelyaron.com>, 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Mon, 12 Feb 2024 00:45:47 +0200

On 11/02/2024 19:37, Juri Linkov wrote:
>>> I once tried to use 'revert-buffer-function' to restore
>>> xref buffers from the desktop, but abandoned the idea.
>>> Not because xref.el doesn't set 'revert-buffer-function'.
>>> But because it would take too much time to restore
>>> the desktop while it will rerun all saved xref buffers.
>>> OTOH, saving an xref bookmark makes more sense.
>>> And probably your patch will help to implement
>>> 'revert-buffer-function' for xref as well.
>> I don't think bookmark would save the whole buffer contents. Would it?
>> Otherwise, re-running the search(es) seems inevitable.
> Indeed, saving the buffer contents of transient buffers
> either to the bookmark or to the desktop makes no sense.
> So re-running the command is inevitable.
> 
> But then I can't imagine how would it be possible
> to move point to its saved location when initially
> a restored buffer is empty before re-running the command.

As long as the search is performed synchronously, first you "revert" the 
buffer and then move point. Right?

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Mon, 12 Feb 2024 01:01:28 +0200

On 11/02/2024 19:21, Eshel Yaron wrote:
> Dmitry Gutov <dmitry <at> gutov.dev> writes:
> 
>> On 11/02/2024 08:18, Eshel Yaron wrote:
>>
>>> Again, the name of the bookmark is really not the focus here.  We can't
>>> persist the value of xref--fetcher, since it's an anonymous function, so
>>> we get all the info needed to /recreate/ that function to the frontend.
>>> If there's another (simpler?) way to provide this feature, please do tell.
>>
>> All right, that's a good point.
>>
>> Could we really not persist an anonymous function, though? It can be
>> printed and, I suppose, evaluated. At least in theory, whatever links
>> it has to containing lexical contexts, should be possible to "detach"
>> when writing the literal to disk, to be read later.
> 
> I'm not sure.  It certainly cant' work for arbitrary function objects
> (say, if you create a function with 'make_function' in a module).

We could detect some such functions (e.g. when the value is a subr, and 
thus not readable), but we would still support a large varied class of 
functions this way.

Also, we'd probably want to limit the size of the printed representation 
(FETCHER created by project-find-regexp currently closes over the full 
list of files, that's too much to save; it will probably be a good idea 
to rewrite it to fetch the list of files anew).

>> The issue with doing this at the level of xref--create-fetcher, is
>> that the addition becomes specific to the Xref searches only (find
>> definitions/references), and the more generic Xref UI infrastructure
>> remains unsupported (such as 'M-x project-find-regexp' or whatever
>> calls to xref-show-xrefs exist in third-party packages) -- so those
>> Xref buffers would remain not bookmark-able, or they will each require
>> specialized code like the one you proposed here.
> 
> Yes, but such callers of xref-show-xrefs can implement the same API to
> have the corresponding *xref* buffer bookmark-able.  Namely, arrange for
> the fetcher function to populate xref-fetcher-alist with relevant data.

With that, the simple API "patch FETCHER and DISPLAY-ACTION (probably 
nil)" would turn into something at least twice (or 3x) as complex.

I'm not quite convinced that being able to bookmark Xref buffers is 
worth this cost.

Also note that with such addition the clients would basically pass the 
same information twice: they would both create the fetcher, *and* still 
have to produce the data with which this fetcher is created, and the 
logic to work on it.

This information duplication could be avoided perhaps if we split the 
fetcher into a function symbol and arguments (a new optional argument to 
xref-show-xrefs and xref--show-defs, I suppose). When the caller is able 
to restructure the code to pass a named function as the first arg, the 
result should be print-able. But then they'd have to be careful to keep 
the arguments "simple" too, like not referencing the buffer object 
itself (just its name), etc. That's a fair amount of implicit 
requirements...

> Indeed, I plan to work on doing that for project-find-regexp next.

The approach with xref-backend-restore wouldn't work for it because 
there is no backend to work with.

> If
> we had a solution that doesn't require any work from third party to
> benefit from this new feature, that'd be even better, of course.  But
> since the current API to Xref frontends accepts any fetcher function,
> I'm not sure that's possible...

Perhaps our interpreter wizards could chime in regarding the 
roundtrip-ability of anonymous functions.

Whatever is required to make this work, would likely still require less 
collective effort than redoing the Xref APIs.

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

From: Eshel Yaron <me <at> eshelyaron.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Mon, 12 Feb 2024 12:45:35 +0100

Hi Dmitry,

Dmitry Gutov <dmitry <at> gutov.dev> writes:

>>> The issue with doing this at the level of xref--create-fetcher, is
>>> that the addition becomes specific to the Xref searches only (find
>>> definitions/references), and the more generic Xref UI infrastructure
>>> remains unsupported (such as 'M-x project-find-regexp' or whatever
>>> calls to xref-show-xrefs exist in third-party packages) -- so those
>>> Xref buffers would remain not bookmark-able, or they will each require
>>> specialized code like the one you proposed here.
>> Yes, but such callers of xref-show-xrefs can implement the same API
>> to
>> have the corresponding *xref* buffer bookmark-able.  Namely, arrange for
>> the fetcher function to populate xref-fetcher-alist with relevant data.
>
> With that, the simple API "patch FETCHER and DISPLAY-ACTION (probably
> nil)" would turn into something at least twice (or 3x) as complex.

Well, ISTM that one could also say that the API is as simple as it was:
you pass the same stuff, and get the same result.  It's just that,
optionally, you can also do a bit more (set a variable inside FETCHER)
and get a bit more (bookmarking).  I agree that redundant complexity is
better avoided, but this is the simplest compatible extension to the API
I came up with to implement this feature.

> I'm not quite convinced that being able to bookmark Xref buffers is
> worth this cost.

Fair enough, it's your call.  IMO this is a rather useful capability,
and I don't think it's that important to keep the API maximally simple
if it doesn't facilitate everything we want it to.  In other words, as
long as we maintain compatibility, what's wrong with extending the API
to support more features?

> Also note that with such addition the clients would basically pass the
> same information twice: they would both create the fetcher, *and*
> still have to produce the data with which this fetcher is created, and
> the logic to work on it.

Yes.  We could drop the pre-prepared fetcher function and keep only the
"ingredients", but that'd break compatibility with existing frontends,
so we're stuck with some duplication here AFAICT.

> This information duplication could be avoided perhaps if we split the
> fetcher into a function symbol and arguments (a new optional argument
> to xref-show-xrefs and xref--show-defs, I suppose). When the caller is
> able to restructure the code to pass a named function as the first
> arg, the result should be print-able. But then they'd have to be
> careful to keep the arguments "simple" too, like not referencing the
> buffer object itself (just its name), etc. That's a fair amount of
> implicit requirements...
>
>> Indeed, I plan to work on doing that for project-find-regexp next.
>
> The approach with xref-backend-restore wouldn't work for it because
> there is no backend to work with.

My thinking was that we can add an appropriate backend, and perhaps
adapt project-find-regexp to use the new xref-make-fetcher instead of
concocting the fetcher function itself.  But I didn't try it yet so
maybe I'm missing some involved challenges.


Best,

Eshel

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Eshel Yaron <me <at> eshelyaron.com>, 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Mon, 12 Feb 2024 20:31:20 +0200

>>>> I once tried to use 'revert-buffer-function' to restore
>>>> xref buffers from the desktop, but abandoned the idea.
>>>> Not because xref.el doesn't set 'revert-buffer-function'.
>>>> But because it would take too much time to restore
>>>> the desktop while it will rerun all saved xref buffers.
>>>> OTOH, saving an xref bookmark makes more sense.
>>>> And probably your patch will help to implement
>>>> 'revert-buffer-function' for xref as well.
>>> I don't think bookmark would save the whole buffer contents. Would it?
>>> Otherwise, re-running the search(es) seems inevitable.
>> Indeed, saving the buffer contents of transient buffers
>> either to the bookmark or to the desktop makes no sense.
>> So re-running the command is inevitable.
>> But then I can't imagine how would it be possible
>> to move point to its saved location when initially
>> a restored buffer is empty before re-running the command.
>
> As long as the search is performed synchronously, first you "revert" the
> buffer and then move point. Right?

Sorry, I forgot that xref is not asynchronous as rgrep.
But this is not a problem: most of the time xref delay
is negligible.  Still would not want to run it while
restoring the desktop.  But from a bookmark is fine.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eshel Yaron <me <at> eshelyaron.com>, 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Mon, 12 Feb 2024 20:40:49 +0200

On 12/02/2024 20:31, Juri Linkov wrote:
>>>>> I once tried to use 'revert-buffer-function' to restore
>>>>> xref buffers from the desktop, but abandoned the idea.
>>>>> Not because xref.el doesn't set 'revert-buffer-function'.
>>>>> But because it would take too much time to restore
>>>>> the desktop while it will rerun all saved xref buffers.
>>>>> OTOH, saving an xref bookmark makes more sense.
>>>>> And probably your patch will help to implement
>>>>> 'revert-buffer-function' for xref as well.
>>>> I don't think bookmark would save the whole buffer contents. Would it?
>>>> Otherwise, re-running the search(es) seems inevitable.
>>> Indeed, saving the buffer contents of transient buffers
>>> either to the bookmark or to the desktop makes no sense.
>>> So re-running the command is inevitable.
>>> But then I can't imagine how would it be possible
>>> to move point to its saved location when initially
>>> a restored buffer is empty before re-running the command.
>> As long as the search is performed synchronously, first you "revert" the
>> buffer and then move point. Right?
> Sorry, I forgot that xref is not asynchronous as rgrep.
> But this is not a problem: most of the time xref delay
> is negligible.  Still would not want to run it while
> restoring the desktop.  But from a bookmark is fine.

Desktop could use some deferring strategy - e.g. delaying the search 
until the buffer is shown.

Anyway, that's secondary to being able to restore such a buffer at all.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Tue, 13 Feb 2024 05:18:28 +0200

On 12/02/2024 13:45, Eshel Yaron wrote:

>> With that, the simple API "patch FETCHER and DISPLAY-ACTION (probably
>> nil)" would turn into something at least twice (or 3x) as complex.
> 
> Well, ISTM that one could also say that the API is as simple as it was:
> you pass the same stuff, and get the same result.  It's just that,
> optionally, you can also do a bit more (set a variable inside FETCHER)
> and get a bit more (bookmarking).

A fair amount more: to fill out an alist, and have a new generic 
function implemented.

> I agree that redundant complexity is
> better avoided, but this is the simplest compatible extension to the API
> I came up with to implement this feature.

If we're going to recommend the callers use the new capability, I'd 
rather they didn't have to be redundant every time.

>> I'm not quite convinced that being able to bookmark Xref buffers is
>> worth this cost.
> 
> Fair enough, it's your call.  IMO this is a rather useful capability,
> and I don't think it's that important to keep the API maximally simple
> if it doesn't facilitate everything we want it to.  In other words, as
> long as we maintain compatibility, what's wrong with extending the API
> to support more features?

A more concise yet backward-compatible approach could also look like the 
below.

Though I'm not sure whether the fetcher should reach 
xref-show-xrefs-function intact (simpler code, but a breakage in the 
interface, which could be mended with catching 
wrong-number-of-arguments), or like in this example, both the original 
fetcher and the arguments should be passed through alist.

Otherwise, the requirements on the arguments are the same (fetcher -- 
named function, args -- printability).

Also, I'm not sure how we're supposed to guarantee that 
xref--original-buffer is live. Is that for use with desktop-mode only?

And it seems like as soon as the buffer has some new changes, the 
bookmark is likely to become invalid (the same value of point will point 
to a different identifier).

Anyway, regarding that partial patch:

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 717b837a2e5..cfdb9cd72de 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -1476,13 +1476,13 @@ xref--read-identifier-history
 (defvar xref--read-pattern-history nil)

 ;;;###autoload
-(defun xref-show-xrefs (fetcher display-action)
+(defun xref-show-xrefs (fetcher display-action &optional args)
   "Display some Xref values produced by FETCHER using DISPLAY-ACTION.
 The meanings of both arguments are the same as documented in
 `xref-show-xrefs-function'."
-  (xref--show-xrefs fetcher display-action))
+  (xref--show-xrefs fetcher display-action args))

-(defun xref--show-xrefs (fetcher display-action &optional 
_always-show-list)
+(defun xref--show-xrefs (fetcher display-action &optional 
_always-show-list args)
   (unless (functionp fetcher)
     ;; Old convention.
     (let ((xrefs fetcher))
@@ -1494,12 +1494,16 @@ xref--show-xrefs
                     xrefs
                   (setq xrefs 'called-already)))))))
   (let ((cb (current-buffer))
-        (pt (point)))
-    (prog1
+        (pt (point))
+        (orig-fetcher fetcher))
+    (when args (setq fetcher (lambda () (apply fetcher args))))
+      (prog1
         (funcall xref-show-xrefs-function fetcher
                  `((window . ,(selected-window))
                    (display-action . ,display-action)
-                   (auto-jump . ,xref-auto-jump-to-first-xref)))
+                   (auto-jump . ,xref-auto-jump-to-first-xref)
+                   (orig-fetcher . ,orig-fetcher)
+                   (fetcher-args . ,args)))
       (xref--push-markers cb pt))))

 (defun xref--show-defs (xrefs display-action)

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

From: Eshel Yaron <me <at> eshelyaron.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Tue, 13 Feb 2024 08:10:50 +0100

Dmitry Gutov <dmitry <at> gutov.dev> writes:

> On 12/02/2024 13:45, Eshel Yaron wrote:
>
>> I agree that redundant complexity is better avoided, but this is the
>> simplest compatible extension to the API I came up with to implement
>> this feature.
>
> If we're going to recommend the callers use the new capability, I'd
> rather they didn't have to be redundant every time.

Often callers can use xref-make-fetcher to make the fetcher function,
and that takes care of the redundant work for them.  That's was I did
for project-find-regexp and friends in my working branch, works well :)

[ BTW, while at it, I noticed that the docstring for
  project-or-external-find-regexp mentions a prefix argument, but the
  function doesn't actually handle one. ]

>>> I'm not quite convinced that being able to bookmark Xref buffers is
>>> worth this cost.
>> Fair enough, it's your call.  IMO this is a rather useful
>> capability,
>> and I don't think it's that important to keep the API maximally simple
>> if it doesn't facilitate everything we want it to.  In other words, as
>> long as we maintain compatibility, what's wrong with extending the API
>> to support more features?
>
> A more concise yet backward-compatible approach could also look like
> the below.
>
> Though I'm not sure whether the fetcher should reach
> xref-show-xrefs-function intact (simpler code, but a breakage in the
> interface, which could be mended with catching
> wrong-number-of-arguments), or like in this example, both the original
> fetcher and the arguments should be passed through alist.
>
> Otherwise, the requirements on the arguments are the same (fetcher --
> named function, args -- printability).

That might work, although it seems rather difficult to explain such
requirements, and it's difficult for callers to ensure or even check
whether they're kept (how do you know if your argument is too big
without printing it in advance?)

Furthermore, IIUC, what you get is an opaque function and argument list,
and the frontend cannot reason about these, it can only apply the
function to these arguments to get a list of xrefs.  In contrast,
xref-fetcher-alist provides clear (documented) semantics.  We use it for
bookmarking first and foremost, but the frontend can legitimately use it
for other stuff too, like showing some info in the mode line.

> Also, I'm not sure how we're supposed to guarantee that
> xref--original-buffer is live.

In my patch, we don't guarantee that (see xref-bookmark-make-record).
And that's fine, it's a best effort to give the backend all the context
it might need.  If there's no original buffer, we just don't save and
restore that bit of context.  The backend can handle a nil CONTEXT
argument in xref-backend-restore however it sees fit.  By default, it
does nothing.

> Is that for use with desktop-mode only?

What do you mean?  To be clear, this is unrelated to desktop-mode, or at
least I didn't design/implement any of this with desktop-mode in mind.

> And it seems like as soon as the buffer has some new changes, the
> bookmark is likely to become invalid (the same value of point will
> point to a different identifier).

We don't keep the value of point as such, we use the standard bookmark
facilities to save some context around point so we can relocate to the
right place if something changes.  If we can't find that context when
restoring the bookmark, point is just left at the beginning of the
*xref* buffer.  That's also fine.  Does that make sense?

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

From: Juri Linkov <juri <at> linkov.net>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, Eli Zaretskii <eliz <at> gnu.org>,
 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Wed, 14 Feb 2024 09:14:17 +0200

>> Is that for use with desktop-mode only?
>
> What do you mean?  To be clear, this is unrelated to desktop-mode, or at
> least I didn't design/implement any of this with desktop-mode in mind.

Desktop-mode is from the parallel discussion that I started to consider
other beneficial outcomes of applying your patch.  It will also allow
to replace xref-revert-buffer with revert-buffer-function: currently
xref-revert-buffer uses xref--fetcher, but with revert-buffer-function
could use xref-fetcher-alist.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Eshel Yaron <me <at> eshelyaron.com>,
 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Thu, 15 Feb 2024 09:58:19 +0200

> Also, we'd probably want to limit the size of the printed representation
> (FETCHER created by project-find-regexp currently closes over the full list
> of files, that's too much to save; it will probably be a good idea to
> rewrite it to fetch the list of files anew).

Indeed, I confirm the problem: reverting an xref buffer with 'g'
can't find matches in a new file added after the first run.

Doesn't this hint that both a bookmark and a revert function
should store more high-level arguments?  For example, for
'project-find-regexp' it should be sufficient to store
its argument 'regexp' together with 'default-directory'
to reconstruct the previous xref buffer contents.
Then it could store these arguments in a buffer-local variable
that could be used to create a bookmark.

PS: With C-u 'project-find-regexp' reads more arguments
in the body, but such reading should be moved to the
interactive form anyway like 'rgrep' does.

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

From: Eshel Yaron <me <at> eshelyaron.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, Eli Zaretskii <eliz <at> gnu.org>,
 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Thu, 15 Feb 2024 10:28:02 +0100

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

Hi Juri,

Juri Linkov <juri <at> linkov.net> writes:

>> Also, we'd probably want to limit the size of the printed representation
>> (FETCHER created by project-find-regexp currently closes over the full list
>> of files, that's too much to save; it will probably be a good idea to
>> rewrite it to fetch the list of files anew).
>
> Indeed, I confirm the problem: reverting an xref buffer with 'g'
> can't find matches in a new file added after the first run.
>
> Doesn't this hint that both a bookmark and a revert function
> should store more high-level arguments?  For example, for
> 'project-find-regexp' it should be sufficient to store
> its argument 'regexp' together with 'default-directory'
> to reconstruct the previous xref buffer contents.
> Then it could store these arguments in a buffer-local variable
> that could be used to create a bookmark.

Yes, that's almost exactly what I did in my bookmarking implementation.
Only difference is I kept the whole project object instead of just
'default-directory', since different project types can use different
information.  I'm attaching below the patch I applied in my local
branch, as a reference.

> PS: With C-u 'project-find-regexp' reads more arguments
> in the body, but such reading should be moved to the
> interactive form anyway like 'rgrep' does.

Agreed.  This patch does that too:

[0001-Support-bookmarking-project-find-regexp-results-buff.patch (text/x-patch, attachment)]

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Thu, 15 Feb 2024 19:57:08 +0200

On 13/02/2024 09:10, Eshel Yaron wrote:
> Dmitry Gutov <dmitry <at> gutov.dev> writes:
> 
>> On 12/02/2024 13:45, Eshel Yaron wrote:
>>
>>> I agree that redundant complexity is better avoided, but this is the
>>> simplest compatible extension to the API I came up with to implement
>>> this feature.
>>
>> If we're going to recommend the callers use the new capability, I'd
>> rather they didn't have to be redundant every time.
> 
> Often callers can use xref-make-fetcher to make the fetcher function,
> and that takes care of the redundant work for them.  That's was I did
> for project-find-regexp and friends in my working branch, works well :)
> 
> [ BTW, while at it, I noticed that the docstring for
>    project-or-external-find-regexp mentions a prefix argument, but the
>    function doesn't actually handle one. ]

Thank you for noting, now fixed.

>> Though I'm not sure whether the fetcher should reach
>> xref-show-xrefs-function intact (simpler code, but a breakage in the
>> interface, which could be mended with catching
>> wrong-number-of-arguments), or like in this example, both the original
>> fetcher and the arguments should be passed through alist.
>>
>> Otherwise, the requirements on the arguments are the same (fetcher --
>> named function, args -- printability).
> 
> That might work, although it seems rather difficult to explain such
> requirements, and it's difficult for callers to ensure or even check
> whether they're kept (how do you know if your argument is too big
> without printing it in advance?)

You can usually track that on the level of user input. A good rule of 
thumb would be not to pass a generated list of files. And if some user's 
interactive input string is veeeeeery long, well, whatever disk space is 
wasted as a result is their own doing.

What's the alternative, though? Writing a separate bookmark storage 
function for every sort of search? For project, lsp-mode/eglot (they 
both have additional commands doing extra searches), etc?

And the return value of xref-backend-context (from your proposal) must 
likewise be print-able and compact enough, right?

> Furthermore, IIUC, what you get is an opaque function and argument list,
> and the frontend cannot reason about these, it can only apply the
> function to these arguments to get a list of xrefs.  In contrast,
> xref-fetcher-alist provides clear (documented) semantics.

Which will only work for Xref's own commands but not for any external 
callers of xref-show-xrefs. Right?

> We use it for
> bookmarking first and foremost, but the frontend can legitimately use it
> for other stuff too, like showing some info in the mode line.
> 
>> Also, I'm not sure how we're supposed to guarantee that
>> xref--original-buffer is live.
> 
> In my patch, we don't guarantee that (see xref-bookmark-make-record).
> And that's fine, it's a best effort to give the backend all the context
> it might need.  If there's no original buffer, we just don't save and
> restore that bit of context. 

Okay, I see that. Basically, you bookmark the "original point" and then 
restore it from xref-backend-restore. So this would work, most of the time.

> The backend can handle a nil CONTEXT
> argument in xref-backend-restore however it sees fit.  By default, it
> does nothing.

I don't any LSP backend could handle nil, though. It would need 
additional data, like the origin file name, the value of point, etc.

>> Is that for use with desktop-mode only?
> 
> What do you mean?  To be clear, this is unrelated to desktop-mode, or at
> least I didn't design/implement any of this with desktop-mode in mind.

I meant that if you require the original buffer to be available when the 
bookmark is loaded, the easiest way to satisfy that is for desktop-mode 
to be used. But I see you solved that in a different way.

>> And it seems like as soon as the buffer has some new changes, the
>> bookmark is likely to become invalid (the same value of point will
>> point to a different identifier).
> 
> We don't keep the value of point as such, we use the standard bookmark
> facilities to save some context around point so we can relocate to the
> right place if something changes.  If we can't find that context when
> restoring the bookmark, point is just left at the beginning of the
> *xref* buffer.  That's also fine.  Does that make sense?

I meant the position of point in the original buffer, not in the Xref 
buffer, which is required for the Xref searches to work in LSP backends.

I suppose the same bookmark mechanism would be used, too, though.

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

From: Eshel Yaron <me <at> eshelyaron.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68958 <at> debbugs.gnu.org
Subject: Re: bug#68958: [PATCH] Support bookmarking Xref results buffers
Date: Thu, 15 Feb 2024 22:49:26 +0100

Dmitry Gutov <dmitry <at> gutov.dev> writes:

> On 13/02/2024 09:10, Eshel Yaron wrote:
>> Dmitry Gutov <dmitry <at> gutov.dev> writes:
>>
>>> Otherwise, the requirements on the arguments are the same (fetcher --
>>> named function, args -- printability).
>> That might work, although it seems rather difficult to explain such
>> requirements, and it's difficult for callers to ensure or even check
>> whether they're kept (how do you know if your argument is too big
>> without printing it in advance?)
>
> You can usually track that on the level of user input. A good rule of
> thumb would be not to pass a generated list of files. And if some
> user's interactive input string is veeeeeery long, well, whatever disk
> space is wasted as a result is their own doing.

I agree, that's a good heuristic.

> What's the alternative, though? Writing a separate bookmark storage
> function for every sort of search? For project, lsp-mode/eglot (they
> both have additional commands doing extra searches), etc?

I think we should have an extensible interface that covers the Xref
commands by default, and allows other callers of `xref-show-xrefs` to
override the default to suite their needs.

> And the return value of xref-backend-context (from your proposal) must
> likewise be print-able and compact enough, right?

Yes, you're right.  By default, in my proposal, the return value of this
method is itself a bookmark record (pointing to the position where you
initiate the search), so we rely on the major mode of the original
buffer to define a reasonable `bookmark-make-record-function`.  If a
backend overrides the default method, it also needs to take into account
these limitations, indeed.

>> Furthermore, IIUC, what you get is an opaque function and argument list,
>> and the frontend cannot reason about these, it can only apply the
>> function to these arguments to get a list of xrefs.  In contrast,
>> xref-fetcher-alist provides clear (documented) semantics.
>
> Which will only work for Xref's own commands but not for any external
> callers of xref-show-xrefs. Right?

It doesn't work out of the box for external callers, but it isn't
strictly restricted to Xref commands either: it works for any caller of
`xref-show-xrefs` that defines a (possibly trivial) Xref backend, and
passes a fetcher function that sets `xref-fetcher-alist`.
`xref-make-fetcher` is supposed to make it easier to create the such a
fetcher function.

>> We use it for
>> bookmarking first and foremost, but the frontend can legitimately use it
>> for other stuff too, like showing some info in the mode line.
>>
>>> Also, I'm not sure how we're supposed to guarantee that
>>> xref--original-buffer is live.
>> In my patch, we don't guarantee that (see
>> xref-bookmark-make-record).
>> And that's fine, it's a best effort to give the backend all the context
>> it might need.  If there's no original buffer, we just don't save and
>> restore that bit of context.
>
> Okay, I see that. Basically, you bookmark the "original point" and
> then restore it from xref-backend-restore. So this would work, most of
> the time.
>
>> The backend can handle a nil CONTEXT
>> argument in xref-backend-restore however it sees fit.  By default, it
>> does nothing.
>
> I don't any LSP backend could handle nil, though. It would need
> additional data, like the origin file name, the value of point, etc.

Right.  For Eglot, we cannot restore a bookmark with nil context, and we
also need to make sure we're connected to the server.  Adding something
like the following in eglot.el seems to do the trick:

--8<---------------cut here---------------start------------->8---
(cl-defmethod xref-backend-restore ((_backend (eql eglot)) context)
  (unless context
    (error "No context available for restoring Xref search"))
  (bookmark-handle-bookmark context)
  (unless eglot--managed-mode
    (apply #'eglot--connect (eglot--guess-contact))))
--8<---------------cut here---------------end--------------->8---

Previous Next

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