GNU bug report logs - #33992
27.0.50; xref-find-definitions wastes too much space

Previous Next

Package: emacs;

Reported by: Juri Linkov <juri <at> linkov.net>

Date: Sat, 5 Jan 2019 23:51:01 UTC

Severity: wishlist

Tags: patch

Found in version 27.0.50

Done: Juri Linkov <juri <at> linkov.net>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 33992 in the body.
You can then email your comments to 33992 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to joaotavora <at> gmail.com, dgutov <at> yandex.ru, bug-gnu-emacs <at> gnu.org:
bug#33992; Package emacs. (Sat, 05 Jan 2019 23:51:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Juri Linkov <juri <at> linkov.net>:
New bug report received and forwarded. Copy sent to joaotavora <at> gmail.com, dgutov <at> yandex.ru, bug-gnu-emacs <at> gnu.org. (Sat, 05 Jan 2019 23:51:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.50; xref-find-definitions wastes too much space
Date: Sun, 06 Jan 2019 01:43:35 +0200
[Message part 1 (text/plain, inline)]
Tags: patch
Severity: wishlist
X-Debbugs-CC: João Távora <joaotavora <at> gmail.com>, Dmitry Gutov <dgutov <at> yandex.ru>

As noted in bug#33870 the buffer produced by xref-find-definitions
(bound to ‘M-.’) has a transient nature like the *Completions* buffer.
Therefore it makes sense not to waste its space needlessly:

[display-action-alist.patch (text/x-diff, inline)]
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 87ce2299c5..169f49a348 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -782,7 +773,7 @@ xref--show-xref-buffer
         (erase-buffer)
         (xref--insert-xrefs xref-alist)
         (xref--xref-buffer-mode)
-        (pop-to-buffer (current-buffer))
+        (pop-to-buffer (current-buffer) (assoc-default 'display-action-alist alist))
         (goto-char (point-min))
         (setq xref--original-window (assoc-default 'window alist)
               xref--original-window-intent (assoc-default 'display-action alist))
@@ -808,12 +799,15 @@ xref--show-xrefs
   (cond
    ((and (not (cdr xrefs)) (not always-show-list))
     (xref-push-marker-stack)
-    (xref--pop-to-location (car xrefs) display-action))
+    (xref--pop-to-location (car xrefs) (unless (listp display-action) display-action)))
    (t
     (xref-push-marker-stack)
     (funcall xref-show-xrefs-function xrefs
              `((window . ,(selected-window))
-               (display-action . ,display-action))))))
+               (display-action
+                . ,(unless (listp display-action) display-action))
+               (display-action-alist
+                . ,(when (listp display-action) display-action)))))))
 
 (defun xref--prompt-p (command)
   (or (eq xref-prompt-for-identifier t)
@@ -864,7 +858,7 @@ xref-find-definitions
 Otherwise, display the list of the possible definitions in a
 buffer where the user can select from the list."
   (interactive (list (xref--read-identifier "Find definitions of: ")))
-  (xref--find-definitions identifier nil))
+  (xref--find-definitions identifier '((display-buffer-maybe-below-selected))))
 
 ;;;###autoload
 (defun xref-find-definitions-other-window (identifier)

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#33992; Package emacs. (Sun, 06 Jan 2019 11:04:01 GMT) Full text and rfc822 format available.

Message #8 received at 33992 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 33992 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#33992: 27.0.50; xref-find-definitions wastes too much space
Date: Sun, 6 Jan 2019 11:03:18 +0000
[Message part 1 (text/plain, inline)]
On Sat, Jan 5, 2019, 23:51 Juri Linkov <juri <at> linkov.net wrote:

> Tags: patch
> Severity: wishlist
> X-Debbugs-CC: João Távora <joaotavora <at> gmail.com>, Dmitry Gutov <
> dgutov <at> yandex.ru>
>
> As noted in bug#33870 the buffer produced by xref-find-definitions
> (bound to ‘M-.’) has a transient nature like the *Completions* buffer.
> Therefore it makes sense not to waste its space needlessly:
>

It doesn't make sense to review this patch until the patch that fixes 33870
has been produced.

João

>
>
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#33992; Package emacs. (Wed, 20 Mar 2019 22:36:01 GMT) Full text and rfc822 format available.

Message #11 received at 33992 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: João Távora <joaotavora <at> gmail.com>
Cc: 33992 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#33992: 27.0.50; xref-find-definitions wastes too much space
Date: Wed, 20 Mar 2019 23:37:54 +0200
>> As noted in bug#33870 the buffer produced by xref-find-definitions
>> (bound to ‘M-.’) has a transient nature like the *Completions* buffer.
>> Therefore it makes sense not to waste its space needlessly:
>
> It doesn't make sense to review this patch until the patch that fixes 33870
> has been produced.

Since the patch from bug#33870 has been already committed,
this bug#33992 is undeadlocked now.

The problem with xref-find-definitions is its unexpected outcome:
sometimes it pops up a window, sometimes doesn't.

It's like Russian roulette: pull the trigger, bang and there is
a hole in the screen that takes the form of glaringly empty space.

The proposed change is not to break the window configuration:
either split the selected window and show *xref* below,
or do inline placement, i.e. to replace the content of the
selected window with the *xref* buffer.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#33992; Package emacs. (Wed, 20 Mar 2019 23:25:01 GMT) Full text and rfc822 format available.

Message #14 received at 33992 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 33992 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#33992: 27.0.50; xref-find-definitions wastes too much space
Date: Wed, 20 Mar 2019 23:23:43 +0000
[Message part 1 (text/plain, inline)]
On Wed, Mar 20, 2019 at 10:35 PM Juri Linkov <juri <at> linkov.net> wrote:

> >> As noted in bug#33870 the buffer produced by xref-find-definitions
> >> (bound to ‘M-.’) has a transient nature like the *Completions* buffer.
> >> Therefore it makes sense not to waste its space needlessly:
> >
> > It doesn't make sense to review this patch until the patch that fixes
> 33870
> > has been produced.
>
> Since the patch from bug#33870 has been already committed,
> this bug#33992 is undeadlocked now.
>
> The problem with xref-find-definitions is its unexpected outcome:
> sometimes it pops up a window, sometimes doesn't.
>
> It's like Russian roulette: pull the trigger, bang and there is
> a hole in the screen that takes the form of glaringly empty space.
>

I understand, but somehow I'm not very offended by that.  It's just
something I've grown so used to, even before xref.  Just C-h f to
a function description of which you know nothing of the length
and boom, a lot of empty space too.

While I do vaguely remember being annoyed by it many years ago,
I don't now. And for xref I like to see it well highlighted away from any
other text, so I can easily count the references.  Displaying next to the
minibuffer are would surround it with more noise.

It's just the way I work.  Fortunately now it is customizable, right? How
about
adding an entry to the manual, in the xref section, mentioning a couple of
commands or customization variables that can be used to get the interface
that you prefer, Juri?

João
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#33992; Package emacs. (Thu, 04 Apr 2019 00:03:01 GMT) Full text and rfc822 format available.

Message #17 received at 33992 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>, João Távora
 <joaotavora <at> gmail.com>
Cc: 33992 <at> debbugs.gnu.org
Subject: Re: bug#33992: 27.0.50; xref-find-definitions wastes too much space
Date: Thu, 4 Apr 2019 03:02:33 +0300
On 20.03.2019 23:37, Juri Linkov wrote:
> The problem with xref-find-definitions is its unexpected outcome:
> sometimes it pops up a window, sometimes doesn't.

Imagine the process of code completion.

Sometimes you press TAB (or C-M-i) and it completes to the symbol you 
wanted. Sometimes it pops up a window with all matching completions instead.

Does it feel the same way to you?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#33992; Package emacs. (Thu, 04 Apr 2019 21:07:02 GMT) Full text and rfc822 format available.

Message #20 received at 33992 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 33992 <at> debbugs.gnu.org,
 João Távora <joaotavora <at> gmail.com>
Subject: Re: bug#33992: 27.0.50; xref-find-definitions wastes too much space
Date: Thu, 04 Apr 2019 23:49:27 +0300
>> The problem with xref-find-definitions is its unexpected outcome:
>> sometimes it pops up a window, sometimes doesn't.
>
> Imagine the process of code completion.
>
> Sometimes you press TAB (or C-M-i) and it completes to the symbol you
> wanted. Sometimes it pops up a window with all matching
> completions instead.
>
> Does it feel the same way to you?

The difference is that completions pop up in a small unobtrusive window.
But this should be easy to do in xref now too.

Thanks to João, we now have configurable window management in xref,
so I tried different customizations, and one of the most appealing
is this:

  (defun display-buffer-condition-xref (buffer-name _action)
    (and (string-match-p "\\`\\*\\(xref\\)\\*\\(\\|<[0-9]+>\\)\\'"
                         buffer-name)
         (memq this-command '(xref-find-definitions))))

  (defun display-buffer-condition-from-xref (_buffer-name _action)
    (string-match-p "\\`\\*\\(xref\\)\\*\\(\\|<[0-9]+>\\)\\'"
                    (buffer-name (current-buffer))))

  (setq display-buffer-alist
     '((display-buffer-condition-xref
        display-buffer-in-direction
       (direction . below) (window-height . fit-window-to-buffer))))

  (with-eval-after-load 'xref
    (define-key xref--button-map [(control ?m)] #'xref-quit-and-goto-xref))

How do you like that?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#33992; Package emacs. (Thu, 04 Apr 2019 23:08:02 GMT) Full text and rfc822 format available.

Message #23 received at 33992 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: 33992 <at> debbugs.gnu.org,
 João Távora <joaotavora <at> gmail.com>
Subject: Re: bug#33992: 27.0.50; xref-find-definitions wastes too much space
Date: Fri, 5 Apr 2019 02:06:57 +0300
On 04.04.2019 23:49, Juri Linkov wrote:

>> Does it feel the same way to you?
> 
> The difference is that completions pop up in a small unobtrusive window.

Small window? I usually have a side-by-side fullscreen split, and if I 
initiate completion in one of the windows, *Completion* takes up the 
whole other window. Temporarily, of course.

> But this should be easy to do in xref now too.
> 
> Thanks to João, we now have configurable window management in xref,
> so I tried different customizations, and one of the most appealing
> is this:
> 
>    (defun display-buffer-condition-xref (buffer-name _action)
>      (and (string-match-p "\\`\\*\\(xref\\)\\*\\(\\|<[0-9]+>\\)\\'"
>                           buffer-name)
>           (memq this-command '(xref-find-definitions))))
> 
>    (defun display-buffer-condition-from-xref (_buffer-name _action)
>      (string-match-p "\\`\\*\\(xref\\)\\*\\(\\|<[0-9]+>\\)\\'"
>                      (buffer-name (current-buffer))))

This function seems unused.

>    (setq display-buffer-alist
>       '((display-buffer-condition-xref
>          display-buffer-in-direction

And this function is undefined in my Emacs.

>         (direction . below) (window-height . fit-window-to-buffer))))
> 
>    (with-eval-after-load 'xref
>      (define-key xref--button-map [(control ?m)] #'xref-quit-and-goto-xref))
> 
> How do you like that?

I might, but since I can't really try your customization myself yet, 
I'll repeat a question you might be familiar with already:

  Will this also affect xref-find-references and project-find-regexp?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#33992; Package emacs. (Fri, 05 Apr 2019 09:45:02 GMT) Full text and rfc822 format available.

Message #26 received at 33992 <at> debbugs.gnu.org (full text, mbox):

From: Felician Nemeth <felician.nemeth <at> gmail.com>
To: 33992 <at> debbugs.gnu.org
Cc: Juri Linkov <juri <at> linkov.net>,
 João Távora <joaotavora <at> gmail.com>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#33992: 27.0.50; xref-find-definitions wastes too much space
Date: Fri, 05 Apr 2019 11:44:16 +0200
(Sorry for replying late, I've just read this bug report.)

I thought that I didn't need to see the list of the xref results and the
xrefs' window shrank the view of the code I wanted to study.  So, I came
up with the defun below.  It presents the xref results without showing
the xref window.  I think this idea can be further developed.
xref-show--xrefs-buffer could have an 'm' key binding that "minimizes"
its window by switching to xref-show-xrefs-without-buffer (below) and
that function can "maximize" back with the same 'm' key.  A customizable
variable could define the initial behavior.

Also, I think we can enhance xref-pulse-momentarily to use a different
face if there's only one xref to present.

(defun xref-show-xrefs-without-buffer (xrefs alist)
  "Present the results of an xref query in a simple manner.
To activate this feature, customize `xref-show-xrefs-function'."
  (xref--show-xref-buffer xrefs alist)
  (quit-window)
  (next-error)
  (message "%s (%s xrefs in total)"
           "\",\": previous xref \".\":next xref \"m\":show xref buffer"
           (length xrefs))
  (set-transient-map
   (let ((map (make-sparse-keymap)))
     (define-key map (kbd ",") 'previous-error)
     (define-key map (kbd ".") 'next-error)
     (define-key map (kbd "m")
       (lambda () (interactive) (pop-to-buffer xref-buffer-name)))
     map)
   t))
(setq xref-show-xrefs-function 'xref-show-xrefs-without-buffer)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#33992; Package emacs. (Fri, 05 Apr 2019 23:21:01 GMT) Full text and rfc822 format available.

Message #29 received at 33992 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Felician Nemeth <felician.nemeth <at> gmail.com>, 33992 <at> debbugs.gnu.org
Cc: João Távora <joaotavora <at> gmail.com>,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#33992: 27.0.50; xref-find-definitions wastes too much space
Date: Sat, 6 Apr 2019 02:20:02 +0300
On 05.04.2019 12:44, Felician Nemeth wrote:
> (Sorry for replying late, I've just read this bug report.)
> 
> I thought that I didn't need to see the list of the xref results and the
> xrefs' window shrank the view of the code I wanted to study.  So, I came
> up with the defun below.  It presents the xref results without showing
> the xref window.  I think this idea can be further developed.

For instance, by not calling xref--show-xref-buffer right away, and 
instead storing the arguments. And then maybe calling it later if the 
user types 'm'.

You won't be able to use previous/next-error for such an implementation, 
though.

> xref-show--xrefs-buffer could have an 'm' key binding that "minimizes"
> its window by switching to xref-show-xrefs-without-buffer (below) and
> that function can "maximize" back with the same 'm' key.  A customizable
> variable could define the initial behavior.

I'm not a fan of this interface, personally. It's good that it's 
available, though.

We will make xref-show-xrefs-function a defcustom sooner or later, when 
we're sure that it doesn't need to be changed much.

> Also, I think we can enhance xref-pulse-momentarily to use a different
> face if there's only one xref to present.

Yes, but I'm not sure the user will quickly understand the meaning of 
the different face.

Anyway, we can make the exact face name a defcustom (it's currently 
'next-error'), and you'd be able to change it via 'let'.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#33992; Package emacs. (Sat, 06 Apr 2019 21:48:02 GMT) Full text and rfc822 format available.

Message #32 received at 33992 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 33992 <at> debbugs.gnu.org,
 João Távora <joaotavora <at> gmail.com>
Subject: Re: bug#33992: 27.0.50; xref-find-definitions wastes too much space
Date: Sun, 07 Apr 2019 00:03:09 +0300
>>> Does it feel the same way to you?
>>
>> The difference is that completions pop up in a small unobtrusive window.
>
> Small window? I usually have a side-by-side fullscreen split, and if
> I initiate completion in one of the windows, *Completion* takes up the
> whole other window. Temporarily, of course.

The key word here is 'Temporarily'.  Unlike *Completions*,
the *xref* buffer doesn't go out easily.

>>    (defun display-buffer-condition-xref (buffer-name _action)
>>      (and (string-match-p "\\`\\*\\(xref\\)\\*\\(\\|<[0-9]+>\\)\\'"
>>                           buffer-name)
>>           (memq this-command '(xref-find-definitions))))
>>
>>    (defun display-buffer-condition-from-xref (_buffer-name _action)
>>      (string-match-p "\\`\\*\\(xref\\)\\*\\(\\|<[0-9]+>\\)\\'"
>>                      (buffer-name (current-buffer))))
>
> This function seems unused.

It's unused because it would be useful only in the *xref* buffer
created by the xref-find-definitions command, so xref needs to
provide a way to distinguish such case.

>>    (setq display-buffer-alist
>>       '((display-buffer-condition-xref
>>          display-buffer-in-direction
>
> And this function is undefined in my Emacs.

This function is implemented by Martin in bug#33870.

>>    (with-eval-after-load 'xref
>>      (define-key xref--button-map [(control ?m)] #'xref-quit-and-goto-xref))
>>
>> How do you like that?
>
> I might, but since I can't really try your customization myself yet, I'll
> repeat a question you might be familiar with already:
>
>   Will this also affect xref-find-references and project-find-regexp?

It should not affect them due to (memq this-command '(xref-find-definitions))
above.  But also to not affect commands active in the *xref* buffer,
xref should provide a way to check if the *xref* buffer was created
by xref-find-definitions.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#33992; Package emacs. (Sat, 06 Apr 2019 21:48:03 GMT) Full text and rfc822 format available.

Message #35 received at 33992 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Felician Nemeth <felician.nemeth <at> gmail.com>
Cc: 33992 <at> debbugs.gnu.org,
 João Távora <joaotavora <at> gmail.com>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#33992: 27.0.50; xref-find-definitions wastes too much space
Date: Sun, 07 Apr 2019 00:08:38 +0300
> I thought that I didn't need to see the list of the xref results and the
> xrefs' window shrank the view of the code I wanted to study.  So, I came
> up with the defun below.  It presents the xref results without showing
> the xref window.  I think this idea can be further developed.
> xref-show--xrefs-buffer could have an 'm' key binding that "minimizes"
> its window by switching to xref-show-xrefs-without-buffer (below) and
> that function can "maximize" back with the same 'm' key.  A customizable
> variable could define the initial behavior.

This looks like good ol' find-tag and tags-loop-continue bound to M-,
with an optional pop-up to the xref buffer by a dedicated key.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#33992; Package emacs. (Thu, 02 May 2019 23:07:01 GMT) Full text and rfc822 format available.

Message #38 received at 33992 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: 33992 <at> debbugs.gnu.org,
 João Távora <joaotavora <at> gmail.com>
Subject: Re: bug#33992: 27.0.50; xref-find-definitions wastes too much space
Date: Fri, 3 May 2019 02:05:55 +0300
On 07.04.2019 0:03, Juri Linkov wrote:
>>>> Does it feel the same way to you?
>>>
>>> The difference is that completions pop up in a small unobtrusive window.
>>
>> Small window? I usually have a side-by-side fullscreen split, and if
>> I initiate completion in one of the windows, *Completion* takes up the
>> whole other window. Temporarily, of course.
> 
> The key word here is 'Temporarily'.  Unlike *Completions*,
> the *xref* buffer doesn't go out easily.

I can understand that. So yes, I can see myself preferring some 
different behavior for a particular command.

>>>     (defun display-buffer-condition-from-xref (_buffer-name _action)
>>>       (string-match-p "\\`\\*\\(xref\\)\\*\\(\\|<[0-9]+>\\)\\'"
>>>                       (buffer-name (current-buffer))))
>>
>> This function seems unused.
> 
> It's unused because it would be useful only in the *xref* buffer
> created by the xref-find-definitions command, so xref needs to
> provide a way to distinguish such case.

Shouldn't it be referenced somewhere else in your patch as well?

>>>     (setq display-buffer-alist
>>>        '((display-buffer-condition-xref
>>>           display-buffer-in-direction
>>
>> And this function is undefined in my Emacs.
> 
> This function is implemented by Martin in bug#33870.

OK, found it, tried it. Seems to work okay-ish for 
xref-find-definitions, except xref-quit-and-goto-xref doesn't seem to be 
functioning too well together with your customization (every other time 
it seemed to use a different window to display the location, not the one 
I called xref-find-definitions from).

>>>     (with-eval-after-load 'xref
>>>       (define-key xref--button-map [(control ?m)] #'xref-quit-and-goto-xref))
>>>
>>> How do you like that?
>>
>> I might, but since I can't really try your customization myself yet, I'll
>> repeat a question you might be familiar with already:
>>
>>    Will this also affect xref-find-references and project-find-regexp?
> 
> It should not affect them due to (memq this-command '(xref-find-definitions))
> above.

It would affect them due to the modification of xref--button-map above, 
though. This part I don't like.

> But also to not affect commands active in the *xref* buffer,
> xref should provide a way to check if the *xref* buffer was created
> by xref-find-definitions.

Yes, we should retain some extra information, e.g. to support revert-buffer.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#33992; Package emacs. (Wed, 15 May 2019 21:55:02 GMT) Full text and rfc822 format available.

Message #41 received at 33992 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 33992 <at> debbugs.gnu.org,
 João Távora <joaotavora <at> gmail.com>
Subject: Re: bug#33992: 27.0.50; xref-find-definitions wastes too much space
Date: Wed, 15 May 2019 23:57:07 +0300
>>>>> Does it feel the same way to you?
>>>>
>>>> The difference is that completions pop up in a small unobtrusive window.
>>>
>>> Small window? I usually have a side-by-side fullscreen split, and if
>>> I initiate completion in one of the windows, *Completion* takes up the
>>> whole other window. Temporarily, of course.
>>
>> The key word here is 'Temporarily'.  Unlike *Completions*,
>> the *xref* buffer doesn't go out easily.
>
> I can understand that. So yes, I can see myself preferring some different
> behavior for a particular command.
>
>>>>     (defun display-buffer-condition-from-xref (_buffer-name _action)
>>>>       (string-match-p "\\`\\*\\(xref\\)\\*\\(\\|<[0-9]+>\\)\\'"
>>>>                       (buffer-name (current-buffer))))
>>>
>>> This function seems unused.
>>
>> It's unused because it would be useful only in the *xref* buffer
>> created by the xref-find-definitions command, so xref needs to
>> provide a way to distinguish such case.
>
> Shouldn't it be referenced somewhere else in your patch as well?

A patch is proposed in a separate bug#35737.

>>>>     (setq display-buffer-alist
>>>>        '((display-buffer-condition-xref
>>>>           display-buffer-in-direction
>>>
>>> And this function is undefined in my Emacs.
>>
>> This function is implemented by Martin in bug#33870.
>
> OK, found it, tried it. Seems to work okay-ish for xref-find-definitions,

Created a separate bug#35592.

> except xref-quit-and-goto-xref doesn't seem to be functioning too well
> together with your customization (every other time it seemed to use
> a different window to display the location, not the one I called
> xref-find-definitions from).

Yes, it should change its behavior depending on xref--original-command.

>>>>     (with-eval-after-load 'xref
>>>>       (define-key xref--button-map [(control ?m)] #'xref-quit-and-goto-xref))
>>>>
>>>> How do you like that?
>>>
>>> I might, but since I can't really try your customization myself yet, I'll
>>> repeat a question you might be familiar with already:
>>>
>>>    Will this also affect xref-find-references and project-find-regexp?
>>
>> It should not affect them due to (memq this-command '(xref-find-definitions))
>> above.
>
> It would affect them due to the modification of xref--button-map above,
> though. This part I don't like.
>
>> But also to not affect commands active in the *xref* buffer,
>> xref should provide a way to check if the *xref* buffer was created
>> by xref-find-definitions.
>
> Yes, we should retain some extra information, e.g. to support revert-buffer.

Created a separate bug#35702.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#33992; Package emacs. (Wed, 15 May 2019 22:39:02 GMT) Full text and rfc822 format available.

Message #44 received at 33992 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: 33992 <at> debbugs.gnu.org,
 João Távora <joaotavora <at> gmail.com>
Subject: Re: bug#33992: 27.0.50; xref-find-definitions wastes too much space
Date: Thu, 16 May 2019 01:37:53 +0300
On 15.05.2019 23:57, Juri Linkov wrote:

>>>>>      (defun display-buffer-condition-from-xref (_buffer-name _action)
>>>>>        (string-match-p "\\`\\*\\(xref\\)\\*\\(\\|<[0-9]+>\\)\\'"
>>>>>                        (buffer-name (current-buffer))))
>>>>
>>>> This function seems unused.
>>>
>>> It's unused because it would be useful only in the *xref* buffer
>>> created by the xref-find-definitions command, so xref needs to
>>> provide a way to distinguish such case.
>>
>> Shouldn't it be referenced somewhere else in your patch as well?
> 
> A patch is proposed in a separate bug#35737.

I don't see display-buffer-condition-from-xref used in that patch 
either, but it's not hugely important, really.

> Created a separate bug#35592.

Thanks.

> Created a separate bug#35702.

Thanks.




Reply sent to Juri Linkov <juri <at> linkov.net>:
You have taken responsibility. (Tue, 28 May 2019 20:48:03 GMT) Full text and rfc822 format available.

Notification sent to Juri Linkov <juri <at> linkov.net>:
bug acknowledged by developer. (Tue, 28 May 2019 20:48:03 GMT) Full text and rfc822 format available.

Message #49 received at 33992-done <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 33992-done <at> debbugs.gnu.org
Subject: Re: bug#33992: 27.0.50; xref-find-definitions wastes too much space
Date: Tue, 28 May 2019 23:35:16 +0300
>> A patch is proposed in a separate bug#35737.

I'm closing this request because now xref is customizable enough.
And we have no more opinions for changing the default behavior.

FWIW, this is what I currently use to customize xref-find-definitions
to act more like Completions:

(custom-set-variables
 '(display-buffer-alist
   '((display-buffer-to-xref-p display-buffer-in-direction
                               (direction . below)
                               (window-height . fit-window-to-buffer)))))

(defun display-buffer-to-xref-p (buffer-name _action)
  (and (string-match-p "\\`\\*\\(xref\\)\\*\\(\\|<[0-9]+>\\)\\'"
                       buffer-name)
       (memq this-command '(xref-find-definitions))))

(with-eval-after-load 'xref
  (defvar xref--original-command nil)
  (advice-add 'xref-find-definitions :after
	      (lambda (&rest _args)
                (with-current-buffer (window-buffer)
                  (setq-local xref--original-command 'xref-find-definitions))))
  (define-key xref--button-map [(control ?m)]
    (lambda ()
      (interactive)
      (if (memq xref--original-command '(xref-find-definitions))
          (call-interactively 'xref-quit-and-goto-xref)
        (setq xref--original-window nil)
        (call-interactively 'xref-goto-xref)))))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#33992; Package emacs. (Mon, 10 Jun 2019 00:36:02 GMT) Full text and rfc822 format available.

Message #52 received at 33992-done <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: 33992-done <at> debbugs.gnu.org,
 João Távora <joaotavora <at> gmail.com>
Subject: Re: bug#33992: 27.0.50; xref-find-definitions wastes too much space
Date: Mon, 10 Jun 2019 03:35:36 +0300
Hi Juri,

On 28.05.2019 23:35, Juri Linkov wrote:

> FWIW, this is what I currently use to customize xref-find-definitions
> to act more like Completions:
> 
> (custom-set-variables
>   '(display-buffer-alist
>     '((display-buffer-to-xref-p display-buffer-in-direction
>                                 (direction . below)
>                                 (window-height . fit-window-to-buffer)))))
> 
> (defun display-buffer-to-xref-p (buffer-name _action)
>    (and (string-match-p "\\`\\*\\(xref\\)\\*\\(\\|<[0-9]+>\\)\\'"
>                         buffer-name)
>         (memq this-command '(xref-find-definitions))))
> 
> (with-eval-after-load 'xref
>    (defvar xref--original-command nil)
>    (advice-add 'xref-find-definitions :after
> 	      (lambda (&rest _args)
>                  (with-current-buffer (window-buffer)
>                    (setq-local xref--original-command 'xref-find-definitions))))
>    (define-key xref--button-map [(control ?m)]
>      (lambda ()
>        (interactive)
>        (if (memq xref--original-command '(xref-find-definitions))
>            (call-interactively 'xref-quit-and-goto-xref)
>          (setq xref--original-window nil)
>          (call-interactively 'xref-goto-xref)))))

JFYI, one of the changes I've pushed yesterday was the patch I've shown 
before, and it should let you have the same behavior with one line:

(setq xref-show-definitions-function 'xref--show-defs-buffer-at-bottom)

It doesn't seem like xref-quit-and-goto-xref works well, though.

It doesn't always honor the intention to open the location in the window 
the command was called from. It can show the location in a different 
window, and then if I press M-, from there, Emacs does not return to the 
previous window configuration.

I've tracked this down to xref--show-pos-in-buf. Apparently, calling 
display-buffer with

  `((display-buffer-in-previous-window)
    (previous-window . ,xref--original-window))

as its second argument doesn't do the trick. Looking at it, it contains 
logic where when (eq window (selected-window)), it sets best-window to 
something else (in particular, it can favor a window where this buffer 
had been displayed previously). So, should some other action function be 
used?

I can trace this choice back to your commit 94b320849e9 where this bug 
was apparently introduced.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 08 Jul 2019 11:24:07 GMT) Full text and rfc822 format available.

This bug report was last modified 4 years and 265 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.