bug#51062: Customizable window resizing

bug#51062: Customizable window resizing

[Juri Linkov]

There are two ways how window resizing is currently hard-coded:

1. As discussed in
   https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg01627.html
   when temp-buffer-resize-mode is enabled, it's impossible
   to disable automatic window resizing of the Help buffer.

   Thanks to Martin, the patch in
   https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg01873.html
   allows customization when display-buffer-alist contains
   (window-height . nil)

2. As discussed in bug#45688, such commands as vc-log, vc-diff
   automatically resize the window, and the problem is that currently
   shrink-window-if-larger-than-buffer is used in these commands by default,
   but this behavior is hard-coded and can't be disabled by customization.
   A solution is to create a new hook like 'after-display-buffer-function'
   that by default will contain 'shrink-window-if-larger-than-buffer'.

bug#51062: Customizable window resizing

[martin rudalics]

 > There are two ways how window resizing is currently hard-coded:
 >
 > 1. As discussed in
 >     https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg01627.html
 >     when temp-buffer-resize-mode is enabled, it's impossible
 >     to disable automatic window resizing of the Help buffer.
 >
 >     Thanks to Martin, the patch in
 >     https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg01873.html
 >     allows customization when display-buffer-alist contains
 >     (window-height . nil)

That patch should help wherever 'temp-buffer-resize-mode' applies, not
just in a Help buffer.

 > 2. As discussed in bug#45688, such commands as vc-log

Which command is 'vc-log'?

 >     , vc-diff
 >     automatically resize the window, and the problem is that currently
 >     shrink-window-if-larger-than-buffer is used in these commands by default,
 >     but this behavior is hard-coded and can't be disabled by customization.
 >     A solution is to create a new hook like 'after-display-buffer-function'
 >     that by default will contain 'shrink-window-if-larger-than-buffer'.

Wherever we currently use an idiom like

(pop-to-buffer (current-buffer))
(goto-char (point-min))
(shrink-window-if-larger-than-buffer)

we should use

(pop-to-buffer
  (current-buffer)
  '(nil (window-height . shrink-window-if-larger-than-buffer)))
(goto-char (point-min))

instead.  But if 'display-buffer' is not involved, there isn't much we
can do.  Maybe something like 'display-buffer-same-window' _after_
putting the buffer into the selected window, but this might not be 100%
compatible.  Certainly not for Emacs 28.

martin

bug#51062: Customizable window resizing

[martin rudalics]

 >     Thanks to Martin, the patch in
 >     https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg01873.html
 >     allows customization when display-buffer-alist contains
 >     (window-height . nil)

This patch has been meanwhile applied on master.

martin

bug#51062: Customizable window resizing

[Juri Linkov]

>> There are two ways how window resizing is currently hard-coded:
>>
>> 1. As discussed in
>>     https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg01627.html
>>     when temp-buffer-resize-mode is enabled, it's impossible
>>     to disable automatic window resizing of the Help buffer.
>>
>>     Thanks to Martin, the patch in
>>     https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg01873.html
>>     allows customization when display-buffer-alist contains
>>     (window-height . nil)
>
> That patch should help wherever 'temp-buffer-resize-mode' applies, not
> just in a Help buffer.
>
> This patch has been meanwhile applied on master.

Strange, when I tested the patch that you sent to emacs-devel,
it worked correctly.  But after you applied the patch on master,
it doesn't work.  I'm trying to debug why (window-height . nil)
has no effect on Help buffers.

>> 2. As discussed in bug#45688, such commands as vc-log
>
> Which command is 'vc-log'?

I meant log-related vc-commands that use
shrink-window-if-larger-than-buffer, i.e.
all commands that use vc-log-internal-common.

>>     , vc-diff
>>     automatically resize the window, and the problem is that currently
>>     shrink-window-if-larger-than-buffer is used in these commands by default,
>>     but this behavior is hard-coded and can't be disabled by customization.
>>     A solution is to create a new hook like 'after-display-buffer-function'
>>     that by default will contain 'shrink-window-if-larger-than-buffer'.
>
> Wherever we currently use an idiom like
>
> (pop-to-buffer (current-buffer))
> (goto-char (point-min))
> (shrink-window-if-larger-than-buffer)
>
> we should use
>
> (pop-to-buffer
>  (current-buffer)
>  '(nil (window-height . shrink-window-if-larger-than-buffer)))
> (goto-char (point-min))
>
> instead.  But if 'display-buffer' is not involved, there isn't much we
> can do.  Maybe something like 'display-buffer-same-window' _after_
> putting the buffer into the selected window, but this might not be 100%
> compatible.  Certainly not for Emacs 28.

This is a good idea to try.  But not sure how this is possible to use in
vc-run-delayed that is called when the buffer was already displayed long ago.

bug#51062: Customizable window resizing

[martin rudalics]

 > Strange, when I tested the patch that you sent to emacs-devel,
 > it worked correctly.  But after you applied the patch on master,
 > it doesn't work.  I'm trying to debug why (window-height . nil)
 > has no effect on Help buffers.

Indeed.  I probably tried to optimize it before pushing.  Should be
fixed now.

 >> Which command is 'vc-log'?
 >
 > I meant log-related vc-commands that use
 > shrink-window-if-larger-than-buffer, i.e.
 > all commands that use vc-log-internal-common.

I see.

 >> But if 'display-buffer' is not involved, there isn't much we
 >> can do.  Maybe something like 'display-buffer-same-window' _after_
 >> putting the buffer into the selected window, but this might not be 100%
 >> compatible.  Certainly not for Emacs 28.
 >
 > This is a good idea to try.  But not sure how this is possible to use in
 > vc-run-delayed that is called when the buffer was already displayed long ago.

The problem is rather that 'display-buffer' wasn't used in the first
place to show the buffer.  Two things we could try to cover buffers that
are shown via 'set-window-buffer' are

- Invent a new buffer-local variable, say 'inhibit-temp-buffer-resize'
   with the obvious meaning.

- Invent a new window parameter 'inhibit-temp-buffer-resize' with the
   obvious meaning.

martin

bug#51062: Customizable window resizing

[Juri Linkov]

>> Strange, when I tested the patch that you sent to emacs-devel,
>> it worked correctly.  But after you applied the patch on master,
>> it doesn't work.  I'm trying to debug why (window-height . nil)
>> has no effect on Help buffers.
>
> Indeed.  I probably tried to optimize it before pushing.  Should be
> fixed now.

Thanks, I confirm it's working now.

>>> But if 'display-buffer' is not involved, there isn't much we
>>> can do.  Maybe something like 'display-buffer-same-window' _after_
>>> putting the buffer into the selected window, but this might not be 100%
>>> compatible.  Certainly not for Emacs 28.
>>
>> This is a good idea to try.  But not sure how this is possible to use in
>> vc-run-delayed that is called when the buffer was already displayed long ago.
>
> The problem is rather that 'display-buffer' wasn't used in the first
> place to show the buffer.  Two things we could try to cover buffers that
> are shown via 'set-window-buffer' are
>
> - Invent a new buffer-local variable, say 'inhibit-temp-buffer-resize'
>   with the obvious meaning.
>
> - Invent a new window parameter 'inhibit-temp-buffer-resize' with the
>   obvious meaning.

Earlier I proposed to add a new hook whose default value contains
shrink-window-if-larger-than-buffer, so it can easily be removed from the hook.

bug#51062: Customizable window resizing

[martin rudalics]

 >> The problem is rather that 'display-buffer' wasn't used in the first
 >> place to show the buffer.  Two things we could try to cover buffers that
 >> are shown via 'set-window-buffer' are
 >>
 >> - Invent a new buffer-local variable, say 'inhibit-temp-buffer-resize'
 >>    with the obvious meaning.
 >>
 >> - Invent a new window parameter 'inhibit-temp-buffer-resize' with the
 >>    obvious meaning.
 >
 > Earlier I proposed to add a new hook whose default value contains
 > shrink-window-if-larger-than-buffer, so it can easily be removed from the hook.

What was that hook called?  I think we first have to decide whether we
want to inhibit such shrinks and fits for any display operation or only
those administrated by 'display-buffer'.  Then we'd either install that
inhibiting thing via 'window--display-buffer' (where we work on alist so
we can decide who overrides whom) or directly in 'fit-window-to-buffer'
(where we cannot discriminate any more whether an application very
urgently wants to impose its behavior and we always rely on the user's
setting).

martin

bug#51062: Customizable window resizing

[Juri Linkov]

>>> The problem is rather that 'display-buffer' wasn't used in the first
>>> place to show the buffer.  Two things we could try to cover buffers that
>>> are shown via 'set-window-buffer' are
>>>
>>> - Invent a new buffer-local variable, say 'inhibit-temp-buffer-resize'
>>>    with the obvious meaning.
>>>
>>> - Invent a new window parameter 'inhibit-temp-buffer-resize' with the
>>>    obvious meaning.
>>
>> Earlier I proposed to add a new hook whose default value contains
>> shrink-window-if-larger-than-buffer, so it can easily be removed from the hook.
>
> What was that hook called?  I think we first have to decide whether we
> want to inhibit such shrinks and fits for any display operation or only
> those administrated by 'display-buffer'.  Then we'd either install that
> inhibiting thing via 'window--display-buffer' (where we work on alist so
> we can decide who overrides whom) or directly in 'fit-window-to-buffer'
> (where we cannot discriminate any more whether an application very
> urgently wants to impose its behavior and we always rely on the user's
> setting).

This hook was called 'after-display-buffer-function' in
https://debbugs.gnu.org/45688#137

bug#51062: Customizable window resizing

[martin rudalics]

 > This hook was called 'after-display-buffer-function' in
 > https://debbugs.gnu.org/45688#137

But what would that accomplish when an application does

(setq window (display-buffer foo))
[...]
(shrink-window-if-larger-than-buffer window)

The 'shrink-window-if-larger-than-buffer' can be inhibited only if the
hook sets something for FOO or WINDOW that inhibits shrinking it.  And
that can be easily accomplished by a 'window-height' alist entry.

OTOH with

(setq window (split-window))
[...]
(shrink-window-if-larger-than-buffer window)

your hook (and the alist entry) cannot inhibit anything.

martin

bug#51062: Customizable window resizing

[Juri Linkov]

forcemerge 36859 51062
stop

>> What was that hook called?
>
> This hook was called 'after-display-buffer-function' in
> https://debbugs.gnu.org/45688#137

Actually, the right name was 'vc-post-process-functions'
proposed in bug#36859 (merged).

bug#51062: Customizable window resizing

[martin rudalics]

 > Actually, the right name was 'vc-post-process-functions'
 > proposed in bug#36859 (merged).

You mean to run 'shrink-window-if-larger-than-buffer' from that hook
unless the user removed it?

martin

bug#51062: Customizable window resizing

[Juri Linkov]

>> Actually, the right name was 'vc-post-process-functions'
>> proposed in bug#36859 (merged).
>
> You mean to run 'shrink-window-if-larger-than-buffer' from that hook
> unless the user removed it?

Yep.

bug#51062: Customizable window resizing

[Juri Linkov]

forcemerge 51062 52855
stop

>> Actually, the right name was 'vc-post-process-functions'
>> proposed in bug#36859 (merged).
>
> You mean to run 'shrink-window-if-larger-than-buffer' from that hook
> unless the user removed it?

The problem is that the argument of this function should be a buffer,
not a window as in 'shrink-window-if-larger-than-buffer'.
This is why bug#52855 proposed to add a new function

  shrink-buffer-if-larger-than-buffer

But not sure if this should be added to window.el,
or better to leave in vc.el with the name vc-shrink-buffer?

bug#51062: [External] : bug#51062: Customizable window resizing

[Drew Adams]

> The problem is that the argument of this function should be a buffer,
> not a window as in 'shrink-window-if-larger-than-buffer'.
> This is why bug#52855 proposed to add a new function
> 
>   shrink-buffer-if-larger-than-buffer

Apologies for not following this thread in detail.

But is this really about shrinking a _buffer_?
What does that even mean?

Same question for `vc-shrink-buffer', I guess.

(I don't see that function in Emacs 27.2.  If it
isn't in an Emacs release, and if it's not about
shrinking a buffer, then please consider renaming it.)

bug#51062: Customizable window resizing

[martin rudalics]

 >>> Actually, the right name was 'vc-post-process-functions'
 >>> proposed in bug#36859 (merged).
 >>
 >> You mean to run 'shrink-window-if-larger-than-buffer' from that hook
 >> unless the user removed it?
 >
 > The problem is that the argument of this function should be a buffer,
 > not a window as in 'shrink-window-if-larger-than-buffer'.
 > This is why bug#52855 proposed to add a new function
 >
 >    shrink-buffer-if-larger-than-buffer
 >
 > But not sure if this should be added to window.el,
 > or better to leave in vc.el with the name vc-shrink-buffer?

"shrink-buffer-if-larger-than-buffer"?  Maybe this should become
'shrink-buffer-window-if-larger-than-buffer' then.  With the usual
problem what to do when there are two windows showing that buffer.

martin

bug#51062: [External] : bug#51062: Customizable window resizing

[Drew Adams]

> "shrink-buffer-if-larger-than-buffer"?  Maybe this should
> become 'shrink-buffer-window-if-larger-than-buffer' then.

That was what I asked also:

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=51062#48

(Similar question for `vc-shrink-buffer'.)

bug#51062: Customizable window resizing

[Juri Linkov]

>>> You mean to run 'shrink-window-if-larger-than-buffer' from that hook
>>> unless the user removed it?
>>
>> The problem is that the argument of this function should be a buffer,
>> not a window as in 'shrink-window-if-larger-than-buffer'.
>> This is why bug#52855 proposed to add a new function
>>
>>    shrink-buffer-if-larger-than-buffer
>>
>> But not sure if this should be added to window.el,
>> or better to leave in vc.el with the name vc-shrink-buffer?
>
> "shrink-buffer-if-larger-than-buffer"?  Maybe this should become
> 'shrink-buffer-window-if-larger-than-buffer' then.

Should such a function be added to window.el?

> With the usual problem what to do when there are two windows showing
> that buffer.

It's understandable why vc-diff-internal and vc-diff-finish
use get-buffer-window to get a window:

vc-diff-internal:
      (pop-to-buffer (current-buffer))
      (let ((buf (current-buffer)))
        (vc-run-delayed (vc-diff-finish buf ...

vc-diff-finish:
  (let ((window (get-buffer-window buffer t)))
        (with-current-buffer buffer
          ...
          (when window
            (shrink-window-if-larger-than-buffer window))))

This is needed to handle the case when the window was deleted
before the vc command finished.

But why vc-log-internal-common doesn't so the same?

vc-log-internal-common:
    (pop-to-buffer buffer)
    (vc-run-delayed
      ...
      (shrink-window-if-larger-than-buffer)

So I posted a patch in bug#52855 to use the same function
that relies on get-buffer-window in both cases.

But it seems you propose to remember the window with e.g.

  (setq window (pop-to-buffer (current-buffer)))

then to use it in:

  (run-hook-with-args 'vc-diff-finish-functions window)

where the defalut value of 'vc-diff-finish-functions' is:

  '(shrink-window-if-larger-than-buffer)

Then 'shrink-buffer-window-if-larger-than-buffer' is not needed.

bug#51062: Customizable window resizing

[martin rudalics]

 >> "shrink-buffer-if-larger-than-buffer"?  Maybe this should become
 >> 'shrink-buffer-window-if-larger-than-buffer' then.
 >
 > Should such a function be added to window.el?

Maybe.  But we could also rename the first argument of
'shrink-window-if-larger-than-buffer' to WINDOW-OR-BUFFER.

 >> With the usual problem what to do when there are two windows showing
 >> that buffer.
 >
 > It's understandable why vc-diff-internal and vc-diff-finish
 > use get-buffer-window to get a window:
 >
 > vc-diff-internal:
 >        (pop-to-buffer (current-buffer))
 >        (let ((buf (current-buffer)))
 >          (vc-run-delayed (vc-diff-finish buf ...
 >
 > vc-diff-finish:
 >    (let ((window (get-buffer-window buffer t)))
 >          (with-current-buffer buffer
 >            ...
 >            (when window
 >              (shrink-window-if-larger-than-buffer window))))
 >
 > This is needed to handle the case when the window was deleted
 > before the vc command finished.
 >
 > But why vc-log-internal-common doesn't so the same?
 >
 > vc-log-internal-common:
 >      (pop-to-buffer buffer)
 >      (vc-run-delayed
 >        ...
 >        (shrink-window-if-larger-than-buffer)
 >
 > So I posted a patch in bug#52855 to use the same function
 > that relies on get-buffer-window in both cases.
 >
 > But it seems you propose to remember the window with e.g.
 >
 >    (setq window (pop-to-buffer (current-buffer)))
 >
 > then to use it in:
 >
 >    (run-hook-with-args 'vc-diff-finish-functions window)
 >
 > where the defalut value of 'vc-diff-finish-functions' is:
 >
 >    '(shrink-window-if-larger-than-buffer)
 >
 > Then 'shrink-buffer-window-if-larger-than-buffer' is not needed.

Personally I dislike abusing hooks like that.  But I probably do not use
vc-... much either.  So do whatever you like more.

martin

bug#51062: Customizable window resizing

[Juri Linkov]

close 51062 29.0.50
thanks

>>> "shrink-buffer-if-larger-than-buffer"?  Maybe this should become
>>> 'shrink-buffer-window-if-larger-than-buffer' then.
>>
>> Should such a function be added to window.el?
>
> Maybe.  But we could also rename the first argument of
> 'shrink-window-if-larger-than-buffer' to WINDOW-OR-BUFFER.

This can't be generalized because vc uses specific logic:

    (let ((window (get-buffer-window buffer t)))
          (with-current-buffer buffer
            ...
            (when window
              (shrink-window-if-larger-than-buffer window))))

>>> With the usual problem what to do when there are two windows showing
>>> that buffer.
>>
>> It's understandable why vc-diff-internal and vc-diff-finish
>> use get-buffer-window to get a window:
>>
>> vc-diff-internal:
>>        (pop-to-buffer (current-buffer))
>>        (let ((buf (current-buffer)))
>>          (vc-run-delayed (vc-diff-finish buf ...
>>
>> vc-diff-finish:
>>    (let ((window (get-buffer-window buffer t)))
>>          (with-current-buffer buffer
>>            ...
>>            (when window
>>              (shrink-window-if-larger-than-buffer window))))
>>
>> This is needed to handle the case when the window was deleted
>> before the vc command finished.
>>
>> But why vc-log-internal-common doesn't so the same?
>>
>> vc-log-internal-common:
>>      (pop-to-buffer buffer)
>>      (vc-run-delayed
>>        ...
>>        (shrink-window-if-larger-than-buffer)
>>
>> So I posted a patch in bug#52855 to use the same function
>> that relies on get-buffer-window in both cases.
>>
>> But it seems you propose to remember the window with e.g.
>>
>>    (setq window (pop-to-buffer (current-buffer)))
>>
>> then to use it in:
>>
>>    (run-hook-with-args 'vc-diff-finish-functions window)
>>
>> where the defalut value of 'vc-diff-finish-functions' is:
>>
>>    '(shrink-window-if-larger-than-buffer)
>>
>> Then 'shrink-buffer-window-if-larger-than-buffer' is not needed.
>
> Personally I dislike abusing hooks like that.  But I probably do not use
> vc-... much either.

And it would require a weird condition:

  (when window
    (run-hook-with-args 'vc-diff-finish-functions window))

> So do whatever you like more.

So instead of above, I pushed the original patch with a better function name
'vc-shrink-buffer-window'.