bug#42149: Substring and flex completion ignore implicit trailing ‘any’

[João Távora]

I'm sorry, I seem to have sent the previous reply prematurely.

However, there was not much more to add except my signature
and some more reporting on test results.

Thanks,
João

On Sun, Dec 27, 2020 at 8:08 PM João Távora <joaotavora@gmail.com> wrote:
>
> Dario Gjorgjevski <dario.gjorgjevski@gmail.com> writes:
>
> > Hi,
> >
> > Has anyone had the time to look into this?
> >
> > Best regards,
> > Dario
>
> Hi Dario,
>
> After a long long delay, I've now looked at this in earnest.
>
> I can report that I think the problem lies somewhere completely
> different than what I think you patch addresses.  Instead of reworking
>
>     completion-pcm--hilit-commonality   [1]
>
> I think we should take a better look at
> completion-pcm--optimize-pattern.  In its current form, it will thus
> "optimize" the pcm patterns like so:
>
> 1 -> (completion-pcm--optimize-pattern (prefix "f" any point))
> 1 <- completion-pcm--optimize-pattern: (prefix "f")
>
> whereas I think it shouldn't be optimizing away the "any".  When I make
> it keep the any with this simple patch, _most_ of your tests start
> passing becasue completion-pcm--hilit-commonality starts doing the right
> thing, i.e. it starts working the way it was intented to work,
> considering a (potentially empty) hole in the back of the pattern form.
>
> This is that simple patch:
>
> diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
> index 7d05f7704e..637a29eaa0 100644
> --- a/lisp/minibuffer.el
> +++ b/lisp/minibuffer.el
> @@ -3165,7 +3165,7 @@ completion-pcm--optimize-pattern
>          ;; the final position of point (because `point' gets turned
>          ;; into a non-greedy ".*?" regexp whereas we need it to be
>          ;; greedy when it's at the end, see bug#38458).
> -        (`(point) (setq p nil)) ;Implicit terminating `any'.
> +        (`(point) (setq p '(any)))  ;Implicit terminating `any'.
>          (_ (push (pop p) n))))
>      (nreverse n)))
>
> However, I'm pretty sure Stefan will tell us to hold our horses with any
> changes to this, since it's used in many more mysterious ways that I
> can't fathom.
>
> So, maybe this is a smaller, safer change:
>
> diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
> index 7d05f7704e..cc2573db19 100644
> --- a/lisp/minibuffer.el
> +++ b/lisp/minibuffer.el
> @@ -3245,6 +3245,8 @@ flex-score-match-tightness
>
>  (defun completion-pcm--hilit-commonality (pattern completions)
>    (when completions
> +    (unless (eq (car (last pattern)) 'any)
> +      (setq pattern (append pattern '(any))))
>      (let* ((re (completion-pcm--pattern->regex pattern 'group))
>             (point-idx (completion-pcm--pattern-point-idx pattern))
>             (case-fold-search completion-ignore-case))
>
> [1]: completion-pcm--hilit-commonality, which does seem to have a couple
> of superflous calls to the update-score local function)
>
> [2]: please Stefan: remind me for the 1000th time what "pcm" stands for



-- 
João Távora