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

From: João Távora <[email-masked]>
Date: Mon, 28 Dec 2020 09:30:33 +0000

Stefan Monnier <[email-masked]> writes:

>> @@ -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'.
>
> There is always an implicit terminating `any`, and I'd really prefer to
> keep it implicit.

OK, but can you elaborate on why that is?

Regardless, if you don't want to touch that funciton, I understand, it
is used in more places than just completion-pcm--hilit-commonality,
which really should be called

   completion--given-that-we-know-this-matches-tell-me-where-and-how-well

(or maybe it should have a docstring, which I've done just now).

I propose something around this simpler patch.

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 7d05f7704e..df4ec67e35 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3255,10 +3255,10 @@ completion-pcm--hilit-commonality
          (unless (string-match re str)
            (error "Internal error: %s does not match %s" re str))
          (let* ((pos (if point-idx (match-beginning point-idx) (match-end 0)))
-                (md (match-data))
-                (start (pop md))
-                (end (pop md))
+                (md (cddr (match-data)))
+                (start 0)
                 (len (length str))
+                (end len)
                 ;; To understand how this works, consider these bad
                 ;; ascii(tm) diagrams showing how the pattern "foo"
                 ;; flex-matches "fabrobazo", "fbarbazoo" and

It seems to be passing your original report, but still failing some of
your test assertions.  However, i don't know if I agree with those test
assertions.

This is one of them.

    (ert-deftest completion-pcm-test-3 ()
      ;; Full match!
      (should (eql
               (completion--pcm-score
                (car (completion-pcm-all-completions
                      "R" '("R" "hello") nil 1)))
               1.0)))

I still don't know why this misses, but I do know that the scoring part
of completion-pcm--hilit-commonality is not meant for the "bare" pcm
completion style or substring completion style.  It could, though.

I'll take a better look at those cases.

Anyway, I've pushed this simple fix (and some another fix to the test
cases) to the working branch:

     scratch/bug-42149-funny-pcm-completion-scores

which I've also rebased on top of origin/master by deleting/recreating.

João