xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Dmitry Gutov]

Moving from Debbugs here.

On 11.10.2022 09:37, Eli Zaretskii wrote:
>> Date: Tue, 11 Oct 2022 05:12:11 +0300
>> Cc: gerd.moellmann@gmail.com, 58158@debbugs.gnu.org, monnier@iro.umontreal.ca
>> From: Dmitry Gutov <dgutov@yandex.ru>
>>
>> What is a "subset of matches"?
> 
> Feel free to suggest a less vague description.  The idea is that the
> list in Xref buffer doesn't show all the references to the identifier,
> making renaming infeasible.

How about:

   Cannot perform replacements in this search's results

>> Perhaps we should make the error very specific, like "you can't replace
>> inside xref-find-definitions results". Since that is going to be the
>> exception in like 99.9% of the cases.
> 
> That'd be my preference, but what are those 0.1% of cases where the
> Xref buffer produced by other commands could fit?

I suppose they will be covered by the above message, while in the manual 
you can explain it in more simple terms, since you don't need to worry 
about third-party code as much.

> More generally, what exactly does xref.el test to produce the error
> message, and how to describe that in user-level terms?

It tests whether the method xref-match-length returns non-nil for any 
search results. When they do, it would identify them as "match xrefs" 
(mentioned in the Commentary).

But I suppose that clashes with the terminology you prefer to use.

>> It's possible for other backend commands (such as find-references) to
>> return unsuitable xref values in third-party backends, or for other code
>> to use xref-show-xrefs with such, but those will be really very rare, if
>> happen at all.
> 
> You are saying that 'r' is only useful after M-?, is that right?  The
> manual says so, but the manual doesn't have to say "the whole truth".
> The doc string should.

It works after dired-do-find-regexp and project-find-regexp as well.

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Eli Zaretskii]

> Date: Tue, 11 Oct 2022 14:36:11 +0300
> Cc: emacs-devel <emacs-devel@gnu.org>
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> Moving from Debbugs here.
> 
> On 11.10.2022 09:37, Eli Zaretskii wrote:
> >> Date: Tue, 11 Oct 2022 05:12:11 +0300
> >> Cc: gerd.moellmann@gmail.com, 58158@debbugs.gnu.org, monnier@iro.umontreal.ca
> >> From: Dmitry Gutov <dgutov@yandex.ru>
> >>
> >> What is a "subset of matches"?
> > 
> > Feel free to suggest a less vague description.  The idea is that the
> > list in Xref buffer doesn't show all the references to the identifier,
> > making renaming infeasible.
> 
> How about:
> 
>    Cannot perform replacements in this search's results

This is similar to the original message.  Its problem is that it
states the fact, but doesn't attempt to explain it, and thus doesn't
give a clue what the user did wrong and how to fix that.

> > More generally, what exactly does xref.el test to produce the error
> > message, and how to describe that in user-level terms?
> 
> It tests whether the method xref-match-length returns non-nil for any 
> search results. When they do, it would identify them as "match xrefs" 
> (mentioned in the Commentary).
> 
> But I suppose that clashes with the terminology you prefer to use.

If it's possible to come up with the semantics of xref-match-length or
of "match xrefs", maybe that could be useful.  The commentary just
says the "correspond to search result", which is not very useful for
this purpose.

> > You are saying that 'r' is only useful after M-?, is that right?  The
> > manual says so, but the manual doesn't have to say "the whole truth".
> > The doc string should.
> 
> It works after dired-do-find-regexp and project-find-regexp as well.

So wed cannot say something like "This can only be used after M-?",
sigh...

I still have no idea how to improve the error message.

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Dmitry Gutov]

On 11.10.2022 14:51, Eli Zaretskii wrote:
>> Date: Tue, 11 Oct 2022 14:36:11 +0300
>> Cc: emacs-devel <emacs-devel@gnu.org>
>> From: Dmitry Gutov <dgutov@yandex.ru>
>>
>> Moving from Debbugs here.
>>
>> On 11.10.2022 09:37, Eli Zaretskii wrote:
>>>> Date: Tue, 11 Oct 2022 05:12:11 +0300
>>>> Cc: gerd.moellmann@gmail.com, 58158@debbugs.gnu.org, monnier@iro.umontreal.ca
>>>> From: Dmitry Gutov <dgutov@yandex.ru>
>>>>
>>>> What is a "subset of matches"?
>>>
>>> Feel free to suggest a less vague description.  The idea is that the
>>> list in Xref buffer doesn't show all the references to the identifier,
>>> making renaming infeasible.
>>
>> How about:
>>
>>     Cannot perform replacements in this search's results
> 
> This is similar to the original message.

It is, because it tries to be accurate foremost, covering all potential 
situations.

But, like I explained, your new message is not much better: it still 
tries to be "high-level", rather than stating particulars, and while 
doing that, contradicts some objective facts.

It might be worth it if it were very clear to the user and true in 99% 
of the situations, but that doesn't looks like the case.

> Its problem is that it
> states the fact, but doesn't attempt to explain it, and thus doesn't
> give a clue what the user did wrong and how to fix that.

How does one explain that we cannot replace in xref-find-definitions 
results? E.g. because the abstract we use for this operations (to 
support different backends) doesn't give us enough information to 
perform that replacement.

And also because replacing in xref-find-definitions results doesn't make 
sense to begin with.

>>> More generally, what exactly does xref.el test to produce the error
>>> message, and how to describe that in user-level terms?
>>
>> It tests whether the method xref-match-length returns non-nil for any
>> search results. When they do, it would identify them as "match xrefs"
>> (mentioned in the Commentary).
>>
>> But I suppose that clashes with the terminology you prefer to use.
> 
> If it's possible to come up with the semantics of xref-match-length or
> of "match xrefs", maybe that could be useful.

Those are basically generalized versions of xref file matches (also 
almost same info as what M-x Grep provides), which contain the line 
number and column, and length of the match. We obtain the first two 
pieces of info lazily, but we need the last one as well.

> The commentary just
> says the "correspond to search result", which is not very useful for
> this purpose.

Search result as in result of a "full scan" of the directory. As opposed 
to looking in some small index which contains the definitions. At least 
what's what I was thinking of, probably.

But the find-references search could also be sped up with an index, so 
this is probably not worth differentiating in these terms.

>>> You are saying that 'r' is only useful after M-?, is that right?  The
>>> manual says so, but the manual doesn't have to say "the whole truth".
>>> The doc string should.
>>
>> It works after dired-do-find-regexp and project-find-regexp as well.
> 
> So wed cannot say something like "This can only be used after M-?",
> sigh...
> 
> I still have no idea how to improve the error message.

Perhaps I should remind that xref-find-definitions is still the main 
exception -- where this command doesn't work.

We also had some talks previously where it's been suggested that we 
should try to show different UIs by default, for xref-find-definitions 
results, and for other xref searches. IIRC you disagreed.

I tried to make a poll for that idea, but there were no conclusive 
choice on what alternative xref-show-definitions-function to use.

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Eli Zaretskii]

> Date: Tue, 11 Oct 2022 15:10:47 +0300
> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> >> How about:
> >>
> >>     Cannot perform replacements in this search's results
> > 
> > This is similar to the original message.
> 
> It is, because it tries to be accurate foremost, covering all potential 
> situations.
> 
> But, like I explained, your new message is not much better: it still 
> tries to be "high-level", rather than stating particulars, and while 
> doing that, contradicts some objective facts.
> 
> It might be worth it if it were very clear to the user and true in 99% 
> of the situations, but that doesn't looks like the case.
> 
> > Its problem is that it
> > states the fact, but doesn't attempt to explain it, and thus doesn't
> > give a clue what the user did wrong and how to fix that.
> 
> How does one explain that we cannot replace in xref-find-definitions 
> results?

That's what the "subset of matches of identifier" part attempts to do.

> And also because replacing in xref-find-definitions results doesn't make 
> sense to begin with.

I agree that it makes no sense.  The problem is how to say that in a
general enough way.

> > If it's possible to come up with the semantics of xref-match-length or
> > of "match xrefs", maybe that could be useful.
> 
> Those are basically generalized versions of xref file matches (also 
> almost same info as what M-x Grep provides), which contain the line 
> number and column, and length of the match. We obtain the first two 
> pieces of info lazily, but we need the last one as well.

And why do the results of xref-find-definitions lack that?

> > I still have no idea how to improve the error message.
> 
> Perhaps I should remind that xref-find-definitions is still the main 
> exception -- where this command doesn't work.

But not the only one?

> We also had some talks previously where it's been suggested that we 
> should try to show different UIs by default, for xref-find-definitions 
> results, and for other xref searches. IIRC you disagreed.

'r' is just one command which is sensitive to the differences.  AFAIR,
most other commands aren't.  So it makes sense to use the same UI.

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Dmitry Gutov]

On 11.10.2022 15:17, Eli Zaretskii wrote:

>>> Its problem is that it
>>> states the fact, but doesn't attempt to explain it, and thus doesn't
>>> give a clue what the user did wrong and how to fix that.
>>
>> How does one explain that we cannot replace in xref-find-definitions
>> results?
> 
> That's what the "subset of matches of identifier" part attempts to do.

A high-level and not very accurate description, because it's only 
relevant for the difference between xref-find-definitions vs 
xref-find-references, but not when the *-find-regexp commands come into 
play.

>> And also because replacing in xref-find-definitions results doesn't make
>> sense to begin with.
> 
> I agree that it makes no sense.  The problem is how to say that in a
> general enough way.

"Not supported" is not too terrible an error message if we are sure the 
user is trying to do something they shouldn't even attempt to.

But we can try to be helpful by offering an alternative:

   Cannot replace in this search; to rename a symbol, invoke 
\\[xref-find-references] first

>>> If it's possible to come up with the semantics of xref-match-length or
>>> of "match xrefs", maybe that could be useful.
>>
>> Those are basically generalized versions of xref file matches (also
>> almost same info as what M-x Grep provides), which contain the line
>> number and column, and length of the match. We obtain the first two
>> pieces of info lazily, but we need the last one as well.
> 
> And why do the results of xref-find-definitions lack that?

So that the backend isn't forced to provide info that's harder to get, 
and that we don't use anyway. E.g. M-x find-function just brings you to 
BOL rather than to the beginning of the symbol.

>>> I still have no idea how to improve the error message.
>>
>> Perhaps I should remind that xref-find-definitions is still the main
>> exception -- where this command doesn't work.
> 
> But not the only one?

The only known one, so far. Although it might depend on the backend as 
well. The built-in backends are going to fail, but it seems like 
lsp-mode at least returns "match xrefs" for all searches. Maybe Eglot 
too, I haven't checked.

That would mean that one 'r' can work in lsp-mode's 
xref-find-definitions results (they define a bunch of custom commands 
like lsp-find-definition and lsp-find-declaration, but that probably 
doesn't matter). Not sure if we should do something about that.

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Eli Zaretskii]

> Date: Tue, 11 Oct 2022 15:44:18 +0300
> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 11.10.2022 15:17, Eli Zaretskii wrote:
> 
> >>> Its problem is that it
> >>> states the fact, but doesn't attempt to explain it, and thus doesn't
> >>> give a clue what the user did wrong and how to fix that.
> >>
> >> How does one explain that we cannot replace in xref-find-definitions
> >> results?
> > 
> > That's what the "subset of matches of identifier" part attempts to do.
> 
> A high-level and not very accurate description, because it's only 
> relevant for the difference between xref-find-definitions vs 
> xref-find-references, but not when the *-find-regexp commands come into 
> play.

I know.  As I said, suggestions for better wording will be moist
welcome.

> But we can try to be helpful by offering an alternative:
> 
>    Cannot replace in this search; to rename a symbol, invoke 
> \\[xref-find-references] first

But then we'd need to name the other 2 commands as well, to be
accurate, yes?

> >> Perhaps I should remind that xref-find-definitions is still the main
> >> exception -- where this command doesn't work.
> > 
> > But not the only one?
> 
> The only known one, so far.

So maybe just saying

  Cannot do global replacement using results of \\[xref-find-definitions]

should be okay?

> That would mean that one 'r' can work in lsp-mode's 
> xref-find-definitions results (they define a bunch of custom commands 
> like lsp-find-definition and lsp-find-declaration, but that probably 
> doesn't matter). Not sure if we should do something about that.

If 'r' happens to work in that case, we don't have to worry about the
error message, right?

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Stefan Monnier]

>> >> What is a "subset of matches"?
[...]
>>    Cannot perform replacements in this search's results

Not sure if it's been proposed already, but how 'bout something along
the lines of "There may be more matches <elsewhere>".


        Stefan

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Stefan Monnier]

Stefan Monnier [2022-10-11 10:04:28] wrote:
>>> >> What is a "subset of matches"?
> [...]
>>>    Cannot perform replacements in this search's results
>
> Not sure if it's been proposed already, but how 'bout something along
> the lines of "There may be more matches <elsewhere>".

Or "This list is probably incomplete"?


        Stefan

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Dmitry Gutov]

On 11.10.2022 15:55, Eli Zaretskii wrote:

>> But we can try to be helpful by offering an alternative:
>>
>>     Cannot replace in this search; to rename a symbol, invoke
>> \\[xref-find-references] first
> 
> But then we'd need to name the other 2 commands as well, to be
> accurate, yes?

If our conclusion is that the error is due to user trying to rename a 
symbol (and failing because xref-find-definitions's results don't allow 
them to do so), then xref-find-references is exactly the right suggestion.

Users who try to replace in regexp search matches don't see the error.

>>>> Perhaps I should remind that xref-find-definitions is still the main
>>>> exception -- where this command doesn't work.
>>>
>>> But not the only one?
>>
>> The only known one, so far.
> 
> So maybe just saying
> 
>    Cannot do global replacement using results of \\[xref-find-definitions]
> 
> should be okay?

Isn't it almost the same as I suggested upthread? Except I suggested 
"this search" instead of naming the specific command.

Do you think naming it will be helpful enough to sacrifice the 10% 
accuracy of the message? I suppose someone might have indeed forgotten 
that they did the search using xref-find-definitions.

>> That would mean that one 'r' can work in lsp-mode's
>> xref-find-definitions results (they define a bunch of custom commands
>> like lsp-find-definition and lsp-find-declaration, but that probably
>> doesn't matter). Not sure if we should do something about that.
> 
> If 'r' happens to work in that case, we don't have to worry about the
> error message, right?

That's correct, but having the command succeed might be a problem by 
itself, couldn't it? It will rename the definitions (and/or 
declarations), but not other occurrences.

If we go in from this direction, we can have 
xref-show-definitions-buffer (the default 
xref-show-definitions-function) ensure that the binding for 'r' is set 
to some command that always reports an error (like 'cannot replace in 
definitions'), or is unbound.

This would do nothing for custom values of 
xref-show-definitions-function, but should remove most of the confusion 
with default configuration. And some non-default ones as well (lsp-mode 
doesn't change the value of xref-show-definitions-function).

If the docstring of xref-show-definitions-function looks okay to you, we 
can use its vocabulary.

   Cannot replace in definition search results

should cover xref-find-definitions, lsp-find-definition and 
lsp-find-declaration. Wouldn't help with lsp-find-implementation, though 
(its results are also questionable WRT renaming because they don't 
include all references either), but it won't make it worse.

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Dmitry Gutov]

On 11.10.2022 17:07, Stefan Monnier wrote:
> Stefan Monnier [2022-10-11 10:04:28] wrote:
>>>>>> What is a "subset of matches"?
>> [...]
>>>>     Cannot perform replacements in this search's results
>> Not sure if it's been proposed already, but how 'bout something along
>> the lines of "There may be more matches <elsewhere>".
> Or "This list is probably incomplete"?

That's the logical reason, but not really the technical one.

Although we might make a stronger effort to align these two (see my 
previous message in this thread).

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Dmitry Gutov <dgutov@yandex.ru>,  emacs-devel@gnu.org
> Date: Tue, 11 Oct 2022 10:04:28 -0400
> 
> >> >> What is a "subset of matches"?
> [...]
> >>    Cannot perform replacements in this search's results
> 
> Not sure if it's been proposed already, but how 'bout something along
> the lines of "There may be more matches <elsewhere>".

Thanks.  But that doesn't sound like an error message to me.  That
there are more matches doesn't make it clear why the ones that are
present cannot be used.  It requires a lengthy explanation or
inference.

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Eli Zaretskii]

> Date: Tue, 11 Oct 2022 17:55:02 +0300
> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 11.10.2022 15:55, Eli Zaretskii wrote:
> 
> >> But we can try to be helpful by offering an alternative:
> >>
> >>     Cannot replace in this search; to rename a symbol, invoke
> >> \\[xref-find-references] first
> > 
> > But then we'd need to name the other 2 commands as well, to be
> > accurate, yes?
> 
> If our conclusion is that the error is due to user trying to rename a 
> symbol (and failing because xref-find-definitions's results don't allow 
> them to do so), then xref-find-references is exactly the right suggestion.

That's true.

> > So maybe just saying
> > 
> >    Cannot do global replacement using results of \\[xref-find-definitions]
> > 
> > should be okay?
> 
> Isn't it almost the same as I suggested upthread? Except I suggested 
> "this search" instead of naming the specific command.

I'm not trying to argue, I'm trying to find the best message text.  It
doesn't matter who suggested it first, what matters whether it is
accurate and self-explanatory.

> Do you think naming it will be helpful enough to sacrifice the 10% 
> accuracy of the message? I suppose someone might have indeed forgotten 
> that they did the search using xref-find-definitions.

What are the 10% of inaccuracy?  If the message will only ever be
shown when 'r' is invoked after M-., then what is inaccurate in it?

> >> That would mean that one 'r' can work in lsp-mode's
> >> xref-find-definitions results (they define a bunch of custom commands
> >> like lsp-find-definition and lsp-find-declaration, but that probably
> >> doesn't matter). Not sure if we should do something about that.
> > 
> > If 'r' happens to work in that case, we don't have to worry about the
> > error message, right?
> 
> That's correct, but having the command succeed might be a problem by 
> itself, couldn't it? It will rename the definitions (and/or 
> declarations), but not other occurrences.

No error message could possibly prevent that from happening, could it?

> If we go in from this direction, we can have 
> xref-show-definitions-buffer (the default 
> xref-show-definitions-function) ensure that the binding for 'r' is set 
> to some command that always reports an error (like 'cannot replace in 
> definitions'), or is unbound.

But users can always invoke the command by name as well.

> This would do nothing for custom values of 
> xref-show-definitions-function, but should remove most of the confusion 
> with default configuration. And some non-default ones as well (lsp-mode 
> doesn't change the value of xref-show-definitions-function).
> 
> If the docstring of xref-show-definitions-function looks okay to you, we 
> can use its vocabulary.
> 
>    Cannot replace in definition search results
> 
> should cover xref-find-definitions, lsp-find-definition and 
> lsp-find-declaration. Wouldn't help with lsp-find-implementation, though 
> (its results are also questionable WRT renaming because they don't 
> include all references either), but it won't make it worse.

Hmm... maybe

  Cannot perform global replacement in find-definition results

Another idea would be for the error message to be constructed
dynamically, and include the precise command that produced the Xref
buffer, if xref.el could record that in some buffer-local variable.

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Dmitry Gutov]

On 11.10.2022 19:01, Eli Zaretskii wrote:

>>> So maybe just saying
>>>
>>>     Cannot do global replacement using results of \\[xref-find-definitions]
>>>
>>> should be okay?
>>
>> Isn't it almost the same as I suggested upthread? Except I suggested
>> "this search" instead of naming the specific command.
> 
> I'm not trying to argue, I'm trying to find the best message text.  It
> doesn't matter who suggested it first, what matters whether it is
> accurate and self-explanatory.

I wasn't playing "who said it first" either, but pointing at the 
previous suggestion as a little more inherently-accurate.

>> Do you think naming it will be helpful enough to sacrifice the 10%
>> accuracy of the message? I suppose someone might have indeed forgotten
>> that they did the search using xref-find-definitions.
> 
> What are the 10% of inaccuracy?

Sorry, losing 100% accuracy, to get a slightly smaller percentage.

> If the message will only ever be
> shown when 'r' is invoked after M-., then what is inaccurate in it?

Almost only ever. With potential for exceptions, unless we try to 
enforce their absence.

>>>> That would mean that one 'r' can work in lsp-mode's
>>>> xref-find-definitions results (they define a bunch of custom commands
>>>> like lsp-find-definition and lsp-find-declaration, but that probably
>>>> doesn't matter). Not sure if we should do something about that.
>>>
>>> If 'r' happens to work in that case, we don't have to worry about the
>>> error message, right?
>>
>> That's correct, but having the command succeed might be a problem by
>> itself, couldn't it? It will rename the definitions (and/or
>> declarations), but not other occurrences.
> 
> No error message could possibly prevent that from happening, could it?

Only if it happens somewhere else, or if the check before the errors is 
changed significantly.

>> If we go in from this direction, we can have
>> xref-show-definitions-buffer (the default
>> xref-show-definitions-function) ensure that the binding for 'r' is set
>> to some command that always reports an error (like 'cannot replace in
>> definitions'), or is unbound.
> 
> But users can always invoke the command by name as well.

That's, uh, true. But having commands intended for one major mode 
silently failing (or doing that in a weird fashion) when invoked with 
'M-x' in another major mode has not usually been a concern for us.

>> This would do nothing for custom values of
>> xref-show-definitions-function, but should remove most of the confusion
>> with default configuration. And some non-default ones as well (lsp-mode
>> doesn't change the value of xref-show-definitions-function).
>>
>> If the docstring of xref-show-definitions-function looks okay to you, we
>> can use its vocabulary.
>>
>>     Cannot replace in definition search results
>>
>> should cover xref-find-definitions, lsp-find-definition and
>> lsp-find-declaration. Wouldn't help with lsp-find-implementation, though
>> (its results are also questionable WRT renaming because they don't
>> include all references either), but it won't make it worse.
> 
> Hmm... maybe
> 
>    Cannot perform global replacement in find-definition results
> 
> Another idea would be for the error message to be constructed
> dynamically, and include the precise command that produced the Xref
> buffer, if xref.el could record that in some buffer-local variable.

I suppose it could. But then some backend (such as lsp or possibly 
eglot) might return 'definitions' results in format suitable for 
replacements, and that will mean that one can replace in 
xref-find-definitions's results, just with some backends but not others.

And with that, the error message

   Cannot do global replacement using results of \\[xref-find-definitions]

shown to the same user at a different time (perhaps when they're editing 
Elisp) will become a lie.

And then, okay, we could try to add the name of the backend symbol to 
the error message as well, but it's much harder to capture that one, 
especially given that not every command using Xref output will go 
through xref-backend-functions (project-find-regexp is a counter-example).

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Eli Zaretskii]

> Date: Tue, 11 Oct 2022 19:41:35 +0300
> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> >> This would do nothing for custom values of
> >> xref-show-definitions-function, but should remove most of the confusion
> >> with default configuration. And some non-default ones as well (lsp-mode
> >> doesn't change the value of xref-show-definitions-function).
> >>
> >> If the docstring of xref-show-definitions-function looks okay to you, we
> >> can use its vocabulary.
> >>
> >>     Cannot replace in definition search results
> >>
> >> should cover xref-find-definitions, lsp-find-definition and
> >> lsp-find-declaration. Wouldn't help with lsp-find-implementation, though
> >> (its results are also questionable WRT renaming because they don't
> >> include all references either), but it won't make it worse.
> > 
> > Hmm... maybe
> > 
> >    Cannot perform global replacement in find-definition results
> > 
> > Another idea would be for the error message to be constructed
> > dynamically, and include the precise command that produced the Xref
> > buffer, if xref.el could record that in some buffer-local variable.
> 
> I suppose it could. But then some backend (such as lsp or possibly 
> eglot) might return 'definitions' results in format suitable for 
> replacements, and that will mean that one can replace in 
> xref-find-definitions's results, just with some backends but not others.
> 
> And with that, the error message
> 
>    Cannot do global replacement using results of \\[xref-find-definitions]
> 
> shown to the same user at a different time (perhaps when they're editing 
> Elisp) will become a lie.
> 
> And then, okay, we could try to add the name of the backend symbol to 
> the error message as well, but it's much harder to capture that one, 
> especially given that not every command using Xref output will go 
> through xref-backend-functions (project-find-regexp is a counter-example).

So is this:

  Cannot perform global replacement in find-definition results

the best that can be done?  It sounds like no alternative is
significantly better.

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Dmitry Gutov]

On 11.10.2022 19:50, Eli Zaretskii wrote:

> So is this:
> 
>    Cannot perform global replacement in find-definition results
> 
> the best that can be done?  It sounds like no alternative is
> significantly better.

Perhaps.

Let's drop the "global" adjective, though: whether the command works 
"globally", "locally", or etc, depends on how the user made the search.

Then we can make this the error message and wait and see until the next 
time anybody complains about the new phrasing. Probably after Emacs 29.

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Eli Zaretskii]

> Date: Tue, 11 Oct 2022 23:31:01 +0300
> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 11.10.2022 19:50, Eli Zaretskii wrote:
> 
> > So is this:
> > 
> >    Cannot perform global replacement in find-definition results
> > 
> > the best that can be done?  It sounds like no alternative is
> > significantly better.
> 
> Perhaps.
> 
> Let's drop the "global" adjective, though: whether the command works 
> "globally", "locally", or etc, depends on how the user made the search.

The "global" part is important, though: it's supposed to hint on why
find-definition results cannot be used.  The opposite of "global" here
is "partial", not "local".  If you can suggest a better word for
"global" here, please do.

> Then we can make this the error message and wait and see until the next 
> time anybody complains about the new phrasing. Probably after Emacs 29.

Yep.

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[John Yates]

On Wed, Oct 12, 2022 at 1:18 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> The "global" part is important, though: it's supposed to hint on why
> find-definition results cannot be used.  The opposite of "global" here
> is "partial", not "local".  If you can suggest a better word for
> "global" here, please do.

Clearly the 'global' / 'local' dichotomy is too deeply ingrained in emacs
concepts to allow easy reuse of those terms with even slightly different
meanings.  Perhaps 'full', 'complete', 'total', 'universal', or some such.

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Eli Zaretskii]

On October 12, 2022 1:06:22 PM GMT+03:00, John Yates <john@yates-sheets.org> wrote:
> On Wed, Oct 12, 2022 at 1:18 AM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > The "global" part is important, though: it's supposed to hint on why
> > find-definition results cannot be used.  The opposite of "global" here
> > is "partial", not "local".  If you can suggest a better word for
> > "global" here, please do.
> 
> Clearly the 'global' / 'local' dichotomy is too deeply ingrained in emacs
> concepts to allow easy reuse of those terms with even slightly different
> meanings.  Perhaps 'full', 'complete', 'total', 'universal', or some such.


We are talking about "global replacement", yes?

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Eli Zaretskii]

> From: John Yates <john@yates-sheets.org>
> Date: Wed, 12 Oct 2022 07:49:06 -0400
> 
> On Wed, Oct 12, 2022 at 6:17 AM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > On October 12, 2022 1:06:22 PM GMT+03:00, John Yates <john@yates-sheets.org> wrote:
> > > . . .  Perhaps 'full', 'complete', 'total', 'universal', or some such.
> >
> > We are talking about "global replacement", yes?
> 
> Yes we are.  And, IIUC, your point is that the concept of
> "global replacement" is comparably embedded in software
> development.  Hmm...

Not only that, the replacements (pun intended) you proposed don't seem
to fit this context.  What synonyms of "global replacement" exist that
don't use the word "global"?

P.S. I restored the CC to emacs-devel on the assumption that you
omitted it by accident; apologies if that was intentional.

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Dmitry Gutov]

On 12.10.2022 08:17, Eli Zaretskii wrote:
>> Let's drop the "global" adjective, though: whether the command works
>> "globally", "locally", or etc, depends on how the user made the search.
> The "global" part is important, though: it's supposed to hint on why
> find-definition results cannot be used.  The opposite of "global" here
> is "partial", not "local".  If you can suggest a better word for
> "global" here, please do.

I think you're trying hard to make it clearer, but it still won't have 
100% the intended effect.

And it makes the error more likely to confuse the user and make them 
misunderstand how things work:

1) Saying "Cannot perform global replacement in find-definition 
results", with the explicit qualifier "global", potentially implies that 
a "local" replacement in find-definition results can be done. But it 
cannot. We can't do either currently for technical reasons.

We don't want to do "local" replacements in find-definition results also 
for logical reasons, but that's just the reason why we're not in a hurry 
to remove the technical limitation.

2) The message implies 'r' is limited to "global" replacement. But it 
can easily do "partial" replacement, as long as the command that 
produces the list returns "match xrefs". dired-do-find-regexp, for 
example. Or project-find-regexp, when called with C-u, and the user 
specifies a specific directory. They will return a "partial" list of 
matches, compared to the full project search.

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[Eli Zaretskii]

> Date: Wed, 12 Oct 2022 16:47:15 +0300
> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 12.10.2022 08:17, Eli Zaretskii wrote:
> >> Let's drop the "global" adjective, though: whether the command works
> >> "globally", "locally", or etc, depends on how the user made the search.
> > The "global" part is important, though: it's supposed to hint on why
> > find-definition results cannot be used.  The opposite of "global" here
> > is "partial", not "local".  If you can suggest a better word for
> > "global" here, please do.
> 
> I think you're trying hard to make it clearer, but it still won't have 
> 100% the intended effect.
> 
> And it makes the error more likely to confuse the user and make them 
> misunderstand how things work:
> 
> 1) Saying "Cannot perform global replacement in find-definition 
> results", with the explicit qualifier "global", potentially implies that 
> a "local" replacement in find-definition results can be done. But it 
> cannot. We can't do either currently for technical reasons.
> 
> We don't want to do "local" replacements in find-definition results also 
> for logical reasons, but that's just the reason why we're not in a hurry 
> to remove the technical limitation.
> 
> 2) The message implies 'r' is limited to "global" replacement. But it 
> can easily do "partial" replacement, as long as the command that 
> produces the list returns "match xrefs". dired-do-find-regexp, for 
> example. Or project-find-regexp, when called with C-u, and the user 
> specifies a specific directory. They will return a "partial" list of 
> matches, compared to the full project search.

Sorry, I'm unconvinced.  AFAICT, you are reiterating issues and
arguments we already have been through.

Re: xref-query-replace-in-results error message after xref-find-definitions, was: Re: bug#58158: 29.0.50; [overlay] Interval tree iteration considered harmful

[John Yates]

On Wed, Oct 12, 2022 at 9:20 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> Not only that, the replacements (pun intended) you proposed don't seem
> to fit this context.

I agree.  Chalk it up to a "senior moment" :-)

> What synonyms of "global replacement" exist that
> don't use the word "global"?

A synonym search suggests "universal" or "galactic" :-)

> P.S. I restored the CC to emacs-devel on the assumption that you
> omitted it by accident; apologies if that was intentional.

I do not always remember to use reply-all.  Thank you.