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

Previous Next

To reply to this bug, email your comments to 70894 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix width calculation
Date: Sun, 12 May 2024 09:25:31 -0400

[Message part 1 (text/plain, inline)]

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

[0001-lisp-window.el-fit-window-to-buffer-Fix-width-calcul.patch (text/patch, attachment)]

Message #8 received at 70894 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Morgan Smith <Morgan.J.Smith <at> outlook.com>,
 martin rudalics <rudalics <at> gmx.at>
Cc: 70894 <at> debbugs.gnu.org
Subject: Re: bug#70894: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix
 width calculation
Date: Sat, 18 May 2024 12:39:06 +0300

> From: Morgan Smith <Morgan.J.Smith <at> 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 <at> 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
> 

Message #11 received at 70894 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>, Morgan Smith <Morgan.J.Smith <at> outlook.com>
Cc: 70894 <at> debbugs.gnu.org
Subject: Re: bug#70894: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix
 width calculation
Date: Sun, 19 May 2024 09:58:58 +0200

> 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

Message #14 received at 70894 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: Morgan.J.Smith <at> outlook.com, 70894 <at> debbugs.gnu.org
Subject: Re: bug#70894: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix
 width calculation
Date: Sun, 19 May 2024 11:09:29 +0300

> Date: Sun, 19 May 2024 09:58:58 +0200
> Cc: 70894 <at> debbugs.gnu.org
> From: martin rudalics <rudalics <at> 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?

Message #17 received at 70894 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Morgan.J.Smith <at> outlook.com, 70894 <at> debbugs.gnu.org
Subject: Re: bug#70894: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix
 width calculation
Date: Sun, 19 May 2024 11:22:36 +0200

> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.