bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Juri Linkov]

Suppose we have such customization:

  (setq display-buffer-alist '(("\\`\\*grep\\*\\'" display-buffer-same-window)))

Now after running grep with a non-empty output in ‘emacs -Q’,
typing ‘C-o’ (compilation-display-error) will display two identical
buffers instead of displaying the buffer with the found grep hit.

The problem is with pop-to-buffer in next-error-no-select.
I wonder what is an idiomatic way to ensure a buffer is already
displayed in some window, to not display the same buffer
in other window?

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[martin rudalics]

 > Suppose we have such customization:
 >
 >    (setq display-buffer-alist '(("\\`\\*grep\\*\\'" display-buffer-same-window)))
 >
 > Now after running grep with a non-empty output in ‘emacs -Q’,
 > typing ‘C-o’ (compilation-display-error) will display two identical
 > buffers instead of displaying the buffer with the found grep hit.
 >
 > The problem is with pop-to-buffer in next-error-no-select.
 > I wonder what is an idiomatic way to ensure a buffer is already
 > displayed in some window, to not display the same buffer
 > in other window?

I'm not quite sure I understand: Is there a reason you did not supply
'display-buffer-reuse-window' in the above customization?

martin

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Juri Linkov]

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

>> Suppose we have such customization:
>>
>>    (setq display-buffer-alist '(("\\`\\*grep\\*\\'" display-buffer-same-window)))
>>
>> Now after running grep with a non-empty output in ‘emacs -Q’,
>> typing ‘C-o’ (compilation-display-error) will display two identical
>> buffers instead of displaying the buffer with the found grep hit.
>>
>> The problem is with pop-to-buffer in next-error-no-select.
>> I wonder what is an idiomatic way to ensure a buffer is already
>> displayed in some window, to not display the same buffer
>> in other window?
>
> I'm not quite sure I understand: Is there a reason you did not supply
> 'display-buffer-reuse-window' in the above customization?

Because it's an idiomatic way to tell the Emacs window manager to
display the *grep* buffer in the same window (an old way to do the same
was using same-window-buffer-names and same-window-regexps)
when manually running M-x grep.

But it fails when using C-o from the already displayed *grep*.
I think next-error-no-select should override the user setting with
display-buffer-overriding-action because the purpose of pop-to-buffer in
next-error-no-select is to ensure the *grep* buffer is displayed somewhere:

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

diff --git a/lisp/simple.el b/lisp/simple.el
index 0ccf2f1d22..df7d86a835 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -385,7 +385,8 @@ next-error-no-select
   (interactive "p")
   (let ((next-error-highlight next-error-highlight-no-select))
     (next-error n))
-  (pop-to-buffer next-error-last-buffer))
+  (let ((display-buffer-overriding-action '(display-buffer-reuse-window)))
+    (pop-to-buffer next-error-last-buffer)))
 
 (defun previous-error-no-select (&optional n)
   "Move point to the previous error in the `next-error' buffer and highlight match.

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[martin rudalics]

 >> I'm not quite sure I understand: Is there a reason you did not supply
 >> 'display-buffer-reuse-window' in the above customization?
 >
 > Because it's an idiomatic way to tell the Emacs window manager to
 > display the *grep* buffer in the same window (an old way to do the same
 > was using same-window-buffer-names and same-window-regexps)
 > when manually running M-x grep.

But that's what 'display-buffer-same-window' has been devised for.
'display-buffer-reuse-window' is supposed to use _any_ window that
already displays the buffer, not just the selected one.

 > But it fails when using C-o from the already displayed *grep*.
 > I think next-error-no-select should override the user setting with
 > display-buffer-overriding-action because the purpose of pop-to-buffer in
 > next-error-no-select is to ensure the *grep* buffer is displayed somewhere:

And if a user wanted to pop up a new window here?

martin

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Juri Linkov]

>> But it fails when using C-o from the already displayed *grep*.
>> I think next-error-no-select should override the user setting with
>> display-buffer-overriding-action because the purpose of pop-to-buffer in
>> next-error-no-select is to ensure the *grep* buffer is displayed somewhere:
>
> And if a user wanted to pop up a new window here?

It pops up according to user setting but only when
this buffer is not displayed already in other window.

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[martin rudalics]

 >>> But it fails when using C-o from the already displayed *grep*.
 >>> I think next-error-no-select should override the user setting with
 >>> display-buffer-overriding-action because the purpose of pop-to-buffer in
 >>> next-error-no-select is to ensure the *grep* buffer is displayed somewhere:
 >>
 >> And if a user wanted to pop up a new window here?
 >
 > It pops up according to user setting but only when
 > this buffer is not displayed already in other window.

Still: IMHO it's not the task of the calling function to fix a bad
user customization.

martin

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Juri Linkov]

>>>> But it fails when using C-o from the already displayed *grep*.
>>>> I think next-error-no-select should override the user setting with
>>>> display-buffer-overriding-action because the purpose of pop-to-buffer in
>>>> next-error-no-select is to ensure the *grep* buffer is displayed somewhere:
>>>
>>> And if a user wanted to pop up a new window here?
>>
>> It pops up according to user setting but only when
>> this buffer is not displayed already in other window.
>
> Still: IMHO it's not the task of the calling function to fix a bad
> user customization.

What is bad in user customization?  It's normal customization
that corresponds to the old way of using same-window-regexps.
It's not responsibility of user customization to workaround
deficiencies in core functions.  The task of next-error-no-select
is to check if the buffer is already displayed, not to pop up
a new buffer.  There is no such problem in other commands that
use pop-to-buffer.

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[martin rudalics]

 > What is bad in user customization?

That this particular one

(setq display-buffer-alist '(("\\`\\*grep\\*\\'" display-buffer-same-window)))

apparently doesn't do what its writer intended.  This customization
means that its writer requests to display the buffer in the selected
window _regardless_ of whether the displayed buffer is already
displayed anywhere else.  Such a request is legitimate (think of the
case where I am perviewing a part of a buffer in another window and I
do not want to change that window's point) and an application should
not override it.  I think the intended customization in the case at
hand is

(setq display-buffer-alist '(("\\`\\*grep\\*\\'" (display-buffer-reuse-window display-buffer-same-window))))

Or what am I missing?

martin

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Juri Linkov]

>> What is bad in user customization?
>
> That this particular one
>
> (setq display-buffer-alist '(("\\`\\*grep\\*\\'" display-buffer-same-window)))
>
> apparently doesn't do what its writer intended.  This customization
> means that its writer requests to display the buffer in the selected
> window _regardless_ of whether the displayed buffer is already
> displayed anywhere else.  Such a request is legitimate (think of the
> case where I am perviewing a part of a buffer in another window and I
> do not want to change that window's point) and an application should
> not override it.

This is the right customization.  The intended customization is to
display the buffer in the same window, even if this buffer is already
displayed in another window: the user navigates to the needed window
and wants this buffer to appear in the selected window, not to switch to
some other window that might already display it.  That's the point of
this bug report.

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[martin rudalics]

 >> That this particular one
 >>
 >> (setq display-buffer-alist '(("\\`\\*grep\\*\\'" display-buffer-same-window)))
 >>
 >> apparently doesn't do what its writer intended.  This customization
 >> means that its writer requests to display the buffer in the selected
 >> window _regardless_ of whether the displayed buffer is already
 >> displayed anywhere else.  Such a request is legitimate (think of the
 >> case where I am perviewing a part of a buffer in another window and I
 >> do not want to change that window's point) and an application should
 >> not override it.
 >
 > This is the right customization.  The intended customization is to
 > display the buffer in the same window, even if this buffer is already
 > displayed in another window: the user navigates to the needed window
 > and wants this buffer to appear in the selected window, not to switch to
 > some other window that might already display it.  That's the point of
 > this bug report.

Funny.  I don't recall that in all those years we ever disagreed on a
single issue.  And now we have right two of them.

If the intended customization is to display the *grep* buffer always
in the selected window, then why

    "next-error-no-select should override the user setting with
display-buffer-overriding-action because the purpose of pop-to-buffer in
next-error-no-select is to ensure the *grep* buffer is displayed somewhere"

Isn't *grep* displayed somewhere when it shows up in the selected
window and also in some other window?  Maybe the *grep* case is
special but I still fail to see why.

martin

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Juri Linkov]

> Isn't *grep* displayed somewhere when it shows up in the selected
> window and also in some other window?  Maybe the *grep* case is
> special but I still fail to see why.

The special case is in next-error-no-select.  Its purpose is to display
the next-error buffer (the buffer with the location of the next-error),
not to display next-error-last-buffer (the buffer with the list of errors)
because next-error-last-buffer is already displayed.  The user runs
next-error-no-select from the selected window that already displays
next-error-last-buffer, there is no need to try and display the same
buffer again.  This behavior is not even documented.  So pop-to-buffer
is useless here.  But I suspect that maybe it was added for some use cases
to ensure that next-error-last-buffer is displayed.  So at least
we need to make it harmless.  But currently it makes harm with the
reported configuration.  I provided a patch to fix next-error-no-select
to not fail with the legitimate user configuration.

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Fri, 07 Sep 2018 00:56:51 +0300
> Cc: 32607@debbugs.gnu.org
> 
> The special case is in next-error-no-select.  Its purpose is to display
> the next-error buffer (the buffer with the location of the next-error),
> not to display next-error-last-buffer (the buffer with the list of errors)
> because next-error-last-buffer is already displayed.  The user runs
> next-error-no-select from the selected window that already displays
> next-error-last-buffer, there is no need to try and display the same
> buffer again.  This behavior is not even documented.

Can we please document all this?  I don't see how could anyone glean
all that from the available documentation.

And I think it's high time to have next-error facilities described in
all pertinent details in the ELisp manual.  It is way too complex and
powerful not to be mentioned there even as a hint.

Thanks.

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Juri Linkov]

>> The special case is in next-error-no-select.  Its purpose is to display
>> the next-error buffer (the buffer with the location of the next-error),
>> not to display next-error-last-buffer (the buffer with the list of errors)
>> because next-error-last-buffer is already displayed.  The user runs
>> next-error-no-select from the selected window that already displays
>> next-error-last-buffer, there is no need to try and display the same
>> buffer again.  This behavior is not even documented.
>
> Can we please document all this?  I don't see how could anyone glean
> all that from the available documentation.
>
> And I think it's high time to have next-error facilities described in
> all pertinent details in the ELisp manual.  It is way too complex and
> powerful not to be mentioned there even as a hint.

Whereas I helped to develop the next-error framework, I'm not its author,
so I don't know the purpose of some parts, for example, I had no idea
why next-error-no-select was implemented this way.  I see that
next-error-no-select is called from compilation-display-error,
but don't understand why in grep mode the ‘n’ key is bound directly
to next-error-no-select, but not to compilation-display-error.
Also I see a glaring question mark in the comments of grep-mode-map
asking why "\r" is bound to compile-goto-error, and unfortunately
I have no answer.

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: rudalics@gmx.at,  32607@debbugs.gnu.org
> Date: Sun, 09 Sep 2018 02:28:23 +0300
> 
> > Can we please document all this?  I don't see how could anyone glean
> > all that from the available documentation.
> >
> > And I think it's high time to have next-error facilities described in
> > all pertinent details in the ELisp manual.  It is way too complex and
> > powerful not to be mentioned there even as a hint.
> 
> Whereas I helped to develop the next-error framework, I'm not its author,
> so I don't know the purpose of some parts, for example, I had no idea
> why next-error-no-select was implemented this way.  I see that
> next-error-no-select is called from compilation-display-error,
> but don't understand why in grep mode the ‘n’ key is bound directly
> to next-error-no-select, but not to compilation-display-error.
> Also I see a glaring question mark in the comments of grep-mode-map
> asking why "\r" is bound to compile-goto-error, and unfortunately
> I have no answer.

That's OK: partial documentation is better than no documentation.  And
we can ask those who were involved in those other parts to contribute
their part of knowledge, whether by a patch or by describing the stuff
here, so that someone else could then convert that to Texinfo.

TIA

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[martin rudalics]

 > The special case is in next-error-no-select.  Its purpose is to display
 > the next-error buffer (the buffer with the location of the next-error),
 > not to display next-error-last-buffer (the buffer with the list of errors)
 > because next-error-last-buffer is already displayed.  The user runs
 > next-error-no-select from the selected window that already displays
 > next-error-last-buffer, there is no need to try and display the same
 > buffer again.  This behavior is not even documented.  So pop-to-buffer
 > is useless here.  But I suspect that maybe it was added for some use cases
 > to ensure that next-error-last-buffer is displayed.  So at least
 > we need to make it harmless.  But currently it makes harm with the
 > reported configuration.  I provided a patch to fix next-error-no-select
 > to not fail with the legitimate user configuration.

I suppose the idea is to implicitly emphasize the "-no-select" postfix
by showing 'next-error-buffer' somewhere and simultaneously selecting
a window showing 'next-error-last-buffer'.  That's a rather unpleasant
hack, your analysis is correct and 'display-buffer-overriding-action'
is a valid remedy here (it's at least as good as

   (unless (get-buffer-window next-error-last-buffer)
     (pop-to-buffer next-error-last-buffer))

IMHO) so install it.

martin

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Juri Linkov]

>> The special case is in next-error-no-select.  Its purpose is to display
>> the next-error buffer (the buffer with the location of the next-error),
>> not to display next-error-last-buffer (the buffer with the list of errors)
>> because next-error-last-buffer is already displayed.  The user runs
>> next-error-no-select from the selected window that already displays
>> next-error-last-buffer, there is no need to try and display the same
>> buffer again.  This behavior is not even documented.  So pop-to-buffer
>> is useless here.  But I suspect that maybe it was added for some use cases
>> to ensure that next-error-last-buffer is displayed.  So at least
>> we need to make it harmless.  But currently it makes harm with the
>> reported configuration.  I provided a patch to fix next-error-no-select
>> to not fail with the legitimate user configuration.
>
> I suppose the idea is to implicitly emphasize the "-no-select" postfix
> by showing 'next-error-buffer' somewhere and simultaneously selecting
> a window showing 'next-error-last-buffer'.  That's a rather unpleasant
> hack, your analysis is correct and 'display-buffer-overriding-action'
> is a valid remedy here (it's at least as good as
>
>   (unless (get-buffer-window next-error-last-buffer)
>     (pop-to-buffer next-error-last-buffer))
>
> IMHO) so install it.

Sorry that I failed to explain this earlier, but it just occurred to me
that the purpose of pop-to-buffer in next-error-no-select is to switch
the selected window to next-error-last-buffer from the buffer's window
with next-error.  Because when I tried your code it failed by leaving
point in wrong window: after this command point should be back in the
next-error-last-buffer window, but without calling pop-to-buffer
it remains in the window with the next-error buffer.  The solution
with using display-buffer-overriding-action correctly puts point back
to select next-error-last-buffer window.

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[martin rudalics]

 > Sorry that I failed to explain this earlier, but it just occurred to me
 > that the purpose of pop-to-buffer in next-error-no-select is to switch
 > the selected window to next-error-last-buffer from the buffer's window
 > with next-error.  Because when I tried your code it failed by leaving
 > point in wrong window: after this command point should be back in the
 > next-error-last-buffer window, but without calling pop-to-buffer
 > it remains in the window with the next-error buffer.  The solution
 > with using display-buffer-overriding-action correctly puts point back
 > to select next-error-last-buffer window.

I have no idea what the precise semantics of "visit" in Compilation
Mode or the first line of the 'next-error' doc-string

   "Visit next `next-error' message and corresponding source code.

are.  I suspect that the initial idea of compilation mode was to keep
or make the buffer with the error messages visible and pop to a window
showing the error locus.  Later on, the capability to keep the window
with the error messages selected was added which means that the
selected window jumps from that of the messages buffer to that of the
locus buffer and then back to that of the messages buffer.

That later capability seems to be that of 'next-error-no-select' whose
doc-string should probably not say what the function does not ("Finds
and highlights the source line like \\[next-error], but does not
select the source buffer.") but rather what the function is supposed
to do instead.  I have no idea why the messges buffer should be made
or kept visible (a user might want to keep a completely unrelated
window selected and navigate the messages buffer from there with the
help of some private key binding).

If the purpose of 'next-error-no-select' is simply to not select the
window of the locus buffer, then 'next-error-no-select' should no rely
on 'next-error'.  Rather we should provide a generic function to
display the locus buffer and have 'next-error' select the window used
and 'next-error-no-select' not select that window.

martin

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Juri Linkov]

> If the purpose of 'next-error-no-select' is simply to not select the
> window of the locus buffer, then 'next-error-no-select' should no rely
> on 'next-error'.  Rather we should provide a generic function to
> display the locus buffer and have 'next-error' select the window used

Maybe you mean something like

  (save-window-excursion (next-error))

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[martin rudalics]

 >> If the purpose of 'next-error-no-select' is simply to not select the
 >> window of the locus buffer, then 'next-error-no-select' should no rely
 >> on 'next-error'.  Rather we should provide a generic function to
 >> display the locus buffer and have 'next-error' select the window used
 >
 > Maybe you mean something like
 >
 >    (save-window-excursion (next-error))

Certainly not because that function may have to display the locus
buffer in a new window.

martin

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Juri Linkov]

>>> If the purpose of 'next-error-no-select' is simply to not select the
>>> window of the locus buffer, then 'next-error-no-select' should no rely
>>> on 'next-error'.  Rather we should provide a generic function to
>>> display the locus buffer and have 'next-error' select the window used
>>
>> Maybe you mean something like
>>
>>    (save-window-excursion (next-error))
>
> Certainly not because that function may have to display the locus
> buffer in a new window.

All that I understood from this thread, is that since next-error-no-select
window juggling involves two windows, it has to override the same-window
user customization using display-buffer-overriding-action.  Currently
I see no other solution that don't involve more fundamental changes.

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[martin rudalics]

 > All that I understood from this thread, is that since next-error-no-select
 > window juggling involves two windows, it has to override the same-window
 > user customization using display-buffer-overriding-action.  Currently
 > I see no other solution that don't involve more fundamental changes.

Agreed.  Let's install the 'display-buffer-overriding-action' change
and leave it there.

martin

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Juri Linkov]

>> All that I understood from this thread, is that since next-error-no-select
>> window juggling involves two windows, it has to override the same-window
>> user customization using display-buffer-overriding-action.  Currently
>> I see no other solution that don't involve more fundamental changes.
>
> Agreed.  Let's install the 'display-buffer-overriding-action' change
> and leave it there.

OK, done.

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Stefan Monnier]

IIUC next-error-no-select is provided so you can do `n n n n n` to display
the various grep hits "in their habitats" while staying within the
*grep* buffer.

> If the purpose of 'next-error-no-select' is simply to not select the
> window of the locus buffer, then 'next-error-no-select' should no rely
> on 'next-error'.  Rather we should provide a generic function to
> display the locus buffer and have 'next-error' select the window used
> and 'next-error-no-select' not select that window.

Sounds right,


        Stefan

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Juri Linkov]

> IIUC next-error-no-select is provided so you can do `n n n n n` to display
> the various grep hits "in their habitats" while staying within the
> *grep* buffer.

I checked that this workflow works correctly with the recent fix and the
reported configuration.

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Stefan Monnier]

> on 'next-error'.  Rather we should provide a generic function to
> display the locus buffer and have 'next-error' select the window used
> and 'next-error-no-select' not select that window.

BTW, it's not just a question of selection: "display the locus buffer"
may also hide the source buffer, hence the need for the subsequent
pop-to-buffer.


        Stefan

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[martin rudalics]

 > BTW, it's not just a question of selection: "display the locus buffer"
 > may also hide the source buffer, hence the need for the subsequent
 > pop-to-buffer.

... which may then hide the locus buffer.

martin

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Stefan Monnier]

>> BTW, it's not just a question of selection: "display the locus buffer"
>> may also hide the source buffer, hence the need for the subsequent
>> pop-to-buffer.
> ... which may then hide the locus buffer.

Indeed.  I guess for this reason next-error-no-select might want to
let-bind display-buffer-overriding-action to something like
(nil (inhibit-same-window . t)) during the call to next-error.


        Stefan

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Juri Linkov]

>>> BTW, it's not just a question of selection: "display the locus buffer"
>>> may also hide the source buffer, hence the need for the subsequent
>>> pop-to-buffer.
>> ... which may then hide the locus buffer.
>
> Indeed.  I guess for this reason next-error-no-select might want to
> let-bind display-buffer-overriding-action to something like
> (nil (inhibit-same-window . t)) during the call to next-error.

Then the same overriding should be applied to both:

diff --git a/lisp/simple.el b/lisp/simple.el
index ffd7fcc067..17ebcb2e16 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -383,9 +383,10 @@ next-error-no-select
 Finds and highlights the source line like \\[next-error], but does not
 select the source buffer."
   (interactive "p")
-  (let ((next-error-highlight next-error-highlight-no-select))
+  (let ((next-error-highlight next-error-highlight-no-select)
+        (display-buffer-overriding-action '(nil (inhibit-same-window . t))))
     (next-error n))
-  (let ((display-buffer-overriding-action '(display-buffer-reuse-window)))
+  (let ((display-buffer-overriding-action '(nil (inhibit-same-window . t))))
     ;; Override user customization such as display-buffer-same-window
     ;; and use display-buffer-reuse-window to ensure next-error-last-buffer
     ;; is displayed somewhere, not necessarily in the same window (bug#32607).
     (pop-to-buffer next-error-last-buffer)

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Stefan Monnier]

>> Indeed.  I guess for this reason next-error-no-select might want to
>> let-bind display-buffer-overriding-action to something like
>> (nil (inhibit-same-window . t)) during the call to next-error.
>
> Then the same overriding should be applied to both:
>
> diff --git a/lisp/simple.el b/lisp/simple.el
> index ffd7fcc067..17ebcb2e16 100644
> --- a/lisp/simple.el
> +++ b/lisp/simple.el
> @@ -383,9 +383,10 @@ next-error-no-select
>  Finds and highlights the source line like \\[next-error], but does not
>  select the source buffer."
>    (interactive "p")
> -  (let ((next-error-highlight next-error-highlight-no-select))
> +  (let ((next-error-highlight next-error-highlight-no-select)
> +        (display-buffer-overriding-action '(nil (inhibit-same-window . t))))
>      (next-error n))
> -  (let ((display-buffer-overriding-action '(display-buffer-reuse-window)))
> +  (let ((display-buffer-overriding-action '(nil (inhibit-same-window . t))))
>      ;; Override user customization such as display-buffer-same-window
>      ;; and use display-buffer-reuse-window to ensure next-error-last-buffer
>      ;; is displayed somewhere, not necessarily in the same window (bug#32607).
>      (pop-to-buffer next-error-last-buffer)

The first inhibit-same-window should hopefully make the
second unnecessary.  If the first fails to do its job or somehow
indirectly causes the original buffer not to be displayed in the
original window, I'm not really sure what we should do about it.
IOW, for the second part I'm not sure either of
display-buffer-reuse-window or inhibit-same-window is clearly superior
to the other.

Maybe to get closer to "the ideal", we should go for something like:

    (let* ((orig-window (selected-window))
           (orig-buf (window-buffer orig-window)))
      (let ((next-error-highlight next-error-highlight-no-select)
            (display-buffer-overriding-action '(nil (inhibit-same-window . t))))
        (next-error n))
      (cond
       ((eql (window-buffer orig-window) next-error-last-buffer)
        ;; inhibit-same-window did its job, we can just return to the original
        ;; window.
        (select-window orig-window))
       ((eql orig-buf next-error-last-buffer)
        ;; Somehow the original window was affected by `next-error`, so
        ;; we need to work harder to bring the buffer back.
        (select-window orig-window)
        (pop-to-buffer-same-window next-error-last-buffer))
       (t
        ;; Something weird is going on.  We don't really know where we were
        ;; (orig-window was not showing the buffer where we were supposed
        ;; to "stay"), so let's just try and keep both buffers displayed
        ;; while at the same time trying not to gratuitously creating new
        ;; windows either.
        (let ((display-buffer-overriding-action '(display-buffer-reuse-window
                                                  (inhibit-same-window . t))))
          (pop-to-buffer next-error-last-buffer)))))

But maybe we should instead trust inhibit-same-window to do its job and
go for a simple:

      (save-selected-window
        (let ((next-error-highlight next-error-highlight-no-select)
              (display-buffer-overriding-action
               '(nil (inhibit-same-window . t))))
          (next-error n)))


-- Stefan

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Juri Linkov]

> The first inhibit-same-window should hopefully make the
> second unnecessary.  If the first fails to do its job or somehow
> indirectly causes the original buffer not to be displayed in the
> original window, I'm not really sure what we should do about it.
> IOW, for the second part I'm not sure either of
> display-buffer-reuse-window or inhibit-same-window is clearly superior
> to the other.
>
> Maybe to get closer to "the ideal", we should go for something like:
>
>     (let* ((orig-window (selected-window))
>            (orig-buf (window-buffer orig-window)))
>       (let ((next-error-highlight next-error-highlight-no-select)
>             (display-buffer-overriding-action '(nil (inhibit-same-window . t))))
>         (next-error n))
>       (cond
>        ((eql (window-buffer orig-window) next-error-last-buffer)
>         ;; inhibit-same-window did its job, we can just return to the original
>         ;; window.
>         (select-window orig-window))
>        ((eql orig-buf next-error-last-buffer)
>         ;; Somehow the original window was affected by `next-error`, so
>         ;; we need to work harder to bring the buffer back.
>         (select-window orig-window)
>         (pop-to-buffer-same-window next-error-last-buffer))
>        (t
>         ;; Something weird is going on.  We don't really know where we were
>         ;; (orig-window was not showing the buffer where we were supposed
>         ;; to "stay"), so let's just try and keep both buffers displayed
>         ;; while at the same time trying not to gratuitously creating new
>         ;; windows either.
>         (let ((display-buffer-overriding-action '(display-buffer-reuse-window
>                                                   (inhibit-same-window . t))))
>           (pop-to-buffer next-error-last-buffer)))))

I see that such explicit handling even supports the case when next-error-last-buffer
gets changed on different frames (when using next-error-buffer-on-selected-frame).

> But maybe we should instead trust inhibit-same-window to do its job and
> go for a simple:
>
>       (save-selected-window
>         (let ((next-error-highlight next-error-highlight-no-select)
>               (display-buffer-overriding-action
>                '(nil (inhibit-same-window . t))))
>           (next-error n)))

This is much simpler.  Actually, this is what I wanted to propose as
a solution to Martin in one of previous messages, but I mistakenly wrote
save-window-excursion whereas I actually intended save-selected-window.

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[martin rudalics]

 >> But maybe we should instead trust inhibit-same-window to do its job and
 >> go for a simple:
 >>
 >>        (save-selected-window
 >>          (let ((next-error-highlight next-error-highlight-no-select)
 >>                (display-buffer-overriding-action
 >>                 '(nil (inhibit-same-window . t))))
 >>            (next-error n)))
 >
 > This is much simpler.  Actually, this is what I wanted to propose as
 > a solution to Martin in one of previous messages, but I mistakenly wrote
 > save-window-excursion whereas I actually intended save-selected-window.

I still do not understand whether we are sure that at the time the
code above gets called 'next-error-last-buffer' is really shown in the
selected window.

martin

bug#32607: 27.0.50; pop-to-buffer in next-error-no-select

[Stefan Monnier]

>>>        (save-selected-window
>>>          (let ((next-error-highlight next-error-highlight-no-select)
>>>                (display-buffer-overriding-action
>>>                 '(nil (inhibit-same-window . t))))
>>>            (next-error n)))
>>
>> This is much simpler.  Actually, this is what I wanted to propose as
>> a solution to Martin in one of previous messages, but I mistakenly wrote
>> save-window-excursion whereas I actually intended save-selected-window.
>
> I still do not understand whether we are sure that at the time the
> code above gets called 'next-error-last-buffer' is really shown in the
> selected window.

Indeed, I don't see any obvious indication that this is the case.
I suspect it's *usually* the case because next-error-no-select is only
bound to keys in "error buffers", but if you

    M-x next-error-no-select

from a C file, I suspect that the buffer shown in the selected-window at
the beginning of the command won't be equal to the next-error-last-buffer.

I'm not sure what would be considered the *right* behavior in that case:
should the selected-window at the end display the same buffer as at the
beginning, or should it show the buffer that contains the error
(e.g. *grep* or *compilation*)?

My own take on it is that it's perfectly OK to have the final
selected-window show the buffer in which the command was typed (rather
than the *grep*/*compilation* buffer).


        Stefan