Subject: Re: bug#79265: [PATCH] Treat point more consistently in PCM completion, [PATCH] Treat point more consistently in PCM completion

From: Stefan Monnier <[email-masked]>
Date: Thu, 21 Aug 2025 14:58:07 -0400

[ Sorry for the delay reviewing this patch (and others, still waiting).
  I still have rather spotty access to my keyboard, tho things should
  slowly return to normal over the next two weeks.  ]

> From: Spencer Baugh <[email-masked]>
> Date: Wed, 20 Aug 2025 13:23:34 -0400
> Subject: [PATCH] Avoid duplicating strings in pcm--merge-completions
>
> Make completion-pcm--merge-completions operate only on the text
> matched by the wildcards, instead of also the text in between
> the wildcards.  This improves performance and simplifies the
> code by removing the need for the previous mutable variable
> "fixed".
>
> * lisp/minibuffer.el (completion-pcm--merge-completions):
> Operate only on text matched by wildcards. (bug#79265)

Could we have a regression test for this change?
Or is it supposed to make no difference to behavior?

> -    (let ((re (completion-pcm--pattern->regex pattern 'group))
> +    (let ((re (concat
> +               (completion-pcm--pattern->regex pattern 'group)
> +               ;; The implicit trailing `any' is greedy.
> +               "\\([^z-a]*\\)"))
>            (ccs ()))                     ;Chopped completions.

The comment makes sense only if you know that
`completion-pcm--pattern->regex` would use a non-greedy regexp for
something like `any`.

> -            (while (setq next (match-end i))
> -              (push (substring str last next) chopped)
> -              (setq last next)
> +            (while (setq next (match-string i str))
> +              (push next chopped)

IIUC this means that when `completion-ignore-case` is t we will keep the
capitalization typed by the user rather than adjust it based on the
completion candidates.

E.g.

    (let ((completion-ignore-case t))
      (completion-pcm--merge-completions '("ABC" "ABD") '("a")))

will complete "a" to "aB" rather than to "AB".

See commit 681e0e7c02c4f8ff6d316c8c24001106cb847023 (which should have
come with a regression test, obviously).

        Stefan