GNU bug report logs -
#68958
[PATCH] Support bookmarking Xref results buffers
Previous Next
Full log
Message #68 received at 68958 <at> debbugs.gnu.org (full text, mbox):
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.
This bug report was last modified 128 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.