From: Ingo Lohmar <ingo.lohmar@posteo.net>
To: martin rudalics <rudalics@gmx.at>
Cc: 37563@debbugs.gnu.org
Subject: bug#37563: 27.0.50; fit-frame-to-buffer does not account for line-spacing
Date: Thu, 03 Oct 2019 10:48:09 +0200 [thread overview]
Message-ID: <87zhiixliu.fsf@kenko.localhost.com> (raw)
In-Reply-To: <d149be6f-eb56-b4b9-f687-91cded5969c7@gmx.at>
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
next prev parent reply other threads:[~2019-10-03 8:48 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-30 18:41 bug#37563: 27.0.50; fit-frame-to-buffer does not account for line-spacing Ingo Lohmar
2019-10-01 7:32 ` martin rudalics
[not found] ` <87lfu4aook.fsf@kenko.localhost.com>
2019-10-01 8:10 ` martin rudalics
2019-10-01 8:28 ` Ingo Lohmar
2019-10-02 8:54 ` martin rudalics
2019-10-03 8:15 ` martin rudalics
2019-10-03 8:48 ` Ingo Lohmar [this message]
2019-10-03 18:10 ` martin rudalics
2019-10-03 18:21 ` Ingo Lohmar
2019-10-05 8:41 ` martin rudalics
2019-10-05 9:05 ` Ingo Lohmar
2019-10-07 9:25 ` martin rudalics
2019-10-07 17:45 ` Ingo Lohmar
2019-10-08 8:44 ` martin rudalics
2019-10-11 8:16 ` martin rudalics
2019-10-11 17:45 ` Ingo Lohmar
2019-10-03 8:56 ` Ingo Lohmar
2019-10-03 9:12 ` Robert Pluim
2019-10-03 16:09 ` Eli Zaretskii
2019-10-03 18:10 ` martin rudalics
2019-10-03 18:22 ` Ingo Lohmar
2019-10-01 7:39 ` bug#37563: [PATCH] please review Ingo Lohmar
2019-10-02 8:53 ` martin rudalics
[not found] ` <handler.37563.B.156987198814967.ack@debbugs.gnu.org>
2019-10-11 17:50 ` bug#37563: Acknowledgement (27.0.50; fit-frame-to-buffer does not account for line-spacing) Ingo Lohmar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zhiixliu.fsf@kenko.localhost.com \
--to=ingo.lohmar@posteo.net \
--cc=37563@debbugs.gnu.org \
--cc=rudalics@gmx.at \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).