Subject: Re: bug#77718: 31.0.50; completion styles substring and flex are broken

From: Spencer Baugh <[email-masked]>
Date: Thu, 01 May 2025 13:59:48 -0400

Stephen Berman <[email-masked]> writes:
> For the other regression, here is a reproducer:
>
> 0. mkdir /tmp/{Test,retest}
> 1. emacs -Q --eval "(custom-set-variables                        \
>    '(completion-category-overrides '((file (styles substring)))) \
>    '(read-file-name-completion-ignore-case t))"
> 2. Type `C-x d /tmp/tes TAB': this completes to /tmp/test/
> 3. Type TAB again: Emacs dings and shows "[no match]"
>
> If you repeat this recipe in emacs-30, then after the TAB in step 3, this
> completion is shown: /tmp/Test/. and typing TAB again pops up the
> *Completions* buffer displaying the two completions ../ and ./
>
> While the result in emacs-30 seems suboptimal, since it omits the
> possible completion /tmp/retest/, still it's better than "[no match]".
>
> A workaround for both emacs-30 and master is to enter at step 2 `C-x d
> /tmp/*tes TAB'. This shows the completion /tmp/*test/, then hitting TAB
> again shows /tmp/*test/., and hitting TAB a third time pops up the
> *Completions* buffer showing the four completions Test/../, Test/./,
> retest/../, and retest/./
>
> Steve Berman

Thanks for the prompt and detailed report.  Does the attached patch fix
this issue for you?

>From 2619db58b2f3bdaeed0ea077b3292d93621e15d8 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <[email-masked]>
Date: Thu, 1 May 2025 13:56:37 -0400
Subject: [PATCH] Fix completion-ignore-case with completion--file-name-table

509cbe1c35b3d "Improve env var handling in read-file-name"
caused try-completion and all-completion operations with
completion--file-name-table to no longer update the case of text
which was already present in the input string.  That is,
completions would be returned ignoring case, but the completions
would have letter-casing which matched the input string rather
than matching the actual file names.

Fix this by always using text from the file name completions
whenever it might have changed case.  Also, fix a related bug
where mixing completion-ignore-case and expanding environment
variables would cause `read-file-name' to return the wrong
result; do this by suppressing completion-ignore-case in the
problematic case.

* lisp/minibuffer.el (completion--file-name-table): Use text
from the completions, not the input string.  (bug#77718)
* test/lisp/minibuffer-tests.el (completion-table-test-quoting):
Test with completion-ignore-case as well.
---
 lisp/minibuffer.el            | 42 +++++++++++++++++++++++------------
 test/lisp/minibuffer-tests.el | 18 ++++++++++++++-
 2 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 7b2b986aa1d..e35db9dad64 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3597,8 +3597,18 @@ completion--file-name-table
     (if (eq (car-safe action) 'boundaries)
         (cons 'boundaries (completion--sifn-boundaries orig table pred (cdr action)))
       (let* ((sifned (substitute-in-file-name orig))
+             (orig-start (car (completion--sifn-boundaries orig table pred "")))
+             (sifned-start (car (completion-boundaries sifned table pred "")))
+             (orig-in-bounds (substring orig orig-start))
+             (sifned-in-bounds (substring sifned sifned-start))
+             (did-expansion-in-bounds (not (string-equal orig-in-bounds sifned-in-bounds)))
              (result
-              (let ((completion-regexp-list
+              (let ((completion-ignore-case
+                     ;; If we expanded an environment variable in the completion
+                     ;; bounds, don't ignore-case, otherwise `read-file-name'
+                     ;; can return the wrong filename.
+                     (and (not did-expansion-in-bounds) completion-ignore-case))
+                    (completion-regexp-list
                      ;; Regexps are matched against the real file names after
                      ;; expansion, so regexps containing $ won't work.  Drop
                      ;; them; we'll return more completions, but callers need to
@@ -3606,26 +3616,30 @@ completion--file-name-table
                      (seq-remove (lambda (regexp) (string-search "$" regexp))
                                  completion-regexp-list)))
                 (complete-with-action action table sifned pred))))
+        ;; For each completion, quote dollar signs in newly added text.  If sifn
+        ;; changed text within the completion boundaries, replace that with text
+        ;; from ORIG; otherwise, use text from the completion (it may differ if
+        ;; `completion-ignore-case' is non-nil)
         (cond
          ((null action)                 ; try-completion
           (if (stringp result)
-              ;; Extract the newly added text, quote any dollar signs, and
-              ;; append it to ORIG.
               (let ((new-text (substring result (length sifned))))
-                (concat orig (minibuffer--double-dollars new-text)))
+                ;; Unconditionally replace the text outside the
+                ;; completion boundaries with text from ORIG.
+                (concat (substring orig nil orig-start)
+                        (if did-expansion-in-bounds
+                            orig-in-bounds
+                          (substring result sifned-start (length sifned)))
+                        (minibuffer--double-dollars new-text)))
             result))
          ((eq action t)                 ; all-completions
           (mapcar
-           (let ((orig-prefix
-                  (substring orig (car (completion--sifn-boundaries orig table pred ""))))
-                 (sifned-prefix-length
-                  (- (length sifned)
-                     (car (completion-boundaries sifned table pred "")))))
-             ;; Extract the newly added text, quote any dollar signs, and append
-             ;; it to the part of ORIG inside the completion boundaries.
-             (lambda (compl)
-               (let ((new-text (substring compl sifned-prefix-length)))
-                 (concat orig-prefix (minibuffer--double-dollars new-text)))))
+           (lambda (compl)
+             (let ((new-text (substring compl (length sifned-in-bounds))))
+               (concat (if did-expansion-in-bounds
+                           orig-in-bounds
+                         (substring compl nil (length sifned-in-bounds)))
+                       (minibuffer--double-dollars new-text))))
            result))
          (t result))))))
 
diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el
index bed797bdb14..5e013668a2e 100644
--- a/test/lisp/minibuffer-tests.el
+++ b/test/lisp/minibuffer-tests.el
@@ -108,7 +108,23 @@ completion-table-test-quoting
       (should (equal (completion-try-completion input
                                                 #'completion--file-name-table
                                                 nil (length input))
-                     (cons output (length output)))))))
+                     (cons output (length output)))))
+    ;; Everything also works with `completion-ignore-case'.
+    (let ((completion-ignore-case t))
+      (pcase-dolist (`(,input ,output)
+                     '(
+                       ("data/M-CTTQ" "data/minibuffer-test-cttq$$tion")
+                       ("data/M-CTTQ$$t" "data/minibuffer-test-cttq$$tion")
+                       ("lisp/c${CTTQ1}et/SE-U" "lisp/c${CTTQ1}et/semantic-utest")
+                       ;; `completion-ignore-case' is suppressed when an
+                       ;; environment variable is in the completion bounds.
+                       ("lisp/ced${CTTQ2}SE-U" nil)
+                       ("lis/c${CTTQ1}/SE-U" "lisp/c${CTTQ1}et/semantic-utest")
+                       ))
+        (should (equal (car (completion-try-completion input
+                                                       #'completion--file-name-table
+                                                       nil (length input)))
+                       output))))))
 
 (ert-deftest completion--insert-strings-faces ()
   (with-temp-buffer
-- 
2.39.3