bug#74420: 31.0.50; PCM completion for ~/src/emacs/trunk/*/minibuf breaks

bug#74420: 31.0.50; PCM completion for ~/src/emacs/trunk/*/minibuf breaks

[Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Assuming one has an Emacs repo at ~/src/emacs/trunk,

1. emacs -Q
2. C-x C-f C-a C-k ~/src/emacs/trunk/*/minibuf TAB
3. The minibuffer contents change to
   ~/src/emacs/trunk//minibuf
4. Since there's a //, subsequent file name completion tries to complete
   on the root directory, which is not what should happen.

A more concise reproduction:

(completion-pcm-try-completion
 "*/minibuf" '("lisp/minibufA" "src/minibufB") nil (length "*/minibuf"))

evalutes to

("/minibuf" . 8)

We should be preserving the *.  A patch to fix this will follow.


In GNU Emacs 31.0.50 (build 18, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.15.12, Xaw scroll bars) of 2024-11-18 built on
 igm-qws-u22796a
Repository revision: 6610135a50ac2fd0683a5e467fe33faa81df21b8
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Rocky Linux 8.10 (Green Obsidian)

Configured using:
 'configure -C --with-gif=no --with-x-toolkit=lucid
 --with-native-compilation=yes --prefix=/home/sbaugh/prefix-jane-emacs'

Configured features:
CAIRO DBUS FREETYPE GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG LIBSELINUX
LIBSYSTEMD LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG
SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM
XINPUT2 XPM LUCID ZLIB

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
  show-paren-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
  minibuffer-regexp-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr compile comint ansi-osc ansi-color ring comp-run
bytecomp byte-compile comp-common rx emacsbug message mailcap yank-media
puny dired dired-loaddefs rfc822 mml mml-sec password-cache epa derived
epg rfc6068 epg-config gnus-util text-property-search time-date subr-x
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 rmc iso-transl tooltip cconv eldoc paren
electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/x-win x-win term/common-win x-dnd touch-screen tool-bar dnd fontset
image regexp-opt fringe tabulated-list replace newcomment text-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine 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 emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify
dynamic-setting system-font-setting font-render-setting cairo x-toolkit
xinput2 x multi-tty move-toolbar make-network-process native-compile
emacs)

Memory information:
((conses 16 107684 9391) (symbols 48 11073 0) (strings 32 32202 1340)
 (string-bytes 1 982590) (vectors 16 14035)
 (vector-slots 8 173934 7491) (floats 8 29 1) (intervals 56 284 0)
 (buffers 984 11))

bug#74420: 31.0.50; PCM completion for ~/src/emacs/trunk/*/minibuf breaks

[Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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


Patch to fix this:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Preserve-an-explicit-in-pcm-try-completion.patch --]
[-- Type: text/x-patch, Size: 5345 bytes --]

From d7377eb6abfc57552f43687aec358934b33707e6 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Mon, 18 Nov 2024 12:26:55 -0500
Subject: [PATCH] Preserve an explicit * in pcm-try-completion

An explicitly typed * has different semantics from automatically
inserted PCM wildcards, so it should be preserved on
try-completion.  We already do this in some cases, but now we do
it more.  Concretely, we do it by optimizing the PCM pattern
more aggressively to avoid having multiple wildcards in a row:
after those are removed, the existing code in
completion-pcm--merge-completions is able to preserve * in more
cases.  The additional optimization should also improve
performance.

This is especially significant for filename completion: removing
an explicit * can take us from

~/src/emacs/trunk/*/minibuf

to

~/src/emacs/trunk//minibuf

The explicit double slash is interpreted by the file name
completion table to mean "start completing from the root
directory", so deleting the * here substantially changes
semantics.

* lisp/minibuffer.el (completion-pcm--optimize-pattern): Add
more optimizations. (bug#74420)
(completion-pcm--find-all-completions): Optimize the pattern
after concatenating two subpatterns.
* test/lisp/minibuffer-tests.el (completion-pcm--optimize-pattern)
(completion-pcm-test-7): Add tests.
---
 lisp/minibuffer.el            | 20 ++++++++++++++++----
 test/lisp/minibuffer-tests.el | 30 +++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 5f3f5d3ead1..e48d85b777d 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -4073,8 +4073,18 @@ completion-pcm--optimize-pattern
   (let ((n '()))
     (while p
       (pcase p
-        (`(,(or 'any 'any-delim) ,(or 'any 'point) . ,_)
-         (setq p (cdr p)))
+        ;; Remove duplicate `any' and `prefix'
+        (`(any any . ,rest)
+         (setq p (cons 'any rest)))
+        (`(prefix prefix . ,rest)
+         (setq p (cons 'prefix rest)))
+        ;; `any' matches anything `any-delim' does, and grows the same way.
+        (`(any-delim any . ,rest)
+         (setq p (cons 'any rest)))
+        ;; Remove other wildcards found around `star' or `point'.
+        ((or `(,(and keep (or 'star 'point)) ,(or 'any 'any-delim 'prefix) . ,rest)
+             `(,(or 'any 'any-delim 'prefix) ,(and keep (or 'star 'point)) . ,rest))
+         (setq p (cons keep rest)))
         ;; This is not just a performance improvement: it turns a
         ;; terminating `point' into an implicit `any', which affects
         ;; the final position of point (because `point' gets turned
@@ -4445,8 +4455,10 @@ completion-pcm--find-all-completions
               ;;   (dolist (submatch suball)
               ;;     (push (concat submatch between newsubstring) all)))
               ))
-          (setq pattern (append subpat (list 'any (string sep))
-                                (if between (list between)) pattern))
+          (setq pattern
+                (completion-pcm--optimize-pattern
+                 (append subpat (list 'any (string sep))
+                         (if between (list between)) pattern)))
           (setq prefix subprefix)))
       (if (and (null all) firsterror)
           (signal (car firsterror) (cdr firsterror))
diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el
index 38c2b8c4552..d988a2007cb 100644
--- a/test/lisp/minibuffer-tests.el
+++ b/test/lisp/minibuffer-tests.el
@@ -133,7 +133,19 @@ completion-pcm--optimize-pattern
   (should (equal (completion-pcm--optimize-pattern '("buf" point "f"))
                  '("buf" point "f")))
   (should (equal (completion-pcm--optimize-pattern '(any "" any))
-                 '(any))))
+                 '(any)))
+  (should (equal (completion-pcm--optimize-pattern '(any-delim "" any))
+                 '(any)))
+  (should (equal (completion-pcm--optimize-pattern '(prefix "" prefix))
+                 '(prefix)))
+  (should (equal (completion-pcm--optimize-pattern '(prefix star any))
+                 '(star)))
+  (should (equal (completion-pcm--optimize-pattern '(any point prefix "foo"))
+                 '(point "foo")))
+  ;; The `any' and `prefix' are erased because they're next to `point',
+  ;; then `point' is erased because it's at the end.
+  (should (equal (completion-pcm--optimize-pattern '(any point prefix))
+                 '())))
 
 (defun test-completion-all-sorted-completions (base def history-var history-list)
   (with-temp-buffer
@@ -258,6 +270,22 @@ completion-pcm-test-6
            (car (completion-pcm-all-completions
                  "li-pac*" '("do-not-list-packages") nil 7)))))
 
+(ert-deftest completion-pcm-test-7 ()
+  ;; Wildcards are preserved even when right before a delimiter.
+  (should (equal
+           (completion-pcm-try-completion
+            "x*/"
+            '("x1/y1" "x2/y2")
+            nil 3)
+           '("x*/y" . 4)))
+  ;; This is important if the wildcard is at the start of a component.
+  (should (equal
+           (completion-pcm-try-completion
+            "*/minibuf"
+            '("lisp/minibuffer.el" "src/minibuf.c")
+            nil 9)
+           ("*/minibuf" . 9))))
+
 (ert-deftest completion-substring-test-1 ()
   ;; One third of a match!
   (should (equal
-- 
2.39.3

bug#74420: 31.0.50; PCM completion for ~/src/emacs/trunk/*/minibuf breaks

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> An explicitly typed * has different semantics from automatically
> inserted PCM wildcards, so it should be preserved on
> try-completion.  We already do this in some cases, but now we do
> it more.  Concretely, we do it by optimizing the PCM pattern
> more aggressively to avoid having multiple wildcards in a row:
> after those are removed, the existing code in
> completion-pcm--merge-completions is able to preserve * in more
> cases.

Oh, indeed, thank you.

The patch looks good to me, so if there are no objection, I'll install
it in a few days (feel free to ping me if I forget).
Banter follows.

> * lisp/minibuffer.el (completion-pcm--optimize-pattern): Add
> more optimizations. (bug#74420)

Aha, so that's why you didn't submit the patch together with the initial
bug report!

> -        (`(,(or 'any 'any-delim) ,(or 'any 'point) . ,_)
> -         (setq p (cdr p)))
> +        ;; Remove duplicate `any' and `prefix'
> +        (`(any any . ,rest)
> +         (setq p (cons 'any rest)))
> +        (`(prefix prefix . ,rest)
> +         (setq p (cons 'prefix rest)))
> +        ;; `any' matches anything `any-delim' does, and grows the same way.
> +        (`(any-delim any . ,rest)
> +         (setq p (cons 'any rest)))

AFAICT you can still merge the `any-delim any` and `any any` cases.

> +        ;; Remove other wildcards found around `star' or `point'.
> +        ((or `(,(and keep (or 'star 'point)) ,(or 'any 'any-delim 'prefix) . ,rest)
> +             `(,(or 'any 'any-delim 'prefix) ,(and keep (or 'star 'point)) . ,rest))
> +         (setq p (cons keep rest)))

BTW, maybe we should go with something like

       (`(,(and (pred symbolp) s1) ,(and (pred symbolp) s2) . ,rest)
        (cond ((completion-pcm--<something>-p s1 s2) (setq p (cons s1 rest)))
              ((completion-pcm--<something>-p s2 s1) (setq p (cons s2 rest)))
              (t (push (pop p) n))))

Where `completion-pcm--<something>-p` is some kind of partial ordering?


        Stefan

bug#74420: 31.0.50; PCM completion for ~/src/emacs/trunk/*/minibuf breaks

[Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> An explicitly typed * has different semantics from automatically
>> inserted PCM wildcards, so it should be preserved on
>> try-completion.  We already do this in some cases, but now we do
>> it more.  Concretely, we do it by optimizing the PCM pattern
>> more aggressively to avoid having multiple wildcards in a row:
>> after those are removed, the existing code in
>> completion-pcm--merge-completions is able to preserve * in more
>> cases.
>
> Oh, indeed, thank you.
>
> The patch looks good to me, so if there are no objection, I'll install
> it in a few days (feel free to ping me if I forget).

Actually... I just realized this misses some cases, namely when we have
"star point" or "point star".  Attached at the end is a different patch
which takes a different approach (and which includes new tests for the
problematic cases).

The changes to optimize still seem reasonable to me, since they might
improve performance, so feel free to install it if you think it's good.
Although maybe we want to revise it with the below discussion.

> Banter follows.
>
>> * lisp/minibuffer.el (completion-pcm--optimize-pattern): Add
>> more optimizations. (bug#74420)
>
> Aha, so that's why you didn't submit the patch together with the initial
> bug report!
>
>> -        (`(,(or 'any 'any-delim) ,(or 'any 'point) . ,_)
>> -         (setq p (cdr p)))
>> +        ;; Remove duplicate `any' and `prefix'
>> +        (`(any any . ,rest)
>> +         (setq p (cons 'any rest)))
>> +        (`(prefix prefix . ,rest)
>> +         (setq p (cons 'prefix rest)))
>> +        ;; `any' matches anything `any-delim' does, and grows the same way.
>> +        (`(any-delim any . ,rest)
>> +         (setq p (cons 'any rest)))
>
> AFAICT you can still merge the `any-delim any` and `any any` cases.

True, I just thought this was a bit easier to read, since the cases are
justified as an optimization in slightly different ways.

>> +        ;; Remove other wildcards found around `star' or `point'.
>> +        ((or `(,(and keep (or 'star 'point)) ,(or 'any 'any-delim 'prefix) . ,rest)
>> +             `(,(or 'any 'any-delim 'prefix) ,(and keep (or 'star 'point)) . ,rest))
>> +         (setq p (cons keep rest)))
>
> BTW, maybe we should go with something like
>
>        (`(,(and (pred symbolp) s1) ,(and (pred symbolp) s2) . ,rest)
>         (cond ((completion-pcm--<something>-p s1 s2) (setq p (cons s1 rest)))
>               ((completion-pcm--<something>-p s2 s1) (setq p (cons s2 rest)))
>               (t (push (pop p) n))))
>
> Where `completion-pcm--<something>-p` is some kind of partial ordering?

Interesting thought.  Maybe it would make sense to have something like
- completion-pcm--wildcard-grows-on-left-p
  (non-nil for star, point, any, any-delim)
- completion-pcm--wildcard-grows-on-right-p
  (non-nil for star, point, prefix)
- completion-pcm--wildcard-in-text-p
  (non-nil for star and point)

Then this case would be something like "if grows-left/right-p is the
same, and at most one of the symbols is in-text-p, delete the
non-in-text-p one".

Also, then the first two helpers could be used in
completion-pcm--merge-completions as well, which could make the wildcard
behavior easier to understand.  (I want to do a bit of refactoring in
completion-pcm--merge-completions, especially after my attached change;
I think it could all be a bit easier to understand.)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Preserve-an-explicit-in-pcm-try-completion.patch --]
[-- Type: text/x-patch, Size: 5158 bytes --]

From 8c2ff1152800ede05cb9a9fd80deac45646ce52f Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Mon, 18 Nov 2024 12:26:55 -0500
Subject: [PATCH] Preserve an explicit * in pcm-try-completion

An explicitly typed * has different semantics from automatically
inserted PCM wildcards, so it should be preserved on try-completion.  We
already do this in some cases, but now we do it more.

This is especially significant for filename completion: removing an
explicit * can take us from

~/src/emacs/trunk/*/minibuf

to

~/src/emacs/trunk//minibuf

The explicit double slash is interpreted by the file name completion
table to mean "start completing from the root directory", so deleting
the * here substantially changes semantics.

* lisp/minibuffer.el (completion-pcm--merge-completions): Don't drop
important wildcards. (bug#74420)
* test/lisp/minibuffer-tests.el (completion-pcm-test-7): Add tests.
---
 lisp/minibuffer.el            | 21 +++++++++++++++------
 test/lisp/minibuffer-tests.el | 25 +++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 5f3f5d3ead1..2d27fef44ab 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -4513,12 +4513,17 @@ completion-pcm--merge-completions
       ;; Then for each of those non-constant elements, extract the
       ;; commonality between them.
       (let ((res ())
-            (fixed ""))
+            (fixed "")
+            ;; Accumulate each stretch of wildcards, and process them as a unit.
+            (wildcards ()))
         ;; Make the implicit trailing `any' explicit.
         (dolist (elem (append pattern '(any)))
           (if (stringp elem)
-              (setq fixed (concat fixed elem))
+              (progn
+                (setq fixed (concat fixed elem))
+                (setq wildcards nil))
             (let ((comps ()))
+              (push elem wildcards)
               (dolist (cc (prog1 ccs (setq ccs nil)))
                 (push (car cc) comps)
                 (push (cdr cc) ccs))
@@ -4542,14 +4547,16 @@ completion-pcm--merge-completions
                     (push prefix res)
                   ;; `prefix' only wants to include the fixed part before the
                   ;; wildcard, not the result of growing that fixed part.
-                  (when (eq elem 'prefix)
+                  (when (seq-some (lambda (elem) (eq elem 'prefix)) wildcards)
                     (setq prefix fixed))
                   (push prefix res)
-                  (push elem res)
+                  ;; Push all the wildcards in this stretch, to preserve `point' and
+                  ;; `star' wildcards before ELEM.
+                  (setq res (nconc wildcards res))
                   ;; Extract common suffix additionally to common prefix.
                   ;; Don't do it for `any' since it could lead to a merged
                   ;; completion that doesn't itself match the candidates.
-                  (when (and (memq elem '(star point prefix))
+                  (when (and (seq-some (lambda (elem) (memq elem '(star point prefix))) wildcards)
                              ;; If prefix is one of the completions, there's no
                              ;; suffix left to find.
                              (not (assoc-string prefix comps t)))
@@ -4563,7 +4570,9 @@ completion-pcm--merge-completions
                                         comps))))))
                       (cl-assert (stringp suffix))
                       (unless (equal suffix "")
-                        (push suffix res)))))
+                        (push suffix res))))
+                  ;; We pushed these wildcards on RES, so we're done with them.
+                  (setq wildcards nil))
                 (setq fixed "")))))
         ;; We return it in reverse order.
         res)))))
diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el
index 38c2b8c4552..c166450d87a 100644
--- a/test/lisp/minibuffer-tests.el
+++ b/test/lisp/minibuffer-tests.el
@@ -258,6 +258,31 @@ completion-pcm-test-6
            (car (completion-pcm-all-completions
                  "li-pac*" '("do-not-list-packages") nil 7)))))
 
+(ert-deftest completion-pcm-test-7 ()
+  ;; Wildcards are preserved even when right before a delimiter.
+  (should (equal
+           (completion-pcm-try-completion
+            "x*/"
+            '("x1/y1" "x2/y2")
+            nil 3)
+           '("x*/y" . 4)))
+  ;; Or around point.
+  (should (equal
+           (completion-pcm--merge-try
+            '(point star "foo") '("xxfoo" "xyfoo") "" "")
+           '("x*foo" . 1)))
+  (should (equal
+           (completion-pcm--merge-try
+            '(star point "foo") '("xxfoo" "xyfoo") "" "")
+           '("x*foo" . 2)))
+  ;; This is important if the wildcard is at the start of a component.
+  (should (equal
+           (completion-pcm-try-completion
+            "*/minibuf"
+            '("lisp/minibuffer.el" "src/minibuf.c")
+            nil 9)
+           '("*/minibuf" . 9))))
+
 (ert-deftest completion-substring-test-1 ()
   ;; One third of a match!
   (should (equal
-- 
2.39.3

bug#74420: 31.0.50; PCM completion for ~/src/emacs/trunk/*/minibuf breaks

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Actually... I just realized this misses some cases, namely when we have
> "star point" or "point star".

FWIW, my local patches have included for years optimizations like the
ones you suggested *and* they replaced `star point` and `point star`
with just `star`.

Do you think it's important to preserve `point` in those cases?

>> BTW, maybe we should go with something like
>>
>>        (`(,(and (pred symbolp) s1) ,(and (pred symbolp) s2) . ,rest)
>>         (cond ((completion-pcm--<something>-p s1 s2) (setq p (cons s1 rest)))
>>               ((completion-pcm--<something>-p s2 s1) (setq p (cons s2 rest)))
>>               (t (push (pop p) n))))
>>
>> Where `completion-pcm--<something>-p` is some kind of partial ordering?
>
> Interesting thought.  Maybe it would make sense to have something like
> - completion-pcm--wildcard-grows-on-left-p
>   (non-nil for star, point, any, any-delim)
> - completion-pcm--wildcard-grows-on-right-p
>   (non-nil for star, point, prefix)
> - completion-pcm--wildcard-in-text-p
>   (non-nil for star and point)
> Then this case would be something like "if grows-left/right-p is the
> same, and at most one of the symbols is in-text-p, delete the
> non-in-text-p one".

But I guess your `completion-pcm--merge-completions` is still needed as
long as we can't optimize all sequences of symbols to a single symbol.
🙁


        Stefan

bug#74420: 31.0.50; PCM completion for ~/src/emacs/trunk/*/minibuf breaks

[Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Actually... I just realized this misses some cases, namely when we have
>> "star point" or "point star".
>
> FWIW, my local patches have included for years optimizations like the
> ones you suggested *and* they replaced `star point` and `point star`
> with just `star`.
>
> Do you think it's important to preserve `point` in those cases?

Hm, I'm not sure.  I've been playing around with alternative ways to
move point in pcm-try-completion but haven't yet got something I'm
satisfied with.  So let me defer this question until I've hacked on that
some more :)

>>> BTW, maybe we should go with something like
>>>
>>>        (`(,(and (pred symbolp) s1) ,(and (pred symbolp) s2) . ,rest)
>>>         (cond ((completion-pcm--<something>-p s1 s2) (setq p (cons s1 rest)))
>>>               ((completion-pcm--<something>-p s2 s1) (setq p (cons s2 rest)))
>>>               (t (push (pop p) n))))
>>>
>>> Where `completion-pcm--<something>-p` is some kind of partial ordering?
>>
>> Interesting thought.  Maybe it would make sense to have something like
>> - completion-pcm--wildcard-grows-on-left-p
>>   (non-nil for star, point, any, any-delim)
>> - completion-pcm--wildcard-grows-on-right-p
>>   (non-nil for star, point, prefix)
>> - completion-pcm--wildcard-in-text-p
>>   (non-nil for star and point)
>> Then this case would be something like "if grows-left/right-p is the
>> same, and at most one of the symbols is in-text-p, delete the
>> non-in-text-p one".
>
> But I guess your `completion-pcm--merge-completions` is still needed as
> long as we can't optimize all sequences of symbols to a single symbol.
> 🙁

Yep.  And here's one case that I think makes optimizing all sequences
down to a single symbol impossible: (star star star) is reified as ***,
not shrunk down to a single *.  I think that behavior is probably
reasonable (or at least I'm not in any rush to get rid of it), but to
preserve it we have to preserve sequences of multiple symbols.

bug#74420: 31.0.50; PCM completion for ~/src/emacs/trunk/*/minibuf breaks

[Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Spencer Baugh <sbaugh@janestreet.com> writes:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> Actually... I just realized this misses some cases, namely when we have
>>> "star point" or "point star".
>>
>> FWIW, my local patches have included for years optimizations like the
>> ones you suggested *and* they replaced `star point` and `point star`
>> with just `star`.
>>
>> Do you think it's important to preserve `point` in those cases?
>
> Hm, I'm not sure.  I've been playing around with alternative ways to
> move point in pcm-try-completion but haven't yet got something I'm
> satisfied with.  So let me defer this question until I've hacked on that
> some more :)

Okay, so I ended up not wanting to move point.

What I have instead now is something which explicitly inserts a * in the
minibuffer, specifically with completion-pcm-leading-wildcard:

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 01502235403..db13c659004 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3925,7 +3925,7 @@ completion-pcm--string->pattern
               (setq p0 p)
             (push (substring string p (match-end 0)) pattern)
             ;; `any-delim' is used so that "a-b" also finds "array->beginning".
-            (setq pending 'any-delim)
+            (setq pending (if completion-pcm-leading-wildcard 'star 'any-delim))
             (setq p0 (match-end 0))))
         (setq p p0))
 
@@ -3935,7 +3935,7 @@ completion-pcm--string->pattern
       (setq pattern (nreverse pattern))
       (when completion-pcm-leading-wildcard
         (when (stringp (car pattern))
-          (push 'prefix pattern)))
+          (push 'star pattern)))
       pattern)))
 
 (defun completion-pcm--optimize-pattern (p)
-- 
2.39.3


This does two things:

- completion-pcm-leading-wildcard previously had inconsistent behavior:
  if completion-pcm--find-all-completions makes a recursive call,
  completion-pcm-leading-wildcard would effectively add a leading
  wildcard to each component.  But that wouldn't happen if the original
  pattern happens to match, in the first completion-pcm--all-completions
  call.  To make them behave identically, now
  completion-pcm-leading-wildcard changes the 'any-delim inserted at the
  start of each component into a proper wildcard.  This is just a
  bugfix.

- More radically, actually insert a * in the minibuffer text if we run
  completion-pcm-try-completion with completion-pcm-leading-wildcard=t.

Inserting a * is pretty nice for multiple reasons.

Paradoxically, inserting a * makes things more efficient for the
configuration I intend people to use, where completion-styles contains:

... partial-completion (partial-completion ((completion-pcm-leading-wildcard t))) ...

With this configuration, if partial-completion fails, we make one call
to the expensive completion-pcm-leading-wildcard=t mode, and then
afterwards the explicit *s cause us to stay in regular
partial-completion, with only the exact amount of leading wildcards that
we actually need, rather than having an extra 'prefix wildcard at the
start of every single component.

And, inserting the * ensures that we stay in this "leading wildcard"
mode.  I've found that tab-completing with
completion-pcm-leading-wildcard tends to sometimes change the minibuffer
contents such that subsequent tab-completions get handled by another
completion style.  With leading wildcards, that's pretty undesirable,
because it can make the set of completions much narrower.  But
explicitly inserting the * forces the leading-wildcard behavior to stay
around.

And it just looks pretty good: because the 'star wildcards are removed
if they can't expand to anything, when you hit TAB the * are basically
inserted everywhere that you might add new text.  This is pretty neat.

I usually use completion-pcm-leading-wildcard with project-find-file,
so e.g. this is a decent way to test:

(setf (alist-get 'project-file completion-category-overrides)
      '((styles basic partial-completion (partial-completion ((completion-pcm-leading-wildcard t))))))

Then C-x p f in the Emacs repo

What do you think?