Re: Improvement proposals for `completing-read'

[Daniel Mendler]

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