bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Joost Kremers]

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


Following discussion on emacs-devel (thread "Window splitting issues
with margins"), this patch replaces the call to `window-width' in
`window-splittable-p' with `window-total-width'. This takes the window
margins into account when determining if a window can be split
horizontally.

Note: a similar change to `window-height' isn't necessary, because
`window-height' is an alias for `window-total-height'. (Whereas
`window-width' is an alias for `window-body-width'.)

BTW: this is my first patch. I have no idea if I got all the conventions
right...



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-lisp-window.el-window-splittable-p-use-window-total-.patch --]
[-- Type: text/x-diff, Size: 992 bytes --]

From 0fa1f344824530f9ee374cc0d84fa02b61b303bf Mon Sep 17 00:00:00 2001
From: Joost Kremers <joostkremers@fastmail.fm>
Date: Wed, 25 Nov 2015 13:36:06 +0100
Subject: [PATCH] * lisp/window.el (window-splittable-p): use
 `window-total-width'

Take the window margins into account when determining if a window can be
split horizontally.

Copyright-paperwork-exempt: yes
---
 lisp/window.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/window.el b/lisp/window.el
index 6d18905..bcd8922 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -6115,7 +6115,7 @@ window-splittable-p
 	       ;; sense nowadays.  This can be done more intuitively by
 	       ;; setting up `split-width-threshold' appropriately.
 	       (numberp split-width-threshold)
-	       (>= (window-width window)
+	       (>= (window-total-width window)
 		   (max split-width-threshold
 			(* 2 (max window-min-width 2)))))
 	;; A window can be split vertically when its height is not
-- 
2.6.0.GIT


[-- Attachment #3: Type: text/plain, Size: 40 bytes --]


-- 
Joost Kremers
Life has its moments

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[martin rudalics]

 > Following discussion on emacs-devel (thread "Window splitting issues
 > with margins"), this patch replaces the call to `window-width' in
 > `window-splittable-p' with `window-total-width'. This takes the window
 > margins into account when determining if a window can be split
 > horizontally.

If no one objects I'll check it in in a few days.

Thanks, martin

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Eli Zaretskii]

> Date: Wed, 25 Nov 2015 18:48:15 +0100
> From: martin rudalics <rudalics@gmx.at>
> 
>  > Following discussion on emacs-devel (thread "Window splitting issues
>  > with margins"), this patch replaces the call to `window-width' in
>  > `window-splittable-p' with `window-total-width'. This takes the window
>  > margins into account when determining if a window can be split
>  > horizontally.
> 
> If no one objects I'll check it in in a few days.

To master, right?

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[martin rudalics]

>> If no one objects I'll check it in in a few days.
>
> To master, right?

To emacs-25 preferably.

martin

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Eli Zaretskii]

> Date: Wed, 25 Nov 2015 19:14:58 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: joostkremers@fastmail.fm, 22009@debbugs.gnu.org
> 
> >> If no one objects I'll check it in in a few days.
> >
> > To master, right?
> 
> To emacs-25 preferably.

Convince me.  (It doesn't sound like a bug, does it?)

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[martin rudalics]

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

martin

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Eli Zaretskii]

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

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[martin rudalics]

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

Note that ‘window-splittable-p’ is exclusively used by ‘display-buffer’
and the probability is very high that the new window will not be used
for displaying the buffer of the original window.  The new window will
very likely have its own margins and header line.

 > So when we decide whether to split
 > vertically,

... horizontally ...

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

This is a problem that bites us much more with ‘split-window’ than with
‘display-buffer’.  As a matter of fact it has been already addressed in
one of the present threads discussing margins.  My opinion is that modes
that manage wide margins have to handle this in the hooks called when
windows are split, deleted or resized.  They also calculate the size of
the margins based on the current window width when they get activated.

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

‘window-min-width’ (and all related variables and functions) refer to
the total width of windows.  Changing its semantics would constitute
quite some effort.

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

This is not mode-triggered.  ‘window-splittable-p’ is used by
‘display-buffer’ to find the place for displaying *help*, *info*,
*completions*, ...

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

It's bug#17065, which talks about line 5944 of window.el.  And I think
you might agree with the poster ;-)

martin

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Eli Zaretskii]

> Date: Thu, 26 Nov 2015 17:58:10 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: joostkremers@fastmail.fm, 22009@debbugs.gnu.org
> 
>  > 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.
> 
> Note that ‘window-splittable-p’ is exclusively used by ‘display-buffer’
> and the probability is very high that the new window will not be used
> for displaying the buffer of the original window.  The new window will
> very likely have its own margins and header line.

Is it forbidden to call that function from any other Lisp?  If so,
disregard what I wrote.  But if not, window-splittable-p should do a
correct job no matter who calls it, do you agree?

>  > 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.
> 
> This is a problem that bites us much more with ‘split-window’ than with
> ‘display-buffer’.  As a matter of fact it has been already addressed in
> one of the present threads discussing margins.  My opinion is that modes
> that manage wide margins have to handle this in the hooks called when
> windows are split, deleted or resized.  They also calculate the size of
> the margins based on the current window width when they get activated.

My text that you quoted here talks about the "normal" case, where the
margins stay put upon splitting windows.  I'm saying that in such a
situation a decision made using the total width could be terribly
wrong.

>  > 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.
> 
> ‘window-min-width’ (and all related variables and functions) refer to
> the total width of windows.  Changing its semantics would constitute
> quite some effort.

If it's complicated, let's do that on master.  But do it we must, IMO.

>  > 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?
> 
> This is not mode-triggered.

In my text, "this" referred to this bug report.

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[martin rudalics]

 >> Note that ‘window-splittable-p’ is exclusively used by ‘display-buffer’
 >> and the probability is very high that the new window will not be used
 >> for displaying the buffer of the original window.  The new window will
 >> very likely have its own margins and header line.
 >
 > Is it forbidden to call that function from any other Lisp?  If so,
 > disregard what I wrote.  But if not, window-splittable-p should do a
 > correct job no matter who calls it, do you agree?

Presently ‘window-splittable-p’ is misnamed and I think that's the main
cause of the confusion.  But its doc-string clearly names
‘split-window-sensibly’ as the caller (which is misnamed as well but all
of these were written before ‘display-buffer’ was reorganized).  In any
case both functions belong exclusively to ‘display-buffer’ and are not
related to C-x 3.

 > My text that you quoted here talks about the "normal" case, where the
 > margins stay put upon splitting windows.  I'm saying that in such a
 > situation a decision made using the total width could be terribly
 > wrong.

But the "normal" case where this happens is C-x 3 and there the mode
that created the large margins should adapt.

 >> ‘window-min-width’ (and all related variables and functions) refer to
 >> the total width of windows.  Changing its semantics would constitute
 >> quite some effort.
 >
 > If it's complicated, let's do that on master.  But do it we must, IMO.

Basically it would amount to ‘split-window-horizontally’ telling us that
it can't split the window.  Which might not be the intention of the user
who expects the margins (whose sole purpose is to center the text in the
window) to shrink accordingly.

But honestly I don't know which implications changing the semantics of
‘window-min-width’ could have.  I could imagine adding a new option say
‘window-min-text-width’ and have the resizing routines respect this as
far as possible.

But sanitizing window sizes when, for example, making a frame very
small, would have to ignore such an option anyway.  The text area is the
only part I can freely shrink and enlarge down to one line and two
columns.  I cannot restore previous fringes, margins and scroll bars
once I have shrunk them because I don't remember their previous size.
And adding for every window a slot, say previous_left_fringe_width,
remembering it in window configurations, setting it when a user sets
‘set-window-fringes’ in between is something I'm not inclined to do.

 >>   > 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?
 >>
 >> This is not mode-triggered.
 >
 > In my text, "this" referred to this bug report.

If you mean that a mode introducing large margins should override
‘split-window-sensibly’ then this is not TRT.  ‘split-window-sensibly’
is user territory and ‘window-splittable-p’ too.

martin

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Eli Zaretskii]

> Date: Thu, 26 Nov 2015 19:06:07 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: joostkremers@fastmail.fm, 22009@debbugs.gnu.org
> 
>  >> Note that ‘window-splittable-p’ is exclusively used by ‘display-buffer’
>  >> and the probability is very high that the new window will not be used
>  >> for displaying the buffer of the original window.  The new window will
>  >> very likely have its own margins and header line.
>  >
>  > Is it forbidden to call that function from any other Lisp?  If so,
>  > disregard what I wrote.  But if not, window-splittable-p should do a
>  > correct job no matter who calls it, do you agree?
> 
> Presently ‘window-splittable-p’ is misnamed and I think that's the main
> cause of the confusion.  But its doc-string clearly names
> ‘split-window-sensibly’ as the caller (which is misnamed as well but all
> of these were written before ‘display-buffer’ was reorganized).  In any
> case both functions belong exclusively to ‘display-buffer’ and are not
> related to C-x 3.
> 
>  > My text that you quoted here talks about the "normal" case, where the
>  > margins stay put upon splitting windows.  I'm saying that in such a
>  > situation a decision made using the total width could be terribly
>  > wrong.
> 
> But the "normal" case where this happens is C-x 3 and there the mode
> that created the large margins should adapt.
> 
>  >> ‘window-min-width’ (and all related variables and functions) refer to
>  >> the total width of windows.  Changing its semantics would constitute
>  >> quite some effort.
>  >
>  > If it's complicated, let's do that on master.  But do it we must, IMO.
> 
> Basically it would amount to ‘split-window-horizontally’ telling us that
> it can't split the window.  Which might not be the intention of the user
> who expects the margins (whose sole purpose is to center the text in the
> window) to shrink accordingly.
> 
> But honestly I don't know which implications changing the semantics of
> ‘window-min-width’ could have.  I could imagine adding a new option say
> ‘window-min-text-width’ and have the resizing routines respect this as
> far as possible.

Maybe we should step back and recall what problem are we trying to
solve with the proposed patch, then.

>  >>   > 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?
>  >>
>  >> This is not mode-triggered.
>  >
>  > In my text, "this" referred to this bug report.
> 
> If you mean that a mode introducing large margins should override
> ‘split-window-sensibly’ then this is not TRT.  ‘split-window-sensibly’
> is user territory and ‘window-splittable-p’ too.

Didn't you just tell they are called by display-buffer?  How's that
"user territory"?

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[martin rudalics]

 > Maybe we should step back and recall what problem are we trying to
 > solve with the proposed patch, then.

The current code is inherently wrong because it treats height and width
differently.  And it's not about the margins alone but also about
fringes, scroll bars and dividers.  But this is a very old inconsistency
and I never worried to much about it.  Joost uncovered it.

 >> If you mean that a mode introducing large margins should override
 >> ‘split-window-sensibly’ then this is not TRT.  ‘split-window-sensibly’
 >> is user territory and ‘window-splittable-p’ too.
 >
 > Didn't you just tell they are called by display-buffer?  How's that
 > "user territory"?

The option ‘split-window-preferred-function’ is by default set to
‘split-window-sensibly’.  It's exclusively in the hand of the user to
change that.  A mode that wants to change whether and how
‘display-buffer’ is supposed to split a window should specify its own
buffer display action in the call to ‘display-buffer’.

It would be more correct to rename

‘split-window-preferred-function’ -> ‘display-buffer-split-window-preferred-function’

‘split-window-sensibly’ -> ‘display-buffer-split-window-sensibly’

‘window-splittable-p’ -> ‘display-buffer-window-splittable-p’

in order to avoid the misunderstandings we have here.  But this is
clumsy.  Suggestions welcome.

martin

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Eli Zaretskii]

> Date: Fri, 27 Nov 2015 09:27:39 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: joostkremers@fastmail.fm, 22009@debbugs.gnu.org
> 
>  > Maybe we should step back and recall what problem are we trying to
>  > solve with the proposed patch, then.
> 
> The current code is inherently wrong because it treats height and width
> differently.  And it's not about the margins alone but also about
> fringes, scroll bars and dividers.  But this is a very old inconsistency
> and I never worried to much about it.  Joost uncovered it.

Yes, but this doesn't seem to answer my question.  I already agreed
that there's inconsistency, the question is how to fix it.  I
suggested one way of reasoning, but you rejected it saying that the
code in question only affects a function that is only used by
display-buffer.  But display-buffer is not relevant to the use case
that Joost described, with those modes which use margins.  So why
should we do anything about window-splittable-p if it only affects
functions that are not relevant to Joost's use case?

Hence my question: if we aren't fixing Joost's use case, and
display-buffer is not relevant to splitting windows with large
margins, what problem we are trying to solve with that patch?

More importantly, if there isn't a clear-cut use case that will allow
us to decide which is the correct behavior, how can we decide whether
to make this change on master or on the release branch?  And why did
you feel it was so important to fix it on the branch?  What's the
urgency?

>  >> If you mean that a mode introducing large margins should override
>  >> ‘split-window-sensibly’ then this is not TRT.  ‘split-window-sensibly’
>  >> is user territory and ‘window-splittable-p’ too.
>  >
>  > Didn't you just tell they are called by display-buffer?  How's that
>  > "user territory"?
> 
> The option ‘split-window-preferred-function’ is by default set to
> ‘split-window-sensibly’.  It's exclusively in the hand of the user to
> change that.  A mode that wants to change whether and how
> ‘display-buffer’ is supposed to split a window should specify its own
> buffer display action in the call to ‘display-buffer’.

OK, and I'm still asking: how is all that related to
window-splittable-p?  When we talk about that function, what is the
relevance of the user's ability to replace it to the discussion?

> It would be more correct to rename
> 
> ‘split-window-preferred-function’ -> ‘display-buffer-split-window-preferred-function’
> 
> ‘split-window-sensibly’ -> ‘display-buffer-split-window-sensibly’
> 
> ‘window-splittable-p’ -> ‘display-buffer-window-splittable-p’
> 
> in order to avoid the misunderstandings we have here.

And how is this renaming relevant to the patch that is being
discussed?

I should probably bail out of this discussion, because I feel I'm not
contributing anything but my own confusion to it.

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[martin rudalics]

 > Yes, but this doesn't seem to answer my question.  I already agreed
 > that there's inconsistency, the question is how to fix it.  I
 > suggested one way of reasoning, but you rejected it saying that the
 > code in question only affects a function that is only used by
 > display-buffer.  But display-buffer is not relevant to the use case
 > that Joost described, with those modes which use margins.

Indeed.  Joost's description of his fix is too general, it alludes to
solving problems with C-x 3.  I initially misunderstood too as you can
derive from the following dialogue in a different thread.  I said that

 > As I tried to explain before: ‘window-splittable-p’ is hardly the
 > function we care about here.  We care about ‘split-window’ itself.

and Joost answered

 > Actually, I care about window-splittable-p, not about
 > split-window. split-window is not a problem, because after it's done
 > its work, window-configuration-change-hook is run, where
 > writeroom-mode can (and in fact already does) adjust the window
 > margins.

So your question

 > So why
 > should we do anything about window-splittable-p if it only affects
 > functions that are not relevant to Joost's use case?

should be answered now.  Correct?

 > Hence my question: if we aren't fixing Joost's use case, and
 > display-buffer is not relevant to splitting windows with large
 > margins, what problem we are trying to solve with that patch?

Joost's use case with making a window on the right of an existing one to
show a buffer for `display-buffer'.

 > More importantly, if there isn't a clear-cut use case that will allow
 > us to decide which is the correct behavior, how can we decide whether
 > to make this change on master or on the release branch?  And why did
 > you feel it was so important to fix it on the branch?  What's the
 > urgency?

If the fix is wrong it will be wrong for master too.  And if I don't fix
the code for the release branch but do care about the correctness of our
docs, I have to rewrite doc-strings and Elisp reference in order to
describe the inconsistency between vertical and horizontal splitting for
the release branch.  This is a bug I could ignore for some time simply
by not looking at the code more deeply.  Once I look at the code and see
what it does, I have to react either by correcting the code or by
changing the documentation.  Anything else would be irresponsible.

 >>   > Didn't you just tell they are called by display-buffer?  How's that
 >>   > "user territory"?
 >>
 >> The option ‘split-window-preferred-function’ is by default set to
 >> ‘split-window-sensibly’.  It's exclusively in the hand of the user to
 >> change that.  A mode that wants to change whether and how
 >> ‘display-buffer’ is supposed to split a window should specify its own
 >> buffer display action in the call to ‘display-buffer’.
 >
 > OK, and I'm still asking: how is all that related to
 > window-splittable-p?  When we talk about that function, what is the
 > relevance of the user's ability to replace it to the discussion?

If it is not related then I missed again what you were asking.  I tried
to guess twice but am apparently too silly to grok it.

 >> It would be more correct to rename
 >>
 >> ‘split-window-preferred-function’ -> ‘display-buffer-split-window-preferred-function’
 >>
 >> ‘split-window-sensibly’ -> ‘display-buffer-split-window-sensibly’
 >>
 >> ‘window-splittable-p’ -> ‘display-buffer-window-splittable-p’
 >>
 >> in order to avoid the misunderstandings we have here.
 >
 > And how is this renaming relevant to the patch that is being
 > discussed?

It's relevant because above you said that "display-buffer is not
relevant to the use case that Joost described, with those modes which
use margins".  All I'm trying to do is to say that "only display-buffer
is relevant to the use case Joost cares about, with those modes which
use margins".  But maybe it's better for Joost to tell.

 > I should probably bail out of this discussion, because I feel I'm not
 > contributing anything but my own confusion to it.

Let me try to sum up the possible ways to get confused here.  For a long
time we didn't have any problems or reports in this area because margins
were only used occasionally and in a very disciplined way.  Then we had
‘(n)linum-mode’ which started to use margins for displaying line
numbers.  And now we have modes that use potentially large margins to
center text within a window.  It's only the latter that introduce the
problems I list below.

Another problem is that these modes conflict(ed) with ‘(n)linum-mode’
when deciding on the size of the margins.  This is part of other
threads, so I won't describe it here.  The three major problems I see
are:

(1) When a mode uses margins to center text and you want to split the
     corresponding window via C-x 3, ‘split-window-right’ may refuse to
     split the window because the decision whether to split the window is
     based on the assumption that the margin widths in the original
     window shall be preserved.

(2) When a mode uses margins to center text and you want to resize the
     corresponding window horizontally via ‘mouse-drag-vertical-line’,
     the latter may refuse to do that because the decision whether to
     shrink the window is based on the assumption that its margin widths
     shall be preserved.

(3) When a mode uses margins to center text and you want to split the
     corresponding window for displaying a buffer via ‘display-buffer’,
     ‘split-window-sensibly’ may refuse to split the window because the
     decision whether to split the window is based on the assumption that
     the window to be split is only as wide as its text area.  Since this
     wide can be comparably small in a window that uses margins to center
     text, the attempt to display a buffer on the right may often fail
     unexpectedly.

Joost's fix is exclusively for problem (3).

martin

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Eli Zaretskii]

> Date: Sat, 28 Nov 2015 11:26:05 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: joostkremers@fastmail.fm, 22009@debbugs.gnu.org
> 
>  > Yes, but this doesn't seem to answer my question.  I already agreed
>  > that there's inconsistency, the question is how to fix it.  I
>  > suggested one way of reasoning, but you rejected it saying that the
>  > code in question only affects a function that is only used by
>  > display-buffer.  But display-buffer is not relevant to the use case
>  > that Joost described, with those modes which use margins.
> 
> Indeed.  Joost's description of his fix is too general, it alludes to
> solving problems with C-x 3.  I initially misunderstood too as you can
> derive from the following dialogue in a different thread.  I said that
> 
>  > As I tried to explain before: ‘window-splittable-p’ is hardly the
>  > function we care about here.  We care about ‘split-window’ itself.
> 
> and Joost answered
> 
>  > Actually, I care about window-splittable-p, not about
>  > split-window. split-window is not a problem, because after it's done
>  > its work, window-configuration-change-hook is run, where
>  > writeroom-mode can (and in fact already does) adjust the window
>  > margins.
> 
> So your question
> 
>  > So why
>  > should we do anything about window-splittable-p if it only affects
>  > functions that are not relevant to Joost's use case?
> 
> should be answered now.  Correct?

Yes, thanks.  But that still doesn't suggest how to fix
window-splittable-p, unless we care _only_ for Joost's use case.  And
I don't think we can declare that only such use cases matter, as
window-splittable-p is too general-purpose for that.

>  > More importantly, if there isn't a clear-cut use case that will allow
>  > us to decide which is the correct behavior, how can we decide whether
>  > to make this change on master or on the release branch?  And why did
>  > you feel it was so important to fix it on the branch?  What's the
>  > urgency?
> 
> If the fix is wrong it will be wrong for master too.  And if I don't fix
> the code for the release branch but do care about the correctness of our
> docs, I have to rewrite doc-strings and Elisp reference in order to
> describe the inconsistency between vertical and horizontal splitting for
> the release branch.  This is a bug I could ignore for some time simply
> by not looking at the code more deeply.  Once I look at the code and see
> what it does, I have to react either by correcting the code or by
> changing the documentation.  Anything else would be irresponsible.

We agree that the code should be fixed.  We just disagree about (or
don't know) _how_ to fix it.

>  >>   > Didn't you just tell they are called by display-buffer?  How's that
>  >>   > "user territory"?
>  >>
>  >> The option ‘split-window-preferred-function’ is by default set to
>  >> ‘split-window-sensibly’.  It's exclusively in the hand of the user to
>  >> change that.  A mode that wants to change whether and how
>  >> ‘display-buffer’ is supposed to split a window should specify its own
>  >> buffer display action in the call to ‘display-buffer’.
>  >
>  > OK, and I'm still asking: how is all that related to
>  > window-splittable-p?  When we talk about that function, what is the
>  > relevance of the user's ability to replace it to the discussion?
> 
> If it is not related then I missed again what you were asking.  I tried
> to guess twice but am apparently too silly to grok it.

Forget it, it isn't related to the issue, so there's no need to
explore this further.

> (1) When a mode uses margins to center text and you want to split the
>      corresponding window via C-x 3, ‘split-window-right’ may refuse to
>      split the window because the decision whether to split the window is
>      based on the assumption that the margin widths in the original
>      window shall be preserved.
> 
> (2) When a mode uses margins to center text and you want to resize the
>      corresponding window horizontally via ‘mouse-drag-vertical-line’,
>      the latter may refuse to do that because the decision whether to
>      shrink the window is based on the assumption that its margin widths
>      shall be preserved.
> 
> (3) When a mode uses margins to center text and you want to split the
>      corresponding window for displaying a buffer via ‘display-buffer’,
>      ‘split-window-sensibly’ may refuse to split the window because the
>      decision whether to split the window is based on the assumption that
>      the window to be split is only as wide as its text area.  Since this
>      wide can be comparably small in a window that uses margins to center
>      text, the attempt to display a buffer on the right may often fail
>      unexpectedly.
> 
> Joost's fix is exclusively for problem (3).

Agreed.  And I don't think we can fix this only for that use case.  We
should try to look for a more general solution.

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[martin rudalics]

 > Yes, thanks.  But that still doesn't suggest how to fix
 > window-splittable-p, unless we care _only_ for Joost's use case.  And
 > I don't think we can declare that only such use cases matter, as
 > window-splittable-p is too general-purpose for that.

Indeed.  Joost's fix also takes care of fringes, scroll bars and
dividers.

 > We agree that the code should be fixed.  We just disagree about (or
 > don't know) _how_ to fix it.

If we do not fix the code on the release branch I have to fix a couple
of doc-strings there telling that for historical reasons we do compare
apples and oranges.  Affected are ‘window-splittable-p’,
‘split-window-sensibly’ and ‘split-width-threshold’.  For the latter two
I'd have to amend the Elisp manual as well.

martin

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Joost Kremers]

On Sa, Nov 28 2015, martin rudalics <rudalics@gmx.at> wrote:
>  > Yes, but this doesn't seem to answer my question.  I already agreed
>  > that there's inconsistency, the question is how to fix it.  I
>  > suggested one way of reasoning, but you rejected it saying that the
>  > code in question only affects a function that is only used by
>  > display-buffer.  But display-buffer is not relevant to the use case
>  > that Joost described, with those modes which use margins.
>
> Indeed.  Joost's description of his fix is too general, it alludes to
> solving problems with C-x 3.  I initially misunderstood too 

Yes, sorry about the confusion. I was initially confused myself (but not
aware of it) when I posted my first message. It only became clear to me
later that there were two issues.

> Joost's use case with making a window on the right of an existing one to
> show a buffer for `display-buffer'.

Yes.

> It's relevant because above you said that "display-buffer is not
> relevant to the use case that Joost described, with those modes which
> use margins".  All I'm trying to do is to say that "only display-buffer
> is relevant to the use case Joost cares about, with those modes which
> use margins".  But maybe it's better for Joost to tell.

AFAICT that pretty much sums it up, at least as far as the patch that I
sent is concerned.

>  > I should probably bail out of this discussion, because I feel I'm not
>  > contributing anything but my own confusion to it.
>
> Let me try to sum up the possible ways to get confused here.  For a long
> time we didn't have any problems or reports in this area because margins
> were only used occasionally and in a very disciplined way.  Then we had
> ‘(n)linum-mode’ which started to use margins for displaying line
> numbers.  And now we have modes that use potentially large margins to
> center text within a window.  It's only the latter that introduce the
> problems I list below.
>
> Another problem is that these modes conflict(ed) with ‘(n)linum-mode’
> when deciding on the size of the margins.  This is part of other
> threads, so I won't describe it here.

Yes, it is necessary to keep these two issues apart. But. I think the
only right way to solve the issue of *this* thread will involve solving
the other issue as well.

>  The three major problems I see
> are:
>
> (1) When a mode uses margins to center text and you want to split the
>      corresponding window via C-x 3, ‘split-window-right’ may refuse to
>      split the window because the decision whether to split the window is
>      based on the assumption that the margin widths in the original
>      window shall be preserved.
>
> (2) When a mode uses margins to center text and you want to resize the
>      corresponding window horizontally via ‘mouse-drag-vertical-line’,
>      the latter may refuse to do that because the decision whether to
>      shrink the window is based on the assumption that its margin widths
>      shall be preserved.
>
> (3) When a mode uses margins to center text and you want to split the
>      corresponding window for displaying a buffer via ‘display-buffer’,
>      ‘split-window-sensibly’ may refuse to split the window because the
>      decision whether to split the window is based on the assumption that
>      the window to be split is only as wide as its text area.  Since this
>      wide can be comparably small in a window that uses margins to center
>      text, the attempt to display a buffer on the right may often fail
>      unexpectedly.
>

> Joost's fix is exclusively for problem (3).

Yes. And it has two additional problems: it counts fringes, scroll bars
and right dividers as well (as mentioned above). Plus it assumes that
the *entire* margins are reducible if necessary, which is also not the
case, since modes such as (n)linum-mode set a (left) margin and expect
this margin to be preserved when the window is split horizontally.

AFAIU all three problems amount to the same thing: currently, Emacs
assumes that when a window's width is changed, the margins must be
preserved. This assumption used to be correct, but with the introduction
of `visual-line-mode', various hacks and modes have been written that
use the margins to narrow the text area. When used this way, the margins
do *not* have to be preserved when the window width changes.

So essentially, since `visual-line-mode', the margins are being used in
ways that have conflicting requirements when a window's width changes:
some modes (such as (n)linum-mode) want the margins to be preserved,
other modes (such as `visual-fill-column-mode' / `writeroom-mode') want
the margins to be adjusted.

The only way to resolve such conflicts is to keep track of what mode set
the margins and whether they are adjustable or not. Then any part of
Emacs that needs to decide whether or how a window's width can be
changed can determine whether to just consider the text width, or the
text width plus the margins.

In other words, I don't think there's an easy fix that will really
resolve the issue. From what I understand from Martin's comments, a
proper fix is too involved to include in Emacs 25, so probably the best
thing to do is to update the doc strings and close this bug. A proper
solution can then be discussed on emacs-devel.

-- 
Joost Kremers
Life has its moments

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Joost Kremers]

[My apologies if this message messes up threading. I accidentally
deleted the thread from my mail archive.]

Eli Zaretskii wrote:
> Maybe we should step back and recall what problem are we trying to
> solve with the proposed patch, then.

The problem I'd like to solve (and the reason I submitted the patch) is
that if a window has wide margins that are used to center the text,
`window-splittable-p' will say that the window cannot be split
horizontally, even though it can, because the margins are disposable.

But actually, that's not the real problem. The real problem is that
`window-splittable-p' simply has too little information to make an
informed decision.

The point is that the margins may be used for two different kinds of
purposes:

(1) The margins can be used to display useful information, in which case
they should be retained when the window is split.

(2) The margins can be used to reduce the width of the text area. In
this case, the margins are disposable.

Right now, `window-splittable-p' does the right thing for case (1), but
not for case (2). With the patch I submitted, it would do the right
thing for case (2), but no longer for case (1). So I agree it's actually
a bad patch.

The only way to deal with both (1) and (2) correctly, and also with the
case where different modes use the margins for (1) and (2) *at the same
time*, would be to store information about the modes that use the
margins and for which purpose. Then `window-splittable-p' could tell
which part of the margins must be retained and which part is disposable.

We were discussing just such a proposal on emacs-devel, so perhaps it
would be best to continue the discussion there?


-- 
Joost Kremers
Life has its moments

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[martin rudalics]

 > The problem I'd like to solve (and the reason I submitted the patch) is
 > that if a window has wide margins that are used to center the text,
 > `window-splittable-p' will say that the window cannot be split
 > horizontally, even though it can, because the margins are disposable.
 >
 > But actually, that's not the real problem. The real problem is that
 > `window-splittable-p' simply has too little information to make an
 > informed decision.
 >
 > The point is that the margins may be used for two different kinds of
 > purposes:
 >
 > (1) The margins can be used to display useful information, in which case
 > they should be retained when the window is split.

But ‘window-splittable-p’ is used in a context where usually we use the
new window to display another buffer than that of the original window.
To my knowledge nobody wants *help* or *completions* buffers to display
line numbers.  And if the original window has no margins but the new
window should display them, the current code won't help either.

 > (2) The margins can be used to reduce the width of the text area. In
 > this case, the margins are disposable.

Let's specify the reason more explicitly: The margins are used to center
the text area in the window.  Correct?

 > Right now, `window-splittable-p' does the right thing for case (1),

It usually does not as I tried to sketch above.

 > but
 > not for case (2). With the patch I submitted, it would do the right
 > thing for case (2), but no longer for case (1). So I agree it's actually
 > a bad patch.
 >
 > The only way to deal with both (1) and (2) correctly, and also with the
 > case where different modes use the margins for (1) and (2) *at the same
 > time*, would be to store information about the modes that use the
 > margins and for which purpose. Then `window-splittable-p' could tell
 > which part of the margins must be retained and which part is disposable.

It would have to be clairvoyant wrt which kind of margins the buffer
displayed in the new window will have.  ‘find-file’ can enable
‘linum-mode’ in ‘find-file-hook’ but the real width of the margin needed
to display them will be known only when ‘linum-mode’ calculates it, that
is, _not_ at the time of splitting.

 > We were discussing just such a proposal on emacs-devel, so perhaps it
 > would be best to continue the discussion there?

That thread is for Emacs 26.  Here we have to decide how to proceed with
Emacs 25.

martin

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Joost Kremers]

On Fr, Nov 27 2015, martin rudalics <rudalics@gmx.at> wrote:
>  > The point is that the margins may be used for two different kinds of
>  > purposes:
>  >
>  > (1) The margins can be used to display useful information, in which case
>  > they should be retained when the window is split.
>
> But ‘window-splittable-p’ is used in a context where usually we use the
> new window to display another buffer than that of the original window.
> To my knowledge nobody wants *help* or *completions* buffers to display
> line numbers.

But I'm talking about the original window, which is now narrower than it
was before. If linum-mode is active in that buffer, it will still be
active after the split and the window will need margins to accommodate
the line numbers.

> And if the original window has no margins but the new
> window should display them, the current code won't help either.

No, but the buffer displayed in the new window has a local value for
`window-configuration-change-hook', which should take care of setting
the margins (as it does in writeroom-mode).

>  > (2) The margins can be used to reduce the width of the text area. In
>  > this case, the margins are disposable.
>
> Let's specify the reason more explicitly: The margins are used to center
> the text area in the window.  Correct?

Not necessarily, it's also possible that only one of the margins is
widened. That is what visual-fill-column-mode does, another mode I wrote
that mimics the effect of `fill-column' in buffers that use
visual-line-mode.

>  > The only way to deal with both (1) and (2) correctly, and also with the
>  > case where different modes use the margins for (1) and (2) *at the same
>  > time*, would be to store information about the modes that use the
>  > margins and for which purpose. Then `window-splittable-p' could tell
>  > which part of the margins must be retained and which part is disposable.
>
> It would have to be clairvoyant wrt which kind of margins the buffer
> displayed in the new window will have.  ‘find-file’ can enable
> ‘linum-mode’ in ‘find-file-hook’ but the real width of the margin needed
> to display them will be known only when ‘linum-mode’ calculates it, that
> is, _not_ at the time of splitting.

Yes, of course, there's nothing to be done about that.
`split-width-threshold' and/or `window-min-width' should be set up in
such a way that the new window can comfortably display a buffer that has
the margins that the user wants. But that's the user's responsibility.

I'm only worried about the original window. Perhaps some clarification
with screen shots is in order. Consider the following:

http://wwwuser.gwdg.de/~jkremer/visual-fill-column1.png

This is a buffer with a soft-wrapped text, with the right margin set to
some large value so that the text is wrapped at `fill-column'. If I do
`C-h f <some-function>' in such a window configuration, I get this:

http://wwwuser.gwdg.de/~jkremer/visual-fill-column2.png

which is odd, because there is so much "empty" space on the right. By
using `window-total-width' in `window-splittable-p', I get this:

http://wwwuser.gwdg.de/~jkremer/visual-fill-column3.png

Much nicer, IMHO. (I'm not trying to defind my fix here, I know it's
flawed. Just demonstrating what my use case is.)


-- 
Joost Kremers
Life has its moments

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Eli Zaretskii]

> From: Joost Kremers <joostkremers@fastmail.fm>
> Date: Fri, 27 Nov 2015 02:16:22 +0100
> 
> The problem I'd like to solve (and the reason I submitted the patch) is
> that if a window has wide margins that are used to center the text,
> `window-splittable-p' will say that the window cannot be split
> horizontally, even though it can, because the margins are disposable.
> 
> But actually, that's not the real problem. The real problem is that
> `window-splittable-p' simply has too little information to make an
> informed decision.
> 
> The point is that the margins may be used for two different kinds of
> purposes:
> 
> (1) The margins can be used to display useful information, in which case
> they should be retained when the window is split.
> 
> (2) The margins can be used to reduce the width of the text area. In
> this case, the margins are disposable.

There's one more factor here: whether the margins will remain at their
width after splitting.  The mode which you presented and where the
current behavior was a problem changed the margins width as result of
the split (AFAIU), which is something the split decision cannot
possibly know.  So I don't see how we could solve the problem even in
principle if we want to handle such modes correctly.  The only way is
to ask the modes themselves, or provide a hook for them to override
the decision.

> We were discussing just such a proposal on emacs-devel, so perhaps it
> would be best to continue the discussion there?

Probably.  And I still think such changes are not for the release
branch.

Thanks.

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Joost Kremers]

On Fr, Nov 27 2015, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Joost Kremers <joostkremers@fastmail.fm>
>> Date: Fri, 27 Nov 2015 02:16:22 +0100
>> 
>> The problem I'd like to solve (and the reason I submitted the patch) is
>> that if a window has wide margins that are used to center the text,
>> `window-splittable-p' will say that the window cannot be split
>> horizontally, even though it can, because the margins are disposable.
>> 
>> But actually, that's not the real problem. The real problem is that
>> `window-splittable-p' simply has too little information to make an
>> informed decision.
>> 
>> The point is that the margins may be used for two different kinds of
>> purposes:
>> 
>> (1) The margins can be used to display useful information, in which case
>> they should be retained when the window is split.
>> 
>> (2) The margins can be used to reduce the width of the text area. In
>> this case, the margins are disposable.
>
> There's one more factor here: whether the margins will remain at their
> width after splitting.

Yes, that is the actual issue. The point is that the way the margins are
used determines whether they must remain at their width after splitting
or not.

>  The mode which you presented and where the
> current behavior was a problem changed the margins width as result of
> the split (AFAIU),

Yes, you understand correctly.

> which is something the split decision cannot
> possibly know.

As things stand, no.

>  So I don't see how we could solve the problem even in
> principle if we want to handle such modes correctly.

By keeping track of which modes set the margins and whether they can be
reduced in case of a window split or not, as discussed in the thread on
emacs-devel.

> The only way is
> to ask the modes themselves, or provide a hook for them to override
> the decision.

I'm not sure that would be able to handle cases where different modes
use the margins for different purposes.


-- 
Joost Kremers
Life has its moments

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Eli Zaretskii]

> From: Joost Kremers <joostkremers@fastmail.fm>
> Cc: 22009@debbugs.gnu.org
> Date: Tue, 01 Dec 2015 13:59:36 +0100
> 
> > The only way is
> > to ask the modes themselves, or provide a hook for them to override
> > the decision.
> 
> I'm not sure that would be able to handle cases where different modes
> use the margins for different purposes.

I think this is the actual challenge in this bug report: design a way
that would make it possible.

Thanks.

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Joost Kremers]

On Tue, Dec 01 2015, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Joost Kremers <joostkremers@fastmail.fm>
>> Cc: 22009@debbugs.gnu.org
>> Date: Tue, 01 Dec 2015 13:59:36 +0100
>> 
>> > The only way is
>> > to ask the modes themselves, or provide a hook for them to override
>> > the decision.
>> 
>> I'm not sure that would be able to handle cases where different modes
>> use the margins for different purposes.
>
> I think this is the actual challenge in this bug report: design a way
> that would make it possible.

Yes. We discussed a possible way of doing this in the thread on
emacs-devel. I'll start a new thread there with a summary of the issues
and a proposal on how to solve them.

-- 
Joost Kremers
Life has its moments

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Lars Ingebrigtsen]

Joost Kremers <joostkremers@fastmail.fm> writes:

> Following discussion on emacs-devel (thread "Window splitting issues
> with margins"), this patch replaces the call to `window-width' in
> `window-splittable-p' with `window-total-width'. This takes the window
> margins into account when determining if a window can be split
> horizontally.

[...]

> -	       (>= (window-width window)
> +	       (>= (window-total-width window)

[...]

>>> I'm not sure that would be able to handle cases where different modes
>>> use the margins for different purposes.
>>
>> I think this is the actual challenge in this bug report: design a way
>> that would make it possible.
>
> Yes. We discussed a possible way of doing this in the thread on
> emacs-devel. I'll start a new thread there with a summary of the issues
> and a proposal on how to solve them.

Skimming this bug report, I think everybody pretty much agreed that this
patch was better than the current behaviour, but that this new behaviour
isn't completely correct (especially for those mode that use the margins
to display useful information).

Did the discussion on emacs-devel come to any sort of useful conclusion?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Joost Kremers]

On Mon, Sep 07 2020, Lars Ingebrigtsen wrote:
> Joost Kremers <joostkremers@fastmail.fm> writes:
>>>> I'm not sure that would be able to handle cases where 
>>>> different modes
>>>> use the margins for different purposes.
>>>
>>> I think this is the actual challenge in this bug report: 
>>> design a way
>>> that would make it possible.
>>
>> Yes. We discussed a possible way of doing this in the thread on
>> emacs-devel. I'll start a new thread there with a summary of 
>> the issues
>> and a proposal on how to solve them.
>
> Skimming this bug report, I think everybody pretty much agreed 
> that this
> patch was better than the current behaviour, but that this new 
> behaviour
> isn't completely correct (especially for those mode that use the 
> margins
> to display useful information).

Yes, that summarises my memory of the discussion.

> Did the discussion on emacs-devel come to any sort of useful 
> conclusion?

I'd have to dig up those threads again to be sure, but from what I 
remember, the proposal that I made was deemed too complicated for 
the problem at hand (probably rightly so). In the end, the window 
parameter `min-margins' was added and IIRC some changes were made 
to the way `window-configuration-change-hook' and 
`window-size-change-functions' operate (I may be mistaken on the 
latter point, though). That didn't solve the problem of allowing 
multiple modes to use the margins without interfering with each 
other, but it should solve the original problem of splitting 
windows with large margins.

I say "should", because at the time, I decided against using 
`min-margins' in my code (`visual-fill-column.el), so I never 
actually tested it. (My existing code still worked in Emacs 25 
(and indeed in 27.1) and I didn't want to drop support for Emacs 
24. It might be time to reconsider that decision, though, given 
that Emacs 25.1 is four years old now.)

-- 
Joost Kremers
Life has its moments

bug#22009: PATCH: Use `window-total-width' in `window-splittable-p'

[Lars Ingebrigtsen]

Joost Kremers <joostkremers@fastmail.fm> writes:

> That didn't solve the problem of allowing multiple modes to use the
> margins without interfering with each other, but it should solve the
> original problem of splitting windows with large margins.

Oh, OK.  Then I guess the reported bug here was fixed in a different
manner, so I'm closing this bug report.  It might be nice to allow modes
to express something about their intentions with the margin, but I guess
that's a separate issue.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no