all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#70894: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix width calculation
@ 2024-05-12 13:25 Morgan Smith
  2024-05-18  9:39 ` Eli Zaretskii
  0 siblings, 1 reply; 5+ messages in thread
From: Morgan Smith @ 2024-05-12 13:25 UTC (permalink / raw)
  To: 70894

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


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

* bug#70894: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix width calculation
  2024-05-12 13:25 bug#70894: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix width calculation Morgan Smith
@ 2024-05-18  9:39 ` Eli Zaretskii
  2024-05-19  7:58   ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 5+ messages in thread
From: Eli Zaretskii @ 2024-05-18  9:39 UTC (permalink / raw)
  To: Morgan Smith, martin rudalics; +Cc: 70894

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





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

* bug#70894: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix width calculation
  2024-05-18  9:39 ` Eli Zaretskii
@ 2024-05-19  7:58   ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-19  8:09     ` Eli Zaretskii
  0 siblings, 1 reply; 5+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-19  7:58 UTC (permalink / raw)
  To: Eli Zaretskii, Morgan Smith; +Cc: 70894

 > 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





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

* bug#70894: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix width calculation
  2024-05-19  7:58   ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-05-19  8:09     ` Eli Zaretskii
  2024-05-19  9:22       ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 5+ messages in thread
From: Eli Zaretskii @ 2024-05-19  8:09 UTC (permalink / raw)
  To: martin rudalics; +Cc: Morgan.J.Smith, 70894

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





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

* bug#70894: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix width calculation
  2024-05-19  8:09     ` Eli Zaretskii
@ 2024-05-19  9:22       ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 0 replies; 5+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-19  9:22 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Morgan.J.Smith, 70894

 > 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





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

end of thread, other threads:[~2024-05-19  9:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-12 13:25 bug#70894: [PATCH] * lisp/window.el (fit-window-to-buffer): Fix width calculation Morgan Smith
2024-05-18  9:39 ` Eli Zaretskii
2024-05-19  7:58   ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-19  8:09     ` Eli Zaretskii
2024-05-19  9:22       ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.