xref-query-replace

xref-query-replace

[Eli Zaretskii]

Is this command supposed to work?  If so, could someone please explain
what its argument FROM means?

I tried the command with both ELisp and etags back-ends, and it seems
to always say "No suitable matches here".  The doc string says just

  "Perform interactive replacement in all current matches."

but what are "current matches"?  Matches of what?

TIA

Re: xref-query-replace

[Dmitry Gutov]

On 01/09/2016 06:28 PM, Eli Zaretskii wrote:
> Is this command supposed to work?

You invoke it from an xref output buffer, to perform replacement across 
the results of a "find references" or "find regexp" command.

I've posted a thread about it in November: 
http://lists.gnu.org/archive/html/emacs-devel/2015-11/msg01465.html

> If so, could someone please explain
> what its argument FROM means?

Regexp to match within each match. So, if you made a search for 
xref-query-replace, you can easily replace "query" with "interactive" in 
all those matches.

Yes, it's a bit convoluted.

> I tried the command with both ELisp and etags back-ends, and it seems
> to always say "No suitable matches here".  The doc string says just
>
>    "Perform interactive replacement in all current matches."
>
> but what are "current matches"?  Matches of what?

Matches for input you've given previously to xref-find-regexp?

Maybe the command needs a check to see if the current buffer is 
xref-ish. However, since we supposedly allow output customization via 
xref--show-xref-buffer, it might as well use a different major mode than 
xref--xref-buffer-mode.

Re: xref-query-replace

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 9 Jan 2016 18:50:00 +0300
> 
> On 01/09/2016 06:28 PM, Eli Zaretskii wrote:
> > Is this command supposed to work?
> 
> You invoke it from an xref output buffer, to perform replacement across 
> the results of a "find references" or "find regexp" command.

That's what I believe I tried.  See below.

> I've posted a thread about it in November: 
> http://lists.gnu.org/archive/html/emacs-devel/2015-11/msg01465.html

Thanks.  I've read it now, and it describes exactly what I did.  (Btw,
why does it say to agree to ".*"? aren't other regexps supposed to
work as well?)

> > If so, could someone please explain
> > what its argument FROM means?
> 
> Regexp to match within each match. So, if you made a search for 
> xref-query-replace, you can easily replace "query" with "interactive" in 
> all those matches.

That's what I thought.

> Yes, it's a bit convoluted.

It isn't, really.  The doc string and the prompts could use some
improvement to be less confusing, but they only confused me because
the expected thing didn't happen.

> > I tried the command with both ELisp and etags back-ends, and it seems
> > to always say "No suitable matches here".  The doc string says just
> >
> >    "Perform interactive replacement in all current matches."
> >
> > but what are "current matches"?  Matches of what?
> 
> Matches for input you've given previously to xref-find-regexp?

That's what I thought.

OK, so here's the recipe that didn't work for me on the emacs-25
branch:

  emacs -Q
  M-. deactivate-mark RET

You are now presented with a *xref* buffer with 2 matches, and that
buffer's window is selected.

  r RET ddd RET

Result: "No suitable matches here".

Same result with this recipe:

  emacs -Q
  C-x C-f src/xdisp.c
  M-.

Press RET to confirm visiting the TAGS table in src/ (you should have
one, of course).  You are now presented with a *xref* buffer that
lists many matches for "Display".  (Should xref pick up symbols inside
comments?)  In that buffer, type:

  r RET xyzzy RET

I get the same disappointing result.

Stepping through xref-query-replace, it seems like xref-match-length
always returns nil, so no matches are collected.  Which sounds about
right, since I see no non-trivial definitions for xref-match-length,
or maybe I'm blind.

What am I doing wrong?

Re: xref-query-replace

[Dmitry Gutov]

On 01/09/2016 07:36 PM, Eli Zaretskii wrote:

>> I've posted a thread about it in November:
>> http://lists.gnu.org/archive/html/emacs-devel/2015-11/msg01465.html
>
> Thanks.  I've read it now, and it describes exactly what I did.  (Btw,
> why does it say to agree to ".*"? aren't other regexps supposed to
> work as well?)

Of course. These are just the steps to try it out quickly.

> Same result with this recipe:
>
>    emacs -Q
>    C-x C-f src/xdisp.c
>    M-.
>
> Press RET to confirm visiting the TAGS table in src/ (you should have
> one, of course).  You are now presented with a *xref* buffer that
> lists many matches for "Display".  (Should xref pick up symbols inside
> comments?)

etags does, apparently. Why else would xref show them?

In general, I'd say "yes", for "find references" or "find regexp" 
results. But not for "find definitions".

> In that buffer, type:
>
>    r RET xyzzy RET
>
> I get the same disappointing result.
>
> Stepping through xref-query-replace, it seems like xref-match-length
> always returns nil, so no matches are collected.  Which sounds about
> right, since I see no non-trivial definitions for xref-match-length,
> or maybe I'm blind.
>
> What am I doing wrong?

You're calling the "find definitions" command, not the "find references" 
or "find regexp".

xref-find-definitions returns a set of xrefs which we don't consider to 
be a set of "matches", currently. While the English language may frown 
on this, I think it's good that xref-query-replace doesn't work across 
the "find definitions" result, because if it did, you'd end up with a 
codebase in which the function's (definition is|definitions are) 
renamed, but all its usages keep referring to the old name.

Try starting with xref-find-references (bound to M-?).

Re: xref-query-replace

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 9 Jan 2016 19:48:17 +0300
> 
> > (Should xref pick up symbols inside comments?)
> 
> etags does, apparently. Why else would xref show them?

I thought xref didn't use etags to deduce the symbol at point, but if
I was mistaken, so be it.

> In general, I'd say "yes", for "find references" or "find regexp" 
> results. But not for "find definitions".

Then maybe we should do something about this.

> > What am I doing wrong?
> 
> You're calling the "find definitions" command, not the "find references" 
> or "find regexp".
> 
> xref-find-definitions returns a set of xrefs which we don't consider to 
> be a set of "matches", currently.

The results of the two commands look very similar, and both buffers
are under XREF mode.  So I think this distinction is very confusing.
At the very least, 'r' in the buffer where the command will do nothing
but barf should not invoke the command.

> While the English language may frown on this, I think it's good that
> xref-query-replace doesn't work across the "find definitions"
> result, because if it did, you'd end up with a codebase in which the
> function's (definition is|definitions are) renamed, but all its
> usages keep referring to the old name.

It's up to the user to decide whether this makes sense, I think.  So
we could ask her for confirmation, and if given, could proceed anyway.

> Try starting with xref-find-references (bound to M-?).

When I do that from *scratch*, I get this unfriendly error message:

  Customize ‘semantic-symref-filepattern-alist’ for lisp-interaction-mode

Can we please make this work by default from *scratch*?

Thanks.

Re: xref-query-replace

[Dmitry Gutov]

On 01/09/2016 08:08 PM, Eli Zaretskii wrote:

> I thought xref didn't use etags to deduce the symbol at point, but if
> I was mistaken, so be it.

xref-find-definitions doesn't use the "symbol at point" check. But if it 
did, the check would succeed: symbols "exist" within comments, too.

> Then maybe we should do something about this.

In etags? xref-find-definitions, when using the etags backend, uses the 
info from TAGS for this.

> The results of the two commands look very similar, and both buffers
> are under XREF mode.  So I think this distinction is very confusing.
> At the very least, 'r' in the buffer where the command will do nothing
> but barf should not invoke the command.

So if would be better to make `r' be bound in some buffers under XREF 
mode, and unbound in other ones that look very similar?

Let's make a better error message instead (suggestions welcome!).

> It's up to the user to decide whether this makes sense, I think.  So
> we could ask her for confirmation, and if given, could proceed anyway.

I don't know how to make the warning both informational and general 
enough. Better limit the functionality here.

There are technical reasons, too, for this to be hard: we don't ask xref 
backends to return "match" xrefs in xref-backend-definitions 
implementations. I expect it might be non-trivial for some backends.

>> Try starting with xref-find-references (bound to M-?).
>
> When I do that from *scratch*, I get this unfriendly error message:
>
>    Customize ‘semantic-symref-filepattern-alist’ for lisp-interaction-mode
>
> Can we please make this work by default from *scratch*?

Will do.

Re: xref-query-replace

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 9 Jan 2016 23:05:21 +0300
> 
> > The results of the two commands look very similar, and both buffers
> > are under XREF mode.  So I think this distinction is very confusing.
> > At the very least, 'r' in the buffer where the command will do nothing
> > but barf should not invoke the command.
> 
> So if would be better to make `r' be bound in some buffers under XREF 
> mode, and unbound in other ones that look very similar?
> 
> Let's make a better error message instead (suggestions welcome!).

"This command can only be used in a buffer created by xref-find-references"

> >    Customize ‘semantic-symref-filepattern-alist’ for lisp-interaction-mode
> >
> > Can we please make this work by default from *scratch*?
> 
> Will do.

Thanks.

Re: xref-query-replace

[Dmitry Gutov]

On 01/09/2016 11:10 PM, Eli Zaretskii wrote:

>> Let's make a better error message instead (suggestions welcome!).
>
> "This command can only be used in a buffer created by xref-find-references"

...or project-find-regexp, or project-or-external-find-regexp?

I think we need a collective term for those commands that return output 
suitable for xref-query-replace.

Re: xref-query-replace

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 9 Jan 2016 23:22:03 +0300
> 
> On 01/09/2016 11:10 PM, Eli Zaretskii wrote:
> 
> >> Let's make a better error message instead (suggestions welcome!).
> >
> > "This command can only be used in a buffer created by xref-find-references"
> 
> ...or project-find-regexp, or project-or-external-find-regexp?
> 
> I think we need a collective term for those commands that return output 
> suitable for xref-query-replace.

Or maybe just let it work in all XREF buffers, and trust the user that
she knows what she is doing?

Re: xref-query-replace

[Dmitry Gutov]

On 01/09/2016 11:27 PM, Eli Zaretskii wrote:

> Or maybe just let it work in all XREF buffers, and trust the user that
> she knows what she is doing?

I'd rather not.

And, like I said, there are technical reasons for this to be hard to 
support.

Re: xref-query-replace

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 9 Jan 2016 23:35:49 +0300
> 
> On 01/09/2016 11:27 PM, Eli Zaretskii wrote:
> 
> > Or maybe just let it work in all XREF buffers, and trust the user that
> > she knows what she is doing?
> 
> I'd rather not.
> 
> And, like I said, there are technical reasons for this to be hard to 
> support.

I don't really understand why.  But if you insist, the only way to
explain this is to explicitly mention all the commands that produce
"good" XREF buffers in the error message.  Yes, it will be a long
one.  But that's not a catastrophe.

Re: xref-query-replace

[Dmitry Gutov]

On 01/09/2016 11:40 PM, Eli Zaretskii wrote:

> I don't really understand why.

If you have a set of matches for a C++ method A::C#foo, what value 
should xref-match-length return for each of them? Can you tell that 
value in advance, just based on the method's qualified name?

> But if you insist, the only way to
> explain this is to explicitly mention all the commands that produce
> "good" XREF buffers in the error message.  Yes, it will be a long
> one.  But that's not a catastrophe.

Technically, this list is an open one: anyone can call xref--show-xrefs 
with a list of appropriate values. This function, or one like it, should 
be public in some next release.

If we define a collective term for such xref functions, we could mention 
it in their docstrings.

Re: xref-query-replace

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 9 Jan 2016 23:46:21 +0300
> 
> On 01/09/2016 11:40 PM, Eli Zaretskii wrote:
> 
> > I don't really understand why.
> 
> If you have a set of matches for a C++ method A::C#foo, what value 
> should xref-match-length return for each of them? Can you tell that 
> value in advance, just based on the method's qualified name?

I don't understand the importance of the length, but the definition of
a method points to a specific source line, exactly like a reference.
So whatever you gather from a reference could be done with a
definition, no?

> > But if you insist, the only way to
> > explain this is to explicitly mention all the commands that produce
> > "good" XREF buffers in the error message.  Yes, it will be a long
> > one.  But that's not a catastrophe.
> 
> Technically, this list is an open one: anyone can call xref--show-xrefs 
> with a list of appropriate values. This function, or one like it, should 
> be public in some next release.
> 
> If we define a collective term for such xref functions, we could mention 
> it in their docstrings.

I just don't see what kind of collective name would be possble.

Re: xref-query-replace

[Dmitry Gutov]

On 01/10/2016 06:32 AM, Eli Zaretskii wrote:

> I don't understand the importance of the length, but the definition of

Knowing the length lets us determine the boundaries of the match inside 
the file. This information is gathered at the time the xref instance in 
constructed.

> a method points to a specific source line, exactly like a reference.
> So whatever you gather from a reference could be done with a
> definition, no?

What will I do with the source line, then? Suppose TAGS file contains an 
entry for the method "foo" pointing to the line

   def self.foo

And the backend returns it as one of the results for the definition of 
"foo". How will we determine the bounds of the match? (search-forward 
"foo")? What if the line looks line this?

   def self.foo(bar); @foo = bar; self.another_foo(@foo); end

We'll get false positives this way.

In another situation, the identifier-to-search can be more complex: say, 
"Bar.foo". Then, even `search-forward' won't help.

To put it differently, neither etags, nor find-func.el provide the 
necessary information to create xrefs with match boundary information.

>> If we define a collective term for such xref functions, we could mention
>> it in their docstrings.
>
> I just don't see what kind of collective name would be possble.

Well, I wouldn't ask if I managed to come up with one. I don't see 
another way to resolve the issue, however.

Re: xref-query-replace

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sun, 10 Jan 2016 06:54:34 +0300
> 
>     I don't understand the importance of the length, but the definition of
> 
> Knowing the length lets us determine the boundaries of the match inside the file. This information is gathered at the time the xref instance in constructed.
> 
>     a method points to a specific source line, exactly like a reference.
>     So whatever you gather from a reference could be done with a
>     definition, no?
> 
> What will I do with the source line, then? Suppose TAGS file contains an entry for the method "foo" pointing to the line
> 
>   def self.foo
> 
> And the backend returns it as one of the results for the definition of "foo". How will we determine the bounds of the match? (search-forward "foo")? What if the line looks line this?
> 
>   def self.foo(bar); @foo = bar; self.another_foo(@foo); end
> 
> We'll get false positives this way.

By "false positives" do you mean matches to text that isn't really a
reference to 'foo'?  This command is interactive, so the user can
reject irrelevant matches and accept only the relevant ones.  So I see
this issue as a less important one.  We could even display a warning
about potential false matches when the command is invoked in such a
buffer.

By contrast, having a command inexplicably fail in a buffer that looks
and feels exactly like another one, where the command does work, is a
worse problem, IMO.  If you insist on leaving things as they are, be
prepared for bug reports.

> To put it differently, neither etags, nor find-func.el provide the necessary information to create xrefs with match boundary information.

The boundary information just makes the matches more accurate, but it
doesn't invalidate them, AFAIU.

>         If we define a collective term for such xref functions, we could mention
>         it in their docstrings.
> 
>     I just don't see what kind of collective name would be possble.
> 
> Well, I wouldn't ask if I managed to come up with one. I don't see another way to resolve the issue, however. 

Then I guess it will remain unresolved.  Too bad.

Re: xref-query-replace

[Dmitry Gutov]

On 01/10/2016 06:52 PM, Eli Zaretskii wrote:

> By "false positives" do you mean matches to text that isn't really a
> reference to 'foo'?  This command is interactive, so the user can
> reject irrelevant matches and accept only the relevant ones.  So I see
> this issue as a less important one.  We could even display a warning
> about potential false matches when the command is invoked in such a
> buffer.

All that, just to make sure the user isn't confused by the command not 
being applicable in that kind of xref buffer? The they will remain 
subtly confused in other respects.

Also note how you've skipped the more complex case: when the 
identifier-to-search is fully qualified (i.e. not a string that usually 
occurs in a buffer).

> By contrast, having a command inexplicably fail in a buffer that looks
> and feels exactly like another one, where the command does work, is a
> worse problem, IMO.  If you insist on leaving things as they are, be
> prepared for bug reports.

At least they'll see that this buffer is different somehow. And if we 
didn't manage to explain it better, well, maybe a user will have a 
better idea for naming things.

>> To put it differently, neither etags, nor find-func.el provide the necessary information to create xrefs with match boundary information.
>
> The boundary information just makes the matches more accurate, but it
> doesn't invalidate them, AFAIU.

I don't mean "symbol boundary".

"boundary information" = match starts at position 11 and ends at 21

Etags doesn't provide that.

>>      I just don't see what kind of collective name would be possble.
>>
>> Well, I wouldn't ask if I managed to come up with one. I don't see another way to resolve the issue, however.
>
> Then I guess it will remain unresolved.  Too bad.

In my head, I call them something like "general xrefs" vs. "matches 
xrefs". The latter contain the boundary information.

Re: xref-query-replace

[John Wiegley]

[-- Attachment #1: Type: text/plain, Size: 938 bytes --]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

>> Well, I wouldn't ask if I managed to come up with one. I don't see another
>> way to resolve the issue, however.
> Then I guess it will remain unresolved.  Too bad.

Having read this entire thread, I come away thinking that the UI for this
feature is much more complex than it needs to be. If it's not obvious to use,
and it's not obvious when it's used wrong, this must be corrected.

It's symbol lookup. It's a key feature that every IDE has had since the
beginning of time. It should work sanely in Emacs, and the fact that Eli has
had these experiences makes me think that the UI for xref is not ready for
release (by which I mean at this point: we don't document it or indicate that
it's present, until it's ready).

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 629 bytes --]

Re: xref-query-replace

[Dmitry Gutov]

On 01/19/2016 03:11 AM, John Wiegley wrote:

> Having read this entire thread, I come away thinking that the UI for this
> feature is much more complex than it needs to be. If it's not obvious to use,
> and it's not obvious when it's used wrong, this must be corrected.

Please come forward with suggestions.

> It's symbol lookup. It's a key feature that every IDE has had since the
> beginning of time.

What is?

The sentences you're quoting are referring to the semantic difference 
between an arbitrary xref output (where we allow references that resolve 
to a buffer position lazily, as well as which don't point exactly to the 
position where the e.g. function name starts, but to the beginning of 
the line) vs. xref output containing essentially filtered results of a 
Grep search, where the positions of the beginning and end of the "match" 
are known.

> It should work sanely in Emacs, and the fact that Eli has
> had these experiences makes me think that the UI for xref is not ready for
> release (by which I mean at this point: we don't document it or indicate that
> it's present, until it's ready).

UI for xref, or just xref-query-replace?

Anyway, you're welcome to do whatever you think is appropriate.

Re: xref-query-replace

[John Wiegley]

>>>>> Dmitry Gutov <dgutov@yandex.ru> writes:

> The sentences you're quoting are referring to the semantic difference
> between an arbitrary xref output (where we allow references that resolve to
> a buffer position lazily, as well as which don't point exactly to the
> position where the e.g. function name starts, but to the beginning of the
> line) vs. xref output containing essentially filtered results of a Grep
> search, where the positions of the beginning and end of the "match" are
> known.

Why is Eli running into a command that has differing meaning based on which
XREF buffer he's in?  If it doesn't mean anything in one buffer as opposed to
the other, then it shouldn't be a command there.

I'll have to play around with xref myself to see how it feels, and so I can
give you some more meaningful suggestions. Adding that to my list.

Thanks for always being open to suggestions and comments, Dmitry.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: xref-query-replace

[Dmitry Gutov]

On 01/19/2016 03:28 AM, John Wiegley wrote:

> Why is Eli running into a command that has differing meaning based on which
> XREF buffer he's in?  If it doesn't mean anything in one buffer as opposed to
> the other, then it shouldn't be a command there.

Nothing's stopping us from using a different keymap dependent on the 
"kind" of xref buffer, but unless we name the "kinds" of xref buffers 
appropriately, and refer to those names in documentation, having 
xref-query-replace bound in one case and unbound in the other will only 
exacerbate the problem.

We could also make the different "kinds" look somewhat different (they 
do already, but perhaps not by enough), but that won't, by itself, be 
sufficient.