bug#33258: inhibit-select-window

bug#33258: inhibit-select-window

[Juri Linkov]

Many commands use pop-to-buffer to display a buffer and
select its window by default.

Sometimes it's not desirable to select the displayed window.

One of the many examples is 'C-x v =' (vc-diff) where
the need is only to look at the diffs, but not to operate
on the *vc-diff* buffer in its window.

Currently there is no configurable way for the user
to override the default window selection.

I propose a new alist entry to support such feature.
An example of usage in user's customization:

  (push '("\\*vc-diff\\*" nil
          (inhibit-select-window . t))
        display-buffer-alist)

then pop-to-buffer could check for the alist entry
'inhibit-select-window' and to not select the window
if it's non-nil.

OTOH, when a command uses display-buffer that doesn't select a window,
then the same alist entry with a different value or a new entry e.g.
'(select-window . t)' could be used to force selecting the window.
This could be implemented by using the same code from pop-to-buffer
and adding it to display-buffer functions.

bug#33258: inhibit-select-window

[martin rudalics]

 > Many commands use pop-to-buffer to display a buffer and
 > select its window by default.
 >
 > Sometimes it's not desirable to select the displayed window.
 >
 > One of the many examples is 'C-x v =' (vc-diff) where
 > the need is only to look at the diffs, but not to operate
 > on the *vc-diff* buffer in its window.

Is this particular behavior caused by the

       ;; Display the buffer, but at the end because it can change point.
       (pop-to-buffer (current-buffer))

in 'vc-diff-internal'?

 > Currently there is no configurable way for the user
 > to override the default window selection.
 >
 > I propose a new alist entry to support such feature.
 > An example of usage in user's customization:
 >
 >    (push '("\\*vc-diff\\*" nil
 >            (inhibit-select-window . t))
 >          display-buffer-alist)
 >
 > then pop-to-buffer could check for the alist entry
 > 'inhibit-select-window' and to not select the window
 > if it's non-nil.

I understand what you want but I doubt we can put that into practice.
While not selecting the window might not be a great issue, not making
its buffer current is.  We would have to check each and every call of
'pop-to-buffer' as to whether subsequent code relies on the fact that
it made the buffer current.  This is not fathomable IMO.

One could argue that 'display-buffer' may fail to produce a window and
so the buffer would not be made current in that case either but as we
know it is really hard to make 'display-buffer' fail.

 > OTOH, when a command uses display-buffer that doesn't select a window,
 > then the same alist entry with a different value or a new entry e.g.
 > '(select-window . t)' could be used to force selecting the window.
 > This could be implemented by using the same code from pop-to-buffer
 > and adding it to display-buffer functions.

This would be possible.  Such an entry would have to be called
something like 'force-select-window' and could be applied by both
users and programs for one soberingly simple reason: A Lisp program
can never rely upon 'display-buffer' to _not_ select the chosen window
- popping up a new frame may implicitly select the window at any time.

martin

bug#33258: inhibit-select-window

[Juri Linkov]

>> One of the many examples is 'C-x v =' (vc-diff) where
>> the need is only to look at the diffs, but not to operate
>> on the *vc-diff* buffer in its window.
>
> Is this particular behavior caused by the
>
>       ;; Display the buffer, but at the end because it can change point.
>       (pop-to-buffer (current-buffer))

Yes, and rightfully so.

> While not selecting the window might not be a great issue, not making
> its buffer current is.  We would have to check each and every call of
> 'pop-to-buffer' as to whether subsequent code relies on the fact that
> it made the buffer current.  This is not fathomable IMO.

This feature is intended only for user customization in display-buffer-alist,
not in the args of pop-to-buffer.  It makes no sense for a programmer
to use (inhibit-select-window . t) in the args of pop-to-buffer when
a programmer can simply replace pop-to-buffer with a display-buffer call.

However, if an user is annoyed by always selecting a window of
a displayed buffer by default, and such customization doesn't
cause a bad side effect by not making the displayed buffer the current,
and this is true in 99% cases, then we should be proud to make
the users happier.

>> OTOH, when a command uses display-buffer that doesn't select a window,
>> then the same alist entry with a different value or a new entry e.g.
>> '(select-window . t)' could be used to force selecting the window.
>> This could be implemented by using the same code from pop-to-buffer
>> and adding it to display-buffer functions.
>
> This would be possible.  Such an entry would have to be called
> something like 'force-select-window' and could be applied by both
> users and programs for one soberingly simple reason: A Lisp program
> can never rely upon 'display-buffer' to _not_ select the chosen window
> - popping up a new frame may implicitly select the window at any time.

force-select-window looks good, also possible is one of these names:

  (pop-to-buffer . t)
or
  (pop-up-window . t)

as a reference to the related feature.

bug#33258: inhibit-select-window

[martin rudalics]

 > This feature is intended only for user customization in display-buffer-alist,
 > not in the args of pop-to-buffer.  It makes no sense for a programmer
 > to use (inhibit-select-window . t) in the args of pop-to-buffer when
 > a programmer can simply replace pop-to-buffer with a display-buffer call.

Sure.  I nowhere assumed that the caller would add such an entry.

 > However, if an user is annoyed by always selecting a window of
 > a displayed buffer by default, and such customization doesn't
 > cause a bad side effect by not making the displayed buffer the current,
 > and this is true in 99% cases, then we should be proud to make
 > the users happier.

By providing 'pop-to-buffer' we signed a contract that if a program
calls it, it does "Display buffer specified by BUFFER-OR-NAME and
select its window".  This means that when 'pop-to-buffer' returns the
buffer, the calling program can be sure that a window showing that
buffer is selected and the buffer is current.  If a user is allowed to
override that behavior by keeping, for example, the previously
selected window selected and its buffer current, then 'pop-to-buffer'
would have violated the contract and we will get into troubles in due
time.

I think there's a simple way out.  The user provides an
'inhibit-select-window' request via a 'display-buffer-alist' (or
'display-buffer-base-action') entry as you suggested.
'display-buffer', by itself, honors that request iff the caller, in
the ACTION argument, has provided an 'allow-inhibit-select-window'
alist entry (much like your earlier 'allow-no-window' entries)
indicating its willingness to live with the fact that the window does
not get selected.

This way, any caller like 'vc-diff' can make provisions for the case
that the window is not selected.  If, as you say, 99% of the callers
are prepared for that case, there should not be any problems
converting most callers of 'pop-to-buffer' to include such an entry in
their action alists.  But we have more than 99 'pop-to-buffer' calls
in our sources and there are still more in the rest of this world.

 > force-select-window looks good, also possible is one of these names:
 >
 >    (pop-to-buffer . t)
 > or
 >    (pop-up-window . t)
 >
 > as a reference to the related feature.

We could convert 'pop-to-buffer' calls to 'display-buffer' calls with
a 'force-select-window' t alist entry.  Users could override that with
a 'force-select-window' nil entry.  This would be the contrapositive
of the 'inhibit-select-window' approach.

martin

bug#33258: inhibit-select-window

[Juri Linkov]

> By providing 'pop-to-buffer' we signed a contract that if a program
> calls it, it does "Display buffer specified by BUFFER-OR-NAME and
> select its window".  This means that when 'pop-to-buffer' returns the
> buffer, the calling program can be sure that a window showing that
> buffer is selected and the buffer is current.  If a user is allowed to
> override that behavior by keeping, for example, the previously
> selected window selected and its buffer current, then 'pop-to-buffer'
> would have violated the contract and we will get into troubles in due
> time.

You are right.  I think we can reliably restore an originally selected
window only at the end of the command, somewhere at the time when the
post-command-hook is called.

A prototype that demonstrates a problem-free implementation is:

(advice-add 'vc-diff :around
	    (lambda (orig-fun &rest args)
	      (save-selected-window
		(apply orig-fun args)))
	    '((name . inhibit-select-window)))

A question is how to do something like this using 'display-buffer-alist'
with 'inhibit-select-window'.

> I think there's a simple way out.  The user provides an
> 'inhibit-select-window' request via a 'display-buffer-alist' (or
> 'display-buffer-base-action') entry as you suggested.
> 'display-buffer', by itself, honors that request iff the caller, in
> the ACTION argument, has provided an 'allow-inhibit-select-window'
> alist entry (much like your earlier 'allow-no-window' entries)
> indicating its willingness to live with the fact that the window does
> not get selected.
>
> This way, any caller like 'vc-diff' can make provisions for the case
> that the window is not selected.  If, as you say, 99% of the callers
> are prepared for that case, there should not be any problems
> converting most callers of 'pop-to-buffer' to include such an entry in
> their action alists.  But we have more than 99 'pop-to-buffer' calls
> in our sources and there are still more in the rest of this world.

This is a huge task to inspect all existing pop-to-buffer calls
and to add 'allow-inhibit-select-window' where it's safe, or to change
the existing code to be safe (if this is possible at all).

>> force-select-window looks good, also possible is one of these names:
>>
>>    (pop-to-buffer . t)
>> or
>>    (pop-up-window . t)
>>
>> as a reference to the related feature.
>
> We could convert 'pop-to-buffer' calls to 'display-buffer' calls with
> a 'force-select-window' t alist entry.  Users could override that with
> a 'force-select-window' nil entry.  This would be the contrapositive
> of the 'inhibit-select-window' approach.

Does this have the same problem of not making the displayed buffer
current?

bug#33258: inhibit-select-window

[martin rudalics]

 > A prototype that demonstrates a problem-free implementation is:
 >
 > (advice-add 'vc-diff :around
 > 	    (lambda (orig-fun &rest args)
 > 	      (save-selected-window
 > 		(apply orig-fun args)))
 > 	    '((name . inhibit-select-window)))
 >
 > A question is how to do something like this using 'display-buffer-alist'
 > with 'inhibit-select-window'.

You cannot do that in a systematical way - 'display-buffer-alist' is
agnostic of its clients.  And don't forget clients of 'vc-diff' who
might rely on the fact that the latter selects the window chosen.
Ironically, they might do that for exactly the same reason as yours -
they don't want the diffs window to get selected.  Now suppose they do
that by selecting the window preceding or following the diffs window
in the cyclic order of windows right after the 'vc-diff' call ...  So
even your advice is not problem-free.

 > This is a huge task to inspect all existing pop-to-buffer calls
 > and to add 'allow-inhibit-select-window' where it's safe, or to change
 > the existing code to be safe (if this is possible at all).

Nobody will ever acquire enough knowledge to do that.

 >> We could convert 'pop-to-buffer' calls to 'display-buffer' calls with
 >> a 'force-select-window' t alist entry.  Users could override that with
 >> a 'force-select-window' nil entry.  This would be the contrapositive
 >> of the 'inhibit-select-window' approach.
 >
 > Does this have the same problem of not making the displayed buffer
 > current?

The task would remain the same: Inspect all 'pop-to-buffer' calls,
possibly their callers and decide whether converting them is safe.

In the 'vc-diff' case the following stretch already looks hairy:

       ;; Display the buffer, but at the end because it can change point.
       (pop-to-buffer (current-buffer))
       ;; The diff process may finish early, so call `vc-diff-finish'
       ;; after `pop-to-buffer'; the former assumes the diff buffer is
       ;; shown in some window.
       (let ((buf (current-buffer)))
         (vc-run-delayed (vc-diff-finish buf (when verbose messages))))

I suppose the 'buf' binding is superfluous because 'pop-to-buffer'
cannot "pop away" from the current buffer even if a user inhibits
selecting the window chosen.  But if that binding had a real purpose,
then using 'inhibit-select-window' would probably affect it and
'vc-diff-finish' would work on the wrong buffer.

Worse even.  The brutal 'set-buffer' call in 'vc-diff' (right before
entering 'diff-mode') already suggests that eventually selecting the
diffs window becomes unavoidable.  Otherwise, returning from 'vc-diff'
would leave you with the diffs buffer current and a window not showing
the diffs buffer selected - a very unpleasing configuration.

Replacing the 'set-buffer' with 'with-current-buffer' is not devoid of
its problems either: You cannot reliably nest a 'display-buffer' call
in 'with-current-buffer' because 'display-buffer' might select a
window on another frame at any time.

martin

bug#33258: inhibit-select-window

[Juri Linkov]

> You cannot do that in a systematical way - 'display-buffer-alist' is
> agnostic of its clients.  And don't forget clients of 'vc-diff' who
> might rely on the fact that the latter selects the window chosen.

So to rule out non-interactive callers of vc-diff we need to check
for (eq this-command 'vc-diff)?

But there is a simpler and safer solution that I already mentioned:
to restore an original window in post-command-hook.  Or even better
using your new window-state-change-functions: when inhibit-select-window
was found in an alist from display-buffer-alist then check whether
the selected window was changed in an alist of changes provided by
window-state-change-functions and restore an old selected window.

bug#33258: inhibit-select-window

[martin rudalics]

 > So to rule out non-interactive callers of vc-diff we need to check
 > for (eq this-command 'vc-diff)?

For example.

 > But there is a simpler and safer solution that I already mentioned:
 > to restore an original window in post-command-hook.  Or even better
 > using your new window-state-change-functions: when inhibit-select-window
 > was found in an alist from display-buffer-alist then check whether
 > the selected window was changed in an alist of changes provided by
 > window-state-change-functions and restore an old selected window.

In either case let's get 'display-buffer' out of this.  After all, its
doc-string says

   "Display BUFFER-OR-NAME in some window, without selecting it."

so we'd be preaching to the choir here.  We might call it
'pop-to-buffer-no-select'.

martin

bug#33258: inhibit-select-window

[Juri Linkov]

tags 33258 wontfix
close 33258
stop

>> But there is a simpler and safer solution that I already mentioned:
>> to restore an original window in post-command-hook.  Or even better
>> using your new window-state-change-functions: when inhibit-select-window
>> was found in an alist from display-buffer-alist then check whether
>> the selected window was changed in an alist of changes provided by
>> window-state-change-functions and restore an old selected window.
>
> In either case let's get 'display-buffer' out of this.  After all, its
> doc-string says
>
>   "Display BUFFER-OR-NAME in some window, without selecting it."

I see now that it's impossible to implement inhibit-select-window,
so I'm closing this feature request.

For bug#32790 I thought about adding inhibit-select-window to
display-buffer-overriding-action in windmove-display-in-direction,
but it was easier to do the same in post-command-hook.