Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> 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))))
>
> What makes us think that the `cl-assert` will always be satisfied?
> Won't it fail in `/blabla/foo-$BAR.c` when `BAR=bash` ?

Because I bound process-environment to nil around the
substitute-in-file-name call, with the goal of preventing environment
variable expansion.

But actually there's a simpler and better way to do that: just duplicate
all the $s to $$, then substitute-in-file-name turns all the $$ back to
$ without expanding environment variables.  So I did that in the
attached updated patch.

>> @@ -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")
>
> Why do we remove this test?  Maybe it's OK that it doesn't return the
> same value (e.g. that it behaves less well), but if so, we should
> probably keep this commented out with an explanation of what's going on.

Fixed - actually, this test doesn't need to change at all, it still
works, just an oversight on my part.