unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: emacs-25 3722a69: Fix bugs in window resizing code
       [not found] ` <E1aWLnn-000448-7K@vcs.savannah.gnu.org>
@ 2016-03-01  1:31   ` Óscar Fuentes
  2016-03-01  8:07     ` martin rudalics
  0 siblings, 1 reply; 10+ messages in thread
From: Óscar Fuentes @ 2016-03-01  1:31 UTC (permalink / raw)
  To: emacs-devel; +Cc: Martin Rudalics

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.



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

* Re: emacs-25 3722a69: Fix bugs in window resizing code
  2016-03-01  1:31   ` emacs-25 3722a69: Fix bugs in window resizing code Óscar Fuentes
@ 2016-03-01  8:07     ` martin rudalics
  2016-03-01 15:10       ` Óscar Fuentes
  0 siblings, 1 reply; 10+ messages in thread
From: martin rudalics @ 2016-03-01  8:07 UTC (permalink / raw)
  To: Óscar Fuentes, emacs-devel

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




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

* Re: emacs-25 3722a69: Fix bugs in window resizing code
  2016-03-01  8:07     ` martin rudalics
@ 2016-03-01 15:10       ` Óscar Fuentes
  2016-03-01 17:04         ` martin rudalics
  0 siblings, 1 reply; 10+ messages in thread
From: Óscar Fuentes @ 2016-03-01 15:10 UTC (permalink / raw)
  To: martin rudalics; +Cc: emacs-devel

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.



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

* Re: emacs-25 3722a69: Fix bugs in window resizing code
  2016-03-01 15:10       ` Óscar Fuentes
@ 2016-03-01 17:04         ` martin rudalics
  2016-03-01 17:20           ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: martin rudalics @ 2016-03-01 17:04 UTC (permalink / raw)
  To: Óscar Fuentes; +Cc: emacs-devel

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




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

* Re: emacs-25 3722a69: Fix bugs in window resizing code
  2016-03-01 17:04         ` martin rudalics
@ 2016-03-01 17:20           ` Eli Zaretskii
  2016-03-02  8:22             ` martin rudalics
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2016-03-01 17:20 UTC (permalink / raw)
  To: martin rudalics; +Cc: ofv, emacs-devel

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



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

* Re: emacs-25 3722a69: Fix bugs in window resizing code
  2016-03-01 17:20           ` Eli Zaretskii
@ 2016-03-02  8:22             ` martin rudalics
  2016-03-02  9:28               ` martin rudalics
  2016-03-02 15:54               ` Eli Zaretskii
  0 siblings, 2 replies; 10+ messages in thread
From: martin rudalics @ 2016-03-02  8:22 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: ofv, emacs-devel

 > 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




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

* Re: emacs-25 3722a69: Fix bugs in window resizing code
  2016-03-02  8:22             ` martin rudalics
@ 2016-03-02  9:28               ` martin rudalics
  2016-03-02 15:54               ` Eli Zaretskii
  1 sibling, 0 replies; 10+ messages in thread
From: martin rudalics @ 2016-03-02  9:28 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: ofv, emacs-devel

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



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

* Re: emacs-25 3722a69: Fix bugs in window resizing code
  2016-03-02  8:22             ` martin rudalics
  2016-03-02  9:28               ` martin rudalics
@ 2016-03-02 15:54               ` Eli Zaretskii
  2016-03-04  7:48                 ` martin rudalics
  1 sibling, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2016-03-02 15:54 UTC (permalink / raw)
  To: martin rudalics; +Cc: ofv, emacs-devel

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



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

* Re: emacs-25 3722a69: Fix bugs in window resizing code
  2016-03-02 15:54               ` Eli Zaretskii
@ 2016-03-04  7:48                 ` martin rudalics
  2016-03-04  8:28                   ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: martin rudalics @ 2016-03-04  7:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: ofv, emacs-devel

 > 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




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

* Re: emacs-25 3722a69: Fix bugs in window resizing code
  2016-03-04  7:48                 ` martin rudalics
@ 2016-03-04  8:28                   ` Eli Zaretskii
  0 siblings, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2016-03-04  8:28 UTC (permalink / raw)
  To: martin rudalics; +Cc: ofv, emacs-devel

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



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

end of thread, other threads:[~2016-03-04  8:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20160218102715.15576.42604@vcs.savannah.gnu.org>
     [not found] ` <E1aWLnn-000448-7K@vcs.savannah.gnu.org>
2016-03-01  1:31   ` emacs-25 3722a69: Fix bugs in window resizing code Óscar Fuentes
2016-03-01  8:07     ` martin rudalics
2016-03-01 15:10       ` Óscar Fuentes
2016-03-01 17:04         ` martin rudalics
2016-03-01 17:20           ` Eli Zaretskii
2016-03-02  8:22             ` martin rudalics
2016-03-02  9:28               ` martin rudalics
2016-03-02 15:54               ` Eli Zaretskii
2016-03-04  7:48                 ` martin rudalics
2016-03-04  8:28                   ` Eli Zaretskii

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