bug#35702: xref revert-buffer

[Eli Zaretskii]

> Cc: 35702@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 24 May 2019 18:15:57 +0300
> 
> >> 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.
> > 
> > I'm not a great fan of obfuscating the docs, except for reasons of
> > hiding internal implementation details.
> 
> First: my point is, it's possible that it *will* always work wherever 
> you are able to invoke it with 'g'. The Xref buffers where it wouldn't 
> work wouldn't bind 'g'.

Sorry, I don't understand what you are saying here.  If some XREF
buffers won't have the 'g' bound to a command, why do we have it bound
now, and why do we have an error message when the command cannot be
used?

> Second: we can "obfuscate" the docs for things we're not sure about. 

Doc strings aren't sacred: if we change the functionality, we can
modify the doc string accordingly.  We do that all the time.  So
there's no need to avoid documenting something just because we might
change it later.

> Think forward: if we expose the Xref UI as a library for other packages 
> in the future (and we probably will), should we allow some of them to 
> opt out of revert-ability (for simplicity, let's say), or not?

In general, I consider such changes a bad idea: XREF is a mode, and a
mode should be consistent across all its users.

> And if we do, we won't really be able to enumerate all the commands that 
> opted out in xref--revert-xref-buffer's docstring.

We don't need to enumerate all of them, we only need to enumerate the
ones that are provided with Emacs and are known not to support 'g'.
xref-find-definitions is a very frequently used command, so saying
that it doesn't support 'g' is IMO important enough.

> > Why is it a problem to say that XREF buffers created by
> > xref-find-definitions currently don't support 'g'?
> 
> Because it feels ad-hoc and kinda weird.

Are you going to add support for it any time soon?  If so, no need to
document the missing feature just yet.  But if there's a real chance
this will remain unsupported, we should document that now, lest we
forget.

> > For that matter, why shouldn't the error message be
> > explicit about that, instead of saying something technically accurate
> > but quite obscure from user's POV?
> 
> How would you phrase it?

  XREF buffers created by xref-find-definitions cannot be reverted.

> >> *** 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.
> > 
> > This should say explicitly that only some Xref functions can support
> > refreshing the results.  "E.g." can be interpreted as just an example,
> > not that some other examples don't support this.
> 
> The intent was to introduce a kind of classification. That is, to call 
> all commands that do a "wide" search as "commands that show search results".
> 
> xref-find-definitions, meanwhile, might be considered a "jump with 
> completions" command.

The classification should be described in more detail before it's
used.  Just naming it is not enough.  If you add such a description, I
probably won't object (but please watch out for the danger of too long
descriptions which don't belong in NEWS).

> >> Right, but how often would the event of updading the TAGS file with
> >> coincide with you wanting to refresh the xref-find-definitions results?
> > 
> > When the original M-. doesn't show what I expected, for example.
> 
> Hmm. So it's something you would really find useful?

Yes.

> >> Wouldn't you just do the search again with 'M-.'?
> > 
> > Yes, but that argument is true for any 'revert' action as well, isn't
> > it?  And we still have revert actions.
> 
> Not exactly: first, M-. is easier to invoke and re-invoke than 
> project-find-regexp, and you are likely to edit the regexp before searching.

I don't know about "easier", but the use case I had in mind was with
original invocation via "C-u M-.".

> - Do a search for a string with project-find-regexp,
> - Do *something* with the results, e.g. rename them all,
> - Repeat the search, to make sure I have dealt with all occurrences.
> 
> So 'g' helps considerably there. And I don't have any similar scenarios 
> for xref-find-definitions.

I don't think this is a reason good enough not to support 'g'.
Consistency is more important than the importance of use cases.
Moreover, you may not see important use cases now, but someone else
might.  We should only refrain from supporting a command when we are
100% sure it can never be useful.

> To be clear, though, we *can* add support for reverting to 
> xref-find-definitions as well, if you want. Just at the cost of a 
> certain increase of complexity, see if you like the patch. 
> xref-find-defitions-revertable.diff is attached.

Doesn't look to me like it adds any significant complexity.

> >> You can probably see a certain level of duplication there, though.
> > 
> > I don't.  I see one entry referencing another, that's all.
> 
> Just to be clear: I'm referring to two of the three entries I've showed 
> in the previous email mentioning "search-type Xref commands".

Why is that "duplication"?  using the same terminology is a Good
Thing, as it allows the reader easier understanding what is being
discussed.

> > The idea is simple and clear, but trying to understand what it does in
> > any particular invocation isn't.  I tried to figure out what happens
> > after M-., which required me to understand when is the xref--fetcher
> > variable set and to what values.  There's no easy answer to that
> > question, neither in comments nor in the doc strings.
> 
> Would a docstring saying "Function that returns a list of xrefs 
> containing the search results" change things?

I meant a comment that would explain how things worked and in what
scenarios.

> > The value is
> > set from a function passed as an argument to a couple of other
> > internal functions, so now I need to go deeper into the rabbit hole
> > and understand when and how these two functions are called, and with
> > what function as that argument.
> 
> Where the fetcher is created is not to hard to find, I think (only a few 
> local variables with that name).

Finding the places where it is defined is easy enough; understanding
the semantics of that isn't.  The code is obfuscated by using generic
names like "method", and has no comments explaining what is going on
in plain English.

> > Etc. etc. -- it feels like following
> > a deeply OO C++ code, where basically the only way to figure out how
> > the code works is to step through the code with a debugger.
> 
> It's actually more "functional programming" than OOP, and not the worst 
> example of it (very little function composition, for instance).

Thank God for small favors!

> > It used to be possible to understand what happens in a Lisp program by
> > just reading the code, but it no longer is, in many cases.
> 
> Err, my experience of reading old Lisp code in various packages doesn't 
> really agree.

I guess we are talking about different parts of Emacs.

> It's not like I'm against those, but it might take a different person to 
> identify the places that need to be commented.

That different person will not know enough about the code to add
useful comments.  Not unless they invest the effort to study and
understand it.