bug#55169: Can't combine window-min-height with window-height

bug#55169: Can't combine window-min-height with window-height

[Juri Linkov]

As bug#33007 revealed, window-min-height doesn't prevent
from too narrow window height when using such call:

  (pop-to-buffer (generate-new-buffer "*edit string*")
                 '(display-buffer-below-selected
                   (window-min-height . 10)
                   (window-height . fit-window-to-buffer)))

Shouldn't the order of processing these parameters be rather like this:

1. first set window-height with fit-window-to-buffer;
2. then check if the constraint of window-min-height is fulfilled,
   and shrink too high window.

Then the caller could insert the initial string, and
'fit-window-to-buffer' will fit the window.  If the window height
is less than 10 lines, it will enlarge to 10 lines.  But in case of
too many lines, the window height should not be more than
half of the original window.

bug#55169: Can't combine window-min-height with window-height

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> Shouldn't the order of processing these parameters be rather like this:
>
> 1. first set window-height with fit-window-to-buffer;
> 2. then check if the constraint of window-min-height is fulfilled,
>    and shrink too high window.

The original example looks odd to me, but I may just be misunderstanding
the window machinery (not for the first time).

But:

(display-buffer-below-selected
 (current-buffer)
 '((window-min-height . 10)
   (window-height . fit-window-to-buffer)))

I'd expect that to work as you describe (or perhaps with the alist in
opposite order), and it doesn't.  Perhaps Martin has some comments;
added to the CCs.

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

bug#55169: Can't combine window-min-height with window-height

[martin rudalics]

 > I'd expect that to work as you describe (or perhaps with the alist in
 > opposite order), and it doesn't.  Perhaps Martin has some comments;
 > added to the CCs.

Ideally, a 'window-min-height' entry should be applied when an existing
window is used via 'display-buffer-use-some-window'.  Just that in such
case we (i) don't even look at that entry and (ii) we would not apply a
'window-height' entry in the first place because that one is only used
for new windows or windows that always showed one and the same buffer.

Now

 >> 1. first set window-height with fit-window-to-buffer;
 >> 2. then check if the constraint of window-min-height is fulfilled,
 >>     and shrink too high window.

is in general non-trivial because "1." constitutes a fait accompli: We
need a _real_ window to calculate its desired size and whether it is
'window-resizable' to that size.  So we may have to split some window
first to make "1." work.  But what would we do then if "2." were not
satisfied?  Restore the frame to a window configuration saved before
splitting?

Which means that the strategy proposed above would work seamlessly with
'display-buffer-reuse-window' only.  But then would we really reject a
window showing a buffer already only because it's not large enough?

As usual we are caught in between long established conveniences and more
rigorous guide lines for how 'display-buffer' should behave.

martin

bug#55169: Can't combine window-min-height with window-height

[Juri Linkov]

> Ideally, a 'window-min-height' entry should be applied when an existing
> window is used via 'display-buffer-use-some-window'.  Just that in such
> case we (i) don't even look at that entry and (ii) we would not apply a
> 'window-height' entry in the first place because that one is only used
> for new windows or windows that always showed one and the same buffer.
>
> Now
>
>>> 1. first set window-height with fit-window-to-buffer;
>>> 2. then check if the constraint of window-min-height is fulfilled,
>>>     and shrink too high window.
>
> is in general non-trivial because "1." constitutes a fait accompli: We
> need a _real_ window to calculate its desired size and whether it is
> 'window-resizable' to that size.  So we may have to split some window
> first to make "1." work.  But what would we do then if "2." were not
> satisfied?  Restore the frame to a window configuration saved before
> splitting?
>
> Which means that the strategy proposed above would work seamlessly with
> 'display-buffer-reuse-window' only.  But then would we really reject a
> window showing a buffer already only because it's not large enough?
>
> As usual we are caught in between long established conveniences and more
> rigorous guide lines for how 'display-buffer' should behave.

Maybe as a workaround two calls of display-buffer would do the trick?
The first call display-buffer-below-selected will split the window,
then the second call display-buffer-use-some-window will adjust the height.

bug#55169: Can't combine window-min-height with window-height

[martin rudalics]

 > Maybe as a workaround two calls of display-buffer would do the trick?
 > The first call display-buffer-below-selected will split the window,
 > then the second call display-buffer-use-some-window will adjust the height.

There would be no benefit in doing such a thing.  The first call might
not be able to produce a window that satisfies the 'window-min-height'
constraint.

A correct solution would be to, before splitting, check whether the
split off window can be made as high as wanted.  For that, we would have
to investigate all possible splittings over all actions allowed by the
ALIST argument to check whether the emanating split would satisfy the
height constraint (where the maximum possible height of a split off
window would be the sum of the total sizes minus their minimum sizes of
all windows in the same combination as the split one).

Our current strategy to usually split just the largest or LRU window is
much too naive in this regard.  The problem with a new approach is that
we would have to tell people that when a 'window-min-height' entry is
present, the display strategy they got used to may change - the new
window might appear in a non-standard position.  Who's to blame?  The
one who bona fides added the 'window-min-height' constraint in a call or
the people who changed the semantics of 'display-buffer'?

BTW I do not understand well in which sense your original scenario

   (pop-to-buffer (generate-new-buffer "*edit string*")
                  '(display-buffer-below-selected
                    (window-min-height . 10)
                    (window-height . fit-window-to-buffer)))

fails.  Do you mean that the window should be at least ten lines high
despite of the fact that it's fit to an empty buffer?

martin

bug#55169: Can't combine window-min-height with window-height

[Juri Linkov]

> BTW I do not understand well in which sense your original scenario
>
>   (pop-to-buffer (generate-new-buffer "*edit string*")
>                  '(display-buffer-below-selected
>                    (window-min-height . 10)
>                    (window-height . fit-window-to-buffer)))
>
> fails.  Do you mean that the window should be at least ten lines high
> despite of the fact that it's fit to an empty buffer?

Exactly.  I expected that window-min-height takes precedence over window-height.

bug#55169: Can't combine window-min-height with window-height

[martin rudalics]

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

 >> BTW I do not understand well in which sense your original scenario
 >>
 >>    (pop-to-buffer (generate-new-buffer "*edit string*")
 >>                   '(display-buffer-below-selected
 >>                     (window-min-height . 10)
 >>                     (window-height . fit-window-to-buffer)))
 >>
 >> fails.  Do you mean that the window should be at least ten lines high
 >> despite of the fact that it's fit to an empty buffer?
 >
 > Exactly.  I expected that window-min-height takes precedence over window-height.

We could do something like the untested attached .diff.  But we really
should install it only if at least two or three people confirm that it's
the expected behavior.

Note: ‘shrink-window-if-larger-than-buffer’ does not re-enlarge a window
that already shows the buffer if its height is less than the specified
'window-min-height'.

martin

[-- Attachment #2: window--display-buffer.diff --]
[-- Type: text/x-patch, Size: 4504 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index dd297a3169..5643fb9af3 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -7245,8 +7245,12 @@ window--display-buffer
     (let* ((frame (window-frame window))
            (quit-restore (window-parameter window 'quit-restore))
 	   (window-height (assq 'window-height alist))
+           (min-height (or (cdr (assq 'window-min-height alist))
+                           window-min-height))
            (height (cdr window-height))
 	   (window-width (assq 'window-width alist))
+           (min-width (or (cdr (assq 'window-min-width alist))
+                          window-min-width))
            (width (cdr window-width))
            (window-size (assq 'window-size alist))
            (size (cdr window-size))
@@ -7300,26 +7304,31 @@ window--display-buffer
             (setq resize-temp-buffer-window-inhibit 'vertical)))
 	 ((numberp height)
 	  (let* ((new-height
-		  (if (integerp height)
-		      height
-		    (round
-		     (* (window-total-height (frame-root-window window))
-			height))))
+		  (max (if (integerp height)
+		           height
+		         (round
+		          (* (window-total-height (frame-root-window window))
+			     height)))
+                       min-height))
 		 (delta (- new-height (window-total-height window))))
 	    (when (and (window--resizable-p window delta nil 'safe)
 		       (window-combined-p window))
 	      (window-resize window delta nil 'safe)))
           (setq resize-temp-buffer-window-inhibit 'vertical))
          ((and (consp height) (eq (car height) 'body-lines))
-	  (let* ((delta (- (* (frame-char-height frame) (cdr height))
-                           (window-body-height window t))))
-	    (and (window--resizable-p window delta nil 'safe nil nil nil t)
+	  (let* ((delta (max (- (* (frame-char-height frame) (cdr height))
+                                (window-body-height window t))
+                             (- (* (frame-char-height frame) min-height)
+                                (window-pixel-height window)))))
+            (and (window--resizable-p window delta nil 'safe nil nil nil t)
 		 (window-combined-p window)
 	         (window-resize window delta nil 'safe t)))
           (setq resize-temp-buffer-window-inhibit 'vertical))
          ((functionp height)
-	  (ignore-errors (funcall height window))
-          (setq resize-temp-buffer-window-inhibit 'vertical)))
+          (let* ((min-height (cdr (assq 'window-min-height alist)))
+                 (window-min-height min-height))
+            (ignore-errors (funcall height window))
+            (setq resize-temp-buffer-window-inhibit 'vertical))))
 	;; Adjust width of window if asked for.
 	(cond
 	 ((not width)
@@ -7327,26 +7336,31 @@ window--display-buffer
             (setq resize-temp-buffer-window-inhibit 'horizontal)))
 	 ((numberp width)
 	  (let* ((new-width
-		  (if (integerp width)
-		      width
-		    (round
-		     (* (window-total-width (frame-root-window window))
-			width))))
+                  (max
+                   (if (integerp width)
+		       width
+		     (round
+		      (* (window-total-width (frame-root-window window))
+			 width)))
+                   min-width))
 		 (delta (- new-width (window-total-width window))))
 	    (when (and (window--resizable-p window delta t 'safe)
 		       (window-combined-p window t))
 	      (window-resize window delta t 'safe)))
           (setq resize-temp-buffer-window-inhibit 'horizontal))
          ((and (consp width) (eq (car width) 'body-columns))
-	  (let* ((delta (- (* (frame-char-width frame) (cdr width))
-                           (window-body-width window t))))
+	  (let* ((delta (max (- (* (frame-char-width frame) (cdr width))
+                                (window-body-width window t))
+                             (- (* (frame-char-width frame) min-width)
+                                (window-pixel-width window)))))
 	    (and (window--resizable-p window delta t 'safe nil nil nil t)
 		 (window-combined-p window t)
 	         (window-resize window delta t 'safe t)))
           (setq resize-temp-buffer-window-inhibit 'horizontal))
 	 ((functionp width)
-	  (ignore-errors (funcall width window))
-          (setq resize-temp-buffer-window-inhibit 'horizontal)))
+          (let* ((window-min-width min-width))
+	    (ignore-errors (funcall width window))
+            (setq resize-temp-buffer-window-inhibit 'horizontal))))
 
 	;; Preserve window size if asked for.
 	(when (consp preserve-size)

bug#55169: Can't combine window-min-height with window-height

[Juri Linkov]

>>> BTW I do not understand well in which sense your original scenario
>>>
>>>    (pop-to-buffer (generate-new-buffer "*edit string*")
>>>                   '(display-buffer-below-selected
>>>                     (window-min-height . 10)
>>>                     (window-height . fit-window-to-buffer)))
>>>
>>> fails.  Do you mean that the window should be at least ten lines high
>>> despite of the fact that it's fit to an empty buffer?
>>
>> Exactly.  I expected that window-min-height takes precedence over window-height.
>
> We could do something like the untested attached .diff.

Thanks, I confirm that it works correctly.

> But we really should install it only if at least two or three people
> confirm that it's the expected behavior.

Do you think it's a too big change in behavior that might break some
existing configs?

bug#55169: Can't combine window-min-height with window-height

[martin rudalics]

 > Do you think it's a too big change in behavior that might break some
 > existing configs?

It introduces a certain inconsistency.  So far, 'window-min-height' is
obeyed by 'display-buffer-below-selected' only.  My patch would have
other clients obey it as well - but to a certain extent only.  For
example, 'split-window-sensibly' will never care whether a window it
splits off can be made 'window-min-height' lines high.  How would we
document that fact?

martin

bug#55169: Can't combine window-min-height with window-height

[Juri Linkov]

>> Do you think it's a too big change in behavior that might break some
>> existing configs?
>
> It introduces a certain inconsistency.  So far, 'window-min-height' is
> obeyed by 'display-buffer-below-selected' only.  My patch would have
> other clients obey it as well - but to a certain extent only.  For
> example, 'split-window-sensibly' will never care whether a window it
> splits off can be made 'window-min-height' lines high.  How would we
> document that fact?

I didn't expect that 'split-window-sensibly' should pay attention
to 'window-min-height'.  I though only 'display-buffer-*' are affected.

bug#55169: Can't combine window-min-height with window-height

[martin rudalics]

 > I didn't expect that 'split-window-sensibly' should pay attention
 > to 'window-min-height'.  I though only 'display-buffer-*' are affected.

'split-window-sensibly' is the function 'display-buffer-pop-up-window'
runs by default to make a new window.  The dilemma is the following:
'display-buffer-below-selected', before it tries to split the selected
window, asserts that

              (or (not (numberp min-height))
		 (window-sizable-p nil (- min-height)))

where 'min-height' is (cdr (assq 'window-min-height alist)).

'split-window-sensibly', however, may split off a window that is less
than 'min-height' lines high and cannot be enlarged afterwards either.
So far, this is OK because in the manual, in wise foresight, we say for
‘window-min-height’ that "The only client of this entry is presently
‘display-buffer-below-selected’".  But if we obey 'window-min-height' in
'window--display-buffer', then 'display-buffer-pop-up-window' becomes a
client of 'window-min-height' without, however, fulfilling its premise
which says that "If ALIST contains a `window-min-height' entry, this
function ensures that the window used is or can become at least as high
as specified by that entry's value.".

We obviously could call 'split-window-sensibly' with an extra ALIST
argument and have it obey the 'window-min-height' constraint.  But
'split-window-sensibly' is just the default value of the option
'split-window-preferred-function' (all these were invented some time
before 'display-buffer-alist') and that function accepts only one
argument.  So we would have to amend 'window--try-to-split-window' as

		   (or (condition-case nil
		           (funcall split-window-preferred-function window alist)
		         (error nil))
		       (condition-case nil
		           (funcall split-window-preferred-function window)
		         (error nil)))))

No great deal but spilling out the details in the manual will inevitably
increase the mythical confusion around 'display-buffer-alist'.

martin

bug#55169: Can't combine window-min-height with window-height

[Juri Linkov]

> No great deal but spilling out the details in the manual will inevitably
> increase the mythical confusion around 'display-buffer-alist'.

Also this increases the likelihood of regressions.
For example, with your latest patch, *Completions*
are no more fit into a small window, e.g.
'C-h f car TAB TAB' pops up too high window.

bug#55169: Can't combine window-min-height with window-height

[martin rudalics]

 > Also this increases the likelihood of regressions.
 > For example, with your latest patch, *Completions*
 > are no more fit into a small window, e.g.
 > 'C-h f car TAB TAB' pops up too high window.

Likely because the *Completions* window would become smaller than
'window-min-height'.  Either decrease that or provide an explicit
'window-min-height' alist entry of 0 in the 'display-buffer' call.

It's a can or worms we'd open here.

martin

bug#55169: Can't combine window-min-height with window-height

[Juri Linkov]

>> Also this increases the likelihood of regressions.
>> For example, with your latest patch, *Completions*
>> are no more fit into a small window, e.g.
>> 'C-h f car TAB TAB' pops up too high window.
>
> Likely because the *Completions* window would become smaller than
> 'window-min-height'.  Either decrease that or provide an explicit
> 'window-min-height' alist entry of 0 in the 'display-buffer' call.
>
> It's a can or worms we'd open here.

Do you think it would be simpler and thus more reliable
to enforce height constraints such as min-height
in fit-window-to-buffer?

This will be very clear to users to see how it works:

  (pop-to-buffer (generate-new-buffer "*edit string*")
                 '(display-buffer-below-selected
                   (window-height (fit-window-to-buffer 10))))

where 10 is min-height.

Because it should be the task of fit-window-to-buffer,
and it already has the argument MIN-HEIGHT.
Also other arguments of fit-window-to-buffer
could be allowed to use in display-buffer-alist.

bug#55169: Can't combine window-min-height with window-height

[martin rudalics]

 > Do you think it would be simpler and thus more reliable
 > to enforce height constraints such as min-height
 > in fit-window-to-buffer?
 >
 > This will be very clear to users to see how it works:
 >
 >    (pop-to-buffer (generate-new-buffer "*edit string*")
 >                   '(display-buffer-below-selected
 >                     (window-height (fit-window-to-buffer 10))))
 >
 > where 10 is min-height.
 >
 > Because it should be the task of fit-window-to-buffer,
 > and it already has the argument MIN-HEIGHT.
 > Also other arguments of fit-window-to-buffer
 > could be allowed to use in display-buffer-alist.

You mean the value of a 'window-height' entry could also be a

         • A list whose CAR is a function and whose remaining elements
           are the arguments passed to that function.

so we'd run

	    (ignore-errors (funcall (car height) window (nth 1 height) ...))

in 'window--display-buffer'?  Which means the caller has to know the
function and which arguments it needs in which order.  Sounds fragile.

martin

bug#55169: Can't combine window-min-height with window-height

[Juri Linkov]

>> Because it should be the task of fit-window-to-buffer,
>> and it already has the argument MIN-HEIGHT.
>> Also other arguments of fit-window-to-buffer
>> could be allowed to use in display-buffer-alist.
>
> You mean the value of a 'window-height' entry could also be a
>
>         • A list whose CAR is a function and whose remaining elements
>           are the arguments passed to that function.
>
> so we'd run
>
> 	    (ignore-errors (funcall (car height) window (nth 1 height) ...))
>
> in 'window--display-buffer'?  Which means the caller has to know the
> function and which arguments it needs in which order.  Sounds fragile.

It is intended only for special handling of 'fit-window-to-buffer'.
If 'fit-window-to-buffer' with MIN-HEIGHT will really fix this problem,
then details of the entry format could be adapted later.
And indeed this works exactly as should with

(pop-to-buffer (generate-new-buffer "*edit string*")
                 '(display-buffer-below-selected
                   (window-height . fit-window-to-buffer)))

and an experimental patch:

diff --git a/lisp/window.el b/lisp/window.el
index 9f78784612..a21cf109a4 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -7336,7 +7336,9 @@ window--display-buffer
 	         (window-resize window delta nil 'safe t)))
           (setq resize-temp-buffer-window-inhibit 'vertical))
          ((functionp height)
-	  (ignore-errors (funcall height window))
+	  (if (eq height 'fit-window-to-buffer)
+              (ignore-errors (funcall height window nil 10))
+            (ignore-errors (funcall height window)))
           (setq resize-temp-buffer-window-inhibit 'vertical)))
 	;; Adjust width of window if asked for.
 	(cond

bug#55169: Can't combine window-min-height with window-height

[martin rudalics]

 >> You mean the value of a 'window-height' entry could also be a
 >>
 >>          • A list whose CAR is a function and whose remaining elements
 >>            are the arguments passed to that function.
 >>
 >> so we'd run
 >>
 >> 	    (ignore-errors (funcall (car height) window (nth 1 height) ...))
 >>
 >> in 'window--display-buffer'?  Which means the caller has to know the
 >> function and which arguments it needs in which order.  Sounds fragile.
 >
 > It is intended only for special handling of 'fit-window-to-buffer'.
 > If 'fit-window-to-buffer' with MIN-HEIGHT will really fix this problem,
 > then details of the entry format could be adapted later.
 > And indeed this works exactly as should with
 >
 > (pop-to-buffer (generate-new-buffer "*edit string*")
 >                   '(display-buffer-below-selected
 >                     (window-height . fit-window-to-buffer)))
 >
 > and an experimental patch:
 >
 > diff --git a/lisp/window.el b/lisp/window.el
 > index 9f78784612..a21cf109a4 100644
 > --- a/lisp/window.el
 > +++ b/lisp/window.el
 > @@ -7336,7 +7336,9 @@ window--display-buffer
 >   	         (window-resize window delta nil 'safe t)))
 >             (setq resize-temp-buffer-window-inhibit 'vertical))
 >            ((functionp height)
 > -	  (ignore-errors (funcall height window))
 > +	  (if (eq height 'fit-window-to-buffer)
 > +              (ignore-errors (funcall height window nil 10))
 > +            (ignore-errors (funcall height window)))
 >             (setq resize-temp-buffer-window-inhibit 'vertical)))
 >   	;; Adjust width of window if asked for.
 >   	(cond

This boils down to what I sketched above: ALIST must supply a list whose
car is 'fit-window-to-buffer' and 'window--display-buffer' supplies the
window used as first argument in its call to 'fit-window-to-buffer' and
the rest of the list as the remaining arguments.

martin

bug#55169: Can't combine window-min-height with window-height

[Juri Linkov]

>>> You mean the value of a 'window-height' entry could also be a
>>>
>>>          • A list whose CAR is a function and whose remaining elements
>>>            are the arguments passed to that function.
>>>
>>> so we'd run
>>>
>>> 	    (ignore-errors (funcall (car height) window (nth 1 height) ...))
>> ...
> This boils down to what I sketched above: ALIST must supply a list whose
> car is 'fit-window-to-buffer' and 'window--display-buffer' supplies the
> window used as first argument in its call to 'fit-window-to-buffer' and
> the rest of the list as the remaining arguments.

Then this could be an easy to explain rule.  I'm only unsure about the
WINDOW argument.  What if another function has a WINDOW argument in
another position, not the first?  Maybe better to allow a lambda like this:

(pop-to-buffer (generate-new-buffer "*edit string*")
               '(display-buffer-below-selected
                 (window-height . (lambda (window)
                                    (fit-window-to-buffer window nil 10)))))

bug#55169: Can't combine window-min-height with window-height

[martin rudalics]

 > Then this could be an easy to explain rule.  I'm only unsure about the
 > WINDOW argument.  What if another function has a WINDOW argument in
 > another position, not the first?  Maybe better to allow a lambda like this:
 >
 > (pop-to-buffer (generate-new-buffer "*edit string*")
 >                 '(display-buffer-below-selected
 >                   (window-height . (lambda (window)
 >                                      (fit-window-to-buffer window nil 10)))))

Sounds too complicated IMO.  Let's stick to your earlier proposal as

	  (ignore-errors
             (if (eq height 'fit-window-to-buffer)
                 (fit-window-to-buffer
                  window nil (cdr (assq 'window-min-height alist)))
               (funcall height window)))

say something like "As a special case, if `window-height' specifies
`fit-window-to-buffer' and ALIST contains a `window-min-height' entry,
pass the value of that entry as MIN-HEIGHT argument to
`fit-window-to-buffer'." and do a similar thing for `window-min-width'.

And if we really want to, we can always add a MIN-HEIGHT argument to
'shrink-window-if-larger-than-buffer' and handle it the same way.

martin

bug#55169: Can't combine window-min-height with window-height

[Juri Linkov]

>> (pop-to-buffer (generate-new-buffer "*edit string*")
>>                 '(display-buffer-below-selected
>>                   (window-height . (lambda (window)
>>                                      (fit-window-to-buffer window nil 10)))))
>
> Sounds too complicated IMO.

Actually a lambda is already supported, so no changes are required.

> Let's stick to your earlier proposal as
>
> 	  (ignore-errors
>             (if (eq height 'fit-window-to-buffer)
>                 (fit-window-to-buffer
>                  window nil (cdr (assq 'window-min-height alist)))
>               (funcall height window)))
>
> say something like "As a special case, if `window-height' specifies
> `fit-window-to-buffer' and ALIST contains a `window-min-height' entry,
> pass the value of that entry as MIN-HEIGHT argument to
> `fit-window-to-buffer'." and do a similar thing for `window-min-width'.

A special case for fit-window-to-buffer, shrink-window-* and other similar
could be implemented only when you are sure that a `window-min-height' entry
is not going to be used for other purposes besides of using it as an argument
of a window-height entry.

BTW, fit-window-to-buffer has also other arguments: MAX-WIDTH and MIN-WIDTH.
Would they be useful for a window-width entry?

> And if we really want to, we can always add a MIN-HEIGHT argument to
> 'shrink-window-if-larger-than-buffer' and handle it the same way.

This is very much needed, thanks.

bug#55169: Can't combine window-min-height with window-height

[martin rudalics]

 >>> (pop-to-buffer (generate-new-buffer "*edit string*")
 >>>                  '(display-buffer-below-selected
 >>>                    (window-height . (lambda (window)
 >>>                                       (fit-window-to-buffer window nil 10)))))
 >>
 >> Sounds too complicated IMO.
 >
 > Actually a lambda is already supported, so no changes are required.

We have to find a suitable way to explain that in the manual.

 > A special case for fit-window-to-buffer, shrink-window-* and other similar
 > could be implemented only when you are sure that a `window-min-height' entry
 > is not going to be used for other purposes besides of using it as an argument
 > of a window-height entry.

You said that this is the case for 'fit-window-to-buffer' and that
function is the canonical client for a 'window-height' function.  So why
bother about the rest?

 > BTW, fit-window-to-buffer has also other arguments: MAX-WIDTH and MIN-WIDTH.
 > Would they be useful for a window-width entry?

Maybe.  But let's talk about MAX-HEIGHT first.

 >> And if we really want to, we can always add a MIN-HEIGHT argument to
 >> 'shrink-window-if-larger-than-buffer' and handle it the same way.
 >
 > This is very much needed, thanks.

See below (this should also fix the lie about the return value).

martin


(defun shrink-window-if-larger-than-buffer (&optional window min-height)
   "Shrink height of WINDOW if its buffer doesn't need so many lines.
More precisely, shrink WINDOW vertically to be as small as
possible, while still showing the full contents of its buffer.
WINDOW must be a live window and defaults to the selected one.

The optional argument MIN-HEIGHT specifies the minimum number of
lines to which WINDOW may be shrunk and defaults to
`window-min-height'.

Do nothing if the buffer contains more lines than the present
height of WINDOW, some of WINDOW's contents are scrolled out of
view, shrinking WINDOW would also shrink another window, or
WINDOW is the root window of its frame.

Return non-nil if WINDOW was shrunk, nil otherwise."
   (interactive)
   (setq window (window-normalize-window window t))
   ;; Make sure that WINDOW is vertically combined and `point-min' is
   ;; visible (for whatever reason that's needed).  The remaining issues
   ;; should be taken care of by `fit-window-to-buffer'.
   (when (and (window-combined-p window)
	     (pos-visible-in-window-p (point-min) window))
     (let ((old-height (window-pixel-height window)))
       (fit-window-to-buffer
        window (window-total-height window) nil min-height)
       (/= old-height (window-pixel-height window)))))

bug#55169: Can't combine window-min-height with window-height

[martin rudalics]

 >        (/= old-height (window-pixel-height window)))))

Better

         (> old-height (window-pixel-height window)))))

since in some corner cases 'fit-window-to-buffer' might have grown
WINDOW by a few pixels here.

martin

bug#55169: Can't combine window-min-height with window-height

[Juri Linkov]

>>>> (pop-to-buffer (generate-new-buffer "*edit string*")
>>>>                  '(display-buffer-below-selected
>>>>                    (window-height . (lambda (window)
>>>>                                       (fit-window-to-buffer window nil 10)))))
>>>
>>> Sounds too complicated IMO.
>>
>> Actually a lambda is already supported, so no changes are required.
>
> We have to find a suitable way to explain that in the manual.

It's already documented in the manual:

        • If the value specifies a function, that function is called
          with one argument—the chosen window.  The function is supposed
          to adjust the height of the window; its return value is
          ignored.  Suitable functions are ‘fit-window-to-buffer’ and
          ‘shrink-window-if-larger-than-buffer’, see *note Resizing
          Windows::.

It would be nice to add just a small example of a lambda with more args
for fit-window-to-buffer or shrink-window-if-larger-than-buffer.

>> A special case for fit-window-to-buffer, shrink-window-* and other similar
>> could be implemented only when you are sure that a `window-min-height' entry
>> is not going to be used for other purposes besides of using it as an argument
>> of a window-height entry.
>
> You said that this is the case for 'fit-window-to-buffer' and that
> function is the canonical client for a 'window-height' function.  So why
> bother about the rest?

You found another function shrink-window-if-larger-than-buffer.
Then it would be better not to limit only to these two.

>> BTW, fit-window-to-buffer has also other arguments: MAX-WIDTH and MIN-WIDTH.
>> Would they be useful for a window-width entry?
>
> Maybe.  But let's talk about MAX-HEIGHT first.

Maybe MAX-WIDTH is not needed after all, as much as MAX-HEIGHT.

>>> And if we really want to, we can always add a MIN-HEIGHT argument to
>>> 'shrink-window-if-larger-than-buffer' and handle it the same way.
>>
>> This is very much needed, thanks.
>
> See below (this should also fix the lie about the return value).

Thanks, I'll test it.  Does it need more args from fit-window-to-buffer?

bug#55169: Can't combine window-min-height with window-height

[martin rudalics]

 > It would be nice to add just a small example of a lambda with more args
 > for fit-window-to-buffer or shrink-window-if-larger-than-buffer.

Maybe show the code you intend to use and how the user could override it
with 'shrink-window-if-larger-than-buffer' in 'display-buffer-alist'.

 > You found another function shrink-window-if-larger-than-buffer.
 > Then it would be better not to limit only to these two.

OK.

 >>> BTW, fit-window-to-buffer has also other arguments: MAX-WIDTH and MIN-WIDTH.
 >>> Would they be useful for a window-width entry?
 >>
 >> Maybe.  But let's talk about MAX-HEIGHT first.
 >
 > Maybe MAX-WIDTH is not needed after all, as much as MAX-HEIGHT.

Currently, we don't have to bother anyway.  The only client of this is
'display-buffer-below-selected' and that hardly produces a horizontal
combination.

 > Thanks, I'll test it.  Does it need more args from fit-window-to-buffer?

None that I know of.

martin

bug#55169: Can't combine window-min-height with window-height

[Juri Linkov]

>> It would be nice to add just a small example of a lambda with more args
>> for fit-window-to-buffer or shrink-window-if-larger-than-buffer.
>
> Maybe show the code you intend to use and how the user could override it
> with 'shrink-window-if-larger-than-buffer' in 'display-buffer-alist'.

I have an example only for 'fit-window-to-buffer':

  (pop-to-buffer (generate-new-buffer "*edit string*")
                 '(display-buffer-below-selected
                   (window-height . (lambda (window)
                                      (fit-window-to-buffer window nil 10)))))

>>>> BTW, fit-window-to-buffer has also other arguments: MAX-WIDTH and MIN-WIDTH.
>>>> Would they be useful for a window-width entry?
>>>
>>> Maybe.  But let's talk about MAX-HEIGHT first.
>>
>> Maybe MAX-WIDTH is not needed after all, as much as MAX-HEIGHT.
>
> Currently, we don't have to bother anyway.  The only client of this is
> 'display-buffer-below-selected' and that hardly produces a horizontal
> combination.

I wonder why other 'display-buffer-*' function wouldn't need the same?

bug#55169: Can't combine window-min-height with window-height

[martin rudalics]

 > I have an example only for 'fit-window-to-buffer':
 >
 >    (pop-to-buffer (generate-new-buffer "*edit string*")
 >                   '(display-buffer-below-selected
 >                     (window-height . (lambda (window)
 >                                        (fit-window-to-buffer window nil 10)))))

I meant an example for how to put a lambda into 'display-buffer-alist'.

In either case, I still think we should provide an option where we pass
the window _and_ the alist to that function.  Your example illustrates
well that we cannot integrate a user supplied 'window-min-height' entry
say into the application provided lambda.  Vice versa, user supplied
functions cannot interact with application provided 'window-min-height'
entries.

Think of an application suggesting 'fit-window-to-buffer' with some
'window-min-height' value and a user who wants to use
'shrink-window-if-larger-than-buffer' instead but still use the
'window-min-height' value proposed by the application.  How would we do
that?

 > I wonder why other 'display-buffer-*' function wouldn't need the same?

All of 'display-buffer-same-window', 'display-buffer-reuse-window' and
'display-buffer-pop-up-window' should get the same treatment, probably.

martin

bug#55169: Can't combine window-min-height with window-height

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

          > • If the value specifies a function, that function is called
          >   with one argument—the chosen window.  The function is supposed
          >   to adjust the height of the window; its return value is
          >   ignored.  Suitable functions are ‘fit-window-to-buffer’ and
          >   ‘shrink-window-if-larger-than-buffer’, see *note Resizing
          >   Windows::.

That text is hard to read due to use of "that function" rather than
some sort of name.  Would someone please improve this by using a
metasyntactic variable to refer to it?  It could be
"@var{size-action}", perhaps.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#55169: Can't combine window-min-height with window-height

[Juri Linkov]

>           > • If the value specifies a function, that function is called
>           >   with one argument—the chosen window.  The function is supposed
>           >   to adjust the height of the window; its return value is
>           >   ignored.  Suitable functions are ‘fit-window-to-buffer’ and
>           >   ‘shrink-window-if-larger-than-buffer’, see *note Resizing
>           >   Windows::.
>
> That text is hard to read due to use of "that function" rather than
> some sort of name.  Would someone please improve this by using a
> metasyntactic variable to refer to it?  It could be
> "@var{size-action}", perhaps.

This excerpt was shown out of context.  In the manual this
metasyntactic variable is named ‘window-height’.

bug#55169: Can't combine window-min-height with window-height

[Juri Linkov]

>> I have an example only for 'fit-window-to-buffer':
>>
>>    (pop-to-buffer (generate-new-buffer "*edit string*")
>>                   '(display-buffer-below-selected
>>                     (window-height . (lambda (window)
>>                                        (fit-window-to-buffer window nil 10)))))
>
> I meant an example for how to put a lambda into 'display-buffer-alist'.
>
> In either case, I still think we should provide an option where we pass
> the window _and_ the alist to that function.  Your example illustrates
> well that we cannot integrate a user supplied 'window-min-height' entry
> say into the application provided lambda.  Vice versa, user supplied
> functions cannot interact with application provided 'window-min-height'
> entries.

Adding an alist is a good idea, but unfortunately it's not backwards-compatible.

> Think of an application suggesting 'fit-window-to-buffer' with some
> 'window-min-height' value and a user who wants to use
> 'shrink-window-if-larger-than-buffer' instead but still use the
> 'window-min-height' value proposed by the application.  How would we do
> that?

I have no idea how to make this backwards-compatible.

>> I wonder why other 'display-buffer-*' function wouldn't need the same?
>
> All of 'display-buffer-same-window', 'display-buffer-reuse-window' and
> 'display-buffer-pop-up-window' should get the same treatment, probably.

Definitively the functions that create a new window
should take into account height constraints.
But not sure about functions that select the existing window.
Should they resize the existing window?

bug#55169: Can't combine window-min-height with window-height

[martin rudalics]

 > Adding an alist is a good idea, but unfortunately it's not backwards-compatible.

That's what I'm trying to bring up ever since we started this thread.
AFAICT we'd need two new functions at least - an action function say
'display-buffer-split-window' and a 'dislay-buffer-fit-window' function
- both accepting the ALIST as (additional) argument.

 >> Think of an application suggesting 'fit-window-to-buffer' with some
 >> 'window-min-height' value and a user who wants to use
 >> 'shrink-window-if-larger-than-buffer' instead but still use the
 >> 'window-min-height' value proposed by the application.  How would we do
 >> that?
 >
 > I have no idea how to make this backwards-compatible.

It can be done with global variables but the resulting code will be
neither elegant nor comprehensible.  IMO any such information must be
inevitably passed on through the ALIST argument.

 > Definitively the functions that create a new window
 > should take into account height constraints.
 > But not sure about functions that select the existing window.
 > Should they resize the existing window?

They do so already.  With emacs -Q do

(pop-to-buffer (get-buffer-create "*edit string*")
                '(display-buffer-below-selected
                  (window-height . (lambda (window)
                                     (fit-window-to-buffer window nil 10)))))

followed by

(pop-to-buffer (get-buffer-create "*edit string*")
                '(display-buffer-below-selected
                  (window-height . (lambda (window)
                                     (fit-window-to-buffer window nil 20)))))

martin

bug#55169: Can't combine window-min-height with window-height

[Juri Linkov]

>> Adding an alist is a good idea, but unfortunately it's not backwards-compatible.
>
> That's what I'm trying to bring up ever since we started this thread.
> AFAICT we'd need two new functions at least - an action function say
> 'display-buffer-split-window'

Can the existing 'display-buffer-in-direction' be used to split window?
Or a new action 'display-buffer-split-in-direction' is needed?

> and a 'dislay-buffer-fit-window' function
> - both accepting the ALIST as (additional) argument.

But they can't be used simultaneously?  When 'display-buffer-split-window'
splits the window, then the next 'dislay-buffer-fit-window' is not called?

>>> Think of an application suggesting 'fit-window-to-buffer' with some
>>> 'window-min-height' value and a user who wants to use
>>> 'shrink-window-if-larger-than-buffer' instead but still use the
>>> 'window-min-height' value proposed by the application.  How would we do
>>> that?
>>
>> I have no idea how to make this backwards-compatible.
>
> It can be done with global variables but the resulting code will be
> neither elegant nor comprehensible.  IMO any such information must be
> inevitably passed on through the ALIST argument.

Maybe a new alist key could be added, e.g. 'window-fit',
that is like 'window-height' but also adds the new arg 'alist'?

>> Definitively the functions that create a new window
>> should take into account height constraints.
>> But not sure about functions that select the existing window.
>> Should they resize the existing window?
>
> They do so already.  With emacs -Q do
>
> (pop-to-buffer (get-buffer-create "*edit string*")
>                '(display-buffer-below-selected
>                  (window-height . (lambda (window)
>                                     (fit-window-to-buffer window nil 10)))))
>
> followed by
>
> (pop-to-buffer (get-buffer-create "*edit string*")
>                '(display-buffer-below-selected
>                  (window-height . (lambda (window)
>                                     (fit-window-to-buffer window nil 20)))))

Nice!

bug#55169: Can't combine window-min-height with window-height

[martin rudalics]

 >> That's what I'm trying to bring up ever since we started this thread.
 >> AFAICT we'd need two new functions at least - an action function say
 >> 'display-buffer-split-window'
 >
 > Can the existing 'display-buffer-in-direction' be used to split window?

Yes.  I've rewritten it long ago but it now contains references to
functions based on scaling the body sizes of windows and implementing
these has stalled.  Eventually, 'display-buffer-in-direction' should
replace 'display-buffer-pop-up-window' and its heuristics ...

 > Or a new action 'display-buffer-split-in-direction' is needed?

No.  The "-direction" suffix should tell enough about where the window
should go (incidentally, nowadays many people use side windows for that
purpose).

 >> and a 'dislay-buffer-fit-window' function
 >> - both accepting the ALIST as (additional) argument.
 >
 > But they can't be used simultaneously?  When 'display-buffer-split-window'
 > splits the window, then the next 'dislay-buffer-fit-window' is not called?

Sorry, I meant 'dislay-buffer--fit-window'.  Not an action function but
one that would fit or shrink the window and accept the ALIST as extra
argument.  So it could process all sorts of minimum or maximum heights
the user or application wants.

 > Maybe a new alist key could be added, e.g. 'window-fit',
 > that is like 'window-height' but also adds the new arg 'alist'?

Something like that, yes.  It would end up calling the
'dislay-buffer--fit-window' function mentioned above.

martin

bug#55169: Can't combine window-min-height with window-height

[Juri Linkov]

> (defun shrink-window-if-larger-than-buffer (&optional window min-height)
>   "Shrink height of WINDOW if its buffer doesn't need so many lines.
> More precisely, shrink WINDOW vertically to be as small as
> possible, while still showing the full contents of its buffer.
> WINDOW must be a live window and defaults to the selected one.
>
> The optional argument MIN-HEIGHT specifies the minimum number of
> lines to which WINDOW may be shrunk and defaults to
> `window-min-height'.
>
> Do nothing if the buffer contains more lines than the present
> height of WINDOW, some of WINDOW's contents are scrolled out of
> view, shrinking WINDOW would also shrink another window, or
> WINDOW is the root window of its frame.
>
> Return non-nil if WINDOW was shrunk, nil otherwise."
>   (interactive)
>   (setq window (window-normalize-window window t))
>   ;; Make sure that WINDOW is vertically combined and `point-min' is
>   ;; visible (for whatever reason that's needed).  The remaining issues
>   ;; should be taken care of by `fit-window-to-buffer'.
>   (when (and (window-combined-p window)
> 	     (pos-visible-in-window-p (point-min) window))
>     (let ((old-height (window-pixel-height window)))
>       (fit-window-to-buffer
>        window (window-total-height window) nil min-height)

Thanks, this works fine, after removing nil above.

bug#55169: Can't combine window-min-height with window-height

[martin rudalics]

 > Thanks, this works fine, after removing nil above.

Thanks for catching it.

So what shall I do?  Check it in?

martin

bug#55169: Can't combine window-min-height with window-height

[Juri Linkov]

>> Thanks, this works fine, after removing nil above.
>
> Thanks for catching it.
>
> So what shall I do?  Check it in?

It looks safe to check in since it's a backward-compatible change.