bug#33254: 26.1.50; Fixed-size windows break 'balance-windows'

bug#33254: 26.1.50; Fixed-size windows break 'balance-windows'

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 497 bytes --]

I have been informed about the existence of this bug by Akio Burns.
Unfortunately, he has not reacted so far to my request to file a bug
report so I'm sloppily doing this here instead:

With emacs -Q type C-x 2 C-x 2 C-x b RET

This should put you in the *Messages* buffer.  Now do

M-: (setq window-size-fixed t)

followed by

M-: (balance-windows)

Window sizes remain unchanged.  The attached patch should fix that.  I
would like to apply it to the release branch.  Objections?

Thanks, martin

[-- Attachment #2: balance-windows.diff --]
[-- Type: text/plain, Size: 790 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index f96c887..03e0c96 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5305,11 +5305,12 @@ balance-windows-2
 	 (total-sum parent-size)
 	 failed size sub-total sub-delta sub-amount rest)
     (while sub
-      (setq number-of-children (1+ number-of-children))
-      (when (window-size-fixed-p sub horizontal)
-	(setq total-sum
-	      (- total-sum (window-size sub horizontal t)))
-	(set-window-new-normal sub 'ignore))
+      (if (window-size-fixed-p sub horizontal)
+          (progn
+            (setq total-sum
+	          (- total-sum (window-size sub horizontal t)))
+	    (set-window-new-normal sub 'ignore))
+        (setq number-of-children (1+ number-of-children)))
       (setq sub (window-right sub)))
 
     (setq failed t)

bug#33254: 26.1.50; Fixed-size windows break 'balance-windows'

[Eli Zaretskii]

> Date: Sun, 04 Nov 2018 11:18:02 +0100
> From: martin rudalics <rudalics@gmx.at>
> 
> With emacs -Q type C-x 2 C-x 2 C-x b RET
> 
> This should put you in the *Messages* buffer.  Now do
> 
> M-: (setq window-size-fixed t)
> 
> followed by
> 
> M-: (balance-windows)
> 
> Window sizes remain unchanged.  The attached patch should fix that.  I
> would like to apply it to the release branch.  Objections?

Rationale?  Isn't this behavior in Emacs since about forever?

bug#33254: 26.1.50; Fixed-size windows break 'balance-windows'

[martin rudalics]

 >> With emacs -Q type C-x 2 C-x 2 C-x b RET
 >>
 >> This should put you in the *Messages* buffer.  Now do
 >>
 >> M-: (setq window-size-fixed t)
 >>
 >> followed by
 >>
 >> M-: (balance-windows)
 >>
 >> Window sizes remain unchanged.  The attached patch should fix that.  I
 >> would like to apply it to the release branch.  Objections?
 >
 > Rationale?  Isn't this behavior in Emacs since about forever?

No.  It works with Emacs 23.

martin

bug#33254: 26.1.50; Fixed-size windows break 'balance-windows'

[Eli Zaretskii]

> Date: Sun, 04 Nov 2018 14:32:23 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: 33254@debbugs.gnu.org
> 
>  >> With emacs -Q type C-x 2 C-x 2 C-x b RET
>  >>
>  >> This should put you in the *Messages* buffer.  Now do
>  >>
>  >> M-: (setq window-size-fixed t)
>  >>
>  >> followed by
>  >>
>  >> M-: (balance-windows)
>  >>
>  >> Window sizes remain unchanged.  The attached patch should fix that.  I
>  >> would like to apply it to the release branch.  Objections?
>  >
>  > Rationale?  Isn't this behavior in Emacs since about forever?
> 
> No.  It works with Emacs 23.

Still, that's a very long time.  Why is it urgent to fix this in 26.2?

bug#33254: 26.1.50; Fixed-size windows break 'balance-windows'

[martin rudalics]

 >> No.  It works with Emacs 23.
 >
 > Still, that's a very long time.  Why is it urgent to fix this in 26.2?

It's not the visible behavior that's disquieting me.  It's rather the
fact that 'balance-windows-1' goes on calculating all sorts of things
and finally 'balance-windows' sumbits a wrongly calculated structure
to 'window-resize-apply' which only gets caught by window_resize_check
at a very, very late stage.  Ideally, window_resize_check would emit a
warning in such a case, but I never got around to write it.

I think the bug happens to users of 'balance-windows' all the time but
they hardly notice because for that it needs at least three windows,
one of them of fixed size.  With my example it's barely noticeable at
all.  Also it's a plain thinko and probably not that urgent so I can
make the change on Emacs 27 only as well.

martin

bug#33254: 26.1.50; Fixed-size windows break 'balance-windows'

[Eli Zaretskii]

> Date: Sun, 04 Nov 2018 16:46:31 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: 33254@debbugs.gnu.org
> 
>  >> No.  It works with Emacs 23.
>  >
>  > Still, that's a very long time.  Why is it urgent to fix this in 26.2?
> 
> It's not the visible behavior that's disquieting me.  It's rather the
> fact that 'balance-windows-1' goes on calculating all sorts of things
> and finally 'balance-windows' sumbits a wrongly calculated structure
> to 'window-resize-apply' which only gets caught by window_resize_check
> at a very, very late stage.  Ideally, window_resize_check would emit a
> warning in such a case, but I never got around to write it.
> 
> I think the bug happens to users of 'balance-windows' all the time but
> they hardly notice because for that it needs at least three windows,
> one of them of fixed size.  With my example it's barely noticeable at
> all.  Also it's a plain thinko and probably not that urgent so I can
> make the change on Emacs 27 only as well.

OK, let's have it one emacs-26.

bug#33254: 26.1.50; Fixed-size windows break 'balance-windows'

[martin rudalics]

 > OK, let's have it one emacs-26.

Thinking about it twice ...

This bug was hiding another one which is already mentioned in a
comment as

     ;; How can we be sure that `number-of-children' is NOT zero here ?

and in fact making all windows fixed-size would make 'balance-windows'
fail with an arithmetic error.  So the fix is slightly more involved
and I'd rather save that one for master.

Thanks, martin

bug#33254: 26.1.50; Fixed-size windows break 'balance-windows'

[Eli Zaretskii]

> Date: Mon, 05 Nov 2018 10:33:30 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: 33254@debbugs.gnu.org
> 
> This bug was hiding another one which is already mentioned in a
> comment as
> 
>      ;; How can we be sure that `number-of-children' is NOT zero here ?
> 
> and in fact making all windows fixed-size would make 'balance-windows'
> fail with an arithmetic error.  So the fix is slightly more involved
> and I'd rather save that one for master.

I obviously agree.

Thanks.

bug#33254: 26.1.50; Fixed-size windows break 'balance-windows'

[martin rudalics]

;; fixed 33254 27.1
;; quit

 > So the fix is slightly more involved
 > and I'd rather save that one for master.

Installed on master.  Bug closed.

martin