bug#35702: xref revert-buffer

[Dmitry Gutov]

On 24.05.2019 15:25, Eli Zaretskii wrote:

> > > Thanks, but that changeset has a few problems:
> > >
> > >     . the new command xref--revert-xref-buffer uses an internal name,
> >
> > Is that a problem by itself? We have other bindings that use internal
> > command names as well.
>
> That's a problem, yes.  Commands shouldn't be internal functions, by
> their very definition.

I've been kind of using it as a means of denoting "we're free to change 
it later", which is, of course, not great for a user, but I don't feel 
like we're settled on a final UI.

If we have a rule about private commands, though, let's fix them. But 
I'd prefer to do it a bit later, in a separate discussion.

> > >       and has no doc string
> >
> > How about something like:
> >
> >     Refresh the search results by repeating the search.
>
> Given that it doesn't, at least after M-., this sounds like not all
> the truth.  Can it be more detailed?

The truth is, most xref datasets support refreshing, but some don't. At 
least xref-find-definitions doesn't.

I'm not sure we should document that in the command's docstring, though, 
because I think we'll end up with a more different UI for 
xref-find-definitions results, with a different major mode and a keymap 
where 'g' is simply not bound.

> > >     . neither NEWS nor the user manual document the 'g' key in XREF
> > >       buffers
> >
> > I can add the NEWS entry.
>
> Please do, and thanks.

OK. Does it have to mention the command name?

Here's what I have:

** Xref

*** Refreshing the search results
When an Xref buffer shows search results (e.g. from
xref-find-references or project-find-regexp) you can type 'g' to
repeat the search and refresh the results.

> > >     . it looks like this new command is not useful after M-., because I
> > >       get an error message when I try using it (perhaps this is because
> > >       I didn't understand its use case due to lack of docs)
> >
> > It has been a deliberate choice to simplify the implementation. IME, you
> > don't ever want to refresh the list of definitions.
>
> Well, one situation where I'd like to refresh is when the TAGS file
> was updated.  It could mean that more identifiers matching the search
> string are now known.

Right, but how often would the event of updading the TAGS file with 
coincide with you wanting to refresh the xref-find-definitions results?

Wouldn't you just do the search again with 'M-.'? This command has the 
easiest keybinding.

> > But for other search results (references, apropos,
> > project-find-regexp, dired-do-find-regexp) it's a lot more common.
>
> At the very least, this should be reflected in the doc string.

Given that we're dealing with a certain level of abstration, and the 
list of commands using Xref is not limited, how would you phrase it?

> > Commit 49a363c875 also brings in another difference between the
> > behaviors of xref-find-definitions and xref-find-references: the latter
> > now shows the xref buffer even when there is just one hit.
>
> This should arguable be in NEWS.

How about:

*** New variable 'xref-show-definitions-function'
It encapsulates the logic pertinent to showing the result of
xref-find-definitions. The user can change it to customize its
behavior and the display of results.

*** Search results show the buffer even for one hit
The search-type Xref commands (e.g. xref-find-references or
project-find-regexp) now show the results buffer even when there is
only one hit. This can be altered by changing
xref-show-xrefs-function.

You can probably see a certain level of duplication there, though.

> > Do you have a better idea now?
>
> Only slightly so.  The code still doesn't speak to me, but I guess
> there isn't much that can be done about that.

This idea is pretty simple: instead of passing a list of search results 
to xref--show-xrefs, we pass an anonymous function that can be called to 
do the search, as well as repeat it, and returns the said list.