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

From: Stefan Monnier <[email-masked]>
Date: Fri, 02 May 2025 12:04:57 -0400

> 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)))

Doesn't this set `did-expansion-in-bounds` to non-nil if there was a $$
in `orig`?

>               (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))

AFAICT we're not "doing The Right Thing" here but falling back to
a suboptimal behavior that favors not returning bogus completions at the
cost of missing some completions.  And we do this because doing TRT is
a lot more complicated.

I'm not opposed to this trade-off, but the comment should make it more
clear.  And it should come with a good example of the wrong thing we're
trying to avoid.

> +        ;; 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)

Here also, clarify that we choose between those two because just don't
want to write the code that correctly merges the capitalization changes
back into the non-env-var part of the original string.  An example is
probably the easier way to illustrate the difficulty.

        Stefan