bug#36161: 27.0.50; display-buffer-in-previous-window might choose not to use PREVIOUS-WINDOW

bug#36161: 27.0.50; display-buffer-in-previous-window might choose not to use PREVIOUS-WINDOW

[Dmitry Gutov]

In particular, the last paragraph. When PREVIOUS-WINDOW and the selected
window are the same, it can select a different window, contrary to its
description. Which is a problem for xref--show-pos-in-buf, like I've
mentioned in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=33992#52.

In GNU Emacs 27.0.50 (build 58, x86_64-pc-linux-gnu, GTK+ Version 3.22.30)
  of 2019-06-10 built on zappa
Repository revision: 0f67be92592ef82d971d004f4063e68682c123de
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12001000
System Description: Ubuntu 18.04.2 LTS

bug#36161: 27.0.50; display-buffer-in-previous-window might choose not to use PREVIOUS-WINDOW

[martin rudalics]

 > In particular, the last paragraph. When PREVIOUS-WINDOW and the selected
 > window are the same, it can select a different window, contrary to its
 > description.

Right.  I amended doc-string and info entry accordingly.

We can also change the semantics of this function to fit the old
doc-string but I'd prefer to not do that on the release branch.

Thanks, martin

bug#36161: 27.0.50; display-buffer-in-previous-window might choose not to use PREVIOUS-WINDOW

[Dmitry Gutov]

On 11.06.2019 11:16, martin rudalics wrote:
>  > In particular, the last paragraph. When PREVIOUS-WINDOW and the selected
>  > window are the same, it can select a different window, contrary to its
>  > description.
> 
> Right.  I amended doc-string and info entry accordingly.

FWIW, emacs-26 was pretty far from my mind when I filed this.

But now I realize that we should avoid using this function in xref 
either way in case we decide to distribute it on ELPA as a core package 
for older Emacsen. So thanks.

> We can also change the semantics of this function to fit the old
> doc-string but I'd prefer to not do that on the release branch.

Do you know of any reasons not to do that? If only to make this function 
shorter and easier to understand, that would be a good thing.

TBH, the new docstring seems a lot more confusing to me. "may override" 
doesn't really tell us what the function is going to do.

bug#36161: 27.0.50; display-buffer-in-previous-window might choose not to use PREVIOUS-WINDOW

[martin rudalics]

 >> We can also change the semantics of this function to fit the old
 >> doc-string but I'd prefer to not do that on the release branch.
 >
 > Do you know of any reasons not to do that? If only to make this function shorter and easier to understand, that would be a good thing.

I would have to understand first why one and the same buffer was
displayed in different windows previously.

 > TBH, the new docstring seems a lot more confusing to me. "may override" doesn't really tell us what the function is going to do.

OK.  This time I tried to tell everything I know.  Please have a look.

martin

bug#36161: 27.0.50; display-buffer-in-previous-window might choose not to use PREVIOUS-WINDOW

[Dmitry Gutov]

On 12.06.2019 12:18, martin rudalics wrote:

>  > TBH, the new docstring seems a lot more confusing to me. "may 
> override" doesn't really tell us what the function is going to do.
> 
> OK.  This time I tried to tell everything I know.  Please have a look.

It's much better now, thanks. And the wording of "is usable for" is 
somehow more obvious to me (even though the information is the same).

 > I would have to understand first why one and the same buffer was
 > displayed in different windows previously.

Not sure what answer you're looking for. Suppose a user used 
switch-to-buffer sometime in the past to show it there?

I think the question is, if the caller supplies the `previous-window' 
cons in ALIST, is there a point in second-guessing its choice?

FWIW, I've only found one user of this function that does that, and it's 
`debug'. Which behavior do you think that command would prefer?

bug#36161: 27.0.50; display-buffer-in-previous-window might choose not to use PREVIOUS-WINDOW

[martin rudalics]

 >  > I would have to understand first why one and the same buffer was
 >  > displayed in different windows previously.
 >
 > Not sure what answer you're looking for. Suppose a user used switch-to-buffer sometime in the past to show it there?
 >
 > I think the question is, if the caller supplies the `previous-window' cons in ALIST, is there a point in second-guessing its choice?
 >
 > FWIW, I've only found one user of this function that does that, and it's `debug'. Which behavior do you think that command would prefer?

'display-buffer-in-previous-window' was added to fix a problem with
the debugger buffer (Bug#8789).  The idea behind it is that once a
buffer was shown in a specific window and that window still exists, it
would support the principle of least surprise to show it there again.
And to _not_ have this function do anything when a window showing that
buffer was just ephemeral and got deleted in between. In that latter
case a more deterministic function (like below-selected, at-bottom)
should take over.

'display-buffer-in-previous-window' will work well if it is
exclusively used for showing a buffer a second time (maybe together
with 'prev-buffer' and 'next-buffer').  It's behavior deteriorates as
soon as a user switches to that buffer in some arbitrary fashion.
Hence it's more suitable for buffers popping up in special occasions
(like, for example, when a bug occurred and the debugger should be
entered) and less suitable for buffer editing.

A 'previous-window' ALIST entry should continue to support the
principle of least surprise by allowing to choose a non-selected
window that never showed the buffer before.  The selected window,
however, should be always either chosen or excluded from choice by
special means - either via 'display-buffer-same-window' or using an
'inhibit-same-window' entry.

martin

bug#36161: 27.0.50; display-buffer-in-previous-window might choose not to use PREVIOUS-WINDOW

[Dmitry Gutov]

On 13.06.2019 11:43, martin rudalics wrote:

 > Hence it's more suitable for buffers popping up in special occasions
 > (like, for example, when a bug occurred and the debugger should be
 > entered) and less suitable for buffer editing.

I have also noticed it's not always good for special buffers either, 
e.g. if I run xref-find-definitions, press q, and run it again, 
different windows will be used for the two times.

> A 'previous-window' ALIST entry should continue to support the
> principle of least surprise by allowing to choose a non-selected
> window that never showed the buffer before.  The selected window,
> however, should be always either chosen or excluded from choice by
> special means - either via 'display-buffer-same-window' or using an
> 'inhibit-same-window' entry.

So about that main use case (the Debugger), can it just exclude the 
selected window using inhibit-same-window? That would obviate the need 
for special logic in this case.

bug#36161: 27.0.50; display-buffer-in-previous-window might choose not to use PREVIOUS-WINDOW

[martin rudalics]

 >  > Hence it's more suitable for buffers popping up in special occasions
 >  > (like, for example, when a bug occurred and the debugger should be
 >  > entered) and less suitable for buffer editing.
 >
 > I have also noticed it's not always good for special buffers either, e.g. if I run xref-find-definitions, press q, and run it again, different windows will be used for the two times.

When a different window gets used there's usually a reason.  So I'd
need a recipe to tell you more about this ...

 > So about that main use case (the Debugger), can it just exclude the selected window using inhibit-same-window? That would obviate the need for special logic in this case.

An 'inhibit-same-window' entry is the canonical way to "exclude" the
selected window.  A 'previous-window' entry is just a hint to "prefer"
some other window.

martin

bug#36161: 27.0.50; display-buffer-in-previous-window might choose not to use PREVIOUS-WINDOW

[Dmitry Gutov]

On 16.06.2019 11:16, martin rudalics wrote:
>  >  > Hence it's more suitable for buffers popping up in special occasions
>  >  > (like, for example, when a bug occurred and the debugger should be
>  >  > entered) and less suitable for buffer editing.
>  >
>  > I have also noticed it's not always good for special buffers either, 
> e.g. if I run xref-find-definitions, press q, and run it again, 
> different windows will be used for the two times.
> 
> When a different window gets used there's usually a reason.  So I'd
> need a recipe to tell you more about this ...

OK, let's try again. The recipe is:

1. emacs -Q
2. Split the frame into 4 windows, so that new display-buffer calls 
don't pop any new windows.
3. Select the top-left window (for instance).
4. Call M-x project-find-regexp, input "Xref", press RET. The result is 
an *xref* buffer shown in one of the windows.
5. Press q.
6. Try 4. again.
7. Repeat 5,6 as many times as you like. The *xref* buffer will be 
displayed in all three available windows in turn.

>  > So about that main use case (the Debugger), can it just exclude the 
> selected window using inhibit-same-window? That would obviate the need 
> for special logic in this case.
> 
> An 'inhibit-same-window' entry is the canonical way to "exclude" the
> selected window.  A 'previous-window' entry is just a hint to "prefer"
> some other window.

I'm not sure if you're trying to say "no" in response to my question. 
But if so, I suppose this bug report can be closed (and we can conclude 
that this function is definitely not suitable for xref). Thanks for 
updating the docstring.

bug#36161: 27.0.50; display-buffer-in-previous-window might choose not to use PREVIOUS-WINDOW

[martin rudalics]

 > OK, let's try again. The recipe is:
 >
 > 1. emacs -Q
 > 2. Split the frame into 4 windows, so that new display-buffer calls don't pop any new windows.
 > 3. Select the top-left window (for instance).
 > 4. Call M-x project-find-regexp, input "Xref", press RET. The result is an *xref* buffer shown in one of the windows.
 > 5. Press q.
 > 6. Try 4. again.
 > 7. Repeat 5,6 as many times as you like. The *xref* buffer will be displayed in all three available windows in turn.

Sorry, step 4 does not work here so I cannot try that.  The behavior
you see might be caused by the rotation of windows in 'window-list-1'.
But also some other buffer display action might be responsible.  So
the first question is whether you are sure that the window is produced
by 'display-buffer-in-previous-window' in all these cases.

 >>  > So about that main use case (the Debugger), can it just exclude the selected window using inhibit-same-window? That would obviate the need for special logic in this case.
 >>
 >> An 'inhibit-same-window' entry is the canonical way to "exclude" the
 >> selected window.  A 'previous-window' entry is just a hint to "prefer"
 >> some other window.
 >
 > I'm not sure if you're trying to say "no" in response to my question. But if so, I suppose this bug report can be closed (and we can conclude that this function is definitely not suitable for xref). Thanks for updating the docstring.

I've been trying to say "yes" to the question "can it just exclude the
selected window using inhibit-same-window".

martin