bug#72819: [PATCH] Correctly include fixed strings before a prefix wildcard in PCM

[Spencer Baugh <sbaugh@janestreet.com>]

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

Tags: patch


In 63a48252306a631dc07d62d19311433c7877bd27 I fixed a bug with
the PCM implementation of substring completion, relating to the
handling of PCM wildcards.

However, this fix was incomplete.  This change completes the fix by
also including a fixed string if it appears before a `prefix'
wildcard, even if try-completion doesn't discover that fixed string
grows to a unique completion.

I discovered this bug while working on enhancements to PCM completion
related to completion-pcm-leading-wildcard.

In GNU Emacs 29.2.50 (build 12, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.15.12, Xaw scroll bars) of 2024-08-23 built on
 igm-qws-u22796a
Repository revision: 253525ba4598f487e451b93a4ab1680ef212a62d
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Rocky Linux 8.10 (Green Obsidian)

Configured using:
 'configure --with-x-toolkit=lucid --without-gpm --without-gconf
 --without-selinux --without-imagemagick --with-modules --with-gif=no
 --with-cairo --with-rsvg --without-compress-install
 --with-native-compilation=aot --with-tree-sitter
 PKG_CONFIG_PATH=/usr/local/home/garnish/libtree-sitter/0.22.6-1/lib/pkgconfig/'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Correctly-include-fixed-strings-before-a-prefix-wild.patch --]
[-- Type: text/patch, Size: 4228 bytes --]

From 6d7a8f5e491e4ba4eb6e0026278b4df3d70a4a83 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Mon, 26 Aug 2024 10:12:51 -0400
Subject: [PATCH] Correctly include fixed strings before a prefix wildcard in
 PCM

In 63a48252306a631dc07d62d19311433c7877bd27 I fixed a bug with
the PCM implementation of substring completion, relating to the
handling of PCM wildcards.

However, this fix was incomplete.  This change completes the fix by
also including a fixed string if it appears before a `prefix'
wildcard, even if try-completion doesn't discover that fixed string
grows to a unique completion.

I discovered this bug while working on enhancements to PCM completion
related to completion-pcm-leading-wildcard.

* lisp/minibuffer.el (completion-pcm--merge-completions):
* test/lisp/minibuffer-tests.el (completion-substring-test-5):
---
 lisp/minibuffer.el            | 15 +++++++++------
 test/lisp/minibuffer-tests.el | 17 ++++++++++++-----
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 6fae62b3904..1efe71f10e3 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -4418,18 +4418,21 @@ completion-pcm--merge-completions
                      (unique (or (and (eq prefix t) (setq prefix fixed))
                                  (and (stringp prefix)
                                       (eq t (try-completion prefix comps))))))
-                ;; if the common prefix is unique, it also is a common
-                ;; suffix, so we should add it for `prefix' elements
-                (unless (or (and (eq elem 'prefix) (not unique))
-                            (equal prefix ""))
-                  (push prefix res))
                 ;; If there's only one completion, `elem' is not useful
                 ;; any more: it can only match the empty string.
                 ;; FIXME: in some cases, it may be necessary to turn an
                 ;; `any' into a `star' because the surrounding context has
                 ;; changed such that string->pattern wouldn't add an `any'
                 ;; here any more.
-                (unless unique
+                (if unique
+                    ;; if the common prefix is unique, it also is a common
+                    ;; suffix, so we should add it for `prefix' elements
+                    (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)
+                    (setq prefix fixed))
+                  (push prefix res)
                   (push elem res)
                   ;; Extract common suffix additionally to common prefix.
                   ;; Don't do it for `any' since it could lead to a merged
diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el
index df36bce4634..38c2b8c4552 100644
--- a/test/lisp/minibuffer-tests.el
+++ b/test/lisp/minibuffer-tests.el
@@ -306,13 +306,20 @@ completion-substring-test-4
            6)))
 
 (ert-deftest completion-substring-test-5 ()
-  ;; merge-completions needs to work correctly when
+  ;; Normally a `prefix' wildcard ignores the common prefix to its
+  ;; left, since it only grows the common suffix; but if that common
+  ;; prefix is also a common suffix, it should be included.
   (should (equal
-           (completion-pcm--merge-completions '("ab" "sab") '(prefix "b"))
-           '("b" "a" prefix)))
+           (completion-pcm--merge-try '(prefix "b") '("ab" "sab") "" "")
+           '("ab" . 2)))
   (should (equal
-           (completion-pcm--merge-completions '("ab" "ab") '(prefix "b"))
-           '("b" "a")))
+           (completion-pcm--merge-try '(prefix "b") '("ab" "ab") "" "")
+           '("ab" . 2)))
+  ;; When there's a fixed string before `prefix', that fixed string
+  ;; should always be included.
+  (should (equal
+           (completion-pcm--merge-try '("a" prefix "b") '("axb" "ayb") "" "")
+           '("ab" . 2)))
   ;; substring completion should successfully complete the entire string
   (should (equal
            (completion-substring-try-completion "b" '("ab" "ab") nil 0)
-- 
2.39.3