[PATCH] icomplete-vertical

[PATCH] icomplete-vertical

[Gregory Heytings]

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


I attach a patch which implements vertical display of completion 
candidates for icomplete.  I've been using it for a few months, and did 
find any bugs.

It is minimal (less than ten lines of "real" code), and unlike other 
"icomplete vertical" implementations, the number of displayed completion 
candidates depends on the "max-mini-window-height" variable, which means 
that it is the Emacs display engine which decides the number of completion 
candidates that is displayed, depending on the available space in the 
frame at the moment the minibuffer is entered.

[-- Attachment #2: Type: text/x-diff, Size: 4122 bytes --]

From fb9a2b45b97712dba1f622f6dff7a84696b0f71d Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Mon, 5 Apr 2021 21:30:26 +0000
Subject: [PATCH] Add an icomplete-vertical minor mode

* lisp/icomplete.el (icomplete-vertical-mode): New minor mode.
(icomplete-vertical-reformat-completions,
icomplete-vertical-minibuffer-setup): Auxiliary functions for the
new minor mode.

* etc/NEWS: Mention the new minor mode.

* doc/emacs/buffers.texi: Document the new minor mode.
---
 doc/emacs/buffers.texi |  5 ++++-
 etc/NEWS               |  7 +++++++
 lisp/icomplete.el      | 40 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/doc/emacs/buffers.texi b/doc/emacs/buffers.texi
index 3a166e404a..5561a704ca 100644
--- a/doc/emacs/buffers.texi
+++ b/doc/emacs/buffers.texi
@@ -740,7 +740,10 @@ Icomplete
 
   To enable Icomplete mode, type @kbd{M-x icomplete-mode}, or customize
 the variable @code{icomplete-mode} to @code{t} (@pxref{Easy
-Customization}).
+Customization}).  This will display the list of possible completions
+on the same line as the prompt.  To display the completion candidates
+vertically under the prompt instead, type @kbd{M-x icomplete-vertical-mode},
+or customize the variable @code{icomplete-vertical-mode} to @code{t}.
 
 @findex fido-mode
 @cindex fido mode
diff --git a/etc/NEWS b/etc/NEWS
index c8400ba8c2..cbce41d29d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -477,6 +477,13 @@ documented.
 SMIE is now always enabled and 'ruby-use-smie' only controls whether
 indentation is done using SMIE or with the old ad-hoc code.
 
+** Icomplete
+
++++
+*** New minor mode Icomplete-Vertical mode.
+This mode is based on and identical to Icomplete, except that it displays
+the list of completions candidates vertically.
+
 ---
 ** Specific warnings can now be disabled from the warning buffer.
 When a warning is displayed to the user, the resulting buffer now has
diff --git a/lisp/icomplete.el b/lisp/icomplete.el
index da589c0064..87b3431079 100644
--- a/lisp/icomplete.el
+++ b/lisp/icomplete.el
@@ -562,6 +562,46 @@ icomplete--sorted-completions
                  (completion--cache-all-sorted-completions beg end (cons comp all))))
        finally return all)))
 
+(defun icomplete-vertical-reformat-completions (completions)
+  "Reformat the completion candidates returned by `icomplete-completions'."
+  (save-match-data
+    (if (string-match "^\\((.*)\\|\\[.*\\]\\)?{\\(\\(?:.\\|\n\\)+\\)}"
+                      completions)
+        (format "%s \n%s"
+                (or (match-string 1 completions) "")
+                (match-string 2 completions))
+      completions)))
+
+(defun icomplete-vertical-minibuffer-setup ()
+  "Setup the minibuffer for vertical display of completion candidates."
+  (setq-local redisplay-adhoc-scroll-in-resize-mini-windows nil))
+
+;;;###autoload
+(define-minor-mode icomplete-vertical-mode
+  "Toggle incremental minibuffer completion with vertical display.
+
+This global minor mode is identical to `icomplete-mode' (which see),
+except that it displays the list of completions candidates vertically.
+
+As many completion candidates as possible are displayed, depending on
+the value of `max-mini-window-height'."
+  :global t :group 'icomplete
+  (remove-hook 'icomplete-minibuffer-setup-hook
+               #'icomplete-vertical-minibuffer-setup)
+  (advice-remove 'icomplete-completions
+                 #'icomplete-vertical-reformat-completions)
+  (icomplete-mode -1)
+  (when icomplete-vertical-mode
+    (icomplete-mode 1)
+    (setq icomplete-separator "\n")
+    (setq icomplete-hide-common-prefix nil)
+    ;; ask `icomplete-completions' to return enough completions candidates
+    (setq icomplete-prospects-height 25)
+    (add-hook 'icomplete-minibuffer-setup-hook
+              #'icomplete-vertical-minibuffer-setup)
+    (advice-add 'icomplete-completions
+                :filter-return #'icomplete-vertical-reformat-completions)))
+
 \f
 
 
-- 
2.30.2

Re: [PATCH] icomplete-vertical

[Philip Kaludercic]

Gregory Heytings <gregory@heytings.org> writes:

> +;;;###autoload
> +(define-minor-mode icomplete-vertical-mode
> +  "Toggle incremental minibuffer completion with vertical display.
> +
> +This global minor mode is identical to `icomplete-mode' (which see),
> +except that it displays the list of completions candidates vertically.
> +
> +As many completion candidates as possible are displayed, depending on
> +the value of `max-mini-window-height'."
> +  :global t :group 'icomplete
> +  (remove-hook 'icomplete-minibuffer-setup-hook
> +               #'icomplete-vertical-minibuffer-setup)
> +  (advice-remove 'icomplete-completions
> +                 #'icomplete-vertical-reformat-completions)
> +  (icomplete-mode -1)
> +  (when icomplete-vertical-mode
> +    (icomplete-mode 1)
> +    (setq icomplete-separator "\n")
> +    (setq icomplete-hide-common-prefix nil)
> +    ;; ask `icomplete-completions' to return enough completions candidates
> +    (setq icomplete-prospects-height 25)
> +    (add-hook 'icomplete-minibuffer-setup-hook
> +              #'icomplete-vertical-minibuffer-setup)
> +    (advice-add 'icomplete-completions
> +                :filter-return #'icomplete-vertical-reformat-completions)))
> +

Why is this a patch that uses advice and hook instead of a user option
that changes the behaviour of icomplete directly?

-- 
	Philip K.

Re: [PATCH] icomplete-vertical

[Gregory Heytings]

>
> Why is this a patch that uses advice and hook instead of a user option 
> that changes the behaviour of icomplete directly?
>

Because it's the simplest / cleanest way to do it.  It's a kind of derived 
minor mode.

Re: [PATCH] icomplete-vertical

[Stefan Kangas]

Gregory Heytings <gregory@heytings.org> writes:

>> Why is this a patch that uses advice and hook instead of a user option
>> that changes the behaviour of icomplete directly?
>
> Because it's the simplest / cleanest way to do it.  It's a kind of derived
> minor mode.

Advice is not entirely unproblematic though, so the recommendation is
usually to avoid them in libraries for use by others.  Even more so in
Emacs itself, I think.

See the ELisp Manual sections `(elisp) Advising Named Functions' and
`(elisp) Coding Conventions'.

Re: [PATCH] icomplete-vertical

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Mon, 5 Apr 2021 18:08:02 -0700
> Cc: emacs-devel@gnu.org
> 
> Gregory Heytings <gregory@heytings.org> writes:
> 
> >> Why is this a patch that uses advice and hook instead of a user option
> >> that changes the behaviour of icomplete directly?
> >
> > Because it's the simplest / cleanest way to do it.  It's a kind of derived
> > minor mode.
> 
> Advice is not entirely unproblematic though, so the recommendation is
> usually to avoid them in libraries for use by others.  Even more so in
> Emacs itself, I think.
> 
> See the ELisp Manual sections `(elisp) Advising Named Functions' and
> `(elisp) Coding Conventions'.

Indeed, we prefer not to use advice in our own code.

Re: [PATCH] icomplete-vertical

[Gregory Heytings]

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


>>>> Why is this a patch that uses advice and hook instead of a user 
>>>> option that changes the behaviour of icomplete directly?
>>>
>>> Because it's the simplest / cleanest way to do it.  It's a kind of 
>>> derived minor mode.
>>
>> Advice is not entirely unproblematic though, so the recommendation is 
>> usually to avoid them in libraries for use by others.  Even more so in 
>> Emacs itself, I think.
>>
>> See the ELisp Manual sections `(elisp) Advising Named Functions' and 
>> `(elisp) Coding Conventions'.
>
> Indeed, we prefer not to use advice in our own code.
>

Okay, I did not know that there is a strict rule against advice.  Here is 
the updated patch, which uses a hook instead.  This makes the code even 
more flexible.

[-- Attachment #2: Type: text/x-diff, Size: 6628 bytes --]

From c93232c796a2c0f03403e11f18908dd912e2eb4e Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Tue, 6 Apr 2021 07:28:49 +0000
Subject: [PATCH] Add an icomplete-vertical minor mode

* lisp/icomplete.el (icomplete-vertical-mode): New minor mode.
(icomplete-vertical-reformat-completions,
icomplete-vertical-minibuffer-setup): Auxiliary functions for the
new minor mode.
(icomplete-completions-filter-hook): New hook to filter the
completion candidates returned by 'icomplete-completions'.
(icomplete-completions): Use the new hook.

* etc/NEWS: Mention the new minor mode.

* doc/emacs/buffers.texi: Document the new minor mode.
---
 doc/emacs/buffers.texi |  6 +++-
 etc/NEWS               | 12 +++++++
 lisp/icomplete.el      | 77 ++++++++++++++++++++++++++++++++++++++----
 3 files changed, 87 insertions(+), 8 deletions(-)

diff --git a/doc/emacs/buffers.texi b/doc/emacs/buffers.texi
index 3a166e404a..00e73d800a 100644
--- a/doc/emacs/buffers.texi
+++ b/doc/emacs/buffers.texi
@@ -718,6 +718,7 @@ Icomplete
 @subsection Fast minibuffer selection
 
 @findex icomplete-mode
+@findex icomplete-vertical-mode
 @cindex Icomplete mode
 
   Icomplete global minor mode provides a convenient way to quickly select an
@@ -740,7 +741,10 @@ Icomplete
 
   To enable Icomplete mode, type @kbd{M-x icomplete-mode}, or customize
 the variable @code{icomplete-mode} to @code{t} (@pxref{Easy
-Customization}).
+Customization}).  This will display the list of possible completions
+on the same line as the prompt.  To display the completion candidates
+vertically under the prompt instead, type @kbd{M-x icomplete-vertical-mode},
+or customize the variable @code{icomplete-vertical-mode} to @code{t}.
 
 @findex fido-mode
 @cindex fido mode
diff --git a/etc/NEWS b/etc/NEWS
index 411ea72e7c..fbf25b68c3 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -485,6 +485,18 @@ documented.
 SMIE is now always enabled and 'ruby-use-smie' only controls whether
 indentation is done using SMIE or with the old ad-hoc code.
 
+** Icomplete
+
++++
+*** New minor mode Icomplete-Vertical mode.
+This mode is based on and identical to Icomplete, except that it displays
+the list of completions candidates vertically.
+
++++
+*** New user option 'icomplete-completions-filter-hook'.
+This hook is intended to be used for filtering the completion candidate
+list returned by 'icomplete-completions'.
+
 ---
 ** Specific warnings can now be disabled from the warning buffer.
 When a warning is displayed to the user, the resulting buffer now has
diff --git a/lisp/icomplete.el b/lisp/icomplete.el
index da589c0064..624818f613 100644
--- a/lisp/icomplete.el
+++ b/lisp/icomplete.el
@@ -140,6 +140,24 @@ icomplete-minibuffer-setup-hook
   :type 'hook
   :group 'icomplete)
 
+(defvar icomplete-current-completions nil
+  "Current completion candidate list.")
+
+(defcustom icomplete-completions-filter-hook nil
+  "Filter the completion candidate list returned by `icomplete-completions'.
+
+This rook is run after Icomplete has stored the completion candidate list
+in `icomplete-current-completions'.  It is intended to filter that list.
+For instance:
+
+   (add-hook \\='icomplete-completions-filter-hook
+             (lambda ()
+               (setq icomplete-current-completions
+                     (substring-no-properties icomplete-current-completions))))
+
+will remove all text properties from the completion candidates."
+  :type 'hook
+  :group 'icomplete)
 
 ;;;_* Initialization
 
@@ -562,6 +580,48 @@ icomplete--sorted-completions
                  (completion--cache-all-sorted-completions beg end (cons comp all))))
        finally return all)))
 
+(defun icomplete-vertical-reformat-completions ()
+  "Reformat the completion candidates returned by `icomplete-completions'."
+  (save-match-data
+    (setq icomplete-current-completions
+          (if (string-match "^\\((.*)\\|\\[.*\\]\\)?{\\(\\(?:.\\|\n\\)+\\)}"
+                            icomplete-current-completions)
+              (format "%s \n%s"
+                      (or (match-string 1 icomplete-current-completions) "")
+                      (match-string 2 icomplete-current-completions))
+            icomplete-current-completions))))
+
+(defun icomplete-vertical-minibuffer-setup ()
+  "Setup the minibuffer for vertical display of completion candidates."
+  (setq-local redisplay-adhoc-scroll-in-resize-mini-windows nil))
+
+;;;###autoload
+(define-minor-mode icomplete-vertical-mode
+  "Toggle incremental minibuffer completion with vertical display.
+
+This global minor mode is identical to `icomplete-mode' (which see),
+except that it displays the list of completions candidates vertically.
+
+As many completion candidates as possible are displayed, depending on
+the value of `max-mini-window-height', and the way the mini-window is
+resized depends on `resize-mini-windows'."
+  :global t :group 'icomplete
+  (remove-hook 'icomplete-minibuffer-setup-hook
+               #'icomplete-vertical-minibuffer-setup)
+  (remove-hook 'icomplete-completions-filter-hook
+               #'icomplete-vertical-reformat-completions)
+  (icomplete-mode -1)
+  (when icomplete-vertical-mode
+    (icomplete-mode 1)
+    (setq icomplete-separator "\n")
+    (setq icomplete-hide-common-prefix nil)
+    ;; ask `icomplete-completions' to return enough completions candidates
+    (setq icomplete-prospects-height 25)
+    (add-hook 'icomplete-minibuffer-setup-hook
+              #'icomplete-vertical-minibuffer-setup)
+    (add-hook 'icomplete-completions-filter-hook
+              #'icomplete-vertical-reformat-completions)))
+
 \f
 
 
@@ -782,13 +842,16 @@ icomplete-completions
         ;; Restore the base-size info, since completion-all-sorted-completions
         ;; is cached.
         (if last (setcdr last base-size))
-	(if prospects
-	    (concat determ
-		    "{"
-		    (mapconcat 'identity prospects icomplete-separator)
-		    (and limit (concat icomplete-separator ellipsis))
-		    "}")
-	  (concat determ " [Matched]"))))))
+        (setq icomplete-current-completions
+	      (if prospects
+	          (concat determ
+		          "{"
+		          (mapconcat 'identity prospects icomplete-separator)
+		          (and limit (concat icomplete-separator ellipsis))
+		          "}")
+	        (concat determ " [Matched]")))
+        (run-hooks 'icomplete-completions-filter-hook)
+        icomplete-current-completions))))
 
 ;;; Iswitchb compatibility
 
-- 
2.30.2

Re: [PATCH] icomplete-vertical

[Philip Kaludercic]

Sorry for bringing this up again, but I still don't get why this is a
minor mode from a user perspective. Vertical or horizontal doesn't sound
like something that should be a (global) mode, but a regular
option. Wasn't there a patch or a branch that tried to implement it that
way?

(And yes I know that every minor mode is a user option)

-- 
	Philip K.

Re: [PATCH] icomplete-vertical

[Gregory Heytings]

>
> Sorry for bringing this up again, but I still don't get why this is a 
> minor mode from a user perspective. Vertical or horizontal doesn't sound 
> like something that should be a (global) mode, but a regular option.
>
> [...]
>
> (And yes I know that every minor mode is a user option)
>

If you agree that every minor mode is a user option, what problems do you 
see?

Do you really think it is better to ask users to set five options in their 
init file (icomplete-separator, icomplete-hide-common-prefix, 
icomplete-prospects-height, icomplete-minibuffer-setup-hook and 
icomplete-completions-filter-hook) instead of providing them right away 
with what they want with (icomplete-vertical-mode 1)?

Re: [PATCH] icomplete-vertical

[Philip Kaludercic]

Gregory Heytings <gregory@heytings.org> writes:

> Do you really think it is better to ask users to set five options in
> their init file (icomplete-separator, icomplete-hide-common-prefix, 
> icomplete-prospects-height, icomplete-minibuffer-setup-hook and
> icomplete-completions-filter-hook) instead of providing them right
> away with what they want with (icomplete-vertical-mode 1)?

No, I think it would be better to have one user option like
icomplete-presentation set to 'vertical or something like that.

-- 
	Philip K.

Re: [PATCH] icomplete-vertical

[Gregory Heytings]

>> Do you really think it is better to ask users to set five options in 
>> their init file (icomplete-separator, icomplete-hide-common-prefix, 
>> icomplete-prospects-height, icomplete-minibuffer-setup-hook and 
>> icomplete-completions-filter-hook) instead of providing them right away 
>> with what they want with (icomplete-vertical-mode 1)?
>
> No, I think it would be better to have one user option like 
> icomplete-presentation set to 'vertical or something like that.
>

As I said, it's meant to be a derived minor mode, much like lisp-data-mode 
is derived from prog-mode.  A similar example (in the same file) is 
fido-mode which is derived from icomplete-mode, and (in another file) 
whitespace-newline-mode which is derived from whitespace-mode.

Moreover, I think that in general local changes to code are better than 
global ones whenever that is feasible.  Using a user option would do the 
same thing as the minor mode, except that the changes would be scattered 
through the existing code.

Re: [PATCH] icomplete-vertical

[Dmitry Gutov]

On 06.04.2021 11:54, Philip Kaludercic wrote:
> Sorry for bringing this up again, but I still don't get why this is a
> minor mode from a user perspective. Vertical or horizontal doesn't sound
> like something that should be a (global) mode, but a regular
> option. Wasn't there a patch or a branch that tried to implement it that
> way?

That would be the feature/icomplete-vertical branch.

Which we'll certainly need to consider and compare when reviewing this 
patch.

Re: [PATCH] icomplete-vertical

[Gregory Heytings]

>> Sorry for bringing this up again, but I still don't get why this is a 
>> minor mode from a user perspective. Vertical or horizontal doesn't 
>> sound like something that should be a (global) mode, but a regular 
>> option. Wasn't there a patch or a branch that tried to implement it 
>> that way?
>
> That would be the feature/icomplete-vertical branch.
>
> Which we'll certainly need to consider and compare when reviewing this 
> patch.
>

It was a consequence of the development of that feature branch that the 
redisplay-adhoc-scroll-in-resize-mini-windows variable was added to Emacs. 
This variable, which is used by this patch, is the only way to implement 
the feature correctly, that is, while obeying Emacs' constraints on the 
miniwindow size (max-mini-window-height, resize-mini-windows, size of the 
other windows in the frame, and so forth).

Re: [PATCH] icomplete-vertical

[Eli Zaretskii]

> Date: Tue, 06 Apr 2021 07:44:42 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: emacs-devel@gnu.org
> 
> Okay, I did not know that there is a strict rule against advice.  Here is 
> the updated patch, which uses a hook instead.  This makes the code even 
> more flexible.

Thanks.  A minor nit: every new or modified defcustom should have a
suitable :version tag.

Re: [PATCH] icomplete-vertical

[Gregory Heytings]

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


>> Okay, I did not know that there is a strict rule against advice.  Here 
>> is the updated patch, which uses a hook instead.  This makes the code 
>> even more flexible.
>
> Thanks.  A minor nit: every new or modified defcustom should have a 
> suitable :version tag.
>

I'm not sure if I should send a new version of the patch for a minor nit, 
but just in case, here it is.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=0001-Add-an-icomplete-vertical-minor-mode.patch, Size: 6722 bytes --]

From bc100faf2e36055750943230fc9212118cc04d27 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Tue, 6 Apr 2021 12:43:29 +0000
Subject: [PATCH] Add an icomplete-vertical minor mode

* lisp/icomplete.el (icomplete-vertical-mode): New minor mode.
(icomplete-vertical-reformat-completions,
icomplete-vertical-minibuffer-setup): Auxiliary functions for the
new minor mode.
(icomplete-completions-filter-hook): New hook to filter the
completion candidates returned by 'icomplete-completions'.
(icomplete-completions): Use the new hook.

* etc/NEWS: Mention the new minor mode.

* doc/emacs/buffers.texi: Document the new minor mode.
---
 doc/emacs/buffers.texi |  6 +++-
 etc/NEWS               | 12 +++++++
 lisp/icomplete.el      | 78 ++++++++++++++++++++++++++++++++++++++----
 3 files changed, 88 insertions(+), 8 deletions(-)

diff --git a/doc/emacs/buffers.texi b/doc/emacs/buffers.texi
index 3a166e404a..00e73d800a 100644
--- a/doc/emacs/buffers.texi
+++ b/doc/emacs/buffers.texi
@@ -718,6 +718,7 @@ rule or another is easier for you to remember and apply quickly.
 @subsection Fast minibuffer selection
 
 @findex icomplete-mode
+@findex icomplete-vertical-mode
 @cindex Icomplete mode
 
   Icomplete global minor mode provides a convenient way to quickly select an
@@ -740,7 +741,10 @@ of directories.
 
   To enable Icomplete mode, type @kbd{M-x icomplete-mode}, or customize
 the variable @code{icomplete-mode} to @code{t} (@pxref{Easy
-Customization}).
+Customization}).  This will display the list of possible completions
+on the same line as the prompt.  To display the completion candidates
+vertically under the prompt instead, type @kbd{M-x icomplete-vertical-mode},
+or customize the variable @code{icomplete-vertical-mode} to @code{t}.
 
 @findex fido-mode
 @cindex fido mode
diff --git a/etc/NEWS b/etc/NEWS
index c8400ba8c2..5d1e5e3599 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -477,6 +477,18 @@ documented.
 SMIE is now always enabled and 'ruby-use-smie' only controls whether
 indentation is done using SMIE or with the old ad-hoc code.
 
+** Icomplete
+
++++
+*** New minor mode Icomplete-Vertical mode.
+This mode is based on and identical to Icomplete, except that it displays
+the list of completions candidates vertically.
+
++++
+*** New user option 'icomplete-completions-filter-hook'.
+This hook is intended to be used for filtering the completion candidate
+list returned by 'icomplete-completions'.
+
 ---
 ** Specific warnings can now be disabled from the warning buffer.
 When a warning is displayed to the user, the resulting buffer now has
diff --git a/lisp/icomplete.el b/lisp/icomplete.el
index da589c0064..743837f1a7 100644
--- a/lisp/icomplete.el
+++ b/lisp/icomplete.el
@@ -140,6 +140,25 @@ icompletion is occurring."
   :type 'hook
   :group 'icomplete)
 
+(defvar icomplete-current-completions nil
+  "Current completion candidate list.")
+
+(defcustom icomplete-completions-filter-hook nil
+  "Filter the completion candidate list returned by `icomplete-completions'.
+
+This rook is run after Icomplete has stored the completion candidate list
+in `icomplete-current-completions'.  It is intended to filter that list.
+For instance:
+
+   (add-hook \\='icomplete-completions-filter-hook
+             (lambda ()
+               (setq icomplete-current-completions
+                     (substring-no-properties icomplete-current-completions))))
+
+will remove all text properties from the completion candidates."
+  :type 'hook
+  :group 'icomplete
+  :version "28.1")
 
 ;;;_* Initialization
 
@@ -562,6 +581,48 @@ Usually run by inclusion in `minibuffer-setup-hook'."
                  (completion--cache-all-sorted-completions beg end (cons comp all))))
        finally return all)))
 
+(defun icomplete-vertical-reformat-completions ()
+  "Reformat the completion candidates returned by `icomplete-completions'."
+  (save-match-data
+    (setq icomplete-current-completions
+          (if (string-match "^\\((.*)\\|\\[.*\\]\\)?{\\(\\(?:.\\|\n\\)+\\)}"
+                            icomplete-current-completions)
+              (format "%s \n%s"
+                      (or (match-string 1 icomplete-current-completions) "")
+                      (match-string 2 icomplete-current-completions))
+            icomplete-current-completions))))
+
+(defun icomplete-vertical-minibuffer-setup ()
+  "Setup the minibuffer for vertical display of completion candidates."
+  (setq-local redisplay-adhoc-scroll-in-resize-mini-windows nil))
+
+;;;###autoload
+(define-minor-mode icomplete-vertical-mode
+  "Toggle incremental minibuffer completion with vertical display.
+
+This global minor mode is identical to `icomplete-mode' (which see),
+except that it displays the list of completions candidates vertically.
+
+As many completion candidates as possible are displayed, depending on
+the value of `max-mini-window-height', and the way the mini-window is
+resized depends on `resize-mini-windows'."
+  :global t :group 'icomplete
+  (remove-hook 'icomplete-minibuffer-setup-hook
+               #'icomplete-vertical-minibuffer-setup)
+  (remove-hook 'icomplete-completions-filter-hook
+               #'icomplete-vertical-reformat-completions)
+  (icomplete-mode -1)
+  (when icomplete-vertical-mode
+    (icomplete-mode 1)
+    (setq icomplete-separator "\n")
+    (setq icomplete-hide-common-prefix nil)
+    ;; ask `icomplete-completions' to return enough completions candidates
+    (setq icomplete-prospects-height 25)
+    (add-hook 'icomplete-minibuffer-setup-hook
+              #'icomplete-vertical-minibuffer-setup)
+    (add-hook 'icomplete-completions-filter-hook
+              #'icomplete-vertical-reformat-completions)))
+
 \f
 
 
@@ -782,13 +843,16 @@ matches exist."
         ;; Restore the base-size info, since completion-all-sorted-completions
         ;; is cached.
         (if last (setcdr last base-size))
-	(if prospects
-	    (concat determ
-		    "{"
-		    (mapconcat 'identity prospects icomplete-separator)
-		    (and limit (concat icomplete-separator ellipsis))
-		    "}")
-	  (concat determ " [Matched]"))))))
+        (setq icomplete-current-completions
+	      (if prospects
+	          (concat determ
+		          "{"
+		          (mapconcat 'identity prospects icomplete-separator)
+		          (and limit (concat icomplete-separator ellipsis))
+		          "}")
+	        (concat determ " [Matched]")))
+        (run-hooks 'icomplete-completions-filter-hook)
+        icomplete-current-completions))))
 
 ;;; Iswitchb compatibility
 
-- 
2.30.2

Re: [PATCH] icomplete-vertical

[Stefan Monnier]

> As I said, it's meant to be a derived minor mode, much like lisp-data-mode
> is derived from prog-mode.

FWIW, we don't have a notion of "derived minor-mode" yet.

> A similar example (in the same file) is
> fido-mode which is derived from icomplete-mode, and (in another file)
> whitespace-newline-mode which is derived from whitespace-mode.

The example of `fido-mode` is actually the one I would put forward to
argue that icomplete-vertical should not itself activate
`icomplete-mode` but should just change the way completion are
displayed, so it can be combined wither with `fido-mode` or with the
normal `icomplete-mode`.

> Moreover, I think that in general local changes to code are better than
> global ones whenever that is feasible.  Using a user option would do the
> same thing as the minor mode, except that the changes would be scattered
> through the existing code.

I don't see how.  I'd imagine a code like:

    ;;;###autoload
    (define-minor-mode icomplete-vertical-mode
      "Toggle the use of vertical display in `icomplete-mode`.
    
    As many completion candidates as possible are displayed, depending on
    the value of `max-mini-window-height', and the way the mini-window is
    resized depends on `resize-mini-windows'."
      :global t
      (remove-hook 'icomplete-minibuffer-setup-hook
                   #'icomplete-vertical-minibuffer-setup)
      (remove-hook 'icomplete-completions-filter-hook
                   #'icomplete-vertical-reformat-completions)
      (when icomplete-vertical-mode
        (setq icomplete-separator "\n")
        (setq icomplete-hide-common-prefix nil)
        ;; ask `icomplete-completions' to return enough completions candidates
        (setq icomplete-prospects-height 25)
        (add-hook 'icomplete-minibuffer-setup-hook
                  #'icomplete-vertical-minibuffer-setup)
        (add-hook 'icomplete-completions-filter-hook
                  #'icomplete-vertical-reformat-completions)))

[ BTW, in the above code (which I basically copy/pasted from your
  patch), we should save&restore the values of `icomplete-separator`,
  `icomplete-hide-common-prefix`, and `icomplete-prospects-height`.
  Also, I'd recommend to use "--" in the names of the new hook
  functions.  And while I'm nitpicking I might as well mention that
  comments should be capitalized and punctuated.  ]


        Stefan

Re: [PATCH] icomplete-vertical

[Eli Zaretskii]

> Date: Tue, 06 Apr 2021 12:48:03 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: emacs-devel@gnu.org
> 
> > Thanks.  A minor nit: every new or modified defcustom should have a 
> > suitable :version tag.
> 
> I'm not sure if I should send a new version of the patch for a minor nit, 
> but just in case, here it is.

Indeed, it's enough to have those nits fixed in the final version; no
need to send a new one each time someone finds a minor issue.

Thanks.

Re: [PATCH] icomplete-vertical

[Gregory Heytings]

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


>> As I said, it's meant to be a derived minor mode, much like 
>> lisp-data-mode is derived from prog-mode.
>
> FWIW, we don't have a notion of "derived minor-mode" yet.
>

Yes, which is why I tried to "emulate" it.  I think it makes sense.  But 
it's not the point under discussion.

>> Using a user option would do the same thing as the minor mode, except 
>> that the changes would be scattered through the existing code.
>
> I don't see how.  I'd imagine a code like:
>

Thank you, that makes perfect sense indeed.

Thanks a lot also for your other comments, which I included in the 
attached revised patch.  The only thing I wasn't sure is whether it was 
intentional that you removed the :group 'icomplete from the minor mode 
tags, I think it wasn't, so it's still there.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=0001-Add-an-icomplete-vertical-minor-mode.patch, Size: 6986 bytes --]

From c593ba102f16bcd2fad68ae1c9b5a0c873b3c788 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Tue, 6 Apr 2021 13:44:04 +0000
Subject: [PATCH] Add an icomplete-vertical minor mode

* lisp/icomplete.el (icomplete-vertical-mode): New minor mode.
(icomplete--vertical-reformat-completions,
icomplete--vertical-minibuffer-setup): Auxiliary functions for the
new minor mode.
(icomplete-completions-filter-hook): New hook to filter the
completion candidates returned by 'icomplete-completions'.
(icomplete-completions): Use the new hook.

* etc/NEWS: Mention the new minor mode.

* doc/emacs/buffers.texi: Document the new minor mode.
---
 doc/emacs/buffers.texi |  6 ++-
 etc/NEWS               | 12 ++++++
 lisp/icomplete.el      | 83 ++++++++++++++++++++++++++++++++++++++----
 3 files changed, 93 insertions(+), 8 deletions(-)

diff --git a/doc/emacs/buffers.texi b/doc/emacs/buffers.texi
index 3a166e404a..00e73d800a 100644
--- a/doc/emacs/buffers.texi
+++ b/doc/emacs/buffers.texi
@@ -718,6 +718,7 @@ rule or another is easier for you to remember and apply quickly.
 @subsection Fast minibuffer selection
 
 @findex icomplete-mode
+@findex icomplete-vertical-mode
 @cindex Icomplete mode
 
   Icomplete global minor mode provides a convenient way to quickly select an
@@ -740,7 +741,10 @@ of directories.
 
   To enable Icomplete mode, type @kbd{M-x icomplete-mode}, or customize
 the variable @code{icomplete-mode} to @code{t} (@pxref{Easy
-Customization}).
+Customization}).  This will display the list of possible completions
+on the same line as the prompt.  To display the completion candidates
+vertically under the prompt instead, type @kbd{M-x icomplete-vertical-mode},
+or customize the variable @code{icomplete-vertical-mode} to @code{t}.
 
 @findex fido-mode
 @cindex fido mode
diff --git a/etc/NEWS b/etc/NEWS
index c8400ba8c2..5d1e5e3599 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -477,6 +477,18 @@ documented.
 SMIE is now always enabled and 'ruby-use-smie' only controls whether
 indentation is done using SMIE or with the old ad-hoc code.
 
+** Icomplete
+
++++
+*** New minor mode Icomplete-Vertical mode.
+This mode is based on and identical to Icomplete, except that it displays
+the list of completions candidates vertically.
+
++++
+*** New user option 'icomplete-completions-filter-hook'.
+This hook is intended to be used for filtering the completion candidate
+list returned by 'icomplete-completions'.
+
 ---
 ** Specific warnings can now be disabled from the warning buffer.
 When a warning is displayed to the user, the resulting buffer now has
diff --git a/lisp/icomplete.el b/lisp/icomplete.el
index da589c0064..dc9e88f97c 100644
--- a/lisp/icomplete.el
+++ b/lisp/icomplete.el
@@ -140,6 +140,25 @@ icompletion is occurring."
   :type 'hook
   :group 'icomplete)
 
+(defvar icomplete-current-completions nil
+  "Current completion candidate list.")
+
+(defcustom icomplete-completions-filter-hook nil
+  "Filter the completion candidate list returned by `icomplete-completions'.
+
+This rook is run after Icomplete has stored the completion candidate list
+in `icomplete-current-completions'.  It is intended to filter that list.
+For instance:
+
+   (add-hook \\='icomplete-completions-filter-hook
+             (lambda ()
+               (setq icomplete-current-completions
+                     (substring-no-properties icomplete-current-completions))))
+
+will remove all text properties from the completion candidates."
+  :type 'hook
+  :group 'icomplete
+  :version "28.1")
 
 ;;;_* Initialization
 
@@ -562,6 +581,53 @@ Usually run by inclusion in `minibuffer-setup-hook'."
                  (completion--cache-all-sorted-completions beg end (cons comp all))))
        finally return all)))
 
+(defun icomplete--vertical-reformat-completions ()
+  "Reformat the completion candidates returned by `icomplete-completions'."
+  (save-match-data
+    (setq icomplete-current-completions
+          (if (string-match "^\\((.*)\\|\\[.*\\]\\)?{\\(\\(?:.\\|\n\\)+\\)}"
+                            icomplete-current-completions)
+              (format "%s \n%s"
+                      (or (match-string 1 icomplete-current-completions) "")
+                      (match-string 2 icomplete-current-completions))
+            icomplete-current-completions))))
+
+(defun icomplete--vertical-minibuffer-setup ()
+  "Setup the minibuffer for vertical display of completion candidates."
+  (setq-local redisplay-adhoc-scroll-in-resize-mini-windows nil))
+
+(defvar icomplete--vertical-saved-values nil)
+
+;;;###autoload
+(define-minor-mode icomplete-vertical-mode
+  "Toggle the use of vertical display in `icomplete-mode'.
+
+As many completion candidates as possible are displayed, depending on
+the value of `max-mini-window-height', and the way the mini-window is
+resized depends on `resize-mini-windows'."
+  :global t
+  :group 'icomplete
+  (when icomplete--vertical-saved-values
+    (eval `(setq ,@icomplete--vertical-saved-values))
+    (setq icomplete--vertical-saved-values nil))
+  (remove-hook 'icomplete-minibuffer-setup-hook
+               #'icomplete--vertical-minibuffer-setup)
+  (remove-hook 'icomplete-completions-filter-hook
+               #'icomplete--vertical-reformat-completions)
+  (when icomplete-vertical-mode
+    (setq icomplete--vertical-saved-values
+          (list 'icomplete-separator icomplete-separator
+                'icomplete-hide-common-prefix icomplete-hide-common-prefix
+                'icomplete-prospects-height icomplete-prospects-height))
+    (setq icomplete-separator "\n")
+    (setq icomplete-hide-common-prefix nil)
+    ;; Ask `icomplete-completions' to return enough completions candidates.
+    (setq icomplete-prospects-height 25)
+    (add-hook 'icomplete-minibuffer-setup-hook
+              #'icomplete--vertical-minibuffer-setup)
+    (add-hook 'icomplete-completions-filter-hook
+              #'icomplete--vertical-reformat-completions)))
+
 \f
 
 
@@ -782,13 +848,16 @@ matches exist."
         ;; Restore the base-size info, since completion-all-sorted-completions
         ;; is cached.
         (if last (setcdr last base-size))
-	(if prospects
-	    (concat determ
-		    "{"
-		    (mapconcat 'identity prospects icomplete-separator)
-		    (and limit (concat icomplete-separator ellipsis))
-		    "}")
-	  (concat determ " [Matched]"))))))
+        (setq icomplete-current-completions
+	      (if prospects
+	          (concat determ
+		          "{"
+		          (mapconcat 'identity prospects icomplete-separator)
+		          (and limit (concat icomplete-separator ellipsis))
+		          "}")
+	        (concat determ " [Matched]")))
+        (run-hooks 'icomplete-completions-filter-hook)
+        icomplete-current-completions))))
 
 ;;; Iswitchb compatibility
 
-- 
2.30.2

Re: [PATCH] icomplete-vertical

[Stefan Monnier]

> Thanks a lot also for your other comments, which I included in the attached
> revised patch.  The only thing I wasn't sure is whether it was intentional
> that you removed the :group 'icomplete from the minor mode tags,

It was, since I presume this is placed inside the icomplete.el file where
this arg is redundant.


        Stefan

Re: [PATCH] icomplete-vertical

[Gregory Heytings]

>> Thanks a lot also for your other comments, which I included in the 
>> attached revised patch.  The only thing I wasn't sure is whether it was 
>> intentional that you removed the :group 'icomplete from the minor mode 
>> tags,
>
> It was, since I presume this is placed inside the icomplete.el file 
> where this arg is redundant.
>

Okay, I did not know (and did not check) this.  Note that the tag is 
present for both fido-mode and icomplete-mode.

Re: [PATCH] icomplete-vertical

[Ergus]

On Tue, Apr 06, 2021 at 09:20:53AM -0400, Stefan Monnier wrote:
>> As I said, it's meant to be a derived minor mode, much like lisp-data-mode
>> is derived from prog-mode.
>
>FWIW, we don't have a notion of "derived minor-mode" yet.
>
>> A similar example (in the same file) is
>> fido-mode which is derived from icomplete-mode, and (in another file)
>> whitespace-newline-mode which is derived from whitespace-mode.
>
>The example of `fido-mode` is actually the one I would put forward to
>argue that icomplete-vertical should not itself activate
>`icomplete-mode` but should just change the way completion are
>displayed, so it can be combined wither with `fido-mode` or with the
>normal `icomplete-mode`.
>
>> Moreover, I think that in general local changes to code are better than
>> global ones whenever that is feasible.  Using a user option would do the
>> same thing as the minor mode, except that the changes would be scattered
>> through the existing code.
>
>I don't see how.  I'd imagine a code like:
>
>    ;;;###autoload
>    (define-minor-mode icomplete-vertical-mode
>      "Toggle the use of vertical display in `icomplete-mode`.
>
>    As many completion candidates as possible are displayed, depending on
>    the value of `max-mini-window-height', and the way the mini-window is
>    resized depends on `resize-mini-windows'."
>      :global t
>      (remove-hook 'icomplete-minibuffer-setup-hook
>                   #'icomplete-vertical-minibuffer-setup)
>      (remove-hook 'icomplete-completions-filter-hook
>                   #'icomplete-vertical-reformat-completions)
>      (when icomplete-vertical-mode
>        (setq icomplete-separator "\n")
>        (setq icomplete-hide-common-prefix nil)
>        ;; ask `icomplete-completions' to return enough completions candidates
>        (setq icomplete-prospects-height 25)
>        (add-hook 'icomplete-minibuffer-setup-hook
>                  #'icomplete-vertical-minibuffer-setup)
>        (add-hook 'icomplete-completions-filter-hook
>                  #'icomplete-vertical-reformat-completions)))
>
>[ BTW, in the above code (which I basically copy/pasted from your
>  patch), we should save&restore the values of `icomplete-separator`,
>  `icomplete-hide-common-prefix`, and `icomplete-prospects-height`.
>  Also, I'd recommend to use "--" in the names of the new hook
>  functions.  And while I'm nitpicking I might as well mention that
>  comments should be capitalized and punctuated.  ]
>
>
>        Stefan

Just to remind. Some months ago I actually asked here about the
possibility/complexity to add a parameter to the define-minor-mode
function so we could do something like:

(icomplete-mode 'vertical)
(icomplete-mode 'horizontal)
(icomplete-mode 'fido)
(icomplete-mode nil)

And so on... in order to set the proper hooks and initializations

The use case I had in mind was exactly that. Because I was working on
icomplete-vertical, icomplete-tabular, and some others that in general
just require minimal differences between each other.

I don't actually like the idea of adding a new mode for every different
implementation and would prefer an option; but I understand why Gregory
needed a new mode to do this (as well as fido-mode did.)

Re: [PATCH] icomplete-vertical

[Stefan Monnier]

> Okay, I did not know (and did not check) this.  Note that the tag is present
> for both fido-mode and icomplete-mode.

Indeed, it's harmless, a question of style,


        Stefan

Re: [PATCH] icomplete-vertical

[Gregory Heytings]

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


>> Okay, I did not know (and did not check) this.  Note that the tag is 
>> present for both fido-mode and icomplete-mode.
>
> Indeed, it's harmless, a question of style,
>

Okay.  Here is the final (?) version of the patch; I forgot to update the 
docs in the previous one.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=0001-Add-an-icomplete-vertical-minor-mode.patch, Size: 7572 bytes --]

From 93278cb687a2ab25ed67fde026224e6b7d00be70 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Tue, 6 Apr 2021 14:42:25 +0000
Subject: [PATCH] Add an icomplete-vertical minor mode

* lisp/icomplete.el (icomplete-vertical-mode): New minor mode.
(icomplete--vertical-reformat-completions,
icomplete--vertical-minibuffer-setup): Auxiliary functions for the
new minor mode.
(icomplete-completions-filter-hook): New hook to filter the
completion candidates returned by 'icomplete-completions'.
(icomplete-completions): Use the new hook.

* etc/NEWS: Mention the new minor mode.

* doc/emacs/buffers.texi: Document the new minor mode.
---
 doc/emacs/buffers.texi | 12 +++++--
 etc/NEWS               | 12 +++++++
 lisp/icomplete.el      | 82 ++++++++++++++++++++++++++++++++++++++----
 3 files changed, 97 insertions(+), 9 deletions(-)

diff --git a/doc/emacs/buffers.texi b/doc/emacs/buffers.texi
index 3a166e404a..e82bada391 100644
--- a/doc/emacs/buffers.texi
+++ b/doc/emacs/buffers.texi
@@ -718,6 +718,7 @@ rule or another is easier for you to remember and apply quickly.
 @subsection Fast minibuffer selection
 
 @findex icomplete-mode
+@findex icomplete-vertical-mode
 @cindex Icomplete mode
 
   Icomplete global minor mode provides a convenient way to quickly select an
@@ -740,7 +741,10 @@ of directories.
 
   To enable Icomplete mode, type @kbd{M-x icomplete-mode}, or customize
 the variable @code{icomplete-mode} to @code{t} (@pxref{Easy
-Customization}).
+Customization}).  This will display the list of possible completions
+on the same line as the prompt.  To display the completion candidates
+vertically under the prompt, also type @kbd{M-x icomplete-vertical-mode},
+or customize the variable @code{icomplete-vertical-mode} to @code{t}.
 
 @findex fido-mode
 @cindex fido mode
@@ -763,7 +767,11 @@ your initialization file (@pxref{Init File}):
 
   To enable Fido mode, type @kbd{M-x fido-mode}, or customize
 the variable @code{fido-mode} to @code{t} (@pxref{Easy
-Customization}).
+Customization}).  Like Icomplete mode, Fido mode displays the list of
+possible completions on the same line as the prompt.  To display the
+completion candidates vertically under the prompt, also type
+@kbd{M-x icomplete-vertical-mode}, or customize the variable
+@code{icomplete-vertical-mode} to @code{t}.
 
 @node Buffer Menus
 @subsection Customizing Buffer Menus
diff --git a/etc/NEWS b/etc/NEWS
index c8400ba8c2..327c73e73c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -477,6 +477,18 @@ documented.
 SMIE is now always enabled and 'ruby-use-smie' only controls whether
 indentation is done using SMIE or with the old ad-hoc code.
 
+** Icomplete
+
++++
+*** New minor mode Icomplete-Vertical mode.
+This mode is intended to be used with Icomplete or Fido, to display the
+list of completions candidates vertically instead of horizontally.
+
++++
+*** New user option 'icomplete-completions-filter-hook'.
+This hook is intended to be used for filtering the completion candidate
+list returned by 'icomplete-completions'.
+
 ---
 ** Specific warnings can now be disabled from the warning buffer.
 When a warning is displayed to the user, the resulting buffer now has
diff --git a/lisp/icomplete.el b/lisp/icomplete.el
index da589c0064..ec8ad68972 100644
--- a/lisp/icomplete.el
+++ b/lisp/icomplete.el
@@ -140,6 +140,25 @@ icompletion is occurring."
   :type 'hook
   :group 'icomplete)
 
+(defvar icomplete-current-completions nil
+  "Current completion candidate list.")
+
+(defcustom icomplete-completions-filter-hook nil
+  "Filter the completion candidate list returned by `icomplete-completions'.
+
+This rook is run after Icomplete has stored the completion candidate list
+in `icomplete-current-completions'.  It is intended to filter that list.
+For instance:
+
+   (add-hook \\='icomplete-completions-filter-hook
+             (lambda ()
+               (setq icomplete-current-completions
+                     (substring-no-properties icomplete-current-completions))))
+
+will remove all text properties from the completion candidates."
+  :type 'hook
+  :group 'icomplete
+  :version "28.1")
 
 ;;;_* Initialization
 
@@ -562,6 +581,52 @@ Usually run by inclusion in `minibuffer-setup-hook'."
                  (completion--cache-all-sorted-completions beg end (cons comp all))))
        finally return all)))
 
+(defun icomplete--vertical-reformat-completions ()
+  "Reformat the completion candidates returned by `icomplete-completions'."
+  (save-match-data
+    (setq icomplete-current-completions
+          (if (string-match "^\\((.*)\\|\\[.*\\]\\)?{\\(\\(?:.\\|\n\\)+\\)}"
+                            icomplete-current-completions)
+              (format "%s \n%s"
+                      (or (match-string 1 icomplete-current-completions) "")
+                      (match-string 2 icomplete-current-completions))
+            icomplete-current-completions))))
+
+(defun icomplete--vertical-minibuffer-setup ()
+  "Setup the minibuffer for vertical display of completion candidates."
+  (setq-local redisplay-adhoc-scroll-in-resize-mini-windows nil))
+
+(defvar icomplete--vertical-saved-values nil)
+
+;;;###autoload
+(define-minor-mode icomplete-vertical-mode
+  "Toggle the use of vertical display in `icomplete-mode'.
+
+As many completion candidates as possible are displayed, depending on
+the value of `max-mini-window-height', and the way the mini-window is
+resized depends on `resize-mini-windows'."
+  :global t
+  (when icomplete--vertical-saved-values
+    (eval `(setq ,@icomplete--vertical-saved-values))
+    (setq icomplete--vertical-saved-value nil))
+  (remove-hook 'icomplete-minibuffer-setup-hook
+               #'icomplete--vertical-minibuffer-setup)
+  (remove-hook 'icomplete-completions-filter-hook
+               #'icomplete--vertical-reformat-completions)
+  (when icomplete-vertical-mode
+    (setq icomplete--vertical-saved-values
+          (list 'icomplete-separator icomplete-separator
+                'icomplete-hide-common-prefix icomplete-hide-common-prefix
+                'icomplete-prospects-height icomplete-prospects-height))
+    (setq icomplete-separator "\n")
+    (setq icomplete-hide-common-prefix nil)
+    ;; Ask `icomplete-completions' to return enough completions candidates.
+    (setq icomplete-prospects-height 25)
+    (add-hook 'icomplete-minibuffer-setup-hook
+              #'icomplete--vertical-minibuffer-setup)
+    (add-hook 'icomplete-completions-filter-hook
+              #'icomplete--vertical-reformat-completions)))
+
 \f
 
 
@@ -782,13 +847,16 @@ matches exist."
         ;; Restore the base-size info, since completion-all-sorted-completions
         ;; is cached.
         (if last (setcdr last base-size))
-	(if prospects
-	    (concat determ
-		    "{"
-		    (mapconcat 'identity prospects icomplete-separator)
-		    (and limit (concat icomplete-separator ellipsis))
-		    "}")
-	  (concat determ " [Matched]"))))))
+        (setq icomplete-current-completions
+	      (if prospects
+	          (concat determ
+		          "{"
+		          (mapconcat 'identity prospects icomplete-separator)
+		          (and limit (concat icomplete-separator ellipsis))
+		          "}")
+	        (concat determ " [Matched]")))
+        (run-hooks 'icomplete-completions-filter-hook)
+        icomplete-current-completions))))
 
 ;;; Iswitchb compatibility
 
-- 
2.30.2

Re: [PATCH] icomplete-vertical

[Philip Kaludercic]

Ergus <spacibba@aol.com> writes:

> The use case I had in mind was exactly that. Because I was working on
> icomplete-vertical, icomplete-tabular, and some others that in general
> just require minimal differences between each other.

What would icomplete-tabular have looked like?

-- 
	Philip K.

Re: [PATCH] icomplete-vertical

[Ergus]

On Tue, Apr 06, 2021 at 05:17:44PM +0200, Philip Kaludercic wrote:
>Ergus <spacibba@aol.com> writes:
>
>> The use case I had in mind was exactly that. Because I was working on
>> icomplete-vertical, icomplete-tabular, and some others that in general
>> just require minimal differences between each other.
>
>What would icomplete-tabular have looked like?
>
>-- 
>	Philip K.

I never finish that because some of the needed intermediate changes in
icomplete bothered Gregory. 

But the idea was to make it behave similar to zsh, but using the
minibuffer instead if the completions buffer (like the feature/zcomplete
I also abandon).

Re: [PATCH] icomplete-vertical

[Juri Linkov]

> Okay.  Here is the final (?) version of the patch; I forgot to update the
> docs in the previous one.

I tried it out, and everything works nicely.
The only problem I see this code is too hackish:

> +(defun icomplete--vertical-reformat-completions ()
> +  "Reformat the completion candidates returned by `icomplete-completions'."
> +  (save-match-data
> +    (setq icomplete-current-completions
> +          (if (string-match "^\\((.*)\\|\\[.*\\]\\)?{\\(\\(?:.\\|\n\\)+\\)}"
> +                            icomplete-current-completions)
> +              (format "%s \n%s"
> +                      (or (match-string 1 icomplete-current-completions) "")
> +                      (match-string 2 icomplete-current-completions))
> +            icomplete-current-completions))))

I guess like there are variables that affect formatting of the output
e.g. 'icomplete-hide-common-prefix', there could be a new similar
variable 'icomplete-add-initial-newline' to add a newline in
the 'icomplete-completions' formatting function.

Re: [PATCH] icomplete-vertical

[Gregory Heytings]

>> Okay.  Here is the final (?) version of the patch; I forgot to update 
>> the docs in the previous one.
>
> I tried it out, and everything works nicely.
>

Thanks for reviewing!

>
> The only problem I see this code is too hackish:
>
>> +(defun icomplete--vertical-reformat-completions ()
>> +  "Reformat the completion candidates returned by `icomplete-completions'."
>> +  (save-match-data
>> +    (setq icomplete-current-completions
>> +          (if (string-match "^\\((.*)\\|\\[.*\\]\\)?{\\(\\(?:.\\|\n\\)+\\)}"
>> +                            icomplete-current-completions)
>> +              (format "%s \n%s"
>> +                      (or (match-string 1 icomplete-current-completions) "")
>> +                      (match-string 2 icomplete-current-completions))
>> +            icomplete-current-completions))))
>
> I guess like there are variables that affect formatting of the output 
> e.g. 'icomplete-hide-common-prefix', there could be a new similar 
> variable 'icomplete-add-initial-newline' to add a newline in the 
> 'icomplete-completions' formatting function.
>

It is (a bit) hackish because the output of icomplete-completions is (a 
bit) hackish.

Adding an initial newline to the output of icomplete-completions wouldn't 
be enough, icomplete-completions uses () [] {} and | as delimiters.  Of 
course one could change all these delimiters into user options, but IMO it 
would add more complexity than necessary, and with the new 
icomplete-completions-filter-hook anyone can reformat the output the way 
they want it.

Re: [PATCH] icomplete-vertical

[Juri Linkov]

>> The only problem I see this code is too hackish:
>
> It is (a bit) hackish because the output of icomplete-completions is (a
> bit) hackish.

Good to see that the new branch avoids such hacks.  I tested it out,
and everything is nice.  After using it a while I noticed that one
useful feature is missing, but it could be added later to not prevent
merging the current version to master now.  What is missing is the
scrolling commands that could be bound to C-v and M-v to scroll completions.
For the simplest implementation, this means just to call icomplete-forward-completions
N times where N is the height of the displayed completions list.

Re: [PATCH] icomplete-vertical

[João Távora]

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

> Good to see that the new branch avoids such hacks.  I tested it out,
and everything is nice.

Did you test the branch that me and Gregory are working
on off-list?

Probably we should be on-list.  The branch is
scratch/icomplete-vertical-mode-gregory-and-joao and it's
functionally equivalent to Gregory's last on-list patch but a bit
simpler and cleaner on the implementation.

C-n and C-p are supported, but indeed C-v and M-v are not.
It's a good idea to support them, yes. And maybe your idea is
the best way.  Feel free to add a commit or two to that branch.
Else, let's just push this simpler version to master.

João



On Sat, Apr 10, 2021 at 10:53 PM Juri Linkov <juri@linkov.net> wrote:

> >> The only problem I see this code is too hackish:
> >
> > It is (a bit) hackish because the output of icomplete-completions is (a
> > bit) hackish.
>
> Good to see that the new branch avoids such hacks.  I tested it out,
> and everything is nice.  After using it a while I noticed that one
> useful feature is missing, but it could be added later to not prevent
> merging the current version to master now.  What is missing is the
> scrolling commands that could be bound to C-v and M-v to scroll
> completions.
> For the simplest implementation, this means just to call
> icomplete-forward-completions
> N times where N is the height of the displayed completions list.
>
>

-- 
João Távora

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

Re: [PATCH] icomplete-vertical

[Gregory Heytings]

>
> Good to see that the new branch avoids such hacks.  I tested it out, and 
> everything is nice.  After using it a while I noticed that one useful 
> feature is missing, but it could be added later to not prevent merging 
> the current version to master now.  What is missing is the scrolling 
> commands that could be bound to C-v and M-v to scroll completions. For 
> the simplest implementation, this means just to call 
> icomplete-forward-completions N times where N is the height of the 
> displayed completions list.
>

Alas that wouldn't work in all cases (namely when point is not on the 
first line), in such cases you would not see one or more completion 
candidates.  I plan to do that later, but it needs some care.

Re: [PATCH] icomplete-vertical

[Juri Linkov]

>> Good to see that the new branch avoids such hacks.  I tested it out,
>> and everything is nice.
>
> Did you test the branch that me and Gregory are working
> on off-list?
>
> Probably we should be on-list.  The branch is
> scratch/icomplete-vertical-mode-gregory-and-joao and it's
> functionally equivalent to Gregory's last on-list patch but a bit
> simpler and cleaner on the implementation.

Ergus discovered this new branch, so I tried it out.

> C-n and C-p are supported, but indeed C-v and M-v are not.
> It's a good idea to support them, yes. And maybe your idea is
> the best way.  Feel free to add a commit or two to that branch.
> Else, let's just push this simpler version to master.

If the basic functionally already works, then better to push the
existing version to master.

Re: [PATCH] icomplete-vertical

[João Távora]

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

On Sat, Apr 10, 2021 at 11:44 PM Juri Linkov <juri@linkov.net> wrote:

>
> If the basic functionally already works, then better to push the
> existing version to master.
>

OK, done.

Uff, but I shouldn't have.    I just realized that Gregory is not on the
copyright file!

Eli, sorry, I assumed he was since he's been on the list for a while.
The patch is simple, though not 15-line trivial and i think we can
split the line difference between me and him.  But I don't think that's
the way it works... is it?

Should I just revert?  Or should we wait for Gregory to get a copyright
assignment?  Last time I asked for someone else it took less than a
week.

João

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

Re: [PATCH] icomplete-vertical

[Gregory Heytings]

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


> 
> Uff, but I shouldn't have.  I just realized that Gregory is not on the 
> copyright file!
>

I should be (or I will be soon), I did my paperwork a week ago or so.

Re: [PATCH] icomplete-vertical

[João Távora]

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

On Sun, Apr 11, 2021 at 12:00 AM Gregory Heytings <gregory@heytings.org>
wrote:

> Uff, but I shouldn't have.  I just realized that Gregory is not on the
> > copyright file!
> >
>
> I should be (or I will be soon), I did my paperwork a week ago or so.


Hmm, then maybe it's OK.  Let's see what Eli says.

João

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

Re: [PATCH] icomplete-vertical

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Sat, 10 Apr 2021 23:56:50 +0100
> Cc: Gregory Heytings <gregory@heytings.org>, Philip Kaludercic <philipk@posteo.net>, 
> 	Eli Zaretskii <eliz@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca>, 
> 	emacs-devel <emacs-devel@gnu.org>
> 
> Uff, but I shouldn't have.    I just realized that Gregory is not on the
> copyright file!
> 
> Eli, sorry, I assumed he was since he's been on the list for a while.  
> The patch is simple, though not 15-line trivial and i think we can 
> split the line difference between me and him.  But I don't think that's 
> the way it works... is it?
> 
> Should I just revert?  Or should we wait for Gregory to get a copyright 
> assignment?  Last time I asked for someone else it took less than a 
> week.

Gregory should start legal paperwork ASAP, and mention there all the
files he already modified.

Thanks.

Re: [PATCH] icomplete-vertical

[Eli Zaretskii]

> Date: Sat, 10 Apr 2021 23:00:29 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Juri Linkov <juri@linkov.net>, Philip Kaludercic <philipk@posteo.net>, 
>     Eli Zaretskii <eliz@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca>, 
>     emacs-devel <emacs-devel@gnu.org>
> 
> > Uff, but I shouldn't have.  I just realized that Gregory is not on the 
> > copyright file!
> >
> 
> I should be

Not yet.

> (or I will be soon), I did my paperwork a week ago or so.

Good to hear, thanks.

Re: [PATCH] icomplete-vertical

[João Távora]

Gregory,

It's been a month and your assignment is not yet complete.
Am I grepping for the wrong name ("Gregory Heytings")?

Thanks
João

On Sun, Apr 11, 2021 at 8:14 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > Date: Sat, 10 Apr 2021 23:00:29 +0000
> > From: Gregory Heytings <gregory@heytings.org>
> > cc: Juri Linkov <juri@linkov.net>, Philip Kaludercic <philipk@posteo.net>,
> >     Eli Zaretskii <eliz@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca>,
> >     emacs-devel <emacs-devel@gnu.org>
> >
> > > Uff, but I shouldn't have.  I just realized that Gregory is not on the
> > > copyright file!
> > >
> >
> > I should be
>
> Not yet.
>
> > (or I will be soon), I did my paperwork a week ago or so.
>
> Good to hear, thanks.



-- 
João Távora