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

From: João Távora <[email-masked]>
Date: Thu, 13 May 2021 15:31:29 +0100

Lars Ingebrigtsen <[email-masked]> writes:

> João Távora <[email-masked]> writes:
>
>> And I just pushed my cleaned up fix to to master as well, thus hopefully
>> fixing the brunt of this bug.  Dario and others, please test this.  I
>> haven't yet pushed the tests, since we're not entirely sure of those,
>> but I think we should break them up further and push them too, once we
>> come to an aggreement on what and how they should test exactly.
>
> I've only skimmed this long thread, but my understanding of it is that
> the reported bug was fixed...  but there was some discussion about
> including (or not) Dario's tests?
>
> Which (if I'm grepping correctly) would be the patch below?  I tried
> applying it, and:
>
> 2 unexpected results:
>    FAILED  completion-pcm-all-completions-test
>    FAILED  completion-substring-all-completions-test
>
> I have not looked into this further -- João, what's the state here?

I think you applied the original patch of two failing tests, the tests
that demonstrate a particular bug.  So it makes sense that hey fail.

I think we want to merge what's in the
scratch/bug-42149-funny-pcm-completion-scores.  I attach a summary of
the four commtis there.  Then we want to close this issue.

Not sure if it's merged yet, but I don't think so.  I was waiting for
Dario's comments on it, they never arrived, but I'm veryq confident that
this fixes the issues reported here.

There are 4 commits there.  And if you merge this branch, _don't_ also
try to merge the patch you tried earlier: the branch already contains a
rewrite of those tests.

João

commit 03c160fb1573107586355e851c111326debfe95a
Author: João Távora <[email-masked]>
Date:   Tue Dec 29 13:31:46 2020 +0000

    Fix "first-differente" face in completion-pcm--hilit-commonality
    
    Fixes: bug#42149
    
    Depending on the position of point in the completion and the
    completion style being used, it may or may not make sense for this
    face to appear immediately after point.  This patch assumes that it
    should appear in the first non-matched character after point, which
    may likely be the next one to type to disambiguate between two or more
    completions.
    
    Suggested by Dario Gjorgjevski <[email-masked]>.
    
    * lisp/minibuffer.el (completion-pcm--hilit-commonality): Fix
    occasional misplacement of completions-first-differente.

commit d8c596f7309bd6fd6e127b8027dfb4c508afd2ea
Author: João Távora <[email-masked]>
Date:   Mon Dec 28 09:10:19 2020 +0000

    Robustify a helper function for test/lisp/minibuffer-tests.el
    
    completion--pcm-first-difference-pos wasn't taking into account the
    fact that faces may come in lists.  bug#42149
    
    * test/lisp/minibuffer-tests.el
    (completion--pcm-first-difference-pos): Robustify.

commit d333ec4cabd21244e5ee468b3a7475fa2dcbe614
Author: João Távora <[email-masked]>
Date:   Tue Nov 24 23:15:40 2020 +0000

    Make a completion test robust to custom completion styles
    
    * test/lisp/minibuffer-tests.el (completion-test1): Make test
    resilient to more completion styles.

commit 0265a99ed6b035930fdb21d5bcfdab0707b303aa
Author: João Távora <[email-masked]>
Date:   Tue Nov 24 22:34:22 2020 +0000

    Add tests for bug#42149
    
    * test/lisp/minibuffer-tests.el (completion--pcm-score)
    (completion--pcm-first-difference-pos): New helpers.
    (completion-pcm-test-1, completion-pcm-test-2)
    (completion-pcm-test-3, completion-pcm-test-4)
    (completion-pcm-test-5, completion-pcm-test-6)
    (completion-substring-test-1, completion-substring-test-2)
    (completion-substring-test-3, completion-substring-test-4)
    (completion-flex-test-1, completion-flex-test-2)
    (completion-flex-test-3): New tests.
    
    Co-authored-by: Dario Gjorgjevski <[email-masked]>

João