Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> An explicitly typed * has different semantics from automatically
>> inserted PCM wildcards, so it should be preserved on
>> try-completion.  We already do this in some cases, but now we do
>> it more.  Concretely, we do it by optimizing the PCM pattern
>> more aggressively to avoid having multiple wildcards in a row:
>> after those are removed, the existing code in
>> completion-pcm--merge-completions is able to preserve * in more
>> cases.
>
> Oh, indeed, thank you.
>
> The patch looks good to me, so if there are no objection, I'll install
> it in a few days (feel free to ping me if I forget).

Actually... I just realized this misses some cases, namely when we have
"star point" or "point star".  Attached at the end is a different patch
which takes a different approach (and which includes new tests for the
problematic cases).

The changes to optimize still seem reasonable to me, since they might
improve performance, so feel free to install it if you think it's good.
Although maybe we want to revise it with the below discussion.

> Banter follows.
>
>> * lisp/minibuffer.el (completion-pcm--optimize-pattern): Add
>> more optimizations. (bug#74420)
>
> Aha, so that's why you didn't submit the patch together with the initial
> bug report!
>
>> -        (`(,(or 'any 'any-delim) ,(or 'any 'point) . ,_)
>> -         (setq p (cdr p)))
>> +        ;; Remove duplicate `any' and `prefix'
>> +        (`(any any . ,rest)
>> +         (setq p (cons 'any rest)))
>> +        (`(prefix prefix . ,rest)
>> +         (setq p (cons 'prefix rest)))
>> +        ;; `any' matches anything `any-delim' does, and grows the same way.
>> +        (`(any-delim any . ,rest)
>> +         (setq p (cons 'any rest)))
>
> AFAICT you can still merge the `any-delim any` and `any any` cases.

True, I just thought this was a bit easier to read, since the cases are
justified as an optimization in slightly different ways.

>> +        ;; Remove other wildcards found around `star' or `point'.
>> +        ((or `(,(and keep (or 'star 'point)) ,(or 'any 'any-delim 'prefix) . ,rest)
>> +             `(,(or 'any 'any-delim 'prefix) ,(and keep (or 'star 'point)) . ,rest))
>> +         (setq p (cons keep rest)))
>
> BTW, maybe we should go with something like
>
>        (`(,(and (pred symbolp) s1) ,(and (pred symbolp) s2) . ,rest)
>         (cond ((completion-pcm--<something>-p s1 s2) (setq p (cons s1 rest)))
>               ((completion-pcm--<something>-p s2 s1) (setq p (cons s2 rest)))
>               (t (push (pop p) n))))
>
> Where `completion-pcm--<something>-p` is some kind of partial ordering?

Interesting thought.  Maybe it would make sense to have something like
- completion-pcm--wildcard-grows-on-left-p
  (non-nil for star, point, any, any-delim)
- completion-pcm--wildcard-grows-on-right-p
  (non-nil for star, point, prefix)
- completion-pcm--wildcard-in-text-p
  (non-nil for star and point)

Then this case would be something like "if grows-left/right-p is the
same, and at most one of the symbols is in-text-p, delete the
non-in-text-p one".

Also, then the first two helpers could be used in
completion-pcm--merge-completions as well, which could make the wildcard
behavior easier to understand.  (I want to do a bit of refactoring in
completion-pcm--merge-completions, especially after my attached change;
I think it could all be a bit easier to understand.)