embark icon indicating copy to clipboard operation
embark copied to clipboard

Paging with which-key indicator

Open antrmn opened this issue 2 years ago • 6 comments

I'd love to use which-key as indicator when using embark-act and embark-become. That would make a consistent interface when trying to select a keybindings.

However, when I do embark-act on a symbol this is how the echo area looks like:

Act on symbol 'quits' ... (1 of 2) [C-h paging/help]

Problem is, if I enter <C-h> I get the embark-bindings-in-keymap command which displays all possible actions on point in a minibuffer window.

Normally, you could navigate across paging by doing <C-h n> and <C-h p>. There does not seem to be a trivial way to set up a custom command for C-h in the (general) embark keymap. How could one achieve this?

Thanks in advance

antrmn avatar Jul 11 '23 00:07 antrmn

I'd have to look into this again but I think I never managed to get the which-key indicator to do pagination correctly.

oantolin avatar Jul 11 '23 01:07 oantolin

Are you using the embark which-key indicator from the embark wiki? Or is it the which-key support for transient keymaps?

oantolin avatar Jul 11 '23 19:07 oantolin

Thank you for the help!

I'm using the indicator from the wiki. Here's a minimal configuration:

(use-package which-key
  :demand t
  :ensure t
  :custom (which-key-mode t))

(use-package embark
  :demand t
  :ensure t
  :bind (("C-." . embark-act)
	 ("M-." . embark-dwim)))



  (defun embark-which-key-indicator ()
  "An embark indicator that displays keymaps using which-key.
The which-key help message will show the type and value of the
current target followed by an ellipsis if there are further
targets."
  (lambda (&optional keymap targets prefix)
    (if (null keymap)
        (which-key--hide-popup-ignore-command)
      (which-key--show-keymap
       (if (eq (plist-get (car targets) :type) 'embark-become)
           "Become"
         (format "Act on %s '%s'%s"
                 (plist-get (car targets) :type)
                 (embark--truncate-target (plist-get (car targets) :target))
                 (if (cdr targets) "…" "")))
       (if prefix
           (pcase (lookup-key keymap prefix 'accept-default)
             ((and (pred keymapp) km) km)
             (_ (key-binding prefix 'accept-default)))
         keymap)
       nil nil t (lambda (binding)
                   (not (string-suffix-p "-argument" (cdr binding))))))))

(setq embark-indicators
  '(embark-which-key-indicator
    embark-highlight-indicator
    embark-isearch-highlight-indicator))

(defun embark-hide-which-key-indicator (fn &rest args)
  "Hide the which-key indicator immediately when using the completing-read prompter."
  (which-key--hide-popup-ignore-command)
  (let ((embark-indicators
         (remq #'embark-which-key-indicator embark-indicators)))
      (apply fn args)))

(advice-add #'embark-completing-read-prompter
            :around #'embark-hide-which-key-indicator)

I didn't know about support for transient keymaps on which-key. I just tried setting which-key-show-transient-mapt to t with both default or "which-key-from-wiki" indicator, but nothing changes.

antrmn avatar Jul 11 '23 20:07 antrmn

Well, embark-which-key-indicator calls the function which-key--show-keymap with its no-paging argument set to t, which disables paging. So, in principle it should be enough to change that argument to nil (i.e., change the nil nil t in the second to last line of embark-which-key-indicator to nil nil nil), but for some reason that seems to only let you page once. I don't understand why that doesn't work.

oantolin avatar Jul 13 '23 01:07 oantolin

I did a lot of print-debugging, but I feel no closer to understanding. I does work if, instead of configuring which-key to intercept C-h, we dispatch to it's handler ourselves. I'm very happy to have this working: which-key's buffer fits neatly underneath vertico-posframe(which had previously obscured the horizontal split), and it feels very consistant with the rest of my configuration.

~~I don't know that it's sufficient, but would gladly to submit a pull request if you'd like to take it as-is. Otherwise, maybe embark-keymap-help could become a real function ((_ keys cmd keymap update)) so it's possible to distribute this as advice in the wiki snippet? Maybe Chesterton's Fence is at play there. I'd have set embark-help-prefix to nil and re-bound C-h, but I needed embark-keymap-prompter's arguments in scope to make it recur.~~ I'm not pleased that I failed to grok the issue; we'd rather respect the value of which-key-paging-key for a production fix, and more issues may lie under the surface.

In the meantime, interested users can copy my re-definition of embark-keymap-prompter.
Leave the no-paging argument of which-key--show-keymap discussed above set to t.

Edit 1: oh my gosh, congrats on 1.0! 🎉

Edit 2: I think I know what's happening!!! I'm at work, but I'll check it out again tonight; if I'm right, then it's actually the same kind of elisp issue that me and a fedi buddy ran into earlier this week.

antler5 avatar Dec 10 '23 08:12 antler5

Never-mind, I give up for now :'c

I'm going to be leaving it in my personal config in this form, which inherits bindings from which-key's keymap to address my concern above-- but I wasn't able to make any more fundamental fixes. All these timers and simultaneous read's just don't seem to get along, and under no minimal configuration can I get (progn (which-key-show-keymap ...) (read-char)) to behave. To top it all off, just as I was thinking "ah well maybe this is good enough", I discovered that calling undo (C-h u) to go up a level takes you to the top level instead of back into the initial keymap??? I am baffled. I haven't tried it when double-nested bc I didn't have a such a menu-tree handy at the time. On the plus side, toggling doc-strings with (C-h d) does work, and paging itself really is the most important thing.

If I come back to hack on this again, it will likely have to be focused on the which-key side of the namespace. We need like, a which-key--show-keymap that runs it's read-char interception more than once and doesn't return or delegate control-flow to timers until a full seq has been read (a la read-sequence{,-vector}).

Good night!

antler5 avatar Dec 11 '23 10:12 antler5