unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
@ 2018-09-01 22:32 Juri Linkov
  2018-09-02  7:14 ` martin rudalics
  0 siblings, 1 reply; 32+ messages in thread
From: Juri Linkov @ 2018-09-01 22:32 UTC (permalink / raw)
  To: 32607

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?





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-01 22:32 bug#32607: 27.0.50; pop-to-buffer in next-error-no-select Juri Linkov
@ 2018-09-02  7:14 ` martin rudalics
  2018-09-02 22:43   ` Juri Linkov
  0 siblings, 1 reply; 32+ messages in thread
From: martin rudalics @ 2018-09-02  7:14 UTC (permalink / raw)
  To: Juri Linkov, 32607

 > 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






^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-02  7:14 ` martin rudalics
@ 2018-09-02 22:43   ` Juri Linkov
  2018-09-03  7:31     ` martin rudalics
  0 siblings, 1 reply; 32+ messages in thread
From: Juri Linkov @ 2018-09-02 22:43 UTC (permalink / raw)
  To: martin rudalics; +Cc: 32607

[-- 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.

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-02 22:43   ` Juri Linkov
@ 2018-09-03  7:31     ` martin rudalics
  2018-09-03 22:31       ` Juri Linkov
  0 siblings, 1 reply; 32+ messages in thread
From: martin rudalics @ 2018-09-03  7:31 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 32607

 >> 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





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-03  7:31     ` martin rudalics
@ 2018-09-03 22:31       ` Juri Linkov
  2018-09-04  7:51         ` martin rudalics
  0 siblings, 1 reply; 32+ messages in thread
From: Juri Linkov @ 2018-09-03 22:31 UTC (permalink / raw)
  To: martin rudalics; +Cc: 32607

>> 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.





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-03 22:31       ` Juri Linkov
@ 2018-09-04  7:51         ` martin rudalics
  2018-09-04 21:28           ` Juri Linkov
  0 siblings, 1 reply; 32+ messages in thread
From: martin rudalics @ 2018-09-04  7:51 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 32607

 >>> 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





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-04  7:51         ` martin rudalics
@ 2018-09-04 21:28           ` Juri Linkov
  2018-09-05  7:47             ` martin rudalics
  0 siblings, 1 reply; 32+ messages in thread
From: Juri Linkov @ 2018-09-04 21:28 UTC (permalink / raw)
  To: martin rudalics; +Cc: 32607

>>>> 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.





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-04 21:28           ` Juri Linkov
@ 2018-09-05  7:47             ` martin rudalics
  2018-09-05 22:06               ` Juri Linkov
  0 siblings, 1 reply; 32+ messages in thread
From: martin rudalics @ 2018-09-05  7:47 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 32607

 > 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





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-05  7:47             ` martin rudalics
@ 2018-09-05 22:06               ` Juri Linkov
  2018-09-06  7:04                 ` martin rudalics
  0 siblings, 1 reply; 32+ messages in thread
From: Juri Linkov @ 2018-09-05 22:06 UTC (permalink / raw)
  To: martin rudalics; +Cc: 32607

>> 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.





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-05 22:06               ` Juri Linkov
@ 2018-09-06  7:04                 ` martin rudalics
  2018-09-06 21:56                   ` Juri Linkov
  0 siblings, 1 reply; 32+ messages in thread
From: martin rudalics @ 2018-09-06  7:04 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 32607

 >> 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





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-06  7:04                 ` martin rudalics
@ 2018-09-06 21:56                   ` Juri Linkov
  2018-09-07  6:28                     ` Eli Zaretskii
  2018-09-07  7:28                     ` martin rudalics
  0 siblings, 2 replies; 32+ messages in thread
From: Juri Linkov @ 2018-09-06 21:56 UTC (permalink / raw)
  To: martin rudalics; +Cc: 32607

> 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.





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-06 21:56                   ` Juri Linkov
@ 2018-09-07  6:28                     ` Eli Zaretskii
  2018-09-08 23:28                       ` Juri Linkov
  2018-09-07  7:28                     ` martin rudalics
  1 sibling, 1 reply; 32+ messages in thread
From: Eli Zaretskii @ 2018-09-07  6:28 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 32607

> 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.





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-06 21:56                   ` Juri Linkov
  2018-09-07  6:28                     ` Eli Zaretskii
@ 2018-09-07  7:28                     ` martin rudalics
  2018-09-08 23:46                       ` Juri Linkov
  1 sibling, 1 reply; 32+ messages in thread
From: martin rudalics @ 2018-09-07  7:28 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 32607

 > 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





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-07  6:28                     ` Eli Zaretskii
@ 2018-09-08 23:28                       ` Juri Linkov
  2018-09-09  5:44                         ` Eli Zaretskii
  0 siblings, 1 reply; 32+ messages in thread
From: Juri Linkov @ 2018-09-08 23:28 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 32607

>> 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.





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-07  7:28                     ` martin rudalics
@ 2018-09-08 23:46                       ` Juri Linkov
  2018-09-09  8:40                         ` martin rudalics
  0 siblings, 1 reply; 32+ messages in thread
From: Juri Linkov @ 2018-09-08 23:46 UTC (permalink / raw)
  To: martin rudalics; +Cc: 32607

>> 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.





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-08 23:28                       ` Juri Linkov
@ 2018-09-09  5:44                         ` Eli Zaretskii
  0 siblings, 0 replies; 32+ messages in thread
From: Eli Zaretskii @ 2018-09-09  5:44 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 32607

> 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





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-08 23:46                       ` Juri Linkov
@ 2018-09-09  8:40                         ` martin rudalics
  2018-09-09 16:01                           ` Juri Linkov
                                             ` (2 more replies)
  0 siblings, 3 replies; 32+ messages in thread
From: martin rudalics @ 2018-09-09  8:40 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 32607

 > 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





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-09  8:40                         ` martin rudalics
@ 2018-09-09 16:01                           ` Juri Linkov
  2018-09-10  8:29                             ` martin rudalics
  2018-09-12 22:03                           ` Stefan Monnier
  2018-09-12 22:06                           ` Stefan Monnier
  2 siblings, 1 reply; 32+ messages in thread
From: Juri Linkov @ 2018-09-09 16:01 UTC (permalink / raw)
  To: martin rudalics; +Cc: 32607

> 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))





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-09 16:01                           ` Juri Linkov
@ 2018-09-10  8:29                             ` martin rudalics
  2018-09-11 23:47                               ` Juri Linkov
  0 siblings, 1 reply; 32+ messages in thread
From: martin rudalics @ 2018-09-10  8:29 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 32607

 >> 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





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-10  8:29                             ` martin rudalics
@ 2018-09-11 23:47                               ` Juri Linkov
  2018-09-12  6:33                                 ` martin rudalics
  0 siblings, 1 reply; 32+ messages in thread
From: Juri Linkov @ 2018-09-11 23:47 UTC (permalink / raw)
  To: martin rudalics; +Cc: 32607

>>> 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.





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-11 23:47                               ` Juri Linkov
@ 2018-09-12  6:33                                 ` martin rudalics
  2018-09-12 21:47                                   ` Juri Linkov
  0 siblings, 1 reply; 32+ messages in thread
From: martin rudalics @ 2018-09-12  6:33 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 32607

 > 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






^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-12  6:33                                 ` martin rudalics
@ 2018-09-12 21:47                                   ` Juri Linkov
  0 siblings, 0 replies; 32+ messages in thread
From: Juri Linkov @ 2018-09-12 21:47 UTC (permalink / raw)
  To: martin rudalics; +Cc: 32607-done

>> 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.





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-09  8:40                         ` martin rudalics
  2018-09-09 16:01                           ` Juri Linkov
@ 2018-09-12 22:03                           ` Stefan Monnier
  2018-09-12 22:21                             ` Juri Linkov
  2018-09-12 22:06                           ` Stefan Monnier
  2 siblings, 1 reply; 32+ messages in thread
From: Stefan Monnier @ 2018-09-12 22:03 UTC (permalink / raw)
  To: martin rudalics; +Cc: Juri Linkov, 32607


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





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-09  8:40                         ` martin rudalics
  2018-09-09 16:01                           ` Juri Linkov
  2018-09-12 22:03                           ` Stefan Monnier
@ 2018-09-12 22:06                           ` Stefan Monnier
  2018-09-13  7:46                             ` martin rudalics
  2 siblings, 1 reply; 32+ messages in thread
From: Stefan Monnier @ 2018-09-12 22:06 UTC (permalink / raw)
  To: martin rudalics; +Cc: Juri Linkov, 32607

> 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





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-12 22:03                           ` Stefan Monnier
@ 2018-09-12 22:21                             ` Juri Linkov
  0 siblings, 0 replies; 32+ messages in thread
From: Juri Linkov @ 2018-09-12 22:21 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 32607

> 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.





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-12 22:06                           ` Stefan Monnier
@ 2018-09-13  7:46                             ` martin rudalics
  2018-09-13 11:26                               ` Stefan Monnier
  0 siblings, 1 reply; 32+ messages in thread
From: martin rudalics @ 2018-09-13  7:46 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Juri Linkov, 32607

 > 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





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-13  7:46                             ` martin rudalics
@ 2018-09-13 11:26                               ` Stefan Monnier
  2018-09-13 23:04                                 ` Juri Linkov
  0 siblings, 1 reply; 32+ messages in thread
From: Stefan Monnier @ 2018-09-13 11:26 UTC (permalink / raw)
  To: martin rudalics; +Cc: Juri Linkov, 32607

>> 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





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-13 11:26                               ` Stefan Monnier
@ 2018-09-13 23:04                                 ` Juri Linkov
  2018-09-14  1:34                                   ` Stefan Monnier
  0 siblings, 1 reply; 32+ messages in thread
From: Juri Linkov @ 2018-09-13 23:04 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 32607

>>> 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)





^ permalink raw reply related	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-13 23:04                                 ` Juri Linkov
@ 2018-09-14  1:34                                   ` Stefan Monnier
  2018-09-15 23:31                                     ` Juri Linkov
  0 siblings, 1 reply; 32+ messages in thread
From: Stefan Monnier @ 2018-09-14  1:34 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 32607

>> 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





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-14  1:34                                   ` Stefan Monnier
@ 2018-09-15 23:31                                     ` Juri Linkov
  2018-09-16  9:09                                       ` martin rudalics
  0 siblings, 1 reply; 32+ messages in thread
From: Juri Linkov @ 2018-09-15 23:31 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 32607

> 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.





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-15 23:31                                     ` Juri Linkov
@ 2018-09-16  9:09                                       ` martin rudalics
  2018-09-16 21:19                                         ` Stefan Monnier
  0 siblings, 1 reply; 32+ messages in thread
From: martin rudalics @ 2018-09-16  9:09 UTC (permalink / raw)
  To: Juri Linkov, Stefan Monnier; +Cc: 32607

 >> 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





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#32607: 27.0.50; pop-to-buffer in next-error-no-select
  2018-09-16  9:09                                       ` martin rudalics
@ 2018-09-16 21:19                                         ` Stefan Monnier
  0 siblings, 0 replies; 32+ messages in thread
From: Stefan Monnier @ 2018-09-16 21:19 UTC (permalink / raw)
  To: martin rudalics; +Cc: Juri Linkov, 32607

>>>        (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





^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2018-09-16 21:19 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-01 22:32 bug#32607: 27.0.50; pop-to-buffer in next-error-no-select Juri Linkov
2018-09-02  7:14 ` martin rudalics
2018-09-02 22:43   ` Juri Linkov
2018-09-03  7:31     ` martin rudalics
2018-09-03 22:31       ` Juri Linkov
2018-09-04  7:51         ` martin rudalics
2018-09-04 21:28           ` Juri Linkov
2018-09-05  7:47             ` martin rudalics
2018-09-05 22:06               ` Juri Linkov
2018-09-06  7:04                 ` martin rudalics
2018-09-06 21:56                   ` Juri Linkov
2018-09-07  6:28                     ` Eli Zaretskii
2018-09-08 23:28                       ` Juri Linkov
2018-09-09  5:44                         ` Eli Zaretskii
2018-09-07  7:28                     ` martin rudalics
2018-09-08 23:46                       ` Juri Linkov
2018-09-09  8:40                         ` martin rudalics
2018-09-09 16:01                           ` Juri Linkov
2018-09-10  8:29                             ` martin rudalics
2018-09-11 23:47                               ` Juri Linkov
2018-09-12  6:33                                 ` martin rudalics
2018-09-12 21:47                                   ` Juri Linkov
2018-09-12 22:03                           ` Stefan Monnier
2018-09-12 22:21                             ` Juri Linkov
2018-09-12 22:06                           ` Stefan Monnier
2018-09-13  7:46                             ` martin rudalics
2018-09-13 11:26                               ` Stefan Monnier
2018-09-13 23:04                                 ` Juri Linkov
2018-09-14  1:34                                   ` Stefan Monnier
2018-09-15 23:31                                     ` Juri Linkov
2018-09-16  9:09                                       ` martin rudalics
2018-09-16 21:19                                         ` Stefan Monnier

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).