Add previews to consult-completion-in-region

[oantolin]

This adds previews to consult-completion-in-region. The preview code was removed from consult--yank-read, put into a little consult--region-state function and then used both in consult--yank-read and consult-completion-in-region.

(I ran into a small snag: consult--read cannot be passed a completion table function, because it expects functions to be async collections. I think I have a good workaround for that.)

[oantolin]

One case in which completion in region does receive a function as collection is path completion in eshell.

[oantolin]

Mmh, I think we decided that in the file case we would use read-file-name so that you can complete a long path in a single go. I think I should bring that feature back, even though there would be no previews for that case.

[oantolin]

It's worth losing the previews in the file case, since read-file-name really is much smoother for that case. So I brought back the special casing for the file category, and while I was at it changed something about the old function I never liked: in the file case it always inserts absolute paths, even if you started out completing a relative path. So now I have it insert the same type of path, whether absolute or relative, as the initial portion you type before triggering completion.

[minad]

I tested this and it seems to work well. I am a bit worried about the collection trick you are using, since the metadata of the collection is lost. Furthermore is it possible to add support for completion-cycle-threshold or wouldn't you want to have this here? This could probably be done by redirecting to completion--in-region if there are fewer elements than the threshold?

Overall I am not very happy with capf and I am using this intricate company setup as a replacement, which I would like to get rid of (https://github.com/minad/corfu, #175 (comment)). I think a good setup would look somehow like this:

• For fewer than threshold items use TAB cycling
• For more elements show a completion interface. This could be as we have in this PR the minibuffer completion plus the Consult overlay preview. But I think I would prefer it even more to have a company-style popup in that case. Maybe it is somehow possible to add support for an overlay popup to Selectrum.

[minad]

Regarding the collection hack and the preview I think the cleaner solution would be to still use completing-read or read-file-name but wrap it with consult--with-preview. This is the same approach taken by consult--read and consult--prompt. This way you avoid working around the consult--read API but you can still augment the minibuffer with previews. I think you should also get preview for the file name this way.

[oantolin]

Using consult--with-preview directly does sound better, I'll give it a shot.

I'll also try your cycling idea by calling completion--in-region. If you use completion-cycle-threshold with default completion or icomplete it already works, but not quite as you said, since the minibuffer does open anyway.

[oantolin]

I just noticed that consult--default-completion-candidate has a little inconsistency with file completion: it sometimes returns the minibuffer-contents which are a full path, and sometimes it returns (car (completion-all-sorted-completions)) which is just the final component of the path! I'll fix that too.

[minad]

Yes, I didn't implement this correctly since I did not use the function for file completion as of now. But I am aware of this since your Embark function looks differently.

[oantolin]

I fixed consult--default-completion-candidate to always return what we called "full candidates" in Marginalia. I also switched to calling consult--with-review directly, but I'm worried I'm not using consult-preview-key correctly. Do I need to do something extra so consult-config works with consult-completion-in-region now that I am not calling consult--read?

[minad]

You would have to access the fields 'consult-completion-in-region -> :preview-key of consult-config if you want to have the ability to overwrite the key individually. Alternatively we could introduce an extra customizable variable. I would probably prefer to use consult-config since this is more aligned with the rest of Consult.

Did you also try to get this cycle threshold to work?

[oantolin]

Did you also try to get this cycle threshold to work?

Not yet, since it does already work with default completion (except that the minibuffer does open anyway). 😛

[oantolin]

Wait, I take it back, TAB does something really weird, so no, tab-cycling doesn't work yet with default completion.

[oantolin]

Oh, tab cycling does work, but not if you press tab right away, you need to press something else first. If you start with tab it scrolls the window. I have no idea why!

[oantolin]

OK, cycling is done! I think that's everything.

[minad]

Cool! Seems to work almost. I gave it a quick test. There seems to be a small problem with file completions, for example in eshell - where cycling sometimes does not work. Maybe the all-completions length check is not applicable to files and needs something more sophisticated? Did you also observe some issue there?

[oantolin]

Cycling does seem to work for me for file completion in eshell. What exactly happens when you try it?

[minad]

It could be some local misconfiguration. I will try a bit longer and then I can produce a test case. It seems to work mostly, but in some scenarios it fails.

[minad]

Hmm, It somehow behaves weird for file completion when I compare it with default completion (I have completion-at-point configured such that it uses the default completion, no selectrum). It completes full file paths instead of path elements as in completion--in-region. But maybe the current version also misbehaves like this?

[oantolin]

Oh, completing entire paths in one go instead of completing them one component at a time was done on purpose! This was suggested by @clemera, and is what Selectrum's completion in region function does. I like that behavior too. When you want to complete a long path, it feels much faster to navigate in the minibuffer to it than to press C-M-i for each component.

[oantolin]

This is why I use read-file-name. Otherwise we could stick to just completing-read. That will do one path component at a time.

[minad]

Oh, completing entire paths in one go instead of completing them one component at a time was done on purpose! This was suggested by @clemera, and is what Selectrum's completion in region function does. I like that behavior too. When you want to complete a long path, it feels much faster to navigate in the minibuffer to it than to press C-M-i for each component.

Ah okay, I just stumbled over this because my muscles are too stupidly trained for TAB TAB TAB shell completion. Maybe I have to make this configurable 😆 I will merge this at some point but I have to fiddle a bit more with it. Overall it looks good.

[minad]

Note that I didn't really use this function before, and I am now considering it as replacement for company. Therefore I am coming up again with these old discussions. Sorry for that.

[minad]

I merged your PR with some very minor restructuring and I added consult-completion-in-region-styles, which is nil by default. Please check!

[oantolin]

Looks good! Thanks for rewriting the throw/catch thing into an if. At the time I wanted to handle the unique completion case in consult-completion-in-region, but your solution of delegating that case to completion--in-region does exactly the same thing. I should have done it this way from the start.

Also, you added the consult-completion-in-region-styles variable with exactly the meaning I would have chosen, not with the meaning the analogous variable in Selectrum has (which to be honest, I find a little odd).

[clemera]

Also, you added the consult-completion-in-region-styles variable with exactly the meaning I would have chosen, not with the meaning the analogous variable in Selectrum has (which to be honest, I find a little odd).

I thought it might make sense to use the default value to not surprise users. This is how default completion behaves (letting you complete symbols with point within the symbol for example). But I have set it to nil myself (to use all-completions) maybe we should change the default.

[minad]

@clemera It would probably be good if we would align consult-completion-in-region and selectrum-completion-in-region, such that they are mostly exchangeable? Additionally Consult adds the previews now thanks to @oantolin.

[oantolin]

I wasn't talking about the default value of the variable, @clemera. What I meant is that in Selectrum, selectrum-completion-in-region-styles is only used to check if you have 0 or 1 candidates. If there are 2 or more, you get prompted with your usual Selectrum configuration and selectrum-completion-in-region-styles is no longer relevant.

Also, if selectrum-completion-in-region-styles is nil, it means to use all-completions, which I believe does not use completion-styles at all. In Consult, having consult-completion-in-region-styles nil means to use whatever value of completion-styles the user currently has.

[clemera]

What I meant is that in Selectrum, selectrum-completion-in-region-styles is only used to check if you have 0 or 1 candidates. If there are 2 or more, you get prompted with your usual Selectrum configuration and selectrum-completion-in-region-styles is no longer relevant.

I don't know what you mean, the styles should be used for the initial filtering regardless of how many candidates you have.

Also, if selectrum-completion-in-region-styles is nil, it means to use all-completions, which I believe does not use completion-styles at all.

That is right, it is a feature not a bug ;)

[clemera]

It would probably be good if we would align consult-completion-in-region and selectrum-completion-in-region, such that they are mostly exchangeable?

I'm not sure I think they will digress further in the future because I have some plans to offer overlay completion for it.

[oantolin]

[...] selectrum-completion-in-region-styles is only used to check if you have 0 or 1 candidates

I don't know what you mean, the styles should be used for the initial filtering regardless of how many candidates you have.

Oh, sorry, I missed the argument cands to selectrum--read! So if you call selectrum--read with a list of candidates and a :minibuffer-completion-table keyword argument, the list of candidates is used instead of the completion table? (I bet the table is just set, but Selectrum doesn't actually use it, is that right?)

Since I hadn't seen the cands argument, I had assumed that the :minibuffer-completion-table was being used.

Also, if selectrum-completion-in-region-styles is nil, it means to use all-completions, which I believe does not use completion-styles at all.

That is right, it is a feature not a bug ;)

Oh, really?

[oantolin]

One thing I don't like about calling selectrum--read on cands is that you are locked in to your initial input. In consult-completion-in-region, I call completing-read on collection, passing the input as the initial input. That way you can delete part of the initial input if you need to.

[clemera]

I bet the table is just set, but Selectrum doesn't actually use it, is that right?

The table is only passed so metadata can be queried from it as the candidates are a list of strings at that point.

Oh, really?

I didn't consider it common that you want to use the same styles for the initial filtering you use regularly. That was the point of adding the option after all, but maybe I'm wrong it might make sense to do what consult does and additional allow a filter function which can be all-completions for example.

[clemera]

That way you can delete part of the initial input if you need to.

Ah okay that is nice indeed. Though it seems the completion-in-region-styles are used in consult for the whole completion session? I only added it for the initial filtering as for the completion itself I would think one would want to use the regular styles.

[clemera]

Though it seems the completion-in-region-styles are used in consult for the whole completion session?

You probably assumed we are doing the same in Selectrum? I understand your surprise that using all-completions is a feature then.

[oantolin]

Though it seems the completion-in-region-styles are used in consult for the whole completion session? I only added it for the initial filtering as for the completion itself I would think one would want to use the regular styles.

When @minad proposed the variable my initial reaction was that it wasn't necessary because I also thought one would want one's regular styles, but he says for completion in region he prefers more weight to prefix matches and to hit tab to add text, which sounds pretty reasonable (I do stick with only orderless, everywhere).

[oantolin]

You know, I am really loving this function now that I've been using it for a few days! The preview makes it so I most often don't even need to pop up the completions at all (I'm currently on a no-visible-completions-until-needed kick). Pretty often I don't even glance at the minibuffer.