[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Improvement proposals for `completing-read'

From: Daniel Mendler
Subject: Re: Improvement proposals for `completing-read'
Date: Thu, 8 Apr 2021 11:01:42 +0200

On 4/7/21 11:26 PM, Stefan Monnier wrote:
What I meant is that I can't think of many situations where
we specifically want no history (which is where t is used) as opposed to
the many situations where we simply don't care about history (in which
case nil works just as well).

IIRC the t case for read-from-minibuffer was added for the needs of
`read-passwd` where we really don't want the history, for security reasons.
I don't know of any other use case yet.

There is one use case I found a bit useful - if you are browsing a history itself with `completing-read` then you may want to disable writing to the given or another history at the same time, such that the completion you are doing is not side-effecting.

I would also like if `completing-read` is consistent with `read-from-minibuffer` in this case, since this lead to actual confusion, where the false assumption has been made that `completing-read` also accepts `t`.

Proposal 3: Sort file names by history
Currently we're not even close to that.  So I think the first step is to
fix the code to pay attention to boundaries.  My comment was just
pointing out that this can be done right now a simple bug fix without
any extra information.

Okay, I also have the code for that approach. It is a simple bug fix as you say. I dropped it from Vertico for my file name sorting for performance reasons and since I wanted to do better in the special important case of files. But no problem to prepare patches for both and fix the boundaries case first.

`flex` without sorting is *much* less usable, in my experience.
The direction of `flex` is actually to replace filtering with sorting.
E.g. I have a local completion style which I call "forgiving" because it
tries to accommodate typos in the input, so basically every member of
the completion table always matches, only its sorting position is
affected by the input text.  So if you removing sorting, there's
nothing left.

I agree with you that the current way style-based sorting works is
problematic, but I think the principle of style-based sorting is sound
because we don't want this to be specific to a UI nor to a backend.

Okay, I see. I am actually not using the flex style since I find it too slow and generating too many matches. But in the Emacs completion implementation it will only be triggered if the earlier styles do not match so maybe that is okay.

I am not using flex since I am using something else - the orderless style. "regexp1 regexp2 regexp3" matches the regexps in any order. Furthermore I am using "style dispatchers", "regexp flex~ !not literal=", where the "flex~" token is used to generate a flex regexp, etc. Maybe we will see the orderless package in ELPA at some point.

Proposal 5: Forbid the null completions for `REQUIRE-MATCH=t'
Okay, I think it is reasonable to gather some data regarding the extent of
the problem. But then I would strongly prefer to just fix the issue. That
being said, it was not obvious to me that you can actually force require
match by writing a proper completion table which properly handles the
`test-completion` action.

No, you can't force require match this way.  What you can do this way is
to re-loosen the match so as to accept the empty-string again like in
the old (i.e. current) code.

Thanks for clarifying.

Yes, that is right. If you care about the actual candidate you get from
a set of duplicates, you must use a completion UI which can handle
duplicates. If you use an UI which cannot handle this (which rather
completes) you cannot get the distinction.

I think there is a real need for this if you look at the Swiper command and
you want to navigate across equal lines. Then there is the
ivy-occur/embark-export/embark-collect command which allows to export the
lines to an occur buffer where you can again navigate over the duplicate
items. Given how everything is implemented internally (list of candidates,
filtering via completion-all-completions etc), it feels like an unnecessary
restriction to remove duplicates. It would be more natural to just
allow them.

As long as we can offer some way (that's not too cumbersome) to select
between the different cases using things like self-insert-command + RET,
I'm fine with it.  IOW I think it require reifying the "subtle
differences" as some kind of optional extra text.

Okay, maybe I can come up with something. This will require experimentation. The duplicates could be deduplicated at the UI level by appending some index for example "candidate (1)", "candidate (2)", ...

I suspect "pass the small subset of candidates once again through
`completion-all-completions'" can be tricky to do (e.g. for things like
file-name completion with `partial-completion`).
In `vertico--highlight` I am checking the length of the result. If it is not
equal to the original length, the UI simply displays the original
candidates. This is an ugly hack. Maybe it would be better to have
a `completion-style-operation` variable which can be either `'both`,
`'filter` or `'highlight`?

I'm not really interested in hacking a slightly better solution, so
I think your current hack is good enough: the way I see it, the
replacement of `completion-all-completions` should return
non-highlighted candidates along with some extra opaque data, and then
a separate new function can take one of those candidates, together with
that opaque data (plus some additional optional args) to add highlight
one candidate at a time.

This way, the highlighting in *Completions* could be applied
incrementally via jit-lock, and the "additional optional args" should
make it possible for the UI to have control over the color scheme
being used.

One thing to consider - maybe returning the highlights as some extra data is not actually faster than simply propertizing the strings? I guess you only save the allocations of the strings? The idea of the proposed hack is really to avoid the entire work of computing the highlights, since it is mostly unnecessary and slow if you filter many candidates and display only a small subset. I would therefore go with the simple solution first since this provable solves the issue. But using jit-lock in buffers has appeal.

Daniel Mendler

reply via email to

[Prev in Thread] Current Thread [Next in Thread]