Subject: bug#79265: closed (Re: bug#79265: [PATCH] Treat point more consistently in PCM completion)

From: [email-masked] (GNU bug Tracking System)
Date: Thu, 21 Aug 2025 01:06:02 +0000

Your bug report

#79265: [PATCH] Treat point more consistently in PCM completion

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
79265: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=79265
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#79265: [PATCH] Treat point more consistently in PCM completion
From: Dmitry Gutov <[email-masked]>
To: Spencer Baugh <[email-masked]>
Cc: Stefan Monnier <[email-masked]>, [email-masked]
Date: Thu, 21 Aug 2025 04:05:38 +0300
List-Id: <debbugs-submit.debbugs.gnu.org>

On 20/08/2025 17:35, Spencer Baugh wrote:

>> So IDK - maybe it's better to use the alien cl-loop, or maybe to go
>> back to seq-some, if no measurements show a cause for worry.
> 
> Eh, I went back to seq-every-p for now.  This shouldn't really cause a
> performance problem, since it will usually just return nil quickly.
> Also it's only run when the user hits TAB.

Sounds good. Its overhead it two dynamic dispatches, so probably fine 
for most uses.

> (I'd rather not use cl-loop, this code is already hard enough to
> understand :) )

All right then. (I like cl-loop ;-( )

>> Just to check: in our limited test examples all elements of COMP match
>> PREFIX already. And the beginning of
>> 'completion-pcm--merge-completions' even 'error'-s if string-match
>> comparison fails.
>>
>> So it's possible that we could omit this comparison from the lambda:
>>
>>    (and (string-prefix-p prefix comp completion-ignore-case)
>>
>> But I suppose that would make it a bigger change, a bit riskier.
> 
> Ah, you're right.  I missed that we already have this property
> guaranteed, but indeed we do.
> 
> So thanks to that, the change can be further simplified.  Removing my
> cond refactoring, the patch is now just replacing
> 
>    (eq t (try-completion prefix comps))
> 
> with
> 
>    (seq-every-p
>      (lambda (comp) (= (length prefix) (length comp)))
>      comps)
> 
> which is a nice simple change.  I added some comments to clarify why
> this is correct, too.

Thanks, pushed to master, this and the the following patch (nice 
optimization!).

And with that, time to close.

Subject: [PATCH] Treat point more consistently in PCM completion
From: Spencer Baugh <[email-masked]>
To: [email-masked]
Date: Mon, 18 Aug 2025 14:53:24 -0400
List-Id: <debbugs-submit.debbugs.gnu.org>

Tags: patch

Properly fix bug#38458, which is fundamentally an issue with
completion-ignore-case, by checking if the completions are
unique ignoring case.  When the completions are unique, the
normal code to delete a wildcard naturally causes point to be
moved to the end of the minibuffer, which is the correct
behavior.

Now that the bug is fixed properly, remove a hack which
previously was used to "fix" it, which made point behave
inconsistently if it was in the middle of the minibuffer versus
at the end of the minibuffer.

* lisp/minibuffer.el (completion-pcm--merge-completions):
Respect completion-ignore-case when checking for completion
uniqueness.
(completion-pcm--string->pattern)
(completion-pcm--optimize-pattern): Allow point at the end of
the pattern.
* test/lisp/minibuffer-tests.el (completion-table-test-quoting)
(completion-test--pcm-bug38458, completion-pcm-test-8): Update
tests for more correct behavior.

In GNU Emacs 30.1.90 (build 9, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.15.12, Xaw scroll bars) of 2025-08-14 built on
 igm-qws-u22796a
Repository revision: 6adc26ffa74aedbd1cfa9a1ee72073ebccea2b96
Repository branch: emacs-30
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-tree-sitter
 --with-native-compilation=aot
 PKG_CONFIG_PATH=/usr/local/home/garnish/libtree-sitter/0.22.6-1/lib/pkgconfig/'

>From dbb5099a511f90e19761c0cf105c929e71a3c081 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <[email-masked]>
Date: Mon, 18 Aug 2025 14:50:44 -0400
Subject: [PATCH] Treat point more consistently in PCM completion

Properly fix bug#38458, which is fundamentally an issue with
completion-ignore-case, by checking if the completions are
unique ignoring case.  When the completions are unique, the
normal code to delete a wildcard naturally causes point to be
moved to the end of the minibuffer, which is the correct
behavior.

Now that the bug is fixed properly, remove a hack which
previously was used to "fix" it, which made point behave
inconsistently if it was in the middle of the minibuffer versus
at the end of the minibuffer.

* lisp/minibuffer.el (completion-pcm--merge-completions):
Respect completion-ignore-case when checking for completion
uniqueness.
(completion-pcm--string->pattern)
(completion-pcm--optimize-pattern): Allow point at the end of
the pattern.
* test/lisp/minibuffer-tests.el (completion-table-test-quoting)
(completion-test--pcm-bug38458, completion-pcm-test-8): Update
tests for more correct behavior.
---
 lisp/minibuffer.el            | 15 +++++++--------
 test/lisp/minibuffer-tests.el | 28 +++++++++++++++++++++-------
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 689cda7edab..046d792b27d 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -4237,7 +4237,7 @@ completion-pcm--string->pattern
   "Split STRING into a pattern.
 A pattern is a list where each element is either a string
 or a symbol, see `completion-pcm--merge-completions'."
-  (if (and point (< point (length string)))
+  (if (and point (<= point (length string)))
       (let ((prefix (substring string 0 point))
             (suffix (substring string point)))
         (append (completion-pcm--string->pattern prefix)
@@ -4298,12 +4298,6 @@ completion-pcm--optimize-pattern
       (pcase p
         (`(,(or 'any 'any-delim) ,(or 'any 'point) . ,_)
          (setq p (cdr p)))
-        ;; 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
-        ;; into a non-greedy ".*?" regexp whereas we need it to be
-        ;; greedy when it's at the end, see bug#38458).
-        (`(point) (setq p nil)) ;Implicit terminating `any'.
         (_ (push (pop p) n))))
     (nreverse n)))
 
@@ -4757,7 +4751,12 @@ completion-pcm--merge-completions
               (let* ((prefix (try-completion fixed comps))
                      (unique (or (and (eq prefix t) (setq prefix fixed))
                                  (and (stringp prefix)
-                                      (eq t (try-completion prefix comps))))))
+                                      (eq t (try-completion prefix comps)))
+                                 (and completion-ignore-case
+                                      ;; They're all the same string, ignoring case.
+                                      (seq-every-p
+                                       (lambda (elem) (string-equal-ignore-case elem prefix))
+                                       comps)))))
                 ;; 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
diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el
index f9a26d17e58..59b72899e22 100644
--- a/test/lisp/minibuffer-tests.el
+++ b/test/lisp/minibuffer-tests.el
@@ -100,10 +100,10 @@ completion-table-test-quoting
                      ;; Test that $$ in input is properly unquoted.
                      ("data/m-cttq$$t" "data/minibuffer-test-cttq$$tion")
                      ;; Test that env-vars are preserved.
-                     ("lisp/c${CTTQ1}et/se-u" "lisp/c${CTTQ1}et/semantic-utest")
-                     ("lisp/ced${CTTQ2}se-u" "lisp/ced${CTTQ2}semantic-utest")
+                     ("lisp/c${CTTQ1}et/se-u-c" "lisp/c${CTTQ1}et/semantic-utest-c.test")
+                     ("lisp/ced${CTTQ2}se-u-c" "lisp/ced${CTTQ2}semantic-utest-c.test")
                      ;; Test that env-vars don't prevent partial-completion.
-                     ("lis/c${CTTQ1}/se-u" "lisp/c${CTTQ1}et/semantic-utest")
+                     ("lis/c${CTTQ1}/se-u-c" "lisp/c${CTTQ1}et/semantic-utest-c.test")
                      ))
       (should (equal (completion-try-completion input
                                                 #'completion--file-name-table
@@ -118,11 +118,11 @@ completion-table-test-quoting
                        ;; When an env var is in the completion bounds, try-completion
                        ;; won't change letter case.
                        ("lisp/c${CTTQ1}E" "lisp/c${CTTQ1}Et/")
-                       ("lisp/ced${CTTQ2}SE-U" "lisp/ced${CTTQ2}SEmantic-utest")
+                       ("lisp/ced${CTTQ2}SE-U-c" "lisp/ced${CTTQ2}SEmantic-utest-c.test")
                        ;; If the env var is before the completion bounds, try-completion
                        ;; *will* change letter case.
-                       ("lisp/c${CTTQ1}et/SE-U" "lisp/c${CTTQ1}et/semantic-utest")
-                       ("lis/c${CTTQ1}/SE-U" "lisp/c${CTTQ1}et/semantic-utest")
+                       ("lisp/c${CTTQ1}et/SE-U-c" "lisp/c${CTTQ1}et/semantic-utest-c.test")
+                       ("lis/c${CTTQ1}/SE-U-c" "lisp/c${CTTQ1}et/semantic-utest-c.test")
                        ))
         (should (equal (car (completion-try-completion input
                                                        #'completion--file-name-table
@@ -224,7 +224,11 @@ completion-test--pcm-bug38458
                    (completion-pcm--merge-try '("tes" point "ing")
                                               '("Testing" "testing")
                                               "" ""))
-           '("testing" . 4))))
+                 '("testing" . 7)))
+  (should (equal
+           (let ((completion-ignore-case t))
+             (completion-pcm-try-completion "tes" '("Testing" "testing") nil 3))
+           '("testing" . 7))))
 
 (ert-deftest completion-pcm-test-1 ()
   ;; Point is at end, this does not match anything
@@ -318,6 +322,16 @@ completion-pcm-test-7
             '(prefix any "bar" any) '("xbarxfoo" "ybaryfoo") "" "")
            '("bar" . 3))))
 
+(ert-deftest completion-pcm-test-8 ()
+  ;; try-completion inserts the common prefix and suffix at point.
+  (should (equal (completion-pcm-try-completion
+                  "r" '("fooxbar" "fooybar") nil 0)
+                 '("foobar" . 3)))
+  ;; Even if point is at the end of the minibuffer.
+  (should (equal (completion-pcm-try-completion
+                  "" '("fooxbar" "fooybar") nil 0)
+                 '("foobar" . 3))))
+
 (ert-deftest completion-substring-test-1 ()
   ;; One third of a match!
   (should (equal
-- 
2.43.7