Selectrum: Selectrum removes empty candidates

[minad]

I am currently looking into the potential upstream changes. I wonder where default completion removes empty candidates, since Selectrum does that. @clemera Can you please point me to the location where this happens?

[clemera]

The relevant function is completion--insert-strings, the empty string has no effect on insertion so it isn't actually removed but not shown and can't get selected as far as I know. Looking through the code I also noticed that only consecutive duplicates are skipped not duplicates in general.

[minad]

How are empty candidates removed then, I don't see it? But you are right, they cannot be selected. And they will still lead to artifacts in the view. So I think it would be safe to delete the (delete "" ...) from selectrum. Duplicates are removed in general in completion-all-sorted-completions additional to the removal of consecutive duplicates.

[clemera]

You are right they aren't removed but I remember there were cases where empty candidates where shown in Selectrum UI (I think it was with empty strings in history variables).

[minad]

Okay, I see. Thank you! But for which commands? I think it would be better to fix this then for each command separately such that no invalid empty candidates are produced?

[clemera]

We could do that but as they can't be selected it should be safe to ignore them generally, what would be better about changing this?

[minad]

Well, it leads to confusion (it confused us or at least me, otherwise we would not discuss). Then I think it is generally a bad idea to work around bugs since this leads to the bug being ignored and never fixed. It also potentially hides bugs the user is unaware of, the workaround can lead to the introduction of new bugs. I think it would be at least good to correct the comment then to something like this WORK-AROUND: Delete empty candidates, which are produced by buggy completion tables, like ..., see upstream bug#... and report a bug for the affected completion functions.

[clemera]

These are good reasons but not deleting them would mean showing empty lines in case of such buggy tables (in case of empty sting history values it might not be considered a bug) which is a bad user experience, which is more important to me in this case. I updated the comment in 3566556

[minad]

Okay, but do you have actual tables which show this behavior?

in case of empty sting history values it might not be considered a bug

Okay, it is not a bug but rather a bug of the default UI which does not allow selection of such items. Even better then. Why not allow selection of those in Selectrum? I mean if the completing-read user passes empty candidates, they obviously want that. Why prevent it?

[minad]

Note that there is also null completion (which I consider actually to be a bug). I think it has only been introduced for ease of implementation since if a default value is passed it will be used instead. But I think I will send a patch upstream which prevents null completion for require-match=t.

[clemera]

The only case I remember we have seen them is with commands browsing history. If empty candidates can't be selected with the default UI I think we shouldn't allow it to work with Selectrum, it also doesn't appear useful to me.

[clemera]

What is meant by null completion?

[minad]

The only case I remember we have seen them is with commands browsing history. If empty candidates can't be selected with the default UI I think we shouldn't allow it to work with Selectrum, it also doesn't appear useful to me.

I don't feel strongly about this. I just say it is both a bug in default completion and Selectrum then. But are right that it is not useful :laughing:

What is null completion?

You can always complete with the empty string, even if require-match=t. I consider this a bug too in default completion. But it is documented behavior. I think Selectrum also allows it.

[clemera]

You can always complete with the empty string, even if require-match=t. I consider this a bug too in default completion. But it is
documented behavior. I think Selectrum also allows it.

I agree it would be better not to allow it, we added support for this since for example you can't exit the auto-insert for elisp snippet which uses this as the exit condition.

[minad]

Okay, but you can always exit with C-g? In Vertico I do allow the empty input exit via vertico-exit-input, but I disallow prompt selection if require-match=t. Vertico is a bit more strict in that respect - seems to work well for me. I think it won't work if you have a default value which is not a candidate, then you have to explicitly use vertico-exit-input. But I could also allow prompt selection in that case I guess.

[clemera]

With C-g you would exit, the emtpy string is used to indicate you are done and proceed with rest of the snippet expansion. It is the only case I'm aware of where it is problematic but there might be more.

[clemera]

You can test by opening an empty elisp file and call M-x auto-insert, the snippet is defined in auto-insert-alist.

[minad]

Thanks, this is good to know. Then auto-insert would need to be patched somehow if require-match=t is made more strict.

[clemera]

Closing this as won't fix, I also think your points are valid but all in all I prefer to keep it this way.

[minad]

Btw, do you think this generalized deferred highlighting would be useful for Selectrum too?

https://github.com/minad/vertico/commit/a962a8b2b682bb99304aa022bb20d66a838fd1ea

It is not super nice to hardcode orderless and the other highlighting functions but at least it is correct for all styles.

EDIT: I believe this is a better solution than orderless-skip-highlighting. We may consider removing that variable once again.

[minad]

I hope we can feed back some of the Vertico experiments into Selectrum :)

[clemera]

You are doing great work on improving Emacs completions, thanks for that! Nice that it is possible to skip it for other styles, too. I guess in practice when using Selectrum you either use orderless or prescient so for now I would also be okay with leaving things like they are but PRs are always welcome. If you are interested you could also become another maintainer of Selectrum so you could upstream your improvements easily yourself.

[minad]

I guess in practice when using Selectrum you either use orderless or prescient so for now I would also be okay with leaving things like they are but PRs are always welcome.

I will see if I can prepare a PR for this. I think it would be better to have a single method to do this instead of orderless-skip-highlighting, since this variable hack just doesn't generalize if you have multiple styles and so on. Te way vertico--all-completions is written could also be upstreamed by adding a third function to completions-styles-alist and by introducing a completion-all-completions-deferred-highlighting function or by adding an optional argument to completion-all-completions, which adjusts the behavior to return candidates + highlighting pattern/closure. Furthermore one may want to adjust completion-all-completions at that point to return the base directly (if the argument is passed) and not like a madman in the last cdr.

If you are interested you could also become another maintainer of Selectrum so you could upstream your improvements easily yourself.

Sure, if you like you can add me. But I am also fine with proposing changes to you and you give them another look before merging. I guess in practice it would not change anything?

[clemera]

and not like a madman in the last cdr.

:laughing: I always forget what the value is about because in Selectrum we just ignore it.

Sure, if you like you can add me. But I am also fine with proposing changes to you and you give them another look before merging. I guess in practice it would not change anything?

I can't do that, @raxod502 can you add @minad to the project? It may not change much in practice but it is less friction in a lot of cases. If we disagree on something we may have a harder time finding consensus but I hope we can still figure it out then like we have in the past.

[minad]

I always forget what the value is about because in Selectrum we just ignore it.

It is actually useful. The length of the prefix if completion boundaries are used. Boundaries are not a bad idea in general.

[clemera]

So for file completions that would be the length of /some/path/ before your INPUT when the prompt is /some/path/INPUT?

[minad]

Yes, that's right. The problem is that you never know the length of this base since it depends very much on the completion style. In vertico initals and partial-completion works too and when you move with the cursor from left to right the candidates will update too.

[clemera]

I see, that is actually a cool feature in some way!

[raxod502]

@raxod502 can you add @minad to the project?

Yep, absolutely---done! I'd give you permission to do it yourself, but unfortunately that's only possible if you have an organization.

[minad]

@raxod502 Thanks!