bug#56102: 29.0.50; fit-frame-to-buffer's window-text-pixel-size calculation can be incorrect when only is set to vertically

bug#56102: 29.0.50; fit-frame-to-buffer's window-text-pixel-size calculation can be incorrect when only is set to vertically

[Aaron Jensen]

To repro, open emacs -Q and resize your frame so that the ";; This
buffer..." text wraps (this repro assumes your monitor has more than
enough space for it to not wrap if the frame was big enough).

Add a few more lines of text and then:

M-: (fit-frame-to-buffer nil nil nil nil nil 'vertically)

You should see that the frame's height is too short and does not contain
all the lines. It contains one fewer line for each wrapped line.

Screenshots:

https://share.cleanshot.com/huexHe
https://share.cleanshot.com/dnhKex

The problem appears to be the lines:

(size
            (window-text-pixel-size window from to max-width max-height))

As the max-width will be larger than the current frame (meaning the
height calculation will not take wrapping into account).

One possible fix is to set min/max height/width based on `only' to
(frame-parameter frame 'width) / (frame-parameter frame 'height) but I
do not know if that is the best fix.

If that is done, then it may be possible to remove the rest of the
special handling for `only' that sets width/height to nil and handles that.

In GNU Emacs 29.0.50 (build 1, aarch64-apple-darwin21.5.0, NS appkit-2113.50 Version 12.4 (Build 21F79))
 of 2022-05-30 built on aaron-m1.local
Windowing system distributor 'Apple', version 10.3.2113
System Description:  macOS 12.4

bug#56102: 29.0.50; fit-frame-to-buffer's window-text-pixel-size calculation can be incorrect when only is set to vertically

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Sun, 19 Jun 2022 23:03:14 -0400
> 
> 
> To repro, open emacs -Q and resize your frame so that the ";; This
> buffer..." text wraps (this repro assumes your monitor has more than
> enough space for it to not wrap if the frame was big enough).
> 
> Add a few more lines of text and then:
> 
> M-: (fit-frame-to-buffer nil nil nil nil nil 'vertically)
> 
> You should see that the frame's height is too short and does not contain
> all the lines. It contains one fewer line for each wrapped line.
> 
> Screenshots:
> 
> https://share.cleanshot.com/huexHe
> https://share.cleanshot.com/dnhKex
> 
> The problem appears to be the lines:
> 
> (size
>             (window-text-pixel-size window from to max-width max-height))
> 
> As the max-width will be larger than the current frame (meaning the
> height calculation will not take wrapping into account).
> 
> One possible fix is to set min/max height/width based on `only' to
> (frame-parameter frame 'width) / (frame-parameter frame 'height) but I
> do not know if that is the best fix.
> 
> If that is done, then it may be possible to remove the rest of the
> special handling for `only' that sets width/height to nil and handles that.

I think you're right, but I'd like to hear if Martin has any comments.

Thanks.

bug#56102: 29.0.50; fit-frame-to-buffer's window-text-pixel-size calculation can be incorrect when only is set to vertically

[Aaron Jensen]

On Wed, Jun 22, 2022 at 9:59 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Aaron Jensen <aaronjensen@gmail.com>
> > Date: Sun, 19 Jun 2022 23:03:14 -0400
> >
> >
> > To repro, open emacs -Q and resize your frame so that the ";; This
> > buffer..." text wraps (this repro assumes your monitor has more than
> > enough space for it to not wrap if the frame was big enough).
> >
> > Add a few more lines of text and then:
> >
> > M-: (fit-frame-to-buffer nil nil nil nil nil 'vertically)
> >
> > You should see that the frame's height is too short and does not contain
> > all the lines. It contains one fewer line for each wrapped line.
> >
> > Screenshots:
> >
> > https://share.cleanshot.com/huexHe
> > https://share.cleanshot.com/dnhKex
> >
> > The problem appears to be the lines:
> >
> > (size
> >             (window-text-pixel-size window from to max-width max-height))
> >
> > As the max-width will be larger than the current frame (meaning the
> > height calculation will not take wrapping into account).
> >
> > One possible fix is to set min/max height/width based on `only' to
> > (frame-parameter frame 'width) / (frame-parameter frame 'height) but I
> > do not know if that is the best fix.
> >
> > If that is done, then it may be possible to remove the rest of the
> > special handling for `only' that sets width/height to nil and handles that.
>
> I think you're right, but I'd like to hear if Martin has any comments.

Sounds good. The most minimal change I can think of is to use the
current frame's width as max-width in the window-text-pixel-size call
when only is set to vertically. I don't know of any reason we need to
constrain max-height in that call because it doesn't have the same
impact. That can be done either explicitly in that call or by changing
max-width to be set to the current width as I described before. That
would be asymmetrical with max-height though, which would be rather
confusing IMO.

Aaron

bug#56102: 29.0.50; fit-frame-to-buffer's window-text-pixel-size calculation can be incorrect when only is set to vertically

[martin rudalics]

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

 >>> The problem appears to be the lines:
 >>>
 >>> (size
 >>>              (window-text-pixel-size window from to max-width max-height))
 >>>
 >>> As the max-width will be larger than the current frame (meaning the
 >>> height calculation will not take wrapping into account).

I think MAX-WIDTH should be nil here.

 >>> One possible fix is to set min/max height/width based on `only' to
 >>> (frame-parameter frame 'width) / (frame-parameter frame 'height) but I
 >>> do not know if that is the best fix.

This would not work.  The 'height' frame parameter counts in characters
while 'window-text-pixel-size' wants pixels as X-LIMIT.  Also, the sizes
of frame and window decorations would hardly match.

 >>> If that is done, then it may be possible to remove the rest of the
 >>> special handling for `only' that sets width/height to nil and handles that.
 >>
 >> I think you're right, but I'd like to hear if Martin has any comments.
 >
 > Sounds good. The most minimal change I can think of is to use the
 > current frame's width as max-width in the window-text-pixel-size call
 > when only is set to vertically. I don't know of any reason we need to
 > constrain max-height in that call because it doesn't have the same
 > impact. That can be done either explicitly in that call or by changing
 > max-width to be set to the current width as I described before. That
 > would be asymmetrical with max-height though, which would be rather
 > confusing IMO.

Please try the attached diff.

Thanks, martin

[-- Attachment #2: fit-frame-to-buffer.diff --]
[-- Type: text/x-patch, Size: 1413 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index 1b8fe2b262..7b8ca87f6a 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -9382,14 +9382,17 @@ fit-frame-to-buffer-1
              ((numberp (nth 1 sizes)) (* (nth 1 sizes) line-height))
              (t (window-min-size window nil nil t))))
            (max-width
-            (min
-             (cond
-              ((numberp max-width) (* max-width char-width))
-              ((numberp (nth 2 sizes)) (* (nth 2 sizes) char-width))
-              (t parent-or-display-width))
-             ;; The following is the maximum width that fits into the
-             ;; left and right margins.
-             (max (- right-margin left-margin outer-minus-body-width))))
+            (let ((max-width
+                   (cond
+                    ((numberp max-width) (* max-width char-width))
+                    ((numberp (nth 2 sizes)) (* (nth 2 sizes) char-width))
+                    ((not (eq only 'vertically)) parent-or-display-width))))
+              (when (numberp max-width)
+                (min max-width
+                     ;; The following is the maximum width that fits
+                     ;; into the left and right margins.
+                     (max (- right-margin left-margin
+                             outer-minus-body-width))))))
            (min-width
             (cond
              ((numberp min-width) (* min-width char-width))

bug#56102: 29.0.50; fit-frame-to-buffer's window-text-pixel-size calculation can be incorrect when only is set to vertically

[Aaron Jensen]

On Thu, Jun 23, 2022 at 3:30 AM martin rudalics <rudalics@gmx.at> wrote:
>
>  >>> The problem appears to be the lines:
>  >>>
>  >>> (size
>  >>>              (window-text-pixel-size window from to max-width max-height))
>  >>>
>  >>> As the max-width will be larger than the current frame (meaning the
>  >>> height calculation will not take wrapping into account).
>
> I think MAX-WIDTH should be nil here.

Ah, I had no idea X-LIMIT could be nil, that's great.

> Please try the attached diff.

That seems to work for me. I do wonder though if the the check for
`only' should be first (i.e. if only is vertically, max-width is nil).
Is there a reason that we should not ignore a specified max-width when
only is set to vertically? I ask because in the package that I had
this issue with I employed a work-around where I set the max-width to
(frame-parameter frame 'width), which seems to work well enough, but
probably not as good as your fix. We may not be able to remove that
workaround for some time, so ignoring max-width if set would probably
work better in our specific case.

Thanks,

Aaron

bug#56102: 29.0.50; fit-frame-to-buffer's window-text-pixel-size calculation can be incorrect when only is set to vertically

[martin rudalics]

 > That seems to work for me. I do wonder though if the the check for
 > `only' should be first (i.e. if only is vertically, max-width is nil).
 > Is there a reason that we should not ignore a specified max-width when
 > only is set to vertically?

The idea of 'fit-frame-to-buffer' was that an application should be able
to call it (maybe implicitly via 'temp-buffer-resize-mode') and not care
about where on the display the frame will be shown.  Hence, a major
concern of its design was to constrain the frame to some specified area
on the display, to avoid that parts of it move off screen.  That's why
'fit-frame-to-buffer-margins' and 'fit-frame-to-buffer-sizes' have been
provided.

Currently, we check for an explicit MAX-WIDTH first and then consult
'fit-frame-to-buffer-sizes' via SIZES as

                     ((numberp max-width) (* max-width char-width))
                     ((numberp (nth 2 sizes)) (* (nth 2 sizes) char-width))

If we were to override MAX-WIDTH by setting ONLY to 'vertically', where
and how would we check SIZES?

 > I ask because in the package that I had
 > this issue with I employed a work-around where I set the max-width to
 > (frame-parameter frame 'width), which seems to work well enough, but
 > probably not as good as your fix. We may not be able to remove that
 > workaround for some time, so ignoring max-width if set would probably
 > work better in our specific case.

Are these issues really related?  If your workaround works, it should
continue working regardless of whether we ignore MAX-WIDTH when ONLY is
'vertically' or not.  Or am I missing something?

martin

bug#56102: 29.0.50; fit-frame-to-buffer's window-text-pixel-size calculation can be incorrect when only is set to vertically

[Aaron Jensen]

On Fri, Jun 24, 2022 at 5:20 AM martin rudalics <rudalics@gmx.at> wrote:
>
>  > That seems to work for me. I do wonder though if the the check for
>  > `only' should be first (i.e. if only is vertically, max-width is nil).
>  > Is there a reason that we should not ignore a specified max-width when
>  > only is set to vertically?
>
> The idea of 'fit-frame-to-buffer' was that an application should be able
> to call it (maybe implicitly via 'temp-buffer-resize-mode') and not care
> about where on the display the frame will be shown.  Hence, a major
> concern of its design was to constrain the frame to some specified area
> on the display, to avoid that parts of it move off screen.  That's why
> 'fit-frame-to-buffer-margins' and 'fit-frame-to-buffer-sizes' have been
> provided.
>
> Currently, we check for an explicit MAX-WIDTH first and then consult
> 'fit-frame-to-buffer-sizes' via SIZES as
>
>                      ((numberp max-width) (* max-width char-width))
>                      ((numberp (nth 2 sizes)) (* (nth 2 sizes) char-width))
>
> If we were to override MAX-WIDTH by setting ONLY to 'vertically', where
> and how would we check SIZES?

I'm definitely not familiar enough with the workings and the intended
use cases. My only thought about it was one of principle of least
surprise. If I say that I only want it to scale vertically, I would
expect no width changes to occur, which means I would expect that
max-width and anything else width related would not be relevant.
There's likely something I'm missing here, so please feel free to
disregard if this is necessary for some reason.

>  > I ask because in the package that I had
>  > this issue with I employed a work-around where I set the max-width to
>  > (frame-parameter frame 'width), which seems to work well enough, but
>  > probably not as good as your fix. We may not be able to remove that
>  > workaround for some time, so ignoring max-width if set would probably
>  > work better in our specific case.
>
> Are these issues really related?  If your workaround works, it should
> continue working regardless of whether we ignore MAX-WIDTH when ONLY is
> 'vertically' or not.  Or am I missing something?

My concern was this:

> Also, the sizes of frame and window decorations would hardly match.

Effectively, that there would be some subtle difference between using
(frame-parameter frame 'width) as MAX-WIDTH and passing nil to
window-text-pixel-size window.

If there's no difference to be concerned about then my workaround
should be fine with the code as it was in the patch.

Thanks,

Aaron

bug#56102: 29.0.50; fit-frame-to-buffer's window-text-pixel-size calculation can be incorrect when only is set to vertically

[martin rudalics]

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

 > If I say that I only want it to scale vertically, I would
 > expect no width changes to occur, which means I would expect that
 > max-width and anything else width related would not be relevant.

Sounds reasonable.

 > My concern was this:
 >
 >> Also, the sizes of frame and window decorations would hardly match.
 >
 > Effectively, that there would be some subtle difference between using
 > (frame-parameter frame 'width) as MAX-WIDTH and passing nil to
 > window-text-pixel-size window.

I was wrong earlier.  The doc-string of 'fit-frame-to-buffer' says that

   MAX-HEIGHT, MIN-HEIGHT, MAX-WIDTH and MIN-WIDTH specify bounds on
   the new total size of FRAME’s root window.

But actually it is the _body_ size of FRAME's root window the current
code constrains.  Incidentally, the default body width of a live root
window equals the value of FRAME's width parameter (the native width of
the frame minus the widths of the internal border, the scroll bar and
the fringes) which makes your fix work.  With a non-standard setup, say
after doing

(set-window-margins nil 10 10)

in the root window, your fix will fail with the scenario you provided
earlier.

We could try the following: If the new diff I attach now works for you,
I'll push it to master.  If after some period of grace (whose length you
determine) I manage to come up with a reasonable fix, I'll push that to
master too and you and your clients will have to adapt.  WDYT?

martin

[-- Attachment #2: fit-frame-to-buffer.diff --]
[-- Type: text/x-patch, Size: 1280 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index eba888a89d..9333f03822 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -9385,14 +9385,15 @@ fit-frame-to-buffer-1
              ((numberp (nth 1 sizes)) (* (nth 1 sizes) line-height))
              (t (window-min-size window nil nil t))))
            (max-width
-            (min
-             (cond
-              ((numberp max-width) (* max-width char-width))
-              ((numberp (nth 2 sizes)) (* (nth 2 sizes) char-width))
-              (t parent-or-display-width))
-             ;; The following is the maximum width that fits into the
-             ;; left and right margins.
-             (max (- right-margin left-margin outer-minus-body-width))))
+            (unless (eq only 'vertically)
+              (min
+               (cond
+                ((numberp max-width) (* max-width char-width))
+                ((numberp (nth 2 sizes)) (* (nth 2 sizes) char-width))
+                (t parent-or-display-width))
+               ;; The following is the maximum width that fits into the
+               ;; left and right margins.
+               (max (- right-margin left-margin outer-minus-body-width)))))
            (min-width
             (cond
              ((numberp min-width) (* min-width char-width))

bug#56102: 29.0.50; fit-frame-to-buffer's window-text-pixel-size calculation can be incorrect when only is set to vertically

[Aaron Jensen]

On Sun, Jun 26, 2022 at 6:09 AM martin rudalics <rudalics@gmx.at> wrote:
> But actually it is the _body_ size of FRAME's root window the current
> code constrains.  Incidentally, the default body width of a live root
> window equals the value of FRAME's width parameter (the native width of
> the frame minus the widths of the internal border, the scroll bar and
> the fringes) which makes your fix work.  With a non-standard setup, say
> after doing
>
> (set-window-margins nil 10 10)
>
> in the root window, your fix will fail with the scenario you provided
> earlier.

Ah, I can confirm this. Is there a reasonable way for me to calculate
a max-width that would be based on the root window that would work?
There's other math that happens within fit-frame-to-buffer I don't
fully have my head wrapped around yet. I'm not super worried about
this personally as I don't set window margins on this window.


> We could try the following: If the new diff I attach now works for you,
> I'll push it to master.  If after some period of grace (whose length you
> determine) I manage to come up with a reasonable fix, I'll push that to
> master too and you and your clients will have to adapt.  WDYT?

The patch works for me and seems good. When you say if you come up
with a reasonable fix, could I ask what is unreasonable about the
patch you attached?

Regardless, if you do end up updating the fix to respect a supplied
max-width even if only vertically is supplied, I could always make an
Emacs version based decision on whether or not to pass the work-around
max-width in, and Emacs 29 is as good a version as any to stop passing
it in (people already on master will have to recompile, but that's
fine imo). So, I would be fine with the new patch going straight to
master, though as I mentioned I don't know what it would do
differently or why.

Thanks,

Aaron

bug#56102: 29.0.50; fit-frame-to-buffer's window-text-pixel-size calculation can be incorrect when only is set to vertically

[martin rudalics]

 >> With a non-standard setup, say
 >> after doing
 >>
 >> (set-window-margins nil 10 10)
 >>
 >> in the root window, your fix will fail with the scenario you provided
 >> earlier.
 >
 > Ah, I can confirm this. Is there a reasonable way for me to calculate
 > a max-width that would be based on the root window that would work?

Here

(set-window-margins nil 10 10)
(fit-frame-to-buffer nil nil nil (window-body-width) nil 'vertically)

seems to work.

 > There's other math that happens within fit-frame-to-buffer I don't
 > fully have my head wrapped around yet. I'm not super worried about
 > this personally as I don't set window margins on this window.

Similar problems should happen when the fringe or scroll bar sizes of
the root window or its buffer differ from that of the frame.

 > The patch works for me and seems good. When you say if you come up
 > with a reasonable fix, could I ask what is unreasonable about the
 > patch you attached?

The current code interprets all its -HEIGHT and -WIDTH arguments as if
they were body sizes and not total sizes.  This at least contradicts the
doc-strings of 'fit-frame-to-buffer-sizes' and of 'fit-frame-to-buffer'
itself.  If I can fix the problem by rewriting the doc-strings to match
what the code does, I'll probably do that.  In general, you can't get
these right anyway when you allow specifications in terms of character
sizes and windows/frames may have non-integral sizes wrt these.

 > Regardless, if you do end up updating the fix to respect a supplied
 > max-width even if only vertically is supplied, I could always make an
 > Emacs version based decision on whether or not to pass the work-around
 > max-width in, and Emacs 29 is as good a version as any to stop passing
 > it in (people already on master will have to recompile, but that's
 > fine imo). So, I would be fine with the new patch going straight to
 > master, though as I mentioned I don't know what it would do
 > differently or why.

I don't know yet either.  I'll send you a patch as soon as I have one.

martin

bug#56102: 29.0.50; fit-frame-to-buffer's window-text-pixel-size calculation can be incorrect when only is set to vertically

[Aaron Jensen]

On Mon, Jun 27, 2022 at 4:24 AM martin rudalics <rudalics@gmx.at> wrote:
>
>  >> With a non-standard setup, say
>  >> after doing
>  >>
>  >> (set-window-margins nil 10 10)
>  >>
>  >> in the root window, your fix will fail with the scenario you provided
>  >> earlier.
>  >
>  > Ah, I can confirm this. Is there a reasonable way for me to calculate
>  > a max-width that would be based on the root window that would work?
>
> Here
>
> (set-window-margins nil 10 10)
> (fit-frame-to-buffer nil nil nil (window-body-width) nil 'vertically)
>
> seems to work.

Thanks, I'll update the workaround to use this instead.

> I don't know yet either.  I'll send you a patch as soon as I have one.

Sounds good, thanks.

Aaron

bug#56102: 29.0.50; fit-frame-to-buffer's window-text-pixel-size calculation can be incorrect when only is set to vertically

[martin rudalics]

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

 >> I don't know yet either.  I'll send you a patch as soon as I have one.
 >
 > Sounds good, thanks.

I suppose we can manage by fixing some trivial silliness in the code and
making the documentation tell what the code does.  If you don't see any
problems with the attached diff within a week or so, I'll push it.

Thanks, martin

[-- Attachment #2: fit-frame-to-buffer.diff --]
[-- Type: text/x-patch, Size: 5274 bytes --]

diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index e070e84c67..6a6c91e356 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -1151,10 +1151,12 @@ Resizing Windows
 its buffer exactly.  @var{frame} can be any live frame and defaults to
 the selected one.  Fitting is done only if @var{frame}'s root window is
 live.  The arguments @var{max-height}, @var{min-height}, @var{max-width}
-and @var{min-width} specify bounds on the new total size of
-@var{frame}'s root window.  @var{min-height} and @var{min-width} default
-to the values of @code{window-min-height} and @code{window-min-width}
-respectively.
+and @var{min-width}, if non-@code{nil}, specify bounds on the new body
+size of @var{frame}'s root window.  @var{min-height} and @var{min-width}
+default to the values of @code{window-safe-min-height} and
+@code{window-safe-min-width} respectively.  A non-@code{nil} value
+specified by any of these arguments overrides the corresponding value
+specified by @code{fit-frame-to-buffer-sizes}.
 
 If the optional argument @var{only} is @code{vertically}, this function
 may resize the frame vertically only.  If @var{only} is
@@ -1179,10 +1181,10 @@ Resizing Windows
 
 @defopt fit-frame-to-buffer-sizes
 This option specifies size boundaries for @code{fit-frame-to-buffer}.
-It specifies the total maximum and minimum lines and maximum and minimum
-columns of the root window of any frame that shall be fit to its buffer.
-If any of these values is non-@code{nil}, it overrides the corresponding
-argument of @code{fit-frame-to-buffer}.
+It specifies the maximum and minimum lines and maximum and minimum
+columns of the root window's body of any frame that shall be fit to its
+buffer.  Any value this option specifies will be overridden by the
+corresponding argument of @code{fit-frame-to-buffer}, if non-@code{nil}.
 @end defopt
 
 @deffn Command shrink-window-if-larger-than-buffer &optional window
diff --git a/lisp/window.el b/lisp/window.el
index a47a1216d1..cb59adc428 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -9053,10 +9053,11 @@ fit-frame-to-buffer-margins
 
 (defcustom fit-frame-to-buffer-sizes '(nil nil nil nil)
   "Size boundaries of frame for `fit-frame-to-buffer'.
-This list specifies the total maximum and minimum lines and
-maximum and minimum columns of the root window of any frame that
-shall be fit to its buffer.  If any of these values is non-nil,
-it overrides the corresponding argument of `fit-frame-to-buffer'.
+This list specifies the total maximum and minimum numbers of
+lines and the maximum and minimum numbers of columns of the body
+of the root window of any frame that shall be fit to its buffer.
+Any value specified by ths variable will be overridden by the
+corresponding argument of `fit-frame-to-buffer', if non-nil.
 
 On window systems where the menubar can wrap, fitting a frame to
 its buffer may swallow the last line(s).  Specifying an
@@ -9252,30 +9253,30 @@ fit-frame-to-buffer-1
               (t parent-or-display-height))
              ;; The following is the maximum height that fits into the
              ;; top and bottom margins.
-             (max (- bottom-margin top-margin outer-minus-body-height))))
+             (max (- bottom-margin top-margin outer-minus-body-height) 0)))
            (min-height
             (cond
              ((numberp min-height) (* min-height line-height))
              ((numberp (nth 1 sizes)) (* (nth 1 sizes) line-height))
-             (t (window-min-size window nil nil t))))
+             (t (window-safe-min-size window nil t))))
            (max-width
-            (min
-             (cond
-              ((numberp max-width) (* max-width char-width))
-              ((numberp (nth 2 sizes)) (* (nth 2 sizes) char-width))
-              (t parent-or-display-width))
-             ;; The following is the maximum width that fits into the
-             ;; left and right margins.
-             (max (- right-margin left-margin outer-minus-body-width))))
+            (unless (eq only 'vertically)
+              (min
+               (cond
+                ((numberp max-width) (* max-width char-width))
+                ((numberp (nth 2 sizes)) (* (nth 2 sizes) char-width))
+                (t parent-or-display-width))
+               ;; The following is the maximum width that fits into the
+               ;; left and right margins.
+               (max (- right-margin left-margin outer-minus-body-width) 0))))
            (min-width
             (cond
              ((numberp min-width) (* min-width char-width))
-             ((numberp (nth 3 sizes)) (nth 3 sizes))
-             (t (window-min-size window t nil t))))
+             ((numberp (nth 3 sizes)) (* (nth 3 sizes) char-width))
+             (t (window-safe-min-size window t t))))
            ;; Note: Currently, for a new frame the sizes of the header
            ;; and mode line may be estimated incorrectly
-           (size
-            (window-text-pixel-size window from to max-width max-height))
+           (size (window-text-pixel-size window from to max-width max-height))
            (width (max (car size) min-width))
            (height (max (cdr size) min-height)))
       ;; Don't change height or width when the window's size is fixed

bug#56102: 29.0.50; fit-frame-to-buffer's window-text-pixel-size calculation can be incorrect when only is set to vertically

[Aaron Jensen]

On Tue, Jun 28, 2022 at 5:29 AM martin rudalics <rudalics@gmx.at> wrote:
>
>  >> I don't know yet either.  I'll send you a patch as soon as I have one.
>  >
>  > Sounds good, thanks.
>
> I suppose we can manage by fixing some trivial silliness in the code and
> making the documentation tell what the code does.  If you don't see any
> problems with the attached diff within a week or so, I'll push it.

Great, works on first try. I'll run with it and report back if I have
any issues. Thank you.

Aaron

bug#56102: 29.0.50; fit-frame-to-buffer's window-text-pixel-size calculation can be incorrect when only is set to vertically

[Aaron Jensen]

On Tue, Jun 28, 2022 at 10:52 AM Aaron Jensen <aaronjensen@gmail.com> wrote:
>
> On Tue, Jun 28, 2022 at 5:29 AM martin rudalics <rudalics@gmx.at> wrote:
> >
> >  >> I don't know yet either.  I'll send you a patch as soon as I have one.
> >  >
> >  > Sounds good, thanks.
> >
> > I suppose we can manage by fixing some trivial silliness in the code and
> > making the documentation tell what the code does.  If you don't see any
> > problems with the attached diff within a week or so, I'll push it.
>
> Great, works on first try. I'll run with it and report back if I have
> any issues. Thank you.

No issues to report. Has worked fine for me for the last week.

Thanks,

Aaron

bug#56102: 29.0.50; fit-frame-to-buffer's window-text-pixel-size calculation can be incorrect when only is set to vertically

[martin rudalics]

 > No issues to report. Has worked fine for me for the last week.

Thanks.  I've pushed these changes to master now.  If there are any
problems, please complain.  Otherwise, consider marking this bug as
done.

Thanks, martin

bug#56102: 29.0.50; fit-frame-to-buffer's window-text-pixel-size calculation can be incorrect when only is set to vertically

[Aaron Jensen]

On Wed, Jul 6, 2022 at 3:37 AM martin rudalics <rudalics@gmx.at> wrote:
>
>  > No issues to report. Has worked fine for me for the last week.
>
> Thanks.  I've pushed these changes to master now.  If there are any
> problems, please complain.  Otherwise, consider marking this bug as
> done.

Great, thank you. Closing.

Aaron