bug#37563: 27.0.50; fit-frame-to-buffer does not account for line-spacing

[Ingo Lohmar]

On Thu, Oct 03 2019 10:15 (+0200), martin rudalics wrote:

>  > Agreed.  But if I change 'fit-frame-to-buffer', then, for consistency,
>  > I have to at least change 'fit-window-to-buffer' too.
>  >
>  >  > +          (setq height (* (/ (+ height line-height -1) line-height)
>  >  > +                          line-height)))
>  > [...]
>  >  > And then char-height can be dropped.
>  >
>  > Right.
>
> Hmm...  Back to the roots, unfortunately.
>
> When we are here, 'height' is the calculated height the window should
> have in pixels.  When we want to communicate this value to the window
> manager and 'frame-resize-pixelwise' is nil, we have to transform this
> value (which already includes the pixels needed for line spacing) to a
> multiple of the canonical character height of the frame and not the
> line height we calculated earlier.  So using 'line-height' here is not
> the TRT unless I'm missing something.  WDYT?

Well, I don't really know :) I'm not totally sure about the meaning of
`frame-resize-pixelwise'.

1) I think you're right in the sense that rounding to the line height
(not the canonical char height) is NOT what the docstring says it does.
That's bad and should be fixed either way.

2) With the above code, rounding window height (incl line spacing) to a
multiple of the line height (incl line spacing), I do not see any effect
of the option value; because it does not change the height value at all
in my test cases (small popups).

3) BUT that's not generally true, IMO: If the height is restricted by
the screen, or the margin calculation changes it, the rounding will have
an effect.

4) *My interpretation* of `frame-resize-pixelwise' is that the default
value, nil, has a single intention: To make the frame height an exact
multiple of lines (and char width), mainly because of aesthetic reasons.
In that case, I suggest we change the doc-string (which has some
inaccurate phrasing and is a bit wordy, anyway) to say that it nil will
round to a multiple of the default line height (incl line spacing)..

Very briefly browsing the C code using the option seems to confirm that
interpretation.

So I think the code snippet as above is correct (using the line-height).
I will try to come up with an updated docstring for
`frame-resize-pixelwise', if you don't mind.  As for the consistency
changes that you mention, that sounds fine with me, you know the
relevant much(!) better than I do.

Ingo