Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

M-x describe-face still uses standard completion interface #53

Closed
raxod502 opened this issue Apr 17, 2020 · 8 comments
Closed

M-x describe-face still uses standard completion interface #53

raxod502 opened this issue Apr 17, 2020 · 8 comments

Comments

@raxod502
Copy link
Member

describe-face uses completing-read-multiple which uses read-from-minibuffer directly, so it bypasses Selectrum entirely. This is undesirable.

@clemera
Copy link
Collaborator

clemera commented Apr 18, 2020

Yeah it seems completing-read-multiple should be replaced by a completing-read compatible variant in Emacs proper. But for that completing-read would need to be able to select and return multiple candidates. If we could convince Emacs devs to allow this feature this would greatly improve the situation for minibuffer frameworks which all implement their own version of this feature.

@DarwinAwardWinner
Copy link

For what it's worth, I have an experimental hack that reimplements completing-read-multiple using completing-read, which in turn allows you to use any completing-read-function with it: https://github.com/DarwinAwardWinner/crm-custom

@clemera
Copy link
Collaborator

clemera commented Apr 18, 2020

Great, I think we could use a similar approach for selectrum. I think RET should just exit as usual and typing M-RET for example select one candidate and continue.

@clemera
Copy link
Collaborator

clemera commented Apr 18, 2020

Based on crm-custom here is a slightly adapted unfinished but working prototype, pushing new elements with , and finish with RET. If you select just one candidate there is no difference to normal completing-read behavior. One thing that is missing is to indicate to the user somehow that multiple values can be selected:

(defun selectrum-completing-read-multiple
    (prompt table &optional predicate require-match initial-input
            hist def inherit-input-method)
  (let* ((exit nil)
         (selectrum-minibuffer-bindings
          (append selectrum-minibuffer-bindings
                  (list (cons "RET"
                              (lambda (arg)
                                (interactive)
                                (setq exit t)
                                (selectrum-select-current-candidate arg)))
                        (cons "," 'selectrum-select-current-candidate)))))
    (cl-loop
     ;; Initialization stuff
     with return-list = nil
     with next-value = nil
     with def-list = (s-split crm-separator (or def ""))
     with def-list-no-empty-string = (remove "" def-list)
     with def-text = (when def-list-no-empty-string
                       (concat "(" (mapconcat #'identity def-list
                                              crm-custom-separator) ")"))
     ;; Need to clear this
     with def = nil
     ;; Save original prompt and construct prompt with defaults
     with orig-prompt = prompt
     with prompt = orig-prompt
     ;; Pre-expand completions table
     with table = (delete-dups
                   (nconc def-list-no-empty-string
                          (all-completions "" table predicate)))
     with predicate = nil
     do (setq next-value
              (completing-read prompt
                               table
                               predicate
                               require-match
                               initial-input
                               hist
                               def-list
                               inherit-input-method))
     if exit
     return (or return-list (list next-value))
     ;; Fold empty string to nil
     if (string= next-value "")
     do (setq next-value nil)
     ;; Empty input on first prompt returns result
     if (null next-value)
     return (or return-list def-list)
     ;; Collect selected item and go again
     else
     collect next-value into return-list
     ;; Remove selected item from stuff, and unset initial things,
     ;; before looping around again.
     and do (setq
             prompt (concat orig-prompt "("
                            (mapconcat #'identity return-list
                                       crm-custom-separator)
                            crm-custom-separator)
             table (delete next-value table)
             initial-input nil))))

@DarwinAwardWinner
Copy link

One of the key aspects of crm-custom is that in theory it works with any completing-read-function. I think it would be ideal to preserve that, to keep it orthogonal to any specific completion framework.

raxod502 added a commit that referenced this issue Apr 19, 2020
@raxod502
Copy link
Member Author

I think the whole crm-separator thing is kind of a hack, and we might as well implement multiple selection properly as a first-class citizen since it is easy in the Selectrum interface.

@clemera
Copy link
Collaborator

clemera commented Apr 19, 2020

Works great! Would be nice to somehow indicate when multiple selection is possible (using the prompt maybe?) .

@raxod502
Copy link
Member Author

Done. Improvements welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants