bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[Davor Rotim]

Testing with 'emacs -Q' and the following snippet:

(add-to-list 'display-buffer-alist
             '("\\*Completions\\*"
               (display-buffer-in-side-window)
               (window-height . 0.05)
               (side . bottom)
               (slot . 0)
               (window-parameters . ((no-other-window . t)))))

Seems like the 'window-height' parameter is being ignored and I'm unable
to set the height for the *Completions* window this way.

Switching window direction to 'left' or 'right' and setting
'window-width' instead of height seems to be working with this altered
snippet:

(add-to-list 'display-buffer-alist
             '("\\*Completions\\*"
               (display-buffer-in-side-window)
               (window-width . 0.05)
               (side . right)
               (slot . 0)
               (window-parameters . ((no-other-window . t)))))

The 'window-height' parameter gets ignored only when specifying the
'top' or 'bottom' side.


In GNU Emacs 27.0.90 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.14, cairo version 1.17.3)
 of 2020-02-28 built on nostromo
Repository revision: 696ee02c3a40cf0e19f963cfaf8004ca42f7e897
Repository branch: emacs-27
Windowing system distributor 'The X.Org Foundation', version 11.0.12007000
System Description: Arch Linux

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Mark set
(("\\*Completions\\*" (display-buffer-in-side-window) (window-width . 0.05) (side . right) (slot . 0) (window-parameters (no-other-window . t))))
Quit
Configured using:
 'configure 'CFLAGS=-march=native -O2 -pipe -fstack-protector-strong
 -fno-plt' --prefix=/home/drot/.local
 '--program-transform-name=s/^ctags$/ctags.emacs/' --with-cairo
 --with-modules --enable-link-time-optimization --disable-gcc-warnings'

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS LIBSYSTEMD JSON
PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame minibuffer cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
cairo move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 44881 10781)
 (symbols 48 6003 1)
 (strings 32 15441 1795)
 (string-bytes 1 513956)
 (vectors 16 9962)
 (vector-slots 8 129161 9124)
 (floats 8 20 43)
 (intervals 56 201 0)
 (buffers 1000 11))

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[martin rudalics]

 > Testing with 'emacs -Q' and the following snippet:
 >
 > (add-to-list 'display-buffer-alist
 >               '("\\*Completions\\*"
 >                 (display-buffer-in-side-window)
 >                 (window-height . 0.05)
 >                 (side . bottom)
 >                 (slot . 0)
 >                 (window-parameters . ((no-other-window . t)))))
 >
 > Seems like the 'window-height' parameter is being ignored and I'm unable
 > to set the height for the *Completions* window this way.

You _are_ able to do that since

(display-buffer (get-buffer-create "*Completions*"))

displays the buffer as intended within the bounds of the value you
supplied, the size of the frame, the number of windows it shows ...

 > Switching window direction to 'left' or 'right' and setting
 > 'window-width' instead of height seems to be working with this altered
 > snippet:
 >
 > (add-to-list 'display-buffer-alist
 >               '("\\*Completions\\*"
 >                 (display-buffer-in-side-window)
 >                 (window-width . 0.05)
 >                 (side . right)
 >                 (slot . 0)
 >                 (window-parameters . ((no-other-window . t)))))
 >
 > The 'window-height' parameter gets ignored only when specifying the
 > 'top' or 'bottom' side.

What happens is that the function responsible for displaying completions
('minibuffer-completion-help') uses the 'with-displayed-buffer-window'
macro and supplies it with a

	       '(window-height . fit-window-to-buffer)

argument.  'with-displayed-buffer-window', in these two parts

               (vheight-function
                (let ((window-height (assq 'window-height (cdr ,vaction))))
                  (when (functionp (cdr window-height))
                    (cdr window-height))))

and

	 (when vheight-function
	   (ignore-errors
	     (set-window-parameter ,window 'preserve-size nil)
              (funcall vheight-function ,window)))

uses the above supplied 'fit-window-to-buffer' to override the 0.05
window-height value supplied by your customization.

When you display *Completions* in a side window below or above an
already existing side window, you may observe a similar effect for a
side window on the left or right of your frame.  And if
'fit-window-to-buffer-horizontally' is non-nil, you may see the effect
even when there is only one side window on the left or right even when
window-width is 0.05.

Strictly spoken, the behavior you describe is a bug because
'minibuffer-completion-help' violates the contract obligations of
'display-buffer'.  But displaying completions had its own rules ever
since so I'm not sure what to suggest.

martin

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[Davor Rotim]

Hello Martin,

martin rudalics <rudalics@gmx.at> writes:

> Strictly spoken, the behavior you describe is a bug because
> 'minibuffer-completion-help' violates the contract obligations of
> 'display-buffer'.  But displaying completions had its own rules ever
> since so I'm not sure what to suggest.
>
> martin

I'm not seeing any adverse effects by obeying the contract:

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 49daabc..64139bf 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1980,10 +1980,8 @@ variables.")
              ,(if (eq (selected-window) (minibuffer-window))
                   'display-buffer-at-bottom
                 'display-buffer-below-selected))
-           ,(if temp-buffer-resize-mode
-                '(window-height . resize-temp-buffer-window)
-              '(window-height . fit-window-to-buffer))
            ,(when temp-buffer-resize-mode
+               '(window-height . resize-temp-buffer-window)
               '(preserve-size . (nil . t))))
           nil
           ;; Remove the base-size tail because `sort' requires a properly

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[martin rudalics]

 > I'm not seeing any adverse effects by obeying the contract:

Neither would I ...

 > -           ,(if temp-buffer-resize-mode
 > -                '(window-height . resize-temp-buffer-window)
 > -              '(window-height . fit-window-to-buffer))
 >              ,(when temp-buffer-resize-mode
 > +               '(window-height . resize-temp-buffer-window)
 >                 '(preserve-size . (nil . t))))
 >             nil
 >             ;; Remove the base-size tail because `sort' requires a properly

... because I'm using 'temp-buffer-resize-mode'.  But IIRC (Juri likely
knows better) completions windows are traditionally fit to their buffer
so people who never use 'temp-buffer-resize-mode' would now be surprised
to not see their completions window fit.  So while we probably can't do
what you propose above, we should be able to _not_ fit the window when
the alist already provides an explicit height argument as in your case.

martin

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[Juri Linkov]

>> I'm not seeing any adverse effects by obeying the contract:
>
> Neither would I ...
>
>> -           ,(if temp-buffer-resize-mode
>> -                '(window-height . resize-temp-buffer-window)
>> -              '(window-height . fit-window-to-buffer))
>>              ,(when temp-buffer-resize-mode
>> +               '(window-height . resize-temp-buffer-window)
>>                 '(preserve-size . (nil . t))))
>>             nil
>>             ;; Remove the base-size tail because `sort' requires a properly
>
> ... because I'm using 'temp-buffer-resize-mode'.  But IIRC (Juri likely
> knows better) completions windows are traditionally fit to their buffer
> so people who never use 'temp-buffer-resize-mode' would now be surprised
> to not see their completions window fit.  So while we probably can't do
> what you propose above, we should be able to _not_ fit the window when
> the alist already provides an explicit height argument as in your case.

I'm using 'temp-buffer-resize-mode' too.  I don't know why it's disabled
by default.

My old opinion is that we need to try to get rid of these macros
and rely only on alist values.  So it would be easier to override
the default alist values such as 'window-height'.

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[martin rudalics]

 > I'm using 'temp-buffer-resize-mode' too.  I don't know why it's disabled
 > by default.

IIRC Chong didn't like it.

 > My old opinion is that we need to try to get rid of these macros
 > and rely only on alist values.

You mean 'minibuffer-completion-help' should call 'display-buffer'
directly and do the rest of the setup manually?

 > So it would be easier to override
 > the default alist values such as 'window-height'.

Is there such a default value?  Who would provide it?

martin

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[Juri Linkov]

>> My old opinion is that we need to try to get rid of these macros
>> and rely only on alist values.
>
> You mean 'minibuffer-completion-help' should call 'display-buffer'
> directly and do the rest of the setup manually?

Yes, and all other commands that currently use
with-displayed-buffer-window should get along without it.

>> So it would be easier to override
>> the default alist values such as 'window-height'.
>
> Is there such a default value?  Who would provide it?

Since users should be able to override it by e.g.

(add-to-list 'display-buffer-alist
             '("\\*Completions\\*"
               (window-height . 5)
               ...

a default value should have a lower priority, i.e.
in the call of 'display-buffer' in 'minibuffer-completion-help'.

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[martin rudalics]

 >> You mean 'minibuffer-completion-help' should call 'display-buffer'
 >> directly and do the rest of the setup manually?
 >
 > Yes, and all other commands that currently use
 > with-displayed-buffer-window should get along without it.

OK with me.

 >>> So it would be easier to override
 >>> the default alist values such as 'window-height'.
 >>
 >> Is there such a default value?  Who would provide it?
 >
 > Since users should be able to override it by e.g.
 >
 > (add-to-list 'display-buffer-alist
 >               '("\\*Completions\\*"
 >                 (window-height . 5)
 >                 ...
 >
 > a default value should have a lower priority, i.e.
 > in the call of 'display-buffer' in 'minibuffer-completion-help'.

So you mean the default value is the value supplied by
'display-buffer-alist'.

martin

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[Juri Linkov]

>>> You mean 'minibuffer-completion-help' should call 'display-buffer'
>>> directly and do the rest of the setup manually?
>>
>> Yes, and all other commands that currently use
>> with-displayed-buffer-window should get along without it.
>
> OK with me.

Here is a complete list of commands that use
with-displayed-buffer-window:

- dired-mark-pop-up
- minibuffer-completion-help
- save-buffers-kill-emacs

What they all have in common is that they have some post-processing
in the displayed buffer.  I wonder why there are no more such commands
that need to do such post-processing?  Maybe they use some simpler
solution that could be used here as well?

For example, hack-local-variables-confirm uses just

  (pop-to-buffer "*Local Variables*" '(display-buffer--maybe-at-bottom))

and nothing more, without hassles of with-displayed-buffer-window.
And still it fits the window nicely into the buffer height.
Why the above 3 commands couldn't do the same by abandoning
with-displayed-buffer-window?

> So you mean the default value is the value supplied by
> 'display-buffer-alist'.

Actually, I meant the default value is that used as the ACTION arg of
display-buffer.

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[martin rudalics]

 > Here is a complete list of commands that use
 > with-displayed-buffer-window:
 >
 > - dired-mark-pop-up
 > - minibuffer-completion-help
 > - save-buffers-kill-emacs
 >
 > What they all have in common is that they have some post-processing
 > in the displayed buffer.  I wonder why there are no more such commands
 > that need to do such post-processing?  Maybe they use some simpler
 > solution that could be used here as well?
 >
 > For example, hack-local-variables-confirm uses just
 >
 >    (pop-to-buffer "*Local Variables*" '(display-buffer--maybe-at-bottom))

IIUC it neither runs the hooks for temporary buffers nor does it obey
'temp-buffer-resize-mode'.

 > and nothing more, without hassles of with-displayed-buffer-window.
 > And still it fits the window nicely into the buffer height.

How comes?

 > Why the above 3 commands couldn't do the same by abandoning
 > with-displayed-buffer-window?

It depends on how much of the stuff in 'temp-buffer-window-setup' and
'temp-buffer-window-show' they really need.

martin

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[Juri Linkov]

>> Here is a complete list of commands that use
>> with-displayed-buffer-window:
>>
>> - dired-mark-pop-up
>> - minibuffer-completion-help
>> - save-buffers-kill-emacs
>>
>> What they all have in common is that they have some post-processing
>> in the displayed buffer.  I wonder why there are no more such commands
>> that need to do such post-processing?  Maybe they use some simpler
>> solution that could be used here as well?
>>
>> For example, hack-local-variables-confirm uses just
>>
>>    (pop-to-buffer "*Local Variables*" '(display-buffer--maybe-at-bottom))
>
> IIUC it neither runs the hooks for temporary buffers nor does it obey
> 'temp-buffer-resize-mode'.

I don't know why it should run hooks and 'temp-buffer-resize-mode'.
Why other clients of display-buffer don't need these hooks?

>> and nothing more, without hassles of with-displayed-buffer-window.
>> And still it fits the window nicely into the buffer height.
>
> How comes?

Maybe this means it's possible to avoid using with-displayed-buffer-window?

>> Why the above 3 commands couldn't do the same by abandoning
>> with-displayed-buffer-window?
>
> It depends on how much of the stuff in 'temp-buffer-window-setup' and
> 'temp-buffer-window-show' they really need.

Maybe they need none of these?

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[martin rudalics]

 >> IIUC it neither runs the hooks for temporary buffers nor does it obey
 >> 'temp-buffer-resize-mode'.
 >
 > I don't know why it should run hooks and 'temp-buffer-resize-mode'.
 > Why other clients of display-buffer don't need these hooks?
 >
 >>> and nothing more, without hassles of with-displayed-buffer-window.
 >>> And still it fits the window nicely into the buffer height.
 >>
 >> How comes?
 >
 > Maybe this means it's possible to avoid using with-displayed-buffer-window?
 >
 >>> Why the above 3 commands couldn't do the same by abandoning
 >>> with-displayed-buffer-window?
 >>
 >> It depends on how much of the stuff in 'temp-buffer-window-setup' and
 >> 'temp-buffer-window-show' they really need.
 >
 > Maybe they need none of these?

'display-buffer--maybe-at-bottom' handles this by using

   (let ((alist (append alist `(,(if temp-buffer-resize-mode
		                    '(window-height . resize-temp-buffer-window)
	                          '(window-height . fit-window-to-buffer))
	                       ,(when temp-buffer-resize-mode
	                          '(preserve-size . (nil . t)))))))

The question is now whether we want to do that in one place (that is, in
the temporary buffer setup and show functions) or in many places (like
in the buffer display action functions).

martin

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[Juri Linkov]

> 'display-buffer--maybe-at-bottom' handles this by using
>
>   (let ((alist (append alist `(,(if temp-buffer-resize-mode
> 		                    '(window-height . resize-temp-buffer-window)
> 	                          '(window-height . fit-window-to-buffer))
> 	                       ,(when temp-buffer-resize-mode
> 	                          '(preserve-size . (nil . t)))))))
>
> The question is now whether we want to do that in one place (that is, in
> the temporary buffer setup and show functions) or in many places (like
> in the buffer display action functions).

It seems better to handle the temp-buffer setup in the buffer display action
functions (currently there are two such functions that handle temp-buffer:
'display-buffer--maybe-at-bottom' and 'display-buffer-in-direction'),
if this will help to get rid of monstrous macro 'with-displayed-buffer-window'.

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[martin rudalics]

 > It seems better to handle the temp-buffer setup in the buffer display action
 > functions (currently there are two such functions that handle temp-buffer:
 > 'display-buffer--maybe-at-bottom' and 'display-buffer-in-direction'),
 > if this will help to get rid of monstrous macro 'with-displayed-buffer-window'.

Then go for it.

martin

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[Juri Linkov]

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

>> It seems better to handle the temp-buffer setup in the buffer display action
>> functions (currently there are two such functions that handle temp-buffer:
>> 'display-buffer--maybe-at-bottom' and 'display-buffer-in-direction'),
>> if this will help to get rid of monstrous macro 'with-displayed-buffer-window'.
>
> Then go for it.

Here is the first step, this patch seems to keep the original behavior,
but I need your help to finish it.  Could you confirm that calls of
window-preserve-size at the end of with-displayed-buffer-window are
not needed anymore after this patch is applied, because there are the
same calls of window-preserve-size at the end of window--display-buffer
that are called later after buffer contents is filled
by after-display-function in the middle of window--display-buffer:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: after-display-function.patch --]
[-- Type: text/x-diff, Size: 2704 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index fc1e7d4a76..5be9d0ee83 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -7052,6 +7052,11 @@ window--display-buffer
         (set-window-dedicated-p window display-buffer-mark-dedicated))))
     (when (memq type '(window frame tab))
       (set-window-prev-buffers window nil))
+
+    ;; Is this the right place to call former body of with-displayed-buffer-window?
+    (when (functionp (cdr (assq 'after-display-function alist)))
+      (funcall (cdr (assq 'after-display-function alist))))
+
     (let ((quit-restore (window-parameter window 'quit-restore))
 	  (height (cdr (assq 'window-height alist)))
 	  (width (cdr (assq 'window-width alist)))
diff --git a/lisp/dired.el b/lisp/dired.el
index a4de51f609..a76d223bed 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -3465,23 +3465,28 @@ dired-mark-pop-up
       (with-displayed-buffer-window
        buffer
        (cons 'display-buffer-below-selected
-	     '((window-height . fit-window-to-buffer)
-	       (preserve-size . (nil . t))))
+             `((window-height . fit-window-to-buffer)
+               (preserve-size . (nil . t))
+               (after-display-function
+                .
+                ;; Handle (t FILE) just like (FILE), here.  That value is
+                ;; used (only in some cases), to mean just one file that was
+                ;; marked, rather than the current line file.
+                ,#'(lambda ()
+                    (with-current-buffer buffer
+                      (let ((inhibit-read-only t)
+                            (inhibit-modification-hooks t))
+                        (dired-format-columns-of-files
+                         (if (eq (car files) t) (cdr files) files))
+                        (remove-text-properties (point-min) (point-max)
+                                                '(mouse-face nil help-echo nil))
+                        (setq tab-line-exclude nil)))))))
        #'(lambda (window _value)
 	   (with-selected-window window
 	     (unwind-protect
 		 (apply function args)
 	       (when (window-live-p window)
-		 (quit-restore-window window 'kill)))))
-       ;; Handle (t FILE) just like (FILE), here.  That value is
-       ;; used (only in some cases), to mean just one file that was
-       ;; marked, rather than the current line file.
-       (with-current-buffer buffer
-	 (dired-format-columns-of-files
-	  (if (eq (car files) t) (cdr files) files))
-	 (remove-text-properties (point-min) (point-max)
-				 '(mouse-face nil help-echo nil))
-	 (setq tab-line-exclude nil))))))
+		 (quit-restore-window window 'kill)))))))))
 
 (defun dired-format-columns-of-files (files)
   (let ((beg (point)))

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[martin rudalics]

 > Here is the first step, this patch seems to keep the original behavior,
 > but I need your help to finish it.  Could you confirm that calls of
 > window-preserve-size at the end of with-displayed-buffer-window are
 > not needed anymore after this patch is applied, because there are the
 > same calls of window-preserve-size at the end of window--display-buffer
 > that are called later after buffer contents is filled
 > by after-display-function in the middle of window--display-buffer:

Didn't we agree that 'vaction' is harmful anyway so these "same calls"
should never have been applied in the first place?  I wouldn't bother
about them at the moment, when something fails we find out soon enough.

But what if a function like 'dired-format-columns-of-files' wanted to

(1) know the width of the window used for displaying the buffer,

(2) according to that (presumably fixed) width adjust columns, establish
a maximum width of buffer lines or do something else width related,

(3) leave it to 'window--display-buffer' to adjust the window height
afterwards?

And be able to do (1)-(3) in the orthogonal direction, that is, base (2)
on a presumably fixed window height?

I conjecture that in such case, the function (functions?) specified by
'after-display-function' should be supplied the window to display the
buffer as first argument (just in case there's another window showing
the same buffer).  WDYT?

martin

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[Juri Linkov]

>> Here is the first step, this patch seems to keep the original behavior,
>> but I need your help to finish it.  Could you confirm that calls of
>> window-preserve-size at the end of with-displayed-buffer-window are
>> not needed anymore after this patch is applied, because there are the
>> same calls of window-preserve-size at the end of window--display-buffer
>> that are called later after buffer contents is filled
>> by after-display-function in the middle of window--display-buffer:
>
> Didn't we agree that 'vaction' is harmful anyway so these "same calls"
> should never have been applied in the first place?  I wouldn't bother
> about them at the moment, when something fails we find out soon enough.

These calls should remain in 'window--display-buffer' to adjust the
window height afterwards.

> But what if a function like 'dired-format-columns-of-files' wanted to
>
> (1) know the width of the window used for displaying the buffer,

'dired-format-columns-of-files' is called when the window
is already displayed, so it can find the width of the window,
and it uses 'completion--insert-strings' for that, see below.

> (2) according to that (presumably fixed) width adjust columns, establish
> a maximum width of buffer lines or do something else width related,

Same as above.

> (3) leave it to 'window--display-buffer' to adjust the window height
> afterwards?

It already allows 'window--display-buffer' to adjust the window height
afterwards in the previous patch.

> And be able to do (1)-(3) in the orthogonal direction, that is, base (2)
> on a presumably fixed window height?

You mean to not allow adjusting the window height afterwards?
Then the call of 'display-buffer' should be without alist entries
'window-height' and 'preserve-size'.

> I conjecture that in such case, the function (functions?) specified by
> 'after-display-function' should be supplied the window to display the
> buffer as first argument (just in case there's another window showing
> the same buffer).  WDYT?

The current implementation doesn't supply the window, and
'dired-format-columns-of-files' and 'minibuffer-completion-help'
use 'completion--insert-strings':

	   (window (get-buffer-window (current-buffer) 0))
	   (wwidth (if window (1- (window-width window)) 79))

Do you propose to rewrite 'completion--insert-strings' for accept a new
argument 'window'?  Or to rely on the fact that the displayed window should
be already selected by its caller?  Will this break backward-compatibility
for packages that use it?

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[martin rudalics]

 > These calls should remain in 'window--display-buffer' to adjust the
 > window height afterwards.

Yes.

 >> But what if a function like 'dired-format-columns-of-files' wanted to
 >>
 >> (1) know the width of the window used for displaying the buffer,
 >
 > 'dired-format-columns-of-files' is called when the window
 > is already displayed, so it can find the width of the window,
 > and it uses 'completion--insert-strings' for that, see below.

Does it know the window for sure?  Always think of the case where the
buffer is already displayed in some other window.

 >> (2) according to that (presumably fixed) width adjust columns, establish
 >> a maximum width of buffer lines or do something else width related,
 >
 > Same as above.
 >
 >> (3) leave it to 'window--display-buffer' to adjust the window height
 >> afterwards?
 >
 > It already allows 'window--display-buffer' to adjust the window height
 > afterwards in the previous patch.
 >
 >> And be able to do (1)-(3) in the orthogonal direction, that is, base (2)
 >> on a presumably fixed window height?
 >
 > You mean to not allow adjusting the window height afterwards?
 > Then the call of 'display-buffer' should be without alist entries
 > 'window-height' and 'preserve-size'.

We have to be always prepared for the case that a split happens in the
orthogonal direction.

 > The current implementation doesn't supply the window, and
 > 'dired-format-columns-of-files' and 'minibuffer-completion-help'
 > use 'completion--insert-strings':
 >
 > 	   (window (get-buffer-window (current-buffer) 0))

That's precisely what I'm afraid of (and it happens in a dozen other
cases in our code base as well, IIRC).  Better make sure now that we
always act on the window returned by 'display-buffer'.

 > 	   (wwidth (if window (1- (window-width window)) 79))
 >
 > Do you propose to rewrite 'completion--insert-strings' for accept a new
 > argument 'window'?  Or to rely on the fact that the displayed window should
 > be already selected by its caller?  Will this break backward-compatibility
 > for packages that use it?

Maybe it's overkill but I'd give 'completion--insert-strings' an
additional window argument.

martin

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[Juri Linkov]

>> Do you propose to rewrite 'completion--insert-strings' for accept a new
>> argument 'window'?  Or to rely on the fact that the displayed window should
>> be already selected by its caller?  Will this break backward-compatibility
>> for packages that use it?
>
> Maybe it's overkill but I'd give 'completion--insert-strings' an
> additional window argument.

Yes, it's overkill.  I can't imagine a situation when the
*Completions* buffer needs to be displayed in two windows.

I see that you proposed this as a use case that justifies
adding a WINDOW arg to 'after-display-function'.
But I already agreed about adding a new arg to
'after-display-function', and will add it anyway.

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[martin rudalics]

 > Yes, it's overkill.  I can't imagine a situation when the
 > *Completions* buffer needs to be displayed in two windows.

Separate completions in two different frames?

 > I see that you proposed this as a use case that justifies
 > adding a WINDOW arg to 'after-display-function'.
 > But I already agreed about adding a new arg to
 > 'after-display-function', and will add it anyway.

OK.  To avoid confusions 'after-display-function' should be renamed to
'after-display-buffer-function' at least.

martin

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[Juri Linkov]

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

>> But I already agreed about adding a new arg to
>> 'after-display-function', and will add it anyway.
>
> OK.  To avoid confusions 'after-display-function' should be renamed to
> 'after-display-buffer-function' at least.

Are you sure about such long name?  This is not a hook, it's just
an alist entry along with 'window-height' and 'preserve-size'.

A good short name would be 'body-function' where 'body' has
two-fold meaning:

1. it hints to body of the former macro that it replaces;

2. body could also mean window body that this function fills.

Here is a completely tested patch that works in all cases:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: body-function.patch --]
[-- Type: text/x-diff, Size: 11693 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index b54f1633f5..00e793db95 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -7070,6 +7070,12 @@ window--display-buffer
         (set-window-dedicated-p window display-buffer-mark-dedicated))))
     (when (memq type '(window frame tab))
       (set-window-prev-buffers window nil))
+
+    (when (functionp (cdr (assq 'body-function alist)))
+      (let ((inhibit-read-only t)
+            (inhibit-modification-hooks t))
+        (funcall (cdr (assq 'body-function alist)) window)))
+
     (let ((quit-restore (window-parameter window 'quit-restore))
 	  (height (cdr (assq 'window-height alist)))
 	  (width (cdr (assq 'window-width alist)))
diff --git a/lisp/dired.el b/lisp/dired.el
index 41bbf9f56a..51ec9a798e 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -3520,26 +3521,27 @@ dired-mark-pop-up
 	  ;; Mark *Marked Files* window as softly-dedicated, to prevent
 	  ;; other buffers e.g. *Completions* from reusing it (bug#17554).
 	  (display-buffer-mark-dedicated 'soft))
-      (with-displayed-buffer-window
+      (with-current-buffer-window
        buffer
-       (cons 'display-buffer-below-selected
-	     '((window-height . fit-window-to-buffer)
-	       (preserve-size . (nil . t))))
+       `(display-buffer-below-selected
+         (window-height . fit-window-to-buffer)
+         (preserve-size . (nil . t))
+         (body-function
+          . ,#'(lambda (_window)
+                 ;; Handle (t FILE) just like (FILE), here.  That value is
+                 ;; used (only in some cases), to mean just one file that was
+                 ;; marked, rather than the current line file.
+                 (dired-format-columns-of-files
+                  (if (eq (car files) t) (cdr files) files))
+                 (remove-text-properties (point-min) (point-max)
+                                         '(mouse-face nil help-echo nil))
+                 (setq tab-line-exclude nil))))
        #'(lambda (window _value)
 	   (with-selected-window window
 	     (unwind-protect
 		 (apply function args)
 	       (when (window-live-p window)
-		 (quit-restore-window window 'kill)))))
-       ;; Handle (t FILE) just like (FILE), here.  That value is
-       ;; used (only in some cases), to mean just one file that was
-       ;; marked, rather than the current line file.
-       (with-current-buffer buffer
-	 (dired-format-columns-of-files
-	  (if (eq (car files) t) (cdr files) files))
-	 (remove-text-properties (point-min) (point-max)
-				 '(mouse-face nil help-echo nil))
-	 (setq tab-line-exclude nil))))))
+		 (quit-restore-window window 'kill)))))))))
 
 (defun dired-format-columns-of-files (files)
   (let ((beg (point)))
diff --git a/lisp/files.el b/lisp/files.el
index 8ce0187f5b..4b5c7d1e55 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -7253,10 +7253,15 @@ save-buffers-kill-emacs
                   (setq active t))
              (setq processes (cdr processes)))
            (or (not active)
-               (with-displayed-buffer-window
+               (with-current-buffer-window
                 (get-buffer-create "*Process List*")
-                '(display-buffer--maybe-at-bottom
-                  (dedicated . t))
+                `(display-buffer--maybe-at-bottom
+                  (dedicated . t)
+                  (window-height . fit-window-to-buffer)
+                  (preserve-size . (nil . t))
+                  (body-function
+                   . ,#'(lambda (_window)
+                          (list-processes t))))
                 #'(lambda (window _value)
                     (with-selected-window window
                       (unwind-protect
@@ -7264,8 +7269,7 @@ save-buffers-kill-emacs
                             (setq confirm nil)
                             (yes-or-no-p "Active processes exist; kill them and exit anyway? "))
                         (when (window-live-p window)
-                          (quit-restore-window window 'kill)))))
-                (list-processes t)))))
+                          (quit-restore-window window 'kill)))))))))
      ;; Query the user for other things, perhaps.
      (run-hook-with-args-until-failure 'kill-emacs-query-functions)
      (or (null confirm)
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 7f5b597542..d94582a908 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1973,7 +1973,7 @@ minibuffer-completion-help
              ;; minibuffer-hide-completions will know whether to
              ;; delete the window or not.
              (display-buffer-mark-dedicated 'soft))
-        (with-displayed-buffer-window
+        (with-current-buffer-window
           "*Completions*"
           ;; This is a copy of `display-buffer-fallback-action'
           ;; where `display-buffer-use-some-window' is replaced
@@ -1991,62 +1991,64 @@ minibuffer-completion-help
 		 '(window-height . resize-temp-buffer-window)
 	       '(window-height . fit-window-to-buffer))
 	    ,(when temp-buffer-resize-mode
-	       '(preserve-size . (nil . t))))
-          nil
-          ;; Remove the base-size tail because `sort' requires a properly
-          ;; nil-terminated list.
-          (when last (setcdr last nil))
-          (setq completions
-                ;; FIXME: This function is for the output of all-completions,
-                ;; not completion-all-completions.  Often it's the same, but
-                ;; not always.
-                (let ((sort-fun (completion-metadata-get
-                                 all-md 'display-sort-function)))
-                  (if sort-fun
-                      (funcall sort-fun completions)
-                    (sort completions 'string-lessp))))
-          (when afun
-            (setq completions
-                  (mapcar (lambda (s)
-                            (let ((ann (funcall afun s)))
-                              (if ann (list s ann) s)))
-                          completions)))
+	       '(preserve-size . (nil . t)))
+            (body-function
+             . ,#'(lambda (_window)
+                    ;; Remove the base-size tail because `sort' requires a properly
+                    ;; nil-terminated list.
+                    (when last (setcdr last nil))
+                    (setq completions
+                          ;; FIXME: This function is for the output of all-completions,
+                          ;; not completion-all-completions.  Often it's the same, but
+                          ;; not always.
+                          (let ((sort-fun (completion-metadata-get
+                                           all-md 'display-sort-function)))
+                            (if sort-fun
+                                (funcall sort-fun completions)
+                              (sort completions 'string-lessp))))
+                    (when afun
+                      (setq completions
+                            (mapcar (lambda (s)
+                                      (let ((ann (funcall afun s)))
+                                        (if ann (list s ann) s)))
+                                    completions)))
 
-          (with-current-buffer standard-output
-            (set (make-local-variable 'completion-base-position)
-                 (list (+ start base-size)
-                       ;; FIXME: We should pay attention to completion
-                       ;; boundaries here, but currently
-                       ;; completion-all-completions does not give us the
-                       ;; necessary information.
-                       end))
-            (set (make-local-variable 'completion-list-insert-choice-function)
-                 (let ((ctable minibuffer-completion-table)
-                       (cpred minibuffer-completion-predicate)
-                       (cprops completion-extra-properties))
-                   (lambda (start end choice)
-                     (unless (or (zerop (length prefix))
-                                 (equal prefix
-                                        (buffer-substring-no-properties
-                                         (max (point-min)
-                                              (- start (length prefix)))
-                                         start)))
-                       (message "*Completions* out of date"))
-                     ;; FIXME: Use `md' to do quoting&terminator here.
-                     (completion--replace start end choice)
-                     (let* ((minibuffer-completion-table ctable)
-                            (minibuffer-completion-predicate cpred)
-                            (completion-extra-properties cprops)
-                            (result (concat prefix choice))
-                            (bounds (completion-boundaries
-                                     result ctable cpred "")))
-                       ;; If the completion introduces a new field, then
-                       ;; completion is not finished.
-                       (completion--done result
-                                         (if (eq (car bounds) (length result))
-                                             'exact 'finished)))))))
+                    (with-current-buffer standard-output
+                      (set (make-local-variable 'completion-base-position)
+                           (list (+ start base-size)
+                                 ;; FIXME: We should pay attention to completion
+                                 ;; boundaries here, but currently
+                                 ;; completion-all-completions does not give us the
+                                 ;; necessary information.
+                                 end))
+                      (set (make-local-variable 'completion-list-insert-choice-function)
+                           (let ((ctable minibuffer-completion-table)
+                                 (cpred minibuffer-completion-predicate)
+                                 (cprops completion-extra-properties))
+                             (lambda (start end choice)
+                               (unless (or (zerop (length prefix))
+                                           (equal prefix
+                                                  (buffer-substring-no-properties
+                                                   (max (point-min)
+                                                        (- start (length prefix)))
+                                                   start)))
+                                 (message "*Completions* out of date"))
+                               ;; FIXME: Use `md' to do quoting&terminator here.
+                               (completion--replace start end choice)
+                               (let* ((minibuffer-completion-table ctable)
+                                      (minibuffer-completion-predicate cpred)
+                                      (completion-extra-properties cprops)
+                                      (result (concat prefix choice))
+                                      (bounds (completion-boundaries
+                                               result ctable cpred "")))
+                                 ;; If the completion introduces a new field, then
+                                 ;; completion is not finished.
+                                 (completion--done result
+                                                   (if (eq (car bounds) (length result))
+                                                       'exact 'finished)))))))
 
-          (display-completion-list completions))))
+                    (display-completion-list completions))))
+          nil)))
     nil))
 
 (defun minibuffer-hide-completions ()

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[martin rudalics]

 > Are you sure about such long name?  This is not a hook, it's just
 > an alist entry along with 'window-height' and 'preserve-size'.

You're right, I was confused.

 > A good short name would be 'body-function' where 'body' has
 > two-fold meaning:
 >
 > 1. it hints to body of the former macro that it replaces;
 >
 > 2. body could also mean window body that this function fills.
 >
 > Here is a completely tested patch that works in all cases:

LGTM.  'body-function' needs an alist entry in the manual then.

Thanks, martin

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[Juri Linkov]

> LGTM.  'body-function' needs an alist entry in the manual then.

Pushed to master with updating the manual.

Please check if this report could be closed now.

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[Juri Linkov]

>> LGTM.  'body-function' needs an alist entry in the manual then.
>
> Pushed to master with updating the manual.
>
> Please check if this report could be closed now.

Actually it broke choose-completion invoked in *Completions* displayed
from the minibuffer, because choose-completion expects completion-reference-buffer
to be the original buffer (should be the minibuffer in this case),
but it was *Completions*.  This means that display-completion-list
should be run in the original buffer.

Now fixed in master.

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[martin rudalics]

 > Pushed to master with updating the manual.

Thanks.  Strictly spoken, we should mention 'body-function' in the
doc-string of 'display-buffer' but I'd rather stop describing the alist
entries there.  Maybe we should just mention them and refer the user to
the manual.

 > Please check if this report could be closed now.

I think it can be closed now.

Many thanks for fixing this, martin

bug#39822: 27.0.90; Cannot set *Completions* buffer height using display-buffer-alist

[Juri Linkov]

tags 39822 fixed
close 39822 28.0.50
quit

>> Pushed to master with updating the manual.
>
> Thanks.  Strictly spoken, we should mention 'body-function' in the
> doc-string of 'display-buffer' but I'd rather stop describing the alist
> entries there.  Maybe we should just mention them and refer the user to
> the manual.

Thanks for pointing to the doc-string of 'display-buffer', now added.
Feel free to remove descriptions from the docstring to avoid duplication.

>> Please check if this report could be closed now.
>
> I think it can be closed now.

So closing now.