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

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

Stefan Monnier <[email-masked]> writes:

>> Please find attached a patch with an edited commit message.  It should
>> be better.
>
> [ Boy, we are really unacceptably slow at reviewing this.  ]
>
> Thanks very much for your patch.
> It looks good to me, but I think it's important we find a fix with which
> João agrees.

Indeed, I'm very sorry to be a kill-joy here, the patch doesn't look
very good to me.  Last time I looked it was too complex for me to
follow, touches many lines, and created unnecessary consing.  I'm
convinced the current algorithm in completion-pcm--hilit-commonality is
fine for these 1-char edge cases, given that the assumptions hold.

>> Furthermore, ‘completions-first-difference’ and
>> ‘completions-common-part’ would sometimes overlap depending on the
>> position of point within the query string.
>
> Could you point us at the corresponding test?
> [ And thanks so much for the tests, this is great!  ]

Yes, please use the latest tests that I've pushed to the

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

branch.  They are still your test, only discriminated into various
ert-deftest.  If you want, you can split and discriminate further.

> I think João knows the scoring algorithm much more than I do, so I'll
> let him judge if the change is sound.

I'm not aware that it's not sound, but I do believe it's too complex and
not well understood.  In constrast, I can understand the three-line fix
I did earlier and which covers all of Darios's test cases for the flex
completion style.  Previously it was failing 7 cases, now it is only
failing these 3.

F completion-pcm-test-3
F completion-pcm-test-5
F completion-substring-test-4

João