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

[Dmitry Gutov]

On 01/11/2023 20:47, João Távora wrote:
> On Tue, Oct 31, 2023 at 8:52 PM Dmitry Gutov <dmitry@gutov.dev> wrote:
>
> > It seems like the only code that would be concerned with it are
> > completion styles that also do sorting, or completion tables that would
> > do similar things to this "with quoting" business. But I'm not aware of
> > any other examples of the latter aside from what is inside Emacs itself.
>
> If orderless (which I've never tried), does some kind of scoring of
> completions, it probably also needs the same complications of flex.

Turns out, Orderless doesn't do any scoring or sorting. Only filtering.

> > > > Anyway, have you looked into what it would take to solve it?
> > >
> > > No, naively, I just think it's a similar situation of display and business
> > > logic being mixed up.  Presumably the quoted stuff is just for insertion
> > > (and display?), and the unquoted stuff is what patterns/scoring should
> > > operate on.
> >
> > Apparently it's good for insertion, but according to that comment inside
> > the function, the unquoted stuff might actually be better for display.
>
> No idea what the unquoted stuff is for, so I haven't really tested it.

A couple of scenarios:

First:
1. sudo mkdir /home/${USER}-foobar
2. C-x C-f /home/${USER} TAB ; it shows both directories inside home as

  ${USER}-foo/
  ${USER}/

Second:
1. mkdir ~/examples/test\ test\ test/
2. mkdir ~/examples/test\ test/
3. M-x shell
4. In the shell buffer, type 'ls ~/examples/test\ ' and TAB. See:

  test\ test/
  test\ test\ test/

In the current implementation, both the inputs and the text in the 
completions buffer that we see, are "quoted". The "unquoted" versions 
would be the directory name with the variable substitution performed, 
and the directory names without backslashes.

> > I'm not 100% clear which of the versions is better for
> > scoring/highlighting, but apparently the unquoted one.
> >
> > > But, IMO, there's no need to tackle it right now.
> > >
> > > If the thing holding you back from the lazy-hilit-2023-v4.patch is the
> > > completion-score propertization, I can move it to the sorting step
> > > in a future v5 and add spread the completion--unquoted thing a little
> > > bit more.
> >
> > I think that's the main blocker, yes.
>
> Alright, here goes v5 then, with this change.  Note I've implemented
> this unquoted thing which kicks in in C-x f but I haven't actually
> seen  any strings that have different "quoted" "non-quoted" versions.
>
> The performance of the three main patches as measured in yet
> another machine:
>
> ;; C-h v
> ;;
> ;; Daniel+Dmitry: 0.696340454545
> ;; lazy hilit v4: 0.692849642852
> ;; lazy hilit v5: 0.683088541667
> ;;
> ;; completing-read
> ;;
> ;; Daniel+Dmitry: 0.590994909091
> ;; lazy hilit v4: 0.586523307692
> ;; lazy hilit v5: 0.586165466667
>
> Nothing unexpected.

Confirm. The "property allocation" spikes are gone too.

> So if you're satisfied with the general design now, maybe
> we should start looking at finer details, docstrings, style,
> etc.

LGTM overall, and I see that you compressed the sorting code a little.

Both quoting/unquoting scenarios also seem to work as expected (for 
highlighting, that seems to be thanks to completion--twq-all applying 
the faces eagerly anyway).

Though given the examples (and I think others should be similar) it 
wouldn't be an end of the world if scoring didn't really work for them 
-- filtering should have already done most of the job. All of this is to 
say that any new 3rd party completion styles, even those that do 
sorting, would be okay without knowing about this text property.

Some minor nits for the patch:

> +Completion-presenting frontends may opt to bind this variable to
> +non-nil value in the context of completion-producing calls (such
> +as `completion-all-sorted-completions').  This hints the

I suggest mentioning `completion-all-completions' instead, as it is more 
often used directly by the frontends.

> +responsible this fontification.  The frontend binds this variable

responsible for

> +hint and greedily fontify as usual.  It is still safe for a

"fontify eagerly"? I think that's a more common term than "greedily".

> +  "Used by completions styles to honouring `completion-lazy-hilit'.

"to honour", or "styles honouring"

> +(defun completion--flex-score (str re &optional dont-error)

Looks like the third argument is unused in both callers. I think it was 
intended for compose-flex-sort-fn.

> +see) for later lazy highlighting"

Missing period.

> +                      ;; Lazy highlight not requested, so strings are
> +                      ;; assumed to already contain `completion-> score'
> +                      ;; (and highlighting) and we can freely destroy
> +                      ;; list.

Perhaps drop the last two lines, since IIUC the list can be 
destructively sorted in both cases, lazy highlighting or not.

I guess we should wait a few days to see if anyone has more comments, 
and then install this?