GNU bug report logs - #42149
Substring and flex completion ignore implicit trailing ‘any’

Previous Next

Package: emacs;

Reported by: Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>

Date: Wed, 1 Jul 2020 10:41:01 UTC

Severity: normal

Tags: fixed, patch

Fixed in version 28.1

Done: Lars Ingebrigtsen <larsi <at> gnus.org>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: João Távora <joaotavora <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 42149 <at> debbugs.gnu.org, Dario Gjorgjevski <dario.gjorgjevski <at> gmail.com>
Subject: bug#42149: Substring and flex completion ignore implicit trailing ‘any’
Date: Mon, 28 Dec 2020 09:30:33 +0000
Stefan Monnier <monnier <at> iro.umontreal.ca> 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




This bug report was last modified 4 years and 7 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.