bug#68958: [PATCH] Support bookmarking Xref results buffers

[Eshel Yaron]

Hi Dmitry,

Dmitry Gutov <dmitry@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