bug#44611: Prefix arg for xref-goto-xref

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

I recall endless discussions about xref in the past.
But now I realized that xref-goto-xref just misses
an obvious thing, and nothing more is needed.

Like e.g. 'quit-window' allows using a prefix argument
to change its behavior by relying on (interactive "P"),
xref-goto-xref should do the same:

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index a1c4c08c26..e1dd6e56bb 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -600,9 +600,9 @@ xref--item-at-point
 
 (defun xref-goto-xref (&optional quit)
   "Jump to the xref on the current line and select its window.
-Non-interactively, non-nil QUIT means to first quit the *xref*
-buffer."
-  (interactive)
+Non-interactively, non-nil QUIT, or interactively, with prefix argument
+means to first quit the *xref* buffer."
+  (interactive "P")
   (let* ((buffer (current-buffer))
          (xref (or (xref--item-at-point)
                    (user-error "No reference at point")))

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

Hi Juri,

On 13.11.2020 10:18, Juri Linkov wrote:
> I recall endless discussions about xref in the past.
> But now I realized that xref-goto-xref just misses
> an obvious thing, and nothing more is needed.

Sounds promising!

> Like e.g. 'quit-window' allows using a prefix argument
> to change its behavior by relying on (interactive "P"),
> xref-goto-xref should do the same:

The proposed change is not quite the same (bury instead of kill), but if 
it really helps, go ahead.

Please also note that instead of 'C-u RET' you can use 'TAB' already: 
it's bound to xref-quit-and-goto-xref which calls xref-goto-xref with 
QUIT=t.

> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
> index a1c4c08c26..e1dd6e56bb 100644
> --- a/lisp/progmodes/xref.el
> +++ b/lisp/progmodes/xref.el
> @@ -600,9 +600,9 @@ xref--item-at-point
>   
>   (defun xref-goto-xref (&optional quit)
>     "Jump to the xref on the current line and select its window.
> -Non-interactively, non-nil QUIT means to first quit the *xref*
> -buffer."
> -  (interactive)
> +Non-interactively, non-nil QUIT, or interactively, with prefix argument
> +means to first quit the *xref* buffer."
> +  (interactive "P")
>     (let* ((buffer (current-buffer))
>            (xref (or (xref--item-at-point)
>                      (user-error "No reference at point")))

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

tags 44611 fixed
close 44611 28.0.50
quit

>> I recall endless discussions about xref in the past.
>> But now I realized that xref-goto-xref just misses
>> an obvious thing, and nothing more is needed.
>
> Sounds promising!

So now pushed to master.

>> Like e.g. 'quit-window' allows using a prefix argument
>> to change its behavior by relying on (interactive "P"),
>> xref-goto-xref should do the same:
>
> The proposed change is not quite the same (bury instead of kill), but if it
> really helps, go ahead.

I don't know how important would be to kill *xref* instead of burying.

> Please also note that instead of 'C-u RET' you can use 'TAB' already: it's
> bound to xref-quit-and-goto-xref which calls xref-goto-xref with QUIT=t.

I know, but it's very odd to use 'TAB' to quit the window.

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 14.11.2020 22:36, Juri Linkov wrote:
> tags 44611 fixed
> close 44611 28.0.50
> quit
> 
>>> I recall endless discussions about xref in the past.
>>> But now I realized that xref-goto-xref just misses
>>> an obvious thing, and nothing more is needed.
>>
>> Sounds promising!
> 
> So now pushed to master.

Very well.

>>> Like e.g. 'quit-window' allows using a prefix argument
>>> to change its behavior by relying on (interactive "P"),
>>> xref-goto-xref should do the same:
>>
>> The proposed change is not quite the same (bury instead of kill), but if it
>> really helps, go ahead.
> 
> I don't know how important would be to kill *xref* instead of burying.

Indeed, probably not at all important. I'm just saying the parallel 
seems very rough.

Which might or might not be important from the standpoint of trying to 
unify different behaviors in Emacs, to make them more "regular".

>> Please also note that instead of 'C-u RET' you can use 'TAB' already: it's
>> bound to xref-quit-and-goto-xref which calls xref-goto-xref with QUIT=t.
> 
> I know, but it's very odd to use 'TAB' to quit the window.

The original justification for this binding (authored by Joao) was that 
by using it we indicate that the Xref buffer is used for "completion" 
(picking one result), rather then iterating over multiple matches.

That's why it's TAB, because "TAB completion", apparently. Overall, it's 
not obvious, but it kinda makes sense.

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

> The original justification for this binding (authored by Joao) was that by
> using it we indicate that the Xref buffer is used for "completion" (picking
> one result), rather then iterating over multiple matches.
>
> That's why it's TAB, because "TAB completion", apparently. Overall, it's
> not obvious, but it kinda makes sense.

This interpretation of picking one result doesn't fit into my workflow:
I use RET to iterate over multiple matches, then need to close the
*xref* window after visiting the last match, so TAB makes no sense
in this case.  I'd expect TAB rather to iterate over multiple matches,
i.e. like TAB in browsers go to the next match.  Even in the *Completions*
buffer TAB moves to the next completion.  And in icomplete-mode
the closest analogy to picking one result is 'C-j'
(icomplete-force-complete-and-exit).

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 15.11.2020 21:51, Juri Linkov wrote:
>> The original justification for this binding (authored by Joao) was that by
>> using it we indicate that the Xref buffer is used for "completion" (picking
>> one result), rather then iterating over multiple matches.
>>
>> That's why it's TAB, because "TAB completion", apparently. Overall, it's
>> not obvious, but it kinda makes sense.
> 
> This interpretation of picking one result doesn't fit into my workflow:
> I use RET to iterate over multiple matches, then need to close the
> *xref* window after visiting the last match, so TAB makes no sense
> in this case.

Ok, so your scenario is a mix of the two.

> I'd expect TAB rather to iterate over multiple matches,
> i.e. like TAB in browsers go to the next match.  Even in the *Completions*
> buffer TAB moves to the next completion.  And in icomplete-mode
> the closest analogy to picking one result is 'C-j'
> (icomplete-force-complete-and-exit).

If people like it, I'm totally fine with changing the binding to 'C-j'.

bug#44611: Prefix arg for xref-goto-xref

[Drew Adams]

> > And in icomplete-mode the closest analogy to picking one result is 'C-j'
> 
> If people like it, I'm totally fine with changing the binding to 'C-j'.

I don't use xref.  Dunno whether it uses the minibuffer.

But if it does then, FWIW, IMO `C-j' in the minibuffer should
have its usual behavior of inserting a newline.

The minibuffer lets you do ordinary text editing of your
input, which can be multiline.  It should, at least.

(For the same reason, `?', `SPC' etc. should have their usual,
self-inserting behavior in the minibuffer.  It was misguided
to have them show help and complete words.  We finally, after
decades, got `SPC' to self-insert for file-name completion,
at least.)

Just one opinion.

bug#44611: Prefix arg for xref-goto-xref

[João Távora]

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

As for my take on this, I seem to have proposed C-u RET for
this case back in 28814, but Dmitry and Eli opposed it (Eli
called it "almost outlandish" :-) ).  Since it wasn't a do-or-die
thing, I acquiesced and some other binding was found.
But indeed I don't think there's any other place in Emacs
where C-u RET is a thing.

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28814#52

Anyway, if you want to add it now, I have no major objections.
It doesn't break any existing use cases.

João

On Sun, Nov 15, 2020 at 10:54 PM Drew Adams <drew.adams@oracle.com> wrote:

> > > And in icomplete-mode the closest analogy to picking one result is
> 'C-j'
> >
> > If people like it, I'm totally fine with changing the binding to 'C-j'.
>
> I don't use xref.  Dunno whether it uses the minibuffer.
>
> But if it does then, FWIW, IMO `C-j' in the minibuffer should
> have its usual behavior of inserting a newline.
>
> The minibuffer lets you do ordinary text editing of your
> input, which can be multiline.  It should, at least.
>
> (For the same reason, `?', `SPC' etc. should have their usual,
> self-inserting behavior in the minibuffer.  It was misguided
> to have them show help and complete words.  We finally, after
> decades, got `SPC' to self-insert for file-name completion,
> at least.)
>
> Just one opinion.
>


-- 
João Távora

[-- Attachment #2: Type: text/html, Size: 2055 bytes --]

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

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

>>>> Like e.g. 'quit-window' allows using a prefix argument
>>>> to change its behavior by relying on (interactive "P"),
>>>> xref-goto-xref should do the same:
>>>
>>> The proposed change is not quite the same (bury instead of kill), but if it
>>> really helps, go ahead.
>> I don't know how important would be to kill *xref* instead of burying.
>
> Indeed, probably not at all important. I'm just saying the parallel seems
> very rough.
>
> Which might or might not be important from the standpoint of trying to
> unify different behaviors in Emacs, to make them more "regular".

What do you think about adding a prefix arg 'KILL' to xref-pop-marker-stack
that would behave exactly like the same arg 'KILL' in quit-window?

This is very useful when there is a need to quickly peek with 'M-.',
and to kill the temporary buffer when going back with 'C-u M-,'.

Here is the implementation:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xref-pop-marker-stack-kill.patch --]
[-- Type: text/x-diff, Size: 1404 bytes --]

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 6e99e9d8ac..1bfd7be154 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -401,17 +401,23 @@ xref-push-marker-stack
   (ring-insert xref--marker-ring (or m (point-marker))))
 
 ;;;###autoload
-(defun xref-pop-marker-stack ()
-  "Pop back to where \\[xref-find-definitions] was last invoked."
-  (interactive)
+(defun xref-pop-marker-stack (&optional kill)
+  "Pop back to where \\[xref-find-definitions] was last invoked.
+With prefix argument KILL non-nil, kill the previous buffer instead of
+burying it."
+  (interactive "P")
   (let ((ring xref--marker-ring))
     (when (ring-empty-p ring)
       (user-error "Marker stack is empty"))
-    (let ((marker (ring-remove ring 0)))
-      (switch-to-buffer (or (marker-buffer marker)
-                            (user-error "The marked buffer has been deleted")))
+    (let* ((marker (ring-remove ring 0))
+           (new-buffer (or (marker-buffer marker)
+                           (user-error "The marked buffer has been deleted")))
+           (old-buffer (current-buffer)))
+      (switch-to-buffer new-buffer)
       (goto-char (marker-position marker))
       (set-marker marker nil nil)
+      (when (and kill (not (eq old-buffer new-buffer)))
+        (kill-buffer old-buffer))
       (run-hooks 'xref-after-return-hook))))
 
 (defvar xref--current-item nil)

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 11.12.2020 11:30, Juri Linkov wrote:
> What do you think about adding a prefix arg 'KILL' to xref-pop-marker-stack
> that would behave exactly like the same arg 'KILL' in quit-window?
> 
> This is very useful when there is a need to quickly peek with 'M-.',
> and to kill the temporary buffer when going back with 'C-u M-,'.

Sure, why not.

The patch LGTM.

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Fri, 11 Dec 2020 11:30:25 +0200
> Cc: 44611@debbugs.gnu.org
> 
> +(defun xref-pop-marker-stack (&optional kill)
> +  "Pop back to where \\[xref-find-definitions] was last invoked.
> +With prefix argument KILL non-nil, kill the previous buffer instead of
> +burying it."

What is "the previous buffer" in this context?  The doc string leaves
that unexplained, and thus may confuse the reader.

(I don't object to such a feature, but I really don't understand the
urge to kill Emacs buffers just because they were needed temporarily.
Why is that important?)

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

>> +(defun xref-pop-marker-stack (&optional kill)
>> +  "Pop back to where \\[xref-find-definitions] was last invoked.
>> +With prefix argument KILL non-nil, kill the previous buffer instead of
>> +burying it."
>
> What is "the previous buffer" in this context?  The doc string leaves
> that unexplained, and thus may confuse the reader.

Maybe this is clearer: "kill the buffer that was current before popping back."

> (I don't object to such a feature, but I really don't understand the
> urge to kill Emacs buffers just because they were needed temporarily.
> Why is that important?)

The problem is that such temporary file buffers are saved
to the desktop file and dangling in the buffer list forever.

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: dgutov@yandex.ru,  44611@debbugs.gnu.org
> Date: Sat, 12 Dec 2020 22:39:17 +0200
> 
> >> +(defun xref-pop-marker-stack (&optional kill)
> >> +  "Pop back to where \\[xref-find-definitions] was last invoked.
> >> +With prefix argument KILL non-nil, kill the previous buffer instead of
> >> +burying it."
> >
> > What is "the previous buffer" in this context?  The doc string leaves
> > that unexplained, and thus may confuse the reader.
> 
> Maybe this is clearer: "kill the buffer that was current before popping back."

Is that the XREF buffer?  Or is that the buffer where the definition
was shown?

> > (I don't object to such a feature, but I really don't understand the
> > urge to kill Emacs buffers just because they were needed temporarily.
> > Why is that important?)
> 
> The problem is that such temporary file buffers are saved
> to the desktop file and dangling in the buffer list forever.

I don't understand why you call them "temporary": they are normal
buffers visiting files that belong to the program you are working on,
right?  And if you care so much about what goes into your desktop
file, then you review the list of buffers each time before ending a
session, right?

Anyway, I don't want to argue about this.

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

>> Maybe this is clearer: "kill the buffer that was current before popping back."
>
> Is that the XREF buffer?  Or is that the buffer where the definition
> was shown?

The latter.

> I don't understand why you call them "temporary": they are normal
> buffers visiting files that belong to the program you are working on,
> right?

These buffers are considered temporary only in one specific workflow:
when there is a need to quickly peek into a definition and immediately
jump back.

> And if you care so much about what goes into your desktop
> file, then you review the list of buffers each time before ending a
> session, right?

Alas, this is what required to do each time before ending a session.
Since it's a more general problem, I withdraw my patch that adds C-u
to kill the buffer.  It's too much hassle to type C-u before every 'M-,'.
There are too many commands that leave unwanted buffers.  For example,
browsing the grep results, visiting the next file doesn't kill the previous
visited file buffer, etc.  So better to clean up buffers afterwards,
using e.g. project-kill-buffers.

bug#44611: Prefix arg for xref-goto-xref

[João Távora]

> There are too many commands that leave unwanted buffers.  For example,
> browsing the grep results, visiting the next file doesn't kill the previous
> visited file buffer, etc.  So better to clean up buffers afterwards,
> using e.g. project-kill-buffers.

Exactly.  This single patch goes against the grain of Emacs. Maybe
you're better off creating some "tidy-buffer-list-mode" that will try
to somehow guess which non-file and file-visiting-buffers are likely
temporary.  Maybe leverage view-mode or enhance it (I know very
little about view mode, but "view" ~= "peek").

João

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: dgutov@yandex.ru,  44611@debbugs.gnu.org
> Date: Sun, 13 Dec 2020 22:20:10 +0200
> 
> >> Maybe this is clearer: "kill the buffer that was current before popping back."
> >
> > Is that the XREF buffer?  Or is that the buffer where the definition
> > was shown?
> 
> The latter.

Then I suggest "kill the buffer used to display the definition".

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 14.12.2020 18:16, Eli Zaretskii wrote:
> Then I suggest "kill the buffer used to display the definition".

That might be unnecessarily limiting: the xref marker stack is the 
replacement for the tag ring. These aren't used solely for definitions.

It seems Juri has withdrawn the proposal anyway, though.

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

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

>> I'd expect TAB rather to iterate over multiple matches,
>> i.e. like TAB in browsers go to the next match.  Even in the *Completions*
>> buffer TAB moves to the next completion.  And in icomplete-mode
>> the closest analogy to picking one result is 'C-j'
>> (icomplete-force-complete-and-exit).
>
> If people like it, I'm totally fine with changing the binding to 'C-j'.

I'm very sorry for beating this horse again, but after trying to use xref
as a replacement of grep, typing 'C-x p g' pops up a grep-like buffer
and due to habit of typing the same keys that are supported by grep-mode
where among them is TAB bound to compilation-next-error to browse the
results forward, but instead of going to the next match, it does the
worst thing imaginable - kills the output buffer.

Therefore, I propose this patch that binds TAB and S-TAB to command
that behave like compilation-next-error and compilation-previous-error:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xref-compilation-next-error.patch --]
[-- Type: text/x-diff, Size: 1704 bytes --]

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 6e99e9d8ac..6c333fc8c1 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -581,16 +581,26 @@ xref-show-location-at-point
     (when xref
       (xref--show-location (xref-item-location xref)))))
 
+(defun xref-next-line-no-select ()
+  "Move to the next xref but don't display its source."
+  (interactive)
+  (xref--search-property 'xref-item))
+
 (defun xref-next-line ()
   "Move to the next xref and display its source in the appropriate window."
   (interactive)
-  (xref--search-property 'xref-item)
+  (xref-next-line-no-select)
   (xref-show-location-at-point))
 
+(defun xref-prev-line-no-select ()
+  "Move to the previous xref but don't display its source."
+  (interactive)
+  (xref--search-property 'xref-item t))
+
 (defun xref-prev-line ()
   "Move to the previous xref and display its source in the appropriate window."
   (interactive)
-  (xref--search-property 'xref-item t)
+  (xref-prev-line-no-select)
   (xref-show-location-at-point))
 
 (defun xref-next-group ()
@@ -761,7 +771,8 @@ xref--xref-buffer-mode-map
     (define-key map (kbd "P") #'xref-prev-group)
     (define-key map (kbd "r") #'xref-query-replace-in-results)
     (define-key map (kbd "RET") #'xref-goto-xref)
-    (define-key map (kbd "TAB")  #'xref-quit-and-goto-xref)
+    (define-key map "\t" 'xref-next-line-no-select) ; like compilation-next-error
+    (define-key map [backtab] 'xref-prev-line-no-select) ; like compilation-previous-error
     (define-key map (kbd "C-o") #'xref-show-location-at-point)
     ;; suggested by Johan Claesson "to further reduce finger movement":
     (define-key map (kbd ".") #'xref-next-line)

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

Hi Juri,

On 19.12.2020 22:38, Juri Linkov wrote:
>>> I'd expect TAB rather to iterate over multiple matches,
>>> i.e. like TAB in browsers go to the next match.  Even in the *Completions*
>>> buffer TAB moves to the next completion.  And in icomplete-mode
>>> the closest analogy to picking one result is 'C-j'
>>> (icomplete-force-complete-and-exit).
>>
>> If people like it, I'm totally fine with changing the binding to 'C-j'.
> 
> I'm very sorry for beating this horse again, but after trying to use xref
> as a replacement of grep, typing 'C-x p g' pops up a grep-like buffer
> and due to habit of typing the same keys that are supported by grep-mode
> where among them is TAB bound to compilation-next-error to browse the
> results forward, but instead of going to the next match, it does the
> worst thing imaginable - kills the output buffer.
> 
> Therefore, I propose this patch that binds TAB and S-TAB to command
> that behave like compilation-next-error and compilation-previous-error:

Sure, why not.

What about that 'C-j' binding, though? Or are you (and Joao?) satisfied 
with 'C-u RET'?

bug#44611: Prefix arg for xref-goto-xref

[João Távora]

I'm fine with anything new as long as backward compatible.
Both ideas or none of them seem fine at this point. Thanks.

On Sat, Dec 19, 2020 at 9:41 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> Hi Juri,
>
> On 19.12.2020 22:38, Juri Linkov wrote:
> >>> I'd expect TAB rather to iterate over multiple matches,
> >>> i.e. like TAB in browsers go to the next match.  Even in the *Completions*
> >>> buffer TAB moves to the next completion.  And in icomplete-mode
> >>> the closest analogy to picking one result is 'C-j'
> >>> (icomplete-force-complete-and-exit).
> >>
> >> If people like it, I'm totally fine with changing the binding to 'C-j'.
> >
> > I'm very sorry for beating this horse again, but after trying to use xref
> > as a replacement of grep, typing 'C-x p g' pops up a grep-like buffer
> > and due to habit of typing the same keys that are supported by grep-mode
> > where among them is TAB bound to compilation-next-error to browse the
> > results forward, but instead of going to the next match, it does the
> > worst thing imaginable - kills the output buffer.
> >
> > Therefore, I propose this patch that binds TAB and S-TAB to command
> > that behave like compilation-next-error and compilation-previous-error:
>
> Sure, why not.
>
> What about that 'C-j' binding, though? Or are you (and Joao?) satisfied
> with 'C-u RET'?



-- 
João Távora

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 20.12.2020 00:36, João Távora wrote:
> I'm fine with anything new as long as backward compatible.

You mean the command xref-quit-and-goto-xref behaving the same?

The bindings obviously won't be backward compatible.

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Sat, 19 Dec 2020 22:38:12 +0200
> Cc: João Távora <joaotavora@gmail.com>,
>  44611@debbugs.gnu.org
> 
> Therefore, I propose this patch that binds TAB and S-TAB to command
> that behave like compilation-next-error and compilation-previous-error:

This is an incompatible behavior change, no?  If so, I'm not sure we
can simply make it.

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

>> Therefore, I propose this patch that binds TAB and S-TAB to command
>> that behave like compilation-next-error and compilation-previous-error:
>
> This is an incompatible behavior change, no?  If so, I'm not sure we
> can simply make it.

This is what we have the section "Incompatible Editing Changes" in etc/NEWS for.
And since the new keybinding have no disastrous effect, it would safer
for the users to adapt to the new keybinding.

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: dgutov@yandex.ru,  joaotavora@gmail.com,  44611@debbugs.gnu.org
> Date: Sun, 20 Dec 2020 10:39:25 +0200
> 
> >> Therefore, I propose this patch that binds TAB and S-TAB to command
> >> that behave like compilation-next-error and compilation-previous-error:
> >
> > This is an incompatible behavior change, no?  If so, I'm not sure we
> > can simply make it.
> 
> This is what we have the section "Incompatible Editing Changes" in etc/NEWS for.

The fact that the section exists doesn't mean we can make incompatible
changes whenever we feel like.  This key binding existed since Emacs
26.1, so changing it would definitely surprise someone.

> And since the new keybinding have no disastrous effect, it would safer
> for the users to adapt to the new keybinding.

What disastrous effects?  AFAICT, TAB buries the XREF buffer, which is
not a disaster.

More generally, I think it's wrong to try to make XREF buffers behave
like *Grep* buffers.  They are different beasts, I think we
established this long ago, when Xref was added to Emacs (I think I
even made a comment regarding the difference, and Dmitry responded to
the effect that this was intended).  *XREF* and *Grep* look
differently and behave differently, and it's impossible to make them
be similar enough.

So I'm against this change, certainly if it's unconditionally changes
the key binding.

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

Hi Eli,

On 20.12.2020 17:43, Eli Zaretskii wrote:
>>> This is an incompatible behavior change, no?  If so, I'm not sure we
>>> can simply make it.
>>
>> This is what we have the section "Incompatible Editing Changes" in etc/NEWS for.
> 
> The fact that the section exists doesn't mean we can make incompatible
> changes whenever we feel like.

It does mean we can make such changes when we discover a better way to 
do things.

I think you can agree that we have been, overall, fairly conservative 
with UI changes in Xref.

> This key binding existed since Emacs
> 26.1, so changing it would definitely surprise someone.

In any case, it was one of the minor, later additions. I would be 
surprised to find out (if it ever were possible to obtain such data) 
that > 5% of regular Xref users take advantage of it.

And those who do, can discover the change without any terrible 
consequences (the new binding is pretty benign), look up the docs and adapt.

>> And since the new keybinding have no disastrous effect, it would safer
>> for the users to adapt to the new keybinding.
> 
> What disastrous effects?  AFAICT, TAB buries the XREF buffer, which is
> not a disaster.
> 
> More generally, I think it's wrong to try to make XREF buffers behave
> like *Grep* buffers.  They are different beasts, I think we
> established this long ago, when Xref was added to Emacs (I think I
> even made a comment regarding the difference, and Dmitry responded to
> the effect that this was intended).

While I indeed might have said something like that, it was probably in 
the context of copying some behavior from Grep that didn't make sense 
for Xref. This one does (IMO), and, in general, reducing gratuitous 
differences between packages and UIs we provide is a good thing.

And if things like that will make Xref more palatable to long-time 
Compilation/Grep aficionados without conflicting with Xref's goals, I'm 
all in favor of that.

 > *XREF* and *Grep* look
 > differently and behave differently, and it's impossible to make them
 > be similar enough.

If Juri's experience says different, it's a valuable data point. If you 
recall, we have received a fair number of criticisms for Xref's UI from 
him over the years.

Finally, the binding in question was added when the Xref buffers for 
"find definition" and "regexp matches in directory" have (without extra 
user-specific hacks) behaved the same. This has changed with the 
introduction of the option xref-show-definitions-function, using which 
we changed the latter's behavior a little.

I believe we might change xref-show-definitions-function's default soon 
enough, perhaps even in Emacs 28. xref--show-defs-minibuffer, which we 
have come up during a recent discussion in emacs-devel, works quite well 
in my testing for this particular use case (feedback welcome, BTW).

If/when we do, the xref-quit-and-goto-xref binding will lose even more 
of its importance. Though it should still remain useful on occasion.

bug#44611: Prefix arg for xref-goto-xref

[João Távora]

On Sat, Dec 19, 2020 at 11:59 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 20.12.2020 00:36, João Távora wrote:
> > I'm fine with anything new as long as backward compatible.
>
> You mean the command xref-quit-and-goto-xref behaving the same?
>
> The bindings obviously won't be backward compatible.

I just use M-. ,C-n/C-p and RET.  No C-u's.  If that stays, fine with me.

João

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 20.12.2020 22:32, João Távora wrote:
> I just use M-. ,C-n/C-p and RET.  No C-u's.  If that stays, fine with me.

So you don't use the TAB binding yourself?

bug#44611: Prefix arg for xref-goto-xref

[João Távora]

On Sun, Dec 20, 2020 at 8:45 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 20.12.2020 22:32, João Távora wrote:
> > I just use M-. ,C-n/C-p and RET.  No C-u's.  If that stays, fine with me.
>
> So you don't use the TAB binding yourself?

Don't think so. Maybe I did before, don't remember. Don't keep it for me, though
did try it now, doesn't seem stupid.  If I miss it I know how to bring
it back. Thanks.

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

>> And since the new keybinding have no disastrous effect, it would safer
>> for the users to adapt to the new keybinding.
>
> What disastrous effects?  AFAICT, TAB buries the XREF buffer, which is
> not a disaster.

It takes the same amount of efforts to bring it back: either by finding
the buried buffer in the buffer list, or running project-find-regexp again.

> More generally, I think it's wrong to try to make XREF buffers behave
> like *Grep* buffers.  They are different beasts, I think we
> established this long ago, when Xref was added to Emacs (I think I
> even made a comment regarding the difference, and Dmitry responded to
> the effect that this was intended).  *XREF* and *Grep* look
> differently and behave differently, and it's impossible to make them
> be similar enough.

They might be different when *xref* is used as a completions buffer
when the *xref* buffer is created by 'M-.' (xref-find-definitions).

Moreover, now it's possible to use the real *Completions* buffer by
customizing xref-show-definitions-function to xref--show-defs-minibuffer.
Even is this case TAB doesn't close the *Completions* buffer.

But a completely separate case is when the *xref* buffer is created
by such grep-like commands as project-find-regexp.

So currently there are two different uses of the *xref* buffer:

1. as a transient completions buffer that could be buried
   after a completion is selected from the buffer;
2. as a grep buffer for visiting the found matches.

In the second case, it's natural to type TAB to navigate results.

Should these cases use different modes with different buffer names?
Such as a buffer name *xgrep*?

bug#44611: Prefix arg for xref-goto-xref

[João Távora]

On Sun, Dec 20, 2020 at 3:43 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > And since the new keybinding have no disastrous effect, it would safer
> > for the users to adapt to the new keybinding.
> What disastrous effects?  AFAICT, TAB buries the XREF buffer, which is
> not a disaster.

Yes, I'd just like to add that I tried it, and didn't find the results bad
precisely _because_ I looked for *xref* in my buffer list and it was
still there.  I wouldn't have appreciated it if it had killed it, I think.
That said, I don't think I really use TAB there, as I've said before.

João

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 22.12.2020 10:58, Juri Linkov wrote:

> But a completely separate case is when the *xref* buffer is created
> by such grep-like commands as project-find-regexp.
> 
> So currently there are two different uses of the *xref* buffer:
> 
> 1. as a transient completions buffer that could be buried
>     after a completion is selected from the buffer;
> 2. as a grep buffer for visiting the found matches.
> 
> In the second case, it's natural to type TAB to navigate results.
> 
> Should these cases use different modes with different buffer names?

It can have a different name, and even a different mode, but I don't 
think using significantly different key bindings is a good idea while 
the buffer (except for the name, maybe) looks exactly the same. That's 
still a good recipe for confusing a user.

> Such as a buffer name *xgrep*?

Perhaps we could rename the "find definitions" buffer instead, because 
that one only has one "source".

The second case, however, can have "matches" coming from very different 
sources, so calling it "grep" might not always be appropriate.

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: dgutov@yandex.ru,  joaotavora@gmail.com,  44611@debbugs.gnu.org
> Date: Tue, 22 Dec 2020 10:58:35 +0200
> 
> >> And since the new keybinding have no disastrous effect, it would safer
> >> for the users to adapt to the new keybinding.
> >
> > What disastrous effects?  AFAICT, TAB buries the XREF buffer, which is
> > not a disaster.
> 
> It takes the same amount of efforts to bring it back: either by finding
> the buried buffer in the buffer list, or running project-find-regexp again.

That doesn't sound like a disaster.  It just means "C-x b" won't offer
that buffer as the first completion candidate.

> > More generally, I think it's wrong to try to make XREF buffers behave
> > like *Grep* buffers.  They are different beasts, I think we
> > established this long ago, when Xref was added to Emacs (I think I
> > even made a comment regarding the difference, and Dmitry responded to
> > the effect that this was intended).  *XREF* and *Grep* look
> > differently and behave differently, and it's impossible to make them
> > be similar enough.
> 
> They might be different when *xref* is used as a completions buffer
> when the *xref* buffer is created by 'M-.' (xref-find-definitions).
> 
> Moreover, now it's possible to use the real *Completions* buffer by
> customizing xref-show-definitions-function to xref--show-defs-minibuffer.
> Even is this case TAB doesn't close the *Completions* buffer.
> 
> But a completely separate case is when the *xref* buffer is created
> by such grep-like commands as project-find-regexp.

Perhaps those other cases need a special variant of xref-mode, where
TAB could have a binding you'd like to see.

> So currently there are two different uses of the *xref* buffer:
> 
> 1. as a transient completions buffer that could be buried
>    after a completion is selected from the buffer;
> 2. as a grep buffer for visiting the found matches.
> 
> In the second case, it's natural to type TAB to navigate results.

If you mean that the first use is as a kind of "completion" buffer,
then I disagree: at least by default it doesn't look and doesn't
behave like a list of completions.  It's an entirely different beast,
unlike anything else I know of that we have in Emacs core.

> Should these cases use different modes with different buffer names?
> Such as a buffer name *xgrep*?

I think it's a slippery slope to take a minor very specialized mode
and start adding variants to it.  But if you insist on having TAB go
to the next candidate, perhaps we will have no other choice than to
take that path.

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

>> Should these cases use different modes with different buffer names?
>
> I think it's a slippery slope to take a minor very specialized mode
> and start adding variants to it.  But if you insist on having TAB go
> to the next candidate, perhaps we will have no other choice than to
> take that path.

There are already two different modes: the first is
defined by the user option xref-show-definitions-function
(by default, xref--show-defs-buffer), the second is defined by
xref-show-xrefs-function (by default, xref--show-xref-buffer).

It's fine to use TAB to bury the output buffer in the first case,
but not in the second case.

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: dgutov@yandex.ru,  joaotavora@gmail.com,  44611@debbugs.gnu.org
> Date: Tue, 22 Dec 2020 23:20:37 +0200
> 
> >> Should these cases use different modes with different buffer names?
> >
> > I think it's a slippery slope to take a minor very specialized mode
> > and start adding variants to it.  But if you insist on having TAB go
> > to the next candidate, perhaps we will have no other choice than to
> > take that path.
> 
> There are already two different modes: the first is
> defined by the user option xref-show-definitions-function
> (by default, xref--show-defs-buffer), the second is defined by
> xref-show-xrefs-function (by default, xref--show-xref-buffer).
> 
> It's fine to use TAB to bury the output buffer in the first case,
> but not in the second case.

I'm okay with this distinction, provided we decide that we will try to
keep only these two variants, as much as possible.

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

Hi Eli,

On 23.12.2020 17:08, Eli Zaretskii wrote:
>> There are already two different modes: the first is
>> defined by the user option xref-show-definitions-function
>> (by default, xref--show-defs-buffer), the second is defined by
>> xref-show-xrefs-function (by default, xref--show-xref-buffer).
>>
>> It's fine to use TAB to bury the output buffer in the first case,
>> but not in the second case.
> I'm okay with this distinction, provided we decide that we will try to
> keep only these two variants, as much as possible.

Let me get this straight:

You oppose changing the TAB binding, which is currently documented to be 
present in the UI for both cases (and *is* present).

But you're fine with creating a +1 subtly different behavior between the 
cases, by changing the TAB binding in only one of them?

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> Cc: joaotavora@gmail.com, 44611@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 23 Dec 2020 18:20:29 +0200
> 
> > I'm okay with this distinction, provided we decide that we will try to
> > keep only these two variants, as much as possible.
> 
> Let me get this straight:
> 
> You oppose changing the TAB binding, which is currently documented to be 
> present in the UI for both cases (and *is* present).
> 
> But you're fine with creating a +1 subtly different behavior between the 
> cases, by changing the TAB binding in only one of them?

Not exactly.

I still prefer that we don't change the TAB binding at all.  But since
Juri applies pressure (and evidently doesn't consider this difference
between the behaviors in these two cases to be an issue), I yield to
that pressure by the above compromise.

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 23.12.2020 18:46, Eli Zaretskii wrote:
> I still prefer that we don't change the TAB binding at all.  But since
> Juri applies pressure (and evidently doesn't consider this difference
> between the behaviors in these two cases to be an issue)

I do consider it a problem, however.

Having a less intuitive and more error-prone resulting interface just 
because we're afraid to change things (any thing?) is bad.

Why fight for this binding, really? It hasn't been there *that* long, 
and apparently it has zero known users. The original author included.

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> Cc: juri@linkov.net, joaotavora@gmail.com, 44611@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 23 Dec 2020 20:45:22 +0200
> 
> Having a less intuitive and more error-prone resulting interface just 
> because we're afraid to change things (any thing?) is bad.

We are not afraid.  We are not doing it, but not because of fear.

> Why fight for this binding, really? It hasn't been there *that* long, 

It's there since May 2018, and by the time Emacs 28 will be released
it will have existed for 4 years or so and 2 major releases.  That's
long enough for us not to change it.  People who want Grep-like
bindings can always rebind TAB.

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 23.12.2020 21:23, Eli Zaretskii wrote:
> It's there since May 2018, and by the time Emacs 28 will be released
> it will have existed for 4 years or so and 2 major releases.  That's
> long enough for us not to change it.

So is that the rule?

Never change a binding that has been with us for >4 years, no matter the 
arguments?

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> Cc: juri@linkov.net, joaotavora@gmail.com, 44611@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 23 Dec 2020 21:56:02 +0200
> 
> On 23.12.2020 21:23, Eli Zaretskii wrote:
> > It's there since May 2018, and by the time Emacs 28 will be released
> > it will have existed for 4 years or so and 2 major releases.  That's
> > long enough for us not to change it.
> 
> So is that the rule?
> 
> Never change a binding that has been with us for >4 years, no matter the 
> arguments?

If you want to continue this discussion, please drop the attitude and
try to see the issue from my POV.  Otherwise I see no reason to
continue, as I've-been-there-done-that too many times.

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

>> > I'm okay with this distinction, provided we decide that we will try to
>> > keep only these two variants, as much as possible.
>>
>> Let me get this straight:
>>
>> You oppose changing the TAB binding, which is currently documented to be
>> present in the UI for both cases (and *is* present).
>>
>> But you're fine with creating a +1 subtly different behavior between the
>> cases, by changing the TAB binding in only one of them?
>
> Not exactly.
>
> I still prefer that we don't change the TAB binding at all.  But since
> Juri applies pressure (and evidently doesn't consider this difference
> between the behaviors in these two cases to be an issue), I yield to
> that pressure by the above compromise.

I'm not pressuring anyone, mind you.  This is part of efforts to develop
project-find-regexp as a replacement of rgrep.  The previous task was
to remove duplicate output lines that is successfully completed now.
The next tasks are support for multi-line search strings, word search, etc.
Bringing keybindings of project-find-regexp closer to grep mode is one
of such tasks that will help the users to adapt grep usage patterns to
project-find-regexp and other grep-like xref commands.

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 23.12.2020 22:30, Eli Zaretskii wrote:

> If you want to continue this discussion, please drop the attitude and
> try to see the issue from my POV.  Otherwise I see no reason to
> continue, as I've-been-there-done-that too many times.

Could you try seeing it from my POV?

This is the first time I've heard about a rule like that regarding key 
bindings (and not just changes in existing functions, ones that would 
break existing configurations and third-party code). If I knew about it, 
I wouldn't have approved that key binding in the first place, those 4 
years ago. Because "we can't change it later" is a pretty high standard 
for acceptance.

And now it feels like you're like changing the rules under my feet. It's 
not a good way to make a subsystem maintainer feel.

Since there apparently was some sort of misunderstanding here, could we 
maybe proceed like this:

- You allow me the change of this binding, this one time.
- You add some documentation to our "best practices" somewhere, which 
describes this rule, so that I/we can be careful in the future with 
additions to new keybindings, and with changes to existing ones.

And if that rule is documented already, could you please point it out?

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Dmitry Gutov <dgutov@yandex.ru>,  joaotavora@gmail.com,
>   44611@debbugs.gnu.org
> Date: Wed, 23 Dec 2020 23:10:13 +0200
> 
> This is part of efforts to develop
> project-find-regexp as a replacement of rgrep.  The previous task was
> to remove duplicate output lines that is successfully completed now.
> The next tasks are support for multi-line search strings, word search, etc.
> Bringing keybindings of project-find-regexp closer to grep mode is one
> of such tasks that will help the users to adapt grep usage patterns to
> project-find-regexp and other grep-like xref commands.

Why does project-find-regexp need to use Xref for displaying the hits?
why not use Grep-like display we use in *grep* buffers?

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> Cc: juri@linkov.net, joaotavora@gmail.com, 44611@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 23 Dec 2020 23:24:23 +0200
> 
> On 23.12.2020 22:30, Eli Zaretskii wrote:
> 
> > If you want to continue this discussion, please drop the attitude and
> > try to see the issue from my POV.  Otherwise I see no reason to
> > continue, as I've-been-there-done-that too many times.
> 
> Could you try seeing it from my POV?
> 
> This is the first time I've heard about a rule like that regarding key 
> bindings (and not just changes in existing functions, ones that would 
> break existing configurations and third-party code). If I knew about it, 
> I wouldn't have approved that key binding in the first place, those 4 
> years ago. Because "we can't change it later" is a pretty high standard 
> for acceptance.
> 
> And now it feels like you're like changing the rules under my feet. It's 
> not a good way to make a subsystem maintainer feel.

There is no new rule.  I apologize if something I said could be
interpreted that way.

There's only one rule: not to make backward-incompatible changes
without a very good reason.  That is not limited to changes in
functions and APIs, key bindings are definitely included.  In fact, I
think incompatible changes in key bindings are even worse than changes
in code, since they might go against muscle memory, and nothing can
fix that except a long and painful process of unlearning.

I've reviewed all the NEWS since v24.1, and found only one
incompatible change in key bindings -- which started by declaring the
existing binding obsolete.  So I think we change key bindings in
incompatible ways only very rarely and carefully, and that is a Good
Thing.

Given the single precedent I found, I'm fine with declaring the
current binding of TAB obsolete and providing a replacement for it
('b'? 'q'?), so that we could replace it in some future version after
28, if that is okay with you and Juri.

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

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

> There's only one rule: not to make backward-incompatible changes
> without a very good reason.

With further development of the search commands based on xref, more
users perceive it as a grep replacement, that however is not based
on grep mode, so this is a good reason to make xref keybindings more
compatible with grep mode.

> Given the single precedent I found, I'm fine with declaring the
> current binding of TAB obsolete and providing a replacement for it
> ('b'? 'q'?), so that we could replace it in some future version after
> 28, if that is okay with you and Juri.

Thanks, this is surely okay.  So here is the patch that does this.
It binds 'xref-quit-and-goto-xref' to 'C-j' like in icomplete mode:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xref-quit-and-goto-xref.patch --]
[-- Type: text/x-diff, Size: 2433 bytes --]

diff --git a/etc/NEWS b/etc/NEWS
index b155ff9d42..14dba83368 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1820,6 +1820,11 @@ first).
 \f
 * Incompatible Editing Changes in Emacs 28.1
 
+** In xref, 'TAB' now moves to the next xref instead of quitting *xref* buffer.
+Using 'TAB' / 'S-TAB' key bindings to navigate the search results
+is more in the line with other modes like grep mode.  The command
+'xref-quit-and-goto-xref' is now bound to 'C-j' like in icomplete.
+
 ** In 'nroff-mode', 'center-line' is now bound to 'M-o M-s'.
 The original key binding was 'M-s', which interfered with I-search,
 since the latter uses 'M-s' as a prefix key of the search prefix map.
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 181f94b0bc..8b48396495 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -585,16 +585,26 @@ xref-show-location-at-point
     (when xref
       (xref--show-location (xref-item-location xref)))))
 
+(defun xref-next-line-no-select ()
+  "Move to the next xref but don't display its source."
+  (interactive)
+  (xref--search-property 'xref-item))
+
 (defun xref-next-line ()
   "Move to the next xref and display its source in the appropriate window."
   (interactive)
-  (xref--search-property 'xref-item)
+  (xref-next-line-no-select)
   (xref-show-location-at-point))
 
+(defun xref-prev-line-no-select ()
+  "Move to the previous xref but don't display its source."
+  (interactive)
+  (xref--search-property 'xref-item t))
+
 (defun xref-prev-line ()
   "Move to the previous xref and display its source in the appropriate window."
   (interactive)
-  (xref--search-property 'xref-item t)
+  (xref-prev-line-no-select)
   (xref-show-location-at-point))
 
 (defun xref-next-group ()
@@ -765,7 +775,9 @@ xref--xref-buffer-mode-map
     (define-key map (kbd "P") #'xref-prev-group)
     (define-key map (kbd "r") #'xref-query-replace-in-results)
     (define-key map (kbd "RET") #'xref-goto-xref)
-    (define-key map (kbd "TAB")  #'xref-quit-and-goto-xref)
+    (define-key map (kbd "C-j")  #'xref-quit-and-goto-xref)
+    (define-key map "\t" 'xref-next-line-no-select) ; like compilation-next-error
+    (define-key map [backtab] 'xref-prev-line-no-select) ; like compilation-previous-error
     (define-key map (kbd "C-o") #'xref-show-location-at-point)
     ;; suggested by Johan Claesson "to further reduce finger movement":
     (define-key map (kbd ".") #'xref-next-line)

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Dmitry Gutov <dgutov@yandex.ru>,  joaotavora@gmail.com,
>   44611@debbugs.gnu.org
> Date: Thu, 24 Dec 2020 22:19:16 +0200
> 
> With further development of the search commands based on xref, more
> users perceive it as a grep replacement, that however is not based
> on grep mode, so this is a good reason to make xref keybindings more
> compatible with grep mode.

You didn't answer my question why not use the format we use in *grep*
buffers or *occur* buffers.

> > Given the single precedent I found, I'm fine with declaring the
> > current binding of TAB obsolete and providing a replacement for it
> > ('b'? 'q'?), so that we could replace it in some future version after
> > 28, if that is okay with you and Juri.
> 
> Thanks, this is surely okay.  So here is the patch that does this.
> It binds 'xref-quit-and-goto-xref' to 'C-j' like in icomplete mode:

That's not what I meant: I meant to leave the TAB binding for now at
what it is now, while declaring it obsolete in NEWS, and provide an
alternative for it that will remain when we rebind TAB to another
command.  But the rebinding of TAB will have to wait until after Emacs
28.  That's the meaning of "obsolete": we don't remove it right away,
but first warn users to get prepared for a removal in the future.

And why C-j?  That's LFD, a key more suitable for acting on something
at point, not quitting the buffer.  Why not 'q' (for "quit") or 'b'
(for "bury")?

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 24.12.2020 05:36, Eli Zaretskii wrote:

> Why does project-find-regexp need to use Xref for displaying the hits?
> why not use Grep-like display we use in *grep* buffers?

These days the default Xref buffer is pretty much Grep-like. Certainly 
much closer to grep-mode than it had been in the first versions. That 
old UI was a lot more completion-like in its behavior.

This happened gradually, after we have addressed feedback from you and 
other users, so that xref-find-references and project-find-regexp behave 
  more in a fashion that you would expect from it. And those 
expectations were surely informed by Grep and other built-in modes.

Why not use something else "Grep-like"? Well, one objective reason is 
Xref is the only UI we have which is rendered programmatically based on 
a list of standardized values representing matches. Anything else would 
need work done for this to happen. And Xref has more features:

- It has a "replace in matches" command.
- Its UI with grouping by file names saves horizontal space, and it's 
actually how Grep-like search tools look these days (see ag, pt or 
ripgrep). GNU Grep doesn't have an option for such output format, so we 
help it in Elisp.
- Finally, its UI presenter is customizable, so anyone who doesn't like 
it can plug in something else that can interpret a list of "xref match" 
values. That is a good thing.

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> Cc: joaotavora@gmail.com, 44611@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 24 Dec 2020 23:38:52 +0200
> 
> On 24.12.2020 05:36, Eli Zaretskii wrote:
> 
> > Why does project-find-regexp need to use Xref for displaying the hits?
> > why not use Grep-like display we use in *grep* buffers?
> 
> These days the default Xref buffer is pretty much Grep-like. Certainly 
> much closer to grep-mode than it had been in the first versions. That 
> old UI was a lot more completion-like in its behavior.
> 
> This happened gradually, after we have addressed feedback from you and 
> other users, so that xref-find-references and project-find-regexp behave 
>   more in a fashion that you would expect from it. And those 
> expectations were surely informed by Grep and other built-in modes.

Then why do we need to have 2 separate modes?  Are there any plans for
replacing grep-mode with xref-mode in all the commands that use the
former?  Do we also want to replace compilation-mode with xref-mode?

If the plan is to make such replacements, that could be a good idea,
and we can discuss the roadmap towards that goal.  Such a roadmap
should include a transition plan that will help people migrate as
painlessly as possible, including the key bindings and the somewhat
different looks.  And perhaps _then_ we will have a good enough reason
we don't have now to rebind xref-mode's TAB in Emacs 28.

I didn't see any discussions to that effect, and in any case such
discussions need to be on emacs-devel, not here.  If the intent is
indeed to make such a migration, let's announce the plan, and let's
define the roadmap which we want to follow.  It's a significant
change, and we should be careful in how we do it.

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

>> With further development of the search commands based on xref, more
>> users perceive it as a grep replacement, that however is not based
>> on grep mode, so this is a good reason to make xref keybindings more
>> compatible with grep mode.
>
> You didn't answer my question why not use the format we use in *grep*
> buffers or *occur* buffers.

Sorry, I don't know an answer to this question.  But fortunately
Dmitry answered your question.

> That's not what I meant: I meant to leave the TAB binding for now at
> what it is now, while declaring it obsolete in NEWS, and provide an
> alternative for it that will remain when we rebind TAB to another
> command.  But the rebinding of TAB will have to wait until after Emacs
> 28.  That's the meaning of "obsolete": we don't remove it right away,
> but first warn users to get prepared for a removal in the future.

Big corporations don't afraid making much more fundamental changes
that affect billions of users.  For example, on smartphone OS
they can do such a change that on the Task list the same gesture
will remove the wrong task than on older versions.  Also major sites
with billions of users often change their UI completely without hesitation.

So I don't understand such extreme precautions.  Unlike the above examples,
in Emacs everything is configurable, so you can easily add to the init file:

  (define-key xref--xref-buffer-mode-map (kbd "TAB") #'xref-quit-and-goto-xref)

But having no command bound to TAB is a lose-lose situation because
users of grep-like xref commands will have less compatible keys.

> And why C-j?  That's LFD, a key more suitable for acting on something
> at point, not quitting the buffer.

The initial xref UI was closer to completion UI, so C-j makes it consistent
with icomplete mode for users who still perceive xref as completion UI.

> Why not 'q' (for "quit") or 'b' (for "bury")?

'xref-quit-and-goto-xref' also jumps to xref, not only quits,
but 'q' and 'b' should only quit.

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: dgutov@yandex.ru,  joaotavora@gmail.com,  44611@debbugs.gnu.org
> Date: Fri, 25 Dec 2020 11:20:19 +0200
> 
> >> With further development of the search commands based on xref, more
> >> users perceive it as a grep replacement, that however is not based
> >> on grep mode, so this is a good reason to make xref keybindings more
> >> compatible with grep mode.
> >
> > You didn't answer my question why not use the format we use in *grep*
> > buffers or *occur* buffers.
> 
> Sorry, I don't know an answer to this question.  But fortunately
> Dmitry answered your question.

He did, yes.

> Big corporations don't afraid making much more fundamental changes
> that affect billions of users.  For example, on smartphone OS
> they can do such a change that on the Task list the same gesture
> will remove the wrong task than on older versions.  Also major sites
> with billions of users often change their UI completely without hesitation.
> 
> So I don't understand such extreme precautions.

Do you think that what "big corporations" do is okay?  If you do, then
I understand why you consider it OK for us to do something similar.
But if you don't, then why do you suggest that we follow bad examples?

I don't think frequent changes in the UI and the defaults are a Good
Thing.  Each time I upgrade some software which comes from those big
corporations, I have to waste some significant time to get many of my
previous defaults back, disable or reconfigure annoying new features
etc.  Moreover, no matter how conservative we are in Emacs with
breaking backward compatibility, we are still being occasionally
accused of not being conservative enough.  So evidently, veteran users
don't like such changes to happen too frequently.  Looking at NEWS for
past releases, I conclude that we indeed didn't make such changes.

> Unlike the above examples,
> in Emacs everything is configurable, so you can easily add to the init file:
> 
>   (define-key xref--xref-buffer-mode-map (kbd "TAB") #'xref-quit-and-goto-xref)

This works both ways: if you want TAB to do something other than its
default, you can rebind it for you, right?

> But having no command bound to TAB is a lose-lose situation because
> users of grep-like xref commands will have less compatible keys.

I didn't suggest unbinding TAB, I suggested to leave its current
binding as it was in Emacs 27 and older.

> > And why C-j?  That's LFD, a key more suitable for acting on something
> > at point, not quitting the buffer.
> 
> The initial xref UI was closer to completion UI, so C-j makes it consistent
> with icomplete mode for users who still perceive xref as completion UI.

That contradicts what Dmitry said: that the current Xref UI moves away
of the completion UI, and towards compilation-mode and grep-mode.
There, deleting the window with C-j will look odd, I think.

> > Why not 'q' (for "quit") or 'b' (for "bury")?
> 
> 'xref-quit-and-goto-xref' also jumps to xref, not only quits,
> but 'q' and 'b' should only quit.

That's not a catastrophe, but if you want, we could use "C-c C-c"
instead, as that is used in some other modes for similar effect.

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 25.12.2020 09:37, Eli Zaretskii wrote:

>>> Why does project-find-regexp need to use Xref for displaying the hits?
>>> why not use Grep-like display we use in *grep* buffers?
>>
>> These days the default Xref buffer is pretty much Grep-like. Certainly
>> much closer to grep-mode than it had been in the first versions. That
>> old UI was a lot more completion-like in its behavior.
>>
>> This happened gradually, after we have addressed feedback from you and
>> other users, so that xref-find-references and project-find-regexp behave
>>    more in a fashion that you would expect from it. And those
>> expectations were surely informed by Grep and other built-in modes.
> 
> Then why do we need to have 2 separate modes?

Well, Grep (and similar major modes people wrote in third-part packages) 
do have a certain advantage: its execution is asynchronous, and the user 
sees the results as soon as they arrive, even during a search over a 
huge number of files. This can be implemented for xref, with certain 
changes in the API, and with some use of threads, but that's still a 
research problem.

Grep also has less computational overhead. I have spent quite some 
effort to reduce the gap, and xref works fine for me in that regard, but 
with the amount of user feedback I/we generally receive for improvements 
in core features (i.e. close to none) I can't be sure whether there are 
people who avoid 'M-x project-find-regexp' because Grep feels faster 
anyway.

> Are there any plans for
> replacing grep-mode with xref-mode in all the commands that use the
> former?

I think we'd need a more concerted effort for something like that, 
rather than just myself. To get such volunteers, making the current Grep 
users even more satisfied with the current state of 
xref--xref-buffer-mode is a good step, with much upside and really no 
downside (at least to a point where we'd have to compromise on the design).

I can outline a possible roadmap for improving xref, making its code 
structure and data more logical (which includes moving some of them 
out), but I don't think we'll throw out 'M-x grep' away anytime soon.

Changing the latter to use the xref UI (which will have to be renamed 
and become a separate package, BTW) is also likely to encounter much 
bigger resistance than anything we've done in this area before: people 
don't have the same expectations for new commands as they have for 
existing ones, so I'm surprised you asked this (given your overall 
backward compatibility stance, much stronger than mine). But we can try.

Up until now, we've been using xref mostly in places where there its 
programmatic interface is a strong advantage.

> Do we also want to replace compilation-mode with xref-mode?

That wouldn't work. It's possible to write a similar alternative for 
compilation-mode, but that would be a different project. It could reuse 
some ideas, though.

> If the plan is to make such replacements, that could be a good idea,
> and we can discuss the roadmap towards that goal.  Such a roadmap
> should include a transition plan that will help people migrate as
> painlessly as possible, including the key bindings and the somewhat
> different looks.

It's a big change, one I'm not at all confident we could manage before 
Emacs 28 is released.

Do you really want this to happen, though? I never got the impression 
that you enjoy using xref.

> And perhaps _then_ we will have a good enough reason
> we don't have now to rebind xref-mode's TAB in Emacs 28.

Let me get back to that in another subthread.

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> Cc: juri@linkov.net, joaotavora@gmail.com, 44611@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 25 Dec 2020 14:14:30 +0200
> 
> > Then why do we need to have 2 separate modes?
> 
> Well, Grep (and similar major modes people wrote in third-part packages) 
> do have a certain advantage: its execution is asynchronous, and the user 
> sees the results as soon as they arrive, even during a search over a 
> huge number of files. This can be implemented for xref, with certain 
> changes in the API, and with some use of threads, but that's still a 
> research problem.

I'm probably missing something: why cannot we display the Grep hits
received asynchronously in the Xref format?  That would "just" mean
changes in the filter function, right?  IOW, I'm confused by your
reference to asynchronous execution, which AFAIU has nothing to do
with the form in which we present the results, nor with the major mode
in which we put the buffer where the results are displayed.

> > Are there any plans for
> > replacing grep-mode with xref-mode in all the commands that use the
> > former?
> 
> I think we'd need a more concerted effort for something like that, 
> rather than just myself. To get such volunteers, making the current Grep 
> users even more satisfied with the current state of 
> xref--xref-buffer-mode is a good step, with much upside and really no 
> downside (at least to a point where we'd have to compromise on the design).
> 
> I can outline a possible roadmap for improving xref, making its code 
> structure and data more logical (which includes moving some of them 
> out), but I don't think we'll throw out 'M-x grep' away anytime soon.

We don't need to throw out Grep, not right away anyway.  We just need
a plan and a roadmap, and a decision that this is the long-term goal.
And even when we have "M-x grep" showing results in Xref mode, it will
still need at first be an optional feature, so people who are too used
to the old presentation could have it.

> Changing the latter to use the xref UI (which will have to be renamed 
> and become a separate package, BTW) is also likely to encounter much 
> bigger resistance than anything we've done in this area before: people 
> don't have the same expectations for new commands as they have for 
> existing ones, so I'm surprised you asked this (given your overall 
> backward compatibility stance, much stronger than mine).

An optional feature cannot hurt, even if and when it becomes the
default.  Thus, there's no need for me to object to such long-term
plans, if they are announced and proceed at a controlled pace
(including the decision when it becomes the default).

> > Do we also want to replace compilation-mode with xref-mode?
> 
> That wouldn't work.

I don't understand why.  Can you explain?

To clarify, all I'm talking about is a possibility to present compiler
messages in Xref format.  They are similar enough and show the same
information, so why won't that work?

> > If the plan is to make such replacements, that could be a good idea,
> > and we can discuss the roadmap towards that goal.  Such a roadmap
> > should include a transition plan that will help people migrate as
> > painlessly as possible, including the key bindings and the somewhat
> > different looks.
> 
> It's a big change, one I'm not at all confident we could manage before 
> Emacs 28 is released.

It doesn't all have to happen in Emacs 28.

> Do you really want this to happen, though?

I don't know, I just had this thought and wanted to see what others
think.  However, if that isn't the plan, then I really don't
understand the urge to make Xref be like Grep mode.  If they are
indeed different beasts, why make them similar?

> I never got the impression that you enjoy using xref.

Aside of the fact that I use it every hour of every day, you mean?

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 25.12.2020 15:32, Eli Zaretskii wrote:
>> Cc: juri@linkov.net, joaotavora@gmail.com, 44611@debbugs.gnu.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Fri, 25 Dec 2020 14:14:30 +0200
>>
>>> Then why do we need to have 2 separate modes?
>>
>> Well, Grep (and similar major modes people wrote in third-part packages)
>> do have a certain advantage: its execution is asynchronous, and the user
>> sees the results as soon as they arrive, even during a search over a
>> huge number of files. This can be implemented for xref, with certain
>> changes in the API, and with some use of threads, but that's still a
>> research problem.
> 
> I'm probably missing something: why cannot we display the Grep hits
> received asynchronously in the Xref format?  That would "just" mean
> changes in the filter function, right?

Yes, but that would involve creating a different implementation that 
renders in the same format, which we'll have to then keep in sync with 
the original implementation.

I wouldn't mind, of course. If someone wants to bring Grep closer to 
Xref in presentation, and no code sharing is required, I don't even have 
to participate.

Note that if Grep still doesn't share the underlying data format with 
Xref, xref-query-replace-in-results won't work. The jump-to-location 
commands will also need to have different implementations, so we won't 
be able to properly reuse the major mode either.

> IOW, I'm confused by your
> reference to asynchronous execution, which AFAIU has nothing to do
> with the form in which we present the results, nor with the major mode
> in which we put the buffer where the results are displayed.

The major mode also includes the bindings. Its rendering function also 
assumes synchronous execution currently, and the way it retrieves the 
data pieces to render depends on the data format.

>> I can outline a possible roadmap for improving xref, making its code
>> structure and data more logical (which includes moving some of them
>> out), but I don't think we'll throw out 'M-x grep' away anytime soon.
> 
> We don't need to throw out Grep, not right away anyway.  We just need
> a plan and a roadmap, and a decision that this is the long-term goal.
> And even when we have "M-x grep" showing results in Xref mode, it will
> still need at first be an optional feature, so people who are too used
> to the old presentation could have it.

xref-show-xrefs-function could have a Grep-like option. Although that 
one couldn't be the default, or else we'll force the xref commands to 
use it as well, which would be a step back.

>> Changing the latter to use the xref UI (which will have to be renamed
>> and become a separate package, BTW) is also likely to encounter much
>> bigger resistance than anything we've done in this area before: people
>> don't have the same expectations for new commands as they have for
>> existing ones, so I'm surprised you asked this (given your overall
>> backward compatibility stance, much stronger than mine).
> 
> An optional feature cannot hurt, even if and when it becomes the
> default.  Thus, there's no need for me to object to such long-term
> plans, if they are announced and proceed at a controlled pace
> (including the decision when it becomes the default).

This endeavor might need more of an encouragement than "I don't object".

>>> Do we also want to replace compilation-mode with xref-mode?
>>
>> That wouldn't work.
> 
> I don't understand why.  Can you explain?
> 
> To clarify, all I'm talking about is a possibility to present compiler
> messages in Xref format.  They are similar enough and show the same
> information, so why won't that work?

The format is not exactly the same: compilation log includes both the 
location (file name, line, column), the contents of a line, and the 
error message itself (which could span several lines). That doesn't fit 
the current data model.

I mean, we could extend it, but IME the main complexity is not in how to 
make the buffer look similar enough (the main rendering function, 
xref--insert-xrefs, is just 50 lines long), but how to make the buffer 
work usefully overall. I.e. with compilation you usually want to see 
different kind of errors/warnings/infos denoted prominently, and you 
don't usually (ever?) want to search-replace across the results, like 
you'd want with Grep. OTOH, there was that idea about quick fixes which 
you might add with supporting compilers, and for which there is no 
counterpart in Grep search results.

Finally, a compilation log can often include unstructured information 
like free-form logging, which is very often the case in 
rspec-compilation-mode, which is my main point of interaction with 
compilation-mode. Which would make the data format fall outside Xref's 
data model even more. As such, I'm probably not a good person to take 
point on developing that feature.

>> Do you really want this to happen, though?
> 
> I don't know, I just had this thought and wanted to see what others
> think.  However, if that isn't the plan, then I really don't
> understand the urge to make Xref be like Grep mode.  If they are
> indeed different beasts, why make them similar?

They are already fairly close. But there is some obvious usability 
benefit in having similar key bindings in modes that even serve 
different purposes: users will need to remember fewer bindings, and keep 
fewer differences in mind. Especially when the same key does one thing 
in one mode and something starkly different in another.

The more different the two modes are, the more often we won't be able to 
manage that, of course.

Finally, I don't make any plans like that because currently I can only 
focus on what looked like obvious missing spots in Emacs's built-in 
feature set (xref, project and etags-regen all came from that outlook). 
First I try to make the UI work well, and the unification can come 
later. Someday, someone will come and ask:

   Why do both project and dired call xref to perform their searches and 
render the results? That seems like a different kind of responsibility, 
and should be in a separate feature with well-defined semantics. Maybe 
even two different features.

And then they'll hopefully lend some of their spare time to make that 
happen. To get there, though, the more comfortable we make the Xref UI 
for all current and future users, the better.

>> I never got the impression that you enjoy using xref.
> 
> Aside of the fact that I use it every hour of every day, you mean?

Well, even that is not a given for me (in a recent Reddit comment you 
said that you don't use any of the "alternative" packages created in the 
last 10 years, but I guess the built-in ones are exceptions).

Or you could feel it's more-or-less good enough to replace the find-tag 
UI, or at least not painful enough to demand the new bindings be 
reversed. Or you could think it's better than find-tag, but grep-mode is 
still a better interface for what Grep does.

Also, in my usage, xref-find-definitions almost never pops up the xref 
UI. xref-find-references does, though (though I don't use it often). And 
IIUC you don't really use project-find-regexp.

I'm glad you do like it, though, if I understood you correctly now.

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 25.12.2020 13:44, Eli Zaretskii wrote:

>> Big corporations don't afraid making much more fundamental changes
>> that affect billions of users.  For example, on smartphone OS
>> they can do such a change that on the Task list the same gesture
>> will remove the wrong task than on older versions.  Also major sites
>> with billions of users often change their UI completely without hesitation.
>>
>> So I don't understand such extreme precautions.
> 
> Do you think that what "big corporations" do is okay?  If you do, then
> I understand why you consider it OK for us to do something similar.
> But if you don't, then why do you suggest that we follow bad examples?

There are different pieces of software coming from "big corporations", 
and there.

All I know that, say, authors of editors like VS Code, Sublime, etc, are 
not afraid to change things from time to time, if their search says it 
results in a better experience. And I haven't seen many user complaints 
about that.

OTOH, it might very well be a part of their recipe for success.

> I don't think frequent changes in the UI and the defaults are a Good
> Thing.  Each time I upgrade some software which comes from those big
> corporations, I have to waste some significant time to get many of my
> previous defaults back, disable or reconfigure annoying new features
> etc.  Moreover, no matter how conservative we are in Emacs with
> breaking backward compatibility, we are still being occasionally
> accused of not being conservative enough.  So evidently, veteran users
> don't like such changes to happen too frequently.  Looking at NEWS for
> past releases, I conclude that we indeed didn't make such changes.

We are accused either way: we're both not conservative enough on one 
side, and too conservative on the other. Which judgment to give 
preference to, is basically a personal choice at this point. Or a 
strategic one.

It's an old disagreement, but I think it should be fairly obvious that 
if we fear to cause discomfort to even one veteran user, we won't be 
able to achieve much progress.

And it's not like the question here it to change a lot or bindings, or a 
significant/popular one.

>> Unlike the above examples,
>> in Emacs everything is configurable, so you can easily add to the init file:
>>
>>    (define-key xref--xref-buffer-mode-map (kbd "TAB") #'xref-quit-and-goto-xref)
> 
> This works both ways: if you want TAB to do something other than its
> default, you can rebind it for you, right?

It is a question of which is a better/more consistent behavior.

>>> And why C-j?  That's LFD, a key more suitable for acting on something
>>> at point, not quitting the buffer.
>>
>> The initial xref UI was closer to completion UI, so C-j makes it consistent
>> with icomplete mode for users who still perceive xref as completion UI.
> 
> That contradicts what Dmitry said: that the current Xref UI moves away
> of the completion UI, and towards compilation-mode and grep-mode.

Just like that TAB binding was a bit alien to the state of the Xref UI 
as it was 4 years ago, that binding will probably be as well.

But you can also interpret it like "complete the current action". 
Icomplete has that binding, but lisp-interaction-mode has 
eval-print-last-sexp, which is vaguely relevant too.

It's not a perfect parallel, but I think 'C-j' is better for this 
purpose than TAB, and also better than 'q' or 'b'.

On the subject of grep-mode, it could have a similar command introduced 
too, for situations when you only needed to find one occurrence, to jump 
to it and quit the window.

> There, deleting the window with C-j will look odd, I think.
> 
>>> Why not 'q' (for "quit") or 'b' (for "bury")?
>>
>> 'xref-quit-and-goto-xref' also jumps to xref, not only quits,
>> but 'q' and 'b' should only quit.
> 
> That's not a catastrophe, but if you want, we could use "C-c C-c"
> instead, as that is used in some other modes for similar effect.

'C-c C-c' usually acts on the whole buffer contents and does not depend 
on point, I think.

It might also conflict with the potential "inline editing" feature 
if/when we get that into Xref. Though it could use 'C-x C-s', *shrug*.

To be clear, I'm not wedded to 'C-j', and open to other options. We 
could even make xref-quit-and-goto-xref unbound, except in 
xref--transient-buffer-mode-map. Which is the most "completion-like" 
version of Xref UI we currently have anyway.

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> Cc: juri@linkov.net, joaotavora@gmail.com, 44611@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 25 Dec 2020 16:49:39 +0200
> 
> >> Changing the latter to use the xref UI (which will have to be renamed
> >> and become a separate package, BTW) is also likely to encounter much
> >> bigger resistance than anything we've done in this area before: people
> >> don't have the same expectations for new commands as they have for
> >> existing ones, so I'm surprised you asked this (given your overall
> >> backward compatibility stance, much stronger than mine).
> > 
> > An optional feature cannot hurt, even if and when it becomes the
> > default.  Thus, there's no need for me to object to such long-term
> > plans, if they are announced and proceed at a controlled pace
> > (including the decision when it becomes the default).
> 
> This endeavor might need more of an encouragement than "I don't object".

I didn't have encouragement in mind.  I'm asking if this is the plan,
or some kind of desire.  If it is, then I can understand that.  If
there's no such plan, then I don't think I understand why we need to
make Xref behave similarly and have the same key bindings.

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 24.12.2020 19:44, Eli Zaretskii wrote:

> There's only one rule: not to make backward-incompatible changes
> without a very good reason.  That is not limited to changes in
> functions and APIs, key bindings are definitely included.  In fact, I
> think incompatible changes in key bindings are even worse than changes
> in code, since they might go against muscle memory, and nothing can
> fix that except a long and painful process of unlearning.

I'm pretty sure backward incompatible changes in code can be much worse. 
They can break the code a user has been relying on for many years, and 
the user might not have the technical ability to fix that code.

Adapting to a new binding is something that is within anybody's 
capability, though. We shouldn't do those frivolously either, but I 
believe it is something we should be less worried about.

> I've reviewed all the NEWS since v24.1, and found only one
> incompatible change in key bindings -- which started by declaring the
> existing binding obsolete.  So I think we change key bindings in
> incompatible ways only very rarely and carefully, and that is a Good
> Thing.

It has never been my impression that we are too careful about the 
bindings. Let's review the history.

NEWS.27:

   the function
   'package-menu-filter-by-keyword' has been renamed from
   'package-menu-filter'.  Its keybinding has also been changed to '/ k'
   (from 'f')

That binding was changes without an obsoletion period, whereas the 
function itself still has an obsolete-alias. Both are good things, IMO.

NEWS.26:

   *** 'vc-region-history' is now bound to 'C-x v h', replacing the older
   'vc-insert-headers' binding.

The binding changed to a function with an entirely different behavior.

NEWS.25:

   ** The old 'C-x w' bindings in 'hi-lock-mode' are officially
   deprecated in favor of the global 'M-s h' bindings introduced
   in Emacs 23.1. They'll disappear soon.

This is probably the example you're referring to. Note that it talks 
about a whole family of bindings, not just one, and that those bindings 
almost certainly have had their users. They have probably been the main 
way to use hi-lock-mode's functionality at some point in time.

NEWS.24:

   ** New Ibuffer `derived-mode' filter, bound to `/ M'.
   The old binding for `/ M' (filter by used-mode) is now bound to `/ m'.

Bindings changed in an incompatible way (the previous binding had been 
there for 12 years).

   ** Dired

   *** The minibuffer default for `=' (`dired-diff') has changed.
   It is now the backup file for the file at point, if one exists.
   In Transient Mark mode the default is the file at the active mark.

   *** `M-=' is no longer bound to `dired-backup-diff' in Dired buffers.
   The global binding for `M-=', `count-words-region' is in effect.

   ** Ruby mode

   <Lots of key binding changes without advance announcement>

I have omitted a few others too, where the changes have seemingly been 
really critical and thus unlike the present issue.

To sum up, we don't change bindings often (in part because it's hard to 
get everyone to agree on a new default, I guess), but we do that from 
time to time. And out of all examples, there was only one when we 
announced the obsoletion first.

> Given the single precedent I found, I'm fine with declaring the
> current binding of TAB obsolete and providing a replacement for it
> ('b'? 'q'?), so that we could replace it in some future version after
> 28, if that is okay with you and Juri.

Thank you, this is at least one path forward we can take, which is good 
to have. But if that is what we have to do to change that binding, it's 
pretty costly time-wise, so let's discuss some alternatives first.

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

> Do you think that what "big corporations" do is okay?  If you do, then
> I understand why you consider it OK for us to do something similar.
> But if you don't, then why do you suggest that we follow bad examples?

No, this is not the reason to do something similar.  I already
explained the reason below, i.e.: "Unlike the above examples,
in Emacs everything is configurable, so you can easily
add to the init file".

> I don't think frequent changes in the UI and the defaults are a Good
> Thing.  Each time I upgrade some software which comes from those big
> corporations, I have to waste some significant time to get many of my
> previous defaults back, disable or reconfigure annoying new features
> etc.  Moreover, no matter how conservative we are in Emacs with
> breaking backward compatibility, we are still being occasionally
> accused of not being conservative enough.  So evidently, veteran users
> don't like such changes to happen too frequently.  Looking at NEWS for
> past releases, I conclude that we indeed didn't make such changes.

The notion "conservative" is irrelevant here, because TAB was bound in xref
recently, this discussion is not about changing some long-established bindings.

>> Unlike the above examples,
>> in Emacs everything is configurable, so you can easily add to the init file:
>>
>>   (define-key xref--xref-buffer-mode-map (kbd "TAB") #'xref-quit-and-goto-xref)
>
> This works both ways: if you want TAB to do something other than its
> default, you can rebind it for you, right?

This is not about my personal preferences.  For example, my preference
is not to add tab-line-tab-face-special to tab-line-tab-face-functions,
but since everyone wants it, I had to install it as the default value.

>> But having no command bound to TAB is a lose-lose situation because
>> users of grep-like xref commands will have less compatible keys.
>
> I didn't suggest unbinding TAB, I suggested to leave its current
> binding as it was in Emacs 27 and older.

This would be the worst thing to do.  For Emacs 28 we are working
to provide grep-like experience in the xref output, but not adding
a key binding compatible with grep mode would harm our efforts.

>> > And why C-j?  That's LFD, a key more suitable for acting on something
>> > at point, not quitting the buffer.
>>
>> The initial xref UI was closer to completion UI, so C-j makes it consistent
>> with icomplete mode for users who still perceive xref as completion UI.
>
> That contradicts what Dmitry said: that the current Xref UI moves away
> of the completion UI, and towards compilation-mode and grep-mode.
> There, deleting the window with C-j will look odd, I think.

There are two modes: xref-show-xrefs and xref-show-defs.
xref-show-xrefs is similar to grep, whereas xref-show-defs
resembles the Completion UI where C-j makes sense.

>> > Why not 'q' (for "quit") or 'b' (for "bury")?
>>
>> 'xref-quit-and-goto-xref' also jumps to xref, not only quits,
>> but 'q' and 'b' should only quit.
>
> That's not a catastrophe, but if you want, we could use "C-c C-c"
> instead, as that is used in some other modes for similar effect.

I think this is a good idea for completion-like xref-show-defs,
but not for grep-like xref-show-xrefs because in grep-mode
'C-c C-c' is bound to compile-goto-error that doesn't quit window.

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

> On the subject of grep-mode, it could have a similar command introduced
> too, for situations when you only needed to find one occurrence, to jump to
> it and quit the window.

This is what I already successfully use:

diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index d2293151c7..08c44dcb25 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -2572,11 +2572,15 @@ compile-goto-error
   (or (compilation-buffer-p (current-buffer))
       (error "Not in a compilation buffer"))
   (compilation--ensure-parse (point))
+  (let ((window (posn-window (event-end event))))
     (if (get-text-property (point) 'compilation-directory)
         (dired-other-window
          (car (get-text-property (point) 'compilation-directory)))
       (setq compilation-current-error (point))
-    (next-error-internal)))
+      (next-error-internal))
+    (when current-prefix-arg
+      (with-selected-window window
+        (quit-window)))))
 
 ;; This is mostly unused, but we keep it for the sake of some external
 ;; packages which seem to make use of it.

bug#44611: Prefix arg for xref-goto-xref

[martin rudalics]

 > +      (with-selected-window window
 > +        (quit-window)))))

Please always try to use

(quit-restore-window window)

instead of that idiom.

Thanks, martin

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: dgutov@yandex.ru,  joaotavora@gmail.com,  44611@debbugs.gnu.org
> Date: Sun, 27 Dec 2020 18:20:01 +0200
> 
> The notion "conservative" is irrelevant here, because TAB was bound in xref
> recently

Not so recently: 2 years ago.  And by the time Emacs 28 hits the
network, it will probably be 4 years.

> > This works both ways: if you want TAB to do something other than its
> > default, you can rebind it for you, right?
> 
> This is not about my personal preferences.

Your original explanation of the rationale was that you always press
TAB because you are used to it in Grep buffers.

> > I didn't suggest unbinding TAB, I suggested to leave its current
> > binding as it was in Emacs 27 and older.
> 
> This would be the worst thing to do.

It sounds like no compromise I suggest is acceptable.  then I don't
know how to solve this disagreement.

> > That contradicts what Dmitry said: that the current Xref UI moves away
> > of the completion UI, and towards compilation-mode and grep-mode.
> > There, deleting the window with C-j will look odd, I think.
> 
> There are two modes: xref-show-xrefs and xref-show-defs.
> xref-show-xrefs is similar to grep, whereas xref-show-defs
> resembles the Completion UI where C-j makes sense.

AFAIU, Dmitry didn't like the proposal to have different bindings in
these two modes.

> >> > Why not 'q' (for "quit") or 'b' (for "bury")?
> >>
> >> 'xref-quit-and-goto-xref' also jumps to xref, not only quits,
> >> but 'q' and 'b' should only quit.
> >
> > That's not a catastrophe, but if you want, we could use "C-c C-c"
> > instead, as that is used in some other modes for similar effect.
> 
> I think this is a good idea for completion-like xref-show-defs,
> but not for grep-like xref-show-xrefs because in grep-mode
> 'C-c C-c' is bound to compile-goto-error that doesn't quit window.

There are other modes where it does, e.g. message-mode.

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 27.12.2020 18:22, Juri Linkov wrote:
>> On the subject of grep-mode, it could have a similar command introduced
>> too, for situations when you only needed to find one occurrence, to jump to
>> it and quit the window.
> This is what I already successfully use:
> 
> diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
> index d2293151c7..08c44dcb25 100644
> --- a/lisp/progmodes/compile.el
> +++ b/lisp/progmodes/compile.el
> @@ -2572,11 +2572,15 @@ compile-goto-error
>     (or (compilation-buffer-p (current-buffer))
>         (error "Not in a compilation buffer"))
>     (compilation--ensure-parse (point))
> +  (let ((window (posn-window (event-end event))))
>       (if (get-text-property (point) 'compilation-directory)
>           (dired-other-window
>            (car (get-text-property (point) 'compilation-directory)))
>         (setq compilation-current-error (point))
> -    (next-error-internal)))
> +      (next-error-internal))
> +    (when current-prefix-arg
> +      (with-selected-window window
> +        (quit-window)))))

So... if others like it, we could keep just this binding (C-u RET) for 
"quit the window and go to location", in both Grep and Xref.

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 25.12.2020 17:42, Eli Zaretskii wrote:
>> Cc: juri@linkov.net, joaotavora@gmail.com, 44611@debbugs.gnu.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Fri, 25 Dec 2020 16:49:39 +0200
>>
>>>> Changing the latter to use the xref UI (which will have to be renamed
>>>> and become a separate package, BTW) is also likely to encounter much
>>>> bigger resistance than anything we've done in this area before: people
>>>> don't have the same expectations for new commands as they have for
>>>> existing ones, so I'm surprised you asked this (given your overall
>>>> backward compatibility stance, much stronger than mine).
>>>
>>> An optional feature cannot hurt, even if and when it becomes the
>>> default.  Thus, there's no need for me to object to such long-term
>>> plans, if they are announced and proceed at a controlled pace
>>> (including the decision when it becomes the default).
>>
>> This endeavor might need more of an encouragement than "I don't object".
> 
> I didn't have encouragement in mind.  I'm asking if this is the plan,
> or some kind of desire.  If it is, then I can understand that.  If
> there's no such plan, then I don't think I understand why we need to
> make Xref behave similarly and have the same key bindings.

I think I explained that? "Some kind of desire", yes, there is.

In particular, there is movement in splitting the "show xrefs" action 
into two different behaviors: one for definitions (when we expect to 
almost always have very few hits, and usually just one), another for 
anything else like references, "apropos", or grep hits (see the commit 
8c38345629 I just made for some better understanding).

And the latter behavior, governed by xref-show-xrefs-function, might as 
well become even closer to grep-mode.

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 27.12.2020 20:21, Eli Zaretskii wrote:
>>> This works both ways: if you want TAB to do something other than its
>>> default, you can rebind it for you, right?
>> This is not about my personal preferences.
> Your original explanation of the rationale was that you always press
> TAB because you are used to it in Grep buffers.

Avoiding unnecessary ad-hoc differences is not just about any personal 
preference.

>>> That contradicts what Dmitry said: that the current Xref UI moves away
>>> of the completion UI, and towards compilation-mode and grep-mode.
>>> There, deleting the window with C-j will look odd, I think.
>> There are two modes: xref-show-xrefs and xref-show-defs.
>> xref-show-xrefs is similar to grep, whereas xref-show-defs
>> resembles the Completion UI where C-j makes sense.
> AFAIU, Dmitry didn't like the proposal to have different bindings in
> these two modes.

Two buffers that look exactly the same shouldn't behave differently. I 
think it's just basic usability.

What we could do instead, is change the default value of 
xref-show-xrefs-function to use some new function that would use a new 
major mode with the (slightly) new bindings which you have discussed 
with Juri.

And *also* change the default value of xref-show-definitions-function to 
one of the alternative functions, which already behave in some notably 
different way than xref--show-xref-buffer.

I'm not sure what we would buy with such a combination, however, given 
that it will create an bigger difference in behavior than simply 
changing a key binding (*). But the latter change in the default seems 
to be a Good Thing either way. Maybe we should put it to a vote.

(*) I guess you might like that the user could customize 
xref-show-definitions-function back to a value where TAB behaves like it 
did in Emacs 27? Although they could change the key binding back just as 
well with a couple of line of Emacs Lisp, one can't do that via Customize.

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

>> > This works both ways: if you want TAB to do something other than its
>> > default, you can rebind it for you, right?
>>
>> This is not about my personal preferences.
>
> Your original explanation of the rationale was that you always press
> TAB because you are used to it in Grep buffers.

Not just me, all users that expect grep keybindings in grep-like buffers.

>> > I didn't suggest unbinding TAB, I suggested to leave its current
>> > binding as it was in Emacs 27 and older.
>>
>> This would be the worst thing to do.
>
> It sounds like no compromise I suggest is acceptable.  then I don't
> know how to solve this disagreement.

There is one solution currently discussed on emacs-devel: to add
a user option that defines whether to use old keybindings or
to use new keybindings that are more consistent particularly
in xref with grep-mode.

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

>>> On the subject of grep-mode, it could have a similar command introduced
>>> too, for situations when you only needed to find one occurrence, to jump to
>>> it and quit the window.
>> This is what I already successfully use:
>> diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
>> index d2293151c7..08c44dcb25 100644
>> --- a/lisp/progmodes/compile.el
>> +++ b/lisp/progmodes/compile.el
>> @@ -2572,11 +2572,15 @@ compile-goto-error
>>     (or (compilation-buffer-p (current-buffer))
>>         (error "Not in a compilation buffer"))
>>     (compilation--ensure-parse (point))
>> +  (let ((window (posn-window (event-end event))))
>>       (if (get-text-property (point) 'compilation-directory)
>>           (dired-other-window
>>            (car (get-text-property (point) 'compilation-directory)))
>>         (setq compilation-current-error (point))
>> -    (next-error-internal)))
>> +      (next-error-internal))
>> +    (when current-prefix-arg
>> +      (with-selected-window window
>> +        (quit-window)))))
>
> So... if others like it, we could keep just this binding (C-u RET) for
> "quit the window and go to location", in both Grep and Xref.

To make it more future-proof for possible other uses of the prefix arg,
maybe better would be to use 'C-0 RET' with better mnemonics of 0 to
mean deleting the window (like 'C-x 0' deletes the window).

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 28.12.2020 19:24, Juri Linkov wrote:
>>>> On the subject of grep-mode, it could have a similar command introduced
>>>> too, for situations when you only needed to find one occurrence, to jump to
>>>> it and quit the window.
>>> This is what I already successfully use:
>>> diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
>>> index d2293151c7..08c44dcb25 100644
>>> --- a/lisp/progmodes/compile.el
>>> +++ b/lisp/progmodes/compile.el
>>> @@ -2572,11 +2572,15 @@ compile-goto-error
>>>      (or (compilation-buffer-p (current-buffer))
>>>          (error "Not in a compilation buffer"))
>>>      (compilation--ensure-parse (point))
>>> +  (let ((window (posn-window (event-end event))))
>>>        (if (get-text-property (point) 'compilation-directory)
>>>            (dired-other-window
>>>             (car (get-text-property (point) 'compilation-directory)))
>>>          (setq compilation-current-error (point))
>>> -    (next-error-internal)))
>>> +      (next-error-internal))
>>> +    (when current-prefix-arg
>>> +      (with-selected-window window
>>> +        (quit-window)))))
>> So... if others like it, we could keep just this binding (C-u RET) for
>> "quit the window and go to location", in both Grep and Xref.
> To make it more future-proof for possible other uses of the prefix arg,
> maybe better would be to use 'C-0 RET' with better mnemonics of 0 to
> mean deleting the window (like 'C-x 0' deletes the window).

Sorry, missed this message previously.

I don't have a strong opinion on this, but seems like it would conflate 
the "quit" action (what we want the command to do in this case) with the 
"delete" action (which what the binding might imply).

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

>> To make it more future-proof for possible other uses of the prefix arg,
>> maybe better would be to use 'C-0 RET' with better mnemonics of 0 to
>> mean deleting the window (like 'C-x 0' deletes the window).
>
> Sorry, missed this message previously.
>
> I don't have a strong opinion on this, but seems like it would conflate the
> "quit" action (what we want the command to do in this case) with the
> "delete" action (which what the binding might imply).

Yep, like the prefix argument KILL in quit-window.

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 19.01.2021 19:56, Juri Linkov wrote:
>>> To make it more future-proof for possible other uses of the prefix arg,
>>> maybe better would be to use 'C-0 RET' with better mnemonics of 0 to
>>> mean deleting the window (like 'C-x 0' deletes the window).
>> Sorry, missed this message previously.
>>
>> I don't have a strong opinion on this, but seems like it would conflate the
>> "quit" action (what we want the command to do in this case) with the
>> "delete" action (which what the binding might imply).
> Yep, like the prefix argument KILL in quit-window.

Right, so it appears we don't have any suitable precedent, and either 
way could imply something we don't want it to.

I think C-u RET is probably fine, especially if we add a change like you 
described to compile-goto-error as well. But if anyone wanted to propose 
some other alternative, they're welcome to.

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

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

> I think C-u RET is probably fine, especially if we add a change like you
> described to compile-goto-error as well. But if anyone wanted to propose
> some other alternative, they're welcome to.

I want to propose another alternative: to implement C-u RET
not only for compilation-mode, but also for e.g. occur-mode,
maybe current-prefix-arg could be handled in 'next-error-found'
by adding to the end:

  (when (and current-prefix-arg from-buffer (not (eq from-buffer to-buffer)))
    (let ((window (get-buffer-window from-buffer)))
      (with-selected-window window
        (quit-restore-window window))))

But this means that it should take into account possible values
of the prefix arg of the command 'next-error':

  A prefix ARG specifies how many error messages to move;
  negative means move back to previous error messages.
  Just C-u as a prefix means reparse the error message buffer
  and start at the first error.

So since 'C-u M-g n' is already taken, an alternative is 'C-0 M-g n'.
If 'C-0' makes no sense for 'C-0 M-g n', it could be used to close the window.
Or maybe 'C-0 M-g n' still makes sense, e.g. to redisplay the current location?

BTW, here are the commands that work like TAB in grep-mode.
I propose to install this patch, so at least users could bind them
in own init files:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xref-no-display.patch --]
[-- Type: text/x-diff, Size: 1074 bytes --]

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 18fdd963fb..750a090b84 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -607,16 +607,26 @@ xref-show-location-at-point
     (when xref
       (xref--show-location (xref-item-location xref)))))
 
+(defun xref-next-line-no-display ()
+  "Move to the next xref but don't display its source."
+  (interactive)
+  (xref--search-property 'xref-item))
+
 (defun xref-next-line ()
   "Move to the next xref and display its source in the appropriate window."
   (interactive)
-  (xref--search-property 'xref-item)
+  (xref-next-line-no-display)
   (xref-show-location-at-point))
 
+(defun xref-prev-line-no-display ()
+  "Move to the previous xref but don't display its source."
+  (interactive)
+  (xref--search-property 'xref-item t))
+
 (defun xref-prev-line ()
   "Move to the previous xref and display its source in the appropriate window."
   (interactive)
-  (xref--search-property 'xref-item t)
+  (xref-prev-line-no-display)
   (xref-show-location-at-point))
 
 (defun xref-next-group ()

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

> I want to propose another alternative: to implement C-u RET
> not only for compilation-mode, but also for e.g. occur-mode,
> maybe current-prefix-arg could be handled in 'next-error-found'
> by adding to the end:
>
>   (when (and current-prefix-arg from-buffer (not (eq from-buffer to-buffer)))
>     (let ((window (get-buffer-window from-buffer)))
>       (with-selected-window window
>         (quit-restore-window window))))

Actually, 'window' should be checked with window-live-p,
because 'xref-goto-xref' could already quit the window.

> So since 'C-u M-g n' is already taken, an alternative is 'C-0 M-g n'.
> If 'C-0' makes no sense for 'C-0 M-g n', it could be used to close the window.
> Or maybe 'C-0 M-g n' still makes sense, e.g. to redisplay the current location?

I withdraw this proposal because it's easy to do this in the init file:

  (defun next-error-quit-window ()
    (when (and (eq current-prefix-arg 0)
               (not (eq next-error-last-buffer (current-buffer))))
      (let ((window (get-buffer-window next-error-last-buffer)))
        (when (window-live-p window)
          (with-selected-window window
            (quit-restore-window window))))))

  (add-hook 'next-error-hook 'next-error-quit-window)

> BTW, here are the commands that work like TAB in grep-mode.
> I propose to install this patch, so at least users could bind them
> in own init files:

Now pushed to master.

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

Hi Juri,

Sorry for the late response. I was hoping to have made some progress on 
resolving other issues touched in this thread before answering, but none 
so far.

On 16.03.2021 20:07, Juri Linkov wrote:

>> So since 'C-u M-g n' is already taken, an alternative is 'C-0 M-g n'.
>> If 'C-0' makes no sense for 'C-0 M-g n', it could be used to close the window.
>> Or maybe 'C-0 M-g n' still makes sense, e.g. to redisplay the current location?
> 
> I withdraw this proposal because it's easy to do this in the init file:
> 
>    (defun next-error-quit-window ()
>      (when (and (eq current-prefix-arg 0)
>                 (not (eq next-error-last-buffer (current-buffer))))
>        (let ((window (get-buffer-window next-error-last-buffer)))
>          (when (window-live-p window)
>            (with-selected-window window
>              (quit-restore-window window))))))
> 
>    (add-hook 'next-error-hook 'next-error-quit-window)

Since next-error-hook is a defcustom, perhaps we can/should include this 
function as one of the advertised values in :type?

Even if we keep the list nil by default.

>> BTW, here are the commands that work like TAB in grep-mode.
>> I propose to install this patch, so at least users could bind them
>> in own init files:
> 
> Now pushed to master.

Looks good, of course.

bug#44611: Prefix arg for xref-goto-xref

[martin rudalics]

 >            (with-selected-window window
 >              (quit-restore-window window))))))

Please remove the `with-selected-window'.  `quit-restore-window' has to
work regardless of which window is selected.

martin

bug#44611: Prefix arg for xref-goto-xref

[Juri Linkov]

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

>>    (defun next-error-quit-window ()
>>      (when (and (eq current-prefix-arg 0)
>>                 (not (eq next-error-last-buffer (current-buffer))))
>>        (let ((window (get-buffer-window next-error-last-buffer)))
>>          (when (window-live-p window)
>>            (with-selected-window window
>>              (quit-restore-window window))))))
>>    (add-hook 'next-error-hook 'next-error-quit-window)
>
> Since next-error-hook is a defcustom, perhaps we can/should include this
> function as one of the advertised values in :type?
>
> Even if we keep the list nil by default.

I completely forgot that we have 'next-error-found-function',
so implemented it with 'next-error-hook'.  I don't remember
why we have both.

Anyway, here is a patch with corrections from Martin:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: next-error-quit-window.patch --]
[-- Type: text/x-diff, Size: 1185 bytes --]

diff --git a/lisp/simple.el b/lisp/simple.el
index 94d79c2bf1..73020c2c8e 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -377,11 +377,22 @@ next-error-internal
                 (not (eq prev next-error-last-buffer)))
         (message "Current locus from %s" next-error-last-buffer)))))
 
+(defun next-error-quit-window (from-buffer to-buffer)
+  "Quit window of FROM-BUFFER when the prefix arg is 0.
+Intended to be used in `next-error-found-function'."
+  (when (and (eq current-prefix-arg 0) from-buffer
+             (not (eq from-buffer to-buffer)))
+    (let ((window (get-buffer-window from-buffer)))
+      (when (window-live-p window)
+        (quit-restore-window window)))))
+
 (defcustom next-error-found-function #'ignore
   "Function called when a next locus is found and displayed.
 Function is called with two arguments: a FROM-BUFFER buffer
 from which next-error navigated, and a target buffer TO-BUFFER."
   :type '(choice (const :tag "No default" ignore)
+                 (const :tag "Quit previous window with M-0"
+                        next-error-quit-window)
                  (function :tag "Other function"))
   :group 'next-error
   :version "27.1")

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 17.03.2021 19:23, Juri Linkov wrote:
>>>     (defun next-error-quit-window ()
>>>       (when (and (eq current-prefix-arg 0)
>>>                  (not (eq next-error-last-buffer (current-buffer))))
>>>         (let ((window (get-buffer-window next-error-last-buffer)))
>>>           (when (window-live-p window)
>>>             (with-selected-window window
>>>               (quit-restore-window window))))))
>>>     (add-hook 'next-error-hook 'next-error-quit-window)
>>
>> Since next-error-hook is a defcustom, perhaps we can/should include this
>> function as one of the advertised values in :type?
>>
>> Even if we keep the list nil by default.
> 
> I completely forgot that we have 'next-error-found-function',
> so implemented it with 'next-error-hook'.  I don't remember
> why we have both.

next-error-found-function was added recently, and I guess it's handy to 
have FROM-BUFFER and TO-BUFFER available.

I can't find any specific previous uses for it, though.

> Anyway, here is a patch with corrections from Martin:

LGTM.

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

Hi Eli,

On 27.12.2020 20:21, Eli Zaretskii wrote:
>> From: Juri Linkov<juri@linkov.net>
>> Cc:dgutov@yandex.ru,joaotavora@gmail.com,44611@debbugs.gnu.org
>> Date: Sun, 27 Dec 2020 18:20:01 +0200
>>
>> The notion "conservative" is irrelevant here, because TAB was bound in xref
>> recently
> Not so recently: 2 years ago.  And by the time Emacs 28 hits the
> network, it will probably be 4 years.

How about this:

Up until now, Xref still had a "still experimental and can change in 
major ... ways" note in its commentary.

I'm going to remove this note (it's indeed getting ridiculous after 5 
years of existence), but how about we remove the contentious binding 
with it?

And then we can treat new bindings more conservatively, with higher 
expectations of stability.

(Resending because the bug tracker rejected it the first time.)

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> Cc: 44611@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 22 Apr 2021 17:49:53 +0300
> 
> Up until now, Xref still had a "still experimental and can change in 
> major ... ways" note in its commentary.
> 
> I'm going to remove this note (it's indeed getting ridiculous after 5 
> years of existence), but how about we remove the contentious binding 
> with it?
> 
> And then we can treat new bindings more conservatively, with higher 
> expectations of stability.

Sorry, I don't see how that comment changes anything to affect our
disagreements.

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 22.04.2021 20:03, Eli Zaretskii wrote:
> Sorry, I don't see how that comment changes anything to affect our
> disagreements.

The lack of compatibility promise means we don't promise compatibility, 
let alone stability of key bindings.

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> Cc: juri@linkov.net, 44611@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 22 Apr 2021 20:44:30 +0300
> 
> On 22.04.2021 20:03, Eli Zaretskii wrote:
> > Sorry, I don't see how that comment changes anything to affect our
> > disagreements.
> 
> The lack of compatibility promise means we don't promise compatibility, 
> let alone stability of key bindings.

We always promise compatibility in Emacs, after enough time has
passed.  At least on the user level and in interfaces that are not
internal ones.

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 22.04.2021 20:47, Eli Zaretskii wrote:
> We always promise compatibility in Emacs, after enough time has
> passed.  At least on the user level and in interfaces that are not
> internal ones.

When we talked with John Wiegley about project.el and xref.el's 
inclusion in the upcoming release, we agreed to put these notices 
because the packages were not ready enough.

Which was supposed to allow us to make drastic changes in subsequent 
releases.

How do you feel about keeping commitments made by the previous maintainers?

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 24.12.2020 19:44, Eli Zaretskii wrote:
> Given the single precedent I found, I'm fine with declaring the
> current binding of TAB obsolete and providing a replacement for it
> ('b'? 'q'?), so that we could replace it in some future version after
> 28, if that is okay with you and Juri.

Please go ahead with this change (you surely know how to phrase this 
"binding obsoletion" better in the documentation).

I have provided all the necessary info in the latest commit.

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> Cc: juri@linkov.net, joaotavora@gmail.com, 44611@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 23 Apr 2021 13:41:36 +0300
> 
> On 24.12.2020 19:44, Eli Zaretskii wrote:
> > Given the single precedent I found, I'm fine with declaring the
> > current binding of TAB obsolete and providing a replacement for it
> > ('b'? 'q'?), so that we could replace it in some future version after
> > 28, if that is okay with you and Juri.
> 
> Please go ahead with this change (you surely know how to phrase this 
> "binding obsoletion" better in the documentation).

Thanks, will do.

bug#44611: Prefix arg for xref-goto-xref

[Eli Zaretskii]

> Date: Fri, 23 Apr 2021 13:53:56 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: juri@linkov.net, joaotavora@gmail.com, 44611@debbugs.gnu.org
> 
> > On 24.12.2020 19:44, Eli Zaretskii wrote:
> > > Given the single precedent I found, I'm fine with declaring the
> > > current binding of TAB obsolete and providing a replacement for it
> > > ('b'? 'q'?), so that we could replace it in some future version after
> > > 28, if that is okay with you and Juri.
> > 
> > Please go ahead with this change (you surely know how to phrase this 
> > "binding obsoletion" better in the documentation).
> 
> Thanks, will do.

Now done.

Is there anything else left to do in this bug, or can it be closed?

bug#44611: Prefix arg for xref-goto-xref

[Dmitry Gutov]

On 24.04.2021 12:56, Eli Zaretskii wrote:
> Now done.

Thank you.

> Is there anything else left to do in this bug, or can it be closed?

It has been closed for a while, for some reason.