Add better keybindings for consult-completing-read-multiple

[minad]

Follow up on #352

Relevant for consult-vertico/selectrum:

• TAB - Select/deselect
• RET - Submit and exit

cc @bdarcus (You may want to give the basic version a thorough test for now with bibtex-actions)

[bdarcus]

For reasons I can't yet determine, I'm having problems testing the merged version!

My test script doesn't appear to update consult correctly, and if I just add this to my doom config (after an update), I get a Wrong number of arguments error when I select a candidate.

[minad]

I changed the consult--completion-refresh-hook to take an argument. This could be an issue in case your update didn't work well. If it is something else I appreciate a stack trace.

[bdarcus]

I just updated again, which pulled in a subsequent commit, and it works now; that error goes away.

I guess this is related to earlier discussions, but ...

Except now testing embark-act, no errors, but nothing is returned when I select multiple candidates.

Selecting just one does work.

But other than that, looks good!

[minad]

@bdarcus

I just updated again, which pulled in a subsequent commit, and it works now; that error goes away.

Great. It was probably an update issue.

Except now testing embark-act, no errors, but nothing is returned when I select multiple candidates.

Yes, this is the same as in vertico-crm. Support for multiple candidates must be implemented on the side of Embark.

I also added the enhanced keybindings now (TAB/RET) for Vertico and Selectrum in 4e553d5.

[bdarcus]

The keybindings work well. Can you not add the hint to use TAB to select the candidates, as you had in vertico-crm?

[minad]

Yes, I could add the hint. On the other hand Consult also doesn't show narrowing hints, e.g. --- Buffer [b] ---. I think it is sufficient to document this. "Select multiple" at least indicates that selection is possible. Btw, does your has:pdf filter functionality work as expected now?

[bdarcus]

OK.

Btw, does your has:pdf filter functionality work as expected now?

Yes it does! Forgot about that ...

I guess one thing you can't do is the TAB/shift-TAB thing?

One other little thing:

I notice the face for the selected candidates does not extend the width of the window, while vertico-current does.

Is that you, or the theme?

[minad]

I guess one thing you can't do is the TAB/shift-TAB thing?

I could but I think it is better to keep it simple for now. You can try this consult-vertico--crm-select instead of the current version.

(defun consult-vertico--crm-select ()
  "Select/deselect candidate."
  (interactive)
  (let ((content (minibuffer-contents)))
    (run-at-time 0 nil (lambda ()
                         (insert content)
                         (vertico--exhibit))))
  (when (let ((cand (vertico--candidate)))
          (and (vertico--match-p cand) (not (equal cand ""))))
    (vertico-exit)))

I notice the face for the selected candidates does not extend the width of the window, while vertico-current does.
Is that you, or the theme?

Yes this is due to how consult-completing-read-multiple works. I think it is not that bad since then you see the extended current background when scrolling over the already selected candidates.

[bdarcus]

Yes this is due to how consult-completing-read-multiple works.

OK.

I think it is not that bad since then you see the extended current background when scrolling over the already selected candidates.

If not possible, so be it, but I think better would be to distinguish that with the styling of the face itself, rather than the relative width.

I don't really understand the Emacs styling system though; what's possible, and what's not.

In the Doom-one theme, they appear to be exactly the same gray background, so it looks odd for both reasons: because the theme has no distinction between the faces visually, and because one is shorter than the other. So I end up with:

[minad]

This is an oddity due to your special candidate strings I think. If you compare, describe-face should work well. I am not sure if there is anything we can do about this. I am adding the face to the entire candidate string.

[bdarcus]

OK, I'll experiment.

[bdarcus]

@minad do you have any suggestions for how I fix this on my end?

Here's the key piece of code, I imagine.

          (propertize candidate-main) " "
          (propertize candidate-suffix 'face 'bibtex-actions-suffix) " "
          (propertize candidate-hidden 'invisible t)))

So I have a face for the "suffix" part, but adding :extend t does not fix it.

I do not have a face for the main part of the candidate though. Should I add one? If yes, with what definition?

[iyefrat]

(hopefully this thread is new enough that commenting here isn't bad form)

Why were the new keybindings removed in 49b6442?

[minad]

@iyefrat Sorry, the discussion is super scattered over vertico, bibtex-actions and consult. See here for the reasoning emacs-citar/citar#144 (comment). It is not worth the complication in the completion UI integration code. Actually I like the simplicity of the UI, just pressing RET. Only small downside is the double RET in the end, but I don't mind it.

A cheap solution is this one:

(define-key vertico-map [S-return] #'+vertico-double-ret)

(defun +vertico-double-ret ()
  (interactive)
  (run-at-time 0 nil #'vertico-exit)
  (vertico-exit))

Probably I should add a consult-crm-hook or a consult-crm-map such that one can easily setup this keybinding.

EDIT: Added consult-crm-map. You can use this:

(define-key consult-crm-map "\r" #'+vertico-crm-exit)
(define-key consult-crm-map "\t" #'vertico-exit)

(defun +vertico-crm-exit ()
  (interactive)
  (run-at-time 0 nil #'vertico-exit)
  (funcall #'vertico-exit))

[iyefrat]

Gotcha, thanks!

[minad]

Added snippet to the wiki https://github.com/minad/consult/wiki#convenient-keys-for-consult-completing-read-multiple

[bdarcus]

I think, FWIW, the previous behavior, and what was in the vertico-crm prototype, is better, in part because if a user now does hit TAB, they will get something like this, which will look to many like a bug (it's not, but ...).

Perhaps we should add that to the vertico module, @iyefrat?

[minad]

@bdarcus The snippet from the consult wiki should fix this.

[iyefrat]

@minad consult doesn't seem to recognize the bindings on consult-crm-map, does it work on your end?
Also I think it would be better if the selected candidate remained the same after pressing tab. I'll update the snippet after I figure out how to do that.

@bdarcus after all this is sorted out I'll add it to the module.

[minad]

consult doesn't seem to recognize the bindings on consult-crm-map, does it work on your end?
Also I think it would be better if the selected candidate remained the same after pressing tab. I'll update the snippet after I figure out how to do that.

This is not easily possible I think. It requires a lot of UI specific modifications. I would like to keep it now like this and defer the issue until we find something better for acting on multiple candidates at once (oantolin/embark#253).

[iyefrat]

@minad fair enough, but does adding the snippet work on your machine? When I add the snippet the keys lead to the usual vertico commands instead of what's on consult-crm-map.

[minad]

@iyefrat Potential fix in 821df5e

[iyefrat]

@minad yep, works!