Actions

[minad]

@clemera There is another maybe more interesting question I would like to hear your opinion on. Can we somehow support actions in consult? You are probably using embark? I looked into it but I think it is quite complex and I had hopes for something simpler. A lot of the complexity is due to the automatic detection mechanism. I don't know what other edge cases embark covers but the basic idea seems relatively simple to me.

Small example:

• There are two action functions which must be wrapped (action-default, action-switch-to-buffer-other-window)
• This wrapping could be done automatically when generating the action map (define-action-map)
• The test function adds the action keymap temporarily as a prefix map to the minibuffer-local-map

;;;  -*- lexical-binding: t -*-

(defvar action-default-command nil)

(defun define-action (name cmd)
  `(defun ,name ()
     (interactive)
     (let ((cand (selectrum-get-current-candidate)) ;; selectrum-get-current-candidates
           (orig-inhibit-message inhibit-message))
       (run-at-time 0 nil (lambda ()
                            (setq inhibit-message orig-inhibit-message)
                            (letrec ((hook (lambda ()
                                             (setq minibuffer-setup-hook (delq hook minibuffer-setup-hook))
                                             (delete-minibuffer-contents)
                                             (insert cand))))
                              (push hook minibuffer-setup-hook))
                            (push 13 unread-command-events)
                            (call-interactively
                                (setq this-command
                                      ,(if (eq cmd :default)
                                           'action-default-command
                                         `#',cmd)))))
       (setq inhibit-message t)
       (top-level))))

(defmacro define-action-map (name &rest bindings)
  `(progn
     ,@(mapcar (pcase-lambda (`(,_ ,cmd))
                 (define-action (intern (format "action-%s-%s" name cmd))  cmd))
               bindings)
     (defvar ,name
       (let ((map (make-sparse-keymap)))
         ,@(mapcar (pcase-lambda (`(,key ,cmd))
                     `(define-key map ,(kbd key) #',(intern (format "action-%s-%s" name cmd))))
                   bindings)
         map))
     (fset ',name ,name)))

(define-action-map buffer-actions
  ("RET" :default)
  ("o" switch-to-buffer-other-window)
  ("C-g" keyboard-quit))

(defun test ()
  (interactive)
  (define-key minibuffer-local-map [C-return] #'buffer-actions)
  (unwind-protect
      (call-interactively (setq action-default-command 'switch-to-buffer))
    (define-key minibuffer-local-map [C-return] nil)))

This somehow seems to work. The downside is that each command must be equipped with actions using such a unwind-protect-keymap dance. Furthermore I don't see how I can programmatically equip functions with actions, for example if I would want to hard-code that find-file gets the file-action-map etc. I don't think this is easily possible using the advice mechanism since completion can also happen in the interactive code, which is executed before the call. This seems to be one of the reasons why embark goes with the auto-detection? The other is probably that embark avoids a lot of manual work by auto-detecting. But I would not be too unhappy if I have to manually associate commands with keymaps. At least if it is possible to do with a oneliner e.g., (add-actions find-file file-actions). If it is not a oneliner, we are back to the counsel approach since we have to write manual wrappers for each and every function.

What do you think? Any ideas? Should I rather embrace embark? Keep actions out of this?

[okamsn]

What are you trying to do that Embark does not already do?

[minad]

There is probably nothing I want to do which embark doesn't already do. I looked at it and it is seems involved. Its code complexity is comparable to selectrum itself. Its goal is to provide full powered actions like ivy on top of e.g. icomplete. To achieve this embark resorts to classify the type you are completing and then binds the appropriate keymap. I am not entirely convinced of this approach. And I also think its a unique approach on top of the basic concept of actions, as I understand them. I would rather prefer something where I have to explicitly associate commands with keymaps as long as this is possible without much code and without writing wrappers.

In the scope of this library here I would like to have the possibility to add actions to commands, as in the commands in the selectrum wiki: https://github.com/raxod502/selectrum/wiki/Useful-Commands#misc-by-lorniu There this is achieved with not much code. I could probably do the same with embark, but then I am tied to embark, which I didn't buy into yet.

Maybe it would be great to have an embark-core which only provides the key binding facilities without the classification mechanism, but I am not sure if this is possible since embark seems to be centered around the classification concept? Maybe the way embark approaches the problem is the right one if you are coming from ivy. And it is indeed natural to have all the actions around automatically if you complete a certain type. My perspective is a bit different since I am used to a more barebone Emacs. This is also where for example the selectrum appeal lies for me. It has its narrow niche and only gives me a better completer without adding keybindings (except very few in the selectrum-minibuffer-map) and without adding any commands etc.

Overall I am just exploring. Motivated by looking at the commands in the selectrum wiki and the commands provided by counsel which are often equipped with useful actions...

[minad]

Another experiment where I am using the minibuffer-setup-hook to bind the action keymap.

;;;  -*- lexical-binding: t -*-

(defvar action-candidate-function #'selectrum-get-current-candidate)
(defvar action-key [C-return])
(defvar action-stack nil)
(defvar action-command nil)
(defvar action-command-maps nil)
(defvar action-category-maps nil)

(defun action-exit (command)
  (let ((candidate (funcall action-candidate-function))
        (orig-inhibit-message inhibit-message))
    (run-at-time 0 nil (lambda ()
                         (setq inhibit-message orig-inhibit-message)
                         (letrec ((hook (lambda ()
                                          (push 13 unread-command-events)
                                          (setq minibuffer-setup-hook (delq hook minibuffer-setup-hook))
                                          (delete-minibuffer-contents)
                                          (insert candidate))))
                           (push hook minibuffer-setup-hook)
                           (unwind-protect
                               (call-interactively (setq this-command command))
                             (setq minibuffer-setup-hook (delq hook minibuffer-setup-hook))))))
    (setq inhibit-message t)
    (top-level)))

(defun define-action (name cmd &optional no-exit)
  `(defun ,name ()
     (interactive)
     ,(if no-exit
         `(call-interactively ,(if (eq cmd :default) 'action-command `#',cmd))
       `(action-exit ,(if (eq cmd :default) 'action-command `#',cmd)))
     ))

(defmacro define-action-map (name &rest bindings)
  `(progn
     ,@(mapcar (pcase-lambda (`(,_key ,cmd . ,rest))
                 (define-action (intern (format "action-%s-%s" name cmd)) cmd rest))
               bindings)
     (defvar ,name
       (let ((map (make-sparse-keymap)))
         ,@(mapcar (pcase-lambda (`(,key ,cmd . ,_rest))
                     `(define-key map ,(kbd key) #',(intern (format "action-%s-%s" name cmd))))
                   bindings)
         map))))

(defun action-category ()
  (completion-metadata-get
   (completion-metadata
    (buffer-substring-no-properties (field-beginning) (point))
    minibuffer-completion-table
    minibuffer-completion-predicate)
   'category))

(defun action-minibuffer-setup ()
  (push (cons action-command (lookup-key minibuffer-local-map action-key)) action-stack)
  (setq action-command this-command)
  (let ((map (or (alist-get this-command action-command-maps)
                 (alist-get (action-category) action-category-maps))))
    (define-key minibuffer-local-map action-key (and map (symbol-value map)))))

(defun action-minibuffer-exit ()
  (let ((state (pop action-stack)))
    (setq action-command (car state))
    (define-key minibuffer-local-map action-key (cdr state))))

(add-hook 'minibuffer-setup-hook #'action-minibuffer-setup)
(add-hook 'minibuffer-exit-hook #'action-minibuffer-exit)

(define-action-map buffer-action-map
  ("RET" :default)
  ("o" switch-to-buffer-other-window)
  ("x" ignore  :no-exit)
  ("C-g" keyboard-quit))

(push '(buffer . buffer-action-map) action-category-maps)
;;(push '(consult-buffer . buffer-action-map) action-command-maps)

[minad]

I looked into embark again and there is also handling of a completion category which is part of the metadata. This is a nice concept. Probably this is what I've been looking for.

I want something which decouples the command from the associated actions. And then I would like "consult" to support this. Ideally I don't have to define the actions directly in consult, but they could be supplied later on by embark or some other provider.

So to conclude - actions should always be provided externally. This is in contrast to how ivy handles actions.

The only thing "consult" should do here is to play nice, i.e., provide completion metadata (like category) etc.

@clemera @okamsn Do you agree?

[minad]

See https://github.com/minad/consult/commit/277c0142d0eef5f55d58700d67ee42eb747643e0 for category support.

consult-buffer is a bit of an odd one. This would need a special category I think.

[clemera]

Embark is really great and it handles probably everything you might want to do with actions. I wouldn't recommend trying to replicate the hard work done there, it handles a lot of edge cases already and also comes with an API which makes it easy to integrate with different completion frameworks.

As you described embark makes use completion metadata this allows to offer the right actions for the current context otherwise you would have to define the correct actions for each command. If one uses read-buffer or read-file-name the metadata reports that you are completing buffers/files so embark can offer the corresponding actions.

I would rather prefer something where I have to explicitly associate commands with keymaps as long as this is possible without much code and without writing wrappers. You can bypass the usual categorization:

(defun example-command ()
  (interactive)
  (let ((embark-overriding-keymap
         (let ((map (make-sparse-keymap)))
               (define-key map "d"
                 (defun example-action ()
                   (interactive)
                   (message "Do something with %s" (embark-target))))
               map)))
    (completing-read "Check: " '("this" "out"))))

Embark will offer you the general actions (like candidate insertion that always make sense like candidate insertion/copy) plus the ones defined in embark-overriding-keymap.

So to conclude - actions should always be provided externally. This is in contrast to how ivy handles actions

Some actions are very command specific but in that case you can create a keymap for them and bind it like shown above. BTW I highly recommend the which-key integration as described on the selectrum and also on the embark wiki.

[minad]

Thank you @clemera for answering here. For me the question here was mainly if I can enhance commands after having them defined, since this matters for consult. And this is different from ivy, I think, where the actions seem to be fixed by the :actions property (I assume it is still possible to overwrite them somehow dynamically).

Regarding consult, I like to avoid dependencies and ideally make this only dependent on Emacs builtins. Maybe we can allow some small enhancements for external library which are detected at runtime.

This means I want to avoid adding actions directly to the library and instead favor an external solution. But nevertheless I want consult to be at least compatible with actions and in principle allow them. Therefore I added the :category property to consult--read.

For more involved cases special keymaps are needed, as you already wrote:

Some actions are very command specific but in that case you can create a keymap for them and bind it like shown above. BTW I highly recommend the which-key integration as described on the selectrum and also on the embark wiki.

Is it possible in embark to associate keymaps (or maybe only add a few bindings) with commands like I've shown in the example above? Maybe this is possible using some embark-setup-hook, if something like this exists?

(add-hook 'minibuffer-setup-hook
          (lambda ()
            (push (cons action-command (lookup-key minibuffer-local-map action-key)) action-stack)
            (setq action-command this-command)
            (let ((map (alist-get this-command action-maps)))
              (define-key minibuffer-local-map action-key (and map (symbol-value map))))))

[clemera]

That would be a great feature, you could do the following (though checking for this-command does not always work reliably, see here):

(defvar my-action-map
  (let ((map (make-sparse-keymap)))
    (define-key map "d"
      (defun example-action ()
        (interactive)
        (message "Do something with %s" (embark-target))))
    map))

(defvar action-maps-alist
  '((example-command . my-action-map)))

(add-hook 'minibuffer-setup-hook
          (defun my-setup-actions ()
            (when-let ((map (cdr (assoc this-command action-maps-alist))))
              (setq-local embark-overriding-keymap (symbol-value map)))))

(defun example-command ()
  (interactive)
  (completing-read "Check: " '("this" "out")))

[minad]

Cool that its possible to do this with embark! My only concern with embark is its complexity. I looked at it to understand how everything fits together. There are some design decisions I would have to understand better to fully grasp this, for example why transient maps are used. But it also depends very much on the point of view, I am not very accustomed to actions so I am not using most things as of now. If you are coming from ivy or helm you have totally different expectations I guess.

Yes... this-command is not reliable since it is overwritten everywhere. But there is also last-command, this-original-command and last-repeatable-command to help us out. Who is responsible for this holy mess? Stallmann himself?

[clemera]

To add to this there are also real-this-command and real-last-command which are more reliable since they should never be rebound like this-command or last-command but this doesn't help in this case as we would really need something like minibuffer-this-command :smile:

[minad]

I knew there were more, but overlooked them when I did C-h v just now :facepalm:

[minad]

Please don't propose adding another this-command variable on the emacs-dev list!1!!

[clemera]

Actually I think it would make sense to have a variable which records the command which is currently running (ignoring minibuffer commands) as otherwise we have to create one as mentioned in the linked selectrum issue. Ivy has the same problem and uses a keyword arg to pass this information. But don't worry I'm to lazy to propose that ;)