unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: martin rudalics <rudalics@gmx.at>
Cc: joostkremers@fastmail.fm, 22009@debbugs.gnu.org
Subject: bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'
Date: Thu, 26 Nov 2015 17:46:34 +0200	[thread overview]
Message-ID: <83r3jcvk4l.fsf@gnu.org> (raw)
In-Reply-To: <5656C171.9050506@gmx.at>

> Date: Thu, 26 Nov 2015 09:23:13 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: joostkremers@fastmail.fm, 22009@debbugs.gnu.org
> 
> > Convince me.  (It doesn't sound like a bug, does it?)
> 
> It is a bug.  In
> 
>                (>= (window-width window)
>                    (max split-width-threshold
>                         (* 2 (max window-min-width 2))))
> 
> we currently compare apples and oranges.  ‘window-width’ does not
> include margins, fringes, scrollbar and divider.  ‘window-min-width’
> is meant to include them all.

Thanks.  It turns out I was mistaken wrt what issue this attempts to
solve.

Now that you've set me straight, I agree this is a bug.  But I don't
necessarily agree with the proposed solution.

In the discussion that led to this you said:

> > A related problem is the fact that `window-splittable-p' only takes the
> > width of the text area into account when deciding if a window can be
> > split horizontally. This often leads to the situation where a window is
> > split vertically, although there appears to be enough room to split it
> > horizontally (said room being taken up by the margin).
> 
> I agree with this observation.  ‘window-splittable-p’ is asymmetric:
> When it checks the width, it uses the text area while for the height it
> uses the total area (inlcuding mode and header lines, scrollbar, divider
> ...).  If you want to change this, please provide a patch.  I certainly
> won't object it but am afraid that some people eventually will complain
> because one of their packages then doesn't work like it used to over the
> past decades ...

I agree with the asymmetry observation, but I think that the width
check is the one that gets it right, while the height check is wrong
and should be corrected.  When a window is split horizontally, the
part that gets split is the text area, not the margins -- those stay
at their original size.  So when we decide whether to split
vertically, we should consider the text-area size, not the total size.
Otherwise, we can easily wind up in situations where, due to large
margins, a window is split horizontally and the width of its text area
becomes ridiculously small.

Therefore, I think we should document that split-width-threshold and
window-min-width refer to the width of the text area, and if there are
functions in window.el that compare them with window-total-width,
those places need to be fixed, not this one.  We should also fix the
height check in window-splittable-p.

The modes that triggered this are special in that they adapt their
margins to the split, but how would window-splittable-p know that?
Perhaps such modes should override that function, or maybe we should
provide a hook for them?

> BTW: Bug#5944 is about a related issue.  I never got around to resolve
> it for a similar reason.

Did you really mean 5944?  That's not an Emacs bug even.





  reply	other threads:[~2015-11-26 15:46 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-25 13:07 bug#22009: PATCH: Use `window-total-width' in `window-splittable-p' Joost Kremers
2015-11-25 17:48 ` martin rudalics
2015-11-25 18:04   ` Eli Zaretskii
2015-11-25 18:14     ` martin rudalics
2015-11-25 19:15       ` Eli Zaretskii
2015-11-26  8:23         ` martin rudalics
2015-11-26 15:46           ` Eli Zaretskii [this message]
2015-11-26 16:58             ` martin rudalics
2015-11-26 17:25               ` Eli Zaretskii
2015-11-26 18:06                 ` martin rudalics
2015-11-26 18:36                   ` Eli Zaretskii
2015-11-27  8:27                     ` martin rudalics
2015-11-27 20:51                       ` Eli Zaretskii
2015-11-28 10:26                         ` martin rudalics
2015-11-28 14:49                           ` Eli Zaretskii
2015-11-28 15:49                             ` martin rudalics
2015-12-01 12:47                           ` Joost Kremers
2015-11-27  1:16 ` Joost Kremers
2015-11-27  8:28   ` martin rudalics
2015-12-01 14:11     ` Joost Kremers
2015-11-27 20:56   ` Eli Zaretskii
2015-12-01 12:59     ` Joost Kremers
2015-12-01 15:44       ` Eli Zaretskii
2015-12-01 17:31         ` Joost Kremers
2020-09-07 16:48 ` Lars Ingebrigtsen
2020-09-08  8:50   ` Joost Kremers
2020-09-08 10:05     ` Lars Ingebrigtsen

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=83r3jcvk4l.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=22009@debbugs.gnu.org \
    --cc=joostkremers@fastmail.fm \
    --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).