bug#70894: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix width calculation

bug#70894: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix width calculation

[Morgan Smith]

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

Tags: patch

Hello!

I was playing around with `fit-window-to-buffer' after setting
`fit-window-to-buffer-horizontally' to `t'.  I noticed that it kept
setting the window too narrow.  After a quick investigation, I found
some suspicious looking calculation that mixed pixel and column units.

Interactively, it looks like this fixes the issue.

Thanks,

Morgan



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-lisp-window.el-fit-window-to-buffer-Fix-width-calcul.patch --]
[-- Type: text/patch, Size: 1205 bytes --]

From f906b1a219ae7c9ec1374edf6e02b46b845ab776 Mon Sep 17 00:00:00 2001
From: Morgan Smith <Morgan.J.Smith@outlook.com>
Date: Sun, 12 May 2024 09:19:30 -0400
Subject: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix width
 calculation

When pixelwise is nil, we still calculate width in pixels and
then convert it to columns.  However, part of the calculation
was using columns where it should have used pixels.
---
 lisp/window.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/window.el b/lisp/window.el
index 639090752be..f03996fb682 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -9906,8 +9906,8 @@ fit-window-to-buffer
 			       ;; the bottom is wider than the window.
 			       (* (window-body-height window pixelwise)
 				  (if pixelwise 1 char-height))))
-                         (- total-width
-                            (window-body-width window pixelwise)))))
+                         (- (* total-width (if pixelwise 1 char-width))
+                            (window-body-width window t)))))
 	  (unless pixelwise
 	    (setq width (/ (+ width char-width -1) char-width)))
           (setq width (max min-width (min max-width width)))
-- 
2.41.0

bug#70894: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix width calculation

[Eli Zaretskii]

> From: Morgan Smith <Morgan.J.Smith@outlook.com>
> Date: Sun, 12 May 2024 09:25:31 -0400
> 
> I was playing around with `fit-window-to-buffer' after setting
> `fit-window-to-buffer-horizontally' to `t'.  I noticed that it kept
> setting the window too narrow.  After a quick investigation, I found
> some suspicious looking calculation that mixed pixel and column units.
> 
> Interactively, it looks like this fixes the issue.

Thanks.

Martin, any comments?

> >From f906b1a219ae7c9ec1374edf6e02b46b845ab776 Mon Sep 17 00:00:00 2001
> From: Morgan Smith <Morgan.J.Smith@outlook.com>
> Date: Sun, 12 May 2024 09:19:30 -0400
> Subject: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix width
>  calculation
> 
> When pixelwise is nil, we still calculate width in pixels and
> then convert it to columns.  However, part of the calculation
> was using columns where it should have used pixels.
> ---
>  lisp/window.el | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lisp/window.el b/lisp/window.el
> index 639090752be..f03996fb682 100644
> --- a/lisp/window.el
> +++ b/lisp/window.el
> @@ -9906,8 +9906,8 @@ fit-window-to-buffer
>  			       ;; the bottom is wider than the window.
>  			       (* (window-body-height window pixelwise)
>  				  (if pixelwise 1 char-height))))
> -                         (- total-width
> -                            (window-body-width window pixelwise)))))
> +                         (- (* total-width (if pixelwise 1 char-width))
> +                            (window-body-width window t)))))
>  	  (unless pixelwise
>  	    (setq width (/ (+ width char-width -1) char-width)))
>            (setq width (max min-width (min max-width width)))
> -- 
> 2.41.0
> 

bug#70894: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix width calculation

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 > Martin, any comments?

Looks good to me.  Principally, instead of subtracting the sizes of the
decorations from the initial total width and re-adding them later, they
should have been added to the calculated pixel width as is done when
fitting the height.

But mildly spoken, 'fit-window-to-buffer' is a complete mess in the
first place.  Calculating sizes in terms of lines/columns doesn't make
sense.  If really necessary - minibuffer resizing, for example, does not
care - results should be rounded in a final step.  Also, I doubt that
both char-width and char-height are calculated reasonably when buffer
text is scaled or line spacing changed.

martin

bug#70894: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix width calculation

[Eli Zaretskii]

> Date: Sun, 19 May 2024 09:58:58 +0200
> Cc: 70894@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > Martin, any comments?
> 
> Looks good to me.  Principally, instead of subtracting the sizes of the
> decorations from the initial total width and re-adding them later, they
> should have been added to the calculated pixel width as is done when
> fitting the height.

OK, I installed the patch on master now, thanks.

> But mildly spoken, 'fit-window-to-buffer' is a complete mess in the
> first place.  Calculating sizes in terms of lines/columns doesn't make
> sense.  If really necessary - minibuffer resizing, for example, does not
> care - results should be rounded in a final step.  Also, I doubt that
> both char-width and char-height are calculated reasonably when buffer
> text is scaled or line spacing changed.

For the last issue: we should use default-font-width/height.  But the
question is: would replacing frame-char-width/height by these two fix
that use case, or do we need anything else?

bug#70894: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix width calculation

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 > For the last issue: we should use default-font-width/height.  But the
 > question is: would replacing frame-char-width/height by these two fix
 > that use case, or do we need anything else?

It might improve those use cases but won't take care of any extra line
spacing.  But there are too many unresolved issues there: How would we
interpret the MAX/MIN-WIDTH and MAX/MIN-HEIGHT arguments - still in
frame character units as we (partially) do now?  Note that functions
like 'window-min-size' or 'window-safe-min-pixel-height' don't care
about window character units, they operate in frame units.  And there's
always the possibility that intermediate rounding errors sum up and make
things only get worse.

martin