Subject: Re: bug#79265: [PATCH] Treat point more consistently in PCM completion
From: Dmitry Gutov <[email-masked]>
To: Spencer Baugh <[email-masked]>
Cc: Stefan Monnier <[email-masked]>, [email-masked]
Date: Thu, 21 Aug 2025 04:05:38 +0300
List-Id: <debbugs-submit.debbugs.gnu.org>

On 20/08/2025 17:35, Spencer Baugh wrote:

>> So IDK - maybe it's better to use the alien cl-loop, or maybe to go
>> back to seq-some, if no measurements show a cause for worry.
> 
> Eh, I went back to seq-every-p for now.  This shouldn't really cause a
> performance problem, since it will usually just return nil quickly.
> Also it's only run when the user hits TAB.

Sounds good. Its overhead it two dynamic dispatches, so probably fine 
for most uses.

> (I'd rather not use cl-loop, this code is already hard enough to
> understand :) )

All right then. (I like cl-loop ;-( )

>> Just to check: in our limited test examples all elements of COMP match
>> PREFIX already. And the beginning of
>> 'completion-pcm--merge-completions' even 'error'-s if string-match
>> comparison fails.
>>
>> So it's possible that we could omit this comparison from the lambda:
>>
>>    (and (string-prefix-p prefix comp completion-ignore-case)
>>
>> But I suppose that would make it a bigger change, a bit riskier.
> 
> Ah, you're right.  I missed that we already have this property
> guaranteed, but indeed we do.
> 
> So thanks to that, the change can be further simplified.  Removing my
> cond refactoring, the patch is now just replacing
> 
>    (eq t (try-completion prefix comps))
> 
> with
> 
>    (seq-every-p
>      (lambda (comp) (= (length prefix) (length comp)))
>      comps)
> 
> which is a nice simple change.  I added some comments to clarify why
> this is correct, too.

Thanks, pushed to master, this and the the following patch (nice 
optimization!).

And with that, time to close.