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

From: João Távora <[email-masked]>
Date: Sun, 27 Dec 2020 20:08:04 +0000

Dario Gjorgjevski <[email-masked]> 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