[PATCH] `completion-all-sorted-completions`: Add support for completion boundaries

[PATCH] `completion-all-sorted-completions`: Add support for completion boundaries

[Daniel Mendler]

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

I attached a patch which adds support for completion boundaries to the 
sorting by history position. Tests are included.

Daniel

[-- Attachment #2: 0001-completion-all-sorted-completions-Add-completion-bou.patch --]
[-- Type: text/x-diff, Size: 6287 bytes --]

From 73be13a758ef6a43c5b8ac52de2b2c0205e2f21c Mon Sep 17 00:00:00 2001
From: Daniel Mendler <mail@daniel-mendler.de>
Date: Tue, 20 Apr 2021 00:01:44 +0200
Subject: [PATCH] (completion-all-sorted-completions): Add completion boundary
 support

lisp/minibuffer.el (completion-all-sorted-completions): The history is
preprocessed by the function `minibuffer--sort-preprocess-history`.
The default value is prepended to the history.  The completion base
prefix string is removed from the history elements.  This allows
sorting by history position for completion tables which use completion
boundaries, in particular the file completion table.
test/lisp/minibuffer-tests.el (completion-all-sorted-completions): Add
tests for various combinations of with/without history/base/default.
---
 lisp/minibuffer.el            | 36 +++++++++++++++++++--------
 test/lisp/minibuffer-tests.el | 46 +++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 10 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 51e0519d48..24f7326a99 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1381,6 +1381,26 @@ minibuffer--sort-by-length-alpha
                     (and (= (length c1) (length c2))
                          (string< c1 c2))))))
 
+(defun minibuffer--sort-preprocess-history (base)
+  "Preprocess history.
+Remove completion BASE prefix string from history elements."
+  (let* ((def (if (stringp minibuffer-default)
+                  minibuffer-default
+                (car-safe minibuffer-default)))
+         (hist (and (not (eq minibuffer-history-variable t))
+                    (symbol-value minibuffer-history-variable)))
+         (base-size (length base)))
+    ;; Drop base string from the history elements
+    (when (/= base-size 0)
+      (setq hist (delq nil (mapcar
+                            (lambda (c)
+                              (when (string-prefix-p base c)
+                                (substring c base-size)))
+                            hist))))
+    ;; Default comes first
+    (setq hist (if def (cons def hist) hist))
+    hist))
+
 (defun completion-all-sorted-completions (&optional start end)
   (or completion-all-sorted-completions
       (let* ((start (or start (minibuffer-prompt-end)))
@@ -1410,21 +1430,17 @@ completion-all-sorted-completions
           (setq all (delete-dups all))
           (setq last (last all))
 
-          (cond
-           (sort-fun
-            (setq all (funcall sort-fun all)))
-           (t
+          (if sort-fun
+              (setq all (funcall sort-fun all))
             ;; Sort first by length and alphabetically.
             (setq all (minibuffer--sort-by-length-alpha all))
-
             ;; Sort by history position, put the default, if it
             ;; exists, on top.
-            (when (and (minibufferp) (not (eq minibuffer-history-variable t)))
-              (let ((def (car-safe minibuffer-default))
-                    (hist (symbol-value minibuffer-history-variable)))
+            (when (minibufferp)
               (setq all (minibuffer--sort-by-position
-                         (if def (cons def hist) hist)
-                         all))))))
+                         (minibuffer--sort-preprocess-history
+                          (substring string 0 base-size))
+                         all))))
 
           ;; Cache the result.  This is not just for speed, but also so that
           ;; repeated calls to minibuffer-force-complete can cycle through
diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el
index 027711c21e..03520c2b74 100644
--- a/test/lisp/minibuffer-tests.el
+++ b/test/lisp/minibuffer-tests.el
@@ -136,5 +136,51 @@ completion-pcm--optimize-pattern
   (should (equal (completion-pcm--optimize-pattern '(any "" any))
                  '(any))))
 
+(defun test-completion-all-sorted-completions (base def history-var history-list)
+  (with-temp-buffer
+    (insert base)
+    (cl-letf (((symbol-function #'minibufferp) (lambda (&rest _) t)))
+      (let ((completion-styles '(basic))
+            (completion-category-defaults nil)
+            (completion-category-overrides nil)
+            (minibuffer-history-variable history-var)
+            (minibuffer-history history-list)
+            (minibuffer-default def)
+            (minibuffer-completion-table
+             (lambda (str pred action)
+               (pcase action
+                 (`(boundaries . ,_) `(boundaries ,(length base) . 0))
+                 (_ (complete-with-action action
+                                          '(epsilon alpha gamma beta delta)
+                                          (substring str (length base)) pred))))))
+        (completion-all-sorted-completions)))))
+
+(ert-deftest completion-all-sorted-completions ()
+  ;; No base, disabled history, no default
+  (should (equal (test-completion-all-sorted-completions
+                  "" nil t nil)
+                 `("beta" "alpha" "delta" "gamma" "epsilon" . 0)))
+  ;; No base, disabled history, default string
+  (should (equal (test-completion-all-sorted-completions
+                  "" "gamma" t nil)
+                 `("gamma" "beta" "alpha" "delta" "epsilon" . 0)))
+  ;; No base, empty history, default string
+  (should (equal (test-completion-all-sorted-completions
+                  "" "gamma" 'minibuffer-history nil)
+                 `("gamma" "beta" "alpha" "delta" "epsilon" . 0)))
+  ;; No base, empty history, default list
+  (should (equal (test-completion-all-sorted-completions
+                  "" '("gamma" "zeta") 'minibuffer-history nil)
+                 `("gamma" "beta" "alpha" "delta" "epsilon" . 0)))
+  ;; No base, history, default string
+  (should (equal (test-completion-all-sorted-completions
+                  "" "gamma" 'minibuffer-history '("other" "epsilon" "delta"))
+                 `("gamma" "epsilon" "delta" "beta" "alpha"  . 0)))
+  ;; Base, history, default string
+  (should (equal (test-completion-all-sorted-completions
+                  "base/" "gamma" 'minibuffer-history
+                  '("some/alpha" "base/epsilon" "base/delta"))
+                 `("gamma" "epsilon" "delta" "beta" "alpha"  . 5))))
+
 (provide 'minibuffer-tests)
 ;;; minibuffer-tests.el ends here
-- 
2.20.1

Re: [PATCH] `completion-all-sorted-completions`: Add support for completion boundaries

[Stefan Monnier]

> I attached a patch which adds support for completion boundaries to the
> sorting by history position. Tests are included.

Thanks, pushed.

> lisp/minibuffer.el (completion-all-sorted-completions): The history is
> preprocessed by the function `minibuffer--sort-preprocess-history`.
> The default value is prepended to the history.  The completion base
> prefix string is removed from the history elements.  This allows
> sorting by history position for completion tables which use completion
> boundaries, in particular the file completion table.
> test/lisp/minibuffer-tests.el (completion-all-sorted-completions): Add
> tests for various combinations of with/without history/base/default.

I massaged this to use the active voice and stick a bit closer to
our conventions.

> +(defun minibuffer--sort-preprocess-history (base)
> +  "Preprocess history.
> +Remove completion BASE prefix string from history elements."
> +  (let* ((def (if (stringp minibuffer-default)
> +                  minibuffer-default
> +                (car-safe minibuffer-default)))
> +         (hist (and (not (eq minibuffer-history-variable t))
> +                    (symbol-value minibuffer-history-variable)))
> +         (base-size (length base)))
> +    ;; Drop base string from the history elements
> +    (when (/= base-size 0)
> +      (setq hist (delq nil (mapcar
> +                            (lambda (c)
> +                              (when (string-prefix-p base c)
> +                                (substring c base-size)))
> +                            hist))))
> +    ;; Default comes first
> +    (setq hist (if def (cons def hist) hist))
> +    hist))

I massaged this so that `def` is also filtered through `base`
(and also so that comments are properly punctuated).

> +                 (`(boundaries . ,_) `(boundaries ,(length base) . 0))
> +                 (_ (complete-with-action action
> +                                          '(epsilon alpha gamma beta delta)

And I changed this completion table to a list of strings, since lists of
symbols happen to work most of the time but aren't officially supported
(we support alists whose keys are symbols, OTOH).


        Stefan

Re: [PATCH] `completion-all-sorted-completions`: Add support for completion boundaries

[Daniel Mendler]

On 4/25/21 12:19 AM, Stefan Monnier wrote:
>> lisp/minibuffer.el (completion-all-sorted-completions): The history is
>> preprocessed by the function `minibuffer--sort-preprocess-history`.
>> The default value is prepended to the history.  The completion base
>> prefix string is removed from the history elements.  This allows
>> sorting by history position for completion tables which use completion
>> boundaries, in particular the file completion table.
>> test/lisp/minibuffer-tests.el (completion-all-sorted-completions): Add
>> tests for various combinations of with/without history/base/default.
> 
> I massaged this to use the active voice and stick a bit closer to
> our conventions.

Thanks, I am sorry that you had to do the commit message massage again. 
I will write a better message next time.

> I massaged this so that `def` is also filtered through `base`
> (and also so that comments are properly punctuated).

Right, it is correct to remove the base from the default.

> And I changed this completion table to a list of strings, since lists of
> symbols happen to work most of the time but aren't officially supported
> (we support alists whose keys are symbols, OTOH).

Okay, good to know. But if symbol-keyed alists are supported, it makes 
sense to officially allow lists of symbols too? I've never seen breakage 
with symbol lists with various completion systems. I think I am using 
them somewhere.

Daniel