bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting

[João Távora]

On Fri, Aug 13, 2021 at 1:22 PM Daniel Mendler <mail@daniel-mendler.de> wrote:

> It is important to keep this property since this will preclude many bugs
> due to string mutation.

I am aware of this, of course.  Can you give examples of these "many bugs"?
Perhaps other than the one I already described and addressed?

> By separating the filtering and mutation
> (highlighting, scoring) my patch addresses the problem at hand in the
> proper way.
>[ ... ]
> Mutation would be a reasonable choice here if the problem could not be
> solved in a proper way.  But in fact it can be solved in a proper way
> without mutating the strings at all as my patch shows.

"proper" is just an reasonably empty adjective.  There are different ways to
go about this, of course.  What's "proper" and isn't is hard to debate
objectively.

> This solution is much more ad-hoc and you still mutate the string which
> is not allowed.

It's also difficult to debate "ad-hoc" or not.  If you've studied the
problem, what
makes you say that mutating the string (in this case, adding a
'completion--style-face' property to it) is not allowed? What negative things
would derive from it.

> > The advantage that I see is that those adaptations apart, it is a small
> > localized and effective change.
>
> Note that your idea also does not address the other issues which are
> addressed by my patch.

That's for sure.  My patch idea addresses only that single problem.
I think this is a good property of patches: to solve one thing, not many.

We can make more patches to solve other problems, once we
identify them clearly.

> The new API `completion-filter-completions`
> returns data which hasn't been available before, e.g., the end position,
> which cannot be fixed given the existing API.
>
> >> The main problem is that `completion-all-completions` allocates all the
> >> strings every time the completions are filtered.  This is the same
> >> performance issue you encountered in fido-mode/icomplete-mode.
> >
> > OK. I encountered at least two different performance problems there, with
> > quite different causes. So let's stick to the string-allocation problem.  
> > Post
> > a code snippet that demonstrates the problem the way you see it/experience 
> > it?

Look, one needs to evaluate things quantitively. Your patch is not
to Vertico, it's to Emacs. I'm concerned with changes to Emacs and their
effect on all completion frontends.  So trying Vertico isn't very useful.

If you're solving a performance problem (and it seems that you are, among
other things) we really need benchmarks, a description of an experiment whose
results can be reproduced independently. It's the normal scientific method.

Something like:

"before my patch, this code takes 123 seconds to run, after my patch it
takes 12."

> >> The second problem addressed by the new API
> >> `completion-filter-completions` is that `completion-all-completions` is
> >> limited in what it can return.  For example it cannot return the end
> >> position of the completion.
> >
> > And why is this a problem? Can you post an example of something you'd
> > like to do, but can't?  Regardless, it does seem indeed like a "second" 
> > problem
> > (as you state) so perhaps something that can be addressed separately.
>
> Please look at the FIXMEs in minibuffer.el which address this.
> Currently only the beginning position of the completion boundary is
> returned, which is only half of the information.

OK. It does seem like a separate problem, so maybe open a new bug for it?

João Távora