From 3cda1fdc3b755dd329617cabc266e2b86894d0cb Mon Sep 17 00:00:00 2001 From: Spencer Baugh Date: Mon, 26 Aug 2024 10:12:51 -0400 Subject: [PATCH] Correctly include fixed strings before a prefix wildcard in PCM In 03ac16ece40ba3e3ba805d6a61cc457d84bf3792 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): Include fixed strings before 'prefix wildcard. (Bug#72819) * test/lisp/minibuffer-tests.el (completion-substring-test-5): Add a test for this behavior. --- 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 bef565378ea..216385d9cdf 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -4384,18 +4384,21 @@ the same set of elements." (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 @@ 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)