On 24.05.2019 17:10, Eli Zaretskii wrote: >> 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'. Second: we can "obfuscate" the docs for things we're not sure about. 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? 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. > 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. The intention is to support reverting in "search results" buffers, not intentionally refuse support it in xref-find-definitions. But we can do that. > 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? >> *** 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. >> 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? >> 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. Second, I quite frequently do something like this: - 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. 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. > OK, but (a) the heading sentence should end in a period; (b) please > use 2 spaces between sentences. OK. >> 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". >> 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. > > > 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? > 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). > 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). I can feel your pain (there's a lot of the code I have difficulty following as well), but I'm not sure where I could add comments where they wouldn't be self-evident. A couple of docstrings wouldn't hurt, though. > Except > that Edebug doesn't support generics well enough to do that, so I'm > screwed even if I have the time and energy to go down that hole. Well, at least in this case there are no generic calls between xref-find-references and xref--show-xrefs and the fetcher creation. > 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. > Useful comments can go a long way towards making such investigations > much easier and more efficient, but I guess I will hear > counter-arguments about the need to keep comments up-to-date with the > code... It's not like I'm against those, but it might take a different person to identify the places that need to be commented.