GNU bug report logs -
#33870
27.0.50; xref-goto-xref not configurable
Previous Next
Reported by: Juri Linkov <juri <at> linkov.net>
Date: Tue, 25 Dec 2018 20:53:01 UTC
Severity: minor
Found in version 27.0.50
Done: Dmitry Gutov <dgutov <at> yandex.ru>
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 33870 in the body.
You can then email your comments to 33870 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
dgutov <at> yandex.ru, bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 25 Dec 2018 20:53: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
dgutov <at> yandex.ru, bug-gnu-emacs <at> gnu.org.
(Tue, 25 Dec 2018 20:53:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
X-Debbugs-CC: Dmitry Gutov <dgutov <at> yandex.ru>
There is no more need to replace switch-to-buffer with
pop-to-buffer-same-window in xref--pop-to-location
like was asked in https://debbugs.gnu.org/32790#206
because now a new option switch-to-buffer-obey-display-actions
can be customized to t.
But still there is one xref command, namely `xref-goto-xref' bound to
RET in the *xref* buffer that always displays the buffer in the
predefined window, and there is no way to change this behavior.
Is it possible to change it to use either pop-to-buffer-same-window
or at least switch-to-buffer?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 26 Dec 2018 02:11:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 33870 <at> debbugs.gnu.org (full text, mbox):
Hi Juri,
On 25.12.2018 22:42, Juri Linkov wrote:
> X-Debbugs-CC: Dmitry Gutov <dgutov <at> yandex.ru>
>
> There is no more need to replace switch-to-buffer with
> pop-to-buffer-same-window in xref--pop-to-location
> like was asked in https://debbugs.gnu.org/32790#206
> because now a new option switch-to-buffer-obey-display-actions
> can be customized to t.
Sorry I never responded, see the message in that other thread.
> But still there is one xref command, namely `xref-goto-xref' bound to
> RET in the *xref* buffer that always displays the buffer in the
> predefined window, and there is no way to change this behavior.
>
> Is it possible to change it to use either pop-to-buffer-same-window
> or at least switch-to-buffer?
IIUC you want to change xref--show-pos-in-buf to use
pop-to-buffer-same-window or switch-to-buffer instead of display-buffer.
Would you like to propose a patch that would still honor the 'action'
argument (or the values of xref--original-window* directly)?
Also pinging João who wrote that code.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 26 Dec 2018 14:49:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 33870 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi Juri and Dmitry
Any simplification to the implementation that keeps the
"keep original window intent" behavior across xref
intermediate buffers is very welcome.
Juri, do you understand what that particular sub-feature
is about? I wish I had written some tests to go with it
but testing window code is tricky. I will still try, that way
you needn't worry about understanding it and can program
"against the rails".
However here's a tangent that might affect the decision.
Is there any impediment to making xref.el a core ELPA
package? I can see some advantages... The reason I bring
this up here is that using very new elisp in a file can reduce
the usefulness of that goal, which in this case would be
to bring new xref features to users of Emacs 26.1/26.2.
Perhaps it is already using post-26 features in which case
the ship has sailed. In that case, disregard this tangent.
João
On Wed, Dec 26, 2018, 02:10 Dmitry Gutov <dgutov <at> yandex.ru wrote:
> Hi Juri,
>
> On 25.12.2018 22:42, Juri Linkov wrote:
> > X-Debbugs-CC: Dmitry Gutov <dgutov <at> yandex.ru>
> >
> > There is no more need to replace switch-to-buffer with
> > pop-to-buffer-same-window in xref--pop-to-location
> > like was asked in https://debbugs.gnu.org/32790#206
> > because now a new option switch-to-buffer-obey-display-actions
> > can be customized to t.
>
> Sorry I never responded, see the message in that other thread.
>
> > But still there is one xref command, namely `xref-goto-xref' bound to
> > RET in the *xref* buffer that always displays the buffer in the
> > predefined window, and there is no way to change this behavior.
> >
> > Is it possible to change it to use either pop-to-buffer-same-window
> > or at least switch-to-buffer?
>
> IIUC you want to change xref--show-pos-in-buf to use
> pop-to-buffer-same-window or switch-to-buffer instead of display-buffer.
>
> Would you like to propose a patch that would still honor the 'action'
> argument (or the values of xref--original-window* directly)?
>
> Also pinging João who wrote that code.
>
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 26 Dec 2018 15:37:01 GMT)
Full text and
rfc822 format available.
Message #14 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> From: Juri Linkov <juri <at> linkov.net>
> Date: Tue, 25 Dec 2018 22:42:28 +0200
> Cc: dmitry gutov <dgutov <at> yandex.ru>
>
> But still there is one xref command, namely `xref-goto-xref' bound to
> RET in the *xref* buffer that always displays the buffer in the
> predefined window, and there is no way to change this behavior.
How would you like to change this behavior, and why? There are quite
a few tricky use cases, which took us some of time to get right, and I
wouldn't like to break them.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 26 Dec 2018 23:22:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> But still there is one xref command, namely `xref-goto-xref' bound to
>> RET in the *xref* buffer that always displays the buffer in the
>> predefined window, and there is no way to change this behavior.
>
> How would you like to change this behavior, and why? There are quite
> a few tricky use cases, which took us some of time to get right, and I
> wouldn't like to break them.
Since we have a convention that code displaying a buffer in the
window have to obey display actions that are customizable by
`display-buffer-alist', `display-buffer-overriding-action',
and other display related variables (e.g. like implemented
recently with switch-to-buffer-obey-display-actions),
I see no reason why *xref* should differ from *grep* or *Occur*.
It should not involve more complexity than necessary.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 26 Dec 2018 23:22:02 GMT)
Full text and
rfc822 format available.
Message #20 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Any simplification to the implementation that keeps the
> "keep original window intent" behavior across xref
> intermediate buffers is very welcome.
>
> Juri, do you understand what that particular sub-feature
> is about?
I don't understand what does this "keep original window intent" mean.
Please explain. I want to understand how it is different from other
modes that display a buffer in another window, such as e.g.
after `C-x C-b' (list-buffers) typing `C-o' displays the buffer
in another window, or e.g. typing `C-o' in the Dired buffer
opens it in another window, and many other cases.
> However here's a tangent that might affect the decision.
> Is there any impediment to making xref.el a core ELPA
> package? I can see some advantages... The reason I bring
> this up here is that using very new elisp in a file can reduce
> the usefulness of that goal, which in this case would be
> to bring new xref features to users of Emacs 26.1/26.2.
> Perhaps it is already using post-26 features in which case
> the ship has sailed. In that case, disregard this tangent.
Display actions have been in Emacs for a long time customizable
by `display-buffer-alist', so if you need some specific logic,
it's easy to implement the corresponding display action
that can be overridden by the users.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 27 Dec 2018 00:07:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 33870 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
> I don't understand what does this "keep original window intent" mean.
> Please explain.
Hi Juri.
You can read up the whole bug here:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28814
I quote from that thread:
Here are two very simple Emacs -Q recipes that demonstrate [the bug]
emacs -Q
C-x 3 [split-window-right]
C-x 2 [split-window-below]
M-. xref-backend-definitions RET [xref-find-definitions]
C-n [next-line]
RET [xref-goto-xref]
Expected the definition to be found in the original window where I
pressed M-. but instead it was found in another. Another case:
emacs -Q
C-x 4 . xref-backend-definitions RET [xref-find-definitions-other-window]
C-n
RET
Expected the definition to be found in some other window, different
from the one I pressed M-. on. Instead went to the same one. Also,
in both situations, expected the window configuration to be the same
as if I had searched for, say, xref-backend-functions [which only
has a single definition].
The bugfix makes these two recipes work like expected (that is the
"Expected the definition" sentence is now fulfilled). We want to make
sure this is not broken.
> I want to understand how it is different from other modes that display
> a buffer in another window, such as e.g. after `C-x C-b'
> (list-buffers) typing `C-o' displays the buffer in another window, or
> e.g. typing `C-o' in the Dired buffer opens it in another window, and
> many other cases.
*Before* pressing C-x C-b, you probably had no expectations as to what
window should be used to display your decision. But M-. you have:
M-. means "use the same window", C-x 4 . means use "other window" and
C-x 5 . means "other frame". Crucially, the existance or not of
multiple loci of the (something that generally cannot be known in
advange) shouldn't influence the results of "other window" or "other
frame" intention.
So, by default, if display-* variables not are set specially by the
user, then final product of those two recipes should stay the same and,
more importantly, the principle that I described in the previous
paragraph should hold.
>> However here's a tangent that might affect the decision.
>> Is there any impediment to making xref.el a core ELPA
>> package? I can see some advantages... The reason I bring
>> this up here is that using very new elisp in a file can reduce
>> the usefulness of that goal, which in this case would be
>> to bring new xref features to users of Emacs 26.1/26.2.
>> Perhaps it is already using post-26 features in which case
>> the ship has sailed. In that case, disregard this tangent.
>
> Display actions have been in Emacs for a long time customizable
> by `display-buffer-alist', so if you need some specific logic,
> it's easy to implement the corresponding display action
> that can be overridden by the users.
I know that. I think either I or you missed something in this
paragraph. Let me put it like this: are you planning to use, in
master's xref.el a new Elisp feature (a new function, argument to a
function, or a new semantics of a specific argument) that would make
loading that file in Emacs 26.1 fail in some way or other?
If master's xref.el works in 26.1 before your changes, and not anymore
after your changes, then the possibility of putting xref.el in ELPA as a
"core" package is lost. (But perhaps that possibility is not desirable
to begin with, so this is why I said this was a tangent).
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 27 Dec 2018 13:21:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 27.12.2018 2:05, João Távora wrote:
> You can read up the whole bug here:
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28814
>
> I quote from that thread:
>
> Here are two very simple Emacs -Q recipes that demonstrate [the bug]
Does this work well for everybody?
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index c71802c918..85d4325d9e 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -494,7 +494,8 @@ xref--show-pos-in-buf
(or (and (window-live-p xref--original-window)
xref--original-window)
(selected-window))
- (display-buffer buf action))
+ (pop-to-buffer buf action)
+ (selected-window))
(xref--goto-char pos)
(run-hooks 'xref-after-jump-hook)
(let ((buf (current-buffer)))
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 27 Dec 2018 15:28:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> From: Juri Linkov <juri <at> linkov.net>
> Cc: 33870 <at> debbugs.gnu.org, dgutov <at> yandex.ru
> Date: Thu, 27 Dec 2018 01:17:00 +0200
>
> Since we have a convention that code displaying a buffer in the
> window have to obey display actions that are customizable by
> `display-buffer-alist', `display-buffer-overriding-action',
> and other display related variables (e.g. like implemented
> recently with switch-to-buffer-obey-display-actions),
> I see no reason why *xref* should differ from *grep* or *Occur*.
> It should not involve more complexity than necessary.
We are under no obligation to maintain 100% consistency between
different commands, just because some of their aspects are similar.
I'd like to hear of specific real-life situations where the current
code doesn't DTRT, before I'd agree to a change in this area.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 27 Dec 2018 18:09:02 GMT)
Full text and
rfc822 format available.
Message #32 received at 33870 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 27.12.2018 2:05, João Távora wrote:
>> You can read up the whole bug here:
>>
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28814
>>
>> I quote from that thread:
>>
>> Here are two very simple Emacs -Q recipes that demonstrate [the bug]
>
> Does this work well for everybody?
>
> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
> index c71802c918..85d4325d9e 100644
> --- a/lisp/progmodes/xref.el
> +++ b/lisp/progmodes/xref.el
> @@ -494,7 +494,8 @@ xref--show-pos-in-buf
> (or (and (window-live-p xref--original-window)
> xref--original-window)
> (selected-window))
> - (display-buffer buf action))
> + (pop-to-buffer buf action)
> + (selected-window))
> (xref--goto-char pos)
> (run-hooks 'xref-after-jump-hook)
> (let ((buf (current-buffer)))
It works for those two recipes, because `action' is passed directly to
display-buffer. Don't know:
* if it might be introducing a new untested corner case because of the
"recording" of select-window inside. Perhaps this corner case is
unwanted, but it might also be wanted (and we didn't know).
* if it will bring Yuri what he wants. Doesn't seem like it will, since
display buffer is given an explicit ACTION.
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 27 Dec 2018 20:52:02 GMT)
Full text and
rfc822 format available.
Message #35 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 27.12.2018 17:27, Eli Zaretskii wrote:
> We are under no obligation to maintain 100% consistency between
> different commands, just because some of their aspects are similar.
> I'd like to hear of specific real-life situations where the current
> code doesn't DTRT, before I'd agree to a change in this area.
I think it *would* be handy if in the future we could make 'C-x 4' and
similar prefixes work via some common code, instead of requiring
separate key bindings and handling inside each command.
So if we manage support Juri's request without complicating the code too
much, we really should.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 27 Dec 2018 21:25:04 GMT)
Full text and
rfc822 format available.
Message #38 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Here are two very simple Emacs -Q recipes that demonstrate [the bug]
Thanks for the recipes.
> emacs -Q
> C-x 3 [split-window-right]
> C-x 2 [split-window-below]
> M-. xref-backend-definitions RET [xref-find-definitions]
> C-n [next-line]
> RET [xref-goto-xref]
>
> Expected the definition to be found in the original window where I
> pressed M-. but instead it was found in another. Another case:
It could help to try using 'get-mru-window'. Please ask Martin
if there is a display action that uses 'get-mru-window', or how
to temporarily change the default behavior from 'get-lru-window'
to 'get-mru-window'.
> emacs -Q
> C-x 4 . xref-backend-definitions RET [xref-find-definitions-other-window]
> C-n
> RET
>
> Expected the definition to be found in some other window, different
> from the one I pressed M-. on. Instead went to the same one. Also,
> in both situations, expected the window configuration to be the same
> as if I had searched for, say, xref-backend-functions [which only
> has a single definition].
This can be configured with the display buffer alist
`(inhibit-same-window . t)'.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 27 Dec 2018 21:25:04 GMT)
Full text and
rfc822 format available.
Message #41 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Does this work well for everybody?
>
> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
> index c71802c918..85d4325d9e 100644
> --- a/lisp/progmodes/xref.el
> +++ b/lisp/progmodes/xref.el
> @@ -494,7 +494,8 @@ xref--show-pos-in-buf
> (or (and (window-live-p xref--original-window)
> xref--original-window)
> (selected-window))
> - (display-buffer buf action))
> + (pop-to-buffer buf action)
> + (selected-window))
> (xref--goto-char pos)
> (run-hooks 'xref-after-jump-hook)
> (let ((buf (current-buffer)))
Unfortunately, I see no difference. It still selects the original window
before calling pop-to-buffer, so the display action is relative
to the original window, not to the currently selected window
as it would be natural to expect.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 27 Dec 2018 21:50:02 GMT)
Full text and
rfc822 format available.
Message #44 received at 33870 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
>> Here are two very simple Emacs -Q recipes that demonstrate [the bug]
>
> Thanks for the recipes.
>
>> emacs -Q
>> C-x 3 [split-window-right]
>> C-x 2 [split-window-below]
>> M-. xref-backend-definitions RET [xref-find-definitions]
>> C-n [next-line]
>> RET [xref-goto-xref]
>>
>> Expected the definition to be found in the original window where I
>> pressed M-. but instead it was found in another. Another case:
>
> It could help to try using 'get-mru-window'. Please ask Martin
> if there is a display action that uses 'get-mru-window', or how
> to temporarily change the default behavior from 'get-lru-window'
> to 'get-mru-window'.
There may be a misunderstanding here. Those recipes are for a bug that
has already been fixed: this code is now working like it should.
Are you saying that you could make the code use other functions to
produce the same behaviour, i.e. refactor it? That's fine by me: feel
free to try, but I don't see a lot of motivation for it.
>
>> emacs -Q
>> C-x 4 . xref-backend-definitions RET [xref-find-definitions-other-window]
>> C-n
>> RET
>>
>> Expected the definition to be found in some other window, different
>> from the one I pressed M-. on. Instead went to the same one. Also,
>> in both situations, expected the window configuration to be the same
>> as if I had searched for, say, xref-backend-functions [which only
>> has a single definition].
>
> This can be configured with the display buffer alist
> `(inhibit-same-window . t)'.
Same here. I'm not an expert in the `display-buffer-alist' DSL, but I
think you are again papering over the fact that between
xref-find-definitions-other-window and the final destination buffer
there is sometimes an *xref* buffer in its own window. So I don't think
'inhibit-same-window' wouldn't help here. But again, feel free to
rework the code to your standards and if it passes these two tests, it's
a good start.
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 27 Dec 2018 23:24:02 GMT)
Full text and
rfc822 format available.
Message #47 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 27.12.2018 23:21, Juri Linkov wrote:
> Unfortunately, I see no difference. It still selects the original window
> before calling pop-to-buffer, so the display action is relative
> to the original window, not to the currently selected window
> as it would be natural to expect.
That was the intention, making the xref window transient, in a way.
Is it really the thing you want to fix?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 28 Dec 2018 00:02:02 GMT)
Full text and
rfc822 format available.
Message #50 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> Unfortunately, I see no difference. It still selects the original window
>> before calling pop-to-buffer, so the display action is relative
>> to the original window, not to the currently selected window
>> as it would be natural to expect.
>
> That was the intention, making the xref window transient, in a way.
>
> Is it really the thing you want to fix?
Transient is a window like e.g. *Completions*. I don't see any
indication that *xref* is transient in the same sense. It looks
more like multi-occur *Occur*.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 28 Dec 2018 00:37:01 GMT)
Full text and
rfc822 format available.
Message #53 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 28.12.2018 1:47, Juri Linkov wrote:
> Transient is a window like e.g. *Completions*. I don't see any
> indication that *xref* is transient in the same sense. It looks
> more like multi-occur *Occur*.
Well, it's the idea behind the feature which we merged last year, see
2a973edeacefcabb9fd8024188b7e167f0f9a9b6.
I think it makes a certain amount of sense, behavior-wise. Should we add
some visual cues?
I'm not a huge fan of how *Completions* behaves, though, so we're not
going to make it look exactly alike.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 28 Dec 2018 09:27:02 GMT)
Full text and
rfc822 format available.
Message #56 received at 33870 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Fri, Dec 28, 2018 at 12:36 AM Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> On 28.12.2018 1:47, Juri Linkov wrote:
>
> > Transient is a window like e.g. *Completions*. I don't see any
> > indication that *xref* is transient in the same sense. It looks
> > more like multi-occur *Occur*.
> Well, it's the idea behind the feature which we merged last year, see
> 2a973edeacefcabb9fd8024188b7e167f0f9a9b6.
>
What's more, in a sense *xref* is closer to *Completions* than
to *Occur*, though it's not exactly like any of the two, because
*xref*, like *Completions*, only appears in certain situations.
João
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 03 Jan 2019 00:23:02 GMT)
Full text and
rfc822 format available.
Message #59 received at 33870 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi João
> Any simplification to the implementation that keeps the
> "keep original window intent" behavior across xref
> intermediate buffers is very welcome.
Thanks for the explanation. Now I understand better the intent in
xref--show-pos-in-buf. Generally, I'd like to see the “keep original
window intent” behavior in more places, e.g. in *Occur*, *grep*, etc.
Based on your explanation, I've been able to write the patch that does
the following:
1. simplifies ‘xref--show-pos-in-buf’ while at the same time
preserves the current behavior and respects user's customization
of display actions;
2. makes the xref buffer non-obtrusive like *Completions*
in xref--show-xref-buffer;
3. turns the existing arg QUIT of xref-goto-xref into a prefix arg,
so a natural key sequence ‘C-u RET’ will quit the window.
This is similar to the prefix arg of quit-window.
[xref.el.patch (text/x-diff, inline)]
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 87ce2299c5..a166f8299c 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -482,19 +482,9 @@ xref--show-pos-in-buf
(window-live-p xref--original-window)
(or (not (window-dedicated-p xref--original-window))
(eq (window-buffer xref--original-window) buf)))
- `(,(lambda (buf _alist)
- (set-window-buffer xref--original-window buf)
- xref--original-window))))))
- (with-selected-window
- (with-selected-window
- ;; Just before `display-buffer', place ourselves in the
- ;; original window to suggest preserving it. Of course, if
- ;; user has deleted the original window, all bets are off,
- ;; just use the selected one.
- (or (and (window-live-p xref--original-window)
- xref--original-window)
- (selected-window))
- (display-buffer buf action))
+ `(,(lambda (buf alist)
+ (window--display-buffer buf xref--original-window 'reuse alist)))))))
+ (with-selected-window (display-buffer buf action)
(xref--goto-char pos)
(run-hooks 'xref-after-jump-hook)
(let ((buf (current-buffer)))
@@ -548,9 +538,8 @@ 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)
+A prefix arg QUIT 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")))
@@ -782,7 +771,17 @@ 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)
+ `((display-buffer--maybe-same-window
+ display-buffer-reuse-window
+ display-buffer--maybe-pop-up-frame
+ display-buffer-below-selected)
+ ,(if temp-buffer-resize-mode
+ '(window-height . resize-temp-buffer-window)
+ '(window-height . fit-window-to-buffer))
+ ,(when temp-buffer-resize-mode
+ '(preserve-size . (nil . t)))))
(goto-char (point-min))
(setq xref--original-window (assoc-default 'window alist)
xref--original-window-intent (assoc-default 'display-action alist))
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 03 Jan 2019 13:51:01 GMT)
Full text and
rfc822 format available.
Message #62 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> From: Juri Linkov <juri <at> linkov.net>
> Date: Thu, 03 Jan 2019 02:18:50 +0200
> Cc: 33870 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
>
> 1. simplifies ‘xref--show-pos-in-buf’ while at the same time
> preserves the current behavior and respects user's customization
> of display actions;
>
> 2. makes the xref buffer non-obtrusive like *Completions*
> in xref--show-xref-buffer;
>
> 3. turns the existing arg QUIT of xref-goto-xref into a prefix arg,
> so a natural key sequence ‘C-u RET’ will quit the window.
> This is similar to the prefix arg of quit-window.
Please be sure to document any user-visible behavior changes in NEWS
and in the manual.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 03 Jan 2019 14:25:03 GMT)
Full text and
rfc822 format available.
Message #65 received at 33870 <at> debbugs.gnu.org (full text, mbox):
Hi Juri,
Juri Linkov <juri <at> linkov.net> writes:
> 1. simplifies ‘xref--show-pos-in-buf’
... and considerably complexifies xref--show-xref-buffer (more on that
later)
> while at the same time preserves the current behavior and respects
> user's customization of display actions;
That's great!
> 2. makes the xref buffer non-obtrusive like *Completions*
> in xref--show-xref-buffer;
After a brief look, I'm not sure I like the UI change. "not sure" is
not an euphemism for "don't like", I'm ust not sold on the idea yet:
* Certainly you don't mean non-obtrusive, you mean "less obtrusive" and
really it's "slightly less obtrusive". It does use potentially less
space and doesn't temporarily use one of your windows if you happen to
have several. I agree this is an good advantage.
* But by using less space it is also less useful. You don't get to see,
at a glance, a great deal of xrefs. And xrefs are different from
completions, they're closer to grep hits. You wouldn't put *grep*
hits in such a potentially tiny window, would you?
Then again, perhaps you would, and the whole point of this patch is to
make the UI configurable. If so, I'd make the original UI the default,
or at least very very easy to bring back.
> 3. turns the existing arg QUIT of xref-goto-xref into a prefix arg,
> so a natural key sequence ‘C-u RET’ will quit the window.
> This is similar to the prefix arg of quit-window.
No problem here I think.
> - (display-buffer buf action))
> + `(,(lambda (buf alist)
> + (window--display-buffer buf xref--original-window 'reuse alist)))))))
>
Using internal "--" symbols from window.el is a temporary solution I
hope.
> - (pop-to-buffer (current-buffer))
> + (pop-to-buffer
> + (current-buffer)
> + `((display-buffer--maybe-same-window
> + display-buffer-reuse-window
> + display-buffer--maybe-pop-up-frame
> + display-buffer-below-selected)
> + ,(if temp-buffer-resize-mode
> + '(window-height . resize-temp-buffer-window)
> + '(window-height . fit-window-to-buffer))
> + ,(when temp-buffer-resize-mode
> + '(preserve-size . (nil . t)))))
Again, too many --, and seems like a lot of repetition from window.el.
Perhaps you want window.el to export a function that encapsulates
all/some of this cruft to pass as ACTION. Naming that function would be
the hardest problem (best I could do is
display-buffer-use-completions-like-window).
Or maybe put that function in xref.el. But as I said above, I think we
also need a function that brings back the current default.
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 03 Jan 2019 21:40:02 GMT)
Full text and
rfc822 format available.
Message #68 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> 1. simplifies ‘xref--show-pos-in-buf’
>
> ... and considerably complexifies xref--show-xref-buffer (more on that
> later)
>
>> while at the same time preserves the current behavior and respects
>> user's customization of display actions;
>
> That's great!
I realized now that it can be simplified more by replacing
(with-selected-window (display-buffer buf action)
with just
(pop-to-buffer buf action)
but I'm not sure about this change because it could change the current behavior.
>> 2. makes the xref buffer non-obtrusive like *Completions*
>> in xref--show-xref-buffer;
>
> After a brief look, I'm not sure I like the UI change. "not sure" is
> not an euphemism for "don't like", I'm ust not sold on the idea yet:
>
> * Certainly you don't mean non-obtrusive, you mean "less obtrusive" and
> really it's "slightly less obtrusive". It does use potentially less
> space and doesn't temporarily use one of your windows if you happen to
> have several. I agree this is an good advantage.
>
> * But by using less space it is also less useful. You don't get to see,
> at a glance, a great deal of xrefs. And xrefs are different from
> completions, they're closer to grep hits. You wouldn't put *grep*
> hits in such a potentially tiny window, would you?
>
> Then again, perhaps you would, and the whole point of this patch is to
> make the UI configurable. If so, I'd make the original UI the default,
> or at least very very easy to bring back.
I see what you mean. For a command like project-find-regexp I'd like
the original UI as well, because most of the time there are many hits
displayed in the xref window. But when xref-find-definitions pops up
the xref window, usually it contains just 2 lines taking half of the screen
where most space is uselessly empty.
So it seems that project-find-regexp and most other xref-related
commands are more like grep while xref-find-definitions is more like
completions with a small number of lines.
What do you think about allowing only xref-find-definitions
to display a narrow xref window below the original window?
>> - (display-buffer buf action))
>> + `(,(lambda (buf alist)
>> + (window--display-buffer buf xref--original-window 'reuse alist)))))))
>>
>
> Using internal "--" symbols from window.el is a temporary solution I
> hope.
Actually this function is not quite internal. It's intended to be used
in display actions implemented by packages.
>> - (pop-to-buffer (current-buffer))
>> + (pop-to-buffer
>> + (current-buffer)
>> + `((display-buffer--maybe-same-window
>> + display-buffer-reuse-window
>> + display-buffer--maybe-pop-up-frame
>> + display-buffer-below-selected)
>> + ,(if temp-buffer-resize-mode
>> + '(window-height . resize-temp-buffer-window)
>> + '(window-height . fit-window-to-buffer))
>> + ,(when temp-buffer-resize-mode
>> + '(preserve-size . (nil . t)))))
>
>
> Again, too many --, and seems like a lot of repetition from window.el.
The distinction between internal and public window functions is quite fuzzy.
> Perhaps you want window.el to export a function that encapsulates
> all/some of this cruft to pass as ACTION.
Yes, creating a composite display action would be a good thing to do.
> Naming that function would be the hardest problem (best I could do is
> display-buffer-use-completions-like-window).
Or when naming by not its usage but what it does: display-buffer-below-and-resize.
> Or maybe put that function in xref.el. But as I said above, I think we
> also need a function that brings back the current default.
I propose to use the new function only for xref-find-definitions.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 03 Jan 2019 22:09:02 GMT)
Full text and
rfc822 format available.
Message #71 received at 33870 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
> (with-selected-window (display-buffer buf action)
> with just
> (pop-to-buffer buf action)
>
> but I'm not sure about this change because it could change the current
> behavior.
Didn't test, but that would be bad. Is this small reduction worth it?
>>> 2. makes the xref buffer non-obtrusive like *Completions*
>>> in xref--show-xref-buffer;
> What do you think about allowing only xref-find-definitions
> to display a narrow xref window below the original window?
I don't know. Can I get back the original behaviour easily? If so,
how?
I ask because the assumption that xref-find-definitions produces a small
number of lines is really quite brittle. Generic functions can have
many, many methods. In Emacs, cl-print-object has 10 definitions lines,
but that could/should easily grow as anyone who devises a new type of
object can write a cl-print-object for it. In a Common Lisp system
CL:PRINT-OBJECT usually has a ton of methods (and I'm trying to write a
CL IDE that uses xref.el)
>>> - (display-buffer buf action))
>>> + `(,(lambda (buf alist)
>>> + (window--display-buffer buf xref--original-window 'reuse alist)))))))
>>>
>>
>> Using internal "--" symbols from window.el is a temporary solution I
>> hope.
>
> Actually this function is not quite internal. It's intended to be used
> in display actions implemented by packages.
Hmmm, it's used only in lisp/window.el, where it hails from, and in
lisp/windmove.el, where you added it recently.
If it's part of the API, it should really be named
window-display-buffer. I'm just making sure it isn't an implementation
detail for which Martin reserve the to change at any time.
>> Again, too many --, and seems like a lot of repetition from window.el.
> The distinction between internal and public window functions is quite
> fuzzy.
It shouldn't be. If a package A uses -- from package B, either A is
going to break soon, or B's API is insufficient.
>> Perhaps you want window.el to export a function that encapsulates
>> all/some of this cruft to pass as ACTION.
> Yes, creating a composite display action would be a good thing to do.
And can you create one such composite display action that brings exactly
the current *xref* behaviour? Or does one such thing already exist?
>> Naming that function would be the hardest problem (best I could do is
>> display-buffer-use-completions-like-window).
> Or when naming by not its usage but what it does:
> display-buffer-below-and-resize.
OK. Better, I guess (if that's really what it does).
>> Or maybe put that function in xref.el. But as I said above, I think we
>> also need a function that brings back the current default.
> I propose to use the new function only for xref-find-definitions.
OK, but I would say this is a separate request:
* This bug is about making the xref.el window-popping behaviour
configurable using display-buffer-alist&friends while keeping the UI.
That goal is now apparently within reach;
* The goal of changing the default UI for a certain part of xref-*
commands is a different one, which I don't necessarily oppose, but it
should be discussed and implemented separately.
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 03 Jan 2019 22:49:01 GMT)
Full text and
rfc822 format available.
Message #74 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 03.01.2019 17:24, João Távora wrote:
> * But by using less space it is also less useful. You don't get to see,
> at a glance, a great deal of xrefs. And xrefs are different from
> completions, they're closer to grep hits. You wouldn't put*grep*
> hits in such a potentially tiny window, would you?
This would be my main objection as well.
> Then again, perhaps you would, and the whole point of this patch is to
> make the UI configurable. If so, I'd make the original UI the default,
> or at least very very easy to bring back.
Making it configurable on a high level is something I might agree with
(and indeed, the find-definitions command probably should behave
differently from the rest), but the way it's done should be transparent
and not strongly coupled to a particular commands.
Maybe via two different xrefs-show-function values. I'm not sure.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 03 Jan 2019 22:50:02 GMT)
Full text and
rfc822 format available.
Message #77 received at 33870 <at> debbugs.gnu.org (full text, mbox):
And also:
On 03.01.2019 3:18, Juri Linkov wrote:
> (xref--xref-buffer-mode)
> - (pop-to-buffer (current-buffer))
> + (pop-to-buffer
> + (current-buffer)
> + `((display-buffer--maybe-same-window
> + display-buffer-reuse-window
> + display-buffer--maybe-pop-up-frame
> + display-buffer-below-selected)
> + ,(if temp-buffer-resize-mode
> + '(window-height . resize-temp-buffer-window)
> + '(window-height . fit-window-to-buffer))
> + ,(when temp-buffer-resize-mode
> + '(preserve-size . (nil . t)))))
Are we really supposed to use the private functions here, outside of
window.el?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 03 Jan 2019 23:33:01 GMT)
Full text and
rfc822 format available.
Message #80 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 03.01.2019 3:18, Juri Linkov wrote:
> 3. turns the existing arg QUIT of xref-goto-xref into a prefix arg,
> so a natural key sequence ‘C-u RET’ will quit the window.
> This is similar to the prefix arg of quit-window.
Kind of similar, but it has a different effect, right? So the logic
doesn't really translate.
Since we already have the xref-quit-and-goto-xref command, I'm not so
sure this part is particularly useful.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 04 Jan 2019 00:17:02 GMT)
Full text and
rfc822 format available.
Message #83 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>>>> 2. makes the xref buffer non-obtrusive like *Completions*
>>>> in xref--show-xref-buffer;
>> What do you think about allowing only xref-find-definitions
>> to display a narrow xref window below the original window?
>
> I don't know. Can I get back the original behaviour easily? If so,
> how?
This should be easy using a new display action.
> I ask because the assumption that xref-find-definitions produces a small
> number of lines is really quite brittle. Generic functions can have
> many, many methods. In Emacs, cl-print-object has 10 definitions lines,
> but that could/should easily grow as anyone who devises a new type of
> object can write a cl-print-object for it. In a Common Lisp system
> CL:PRINT-OBJECT usually has a ton of methods (and I'm trying to write a
> CL IDE that uses xref.el)
In my experience 10 lines is an exception. Even with more lines the
completions-like xref window remain pretty usable.
> If it's part of the API, it should really be named
> window-display-buffer. I'm just making sure it isn't an implementation
> detail for which Martin reserve the to change at any time.
I agree, window--display-buffer is more public towards other packages
and could be renamed.
>>> Perhaps you want window.el to export a function that encapsulates
>>> all/some of this cruft to pass as ACTION.
>> Yes, creating a composite display action would be a good thing to do.
>
> And can you create one such composite display action that brings exactly
> the current *xref* behaviour? Or does one such thing already exist?
It's as easy as moving components of the particular display action
from the recent patch into a separate function.
>>> Or maybe put that function in xref.el. But as I said above, I think we
>>> also need a function that brings back the current default.
>> I propose to use the new function only for xref-find-definitions.
>
> OK, but I would say this is a separate request:
>
> * This bug is about making the xref.el window-popping behaviour
> configurable using display-buffer-alist&friends while keeping the UI.
> That goal is now apparently within reach;
>
> * The goal of changing the default UI for a certain part of xref-*
> commands is a different one, which I don't necessarily oppose, but it
> should be discussed and implemented separately.
Actually the request was about making xref windows more configurable.
I could rename the subject if necessary, or create a separate request
for more discussion.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 04 Jan 2019 00:17:02 GMT)
Full text and
rfc822 format available.
Message #86 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> * But by using less space it is also less useful. You don't get to see,
>> at a glance, a great deal of xrefs. And xrefs are different from
>> completions, they're closer to grep hits. You wouldn't put*grep*
>> hits in such a potentially tiny window, would you?
>
> This would be my main objection as well.
Actually the window is not tiny. It gets more than a half window
from the window there is was called. But now I propose to do this
only for xref-find-definitions.
>> Then again, perhaps you would, and the whole point of this patch is to
>> make the UI configurable. If so, I'd make the original UI the default,
>> or at least very very easy to bring back.
>
> Making it configurable on a high level is something I might agree with (and
> indeed, the find-definitions command probably should behave differently
> from the rest), but the way it's done should be transparent and not
> strongly coupled to a particular commands.
>
> Maybe via two different xrefs-show-function values. I'm not sure.
Or maybe better xrefs-display-buffer-alist.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 04 Jan 2019 00:18:02 GMT)
Full text and
rfc822 format available.
Message #89 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> 3. turns the existing arg QUIT of xref-goto-xref into a prefix arg,
>> so a natural key sequence ‘C-u RET’ will quit the window.
>> This is similar to the prefix arg of quit-window.
>
> Kind of similar, but it has a different effect, right? So the logic doesn't
> really translate.
>
> Since we already have the xref-quit-and-goto-xref command, I'm not so sure
> this part is particularly useful.
xref-quit-and-goto-xref has no good keybinding. If you type TAB
in a web browser, do you expect it to close the current window
and open a link in a new window?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 04 Jan 2019 00:38:02 GMT)
Full text and
rfc822 format available.
Message #92 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 04.01.2019 3:14, Juri Linkov wrote:
> xref-quit-and-goto-xref has no good keybinding. If you type TAB
> in a web browser, do you expect it to close the current window
> and open a link in a new window?
You yourself likened it to the Completions buffer...
An the binding is customizable anyway. This is just a criticism of the
default value (which we've agreed upon in a different discussion).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 04 Jan 2019 00:40:02 GMT)
Full text and
rfc822 format available.
Message #95 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 04.01.2019 3:12, Juri Linkov wrote:
>> This would be my main objection as well.
>
> Actually the window is not tiny. It gets more than a half window
> from the window there is was called. But now I propose to do this
> only for xref-find-definitions.
In any case, I'd like to see it in a separate patch (maybe a separate
bug#/discussion as well). Let's not mix the proposal in question, which
more or less retains the current behavior, with something where the main
goal is changing it.
>> Maybe via two different xrefs-show-function values. I'm not sure.
>
> Or maybe better xrefs-display-buffer-alist.
I'd have to see the patch.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 04 Jan 2019 00:43:01 GMT)
Full text and
rfc822 format available.
Message #98 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 04.01.2019 3:07, Juri Linkov wrote:
> It's as easy as moving components of the particular display action
> from the recent patch into a separate function.
Which would make xref.el depend on Emacs 27.1. Just making sure
everybody remembers that.
> Actually the request was about making xref windows more configurable.
Via display-buffer-alist, right?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 04 Jan 2019 06:56:02 GMT)
Full text and
rfc822 format available.
Message #101 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> From: Juri Linkov <juri <at> linkov.net>
> Date: Thu, 03 Jan 2019 23:29:18 +0200
> Cc: 33870 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
>
> when xref-find-definitions pops up the xref window, usually it
> contains just 2 lines taking half of the screen where most space is
> uselessly empty.
We have fit-window-to-buffer for these situations.
> The distinction between internal and public window functions is quite fuzzy.
To my mind, internal functions shouldn't be used outside of the file
that defines them.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 04 Jan 2019 07:42:01 GMT)
Full text and
rfc822 format available.
Message #104 received at 33870 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Fri, Jan 4, 2019, 00:16 Juri Linkov <juri <at> linkov.net wrote:
> I don't know. Can I get back the original behaviour easily? If so,
> > how?
> This should be easy using a new display action.
>
Ok. Make sure to include that in your next patch, and make it the default.
>
> > I ask because the assumption that xref-find-definitions produces a small
> > number of lines is really quite brittle. Generic functions can have
> > many, many methods. In Emacs, cl-print-object has 10 definitions lines,
> > but that could/should easily grow as anyone who devises a new type of
> > object can write a cl-print-object for it. In a Common Lisp system
> > CL:PRINT-OBJECT usually has a ton of methods (and I'm trying to write a
> > CL IDE that uses xref.el)
>
> In my experience 10 lines is an exception. Even with more lines the
> completions-like xref window remain pretty usable.
I'm trying to tell your that your experience which seems limited to xref in
emacs lisp, is not a good measure of how xref-find-definitions is used in
the wild. xref-find-definitions came from something called
slime-find-definitions, and has mostly its UI. SLIME is a CL IDE where
100+ long complicated definitions are the norm, not the exception. And one
day SLIME could decide to use xref.el.
> If it's part of the API, it should really be named
> > window-display-buffer. I'm just making sure it isn't an implementation
> > detail for which Martin reserve the to change at any time.
>
> I agree, window--display-buffer is more public towards other packages
> and could be renamed.
>
Good. Include this in your next patch.
>>> Perhaps you want
> > And can you create one such composite display action that brings exactly
> > the current *xref* behaviour? Or does one such thing already exist?
>
> It's as easy as moving components of the particular display action
> from the recent patch into a separate function.
>
OK.
Actually the request was about making xref windows more configurable.
> I could rename the subject if necessary, or create a separate request
> for more discussion.
>
Don't change subject, create a separate request. Submit a second patch that
makes xref windows configurable and leaves the default UI unchanged. Then
install that patch closing this bug. In the separate discussion we can
continue the discussion of the UI changes you want.
Actually I should have made this separation clearly earlier.
João
>
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 04 Jan 2019 07:50:02 GMT)
Full text and
rfc822 format available.
Message #107 received at 33870 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Fri, Jan 4, 2019, 00:17 Juri Linkov <juri <at> linkov.net wrote:
xref-quit-and-goto-xref has no good keybinding. If you type TAB
> in a web browser, do you expect it to close the current window
> and open a link in a new window?
>
I suggested TAB because in Emacs, tab completes stuff and closes
*completions*. So it's not consistent with web browsers but is consistent
with emacs, as usual. I agree it's not a brilliant binding, but could we
not focus efforts. changing the UI here and solve the actual bug first?
>
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sat, 05 Jan 2019 23:32:02 GMT)
Full text and
rfc822 format available.
Message #110 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> xref-quit-and-goto-xref has no good keybinding. If you type TAB
>> in a web browser, do you expect it to close the current window
>> and open a link in a new window?
>
> I suggested TAB because in Emacs, tab completes stuff and closes
> *completions*. So it's not consistent with web browsers but is consistent
> with emacs, as usual.
Typing TAB in *Completions* moves point to the next completion,
so in xref this corresponds to xref-next-line, and Shift-TAB
moves to the previous completion that corresponds to xref-prev-line.
The closest analogue to "close and do it" in Emacs I see only
dired-find-alternate-file bound to ‘a’:
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
@@ -685,7 +674,9 @@ xref--xref-buffer-mode-map
(define-key map (kbd "p") #'xref-prev-line)
(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 "TAB") #'xref-next-line)
+ (define-key map [backtab] #'xref-prev-line)
+ (define-key map (kbd "a") #'xref-quit-and-goto-xref)
(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)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sat, 05 Jan 2019 23:33:02 GMT)
Full text and
rfc822 format available.
Message #113 received at 33870 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
>> when xref-find-definitions pops up the xref window, usually it
>> contains just 2 lines taking half of the screen where most space is
>> uselessly empty.
>
> We have fit-window-to-buffer for these situations.
>
>> The distinction between internal and public window functions is quite fuzzy.
>
> To my mind, internal functions shouldn't be used outside of the file
> that defines them.
This patch addresses all these concerns.
display-buffer--maybe-at-bottom can be renamed to
display-buffer-maybe-at-bottom without a deprecation alias
because it was added in Emacs 27.
[display-bufffer.patch (text/x-diff, inline)]
diff --git a/lisp/window.el b/lisp/window.el
index 37d82c060c..015933839d 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -7457,6 +7457,21 @@ display-buffer-in-child-frame
(unless (cdr (assq 'inhibit-switch-frame alist))
(window--maybe-raise-frame frame)))))
+(defun display-buffer-maybe-below-selected (buffer alist)
+ ;; This is a copy of `display-buffer-fallback-action'
+ ;; where `display-buffer-use-some-window' is replaced
+ ;; with `display-buffer-below-selected'.
+ (let ((alist (append alist `(,(if temp-buffer-resize-mode
+ '(window-height . resize-temp-buffer-window)
+ '(window-height . fit-window-to-buffer))
+ ,(when temp-buffer-resize-mode
+ '(preserve-size . (nil . t)))))))
+ (or (display-buffer--maybe-same-window buffer alist)
+ (display-buffer-reuse-window buffer alist)
+ (display-buffer--maybe-pop-up-frame buffer alist)
+ (display-buffer-in-previous-window buffer alist)
+ (display-buffer-below-selected buffer alist))))
+
(defun display-buffer-below-selected (buffer alist)
"Try displaying BUFFER in a window below the selected window.
If there is a window below the selected one and that window
@@ -7503,7 +7518,10 @@ display-buffer-below-selected
(window--display-buffer
buffer window 'reuse alist display-buffer-mark-dedicated)))))
-(defun display-buffer--maybe-at-bottom (buffer alist)
+(defun display-buffer-maybe-at-bottom (buffer alist)
+ ;; This is a copy of `display-buffer-fallback-action'
+ ;; where `display-buffer-use-some-window' is replaced
+ ;; with `display-buffer-at-bottom'.
(let ((alist (append alist `(,(if temp-buffer-resize-mode
'(window-height . resize-temp-buffer-window)
'(window-height . fit-window-to-buffer))
@@ -7512,6 +7530,7 @@ display-buffer--maybe-at-bottom
(or (display-buffer--maybe-same-window buffer alist)
(display-buffer-reuse-window buffer alist)
(display-buffer--maybe-pop-up-frame buffer alist)
+ (display-buffer-in-previous-window buffer alist)
(display-buffer-at-bottom buffer alist))))
(defun display-buffer-at-bottom (buffer alist)
diff --git a/lisp/files.el b/lisp/files.el
index 6ccb001e35..0741dbc19e 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -3396,7 +3396,7 @@ hack-local-variables-confirm
;; Display the buffer and read a choice.
(save-window-excursion
- (pop-to-buffer buf '(display-buffer--maybe-at-bottom))
+ (pop-to-buffer buf '(display-buffer-maybe-at-bottom))
(let* ((exit-chars '(?y ?n ?\s ?\C-g ?\C-v))
(prompt (format "Please type %s%s: "
(if offer-save "y, n, or !" "y or n")
@@ -7053,7 +7053,9 @@ save-buffers-kill-emacs
(or (not active)
(with-displayed-buffer-window
(get-buffer-create "*Process List*")
- '(display-buffer--maybe-at-bottom)
+ '(display-buffer-maybe-at-bottom
+ (window-height . fit-window-to-buffer)
+ (preserve-size nil . t))
#'(lambda (window _value)
(with-selected-window window
(unwind-protect
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 5760a2e49d..7dede4e616 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1828,18 +1828,12 @@ minibuffer-completion-help
(display-buffer-mark-dedicated 'soft))
(with-displayed-buffer-window
"*Completions*"
- ;; This is a copy of `display-buffer-fallback-action'
- ;; where `display-buffer-use-some-window' is replaced
- ;; with `display-buffer-at-bottom'.
- `((display-buffer--maybe-same-window
- display-buffer-reuse-window
- display-buffer--maybe-pop-up-frame
- ;; Use `display-buffer-below-selected' for inline completions,
- ;; but not in the minibuffer (e.g. in `eval-expression')
- ;; for which `display-buffer-at-bottom' is used.
- ,(if (eq (selected-window) (minibuffer-window))
- 'display-buffer-at-bottom
- 'display-buffer-below-selected))
+ ;; Use `display-buffer-maybe-below-selected' for inline completions,
+ ;; but not in the minibuffer (e.g. in `eval-expression')
+ ;; for which `display-buffer-maybe-at-bottom' is used.
+ `((,(if (eq (selected-window) (minibuffer-window))
+ 'display-buffer-maybe-at-bottom
+ 'display-buffer-maybe-below-selected))
,(if temp-buffer-resize-mode
'(window-height . resize-temp-buffer-window)
'(window-height . fit-window-to-buffer))
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sat, 05 Jan 2019 23:34:01 GMT)
Full text and
rfc822 format available.
Message #116 received at 33870 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Here's a better patch that relies on display-buffer-in-previous-window:
[xref-previous-window.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
@@ -482,19 +482,9 @@ xref--show-pos-in-buf
(window-live-p xref--original-window)
(or (not (window-dedicated-p xref--original-window))
(eq (window-buffer xref--original-window) buf)))
- `(,(lambda (buf _alist)
- (set-window-buffer xref--original-window buf)
- xref--original-window))))))
- (with-selected-window
- (with-selected-window
- ;; Just before `display-buffer', place ourselves in the
- ;; original window to suggest preserving it. Of course, if
- ;; user has deleted the original window, all bets are off,
- ;; just use the selected one.
- (or (and (window-live-p xref--original-window)
- xref--original-window)
- (selected-window))
- (display-buffer buf action))
+ `((display-buffer-in-previous-window)
+ (previous-window . ,xref--original-window))))))
+ (with-selected-window (display-buffer buf action)
(xref--goto-char pos)
(run-hooks 'xref-after-jump-hook)
(let ((buf (current-buffer)))
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sat, 05 Jan 2019 23:53:02 GMT)
Full text and
rfc822 format available.
Message #119 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 06.01.2019 02:17, Juri Linkov wrote:
>> I suggested TAB because in Emacs, tab completes stuff and closes
>> *completions*. So it's not consistent with web browsers but is consistent
>> with emacs, as usual.
Joao probably meant pressing TAB in the original buffer, which is not
something you can do with xref.
> Typing TAB in *Completions* moves point to the next completion,
> so in xref this corresponds to xref-next-line, and Shift-TAB
> moves to the previous completion that corresponds to xref-prev-line.
Maybe that's an argument for not bringing xref closed to Completions: I
would certainly not like if 'n' and 'p' were replaced with 'TAB' and
'S-TAB'.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sat, 05 Jan 2019 23:56:02 GMT)
Full text and
rfc822 format available.
Message #122 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 06.01.2019 02:27, Juri Linkov wrote:
> (with-selected-window (display-buffer buf action)
So using display-buffer is okay here, after all? I thought pop-to-buffer
is where the magic happens.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 06 Jan 2019 09:05:02 GMT)
Full text and
rfc822 format available.
Message #125 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> display-buffer--maybe-at-bottom can be renamed to
> display-buffer-maybe-at-bottom without a deprecation alias
> because it was added in Emacs 27.
The 'display-buffer--maybe-' functions are macros in disguise invented
by Chong to simplify coding the rest. Unless we can't avoid it, I
would not make them public because then we would have to (1) document
them, (2) explain the semantics of the "maybe" and (3) justify why the
remaining 'display-buffer--maybe-' functions are not public.
Also note that 'display-buffer' resizes a window iff that window is
new or always has shown the buffer to display before.
There's one thing about 'display-buffer-at-bottom' that stupefies me:
Here
(let (split-width-threshold)
(setq window (window--try-to-split-window bottom-window alist)))
we bind ‘split-width-threshold’ so we can split the bottom window into
two side by side windows. I recently found a branch of mine where I
bind 'split-height-threshold' to nil instead and now cannot remember
what we really wanted - split that window horizontally or vertically.
Can you? In either case feel free to change that to what you consider
the more appropriate binding - maybe even binding both.
Thanks, martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 06 Jan 2019 20:56:01 GMT)
Full text and
rfc822 format available.
Message #128 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> The 'display-buffer--maybe-' functions are macros in disguise invented
> by Chong to simplify coding the rest. Unless we can't avoid it, I
> would not make them public because then we would have to (1) document
> them, (2) explain the semantics of the "maybe" and (3) justify why the
> remaining 'display-buffer--maybe-' functions are not public.
_Everything_ in Emacs is (and should be) public.
Why would Emacs users not deserve all of #1, #2,
and #3?
Wouldn't you make that info available to Emacs
developers? How are Emacs users different from
Emacs developers with regard to what info they
deserve to know about, including design and
implementation behavior and their reasons?
Many users might not be interested in digging
into such info, but why hide it? Please consider
instead making such info clear and explicit for
everyone.
An absolute minimum in this regard is comments
in the code.
But Emacs has doc strings, and beyond design
and implementation information we should
document function behavior in doc strings.
It should make no difference how tentative or
temporary or "internal" we might currently
think some function is - its behavior deserves
to be documented.
"Unless we can't avoid it...". We should not
avoid it or anything like it. It should be a
moral imperative, as well as a question of
helpfulness and civility, for Emacs to document
itself to users.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Mon, 07 Jan 2019 00:04:02 GMT)
Full text and
rfc822 format available.
Message #131 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> display-buffer--maybe-at-bottom can be renamed to
>> display-buffer-maybe-at-bottom without a deprecation alias
>> because it was added in Emacs 27.
>
> The 'display-buffer--maybe-' functions are macros in disguise invented
> by Chong to simplify coding the rest. Unless we can't avoid it, I
> would not make them public because then we would have to (1) document
> them,
Yes, they are like macros, and this is what makes them useful for public use.
> (2) explain the semantics of the "maybe" and
The semantics is that they do what the default actions do
plus something specific. Maybe then move the default part
from their body to some other fallback layer? Then just use e.g.
display-buffer-at-bottom, without the -maybe part.
Or maybe use an alist for that, something like
((maybe-try . default-actions))
> (3) justify why the remaining 'display-buffer--maybe-' functions are
> not public.
I don't see any justification.
> Also note that 'display-buffer' resizes a window iff that window is
> new or always has shown the buffer to display before.
>
> There's one thing about 'display-buffer-at-bottom' that stupefies me:
> Here
>
> (let (split-width-threshold)
> (setq window (window--try-to-split-window bottom-window alist)))
>
> we bind ‘split-width-threshold’ so we can split the bottom window into
> two side by side windows. I recently found a branch of mine where I
> bind 'split-height-threshold' to nil instead and now cannot remember
> what we really wanted - split that window horizontally or vertically.
> Can you? In either case feel free to change that to what you consider
> the more appropriate binding - maybe even binding both.
It seems this code has no effect, it's never used. Could you suggest
such window configuration to test that would call it?
There is another problem: in two small vertically split windows
'display-buffer-at-bottom' sometimes displays the buffer in the
upper window.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Mon, 07 Jan 2019 00:04:02 GMT)
Full text and
rfc822 format available.
Message #134 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> The 'display-buffer--maybe-' functions are macros in disguise invented
>> by Chong to simplify coding the rest. Unless we can't avoid it, I
>> would not make them public because then we would have to (1) document
>> them, (2) explain the semantics of the "maybe" and (3) justify why the
>> remaining 'display-buffer--maybe-' functions are not public.
>
> _Everything_ in Emacs is (and should be) public.
I agree, I don't see how display-buffer--maybe-at-bottom
is more internal and not public than e.g. set-window-buffer
and more low-level functions.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Mon, 07 Jan 2019 09:04:01 GMT)
Full text and
rfc822 format available.
Message #137 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> The semantics is that they do what the default actions do
> plus something specific. Maybe then move the default part
> from their body to some other fallback layer? Then just use e.g.
> display-buffer-at-bottom, without the -maybe part.
> Or maybe use an alist for that, something like
>
> ((maybe-try . default-actions))
I would never have written these "maybe" function in the first place.
They need more code lines then they spare. So I have no real opinion.
>> (3) justify why the remaining 'display-buffer--maybe-' functions are
>> not public.
>
> I don't see any justification.
As we know all buffer display action functions are "maybe" functions
so people might ask why some of them include that word explicitly.
>> Also note that 'display-buffer' resizes a window iff that window is
>> new or always has shown the buffer to display before.
>>
>> There's one thing about 'display-buffer-at-bottom' that stupefies me:
>> Here
>>
>> (let (split-width-threshold)
>> (setq window (window--try-to-split-window bottom-window alist)))
>>
>> we bind ‘split-width-threshold’ so we can split the bottom window into
>> two side by side windows. I recently found a branch of mine where I
>> bind 'split-height-threshold' to nil instead and now cannot remember
>> what we really wanted - split that window horizontally or vertically.
>> Can you? In either case feel free to change that to what you consider
>> the more appropriate binding - maybe even binding both.
>
> It seems this code has no effect, it's never used. Could you suggest
> such window configuration to test that would call it?
A single window frame where the buffer is not displayed runs this
part.
> There is another problem: in two small vertically split windows
> 'display-buffer-at-bottom' sometimes displays the buffer in the
> upper window.
From judging the code I'd say this is impossible. But with Emacs
nothing is impossible.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Mon, 07 Jan 2019 14:23:02 GMT)
Full text and
rfc822 format available.
Message #140 received at 33870 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Juri Linkov <juri <at> linkov.net> writes:
> Hi João
>
>> Any simplification to the implementation that keeps the
>> "keep original window intent" behavior across xref
>> intermediate buffers is very welcome.
>
> Thanks for the explanation. Now I understand better the intent in
> xref--show-pos-in-buf. Generally, I'd like to see the “keep original
> window intent” behavior in more places, e.g. in *Occur*, *grep*, etc.
> Based on your explanation, I've been able to write the patch that does
> the following:
Hi again, Juri
After re-reading your patch more closely and giving it some more
testing, I've discovered it breaks an existing use case:
Emacs -Q
C-x 2 ;; split-window-horizontally
C-x 4 . ;; xref-find-definitions-other-window
xref-backend-definitions RET
C-n RET ;; in the resulting *xref* buffer
Expected xref.el to appear in the bottom window which was my original
intent when I said "other window". In the current master this works OK,
in your patch it doesn't.
But don't worry, I've fixed that. In the patch that I attach to this
message, none of the current UI changes is changed, but the xref window
should now be configurable as is the original request of this bug.
I've also renamed window.el's window--display-buffer to
window-display-buffer throughout Emacs (i.e. made it public).
After we merge this, we can continue the discussion about the changing
the xref UI in the other bug you opened, bug#33992
[0001-Make-xref-s-choice-of-windows-easier-to-configure.patch (text/x-patch, inline)]
From 424fe397c86026469a3853e86ca486d549f58100 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora <at> gmail.com>
Date: Mon, 7 Jan 2019 14:16:35 +0000
Subject: [PATCH] Make xref's choice of windows easier to configure
Fixes: bug#33870
Allow for the usual user configuration strategies involving
display-buffer-alist, etc. while maintaining the "keep original window
intent" of xref-find-definitions, xref-find-references, etc.
* lisp/windmove.el (windmove-display-in-direction): Use
window--display-buffer.
* lisp/window.el (display-buffer-in-atom-window):
(window--make-major-side-window):
(display-buffer-in-side-window):
(display-buffer-in-side-window):
(display-buffer-in-side-window):
(display-buffer-use-some-frame):
(display-buffer-same-window):
(display-buffer-reuse-window):
(display-buffer-reuse-mode-window):
(display-buffer-pop-up-frame):
(display-buffer-pop-up-window):
(display-buffer-in-child-frame):
(display-buffer-below-selected):
(display-buffer-below-selected):
(display-buffer-below-selected):
(display-buffer-below-selected):
(display-buffer-at-bottom):
(display-buffer-in-previous-window):
(display-buffer-use-some-window): Use window-display-buffer.
(window-display-buffer): Rename from window--display-buffer.
* lisp/progmodes/xref.el (xref--show-pos-in-buf): Use
window-display-buffer.
---
lisp/progmodes/xref.el | 5 ++---
lisp/windmove.el | 2 +-
lisp/window.el | 46 +++++++++++++++++++++---------------------
3 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 87ce2299c5..20eaa51bef 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -482,9 +482,8 @@ xref--show-pos-in-buf
(window-live-p xref--original-window)
(or (not (window-dedicated-p xref--original-window))
(eq (window-buffer xref--original-window) buf)))
- `(,(lambda (buf _alist)
- (set-window-buffer xref--original-window buf)
- xref--original-window))))))
+ `(,(lambda (buf alist)
+ (window-display-buffer buf xref--original-window 'reuse alist)))))))
(with-selected-window
(with-selected-window
;; Just before `display-buffer', place ourselves in the
diff --git a/lisp/windmove.el b/lisp/windmove.el
index 65270d9bbe..54f0098de7 100644
--- a/lisp/windmove.el
+++ b/lisp/windmove.el
@@ -626,7 +626,7 @@ windmove-display-in-direction
(type 'reuse))
(unless window
(setq window (split-window nil nil dir) type 'window))
- (setq new-window (window--display-buffer buffer window type alist)))))
+ (setq new-window (window-display-buffer buffer window type alist)))))
display-buffer-overriding-action)
(message "[display-%s]" dir)))
diff --git a/lisp/window.el b/lisp/window.el
index 37d82c060c..cf923f7dc6 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -700,7 +700,7 @@ display-buffer-in-atom-window
(set-window-parameter window 'window-atom 'main))
(set-window-parameter new 'window-atom side)
;; Display BUFFER in NEW and return NEW.
- (window--display-buffer
+ (window-display-buffer
buffer new 'window alist display-buffer-mark-dedicated))))
(defun window--atom-check-1 (window)
@@ -985,7 +985,7 @@ window--make-major-side-window
(with-current-buffer buffer
(setq window--sides-shown t))
;; Install BUFFER in new window and return WINDOW.
- (window--display-buffer buffer window 'window alist 'side))))
+ (window-display-buffer buffer window 'window alist 'side))))
(defun display-buffer-in-side-window (buffer alist)
"Display BUFFER in a side window of the selected frame.
@@ -1113,7 +1113,7 @@ display-buffer-in-side-window
;; Reuse `this-window'.
(with-current-buffer buffer
(setq window--sides-shown t))
- (window--display-buffer
+ (window-display-buffer
buffer this-window 'reuse alist dedicated))
(and (or (not max-slots) (< slots max-slots))
(or (and next-window
@@ -1131,7 +1131,7 @@ display-buffer-in-side-window
(set-window-parameter window 'window-slot slot)
(with-current-buffer buffer
(setq window--sides-shown t))
- (window--display-buffer
+ (window-display-buffer
buffer window 'window alist dedicated))
(and best-window
;; Reuse `best-window'.
@@ -1140,7 +1140,7 @@ display-buffer-in-side-window
(set-window-parameter best-window 'window-slot slot)
(with-current-buffer buffer
(setq window--sides-shown t))
- (window--display-buffer
+ (window-display-buffer
buffer best-window 'reuse alist dedicated)))))))))
(defun window-toggle-side-windows (&optional frame)
@@ -6748,7 +6748,7 @@ window--even-window-sizes
(/ (- (window-total-height window) (window-total-height)) 2))
(error nil))))))
-(defun window--display-buffer (buffer window type &optional alist dedicated)
+(defun window-display-buffer (buffer window type &optional alist dedicated)
"Display BUFFER in WINDOW.
TYPE must be one of the symbols `reuse', `window' or `frame' and
is passed unaltered to `display-buffer-record-window'. ALIST is
@@ -7190,7 +7190,7 @@ display-buffer-use-some-frame
frame nil (cdr (assq 'inhibit-same-window alist))))))
(when window
(prog1
- (window--display-buffer
+ (window-display-buffer
buffer window 'reuse alist display-buffer-mark-dedicated)
(unless (cdr (assq 'inhibit-switch-frame alist))
(window--maybe-raise-frame frame))))))
@@ -7204,7 +7204,7 @@ display-buffer-same-window
(unless (or (cdr (assq 'inhibit-same-window alist))
(window-minibuffer-p)
(window-dedicated-p))
- (window--display-buffer buffer (selected-window) 'reuse alist)))
+ (window-display-buffer buffer (selected-window) 'reuse alist)))
(defun display-buffer--maybe-same-window (buffer alist)
"Conditionally display BUFFER in the selected window.
@@ -7252,7 +7252,7 @@ display-buffer-reuse-window
(get-buffer-window-list buffer 'nomini
frames))))))
(when (window-live-p window)
- (prog1 (window--display-buffer buffer window 'reuse alist)
+ (prog1 (window-display-buffer buffer window 'reuse alist)
(unless (cdr (assq 'inhibit-switch-frame alist))
(window--maybe-raise-frame (window-frame window)))))))
@@ -7316,7 +7316,7 @@ display-buffer-reuse-mode-window
derived-mode-same-frame
derived-mode-other-frame))))
(when (window-live-p window)
- (prog1 (window--display-buffer buffer window 'reuse alist)
+ (prog1 (window-display-buffer buffer window 'reuse alist)
(unless (cdr (assq 'inhibit-switch-frame alist))
(window--maybe-raise-frame (window-frame window)))))))))
@@ -7356,7 +7356,7 @@ display-buffer-pop-up-frame
(with-current-buffer buffer
(setq frame (funcall fun)))
(setq window (frame-selected-window frame)))
- (prog1 (window--display-buffer
+ (prog1 (window-display-buffer
buffer window 'frame alist display-buffer-mark-dedicated)
(unless (cdr (assq 'inhibit-switch-frame alist))
(window--maybe-raise-frame frame))))))
@@ -7386,7 +7386,7 @@ display-buffer-pop-up-window
(window--try-to-split-window
(get-lru-window frame t) alist))))
- (prog1 (window--display-buffer
+ (prog1 (window-display-buffer
buffer window 'window alist display-buffer-mark-dedicated)
(unless (cdr (assq 'inhibit-switch-frame alist))
(window--maybe-raise-frame (window-frame window)))))))
@@ -7452,7 +7452,7 @@ display-buffer-in-child-frame
(setq frame (make-frame parameters))
(setq window (frame-selected-window frame))))
- (prog1 (window--display-buffer
+ (prog1 (window-display-buffer
buffer window 'frame alist display-buffer-mark-dedicated)
(unless (cdr (assq 'inhibit-switch-frame alist))
(window--maybe-raise-frame frame)))))
@@ -7476,7 +7476,7 @@ display-buffer-below-selected
(eq buffer (window-buffer window))
(or (not (numberp min-height))
(>= (window-height window) min-height)
- ;; 'window--display-buffer' can resize this window if
+ ;; 'window-display-buffer' can resize this window if
;; and only if it has a 'quit-restore' parameter
;; certifying that it always showed BUFFER before.
(let ((height (window-height window))
@@ -7484,7 +7484,7 @@ display-buffer-below-selected
(and quit-restore
(eq (nth 1 quit-restore) 'window)
(window-resizable-p window (- min-height height)))))
- (window--display-buffer buffer window 'reuse alist))
+ (window-display-buffer buffer window 'reuse alist))
(and (not (frame-parameter nil 'unsplittable))
(or (not (numberp min-height))
(window-sizable-p nil (- min-height)))
@@ -7492,7 +7492,7 @@ display-buffer-below-selected
split-width-threshold)
(setq window (window--try-to-split-window
(selected-window) alist)))
- (window--display-buffer
+ (window-display-buffer
buffer window 'window alist display-buffer-mark-dedicated))
(and (setq window (window-in-direction 'below))
(not (window-dedicated-p window))
@@ -7500,7 +7500,7 @@ display-buffer-below-selected
;; A window that showed another buffer before cannot
;; be resized.
(>= (window-height window) min-height))
- (window--display-buffer
+ (window-display-buffer
buffer window 'reuse alist display-buffer-mark-dedicated)))))
(defun display-buffer--maybe-at-bottom (buffer alist)
@@ -7533,20 +7533,20 @@ display-buffer-at-bottom
(setq bottom-window window))))
nil nil 'nomini)
(or (and bottom-window-shows-buffer
- (window--display-buffer
+ (window-display-buffer
buffer bottom-window 'reuse alist display-buffer-mark-dedicated))
(and (not (frame-parameter nil 'unsplittable))
(let (split-width-threshold)
(setq window (window--try-to-split-window bottom-window alist)))
- (window--display-buffer
+ (window-display-buffer
buffer window 'window alist display-buffer-mark-dedicated))
(and (not (frame-parameter nil 'unsplittable))
(setq window (split-window-no-error (window-main-window)))
- (window--display-buffer
+ (window-display-buffer
buffer window 'window alist display-buffer-mark-dedicated))
(and (setq window bottom-window)
(not (window-dedicated-p window))
- (window--display-buffer
+ (window-display-buffer
buffer window 'reuse alist display-buffer-mark-dedicated)))))
(defun display-buffer-in-previous-window (buffer alist)
@@ -7603,7 +7603,7 @@ display-buffer-in-previous-window
(setq best-window window)))
;; Return best or second best window found.
(when (setq window (or best-window second-best-window))
- (window--display-buffer buffer window 'reuse alist))))
+ (window-display-buffer buffer window 'reuse alist))))
(defun display-buffer-use-some-window (buffer alist)
"Display BUFFER in an existing window.
@@ -7643,7 +7643,7 @@ display-buffer-use-some-window
(error nil)))
(prog1
- (window--display-buffer buffer window 'reuse alist)
+ (window-display-buffer buffer window 'reuse alist)
(window--even-window-sizes window)
(unless (cdr (assq 'inhibit-switch-frame alist))
(window--maybe-raise-frame (window-frame window)))))))
--
2.19.2
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Mon, 07 Jan 2019 22:25:02 GMT)
Full text and
rfc822 format available.
Message #143 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> The semantics is that they do what the default actions do
>> plus something specific. Maybe then move the default part
>> from their body to some other fallback layer? Then just use e.g.
>> display-buffer-at-bottom, without the -maybe part.
>> Or maybe use an alist for that, something like
>>
>> ((maybe-try . default-actions))
>
> I would never have written these "maybe" function in the first place.
> They need more code lines then they spare. So I have no real opinion.
I understand that the word “maybe” indicates it's not guaranteed that
the function will do what it's intended to do. IOW, these functions
are conditional.
Since we can't lightly rename old functions, I have a question only
about functions added in Emacs 27, namely, display-buffer--maybe-at-bottom.
Its current body:
(let ((alist (append alist `(,(if temp-buffer-resize-mode
'(window-height . resize-temp-buffer-window)
'(window-height . fit-window-to-buffer))
,(when temp-buffer-resize-mode
'(preserve-size . (nil . t)))))))
(or (display-buffer--maybe-same-window buffer alist)
(display-buffer-reuse-window buffer alist)
(display-buffer--maybe-pop-up-frame buffer alist)
(display-buffer-in-previous-window buffer alist)
(display-buffer-at-bottom buffer alist)))
I propose to remove this function and replace its parts with
more alists, i.e. this blob
`(,(if temp-buffer-resize-mode
'(window-height . resize-temp-buffer-window)
'(window-height . fit-window-to-buffer))
,(when temp-buffer-resize-mode
'(preserve-size . (nil . t))))
with something shorter like `(fit-to-buffer . t)'
And also to replace a long list of display-buffer-* that is a copy of
`display-buffer-fallback-action' with something shorter like an alist
`(pre-action . display-buffer-fallback-action).
>>> Also note that 'display-buffer' resizes a window iff that window is
>>> new or always has shown the buffer to display before.
>>>
>>> There's one thing about 'display-buffer-at-bottom' that stupefies me:
>>> Here
>>>
>>> (let (split-width-threshold)
>>> (setq window (window--try-to-split-window bottom-window alist)))
>>>
>>> we bind ‘split-width-threshold’ so we can split the bottom window into
>>> two side by side windows. I recently found a branch of mine where I
>>> bind 'split-height-threshold' to nil instead and now cannot remember
>>> what we really wanted - split that window horizontally or vertically.
>>> Can you? In either case feel free to change that to what you consider
>>> the more appropriate binding - maybe even binding both.
>>
>> It seems this code has no effect, it's never used. Could you suggest
>> such window configuration to test that would call it?
>
> A single window frame where the buffer is not displayed runs this
> part.
You are lucky if you can invoke its second branch. I always get only
its third branch in all tried configurations when testing with
completions of `C-x C-f TAB TAB'.
>> There is another problem: in two small vertically split windows
>> 'display-buffer-at-bottom' sometimes displays the buffer in the
>> upper window.
>
> From judging the code I'd say this is impossible. But with Emacs
> nothing is impossible.
After resizing an initial frame to 12 lines, so every vertically split
window gets 6 lines, typing `C-x C-f TAB TAB' displays *Completions* in
the upper window, when a previous window where *Completions* was
previously displayed was moved to the upper window, e.g.
0. emacs -Q
1. resize the frame to 12 lines
2. C-x 2
3. C-x C-f TAB TAB C-g ;; *Completions* were displayed in the bottom window
4. C-x 0
5. C-x 2
6. C-x C-f TAB TAB C-g ;; *Completions* displayed in the upper window that was previous
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Mon, 07 Jan 2019 22:25:03 GMT)
Full text and
rfc822 format available.
Message #146 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> After re-reading your patch more closely and giving it some more
> testing, I've discovered it breaks an existing use case:
>
> Emacs -Q
> C-x 2 ;; split-window-horizontally
> C-x 4 . ;; xref-find-definitions-other-window
> xref-backend-definitions RET
> C-n RET ;; in the resulting *xref* buffer
Of course, it doesn't work if you tried it only with part of my changes.
When I submitted my initial patch, I tested it in all your test cases,
including the above test case that was not broken with my patch.
But you asked to break my patch to several pieces and submit them
separately to different bug reports. No wonder that each of them
doesn't do what the whole patch did.
> Expected xref.el to appear in the bottom window which was my original
> intent when I said "other window".
Then the xref buffer is obscured by another buffer visited in the same
window, and if the user wants to visit more hits from the xref buffer,
this is not easy to do.
> In the current master this works OK, in your patch it doesn't.
My initial patch solved this problem gracefully by creating a new window
for the xref buffer.
> I've also renamed window.el's window--display-buffer to
> window-display-buffer throughout Emacs (i.e. made it public).
You can't rename old functions lightly. This will break the existing
code. This needs many years of deprecation process: in one release
declare the function as obsolete, and in another release delete
old aliases, because there are external packages that rely on this
function name like the `other-frame-window' package from ELPA, etc.
> After we merge this, we can continue the discussion about the changing
> the xref UI in the other bug you opened, bug#33992
Better start with bug#33992 because it supports the above test case,
then we could finish this bug#33870.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Mon, 07 Jan 2019 23:48:01 GMT)
Full text and
rfc822 format available.
Message #149 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 08.01.2019 01:16, Juri Linkov wrote:
>> After re-reading your patch more closely and giving it some more
>> testing, I've discovered it breaks an existing use case:
>>
>> Emacs -Q
>> C-x 2 ;; split-window-horizontally
>> C-x 4 . ;; xref-find-definitions-other-window
>> xref-backend-definitions RET
>> C-n RET ;; in the resulting *xref* buffer
>
> Of course, it doesn't work if you tried it only with part of my changes.
> When I submitted my initial patch, I tested it in all your test cases,
> including the above test case that was not broken with my patch.
>
> But you asked to break my patch to several pieces and submit them
> separately to different bug reports. No wonder that each of them
> doesn't do what the whole patch did.
We asked to split the patch into "retains old behavior while allowing
certain customization" and "changes behavior".
Is that not feasible?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 08 Jan 2019 00:31:02 GMT)
Full text and
rfc822 format available.
Message #152 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>>> Emacs -Q
>>> C-x 2 ;; split-window-horizontally
>>> C-x 4 . ;; xref-find-definitions-other-window
>>> xref-backend-definitions RET
>>> C-n RET ;; in the resulting *xref* buffer
>>
>> Of course, it doesn't work if you tried it only with part of my changes.
>> When I submitted my initial patch, I tested it in all your test cases,
>> including the above test case that was not broken with my patch.
>>
>> But you asked to break my patch to several pieces and submit them
>> separately to different bug reports. No wonder that each of them
>> doesn't do what the whole patch did.
>
> We asked to split the patch into "retains old behavior while allowing
> certain customization" and "changes behavior".
The former implies the latter, this is why they were in the same patch.
Please look again at the subject - it says nothing about avoiding
the need to improve old behavior, quite contrary.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 08 Jan 2019 01:05:01 GMT)
Full text and
rfc822 format available.
Message #155 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 08.01.2019 03:23, Juri Linkov wrote:
>> We asked to split the patch into "retains old behavior while allowing
>> certain customization" and "changes behavior".
>
> The former implies the latter, this is why they were in the same patch.
Nope. The former does not imply a change in default behavior, which you
are proposing to do.
> Please look again at the subject - it says nothing about avoiding
> the need to improve old behavior, quite contrary.
The subject doesn't mention turning Emacs into a coffee maker software
(or *not* doing that), and yet, we're still going to avoid it.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 08 Jan 2019 01:06:02 GMT)
Full text and
rfc822 format available.
Message #158 received at 33870 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
> Of course, it doesn't work if you tried it only with part of my changes.
> When I submitted my initial patch, I tested it in all your test cases,
> including the above test case that was not broken with my patch.
You are correct. I was testing under the assumption that making
xref-goto-xref configurable didn't require the "tiny window" for
xref-find-definitions, which is something you stated that you wanted to
do for other xref.el commands like xref-find-references.
Anyway, can't you add the tiny window xref-find-definitions and the
other UI changes as an addon to *my* patch?
> But you asked to break my patch to several pieces and submit them
> separately to different bug reports. No wonder that each of them
> doesn't do what the whole patch did.
>> Expected xref.el to appear in the bottom window which was my original
>> intent when I said "other window".
> Then the xref buffer is obscured by another buffer visited in the same
> window, and if the user wants to visit more hits from the xref buffer,
> this is not easy to do.
>> In the current master this works OK, in your patch it doesn't.
>
> My initial patch solved this problem gracefully by creating a new window
> for the xref buffer.
You may well call this a problem, but it's not a bug. It's the defined
behaviour, it's like this by design. We are trying to create the
conditions that would enable you, or any other user, to create
alternative ways to present *xref* that have other advantages and
drawbacks.
>> I've also renamed window.el's window--display-buffer to
>> window-display-buffer throughout Emacs (i.e. made it public).
> You can't rename old functions lightly. This will break the existing
> code.
What other code? I renamed the uses in Emacs too: do you mean code
outside Emacs? It shouldn't be using that internal function in the
first place. But if it is, no reason to punish it: we can just provide
a deprecated function alias, which will at most warn its developers of
the imprudency.
No. I can because they were internal functions that no-one was supposed
to have been using in the first place. *That* is precisely why internal
functions are useful, so you can decide when and if to make them public.
You may argue that by making them public *now* we are going to have a
more deprecation problem if we decide to rename them again *in the
future*. I would agree with you there.
>> After we merge this, we can continue the discussion about the changing
>> the xref UI in the other bug you opened, bug#33992
> Better start with bug#33992 because it supports the above test case,
> then we could finish this bug#33870.
No. There is consensus on fixing the bug (xref.el doesn't respect
display-buffer-alist et al) without changing the UI. There is no
consensus on changing the UI.
Also I think some would not really view this is a bug at all.
It would seem Juri that you are trying to shoehorn a behaviour change
into a bugfix. Both things have less chances to go into Emacs that way.
I'm sure that:
* you can implement the UI changes that you want on top of my patch;
* you can make them optional;
* we can then agree on a good default.
Is that not so?
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 08 Jan 2019 09:25:01 GMT)
Full text and
rfc822 format available.
Message #161 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> I understand that the word “maybe” indicates it's not guaranteed that
> the function will do what it's intended to do. IOW, these functions
> are conditional.
Like 'display-buffer'. But we don't call it 'display-buffer-maybe'.
> Since we can't lightly rename old functions, I have a question only
> about functions added in Emacs 27, namely, display-buffer--maybe-at-bottom.
> Its current body:
>
> (let ((alist (append alist `(,(if temp-buffer-resize-mode
> '(window-height . resize-temp-buffer-window)
> '(window-height . fit-window-to-buffer))
> ,(when temp-buffer-resize-mode
> '(preserve-size . (nil . t)))))))
> (or (display-buffer--maybe-same-window buffer alist)
> (display-buffer-reuse-window buffer alist)
> (display-buffer--maybe-pop-up-frame buffer alist)
> (display-buffer-in-previous-window buffer alist)
> (display-buffer-at-bottom buffer alist)))
>
> I propose to remove this function and replace its parts with
> more alists, i.e. this blob
>
> `(,(if temp-buffer-resize-mode
> '(window-height . resize-temp-buffer-window)
> '(window-height . fit-window-to-buffer))
> ,(when temp-buffer-resize-mode
> '(preserve-size . (nil . t))))
>
> with something shorter like `(fit-to-buffer . t)'
Can't we add this via a special value for the 'window-height' alist
entry? Where we explicitly state that it obeys
'temp-buffer-resize-mode' if that is active and the buffer qualifies
as temporary and so on ... Or is that what you mean already?
> And also to replace a long list of display-buffer-* that is a copy of
> `display-buffer-fallback-action' with something shorter like an alist
> `(pre-action . display-buffer-fallback-action).
I'm not sure I understand you. 'display-buffer-fallback-action' is
always tried after everything else failed. Would you want to run it
_before_ something else?
>> A single window frame where the buffer is not displayed runs this
>> part.
>
> You are lucky if you can invoke its second branch. I always get only
> its third branch in all tried configurations when testing with
> completions of `C-x C-f TAB TAB'.
I now always display completions in a child frame so I never run into
practical problems with it.
> After resizing an initial frame to 12 lines, so every vertically split
> window gets 6 lines, typing `C-x C-f TAB TAB' displays *Completions* in
> the upper window, when a previous window where *Completions* was
> previously displayed was moved to the upper window, e.g.
>
> 0. emacs -Q
> 1. resize the frame to 12 lines
> 2. C-x 2
> 3. C-x C-f TAB TAB C-g ;; *Completions* were displayed in the bottom window
> 4. C-x 0
> 5. C-x 2
> 6. C-x C-f TAB TAB C-g ;; *Completions* displayed in the upper window that was previous
Your bag of tricks is fathomless :-) Basically, this means that
'display-buffer-in-previous-window' and 'display-buffer-at-bottom' are
inherently irreconcilable when a window was at bottom once and moved
upwards. We could abuse the existing 'side' action alist entry for
not-atomic, non-side windows in the following sense: If 'side' equals
'bottom', a window is eligible for reuse if and only if it appears on
that side of the frame. To be obeyed by 'display-buffer-reuse-window'
and 'display-buffer-in-previous-window', I presume. WDYT?
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 08 Jan 2019 09:26:02 GMT)
Full text and
rfc822 format available.
Message #164 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> You may argue that by making them public *now* we are going to have a
> more deprecation problem if we decide to rename them again *in the
> future*. I would agree with you there.
The DEDICATED argument of 'window--display-buffer' is a very gross
hack that nobody among us will understand in all its consequences.
Try to guess its semantics from the fairly underdocumented variable
'display-buffer-mark-dedicated' and how it's set by the various buffer
display actions - most of them copying the call scheme from another.
IIUC the idea is that a "reused" window should not be made dedicated
while a new window could be made dedicated. So we could guess the
intention from the TYPE argument - unless it's 'reuse', dedicate the
window if asked for. But we do not implement that consistently.
So before making this function public, we should resolve this calling
convention. Personally, I'd proceed as follows:
(1) Deprecate the variable 'display-buffer-mark-dedicated'.
(2) Remove the DEDICATED argument from this function.
(3) Add a 'dedicated' action alist entry to implement the
functionality.
And we should specify once and for all whether a window can remain or
become dedicated when our function displays another buffer in it.
And another thing: The term "reuse" has two meanings in the context of
buffer display: OT1H "reuse" stands for reusing a window showing one
and the same buffer like in 'display-buffer-reuse-window'. In
'window--display-buffer', if TYPE equals 'reuse' this just means that
an existing window has been reused for showing BUFFER - that window
might have shown another buffer before. This confusion would have to
be resolved as well before going public with this function.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 08 Jan 2019 11:18:02 GMT)
Full text and
rfc822 format available.
Message #167 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On Tue, Jan 8, 2019 at 9:25 AM martin rudalics <rudalics <at> gmx.at> wrote:
> So before making this function public, we should resolve this calling
> convention.
Makes sense. Then, in my view, the logical sequence to fix this bug is
A. First do these changes to window.el and publish a decent
window-display-buffer calling convention.
B. Push a xref.el based on the new function that doesn't change
the xref UI.
C. Discuss the xref.el UI in the other bug.
> Personally, I'd proceed as follows:
>
> (1) Deprecate the variable 'display-buffer-mark-dedicated'.
>
> (2) Remove the DEDICATED argument from this function.
>
> (3) Add a 'dedicated' action alist entry to implement the
> functionality.
When do you think you can do this? Be advised there is indeed some
third-party code already relying on the internal "--" version of this function.
We might be breaking some of that code (otoh it was "asking for it" for
using such an implementation detail).
> And we should [...] and another thing
Do both of these more ambitious refactorings really need to
make it in before we can do B as outlined above? Or can we
do them later in parallel?
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 08 Jan 2019 14:45:01 GMT)
Full text and
rfc822 format available.
Message #170 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> And we should specify once and for all whether a window can remain or
> become dedicated when our function displays another buffer in it.
The intention behind display-buffer-mark-dedicated is to have it set to
`soft` which in turn means that the window will be undedicated whenever
the user explicitly asks to display another buffer in it (typically via
switch-to-buffer).
IOW the result is windows are dedicated as long as they have only ever
displayed a single buffer.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 08 Jan 2019 14:48:02 GMT)
Full text and
rfc822 format available.
Message #173 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> So before making this function public, we should resolve this calling
>> convention.
>
> Makes sense. Then, in my view, the logical sequence to fix this bug is
>
> A. First do these changes to window.el and publish a decent
> window-display-buffer calling convention.
>
> B. Push a xref.el based on the new function that doesn't change
> the xref UI.
>
> C. Discuss the xref.el UI in the other bug.
I can only comment on A and even there I have to leave the judgment to
Stefan Monnier as he's our only expert on window dedication and how
'display-buffer' is supposed to handle it.
But I can offer a preambulatory piece of code we could splice into the
function in order to do away with the DEDICATED argument. Untested!
...
(unless (eq buffer (window-buffer window))
(set-window-dedicated-p window nil)
(set-window-buffer window buffer))
(let ((alist-dedicated (assq 'dedicated alist)))
(cond
(alist-dedicated
(set-window-dedicated-p window (cdr alist-dedicated)))
((and (not (eq type 'reuse)) display-buffer-mark-dedicated)
(set-window-dedicated-p window display-buffer-mark-dedicated))))
(when (memq type '(window frame))
(set-window-prev-buffers window nil))
...
> Do both of these more ambitious refactorings really need to
> make it in before we can do B as outlined above? Or can we
> do them later in parallel?
I recommend to do these before making that function public. I don't
understand B and C sufficiently.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 08 Jan 2019 14:56:01 GMT)
Full text and
rfc822 format available.
Message #176 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On Tue, Jan 8, 2019 at 2:47 PM martin rudalics <rudalics <at> gmx.at> wrote:
> I recommend to do these before making that function public. I don't
> understand B and C sufficiently.
Then we can go ahead and do B and C without A, but we would probably
have to tweak B/C later (which is something we have to do already
for many other uses of `window--display-buffer`).
João
--
João Távora
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 08 Jan 2019 15:05:01 GMT)
Full text and
rfc822 format available.
Message #179 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> The intention behind display-buffer-mark-dedicated is to have it set to
> `soft` which in turn means that the window will be undedicated whenever
> the user explicitly asks to display another buffer in it (typically via
> switch-to-buffer).
Who is supposed to set this and when? Why don't we provide a normal
action alist entry for this?
> IOW the result is windows are dedicated as long as they have only ever
> displayed a single buffer.
You mean one and the same buffer all the time? Please leave a note
somewhere what the precise intended behavior is. So far, designers of
buffer display action functions can only guess.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 08 Jan 2019 16:07:01 GMT)
Full text and
rfc822 format available.
Message #182 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> The intention behind display-buffer-mark-dedicated is to have it set to
>> `soft` which in turn means that the window will be undedicated whenever
>> the user explicitly asks to display another buffer in it (typically via
>> switch-to-buffer).
> Who is supposed to set this and when?
The user in ~/.emacs
> Why don't we provide a normal action alist entry for this?
IIRC it was introduced before the new display-buffer-alist and its
action alists. Also it's a global variable because I needed it to apply
to "normal" buffers (rather than those matched by
special-display-regexps), so it needs to go to
display-buffer-base-action which had no equivalent back then.
>> IOW the result is windows are dedicated as long as they have only ever
>> displayed a single buffer.
> You mean one and the same buffer all the time?
Of course: when set to `soft`, it's marked as `soft-dedicated` when
created, so as soon as some other buffer is displayed, the dedication
is removed.
> Please leave a note somewhere what the precise intended behavior is.
> So far, designers of buffer display action functions can only guess.
I'm not sure what to say there. The intended behavior is just as it is
described: to mark windows as dedicated when they're created.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 08 Jan 2019 17:45:02 GMT)
Full text and
rfc822 format available.
Message #185 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> The user in ~/.emacs
Then this should be an option instead of a plain variable.
>> Why don't we provide a normal action alist entry for this?
>
> IIRC it was introduced before the new display-buffer-alist and its
> action alists. Also it's a global variable because I needed it to apply
> to "normal" buffers (rather than those matched by
> special-display-regexps), so it needs to go to
> display-buffer-base-action which had no equivalent back then.
We have 'display-buffer-alist' for quite some time now. So please
consider making this an action alist entry. That way a user can
decide whether all buffers displayed by 'display-buffer' should be
dedicated or only certain ones and which 'dedicated' value they should
get.
> I'm not sure what to say there. The intended behavior is just as it is
> described: to mark windows as dedicated when they're created.
OK. I think we can get rid of the DEDICATED argument then.
Thanks, martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 08 Jan 2019 20:54:02 GMT)
Full text and
rfc822 format available.
Message #188 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> We have 'display-buffer-alist' for quite some time now.
> So please consider making this an action alist entry.
Yes, it would be much better, but it never seems to reach the top of my
todo list.
> That way a user can decide whether all buffers displayed by
> 'display-buffer' should be dedicated or only certain ones and which
> 'dedicated' value they should get.
Historically, special-display-buffer-alist always caused
the created frames/windows to be dedicated, so
display-buffer-mark-dedicated extends this to those windows created for
other reasons.
I haven't looked in detail, but this seems to make it less trivial to
just add a new action alist parameter: it should default to `t` if we
matched in display-buffer-alist but to nil if we only rely on
display-buffer-base-action?
Also, some (all?) let-bindings of display-buffer-mark-dedicated should
now be unnecessary (because of the features you added so bury-buffer (or
was it quit-window?) automatically deletes the window).
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 09 Jan 2019 00:30:02 GMT)
Full text and
rfc822 format available.
Message #191 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> I propose to remove this function and replace its parts with
>> more alists, i.e. this blob
>>
>> `(,(if temp-buffer-resize-mode
>> '(window-height . resize-temp-buffer-window)
>> '(window-height . fit-window-to-buffer))
>> ,(when temp-buffer-resize-mode
>> '(preserve-size . (nil . t))))
>>
>> with something shorter like `(fit-to-buffer . t)'
>
> Can't we add this via a special value for the 'window-height' alist
> entry? Where we explicitly state that it obeys
> 'temp-buffer-resize-mode' if that is active and the buffer qualifies
> as temporary and so on ... Or is that what you mean already?
I meant to make it shorter in any possible way, so using something like
'(window-height . resize)' seems to achieve this goal.
>> And also to replace a long list of display-buffer-* that is a copy of
>> `display-buffer-fallback-action' with something shorter like an alist
>> `(pre-action . display-buffer-fallback-action).
>
> I'm not sure I understand you. 'display-buffer-fallback-action' is
> always tried after everything else failed. Would you want to run it
> _before_ something else?
Exactly. There is a long list of actions in display-buffer--maybe-at-bottom
before calling the main action 'display-buffer-at-bottom', so it makes sense
to move them somewhere to a common place.
>>> A single window frame where the buffer is not displayed runs this
>>> part.
>>
>> You are lucky if you can invoke its second branch. I always get only
>> its third branch in all tried configurations when testing with
>> completions of `C-x C-f TAB TAB'.
>
> I now always display completions in a child frame so I never run into
> practical problems with it.
Then what problems are possible with binding 'split-width-threshold'
or 'split-height-threshold' to nil?
>> After resizing an initial frame to 12 lines, so every vertically split
>> window gets 6 lines, typing `C-x C-f TAB TAB' displays *Completions* in
>> the upper window, when a previous window where *Completions* was
>> previously displayed was moved to the upper window, e.g.
>>
>> 0. emacs -Q
>> 1. resize the frame to 12 lines
>> 2. C-x 2
>> 3. C-x C-f TAB TAB C-g ;; *Completions* were displayed in the bottom window
>> 4. C-x 0
>> 5. C-x 2
>> 6. C-x C-f TAB TAB C-g ;; *Completions* displayed in the upper window that was previous
>
> Your bag of tricks is fathomless :-) Basically, this means that
> 'display-buffer-in-previous-window' and 'display-buffer-at-bottom' are
> inherently irreconcilable when a window was at bottom once and moved
> upwards. We could abuse the existing 'side' action alist entry for
> not-atomic, non-side windows in the following sense: If 'side' equals
> 'bottom', a window is eligible for reuse if and only if it appears on
> that side of the frame. To be obeyed by 'display-buffer-reuse-window'
> and 'display-buffer-in-previous-window', I presume. WDYT?
This makes sense. Even more, maybe it would be possible to use only
an alist '(side . bottom)' instead of specyfying the action
'display-buffer--maybe-at-bottom'?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 09 Jan 2019 00:30:03 GMT)
Full text and
rfc822 format available.
Message #194 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> Of course, it doesn't work if you tried it only with part of my changes.
>> When I submitted my initial patch, I tested it in all your test cases,
>> including the above test case that was not broken with my patch.
>
> You are correct. I was testing under the assumption that making
> xref-goto-xref configurable didn't require the "tiny window" for
> xref-find-definitions, which is something you stated that you wanted to
> do for other xref.el commands like xref-find-references.
Actually the xref window is not tiny at all if there are more results.
It doesn't takes more space than needed, therefore there is no wasted space.
>> My initial patch solved this problem gracefully by creating a new window
>> for the xref buffer.
>
> You may well call this a problem, but it's not a bug. It's the defined
> behaviour, it's like this by design. We are trying to create the
> conditions that would enable you, or any other user, to create
> alternative ways to present *xref* that have other advantages and
> drawbacks.
Let me reiterate the problem that prompted this report: please imagine
a situation that you have two horizontally split windows with visited files
in each of them, and you happily browse xref definitions in the same window
using M-.
Then suddenly M-. replaces other half of the screen with empty space with
only 2 lines at the top. This is because there is an ambiguity in finding
definitions, and you need to resolve it. Then you start trying to reuse some
empty space it creates and trying to split the xref window. Instead of
this, the split is applied to the original window. As a result, you have
the original window split, and still half of the screen wasted with empty
space. And most of all this mess is caused unexpectedly, i.e. you don't
expect the xref window to break your windows when you type M-.
Do you agree that we should respect user configuration, and use
another window only when the user asks for it? If yes, then a good way
to resolve this problem is to use a part of the original window to display
ambiguous results. This will keep the original window configuration.
Now the question is what to do when the user asks to display
a definition in another window using ‘C-x 4 .’
(xref-find-definitions-other-window). The most natural way is to
immediately take the window pointed out by the user configuration
(the user can configure to display it below/above/left/right etc.)
and display the xref window in that window. Then visiting a definition
still will remain in the same window preferred by the user.
The same logic could also apply to xref-find-definitions-other-frame.
This will allow xref-goto-xref to be configurable.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 09 Jan 2019 09:58:01 GMT)
Full text and
rfc822 format available.
Message #197 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On Wed, Jan 9, 2019 at 12:29 AM Juri Linkov <juri <at> linkov.net> wrote:
> This will allow xref-goto-xref to be configurable.
Of course it will. It will also irrevocably change the current UI,
which might not be what other people want. So I am proposing another
approach that makes xref-goto-xref configurable too, but *doesn't*
change the default UI, i.e. the UI when the user chooses *not* to
configure anything.
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 09 Jan 2019 10:04:02 GMT)
Full text and
rfc822 format available.
Message #200 received at 33870 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
>> We have 'display-buffer-alist' for quite some time now.
>> So please consider making this an action alist entry.
>
> Yes, it would be much better, but it never seems to reach the top of my
> todo list.
>
>> That way a user can decide whether all buffers displayed by
>> 'display-buffer' should be dedicated or only certain ones and which
>> 'dedicated' value they should get.
>
> Historically, special-display-buffer-alist always caused
> the created frames/windows to be dedicated, so
> display-buffer-mark-dedicated extends this to those windows created for
> other reasons.
>
> I haven't looked in detail, but this seems to make it less trivial to
> just add a new action alist parameter: it should default to `t` if we
> matched in display-buffer-alist but to nil if we only rely on
> display-buffer-base-action?
I'm missing you here. An ALIST argument is equally passed to all
buffer display actions regardless of whether they are specifed by
'display-buffer-base-action' or by someone else. It's their choice
whether they want to obey or disregard it. The same currently holds
for 'display-buffer-mark-dedicated'.
> Also, some (all?) let-bindings of display-buffer-mark-dedicated should
I don't see any such bindings in our current code base.
> now be unnecessary (because of the features you added so bury-buffer (or
> was it quit-window?) automatically deletes the window).
This use case of dedicated windows should be no more necessary indeed.
I attach a patch of my proposed changes. After applying that I have
no more objections against renaming 'window--display-buffer' any way
people want.
martin
[window--display-buffer.diff (text/plain, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 09 Jan 2019 10:05:01 GMT)
Full text and
rfc822 format available.
Message #203 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>>> I propose to remove this function and replace its parts with
>>> more alists, i.e. this blob
>>>
>>> `(,(if temp-buffer-resize-mode
>>> '(window-height . resize-temp-buffer-window)
>>> '(window-height . fit-window-to-buffer))
>>> ,(when temp-buffer-resize-mode
>>> '(preserve-size . (nil . t))))
>>>
>>> with something shorter like `(fit-to-buffer . t)'
>>
>> Can't we add this via a special value for the 'window-height' alist
>> entry? Where we explicitly state that it obeys
>> 'temp-buffer-resize-mode' if that is active and the buffer qualifies
>> as temporary and so on ... Or is that what you mean already?
>
> I meant to make it shorter in any possible way, so using something like
> '(window-height . resize)' seems to achieve this goal.
'resize' is too short IMHO. 'resize-to-fit' maybe.
> Exactly. There is a long list of actions in display-buffer--maybe-at-bottom
> before calling the main action 'display-buffer-at-bottom', so it makes sense
> to move them somewhere to a common place.
But running a "fallback" action before the others doesn't sound very
intuitive.
>> I now always display completions in a child frame so I never run into
>> practical problems with it.
>
> Then what problems are possible with binding 'split-width-threshold'
> or 'split-height-threshold' to nil?
I can't tell because I'm not sure what we want here. And if you say
that with your setup this part is never executed, things get even more
obscure. So let's leave everything as it is until someone files a
"real" complaint.
>> We could abuse the existing 'side' action alist entry for
>> not-atomic, non-side windows in the following sense: If 'side' equals
>> 'bottom', a window is eligible for reuse if and only if it appears on
>> that side of the frame. To be obeyed by 'display-buffer-reuse-window'
>> and 'display-buffer-in-previous-window', I presume. WDYT?
>
> This makes sense. Even more, maybe it would be possible to use only
> an alist '(side . bottom)' instead of specyfying the action
> 'display-buffer--maybe-at-bottom'?
We could use the six abbreviations we have ('left', 'top', 'above',
'right', 'bottom' and 'below') to make a window on the respective side
either of the selected window or the frame. Then we would need one
action function say 'display-buffer-beside' and yet another action
alist entry say 'beside' with the values 'selected' (on any side of
the selected window), 'main' (on any side of the main window) and a
window (on which side this would have to be created).
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 09 Jan 2019 13:15:01 GMT)
Full text and
rfc822 format available.
Message #206 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> I haven't looked in detail, but this seems to make it less trivial to
>> just add a new action alist parameter: it should default to `t` if we
>> matched in display-buffer-alist but to nil if we only rely on
>> display-buffer-base-action?
> I'm missing you here. An ALIST argument is equally passed to all
> buffer display actions regardless of whether they are specifed by
> 'display-buffer-base-action' or by someone else. It's their choice
> whether they want to obey or disregard it. The same currently holds
> for 'display-buffer-mark-dedicated'.
Never mind, I was confused.
>> Also, some (all?) let-bindings of display-buffer-mark-dedicated should
> I don't see any such bindings in our current code base.
lisp/dired.el: (display-buffer-mark-dedicated 'soft))
lisp/epa.el: (let ((display-buffer-mark-dedicated 'soft))
lisp/minibuffer.el: (display-buffer-mark-dedicated 'soft))
> I attach a patch of my proposed changes. After applying that I have
> no more objections against renaming 'window--display-buffer' any way
> people want.
LGTM. See some comment/question below.
Stefan
> @@ -958,7 +957,11 @@ window--make-major-side-window
> ;; window and not make a new parent window unless needed.
> (window-combination-resize 'side)
> (window-combination-limit nil)
> - (window (split-window-no-error next-to nil on-side)))
> + (window (split-window-no-error next-to nil on-side))
> + (alist (if (or display-buffer-mark-dedicated
> + (assq 'dedicated alist))
> + alist
> + (cons '(dedicated . side) alist))))
Hmm... the old code used (or display-buffer-mark-dedicated 'side),
so when display-buffer-mark-dedicated is non-nil but (assq 'dedicated
alist) is nil, I think we need to use (cons `(dedicated
. ,display-buffer-mark-dedicated) alist), no?
Or rather:
(alist (if (assq 'dedicated alist)
alist
(cons `(dedicated . ,(or display-buffer-mark-dedicated 'side))
alist))))
WDYT?
BTW, this code reappears a second time in your patch, but I haven't
checked if the same reasoning applies there.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 09 Jan 2019 13:29:02 GMT)
Full text and
rfc822 format available.
Message #209 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Or rather:
>
> (alist (if (assq 'dedicated alist)
> alist
> (cons `(dedicated . ,(or display-buffer-mark-dedicated 'side))
> alist))))
>
> WDYT?
I think you're right. Unless you see any problems I'll use that.
> BTW, this code reappears a second time in your patch, but I haven't
> checked if the same reasoning applies there.
I'll use your code there too.
Thanks, martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 09 Jan 2019 23:43:02 GMT)
Full text and
rfc822 format available.
Message #212 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>>>> I propose to remove this function and replace its parts with
>>>> more alists, i.e. this blob
>>>>
>>>> `(,(if temp-buffer-resize-mode
>>>> '(window-height . resize-temp-buffer-window)
>>>> '(window-height . fit-window-to-buffer))
>>>> ,(when temp-buffer-resize-mode
>>>> '(preserve-size . (nil . t))))
>>>>
>>>> with something shorter like `(fit-to-buffer . t)'
>>>
>>> Can't we add this via a special value for the 'window-height' alist
>>> entry? Where we explicitly state that it obeys
>>> 'temp-buffer-resize-mode' if that is active and the buffer qualifies
>>> as temporary and so on ... Or is that what you mean already?
>>
>> I meant to make it shorter in any possible way, so using something like
>> '(window-height . resize)' seems to achieve this goal.
>
> 'resize' is too short IMHO. 'resize-to-fit' maybe.
Good name.
>> Exactly. There is a long list of actions in display-buffer--maybe-at-bottom
>> before calling the main action 'display-buffer-at-bottom', so it makes sense
>> to move them somewhere to a common place.
>
> But running a "fallback" action before the others doesn't sound very
> intuitive.
Maybe some more suitable name for actions to add between
display-buffer-overriding-action and user-action?
>>> We could abuse the existing 'side' action alist entry for
>>> not-atomic, non-side windows in the following sense: If 'side' equals
>>> 'bottom', a window is eligible for reuse if and only if it appears on
>>> that side of the frame. To be obeyed by 'display-buffer-reuse-window'
>>> and 'display-buffer-in-previous-window', I presume. WDYT?
>>
>> This makes sense. Even more, maybe it would be possible to use only
>> an alist '(side . bottom)' instead of specyfying the action
>> 'display-buffer--maybe-at-bottom'?
Or '(direction . bottom) or shorter '(dir . bottom)
compatible with terminology of window-in-direction
because the word "side" is associated with side windows.
> We could use the six abbreviations we have ('left', 'top', 'above',
> 'right', 'bottom' and 'below') to make a window on the respective side
> either of the selected window or the frame. Then we would need one
> action function say 'display-buffer-beside' and yet another action
> alist entry say 'beside' with the values 'selected' (on any side of
> the selected window), 'main' (on any side of the main window) and a
> window (on which side this would have to be created).
Maybe better 'frame' instead of 'main'?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 10 Jan 2019 10:20:02 GMT)
Full text and
rfc822 format available.
Message #215 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Maybe some more suitable name for actions to add between
> display-buffer-overriding-action and user-action?
Nothing should ever come between 'display-buffer-overriding-action'
and 'user-action'.
>>> This makes sense. Even more, maybe it would be possible to use only
>>> an alist '(side . bottom)' instead of specyfying the action
>>> 'display-buffer--maybe-at-bottom'?
>
> Or '(direction . bottom) or shorter '(dir . bottom)
> compatible with terminology of window-in-direction
> because the word "side" is associated with side windows.
Fine with me.
> Maybe better 'frame' instead of 'main'?
No. 'frame' includes side windows (the ones you cite above) and the
minibuffer window. 'main' (from 'window-main-window') or 'root' are
better (but still not perfect).
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 10 Jan 2019 10:20:02 GMT)
Full text and
rfc822 format available.
Message #218 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> I think you're right. Unless you see any problems I'll use that.
>
> > BTW, this code reappears a second time in your patch, but I haven't
> > checked if the same reasoning applies there.
>
> I'll use your code there too.
Now installed on master, please check.
Anyone interested in renaming 'window--display-buffer' - please go
ahead. I think a 'display-buffer-' prefix should then be appropriate.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 10 Jan 2019 22:01:02 GMT)
Full text and
rfc822 format available.
Message #221 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> Maybe some more suitable name for actions to add between
>> display-buffer-overriding-action and user-action?
>
> Nothing should ever come between 'display-buffer-overriding-action'
> and 'user-action'.
Sorry, rather I wanted to mention 'special-action' because it precedes 'action'
that comes from the display-buffer call (that then could specify just
display-buffer-at-bottom instead of display-buffer--maybe-at-bottom).
>>>> This makes sense. Even more, maybe it would be possible to use only
>>>> an alist '(side . bottom)' instead of specyfying the action
>>>> 'display-buffer--maybe-at-bottom'?
>>
>> Or '(direction . bottom) or shorter '(dir . bottom)
>> compatible with terminology of window-in-direction
>> because the word "side" is associated with side windows.
>
> Fine with me.
>
>> Maybe better 'frame' instead of 'main'?
>
> No. 'frame' includes side windows (the ones you cite above) and the
> minibuffer window. 'main' (from 'window-main-window') or 'root' are
> better (but still not perfect).
Yes, still not perfect. Maybe then use just one alist entry with more
possible values whose names clearly indicate where they are applied like
below - bottom
above - top
left - leftmost
right - rightmost
The first column is relative to the selected window,
the second column is relative to the main window.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 11 Jan 2019 01:19:01 GMT)
Full text and
rfc822 format available.
Message #224 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 09.01.2019 03:20, Juri Linkov wrote:
> Then suddenly M-. replaces other half of the screen with empty space with
> only 2 lines at the top. This is because there is an ambiguity in finding
> definitions, and you need to resolve it. Then you start trying to reuse some
> empty space it creates and trying to split the xref window. Instead of
> this, the split is applied to the original window.
Could you write down the commands to get there? I failed to reproduce this.
> Now the question is what to do when the user asks to display
> a definition in another window using ‘C-x 4 .’
> (xref-find-definitions-other-window). The most natural way is to
> immediately take the window pointed out by the user configuration
> (the user can configure to display it below/above/left/right etc.)
> and display the xref window in that window.
I'm not sure if it's the "most natural" way. "A natural" maybe.
> Then visiting a definition
> still will remain in the same window preferred by the user.
>
> The same logic could also apply to xref-find-definitions-other-frame.
>
> This will allow xref-goto-xref to be configurable.
The current behavior seems to work okay for me. So the meaning of
"configurable" I'm expecting here would allow the user to retain the
current behavior if they want. We can discuss the best default afterwards.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 11 Jan 2019 09:26:02 GMT)
Full text and
rfc822 format available.
Message #227 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Yes, still not perfect. Maybe then use just one alist entry with more
> possible values whose names clearly indicate where they are applied like
>
> below - bottom
> above - top
> left - leftmost
> right - rightmost
>
> The first column is relative to the selected window,
> the second column is relative to the main window.
This will confuse us and users. We should try to unify them somehow
in the sense that terms like below/bottom can be used interchangeably
- as you asked for up/above and down/below in 'window-in-direction'.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 13 Jan 2019 03:05:02 GMT)
Full text and
rfc822 format available.
Message #230 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> Yes, still not perfect. Maybe then use just one alist entry with more
>> possible values whose names clearly indicate where they are applied like
>>
>> below - bottom
>> above - top
>> left - leftmost
>> right - rightmost
>>
>> The first column is relative to the selected window,
>> the second column is relative to the main window.
>
> This will confuse us and users. We should try to unify them somehow
> in the sense that terms like below/bottom can be used interchangeably
> - as you asked for up/above and down/below in 'window-in-direction'.
Actually we already have established naming convention:
display-buffer-below-selected
display-buffer-at-bottom
Removing the common prefix gives us the names of alist entries:
below-selected
at-bottom
Using the same naming convention suggests more names:
above-selected
at-top
...
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 13 Jan 2019 03:05:02 GMT)
Full text and
rfc822 format available.
Message #233 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> Then suddenly M-. replaces other half of the screen with empty space with
>> only 2 lines at the top. This is because there is an ambiguity in finding
>> definitions, and you need to resolve it. Then you start trying to reuse some
>> empty space it creates and trying to split the xref window. Instead of
>> this, the split is applied to the original window.
>
> Could you write down the commands to get there? I failed to reproduce this.
Any command that relies on configuration in display-buffer-alist
or display-buffer-overriding-action such as windmove-display-in-direction.
>> Now the question is what to do when the user asks to display
>> a definition in another window using ‘C-x 4 .’
>> (xref-find-definitions-other-window). The most natural way is to
>> immediately take the window pointed out by the user configuration
>> (the user can configure to display it below/above/left/right etc.)
>> and display the xref window in that window.
>
> I'm not sure if it's the "most natural" way. "A natural" maybe.
At least, the current behavior can't be described as "natural".
For example, if the user prefers using frames and types `C-x 5 .'
the xref buffer is displayed in another WINDOW, not FRAME.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 13 Jan 2019 08:35:01 GMT)
Full text and
rfc822 format available.
Message #236 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Actually we already have established naming convention:
>
> display-buffer-below-selected
> display-buffer-at-bottom
>
> Removing the common prefix gives us the names of alist entries:
>
> below-selected
> at-bottom
>
> Using the same naming convention suggests more names:
>
> above-selected
> at-top
> ...
I know. But I have this idea of providing
- one function that catches all cases
- with one nomenclature for a reference window
- and one nomenclature for the direction wrt the reference window.
So while 'below-selected' fits into this nomenclature, 'at-top'
doesn't.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 13 Jan 2019 11:53:01 GMT)
Full text and
rfc822 format available.
Message #239 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On Sun, Jan 13, 2019 at 3:04 AM Juri Linkov <juri <at> linkov.net> wrote:
> > I'm not sure if it's the "most natural" way. "A natural" maybe.
> At least, the current behavior can't be described as "natural".
> For example, if the user prefers using frames and types `C-x 5 .'
> the xref buffer is displayed in another WINDOW, not FRAME.
As you very well know by now, the "other frame" there refers to the
buffer that eventually displays the cross-reference, which
very often doesn't require the *xref* itself, and _not_ *xref* buffer
itself.
Look, I get it that you dislike the current interface very, very
much and would like to change it. As I have repeatedly asked,
do you understand that a viable path to do that might be:
1. Make the current interface configurable
2. Present a number of configurations for xref to work
with and how to select them.
3. Choose the "most natural" one to be the default
(this is up for debate, sorry, but other people have opinions,
too)
?
Let's just work on number 1 and 2 here *before* we go
to number 3. If you press on starting with 3, you make
me unhappy, because I don't know how I can get back
the current configuration if I later decide I don't like your
system. Arguing that it's "natural" won't do it for me, an UI
is too subjective a thing.
--
João Távora
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 13 Jan 2019 22:00:02 GMT)
Full text and
rfc822 format available.
Message #242 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> I know. But I have this idea of providing
I agree this is a good idea.
> - one function that catches all cases
Like we already have such functions as window-in-direction and
windmove-display-in-direction, the new function could have a similar name
display-buffer-in-direction.
> - with one nomenclature for a reference window
For clarity the alist entry name could include the word "window":
(window . selected)
(window . main)
(window . <window_object>)
But for disambiguation maybe also add some prefix like
direction-window
from-window
etc.
> - and one nomenclature for the direction wrt the reference window.
Ok, like (direction . up) and all aliases.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 13 Jan 2019 22:00:03 GMT)
Full text and
rfc822 format available.
Message #245 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> At least, the current behavior can't be described as "natural".
>> For example, if the user prefers using frames and types `C-x 5 .'
>> the xref buffer is displayed in another WINDOW, not FRAME.
>
> As you very well know by now, the "other frame" there refers to the
> buffer that eventually displays the cross-reference, which
> very often doesn't require the *xref* itself, and _not_ *xref* buffer
> itself.
>
> Look, I get it that you dislike the current interface very, very
> much and would like to change it. As I have repeatedly asked,
> do you understand that a viable path to do that might be:
I don't dislike the current interface, thanks for working on what it
does well. But please don't assume that the current UI is so perfect,
there is no way to make it better. There are some details that cause
minor annoyances (so minor that you won't get many reports for them,
e.g. when the xref pops up in a wrong window, it's easy to fix manually).
> 1. Make the current interface configurable
> 2. Present a number of configurations for xref to work
> with and how to select them.
Of course, it should be configurable, I completely agree,
this is the whole point of this report.
> 3. Choose the "most natural" one to be the default
> (this is up for debate, sorry, but other people have opinions,
> too)
Or course, this should be discussed, this is what I do all the time:
for example, when recently Dmitry filed a complaint about the next-error
framework, I happily cooperated to resolve all disagreements and other
controversies and implemented fixes for what we have discussed to make
the next-error framework more configurable and change previous defaults
based on reached consensus.
So far we have only 3 opinions in this discussion. One way to advance is
to ask what others think about this. Since you have already heard my opinion,
at this moment I have nothing more to say.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 13 Jan 2019 23:07:02 GMT)
Full text and
rfc822 format available.
Message #248 received at 33870 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
> I don't dislike the current interface, thanks for working on what it
> does well. But please don't assume that the current UI is so perfect,
> there is no way to make it better.
Juri, if I did assume that, as you suggest, why would I be aggreeing to
make it configurable?
>> 1. Make the current interface configurable
>> 2. Present a number of configurations for xref to work
>> with and how to select them.
>
> Of course, it should be configurable, I completely agree,
> this is the whole point of this report.
Then how about reviewing my patch? It makes this configurable, and
doesn't change the default configuration.
> at this moment I have nothing more to say.
Then if noone objects I'll push the patch I presented earlier in pa few
days. It makes xref-goto-xref configurable, doesn't change the default
configuration, and closes this bug.
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Mon, 14 Jan 2019 07:58:02 GMT)
Full text and
rfc822 format available.
Message #251 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Like we already have such functions as window-in-direction and
> windmove-display-in-direction, the new function could have a similar name
> display-buffer-in-direction.
OK (unless we find something better).
>> - with one nomenclature for a reference window
>
> For clarity the alist entry name could include the word "window":
>
> (window . selected)
> (window . main)
> (window . <window_object>)
>
> But for disambiguation maybe also add some prefix like
>
> direction-window
> from-window
'from-window' is not bad. Maybe also 'reference-window'. We don't
use such a term in windmove.el. There we just say that "WINDOW is the
window that movement is relative to".
> Ok, like (direction . up) and all aliases.
'direction' should be OK then.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 18 Jan 2019 02:34:02 GMT)
Full text and
rfc822 format available.
Message #254 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 14.01.2019 02:06, João Távora wrote:
> Then if noone objects I'll push the patch I presented earlier in pa few
> days.
Wasn't the part where it renames window--display-buffer still under
debate? I think using the private (current) version of it would be better.
Or we could wait until the related subthread comes to a conclusion.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 18 Jan 2019 02:38:02 GMT)
Full text and
rfc822 format available.
Message #257 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 14.01.2019 00:54, Juri Linkov wrote:
> I don't dislike the current interface, thanks for working on what it
> does well. But please don't assume that the current UI is so perfect,
> there is no way to make it better.
I wouldn't say it's perfect either, it's still kind of idiosyncratic.
Not sure your patch will fix that problem, though, instead of just
swinging it the other way.
We basically have two use cases:
* Jump to this symbol, in this/that window/frame.
windmove-display-in-direction should probably affect where the target
buffer ends up, irrespective of whether we have to pop up an *xref*
buffer to resolve any duplicate matches.
* Show a list of search results. Arguably, in this case
windmove-display-in-direction should affect where the *xref* buffer is
displayed.
Neither of y'all's patches solve this, I believe.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 18 Jan 2019 15:23:02 GMT)
Full text and
rfc822 format available.
Message #260 received at 33870 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 14.01.2019 00:54, Juri Linkov wrote:
>> I don't dislike the current interface, thanks for working on what it
>> does well. But please don't assume that the current UI is so perfect,
>> there is no way to make it better.
>
> I wouldn't say it's perfect either, it's still kind of
> idiosyncratic. Not sure your patch will fix that problem, though,
> instead of just swinging it the other way.
I think this bug's raison d'être is that everybody gets to swing it they
way they like it swung.
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 18 Jan 2019 15:27:02 GMT)
Full text and
rfc822 format available.
Message #263 received at 33870 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 14.01.2019 02:06, João Távora wrote:
>> Then if noone objects I'll push the patch I presented earlier in pa few
>> days.
>
> Wasn't the part where it renames window--display-buffer still under
> debate? I think using the private (current) version of it would be
> better.
I'm OK with that, I'll add a FIXME and then I guess we can change it
when we change the other non-window.el users, like windmove.el.
> Or we could wait until the related subthread comes to a conclusion.
I see the prototype of window--display-buffer has changed recently, and
so has it docstring. I assumed that was it for the cleanup Martin
requested, but indeed I could be wrong, as I didn't follow that
subthread closely (has it died down or I am just not Cc: anymore?).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 18 Jan 2019 15:37:01 GMT)
Full text and
rfc822 format available.
Message #266 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 18.01.2019 18:22, João Távora wrote:
> I think this bug's raison d'être is that everybody gets to swing it they
> way they like it swung.
IIUC, supporting buffer display customization via display-buffer-alist
etc won't be enough.
The question is also when to use it, which buffer to apply it to.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 18 Jan 2019 15:42:01 GMT)
Full text and
rfc822 format available.
Message #269 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On Fri, Jan 18, 2019 at 3:35 PM Dmitry Gutov <dgutov <at> yandex.ru> wrote:
>
> On 18.01.2019 18:22, João Távora wrote:
> > I think this bug's raison d'être is that everybody gets to swing it they
> > way they like it swung.
>
> IIUC, supporting buffer display customization via display-buffer-alist
> etc won't be enough.
Maybe, but then we should focus on opening the right doors so that
it is (or at least check if that is very hard to do) instead of automatically
arguing for a permanent UI change.
> The question is also when to use it, which buffer to apply it to.
Doesn't display-buffer-alist have some mechanism for selecting
which buffer the entry applies to? I'm not an expert in this field.
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 18 Jan 2019 17:35:02 GMT)
Full text and
rfc822 format available.
Message #272 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> I see the prototype of window--display-buffer has changed recently, and
> so has it docstring. I assumed that was it for the cleanup Martin
> requested, but indeed I could be wrong, as I didn't follow that
> subthread closely (has it died down or I am just not Cc: anymore?).
The cleanup has been done. The final message of that subthread says:
Anyone interested in renaming 'window--display-buffer' - please go
ahead. I think a 'display-buffer-' prefix should then be appropriate.
What I meant there is that calling it say 'display-buffer-in-window'
would be appropriate but the final name is up to its clients.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 18 Jan 2019 17:35:02 GMT)
Full text and
rfc822 format available.
Message #275 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Doesn't display-buffer-alist have some mechanism for selecting
> which buffer the entry applies to?
That's indeed the raison d'être of 'display-buffer-alist'.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 18 Jan 2019 17:39:02 GMT)
Full text and
rfc822 format available.
Message #278 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 18.01.2019 18:40, João Távora wrote:
>> The question is also when to use it, which buffer to apply it to.
>
> Doesn't display-buffer-alist have some mechanism for selecting
> which buffer the entry applies to? I'm not an expert in this field.
All right. Maybe just using a function that honors display-buffer-alist
for displaying both the xref buffer and the target buffers will do the
trick.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 18 Jan 2019 22:23:01 GMT)
Full text and
rfc822 format available.
Message #281 received at 33870 <at> debbugs.gnu.org (full text, mbox):
martin rudalics <rudalics <at> gmx.at> writes:
>> I see the prototype of window--display-buffer has changed recently, and
>> so has it docstring. I assumed that was it for the cleanup Martin
>> requested, but indeed I could be wrong, as I didn't follow that
>> subthread closely (has it died down or I am just not Cc: anymore?).
>
> The cleanup has been done. The final message of that subthread says:
>
> Anyone interested in renaming 'window--display-buffer' - please go
> ahead. I think a 'display-buffer-' prefix should then be appropriate.
>
> What I meant there is that calling it say 'display-buffer-in-window'
> would be appropriate but the final name is up to its clients.
Actually seems like a really good name. If I make a function alias,
code in window.el can still use the old name. Do you prefer that, or
should I replace all uses that I can find in Emacs?
Actually2, we should keep the old name. A code search on github.com
reveals that other packages/customizations are using the internal
function, too (which now takes one less arg, but if they break for that,
they were really asking for it).
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sat, 19 Jan 2019 21:55:01 GMT)
Full text and
rfc822 format available.
Message #284 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>>> 1. Make the current interface configurable
>>> 2. Present a number of configurations for xref to work
>>> with and how to select them.
>>
>> Of course, it should be configurable, I completely agree,
>> this is the whole point of this report.
>
> Then how about reviewing my patch? It makes this configurable, and
> doesn't change the default configuration.
>
>> at this moment I have nothing more to say.
>
> Then if noone objects I'll push the patch I presented earlier in pa few
> days. It makes xref-goto-xref configurable, doesn't change the default
> configuration, and closes this bug.
Making xref-goto-xref configurable is really important, this is the
raison d'être of this bug. I wouldn't complain about configurable things.
For example, I'm not complaining that diff-hl-mode uses non-standard
blue color for changes whereas the standard diff-mode color is yellow.
Also I'm not complaining that flymake steals the fringe indicator from
diff-hl-mode, although there is no conflict because diff-hl-mode
uses the background color whereas flymake uses the foreground color,
so they can peacefully coexist together on the fringe.
Also I'm not complaining that often in ruby-mode C-M-f navigates to
unexpected places, however it's easy to fall back to M-f for more
predictable navigation, and so on. But in case of xref-goto-xref
nothing helps. This is why is this bug report.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sat, 19 Jan 2019 21:56:02 GMT)
Full text and
rfc822 format available.
Message #287 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>>> I see the prototype of window--display-buffer has changed recently, and
>>> so has it docstring. I assumed that was it for the cleanup Martin
>>> requested, but indeed I could be wrong, as I didn't follow that
>>> subthread closely (has it died down or I am just not Cc: anymore?).
>>
>> The cleanup has been done. The final message of that subthread says:
>>
>> Anyone interested in renaming 'window--display-buffer' - please go
>> ahead. I think a 'display-buffer-' prefix should then be appropriate.
>>
>> What I meant there is that calling it say 'display-buffer-in-window'
>> would be appropriate but the final name is up to its clients.
>
> Actually seems like a really good name. If I make a function alias,
> code in window.el can still use the old name. Do you prefer that, or
> should I replace all uses that I can find in Emacs?
I agree that 'display-buffer-in-window' is a good name.
> Actually2, we should keep the old name. A code search on github.com
> reveals that other packages/customizations are using the internal
> function, too (which now takes one less arg, but if they break for that,
> they were really asking for it).
Then better to keep the old name with its old signature and
declare it obsolete and replace its body with the call to the
function with the new name and one less arg.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sat, 19 Jan 2019 21:56:02 GMT)
Full text and
rfc822 format available.
Message #290 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> I wouldn't say it's perfect either, it's still kind of idiosyncratic. Not
> sure your patch will fix that problem, though, instead of just swinging it
> the other way.
>
> We basically have two use cases:
>
> * Jump to this symbol, in this/that
> window/frame. windmove-display-in-direction should probably affect where
> the target buffer ends up, irrespective of whether we have to pop up an
> *xref* buffer to resolve any duplicate matches.
>
> * Show a list of search results. Arguably, in this case
> windmove-display-in-direction should affect where the *xref* buffer
> is displayed.
>
> Neither of y'all's patches solve this, I believe.
Why do you think this display action proposed in my previous patch can't solve this?
`((display-buffer-in-previous-window)
(previous-window . ,xref--original-window))
Then there is no need to use a new relative-window alist entry 'from-window'
in display-buffer-in-window because xref--original-window is absolute addressing.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sat, 19 Jan 2019 21:56:03 GMT)
Full text and
rfc822 format available.
Message #293 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> Like we already have such functions as window-in-direction and
>> windmove-display-in-direction, the new function could have a similar name
>> display-buffer-in-direction.
>
> OK (unless we find something better).
Then two new functions will have consistent names:
display-buffer-in-direction
display-buffer-in-window
>>> - with one nomenclature for a reference window
>>
>> For clarity the alist entry name could include the word "window":
>>
>> (window . selected)
>> (window . main)
>> (window . <window_object>)
>>
>> But for disambiguation maybe also add some prefix like
>>
>> direction-window
>> from-window
>
> 'from-window' is not bad. Maybe also 'reference-window'. We don't
> use such a term in windmove.el. There we just say that "WINDOW is the
> window that movement is relative to".
relative-window?
>> Ok, like (direction . up) and all aliases.
>
> 'direction' should be OK then.
Or 'dir' for short.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 20 Jan 2019 00:28:03 GMT)
Full text and
rfc822 format available.
Message #296 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 19.01.2019 23:45, Juri Linkov wrote:
> Why do you think this display action proposed in my previous patch can't solve this?
>
> `((display-buffer-in-previous-window)
> (previous-window . ,xref--original-window))
Sorry. To be honest, I came to this conclusion partly because neither of
you likes the other's patch. And you two seem to care more for different
items in that list.
Both patches are a bit too far from my expertise to review just by reading.
> Then there is no need to use a new relative-window alist entry 'from-window'
> in display-buffer-in-window because xref--original-window is absolute addressing.
This makes sense.
If only that patch were able to keep the current behavior by default.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 20 Jan 2019 00:32:01 GMT)
Full text and
rfc822 format available.
Message #299 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On Sun, Jan 20, 2019 at 12:27 AM Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> Sorry. To be honest, I came to this conclusion partly because neither of
> you likes the other's patch.
No, no. That's not the case, at least for me. It's not a question of liking.
> If only that patch were able to keep the current behavior by default.
Yep. If Juri provides a simpler patch that does this I'm all for it.
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 20 Jan 2019 00:35:02 GMT)
Full text and
rfc822 format available.
Message #302 received at 33870 <at> debbugs.gnu.org (full text, mbox):
This is offtopic, but:
On 19.01.2019 23:31, Juri Linkov wrote:
> For example, I'm not complaining that diff-hl-mode uses non-standard
> blue color for changes whereas the standard diff-mode color is yellow.
The standard diff-mode color for "change" is "none" (see the
diff-changed), so I looked at what most other editors use (blue). I've
only noticed the yellow in diff-refine-changed very recently, and I've
yet to see it in practice. When is this face used exactly?
> Also I'm not complaining that flymake steals the fringe indicator from
> diff-hl-mode, although there is no conflict because diff-hl-mode
> uses the background color whereas flymake uses the foreground color,
> so they can peacefully coexist together on the fringe.
If only fringes supported that kind of merging. BTW, diff-hl-mode uses
the foreground color for the border, but any kind of merging would be an
improvement.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 20 Jan 2019 09:15:02 GMT)
Full text and
rfc822 format available.
Message #305 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Then better to keep the old name with its old signature and
> declare it obsolete and replace its body with the call to the
> function with the new name and one less arg.
Agreed.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 20 Jan 2019 09:15:02 GMT)
Full text and
rfc822 format available.
Message #308 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Then two new functions will have consistent names:
>
> display-buffer-in-direction
> display-buffer-in-window
Just that the first is an action function while the second isn't.
>> 'from-window' is not bad. Maybe also 'reference-window'. We don't
>> use such a term in windmove.el. There we just say that "WINDOW is the
>> window that movement is relative to".
>
> relative-window?
Not really good IMHO.
>>> Ok, like (direction . up) and all aliases.
>>
>> 'direction' should be OK then.
>
> Or 'dir' for short.
dir can be misread as directory.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 20 Jan 2019 20:51:01 GMT)
Full text and
rfc822 format available.
Message #311 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> For example, I'm not complaining that diff-hl-mode uses non-standard
>> blue color for changes whereas the standard diff-mode color is yellow.
>
> The standard diff-mode color for "change" is "none" (see the diff-changed),
> so I looked at what most other editors use (blue). I've only noticed the
> yellow in diff-refine-changed very recently, and I've yet to see it in
> practice. When is this face used exactly?
It's not used anymore but it was yellow some time ago. BTW, I still
can't get used to diff-removed yellow and diff-added blue
diff colors on Wikipedia, and they are not configurable.
>> Also I'm not complaining that flymake steals the fringe indicator from
>> diff-hl-mode, although there is no conflict because diff-hl-mode
>> uses the background color whereas flymake uses the foreground color,
>> so they can peacefully coexist together on the fringe.
>
> If only fringes supported that kind of merging. BTW, diff-hl-mode uses the
> foreground color for the border, but any kind of merging would be
> an improvement.
I thought that since add-face-text-property supports face merging
maybe it would be possible to do the same with fringes,
but I have not investigated how feasible it is.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 20 Jan 2019 20:52:02 GMT)
Full text and
rfc822 format available.
Message #314 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> Then two new functions will have consistent names:
>>
>> display-buffer-in-direction
>> display-buffer-in-window
>
> Just that the first is an action function while the second isn't.
Like display-buffer-in-previous-window is an action function
that takes an alist entry `previous-window', couldn't
display-buffer-in-window be an action function that takes
an alist entry with the window where it display the buffer?
But it seems this is not needed.
>>> 'from-window' is not bad. Maybe also 'reference-window'. We don't
>>> use such a term in windmove.el. There we just say that "WINDOW is the
>>> window that movement is relative to".
>>
>> relative-window?
>
> Not really good IMHO.
Agreed.
>>>> Ok, like (direction . up) and all aliases.
>>>
>>> 'direction' should be OK then.
>>
>> Or 'dir' for short.
>
> dir can be misread as directory.
Agreed.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Mon, 21 Jan 2019 07:54:01 GMT)
Full text and
rfc822 format available.
Message #317 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Like display-buffer-in-previous-window is an action function
> that takes an alist entry `previous-window', couldn't
> display-buffer-in-window be an action function that takes
> an alist entry with the window where it display the buffer?
> But it seems this is not needed.
It could be confusing. 'window--display-buffer' receives two
distinguished arguments - a live WINDOW that it _has_ to use for
displaying BUFFER in and a TYPE needed for correctly processing
'display-buffer-mark-dedicated' and the 'quite-restore' parameter. If
callers fail to set these reliably, further processing might be
broken. And keep in mind that unlike ordinary action functions
'window--display-buffer' never fails. Any failure would be with a
broken caller.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Mon, 21 Jan 2019 21:01:02 GMT)
Full text and
rfc822 format available.
Message #320 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>>> Also I'm not complaining that flymake steals the fringe indicator from
>>> diff-hl-mode, although there is no conflict because diff-hl-mode
>>> uses the background color whereas flymake uses the foreground color,
>>> so they can peacefully coexist together on the fringe.
>>
>> If only fringes supported that kind of merging. BTW, diff-hl-mode uses the
>> foreground color for the border, but any kind of merging would be
>> an improvement.
>
> I thought that since add-face-text-property supports face merging
> maybe it would be possible to do the same with fringes,
> but I have not investigated how feasible it is.
Also the question which one to call on clicking the fringe indicator?
It seems for flymake there is no reasonable action to call on click, so
diff-hl-mode is free to use mouse clicks to show the corresponding diff.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Mon, 21 Jan 2019 21:01:02 GMT)
Full text and
rfc822 format available.
Message #323 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> Like we already have such functions as window-in-direction and
>> windmove-display-in-direction, the new function could have a similar name
>> display-buffer-in-direction.
>
> OK (unless we find something better).
We urgently need this. I discovered another case that will benefit from it.
Currently it can be rewritten as in this patch but I don't like how it requires
a non-trivial alist. Could these be replaced with something simpler?
diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 52c0b5b74d..e90d70359f 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -252,7 +252,11 @@ widget-choose
(define-key map [?\M--] 'negative-argument)
(save-window-excursion
(let ((buf (get-buffer " widget-choose")))
- (fit-window-to-buffer (display-buffer buf))
+ (display-buffer
+ buf
+ '(display-buffer-maybe-below-selected
+ (window-height . fit-window-to-buffer)
+ (preserve-size nil . t)))
(let ((cursor-in-echo-area t)
(arg 1))
(while (not value)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 22 Jan 2019 00:08:01 GMT)
Full text and
rfc822 format available.
Message #326 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 21.01.2019 23:43, Juri Linkov wrote:
> Also the question which one to call on clicking the fringe indicator?
> It seems for flymake there is no reasonable action to call on click, so
> diff-hl-mode is free to use mouse clicks to show the corresponding diff.
*shrug* Somebody is welcome to implement that and submit a patch. IMO,
'C-x v =' is easier.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 24 Jan 2019 09:08:02 GMT)
Full text and
rfc822 format available.
Message #329 received at 33870 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
> We urgently need this. I discovered another case that will benefit from it.
> Currently it can be rewritten as in this patch but I don't like how it requires
> a non-trivial alist. Could these be replaced with something simpler?
I've already forgotten what we really want. Find attached a draft of
what I had in mind in the beginning and fill in the details, if
possible.
martin
[display-buffer-in-direction.el (application/emacs-lisp, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 27 Jan 2019 20:43:02 GMT)
Full text and
rfc822 format available.
Message #332 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> I've already forgotten what we really want. Find attached a draft of
> what I had in mind in the beginning and fill in the details, if
> possible.
Thanks, some suggestions to simplify its usage:
1. avoid using dotted pair notation that often causes problems;
2. instead of asking the user to invent a value to use for the selected window,
allow omitting it in this case, by reversing WIN and DIR, for example:
display-buffer-in-direction (direction DIR WIN)
display-buffer-in-direction (direction up main)
display-buffer-in-direction (direction up) -- by default means
from the selected window
PS: what about 'resize-to-fit'? I guess it's impossible to implement it
as an alist, because currently fit-window-to-buffer/preserve-size usually
are used as an argument of the macro 'with-displayed-buffer-window'.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 27 Jan 2019 20:43:02 GMT)
Full text and
rfc822 format available.
Message #335 received at 33870 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
>> If only that patch were able to keep the current behavior by default.
>
> Yep. If Juri provides a simpler patch that does this I'm all for it.
Ok, here's 100% backward-compatible patch:
[xref.simplify.patch (text/x-diff, inline)]
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 87ce2299c5..9522d7e475 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -474,27 +474,17 @@ xref--show-pos-in-buf
(or (eq xref--original-window-intent 'frame)
pop-up-frames))
(action
- (cond ((memq
- xref--original-window-intent
- '(window frame))
+ (cond ((eq xref--original-window-intent 'frame)
t)
+ ((eq xref--original-window-intent 'window)
+ '(display-buffer-same-window))
((and
(window-live-p xref--original-window)
(or (not (window-dedicated-p xref--original-window))
(eq (window-buffer xref--original-window) buf)))
- `(,(lambda (buf _alist)
- (set-window-buffer xref--original-window buf)
- xref--original-window))))))
- (with-selected-window
- (with-selected-window
- ;; Just before `display-buffer', place ourselves in the
- ;; original window to suggest preserving it. Of course, if
- ;; user has deleted the original window, all bets are off,
- ;; just use the selected one.
- (or (and (window-live-p xref--original-window)
- xref--original-window)
- (selected-window))
- (display-buffer buf action))
+ `((display-buffer-in-previous-window)
+ (previous-window . ,xref--original-window))))))
+ (with-selected-window (display-buffer buf action)
(xref--goto-char pos)
(run-hooks 'xref-after-jump-hook)
(let ((buf (current-buffer)))
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Mon, 28 Jan 2019 18:40:01 GMT)
Full text and
rfc822 format available.
Message #338 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Thanks, some suggestions to simplify its usage:
>
> 1. avoid using dotted pair notation that often causes problems;
You probably mean separate 'direction' and 'window' entries instead of
the (direction . (WIN . DIR)). But we didn't find a good term for
denoting the reference window and the two inherently belong together.
> 2. instead of asking the user to invent a value to use for the selected window,
> allow omitting it in this case, by reversing WIN and DIR, for example:
>
> display-buffer-in-direction (direction DIR WIN)
> display-buffer-in-direction (direction up main)
> display-buffer-in-direction (direction up) -- by default means
> from the selected window
We can do that.
> PS: what about 'resize-to-fit'? I guess it's impossible to implement it
> as an alist, because currently fit-window-to-buffer/preserve-size usually
> are used as an argument of the macro 'with-displayed-buffer-window'.
If we don't implement it already via the window-height/window-width
entries we can add a window-size entry. But I forgot what you wanted
here.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Mon, 28 Jan 2019 20:22:01 GMT)
Full text and
rfc822 format available.
Message #341 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> 1. avoid using dotted pair notation that often causes problems;
>
> You probably mean separate 'direction' and 'window' entries instead of
> the (direction . (WIN . DIR)). But we didn't find a good term for
> denoting the reference window and the two inherently belong together.
I think your idea of combining them is good.
>> 2. instead of asking the user to invent a value to use for the selected window,
>> allow omitting it in this case, by reversing WIN and DIR, for example:
>>
>> display-buffer-in-direction (direction DIR WIN)
>> display-buffer-in-direction (direction up main)
>> display-buffer-in-direction (direction up) -- by default means
>> from the selected window
>
> We can do that.
>
>> PS: what about 'resize-to-fit'? I guess it's impossible to implement it
>> as an alist, because currently fit-window-to-buffer/preserve-size usually
>> are used as an argument of the macro 'with-displayed-buffer-window'.
>
> If we don't implement it already via the window-height/window-width
> entries we can add a window-size entry. But I forgot what you wanted
> here.
Currently it requires too much boilerplate code to do such simple things
as displaying the buffer below/bottom with resizing to fit its height.
Please grep “-at-bottom” and “-below-selected” for the current cases,
they are all ugly: some of them use ‘with-displayed-buffer-window’ with
'((window-height . fit-window-to-buffer)
(preserve-size . (nil . t)))
some are more uglier
,(if temp-buffer-resize-mode
'(window-height . resize-temp-buffer-window)
'(window-height . fit-window-to-buffer))
,(when temp-buffer-resize-mode
'(preserve-size . (nil . t)))
some use the macro ‘with-current-buffer-window’, some use
‘pop-to-buffer’ with ‘display-buffer-below-selected’ action.
Do you think it's possible to generalize all these cases
to use simpler display actions/alists?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 29 Jan 2019 08:51:01 GMT)
Full text and
rfc822 format available.
Message #344 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> You probably mean separate 'direction' and 'window' entries instead of
>> the (direction . (WIN . DIR)). But we didn't find a good term for
>> denoting the reference window and the two inherently belong together.
>
> I think your idea of combining them is good.
So using (direction . (DIR . WIN)) would be OK?
> Currently it requires too much boilerplate code to do such simple things
> as displaying the buffer below/bottom with resizing to fit its height.
> Please grep “-at-bottom” and “-below-selected” for the current cases,
> they are all ugly: some of them use ‘with-displayed-buffer-window’ with
>
> '((window-height . fit-window-to-buffer)
> (preserve-size . (nil . t)))
>
> some are more uglier
>
> ,(if temp-buffer-resize-mode
> '(window-height . resize-temp-buffer-window)
> '(window-height . fit-window-to-buffer))
> ,(when temp-buffer-resize-mode
> '(preserve-size . (nil . t)))
>
> some use the macro ‘with-current-buffer-window’, some use
> ‘pop-to-buffer’ with ‘display-buffer-below-selected’ action.
>
> Do you think it's possible to generalize all these cases
> to use simpler display actions/alists?
I'm afraid that this one
> ,(if temp-buffer-resize-mode
> '(window-height . resize-temp-buffer-window)
is not entirely kosher. 'resize-temp-buffer-window' should be called
only from 'temp-buffer-show-hook' or 'temp-buffer-window-show-hook'.
'display-buffer-at-bottom' can't tell whether BUFFER is temporary or
not. Or am I missing something?
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 29 Jan 2019 21:17:03 GMT)
Full text and
rfc822 format available.
Message #347 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>>> You probably mean separate 'direction' and 'window' entries instead of
>>> the (direction . (WIN . DIR)). But we didn't find a good term for
>>> denoting the reference window and the two inherently belong together.
>>
>> I think your idea of combining them is good.
>
> So using (direction . (DIR . WIN)) would be OK?
And what to do when the future will require adding a third arg?
This is why better to avoid dotted pairs, and use a list like
(direction DIR WIN)
>> Currently it requires too much boilerplate code to do such simple things
>> as displaying the buffer below/bottom with resizing to fit its height.
>> Please grep “-at-bottom” and “-below-selected” for the current cases,
>> they are all ugly: some of them use ‘with-displayed-buffer-window’ with
>>
>> '((window-height . fit-window-to-buffer)
>> (preserve-size . (nil . t)))
>>
>> some are more uglier
>>
>> ,(if temp-buffer-resize-mode
>> '(window-height . resize-temp-buffer-window)
>> '(window-height . fit-window-to-buffer))
>> ,(when temp-buffer-resize-mode
>> '(preserve-size . (nil . t)))
>>
>> some use the macro ‘with-current-buffer-window’, some use
>> ‘pop-to-buffer’ with ‘display-buffer-below-selected’ action.
>>
>> Do you think it's possible to generalize all these cases
>> to use simpler display actions/alists?
>
> I'm afraid that this one
>
>> ,(if temp-buffer-resize-mode
>> '(window-height . resize-temp-buffer-window)
>
> is not entirely kosher. 'resize-temp-buffer-window' should be called
> only from 'temp-buffer-show-hook' or 'temp-buffer-window-show-hook'.
> 'display-buffer-at-bottom' can't tell whether BUFFER is temporary or
> not. Or am I missing something?
I don't know. At least, it seems it's doing its job. What doesn't work
is for example (pop-to-buffer buf '(display-buffer--maybe-at-bottom))
in files.el. Please try to use a file with Local Variables that ask
for permissions interactively, using a single-window wide frame:
instead of showing the Local Variables buffer at the bottom
it splits windows horizontally and shows the Local Variables at the top
of the right-hand window. If the windows were already split horizontally,
then it correctly displays the Local Variables at the bottom.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 29 Jan 2019 21:47:01 GMT)
Full text and
rfc822 format available.
Message #350 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> > So using (direction . (DIR . WIN)) would be OK?
>
> And what to do when the future will require adding a third arg?
> This is why better to avoid dotted pairs, and use a list like
> (direction DIR WIN)
(Caveat: I haven't been following this thread.)
But YES to what Juri wrote there. This is a
(minor) pet peeve of mine. I like dotted pairs
for some things, but this is a standard gotcha.
Sometimes doing this might represent premature
optimization. Sometimes it might come from
focusing too closely on the initial use case
(e.g. the only use case, to start with). But
it happens - to all of us, no doubt.
(You could later hack the definition to also
allow something else in place of (DIR . WIN),
but that kind of thing becomes ugly, especially
if abused more than once.)
Example:
Whoever designed the Lisp representation of a
noncontiguous region trapped us the same way.
By using a dotted pair of scalar values, that
design pretty much precludes adding other info
besides the start and end limits to a region
segment.
The zones of `zones.el' are similar to the
segments of a noncontiguous region, but
instead of just (BEGIN . END) a zone has the
form (LIMIT1 LIMIT2 . EXTRA).
I provided from the outset for the possibility
of including EXTRA stuff, even though at that
time I had no special use in mind for it.
Later I was very thankful I had included it.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 30 Jan 2019 08:10:01 GMT)
Full text and
rfc822 format available.
Message #353 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> And what to do when the future will require adding a third arg?
> This is why better to avoid dotted pairs, and use a list like
>
> (direction DIR WIN)
OK. Let's do that.
> I don't know. At least, it seems it's doing its job. What doesn't work
> is for example (pop-to-buffer buf '(display-buffer--maybe-at-bottom))
> in files.el. Please try to use a file with Local Variables that ask
> for permissions interactively, using a single-window wide frame:
> instead of showing the Local Variables buffer at the bottom
> it splits windows horizontally and shows the Local Variables at the top
> of the right-hand window. If the windows were already split horizontally,
> then it correctly displays the Local Variables at the bottom.
Can you give me a simple recipe? I forgot how to trigger the Local
Variables dialogue.
Thanks, martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 30 Jan 2019 21:18:02 GMT)
Full text and
rfc822 format available.
Message #356 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Example:
>
> Whoever designed the Lisp representation of a
> noncontiguous region trapped us the same way.
> By using a dotted pair of scalar values, that
> design pretty much precludes adding other info
> besides the start and end limits to a region
> segment.
You can override region-extract-function with your own
implementation that can support any shape you want.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 30 Jan 2019 21:19:02 GMT)
Full text and
rfc822 format available.
Message #359 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> What doesn't work is for example (pop-to-buffer buf
>> '(display-buffer--maybe-at-bottom)) in files.el. Please try to use
>> a file with Local Variables that ask for permissions interactively,
>> using a single-window wide frame: instead of showing the Local
>> Variables buffer at the bottom it splits windows horizontally and
>> shows the Local Variables at the top of the right-hand window.
>> If the windows were already split horizontally, then it correctly
>> displays the Local Variables at the bottom.
>
> Can you give me a simple recipe? I forgot how to trigger the Local
> Variables dialogue.
Please just add to a new file:
;; Local Variables:
;; foo: bar
;; End:
Visiting it in a single-window wide frame splits it horizontally
instead of displaying Local Variables at the bottom, as it correctly
does when windows are already horizontally split before visiting the file.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 30 Jan 2019 21:41:02 GMT)
Full text and
rfc822 format available.
Message #362 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> > Example:
> >
> > Whoever designed the Lisp representation of a
> > noncontiguous region trapped us the same way.
> > By using a dotted pair of scalar values, that
> > design pretty much precludes adding other info
> > besides the start and end limits to a region
> > segment.
>
> You can override region-extract-function with your own
> implementation that can support any shape you want.
1. It's not about the region shape - at all.
2. The point is more general. Code that invokes
the function that is the value of the variable
does not, in general, know what function that is.
It can only expect, based on the default value of
the function, that for input `bounds' it gets a
cons (START . END).
IOW, any function used as the variable value
really needs to return bounds of the same form,
if it expects to be used in more than an odd,
narrow context. In general, such a function
will not know or care what context it's used in.
As a result, developers will provide functions
that model the args and return values of the
default function.
The default function was designed with a poor
choice for the value returned by input `bounds'.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 31 Jan 2019 08:33:02 GMT)
Full text and
rfc822 format available.
Message #365 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Please just add to a new file:
>
> ;; Local Variables:
> ;; foo: bar
> ;; End:
>
> Visiting it in a single-window wide frame splits it horizontally
> instead of displaying Local Variables at the bottom, as it correctly
> does when windows are already horizontally split before visiting the file.
It's a silly bug in 'display-buffer-at-bottom'. Please try with
(let ((split-height-threshold 0))
instead of
(let (split-height-threshold)
Thanks, martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 31 Jan 2019 21:23:01 GMT)
Full text and
rfc822 format available.
Message #368 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> Visiting it in a single-window wide frame splits it horizontally
>> instead of displaying Local Variables at the bottom, as it correctly
>> does when windows are already horizontally split before visiting the file.
>
> It's a silly bug in 'display-buffer-at-bottom'. Please try with
>
> (let ((split-height-threshold 0))
>
> instead of
>
> (let (split-height-threshold)
Thanks, this fixed an original problem, but introduced a new problem:
now when windows are already horizontally split, the bottom window
is narrowed to the width of the left window. Before this change,
the width of Local Variables window was the same as the frame width.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Thu, 31 Jan 2019 22:15:02 GMT)
Full text and
rfc822 format available.
Message #371 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On Sun, Jan 27, 2019 at 8:42 PM Juri Linkov <juri <at> linkov.net> wrote:
>
> >> If only that patch were able to keep the current behavior by default.
> >
> > Yep. If Juri provides a simpler patch that does this I'm all for it.
>
> Ok, here's 100% backward-compatible patch:
Thanks, Juri!
I know I'm late on this, but I've been very busy. Please give me
some more days to try this out.
João Távora
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 01 Feb 2019 00:18:01 GMT)
Full text and
rfc822 format available.
Message #374 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On Thu, Jan 31, 2019 at 10:14 PM João Távora <joaotavora <at> gmail.com> wrote:
>
> On Sun, Jan 27, 2019 at 8:42 PM Juri Linkov <juri <at> linkov.net> wrote:
> >
> > >> If only that patch were able to keep the current behavior by default.
> > > Yep. If Juri provides a simpler patch that does this I'm all for it.
> > Ok, here's 100% backward-compatible patch:
>
> Thanks, Juri!
>
> I know I'm late on this, but I've been very busy. Please give me
> some more days to try this out.
OK, so I did find time to test this briefly and I found some
bugs. However, they are reasonably hard to reproduce
consistently. Here's the only bug I can reproduce
consistently:
emacs -Q
C-x 2
C-x 4 . xref-backend-definitions RET
C-n
TAB
Expected the definition to appear in the bottom window, but it
goes to the top window instead (the window I used
xref-find-definitions-other-window). This is wrong and the
current xref.el implementation does not suffer from this bug.
However, in all fairness, the current xref.el implementation
suffers from other bugs that I had never uncovered:
emacs -Q
C-x 2
C-x o
C-x 4 . xref-backend-definitions RET
n
This will open a new frame (!) completely unexpectedly, whereas in
your version, it works quite correctly. It works fine in both versions
if the C-x o is not used.
I did not debug any of the problems.
So which bugs are "worse"? :-) Assuming you can reproduce it
and fix the bug, I would have no more objections, and the patch
does indeed simplify the code.
João
PS: I stress the "assuming you can reproduce it": I could be making
a mistake here: I tested with and without your patch on a recent
Emacs.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 01 Feb 2019 01:40:02 GMT)
Full text and
rfc822 format available.
Message #377 received at 33870 <at> debbugs.gnu.org (full text, mbox):
Joao, thank you for testing.
On 01.02.2019 03:17, João Távora wrote:
> emacs -Q
> C-x 2
> C-x o
> C-x 4 . xref-backend-definitions RET
> n
>
> <...> in
> your version, it works quite correctly.
When I try this with the new patch, it results in a third window being
created (the original window is being split, and the definition is shown
there).
Is this the behavior we want?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 01 Feb 2019 07:31:02 GMT)
Full text and
rfc822 format available.
Message #380 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Fri, 1 Feb 2019 04:39:09 +0300
> Cc: 33870 <at> debbugs.gnu.org
>
> On 01.02.2019 03:17, João Távora wrote:
> > emacs -Q
> > C-x 2
> > C-x o
> > C-x 4 . xref-backend-definitions RET
> > n
> >
> > <...> in
> > your version, it works quite correctly.
>
> When I try this with the new patch, it results in a third window being
> created (the original window is being split, and the definition is shown
> there).
>
> Is this the behavior we want?
No, I don't think so.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 01 Feb 2019 08:20:02 GMT)
Full text and
rfc822 format available.
Message #383 received at 33870 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Fri, Feb 1, 2019, 07:30 Eli Zaretskii <eliz <at> gnu.org> wrote:
> > From: Dmitry Gutov <dgutov <at> yandex.ru>
> > Date: Fri, 1 Feb 2019 04:39:09 +0300
> > Cc: 33870 <at> debbugs.gnu.org
> >
> > On 01.02.2019 03:17, João Távora wrote:
> > > emacs -Q
> > > C-x 2
> > > C-x o
> > > C-x 4 . xref-backend-definitions RET
> > > n
> > >
> > > <...> in
> > > your version, it works quite correctly.
> >
> > When I try this with the new patch, it results in a third window being
> > created (the original window is being split, and the definition is shown
> > there).
> > Is this the behavior we want?
> No, I don't think so.
>
It might not be the behavior you want, but it was the behavior I designed
it to have.
You start with two windows, A and B. You ask to find definitions in another
window from A, because you want to preserve its contents. The symbol you
searched for happened to have multiple definitions so you decide to browse
them from *xref* using bare 'n' and 'p' before settling on the definition
you want. Those "prospects" can't be shown in A because that would break
the original "other-window" contract/intention, and they can't be shown in
B because that's where you're browsing from. They need a new window C which
is not available. When the frame is relatively small (as it is with emacs
-Q), C is created by splitting horizontally, which is kind of akward, but
the decision where to create C changes with larger frames.
For some reason, 26.1 sometimes decides to make another frame for C, but
only if you start from B, i.e you add onde 'C-x o' to the beginning of the
recipe, after splitting. This is a bug in the current xref.el or, more
likely, in window.el's window-splitting heuristics. The bug goes away when
you have larger frames, which explains why I didn't catch it earlier.
Fortunately, the whole point of this bug report opened by Juri is to make
this configurable. Later, we can decide on a better default, something Juri
is also very much in favor of. If you add your voice to his and decide to
change the default, and then give me a way to recover 26.1's behavior
(minus the bug), I won't object (much).
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 01 Feb 2019 09:06:02 GMT)
Full text and
rfc822 format available.
Message #386 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Thanks, this fixed an original problem, but introduced a new problem:
> now when windows are already horizontally split, the bottom window
> is narrowed to the width of the left window. Before this change,
> the width of Local Variables window was the same as the frame width.
The entire
(and (not (frame-parameter nil 'unsplittable))
(let ((split-height-threshold 0))
(setq window (window--try-to-split-window bottom-window alist)))
(window--display-buffer buffer window 'window alist))
is bad design.
Note that I removed this behavior in 'display-buffer-in-direction'.
There I (1) disregard 'split-window-preferred-function' and (2) only
split the argument window which would be the main window when called
from 'display-buffer-at-bottom'.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 01 Feb 2019 18:28:02 GMT)
Full text and
rfc822 format available.
Message #389 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> > > When I try this with the new patch, it results in a third window being
> > > created (the original window is being split, and the definition is shown there).
> > > Is this the behavior we want?
> > No, I don't think so.
>
> It might not be the behavior you want, but it was the behavior I designed it to have.
>
> You start with two windows, A and B. You ask to find definitions in another window
> from A, because you want to preserve its contents. The symbol you searched for
> happened to have multiple definitions so you decide to browse them from *xref*
> using bare 'n' and 'p' before settling on the definition you want. Those
> "prospects" can't be shown in A because that would break the original
> "other-window" contract/intention, and they can't be shown in B because that's
> where you're browsing from. They need a new window C which is not available.
> When the frame is relatively small (as it is with emacs -Q), C is created by
> splitting horizontally, which is kind of akward, but the decision where to create
> C changes with larger frames.
I'm so glad I use separate frames by default. It's one thing
to explicitly choose to replace the content of a particular
window with other content (another buffer). It's quite
another thing to have Emacs doing that left and right behind
your back.
It's not Emacs's fault for just trying to DTRT, of course.
The problem is that TRT is hard to specify in advance. We
can try to require users to specify it in advance, by
configuration, but that runs into the same problem.
This is why `pop-up-windows' and `pop-up-frames' are so
helpful - as a start, at least, to prevent window splitting
and replacing window content left and right.
Of course, if Emacs pops up a new window or frame each time
then you really need a good way to change focus among them.
Yes, I know - mine's a (small) minority opinion.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sat, 02 Feb 2019 00:01:02 GMT)
Full text and
rfc822 format available.
Message #392 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 01.02.2019 11:19, João Távora wrote:
> You start with two windows, A and B. You ask to find definitions in
> another window from A, because you want to preserve its contents. The
> symbol you searched for happened to have multiple definitions so you
> decide to browse them from *xref* using bare 'n' and 'p' before settling
> on the definition you want. Those "prospects" can't be shown in A
> because that would break the original "other-window" contract/intention,
> and they can't be shown in B because that's where you're browsing from.
> They need a new window C which is not available. When the frame is
> relatively small (as it is with emacs -Q), C is created by splitting
> horizontally, which is kind of akward, but the decision where to create
> C changes with larger frames.
OK, makes sense to me. Thanks for the reminder.
So, I haven't been able to repro the bugs you mentioned. That's probably
good.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sat, 02 Feb 2019 00:30:02 GMT)
Full text and
rfc822 format available.
Message #395 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 02.02.2019 03:00, Dmitry Gutov wrote:
> OK, makes sense to me. Thanks for the reminder.
On the same subject, do you remember why we made RET and TAB behave
differently?
Maybe that should be in the docstrings. Or at least comments somewhere.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sat, 02 Feb 2019 09:31:02 GMT)
Full text and
rfc822 format available.
Message #398 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> The entire
>
> (and (not (frame-parameter nil 'unsplittable))
> (let ((split-height-threshold 0))
> (setq window (window--try-to-split-window bottom-window alist)))
> (window--display-buffer buffer window 'window alist))
>
> is bad design.
Removed now from master's 'display-buffer-at-bottom'.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sat, 02 Feb 2019 09:32:01 GMT)
Full text and
rfc822 format available.
Message #401 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> For some reason, 26.1 sometimes decides to make another frame for C, but
> only if you start from B, i.e you add onde 'C-x o' to the beginning of the
> recipe, after splitting. This is a bug in the current xref.el or, more
> likely, in window.el's window-splitting heuristics. The bug goes away when
> you have larger frames, which explains why I didn't catch it earlier.
Note that it is never a bug to not split a window in any of these
cases. Making a new frame is the only way out when (1) existing
windows are too small to get split and (2) windows cannot be reused
because they are dedicated or 'inhibit-same-window' is set. And the
choice to not split a window when it's too small is usually up to the
user and applications just have to respect it.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sat, 02 Feb 2019 21:18:02 GMT)
Full text and
rfc822 format available.
Message #404 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> The entire
>>
>> (and (not (frame-parameter nil 'unsplittable))
>> (let ((split-height-threshold 0))
>> (setq window (window--try-to-split-window bottom-window alist)))
>> (window--display-buffer buffer window 'window alist))
>>
>> is bad design.
>
> Removed now from master's 'display-buffer-at-bottom'.
Thanks, glad to see how just removing code is an improvement.
Maybe we could remove more code to fix other problems ;-)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sat, 02 Feb 2019 21:19:02 GMT)
Full text and
rfc822 format available.
Message #407 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Fortunately, the whole point of this bug report opened by Juri is to make
> this configurable. Later, we can decide on a better default, something Juri
> is also very much in favor of.
With a better default such problems wouldn't happen if definitions were
displayed in a new split window taking space from the original window.
> If you add your voice to his and decide to change the default, and
> then give me a way to recover 26.1's behavior (minus the bug), I won't
> object (much).
How do you prefer to configure old behavior? Using a customizable option?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sat, 02 Feb 2019 22:23:01 GMT)
Full text and
rfc822 format available.
Message #410 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On Sat, Feb 2, 2019 at 9:18 PM Juri Linkov <juri <at> linkov.net> wrote:
>
> > Fortunately, the whole point of this bug report opened by Juri is to make
> > this configurable. Later, we can decide on a better default, something Juri
> > is also very much in favor of.
>
> With a better default such problems wouldn't happen if definitions were
> displayed in a new split window taking space from the original window.
It's only a "problem" in your opinion :-) I was assuming Eli also thinks it's
a bad default, but he hasn't confirmed this yet (and like Dmitry perhaps
we was misunderstanding the reason for the behaviour). And neither has he
said that your proposal is better.
Again, for the millionth time in this discussion, let's first merge your patch
and clean up any bugs before we decide on defaults (perhaps you have
done that already, I haven't checked).
> > If you add your voice to his and decide to change the default, and
> > then give me a way to recover 26.1's behavior (minus the bug), I won't
> > object (much).
>
> How do you prefer to configure old behavior? Using a customizable option?
Something that fits in a single line is good IMO. But normally in Emacs we
keep the defaults for a while and give people unhappy with the defaults
a way to choose their preferred configuration. We change the default when
enough people voice their dissent. So, at least for some time, the
configuration line should go into your configuration, not mine.
--
João Távora
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 03 Feb 2019 03:39:02 GMT)
Full text and
rfc822 format available.
Message #413 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> From: João Távora <joaotavora <at> gmail.com>
> Date: Sat, 2 Feb 2019 22:22:05 +0000
> Cc: Eli Zaretskii <eliz <at> gnu.org>, Dmitry Gutov <dgutov <at> yandex.ru>, 33870 <at> debbugs.gnu.org
>
> On Sat, Feb 2, 2019 at 9:18 PM Juri Linkov <juri <at> linkov.net> wrote:
> >
> > > Fortunately, the whole point of this bug report opened by Juri is to make
> > > this configurable. Later, we can decide on a better default, something Juri
> > > is also very much in favor of.
> >
> > With a better default such problems wouldn't happen if definitions were
> > displayed in a new split window taking space from the original window.
>
> It's only a "problem" in your opinion :-) I was assuming Eli also thinks it's
> a bad default, but he hasn't confirmed this yet (and like Dmitry perhaps
> we was misunderstanding the reason for the behaviour). And neither has he
> said that your proposal is better.
I thought I did express my opinions, but maybe I'm confused wrt the
question(s) you are asking. Care to repeat them, for my benefit?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 03 Feb 2019 12:01:01 GMT)
Full text and
rfc822 format available.
Message #416 received at 33870 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: João Távora <joaotavora <at> gmail.com>
>> Date: Sat, 2 Feb 2019 22:22:05 +0000
>> Cc: Eli Zaretskii <eliz <at> gnu.org>, Dmitry Gutov <dgutov <at> yandex.ru>, 33870 <at> debbugs.gnu.org
>>
>> On Sat, Feb 2, 2019 at 9:18 PM Juri Linkov <juri <at> linkov.net> wrote:
>> >
>> > > Fortunately, the whole point of this bug report opened by Juri is to make
>> > > this configurable. Later, we can decide on a better default, something Juri
>> > > is also very much in favor of.
>> >
>> > With a better default such problems wouldn't happen if definitions were
>> > displayed in a new split window taking space from the original window.
>>
>> It's only a "problem" in your opinion :-) I was assuming Eli also thinks it's
>> a bad default, but he hasn't confirmed this yet (and like Dmitry perhaps
>> we was misunderstanding the reason for the behaviour). And neither has he
>> said that your proposal is better.
>
> I thought I did express my opinions, but maybe I'm confused wrt the
> question(s) you are asking. Care to repeat them, for my benefit?
Eli, if that helps clear the confusion up front, this it has to do with
this last exchange, not with your email of 2018-12-26, 15:36, where you
said you would _not_ like to change the current default behaviour.
If that doesn't help, and neither does reading the exchange, I'll try
a summary of the most recent events.
- Juri provided a purportedly 100% backward compatible patch that keeps
current UI and allows xref.el windows to be configured by users.
- I tested the patch with many cases, including a corner use case.
- Dmitry expressed doubts about the behaviour of that case
- You expressed the same doubts as Dmitry's
- I explained that it is the defined behaviour
- Dmitry accepted the explanation
- Drew wrote something that I didn't read/understand fully (sorry Drew!)
- Juri took your doubts as evidence of a problem in the current UI.
- I explained again that it is the defined and default UI, but changing
is on the table, especially if you, unlike Dmitry, don't accept the
explanation I gave for the corner case that you said isn't correct.
So Eli, maintainer of Emacs, the Editor:
1. Should xref.el be made configurable so that multiple UI's are
available to users, keeping the current default in in Emacs 26.1? We
have at least two candidate patches that do this.
2. Should the default UI in Emacs 26.1 be changed?
As has been done at least 10 times in this thread, I propose to do the
former first and then discuss the latter. I can also say that I am a
bit tired of this: the thread has got so entangled that I'm now spending
time re-explaining these relatively simple premises.
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 03 Feb 2019 17:10:02 GMT)
Full text and
rfc822 format available.
Message #419 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> From: João Távora <joaotavora <at> gmail.com>
> Cc: juri <at> linkov.net, dgutov <at> yandex.ru, 33870 <at> debbugs.gnu.org
> Date: Sun, 03 Feb 2019 12:00:42 +0000
>
> So Eli, maintainer of Emacs, the Editor:
>
> 1. Should xref.el be made configurable so that multiple UI's are
> available to users, keeping the current default in in Emacs 26.1? We
> have at least two candidate patches that do this.
>
> 2. Should the default UI in Emacs 26.1 be changed?
>
> As has been done at least 10 times in this thread, I propose to do the
> former first and then discuss the latter. I can also say that I am a
> bit tired of this: the thread has got so entangled that I'm now spending
> time re-explaining these relatively simple premises.
I don't think I get this: you have asked for my opinion. Now you seem
to say you are tired of discussing this and don't want to spend any
more time on it? Fine with me, then I won't take any more of your
time by trying to chime in. Do whatever you would have done without
hearing my opinions. It will certainly be faster, and put less
demands on your time (and on mine).
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 03 Feb 2019 20:23:02 GMT)
Full text and
rfc822 format available.
Message #422 received at 33870 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> As has been done at least 10 times in this thread, I propose to do the
>> former first and then discuss the latter. I can also say that I am a
>> bit tired of this: the thread has got so entangled that I'm now spending
>> time re-explaining these relatively simple premises.
>
> I don't think I get this: you have asked for my opinion. Now you seem
> to say you are tired of discussing this
Not really. But it seems in trying to untangle this thread, I've only
tangled it more...
Let me try one last time. 2 days ago, you decided to chime in very
tersely: "No, I don't think so"
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=33870#380
, presumably saying that the default behaviour in Emacs 26.1 doesn't
make sense for a specific edge case that I had been testing. Because
you may have been misunderstanding I replied with a detailed explanation
of the reasoning for that particular behaviour in:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=33870#383
, which you didn't reply to, but Dmitry accepted as sufficient
validation for the current behaviour. Meanwhile, Juri used this
opportunity to insist that the current behaviour is sub-optimal.
I didn't state (much) disagreement: IMO changing the default can be on
the table, but I said perhaps we should wait for your confirmation that
the "No, I don't think so" really means that you think the 26.1 default
_should_ be changed.
Somewhere along the line we started miscommunicating that someone was
asking the other for input, but for me this is very simple: let's
install Juri's latest patch, which allows for configuring different
behaviors and _then_ discuss which one, if any, of the set of new
possibilities could become the new default.
Also sorry if I sounded rude: I wasn't attributing the reasons I am
tired of this thread to you.
Thanks,
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 03 Feb 2019 20:50:01 GMT)
Full text and
rfc822 format available.
Message #425 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> And what to do when the future will require adding a third arg?
>> This is why better to avoid dotted pairs, and use a list like
>>
>> (direction DIR WIN)
>
> OK. Let's do that.
I'm trying to use your implementation of display-buffer-in-direction,
but with this patch:
diff --git a/lisp/files.el b/lisp/files.el
index 9948bd4a03..dac75fdb78 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -3396,7 +3396,7 @@ hack-local-variables-confirm
;; Display the buffer and read a choice.
(save-window-excursion
- (pop-to-buffer buf '(display-buffer--maybe-at-bottom))
+ (pop-to-buffer buf '(display-buffer-in-direction (direction bottom main)))
(let* ((exit-chars '(?y ?n ?\s ?\C-g ?\C-v))
(prompt (format "Please type %s%s: "
(if offer-save "y, n, or !" "y or n")
while visiting a file with Local Variables it fails with:
Debugger entered--Lisp error: (error "Cannot share edge from within live window #<window...")
signal(error ("Cannot share edge from within live window #<window..."))
error("Cannot share edge from within live window %s" #<window 6 on etc>)
windows-sharing-edge(#<window 6 on etc> below t)
display-buffer-in-direction(#<buffer *Local Variables*> ((direction bottom main)))
display-buffer(#<buffer *Local Variables*> (display-buffer-in-direction (direction bottom main)))
pop-to-buffer(#<buffer *Local Variables*> (display-buffer-in-direction (direction bottom main)))
Please help.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 03 Feb 2019 20:50:02 GMT)
Full text and
rfc822 format available.
Message #428 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> It's only a "problem" in your opinion :-) I was assuming Eli also thinks it's
>> a bad default, but he hasn't confirmed this yet (and like Dmitry perhaps
>> we was misunderstanding the reason for the behaviour). And neither has he
>> said that your proposal is better.
>
> I thought I did express my opinions, but maybe I'm confused wrt the
> question(s) you are asking. Care to repeat them, for my benefit?
Let me summarize my point of view of the current situation:
* Old behavior:
M-. pops up the *xref* buffer in an adjacent window
RET visits references in the original window
TAB visits references in the original window
n visits references in the original window
C-x 4 . pops up the *xref* buffer in an adjacent window
RET visits references in the same window where *xref* buffer was
TAB depending on window configuration visits references either
in the same window where *xref* buffer was or in the original window
n splits the original window and visits references in a tiny window,
sometimes opens a new frame
C-x 5 . pops up the *xref* buffer in an adjacent window
RET visits references in a new frame
TAB visits references in a new frame
n visits references in a new frame
Problems: the case of 'C-x 4 .' is a mess. Other cases are consistent,
but take screen space from an adjacent window. The proposed behavior
solves all these problems:
* Proposed new behavior
M-. pops up the *xref* buffer in a tiny window below the original window
RET visits references in the original window
TAB visits references in the original window
n visits references in the original window
C-x 4 . pops up the *xref* buffer in a tiny window below the original window
RET visits references in a new window
TAB visits references in a new window
n visits references in a new window
C-x 5 . pops up the *xref* buffer in a tiny window below the original window
RET visits references in a new frame
TAB visits references in a new frame
n visits references in a new frame
However, the problem is that I don't understand the complicated logic
of the 'C-x 4 .' case of the old behavior, so I can't completely
support all its intricacies after code simplification.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 03 Feb 2019 21:03:01 GMT)
Full text and
rfc822 format available.
Message #431 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> - Drew wrote something that I didn't read/understand
> fully (sorry Drew!)
I added 4 side points related to points that came
up in the thread. Possibly you didn't read or
understand one or more of them. Enjoy.
1. "Everything in Emacs is (and should be) public."
All `display-buffer*' functions deserve doc
strings or at least developer-oriented comments.
2. Don't limit future use by having something like
(direction . (DIR . WIN)). Instead, as Juri
suggested, use (direction DIR WIN), so you can
easily later have (direction DIR WIN FOO), etc.
3. The mistake of #2 was made, e.g., when defining
"noncontiguous region" segments: (BEG . END).
A better design is (BEG END) or (BEG END . EXTRA),
where EXTRA is from the outset undefined (any
baggage).
4. All-encompassing DWIM for window selection and
splitting is asking too much of Emacs. Guessing
the intention of a user or code in all contexts
is bound to lose some of the time, frustrating
users. The code gets more and more complex,
which doesn't help users guess what Emacs is
guessing. ;-) I'm glad I instead use separate
frames, by default.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 03 Feb 2019 21:09:02 GMT)
Full text and
rfc822 format available.
Message #434 received at 33870 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
>>> It's only a "problem" in your opinion :-) I was assuming Eli also thinks it's
>>> a bad default, but he hasn't confirmed this yet (and like Dmitry perhaps
>>> we was misunderstanding the reason for the behaviour). And neither has he
>>> said that your proposal is better.
>>
>> I thought I did express my opinions, but maybe I'm confused wrt the
>> question(s) you are asking. Care to repeat them, for my benefit?
>
> Let me summarize my point of view of the current situation:
>
> * Old behavior:
>
> M-. pops up the *xref* buffer in an adjacent window
> RET visits references in the original window
> TAB visits references in the original window
> n visits references in the original window
>
> C-x 4 . pops up the *xref* buffer in an adjacent window
> RET visits references in the same window where *xref* buffer was
> TAB depending on window configuration visits references either
> in the same window where *xref* buffer was or in the original window
> n splits the original window and visits references in a tiny window,
> sometimes opens a new frame
Unfortunately, you're trying again to kick up a dust cloud around the
matter. You description is only partially true for the two-window case.
n, for example, doesn't always split the window, only when it needs to
create a new window. And the "new frame" exception is an _obscure bug_,
and even then it's one that your patch and my patch already solve, so
it's a completely moot point.
Let's use your 100%-backward-compatible patch, (since it is the simpler
of the two). For the millionth time: _after_ we get _some_ patch
installed, I invite you to open a new customization option (or just a
simple variable) that lets me toggle on and off between the current
behaviour and the behaviour that you think is superior. Then we can all
try it for a while. _Why_ is this so hard?
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Mon, 04 Feb 2019 07:31:02 GMT)
Full text and
rfc822 format available.
Message #437 received at 33870 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
> I'm trying to use your implementation of display-buffer-in-direction,
> but with this patch:
>
> diff --git a/lisp/files.el b/lisp/files.el
> index 9948bd4a03..dac75fdb78 100644
> --- a/lisp/files.el
> +++ b/lisp/files.el
> @@ -3396,7 +3396,7 @@ hack-local-variables-confirm
>
> ;; Display the buffer and read a choice.
> (save-window-excursion
> - (pop-to-buffer buf '(display-buffer--maybe-at-bottom))
> + (pop-to-buffer buf '(display-buffer-in-direction (direction bottom main)))
> (let* ((exit-chars '(?y ?n ?\s ?\C-g ?\C-v))
> (prompt (format "Please type %s%s: "
> (if offer-save "y, n, or !" "y or n")
>
> while visiting a file with Local Variables it fails with:
>
> Debugger entered--Lisp error: (error "Cannot share edge from within live window #<window...")
> signal(error ("Cannot share edge from within live window #<window..."))
> error("Cannot share edge from within live window %s" #<window 6 on etc>)
> windows-sharing-edge(#<window 6 on etc> below t)
> display-buffer-in-direction(#<buffer *Local Variables*> ((direction bottom main)))
> display-buffer(#<buffer *Local Variables*> (display-buffer-in-direction (direction bottom main)))
> pop-to-buffer(#<buffer *Local Variables*> (display-buffer-in-direction (direction bottom main)))
I attach a version which should handle this now. I still can't get
used to a positional specification of direction and reference window
so ALIST now has to contain separate 'direction' and 'window' entries
as in
(pop-to-buffer buf '(display-buffer-in-direction (direction . bottom) (window . main)))
where the 'direction' entry is mandatory.
martin
[display-buffer-in-direction.el (application/emacs-lisp, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Mon, 04 Feb 2019 21:56:01 GMT)
Full text and
rfc822 format available.
Message #440 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> Let me summarize my point of view of the current situation:
>>
>> * Old behavior:
>>
>> M-. pops up the *xref* buffer in an adjacent window
>> RET visits references in the original window
>> TAB visits references in the original window
>> n visits references in the original window
>>
>> C-x 4 . pops up the *xref* buffer in an adjacent window
>> RET visits references in the same window where *xref* buffer was
>> TAB depending on window configuration visits references either
>> in the same window where *xref* buffer was or in the original window
>> n splits the original window and visits references in a tiny window,
>> sometimes opens a new frame
>
> Unfortunately, you're trying again to kick up a dust cloud around the
> matter. You description is only partially true for the two-window case.
Yes, it's only partially true, I admitted this by saying that
I don't understand its complicated logic.
> Let's use your 100%-backward-compatible patch, (since it is the simpler
> of the two). For the millionth time: _after_ we get _some_ patch
> installed, I invite you to open a new customization option (or just a
> simple variable) that lets me toggle on and off between the current
> behaviour and the behaviour that you think is superior.
I don't think that the behaviour I proposed is superior.
In particular, my previous proposal behaves poorly
in combination with windowmove, e.g. typing
<S-M-up> M-. to display the reference in the upper window,
when there are ambiguities it displays the xref buffer
in the upper window, and then typing RET visits the reference
in a wrong window (in the original window).
I hoped to have more discussion to find the best solution.
> Then we can all try it for a while. _Why_ is this so hard?
It's hard to support all details of old behavior in all possible interactions.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Mon, 04 Feb 2019 21:56:02 GMT)
Full text and
rfc822 format available.
Message #443 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> I attach a version which should handle this now. I still can't get
> used to a positional specification of direction and reference window
> so ALIST now has to contain separate 'direction' and 'window' entries
> as in
>
> (pop-to-buffer buf '(display-buffer-in-direction (direction . bottom) (window . main)))
Thanks, it works without errors, but for some reason it doesn't fit
the buffer into the window, i.e. the old version
(pop-to-buffer buf '(display-buffer--maybe-at-bottom))
correctly resized the window, but the new version above doesn't.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Mon, 04 Feb 2019 23:25:01 GMT)
Full text and
rfc822 format available.
Message #446 received at 33870 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
>>> Let me summarize my point of view of the current situation:
>> Unfortunately, you're trying again to kick up a dust cloud around the
>> matter. You description is only partially true for the two-window case.
> Yes, it's only partially true, I admitted this by saying that
> I don't understand its complicated logic.
It's complicated in your opinion, and I may agree with that, but it's no
more complicated then, say, a situation where you have two windows, but
one of them is dedicated, and you do, say, describe-symbol, or something
else that pops a window. Which is just what happens in xref.el: the
starting window where you invoked C-x 4 . behaves like it it is
temporarily dedicated to its buffer.
>> Let's use your 100%-backward-compatible patch, (since it is the simpler
>> of the two). For the millionth time: _after_ we get _some_ patch
>> installed, I invite you to open a new customization option (or just a
>> simple variable) that lets me toggle on and off between the current
>> behaviour and the behaviour that you think is superior.
>
> I don't think that the behaviour I proposed is superior.
"Superior" in UI is always relative. I think the only way to know if to
test it for a while. But to do that we have to close this bug first.
> I hoped to have more discussion to find the best solution.
I didn't say we _can't_ have more discussion. But let's first wrap up
this easy win by installing the patch that makes this configurable.
I'll test your last patch in a fresh Emacs to check if I can still
reproduce the problems I was finding the last time. If I can't, I'll
just push it , unless someone objects.
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 05 Feb 2019 08:38:01 GMT)
Full text and
rfc822 format available.
Message #449 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> Thanks, it works without errors, but for some reason it doesn't fit
> the buffer into the window, i.e. the old version
>
> (pop-to-buffer buf '(display-buffer--maybe-at-bottom))
>
> correctly resized the window, but the new version above doesn't.
Sorry. You will have to step through it with edebug to find out the
cause. Maybe the new window is not appropriately combined. IIRC
resizing should by default affect only one neighboring window.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 05 Feb 2019 13:45:02 GMT)
Full text and
rfc822 format available.
Message #452 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 03.02.2019 23:33, Juri Linkov wrote:
> Let me summarize my point of view of the current situation:
Could you also describe how M-x project-find-regexp works, with the
"old" code, and with your proposal?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 05 Feb 2019 18:13:02 GMT)
Full text and
rfc822 format available.
Message #455 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> From: João Távora <joaotavora <at> gmail.com>
> Cc: juri <at> linkov.net, dgutov <at> yandex.ru, 33870 <at> debbugs.gnu.org
> Date: Sun, 03 Feb 2019 20:22:17 +0000
>
> Let me try one last time. 2 days ago, you decided to chime in very
> tersely: "No, I don't think so"
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=33870#380
>
> , presumably saying that the default behaviour in Emacs 26.1 doesn't
> make sense for a specific edge case that I had been testing. Because
> you may have been misunderstanding I replied with a detailed explanation
> of the reasoning for that particular behaviour in:
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=33870#383
>
> , which you didn't reply to, but Dmitry accepted as sufficient
> validation for the current behaviour. Meanwhile, Juri used this
> opportunity to insist that the current behaviour is sub-optimal.
The current behavior is somewhat sub-optimal, but so is IMO the
alternative that Juri suggests: the "tiny window below the original
one", as I understand it, will make M-. and fiends behave differently
from any other command which needs to use "the other window", like
"M-x compile", "M-x grep", etc. So I think I like the current
behavior better. The part of the current behavior where RET after
"C-x 4 ." makes a new window I don't like too much, I think it would
be better to reuse the window where *xref* is shown. But I wouldn't
insist on such a change, mainly because I think "C-x 4 ." makes little
sense anyway, as we have "M-," to easily return to the buffer we were
in originally.
So on balance I think we don't need to change the current UI.
> Somewhere along the line we started miscommunicating that someone was
> asking the other for input, but for me this is very simple: let's
> install Juri's latest patch, which allows for configuring different
> behaviors and _then_ discuss which one, if any, of the set of new
> possibilities could become the new default.
If that patch doesn't change the default behavior, I have nothing
against installing it on master.
Thanks.
P.S. Sorry for a delay in responding, I had several urgent tasks that
ate up all my free time.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 05 Feb 2019 18:35:02 GMT)
Full text and
rfc822 format available.
Message #458 received at 33870 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
> behavior better. The part of the current behavior where RET after
> "C-x 4 ." makes a new window I don't like too much,
I don't know if Juri description says it does, but I agree with you it
shouldn't.
> I think it would
> be better to reuse the window where *xref* is shown.
It does exactly this, in my tests. Unless, of course, it has another
available window besides the original one and the *xref* one.
> So on balance I think we don't need to change the current UI.
Yes, though I would like to at least try out Juri's proposal in practice
day-to-day programming.
> If that patch doesn't change the default behavior, I have nothing
> against installing it on master.
That's my position, too. Apparently, Juri's patch makes it easy to
switch between alternatives.
> P.S. Sorry for a delay in responding, I had several urgent tasks that
> ate up all my free time.
No problem.
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Wed, 06 Feb 2019 22:54:01 GMT)
Full text and
rfc822 format available.
Message #461 received at 33870 <at> debbugs.gnu.org (full text, mbox):
After retesting and not being able to reproduce the problems anymore,
I pushed Juri's patch.
On Tue, Feb 5, 2019 at 6:34 PM João Távora <joaotavora <at> gmail.com> wrote:
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> > behavior better. The part of the current behavior where RET after
> > "C-x 4 ." makes a new window I don't like too much,
>
> I don't know if Juri description says it does, but I agree with you it
> shouldn't.
>
> > I think it would
> > be better to reuse the window where *xref* is shown.
>
> It does exactly this, in my tests. Unless, of course, it has another
> available window besides the original one and the *xref* one.
>
> > So on balance I think we don't need to change the current UI.
>
> Yes, though I would like to at least try out Juri's proposal in practice
> day-to-day programming.
>
> > If that patch doesn't change the default behavior, I have nothing
> > against installing it on master.
>
> That's my position, too. Apparently, Juri's patch makes it easy to
> switch between alternatives.
>
> > P.S. Sorry for a delay in responding, I had several urgent tasks that
> > ate up all my free time.
>
> No problem.
>
> João
--
João Távora
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 17 Feb 2019 21:10:04 GMT)
Full text and
rfc822 format available.
Message #464 received at 33870 <at> debbugs.gnu.org (full text, mbox):
> After retesting and not being able to reproduce the problems anymore,
> I pushed Juri's patch.
Thanks, João. Before closing this bug, please confirm if you prefer
renaming window--display-buffer to display-buffer-in-window in this bug,
or creating a new request? I'll also create separate requests for
all other related topics from this bug.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 17 Feb 2019 21:23:01 GMT)
Full text and
rfc822 format available.
Message #467 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> Thanks, it works without errors, but for some reason it doesn't fit
>> the buffer into the window, i.e. the old version
>>
>> (pop-to-buffer buf '(display-buffer--maybe-at-bottom))
>>
>> correctly resized the window, but the new version above doesn't.
>
> Sorry. You will have to step through it with edebug to find out the
> cause. Maybe the new window is not appropriately combined. IIRC
> resizing should by default affect only one neighboring window.
I see the same problem with the *Marked Processes* buffer from proced,
but I'll create a separate request.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Sun, 17 Feb 2019 21:23:02 GMT)
Full text and
rfc822 format available.
Message #470 received at 33870 <at> debbugs.gnu.org (full text, mbox):
>> Let me summarize my point of view of the current situation:
>
> Could you also describe how M-x project-find-regexp works, with the "old"
> code, and with your proposal?
Which one? I have two proposals: one that creates a tiny window,
but has unsolved problems when you ask the window manager to display
a source code buffer in the specific window, it displays the xref buffer
in this window instead. So a better proposal was to always display
the xref buffer in the same window where the user expected to see
a source code buffer. But this could be discussed now in bug#33992.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Fri, 22 Feb 2019 02:19:02 GMT)
Full text and
rfc822 format available.
Message #473 received at 33870 <at> debbugs.gnu.org (full text, mbox):
On 18.02.2019 00:20, Juri Linkov wrote:
> Which one? I have two proposals: one that creates a tiny window,
> but has unsolved problems when you ask the window manager to display
> a source code buffer in the specific window, it displays the xref buffer
> in this window instead. So a better proposal was to always display
> the xref buffer in the same window where the user expected to see
> a source code buffer.
Allow me to rephrase that in the form of a requirement: however the
behavior of xref-find-definition changes, I don't think the results of
project-find-regexp (or xref-find-references, for that matter) should be
displayed in a tiny window.
> But this could be discussed now in bug#33992.
Sure.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#33870; Package
emacs.
(Tue, 11 Jun 2019 00:01:02 GMT)
Full text and
rfc822 format available.
Message #476 received at 33870 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi Juri,
On 27.01.2019 22:29, Juri Linkov wrote:
>>> If only that patch were able to keep the current behavior by default.
>> Yep. If Juri provides a simpler patch that does this I'm all for it.
> Ok, here's 100% backward-compatible patch:
So apparently this patch was installed, but without a link to the
relevant bug report, and it was not closed either.
Which is just as well, because my testing shows that it's really not
100% backward compatible.
1. display-buffer-in-previous-window, as I mentioned in another email,
does not reliably use the supplied `previous-window' value.
2. 'C-x 4 .' followed by TAB (or RET) is broken: instead of using the
other window, it uses the original window, just like M-. does. It's a
misunderstanding inside the code, see below.
> xref.simplify.patch
>
> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
> index 87ce2299c5..9522d7e475 100644
> --- a/lisp/progmodes/xref.el
> +++ b/lisp/progmodes/xref.el
> @@ -474,27 +474,17 @@ xref--show-pos-in-buf
> (or (eq xref--original-window-intent 'frame)
> pop-up-frames))
> (action
> - (cond ((memq
> - xref--original-window-intent
> - '(window frame))
> + (cond ((eq xref--original-window-intent 'frame)
> t)
> + ((eq xref--original-window-intent 'window)
> + '(display-buffer-same-window))
That's not what the `window' value means. It should mean "other window".
I really don't want to revert this change after all this discussion, but
implementing it in a different way is not straightforward. My very first
idea turned out to be to write it more or less like it was before.
But here's an alternative patch. Juri, what do you think? Does it keep
your customizations working?
[xref-display-buffer-functions.diff (text/x-patch, attachment)]
Reply sent
to
Dmitry Gutov <dgutov <at> yandex.ru>:
You have taken responsibility.
(Sun, 16 Jun 2019 00:53:02 GMT)
Full text and
rfc822 format available.
Notification sent
to
Juri Linkov <juri <at> linkov.net>:
bug acknowledged by developer.
(Sun, 16 Jun 2019 00:53:02 GMT)
Full text and
rfc822 format available.
Message #481 received at 33870-done <at> debbugs.gnu.org (full text, mbox):
On 11.06.2019 3:00, Dmitry Gutov wrote:
> But here's an alternative patch. <...>
Installed, and closing.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Sun, 14 Jul 2019 11:24:08 GMT)
Full text and
rfc822 format available.
This bug report was last modified 4 years and 287 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.