Package: emacs;
Reported by: Stephen Berman <stephen.berman <at> gmx.net>
Date: Thu, 10 Apr 2025 22:23:02 UTC
Severity: normal
Found in version 31.0.50
Done: Stefan Monnier <monnier <at> iro.umontreal.ca>
View this message in rfc822 format
From: Spencer Baugh <sbaugh <at> janestreet.com> To: Stefan Monnier <monnier <at> iro.umontreal.ca> Cc: 77718 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, stephen.berman <at> gmx.net Subject: bug#77718: 31.0.50; completion styles substring and flex are broken Date: Thu, 17 Apr 2025 12:39:48 -0400
[Message part 1 (text/plain, inline)]
Stefan Monnier <monnier <at> iro.umontreal.ca> writes: >> I will try to think of another approach which is more similar to how >> completion-table-with-quoting does things, but it will be more >> complicated. > > I thought all that's missing is to double the $$ in the text we append > during completion. Yes, actually you're right, that turns out to be relatively simple. And now the quoting behavior doesn't change at all. Updated patch attached. The $ doubling also needs to happen in all-completions, not just try-completion. But, with sufficiently robust logic for figuring out the completion boundaries, that's straightforward: just double the $ in the newly added text.
[0001-Improve-env-var-handling-in-read-file-name.patch (text/x-patch, inline)]
From 6a8685edc22cba617ddced1f2b2e1b6c4e1c9de2 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Mon, 14 Apr 2025 16:01:38 -0400
Subject: [PATCH] Improve env var handling in read-file-name
Fix various bugs, including bug#77718, by rewriting the way file
name completion handles environment variable expansion. Instead
of using completion-table-with-quoting to manipulate the string
being completed on, simply make the completion table itself
understand substitute-in-file-name.
Tests are updated: partial-completion now preserves unexpanded
environment variables. However, partial-completion no longer
works across environment variables containing delimiters; that's
an acceptable sacrifice.
* lisp/minibuffer.el (completion--sifn-boundaries): Add.
(completion--file-name-table): Rewrite to use
substitute-in-file-name explicitly. (bug#77718)
* test/lisp/minibuffer-tests.el (completion-table-test-quoting):
Update.
---
lisp/minibuffer.el | 59 ++++++++++++++++++++++++++++++++---
test/lisp/minibuffer-tests.el | 6 ++--
2 files changed, 56 insertions(+), 9 deletions(-)
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index c0ccfc00ce5..8edeba1fba9 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3596,13 +3596,62 @@ completion--sifn-requote
(setq qpos (1- qpos)))
(cons qpos #'minibuffer-maybe-quote-filename)))))
-(defalias 'completion--file-name-table
- (completion-table-with-quoting #'completion-file-name-table
- #'substitute-in-file-name
- #'completion--sifn-requote)
+(defun completion--sifn-boundaries (string table pred suffix)
+ "Return completion boundaries on file name STRING.
+
+Runs `substitute-in-file-name' on STRING first, but returns completion
+boundaries for the original string."
+ (let* (;; Calculate the completion boundaries without expanding env vars.
+ (unquoted
+ (let ((process-environment nil)) (substitute-in-file-name string)))
+ (bounds (completion-boundaries unquoted table pred suffix))
+ ;; This is the part of UNQUOTED inside the completion boundaries.
+ (unquoted-suffix (substring unquoted (car bounds)))
+ ;; Each $ in UNQUOTED-SUFFIX is either $ or $$ in STRING.
+ (regex
+ (concat (replace-regexp-in-string "\\$" "\\$\\$?" unquoted-suffix t t) "$")))
+ (cl-assert (string-match regex string) t)
+ (cons (match-beginning 0) (cdr bounds))))
+
+(defun completion--file-name-table (orig pred action)
"Internal subroutine for `read-file-name'. Do not call this.
This is a completion table for file names, like `completion-file-name-table'
-except that it passes the file name through `substitute-in-file-name'.")
+except that it passes the file name through `substitute-in-file-name'."
+ (let ((table #'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))
+ (result
+ (let ((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
+ ;; handle that anyway.
+ (cl-remove-if (lambda (regexp) (string-search "$" regexp))
+ completion-regexp-list)))
+ (complete-with-action action table sifned pred))))
+ (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)))
+ 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)))))
+ result))
+ (t result))))))
(defalias 'read-file-name-internal
(completion-table-in-turn #'completion--embedded-envvar-table
diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el
index 59ac5ab9578..9e28077026c 100644
--- a/test/lisp/minibuffer-tests.el
+++ b/test/lisp/minibuffer-tests.el
@@ -91,7 +91,7 @@ completion-table-subvert-test
(ert-deftest completion-table-test-quoting ()
(let ((process-environment
- `("CTTQ1=ed" "CTTQ2=et/" ,@process-environment))
+ `("CTTQ1=ed" ,@process-environment))
(default-directory (ert-resource-directory)))
(pcase-dolist (`(,input ,output)
'(
@@ -101,10 +101,8 @@ completion-table-test-quoting
("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")
;; Test that env-vars don't prevent partial-completion.
- ;; FIXME: Ideally we'd like to keep the ${CTTQ}!
- ("lis/c${CTTQ1}/se-u" "lisp/cedet/semantic-utest")
+ ("lis/c${CTTQ1}/se-u" "lisp/c${CTTQ1}et/semantic-utest")
))
(should (equal (completion-try-completion input
#'completion--file-name-table
--
2.39.3
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.