Subject: Re: bug#78323: Regression in case-insensitive filename completion

From: Spencer Baugh <[email-masked]>
Date: Mon, 19 May 2025 11:46:57 -0400

Eli Zaretskii <[email-masked]> writes:
>> From: Spencer Baugh <[email-masked]>
>> BTW, the patch I posted has the wrong commit message but is otherwise
>> correct.  Assuming there's review feedback, I'll correct the commit
>> message in the next version.
>
> Please do submit a correct patch, so we could resolve this regression.
>
> Thanks.

Here is the version with a correct commit message.  (I also added some
comments in the test)

>From d3d61cfe7699078c3f6405f2dd7a7a5f61d5985a Mon Sep 17 00:00:00 2001
From: Spencer Baugh <[email-masked]>
Date: Mon, 19 May 2025 11:35:38 -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.

This was caused by unnecessarily replacing text in the returned
file name completions with text from the input string ORIG,
which in turn was caused by the desire to preserve text from
ORIG even after substitute-in-file-name changed it.  Fix this by
detecting when ORIG was not substantially changed by
substitute-in-file-name; in that case, the returned file name
completions also don't need substantial changes.

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

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 7b2b986aa1d..6f9e6c67541 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3597,6 +3597,16 @@ 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))
+             (only-need-double-dollars
+              ;; If true, sifn only un-doubled $s in ORIG, so we can fix a
+              ;; completion to match ORIG by just doubling $s again.  This
+              ;; preserves more text from the completion, behaving better with
+              ;; non-nil `completion-ignore-case'.
+              (string-equal orig-in-bounds (minibuffer--double-dollars sifned-in-bounds)))
              (result
               (let ((completion-regexp-list
                      ;; Regexps are matched against the real file names after
@@ -3611,21 +3621,21 @@ completion--file-name-table
           (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)))
+              (if only-need-double-dollars
+                  (concat (substring orig nil orig-start)
+                          (minibuffer--double-dollars (substring result sifned-start)))
+                (let ((new-text (substring result (length sifned))))
+                  (concat orig (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 "")))))
+           (if only-need-double-dollars
+               #'minibuffer--double-dollars
              ;; 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)))))
+               (let ((new-text (substring compl (length sifned-in-bounds))))
+                 (concat orig-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..f9a26d17e58 100644
--- a/test/lisp/minibuffer-tests.el
+++ b/test/lisp/minibuffer-tests.el
@@ -108,7 +108,26 @@ 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")
+                       ;; 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")
+                       ;; 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")
+                       ))
+        (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