[PATCH] completions-max-height

[PATCH] completions-max-height

[Ergus]

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

Hi:

Do you think that this may be added to vanilla?

Best,
Ergus

[-- Attachment #2: completions-max-height.patch --]
[-- Type: text/plain, Size: 1920 bytes --]

diff --git a/etc/NEWS b/etc/NEWS
index 587e7f6ade..78f41de2ea 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -554,6 +554,10 @@ This option controls the sorting of the completion candidates in
 the "*Completions*" buffer.  Available styles are no sorting,
 alphabetical (the default), or a custom sort function.
 
+*** New user option 'completions-max-height'.
+This option limits the height of the "*Completions*" buffer.
+
+
 ** Isearch and Replace
 
 +++
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 798cba0ac2..a06986fdb7 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -2212,6 +2212,19 @@ completion--done
                (equal pre-msg (and exit-fun (current-message))))
       (completion--message message))))
 
+(defcustom completions-max-height nil
+  "Maximum height for *Completions* buffer."
+  :type 'natnum
+  :version "29.1")
+
+(defun completions--fit-window-to-buffer (&optional win &rest _)
+  "Resize completions."
+  (if temp-buffer-resize-mode
+      (let ((temp-buffer-max-height (or completions-max-height
+                                        temp-buffer-max-height)))
+        (resize-temp-buffer-window win))
+    (fit-window-to-buffer win completions-max-height)))
+
 (defun minibuffer-completion-help (&optional start end)
   "Display a list of possible completions of the current minibuffer contents."
   (interactive)
@@ -2275,9 +2288,7 @@ minibuffer-completion-help
              ,(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))
+            (window-height . completions--fit-window-to-buffer)
             ,(when temp-buffer-resize-mode
                '(preserve-size . (nil . t)))
             (body-function

Re: [PATCH] completions-max-height

[Philip Kaludercic]

Ergus <spacibba@aol.com> writes:

> Hi:
>
> Do you think that this may be added to vanilla?

IMO it would be better if this could be generalised to something like
completion-display-buffer-action, so that you can decide where and how
the completion buffer is displayed.  Though I am uncertain if this might
have unintended side effects, as the behaviour seems to have been
hard-coded for a while.

> Best,
> Ergus
>
> diff --git a/etc/NEWS b/etc/NEWS
> index 587e7f6ade..78f41de2ea 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -554,6 +554,10 @@ This option controls the sorting of the completion candidates in
>  the "*Completions*" buffer.  Available styles are no sorting,
>  alphabetical (the default), or a custom sort function.
>  
> +*** New user option 'completions-max-height'.
> +This option limits the height of the "*Completions*" buffer.
> +
> +
>  ** Isearch and Replace
>  
>  +++
> diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
> index 798cba0ac2..a06986fdb7 100644
> --- a/lisp/minibuffer.el
> +++ b/lisp/minibuffer.el
> @@ -2212,6 +2212,19 @@ completion--done
>                 (equal pre-msg (and exit-fun (current-message))))
>        (completion--message message))))
>  
> +(defcustom completions-max-height nil
> +  "Maximum height for *Completions* buffer."
> +  :type 'natnum
> +  :version "29.1")
> +
> +(defun completions--fit-window-to-buffer (&optional win &rest _)
> +  "Resize completions."
> +  (if temp-buffer-resize-mode
> +      (let ((temp-buffer-max-height (or completions-max-height
> +                                        temp-buffer-max-height)))
> +        (resize-temp-buffer-window win))
> +    (fit-window-to-buffer win completions-max-height)))
> +
>  (defun minibuffer-completion-help (&optional start end)
>    "Display a list of possible completions of the current minibuffer contents."
>    (interactive)
> @@ -2275,9 +2288,7 @@ minibuffer-completion-help
>               ,(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))
> +            (window-height . completions--fit-window-to-buffer)
>              ,(when temp-buffer-resize-mode
>                 '(preserve-size . (nil . t)))
>              (body-function
>

-- 
	Philip Kaludercic

Re: [PATCH] completions-max-height

[Protesilaos Stavrou]

On 2022-03-07, 22:10 +0000, Philip Kaludercic <philipk@posteo.net> wrote:

> Ergus <spacibba@aol.com> writes:
>
>> Hi:
>>
>> Do you think that this may be added to vanilla?
>
> IMO it would be better if this could be generalised to something like
> completion-display-buffer-action, so that you can decide where and how
> the completion buffer is displayed.  Though I am uncertain if this might
> have unintended side effects, as the behaviour seems to have been
> hard-coded for a while.

I agree with Philip.  Or go a step further and just let the Completions
behave like other buffers so the user can contol its placement and
parameters via the display-buffer-alist.

-- 
Protesilaos Stavrou
https://protesilaos.com

Re: [PATCH] completions-max-height

[Ergus]

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

I understand your intention, but in practice making this more complex is useless. The mini buffer is always down and making the completions to move somewhere else is uncomfortable and may require more lisp/emacs knowledge to change the default behavior from the user. Which is completely the opposite to my intention. Actually this same result may be reached with an advise as I discussed on yesterday on emacs help. But a simple custom is better.

I am totally fine if you propose something else more general if that don't forces the user to write a function to change a simple height. 


On March 8, 2022 6:13:13 AM GMT+01:00, Protesilaos Stavrou <info@protesilaos.com> wrote:
>On 2022-03-07, 22:10 +0000, Philip Kaludercic <philipk@posteo.net> wrote:
>
>> Ergus <spacibba@aol.com> writes:
>>
>>> Hi:
>>>
>>> Do you think that this may be added to vanilla?
>>
>> IMO it would be better if this could be generalised to something like
>> completion-display-buffer-action, so that you can decide where and how
>> the completion buffer is displayed.  Though I am uncertain if this might
>> have unintended side effects, as the behaviour seems to have been
>> hard-coded for a while.
>
>I agree with Philip.  Or go a step further and just let the Completions
>behave like other buffers so the user can contol its placement and
>parameters via the display-buffer-alist.
>
>-- 
>Protesilaos Stavrou
>https://protesilaos.com
>

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[-- Attachment #2: Type: text/html, Size: 2222 bytes --]

Re: [PATCH] completions-max-height

[Philip Kaludercic]

Ergus <spacibba@aol.com> writes:

> I understand your intention, but in practice making this more complex
> is useless. The mini buffer is always down and making the completions
> to move somewhere else is uncomfortable and may require more
> lisp/emacs knowledge to change the default behavior from the
> user. Which is completely the opposite to my intention. Actually this
> same result may be reached with an advise as I discussed on yesterday
> on emacs help. But a simple custom is better.
>
> I am totally fine if you propose something else more general if that don't forces the user to write a function to change a simple height. 

What I proposed would just require something like

    (setq completion-display-buffer-option '(display-buffer-at-bottom (window-height . 10)))

> On March 8, 2022 6:13:13 AM GMT+01:00, Protesilaos Stavrou <info@protesilaos.com> wrote:
>>On 2022-03-07, 22:10 +0000, Philip Kaludercic <philipk@posteo.net> wrote:
>>
>>> Ergus <spacibba@aol.com> writes:
>>>
>>>> Hi:
>>>>
>>>> Do you think that this may be added to vanilla?
>>>
>>> IMO it would be better if this could be generalised to something like
>>> completion-display-buffer-action, so that you can decide where and how
>>> the completion buffer is displayed.  Though I am uncertain if this might
>>> have unintended side effects, as the behaviour seems to have been
>>> hard-coded for a while.
>>
>>I agree with Philip.  Or go a step further and just let the Completions
>>behave like other buffers so the user can contol its placement and
>>parameters via the display-buffer-alist.

You can do so now too, but I don't think that that would be an argument
to just have the completion buffer behave like other buffers,
considering that there are certainly many who are used to its current
behaviour.

>>-- 
>>Protesilaos Stavrou
>>https://protesilaos.com
>>

-- 
	Philip Kaludercic

Re: [PATCH] completions-max-height

[Ergus]

On Tue, Mar 08, 2022 at 08:47:13AM +0000, Philip Kaludercic wrote:
>Ergus <spacibba@aol.com> writes:
>
>> I understand your intention, but in practice making this more complex
>> is useless. The mini buffer is always down and making the completions
>> to move somewhere else is uncomfortable and may require more
>> lisp/emacs knowledge to change the default behavior from the
>> user. Which is completely the opposite to my intention. Actually this
>> same result may be reached with an advise as I discussed on yesterday
>> on emacs help. But a simple custom is better.
>>
>> I am totally fine if you propose something else more general if that don't forces the user to write a function to change a simple height.
>
>What I proposed would just require something like
>
>    (setq completion-display-buffer-option '(display-buffer-at-bottom (window-height . 10)))
>
I got the point but if you look at the original code there are
conditions that will need to be set to not change the current default
behavior (because that always ends in arguments from long time users
that want some specific detail of the previous behavior), for example:
temp-buffer-resize-mode or (eq (selected-window) (minibuffer-window)).

We can go around them, but it may be a more complex patch I am not
willing to do because I don't see the utility of a completions window in
any other place than where it goes now (next to the minibuffer).

In a best case the new variable will need to be something like:
`completion-display-from-minibuffer-options` and parse them with some
if-else here and there to apply the position only when (eq
(selected-window) (minibuffer-window)) but provide a default value in
case the user didn't specify it and the height only when not
temp-buffer-resize-mode (or condition it as the new function does).



>> On March 8, 2022 6:13:13 AM GMT+01:00, Protesilaos Stavrou <info@protesilaos.com> wrote:
>>>On 2022-03-07, 22:10 +0000, Philip Kaludercic <philipk@posteo.net> wrote:
>>>
>>>> Ergus <spacibba@aol.com> writes:
>>>>
>>>>> Hi:
>>>>>
>>>>> Do you think that this may be added to vanilla?
>>>>
>>>> IMO it would be better if this could be generalised to something like
>>>> completion-display-buffer-action, so that you can decide where and how
>>>> the completion buffer is displayed.  Though I am uncertain if this might
>>>> have unintended side effects, as the behaviour seems to have been
>>>> hard-coded for a while.
>>>
>>>I agree with Philip.  Or go a step further and just let the Completions
>>>behave like other buffers so the user can contol its placement and
>>>parameters via the display-buffer-alist.
>
>You can do so now too, but I don't think that that would be an argument
>to just have the completion buffer behave like other buffers,
>considering that there are certainly many who are used to its current
>behaviour.
>
>>>--
>>>Protesilaos Stavrou
>>>https://protesilaos.com
>>>
>
>-- 
>	Philip Kaludercic
>

Re: [PATCH] completions-max-height

[Ergus]

On Tue, Mar 08, 2022 at 08:47:13AM +0000, Philip Kaludercic wrote:
>Ergus <spacibba@aol.com> writes:
>
>> I understand your intention, but in practice making this more complex
>> is useless. The mini buffer is always down and making the completions
>> to move somewhere else is uncomfortable and may require more
>> lisp/emacs knowledge to change the default behavior from the
>> user. Which is completely the opposite to my intention. Actually this
>> same result may be reached with an advise as I discussed on yesterday
>> on emacs help. But a simple custom is better.
>>
>> I am totally fine if you propose something else more general if that don't forces the user to write a function to change a simple height.
>
>What I proposed would just require something like
>
>    (setq completion-display-buffer-option '(display-buffer-at-bottom (window-height . 10)))
>

This just works:

(setq display-buffer-alist '(("^\\*Completions\\*$" .
			      (display-buffer-at-bottom
			       (window-height . 10)))))

For the simplest case this works... But there is not a way to handle the
conditionals without affecting some users. For example when using inline
completion or the ones using resize-temp-buffer-window

We can use a function instead, but in that case the user customization
may need a more complex approach instead of just a setq.

Maybe a better lisper could propose something better that allows to set
this without a defun, an advise or defining a tricky alist that may
conflict with something else?

Re: [PATCH] completions-max-height

[Ergus]

On Tue, Mar 08, 2022 at 02:29:10PM +0100, Ergus wrote:
>On Tue, Mar 08, 2022 at 08:47:13AM +0000, Philip Kaludercic wrote:
>>Ergus <spacibba@aol.com> writes:
>>
>>>I understand your intention, but in practice making this more complex
>>>is useless. The mini buffer is always down and making the completions
>>>to move somewhere else is uncomfortable and may require more
>>>lisp/emacs knowledge to change the default behavior from the
>>>user. Which is completely the opposite to my intention. Actually this
>>>same result may be reached with an advise as I discussed on yesterday
>>>on emacs help. But a simple custom is better.
>>>
>>>I am totally fine if you propose something else more general if that don't forces the user to write a function to change a simple height.
>>
>>What I proposed would just require something like
>>
>>   (setq completion-display-buffer-option '(display-buffer-at-bottom (window-height . 10)))
>>
>
>This just works:
>
>(setq display-buffer-alist '(("^\\*Completions\\*$" .
>			      (display-buffer-at-bottom
>			       (window-height . 10)))))
>
>For the simplest case this works... But there is not a way to handle the
>conditionals without affecting some users. For example when using inline
>completion or the ones using resize-temp-buffer-window
>
>We can use a function instead, but in that case the user customization
>may need a more complex approach instead of just a setq.
>
>Maybe a better lisper could propose something better that allows to set
>this without a defun, an advise or defining a tricky alist that may
>conflict with something else?
>
>
After a day with this config I found several issues.

At the end it required to do:

(add-to-list 'display-buffer-alist
	     '("^\\*Completions\\*$" .
	       ((display-buffer--maybe-same-window
		display-buffer-reuse-window
		display-buffer--maybe-pop-up-frame)
		(window-height . 10))))

Otherwise many things happen (completions buffer replication for not
reusing window, not popup frames and so on.)

So at the end I think that the initial patch is the best simpler
approach (the code could be simplified even more by a better lisper
probably) because otherwise the user may need do a complex config for a
most simple need, creating conflicts and changing some of the default
behaviors...

Simple is better than complex.
Complex is better than complicated.

Best,
Ergus