bug#35737: xref--original-command

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#35737: xref--original-command

Juri Linkov-2
Remembering the command that created the *xref* buffer
will allow such customization to treat RET differently,
i.e. it makes sense for RET to quit the transient xref buffer
displayed momentarily while navigating code with xref-find-definitions,
whereas leaving the xref buffer visible while navigating matches
in the xref buffer created by e.g. project-find-regexp:

(define-key xref--button-map [(control ?m)]
  (lambda ()
    (interactive)
    (if (memq xref--original-command '(xref-find-definitions))
        (call-interactively 'xref-quit-and-goto-xref)
      (call-interactively 'xref-goto-xref))))

Better ideas?

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index bf999aeb0d..5c38cac164 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -477,6 +477,9 @@ xref--original-window-intent
 (defvar-local xref--original-window nil
   "The original window this xref buffer was created from.")
 
+(defvar-local xref--original-command nil
+  "The original command that created this xref buffer.")
+
 (defun xref--show-pos-in-buf (pos buf)
   "Goto and display position POS of buffer BUF in a window.
 Honor `xref--original-window-intent', run `xref-after-jump-hook'
@@ -777,7 +788,8 @@ xref--analyze
         (pop-to-buffer (current-buffer))
         (goto-char (point-min))
         (setq xref--original-window (assoc-default 'window alist)
-              xref--original-window-intent (assoc-default 'display-action alist))
+              xref--original-window-intent (assoc-default 'display-action alist)
+              xref--original-command this-command)
         (current-buffer)))))
 



Reply | Threaded
Open this post in threaded view
|

bug#35737: xref--original-command

Dmitry Gutov
On 14.05.2019 23:53, Juri Linkov wrote:

> Remembering the command that created the *xref* buffer
> will allow such customization to treat RET differently,
> i.e. it makes sense for RET to quit the transient xref buffer
> displayed momentarily while navigating code with xref-find-definitions,
> whereas leaving the xref buffer visible while navigating matches
> in the xref buffer created by e.g. project-find-regexp:
>
> (define-key xref--button-map [(control ?m)]
>    (lambda ()
>      (interactive)
>      (if (memq xref--original-command '(xref-find-definitions))
>          (call-interactively 'xref-quit-and-goto-xref)
>        (call-interactively 'xref-goto-xref))))

I'm all for customizability, but as I said in a past discussion on the
subject, I don't think this by itself would be a good default behavior.

Having two buffers that looks basically the same but react to RET
differently is not great for usability.

> Better ideas?

Ideally, the "transient" buffer should look differently from the
"persistent" one.

To take VS Code as an example (I just to see how it behaves exactly), a
search for references opens search results in the sidebar.

Whereas for definitions they have both "Go To Definition" and "Peek
Definition". The latter shows the definitions kind of inline, in a
not-exactly-a-popup kind of way
(https://stackoverflow.com/questions/55599177/go-to-definition-in-vscode-preview-window-ruins-the-edito),
and there, if you type RET, that triggers navigation to the selected
definition. "Go To Definition", when there are several definitions, also
switches to "Peek" mode by default.

Not sure how best to implement this different kind of display in Emacs,
but the idea makes a lot of sense to me.

In Sublime, IIRC, for this they used the dropdown from their
top-of-the-window command line, the same one that becomes active once
one presses Ctrl-P.

> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
> index bf999aeb0d..5c38cac164 100644
> --- a/lisp/progmodes/xref.el
> +++ b/lisp/progmodes/xref.el
> @@ -477,6 +477,9 @@ xref--original-window-intent
>   (defvar-local xref--original-window nil
>     "The original window this xref buffer was created from.")
>  
> +(defvar-local xref--original-command nil
> +  "The original command that created this xref buffer.")
> +
>   (defun xref--show-pos-in-buf (pos buf)
>     "Goto and display position POS of buffer BUF in a window.
>   Honor `xref--original-window-intent', run `xref-after-jump-hook'
> @@ -777,7 +788,8 @@ xref--analyze
>           (pop-to-buffer (current-buffer))
>           (goto-char (point-min))
>           (setq xref--original-window (assoc-default 'window alist)
> -              xref--original-window-intent (assoc-default 'display-action alist))
> +              xref--original-window-intent (assoc-default 'display-action alist)
> +              xref--original-command this-command)
>           (current-buffer)))))

I'm good with this patch, though, if you find it helpful for personal
customization.

When we implement two different display strategies, though, we might
choose between them inside xref--show-xrefs straight away, instead of
saving the original command for later.



Reply | Threaded
Open this post in threaded view
|

bug#35737: xref--original-command

Juri Linkov-2
>> Remembering the command that created the *xref* buffer
>> will allow such customization to treat RET differently,
>> i.e. it makes sense for RET to quit the transient xref buffer
>> displayed momentarily while navigating code with xref-find-definitions,
>> whereas leaving the xref buffer visible while navigating matches
>> in the xref buffer created by e.g. project-find-regexp:
>>
>> (define-key xref--button-map [(control ?m)]
>>    (lambda ()
>>      (interactive)
>>      (if (memq xref--original-command '(xref-find-definitions))
>>          (call-interactively 'xref-quit-and-goto-xref)
>>        (call-interactively 'xref-goto-xref))))
>
> I'm all for customizability, but as I said in a past discussion on the
> subject, I don't think this by itself would be a good default behavior.

I don't propose to change its default behavior.  Just allowing its
customization would be good.

> Having two buffers that looks basically the same but react to RET
> differently is not great for usability.

Using display-buffer-in-direction, they don't look the same: after
customization xref-find-definitions can display the xref buffer below,
whereas other xref command can display in the side window.

>> Better ideas?
>
> Ideally, the "transient" buffer should look differently from the
> "persistent" one.
>
> To take VS Code as an example (I just to see how it behaves exactly),
> a search for references opens search results in the sidebar.

Emacs has side windows too.

> Whereas for definitions they have both "Go To Definition" and "Peek
> Definition". The latter shows the definitions kind of inline, in
> a not-exactly-a-popup kind of way
> (https://stackoverflow.com/questions/55599177/go-to-definition-in-vscode-preview-window-ruins-the-edito),
> and there, if you type RET, that triggers navigation to the selected
> definition. "Go To Definition", when there are several definitions, also
> switches to "Peek" mode by default.
>
> Not sure how best to implement this different kind of display in Emacs, but
> the idea makes a lot of sense to me.
>
> In Sublime, IIRC, for this they used the dropdown from their
> top-of-the-window command line, the same one that becomes active once one
> presses Ctrl-P.

We have Completions for the same purpose.



Reply | Threaded
Open this post in threaded view
|

bug#35737: xref--original-command

Dmitry Gutov
On 16.05.2019 0:04, Juri Linkov wrote:

> I don't propose to change its default behavior.  Just allowing its
> customization would be good.

OK.

But please wait a little, I'd like to show a patch for bug#35702 that
can also provide a basic for the same distinction in functionality but
without this variable.

>> Having two buffers that looks basically the same but react to RET
>> differently is not great for usability.
>
> Using display-buffer-in-direction, they don't look the same: after
> customization xref-find-definitions can display the xref buffer below,
> whereas other xref command can display in the side window.

OK. Still look very similar, but at least the behavior appears different
from the outset.

If you look at Atom's behavior, it actually shows regexp search results
in a pane below the main area. The same direction where you want to show
the definitions search result. Just something to keep in mind.

>> To take VS Code as an example (I just to see how it behaves exactly),
>> a search for references opens search results in the sidebar.
>
> Emacs has side windows too.

You mean like Speedbar? That's the part which I didn't exactly like.

>> Whereas for definitions they have both "Go To Definition" and "Peek
>> Definition". The latter shows the definitions kind of inline, in
>> a not-exactly-a-popup kind of way
>> (https://stackoverflow.com/questions/55599177/go-to-definition-in-vscode-preview-window-ruins-the-edito),
>> and there, if you type RET, that triggers navigation to the selected
>> definition. "Go To Definition", when there are several definitions, also
>> switches to "Peek" mode by default.
>>
>> Not sure how best to implement this different kind of display in Emacs, but
>> the idea makes a lot of sense to me.

BTW, there's a third-party package that implements something like that
for LSP users: https://github.com/emacs-lsp/lsp-ui#lsp-ui-peek

Not sure how likely we are to have this contributed to the core, though.

>> In Sublime, IIRC, for this they used the dropdown from their
>> top-of-the-window command line, the same one that becomes active once one
>> presses Ctrl-P.
>
> We have Completions for the same purpose.

Except we have a UI for it which is expected to be usable without using
arrow keys. And the completion entries in this case can contain spaces,
parens, and basically any other characters. They can also be fairly
long. So completing-read doesn't seem to fit the bill.

I'm open to ideas.



Reply | Threaded
Open this post in threaded view
|

bug#35737: xref--original-command

Juri Linkov-2
> But please wait a little, I'd like to show a patch for bug#35702 that can
> also provide a basic for the same distinction in functionality but without
> this variable.

OK.

>> Emacs has side windows too.
>
> You mean like Speedbar? That's the part which I didn't exactly like.

I don't like Speedbar too because it's too intrusive.  But maybe
it can be rewritten using real side windows as documented in
(info "(elisp) Side Windows")