Re: emacs-25 3722a69: Fix bugs in window resizing code

Re: emacs-25 3722a69: Fix bugs in window resizing code

[Óscar Fuentes]

Hello Martin.

Martin Rudalics <rudalics@gmx.at> writes:

> branch: emacs-25
> commit 3722a694fa094f88f7dfe54ccce7ea7cbaab214a
> Author: Martin Rudalics <rudalics@gmx.at>
> Commit: Martin Rudalics <rudalics@gmx.at>
>
>     Fix bugs in window resizing code
>     
>     * lisp/window.el (adjust-window-trailing-edge): Fix mismatched
>     parenthesis.
>     (shrink-window, enlarge-window): Fix bug#22723 where windows
>     with preserved size would not get resized.  Also now signal an
>     error when the window cannot be shrunk or enlarged as requested.

[snip]

> +      (error "Cannot shrink selected window")))))

I was surprised to see my code failing because of this change. Throwing
an error on a case where previously nothing was done amounts to an API
change. My code was fixed by wrapping the call to enlarge-window with
ignore-errors, but I'm worried about all the code out there.

Re: emacs-25 3722a69: Fix bugs in window resizing code

[martin rudalics]

 >>      (shrink-window, enlarge-window): Fix bug#22723 where windows
 >>      with preserved size would not get resized.  Also now signal an
 >>      error when the window cannot be shrunk or enlarged as requested.
 >
 > [snip]
 >
 >> +      (error "Cannot shrink selected window")))))
 >
 > I was surprised to see my code failing because of this change. Throwing
 > an error on a case where previously nothing was done amounts to an API
 > change. My code was fixed by wrapping the call to enlarge-window with
 > ignore-errors, but I'm worried about all the code out there.

Sorry, but this fix was motivated by the following explanation in the
Emacs manual:

       The command `C-x ^' (`enlarge-window') makes the selected window one
    line taller, taking space from a vertically adjacent window without
    changing the height of the frame.  With a positive numeric argument,
    this command increases the window height by that many lines; with a
    negative argument, it reduces the height by that many lines.  If there
    are no vertically adjacent windows (i.e., the window is at the full
    frame height), that signals an error.  The command also signals an
    error if you attempt to reduce the height of any window below a certain
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    minimum number of lines, specified by the variable `window-min-height'
    (the default is 4).

       Similarly, `C-x }' (`enlarge-window-horizontally') makes the
    selected window wider, and `C-x {' (`shrink-window-horizontally') makes
    it narrower.  These commands signal an error if you attempt to reduce
                                                        ^^^^^^^^^^^^^^^^^
    the width of any window below a certain minimum number of columns,
    ^^^^^^^^^^^^^^^^^^^^^^^
    specified by the variable `window-min-width' (the default is 10).

We can easily change that to not signal an error but then behaviors like
the one reported by Eli will get passed by unnoticed.  Emacsen < 23
silently could delete windows when you made them too small with these
commands.  This was deprecated but IIRC no one ever expressed a
preference as to how these functions _should_ behave when there is not
enough space.  Suggestions welcome.

Personally, I'd never use ‘enlarge-window’ and ‘shrink-window’ in Lisp
code.  IMO these are for interactive use only (supported by the fact
that neither of these functions is documented in the Elisp manual).

martin

Re: emacs-25 3722a69: Fix bugs in window resizing code

[Óscar Fuentes]

martin rudalics <rudalics@gmx.at> writes:

>>>      (shrink-window, enlarge-window): Fix bug#22723 where windows
>>>      with preserved size would not get resized.  Also now signal an
>>>      error when the window cannot be shrunk or enlarged as requested.
>>
>> [snip]
>>
>>> +      (error "Cannot shrink selected window")))))
>>
>> I was surprised to see my code failing because of this change. Throwing
>> an error on a case where previously nothing was done amounts to an API
>> change. My code was fixed by wrapping the call to enlarge-window with
>> ignore-errors, but I'm worried about all the code out there.
>
> Sorry, but this fix was motivated by the following explanation in the
> Emacs manual:
>
>       The command `C-x ^' (`enlarge-window') makes the selected window one
>    line taller, taking space from a vertically adjacent window without
>    changing the height of the frame.  With a positive numeric argument,
>    this command increases the window height by that many lines; with a
>    negative argument, it reduces the height by that many lines.  If there
>    are no vertically adjacent windows (i.e., the window is at the full
>    frame height), that signals an error.  The command also signals an
>    error if you attempt to reduce the height of any window below a certain
>                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>    minimum number of lines, specified by the variable `window-min-height'
>    (the default is 4).
>
>       Similarly, `C-x }' (`enlarge-window-horizontally') makes the
>    selected window wider, and `C-x {' (`shrink-window-horizontally') makes
>    it narrower.  These commands signal an error if you attempt to reduce
>                                                        ^^^^^^^^^^^^^^^^^
>    the width of any window below a certain minimum number of columns,
>    ^^^^^^^^^^^^^^^^^^^^^^^
>    specified by the variable `window-min-width' (the default is 10).

Please note that the above talks about reducing window dimensions below
a threshold. Also, it is about interactive use.

> We can easily change that to not signal an error but then behaviors like
> the one reported by Eli will get passed by unnoticed.  Emacsen < 23
> silently could delete windows when you made them too small with these
> commands.  This was deprecated but IIRC no one ever expressed a
> preference as to how these functions _should_ behave when there is not
> enough space.  Suggestions welcome.
>
> Personally, I'd never use ‘enlarge-window’ and ‘shrink-window’ in Lisp
> code.  IMO these are for interactive use only (supported by the fact
> that neither of these functions is documented in the Elisp manual).

`enlarge-window' appears on several places on the Elisp sources,
including calls like

textmodes/two-column.el:305:6:    (enlarge-window 99999 t))

which now will result on a error.

To be clear, I have nothing against throwing an error on the interactive
case, but against throwing the error on the non-interactive one. That's
what constitutes an API change on my book.

So I would suggest to remove the `error' and consider where and when to
add it after the release, after studying the code in the wild.

Re: emacs-25 3722a69: Fix bugs in window resizing code

[martin rudalics]

 >> Sorry, but this fix was motivated by the following explanation in the
 >> Emacs manual:
 >>
 >>        The command `C-x ^' (`enlarge-window') makes the selected window one
 >>     line taller, taking space from a vertically adjacent window without
 >>     changing the height of the frame.  With a positive numeric argument,
 >>     this command increases the window height by that many lines; with a
 >>     negative argument, it reduces the height by that many lines.  If there
 >>     are no vertically adjacent windows (i.e., the window is at the full
 >>     frame height), that signals an error.  The command also signals an
 >>     error if you attempt to reduce the height of any window below a certain
 >>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 >>     minimum number of lines, specified by the variable `window-min-height'
 >>     (the default is 4).
 >>
 >>        Similarly, `C-x }' (`enlarge-window-horizontally') makes the
 >>     selected window wider, and `C-x {' (`shrink-window-horizontally') makes
 >>     it narrower.  These commands signal an error if you attempt to reduce
 >>                                                         ^^^^^^^^^^^^^^^^^
 >>     the width of any window below a certain minimum number of columns,
 >>     ^^^^^^^^^^^^^^^^^^^^^^^
 >>     specified by the variable `window-min-width' (the default is 10).
 >
 > Please note that the above talks about reducing window dimensions below
 > a threshold.

I'm confused.  Isn't that what would happen in your case?  If you want
to enlarge one window you will always need to shrink at least one other
window.

 > Also, it is about interactive use.

We nowhere ever mention that these functions might behave differently in
interactive/non-interactive use.

 > `enlarge-window' appears on several places on the Elisp sources,
 > including calls like
 >
 > textmodes/two-column.el:305:6:    (enlarge-window 99999 t))
 >
 > which now will result on a error.

Ugly.  Earlier, with code like this you never knew whether the author's
intention was to maximize the window or to delete all other windows in
the same combination.  The author should either use ‘maximize-window’ or
`delete-other-windows-vertically' instead.  Please file a bug report.

 > To be clear, I have nothing against throwing an error on the interactive
 > case, but against throwing the error on the non-interactive one. That's
 > what constitutes an API change on my book.
 >
 > So I would suggest to remove the `error' and consider where and when to
 > add it after the release, after studying the code in the wild.

I can make any change people want but won't make any decision.  John,
Eli?

martin

Re: emacs-25 3722a69: Fix bugs in window resizing code

[Eli Zaretskii]

> Date: Tue, 01 Mar 2016 18:04:17 +0100
> From: martin rudalics <rudalics@gmx.at>
> Cc: emacs-devel@gnu.org
> 
>  > `enlarge-window' appears on several places on the Elisp sources,
>  > including calls like
>  >
>  > textmodes/two-column.el:305:6:    (enlarge-window 99999 t))
>  >
>  > which now will result on a error.
> 
> Ugly.  Earlier, with code like this you never knew whether the author's
> intention was to maximize the window or to delete all other windows in
> the same combination.  The author should either use ‘maximize-window’ or
> `delete-other-windows-vertically' instead.  Please file a bug report.
> 
>  > To be clear, I have nothing against throwing an error on the interactive
>  > case, but against throwing the error on the non-interactive one. That's
>  > what constitutes an API change on my book.
>  >
>  > So I would suggest to remove the `error' and consider where and when to
>  > add it after the release, after studying the code in the wild.
> 
> I can make any change people want but won't make any decision.  John,
> Eli?

MO is that we should fix all uses of enlarge-window and shrink-window
in our sources, and otherwise leave the change in 25.1.  Maybe also
document that in NEWS as potentially incompatible behavior.  I don't
think we should postpone the bugfix to 26.1, because I don't really
believe anything will happen until then that will change the
situation: the same problems this could cause now it will cause then.

Re: emacs-25 3722a69: Fix bugs in window resizing code

[martin rudalics]

 > MO is that we should fix all uses of enlarge-window and shrink-window
 > in our sources, and otherwise leave the change in 25.1.  Maybe also
 > document that in NEWS as potentially incompatible behavior.  I don't
 > think we should postpone the bugfix to 26.1, because I don't really
 > believe anything will happen until then that will change the
 > situation: the same problems this could cause now it will cause then.

The history of this is contrived: After changing the behavior of
‘enlarge-window’ to not delete other windows but signal an error
instead, jidanni filed bug#8862 "compilation-window-height no good
anymore".  In the discussion of that bug Stefan said that

    AFAIK, `enlarge-window' did not signal an error when the size was too
    large, so I think it's best to keep being silent, for backward
    compatibility's sake.

which is what I implemented in the sequel (without changing the
documentation in the Emacs manual, though).  Thereafter, Chong decided
to

2011-11-08  Chong Yidong  <cyd@gnu.org>

	* windows.texi
	(Resizing Windows): Simplify introduction.  Don't document
	enlarge-window, shrink-window, enlarge-window-horizontally, and
	shrink-window-horizontally; they are no longer preferred for
	calling from Lisp, and are already documented in the Emacs manual.

but IIRC no instance of ‘enlarge-window’ in the emacs sources was ever
removed.  In fact, it's not easy to do that for "anyone" because, for
example, the semantics of

textmodes/two-column.el:305:6:    (enlarge-window 99999 t))

mentioned by Óscar should be pretty unclear to anyone but the original
author.


In reaction to bug#22723 where Eli said that

    Moreover, doing nothing silently,
    without any error message, sounds sub-optimal UI to me.

I reinstalled the error reporting behavior as of before bug#8862 which
consequently lead to Óscar's reaction in the present thread.


I shortly recapitulate the possible solutions and their disadvantages:

(1) Don't make this function throw an error when a window cannot be made
as large as requested but try to make the window as large as possible.
This was the state before my last change.  Its major disadvantage is
that enlarging the window may fail silently which makes this differ from
most other window commands.  And, obviously no code can rely on this
function to behave predictably either.

(2) Have this function throw an error when a window cannot be made as
large as requested.  This is the present state.  Its major disadvantage
is that packages could rely on this function to never throw an error
except for a short period in 2011.  Even if we manage to fix all
occurrences in the emacs-25 code base, there might be still packages in
the wild that count on the "silent" behavior.

(3) Have this function throw an error in interactive use as in (1) and
behave as (2) in non-interactive use.  The disadvantage of this approach
is that, strictly spoken, we would have to document this function again
in the Elisp manual, describe its different behaviors and support it
until further notice.

We have to decide what to do for emacs-25.

martin

Re: emacs-25 3722a69: Fix bugs in window resizing code

[martin rudalics]

 > (3) Have this function throw an error in interactive use as in (1) and
 > behave as (2) in non-interactive use.  The disadvantage of this approach
 > is that, strictly spoken, we would have to document this function again
 > in the Elisp manual, describe its different behaviors and support it
 > until further notice.

And obviously this should have been the other way round:

(3) Have this function throw an error in interactive use as in (2) and
behave as (1) in non-interactive use.  The disadvantage of this approach
is that, strictly spoken, we would have to document this function again
in the Elisp manual, describe its different behaviors and support it
until further notice.

martin

Re: emacs-25 3722a69: Fix bugs in window resizing code

[Eli Zaretskii]

> Date: Wed, 02 Mar 2016 09:22:50 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: ofv@wanadoo.es, emacs-devel@gnu.org
> 
> (1) Don't make this function throw an error when a window cannot be made
> as large as requested but try to make the window as large as possible.
> This was the state before my last change.  Its major disadvantage is
> that enlarging the window may fail silently which makes this differ from
> most other window commands.  And, obviously no code can rely on this
> function to behave predictably either.
> 
> (2) Have this function throw an error when a window cannot be made as
> large as requested.  This is the present state.  Its major disadvantage
> is that packages could rely on this function to never throw an error
> except for a short period in 2011.  Even if we manage to fix all
> occurrences in the emacs-25 code base, there might be still packages in
> the wild that count on the "silent" behavior.
> 
> (3) Have this function throw an error in interactive use as in (1) and
> behave as (2) in non-interactive use.  The disadvantage of this approach
> is that, strictly spoken, we would have to document this function again
> in the Elisp manual, describe its different behaviors and support it
> until further notice.
> 
> We have to decide what to do for emacs-25.

Thanks for this clear summary.

If (3) can be done easily and reliably, I think it's the best
alternative.  That it might need documentation changes shouldn't
frighten us.

Re: emacs-25 3722a69: Fix bugs in window resizing code

[martin rudalics]

 > If (3) can be done easily and reliably, I think it's the best
 > alternative.

Done, more or less so.

 > That it might need documentation changes shouldn't
 > frighten us.

I left the documentation alone.  For master, I intend to add an
‘interactive-only’ declaration for the four involved functions.

martin

Re: emacs-25 3722a69: Fix bugs in window resizing code

[Eli Zaretskii]

> Date: Fri, 04 Mar 2016 08:48:36 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: ofv@wanadoo.es, emacs-devel@gnu.org
> 
>  > If (3) can be done easily and reliably, I think it's the best
>  > alternative.
> 
> Done, more or less so.

Thanks.