I originally wanted to see these concerns separated -- to have a separate function that took care of resolving remaps to keep the logic out of notmuch-describe-keymap -- but then I tried to write that function and gave up. If you can think of a way to do that, I think it would make this code clearer, but if not that's fine, too. On Sat, 26 Oct 2013, Mark Walters <markwalters1009@gmail.com> wrote: > If a user or mode uses remap to rebind a keybinding then it appears in > the help as a line <remap><function> New function docstring. Special > case these remapping lines so that we print the actual binding. > --- > emacs/notmuch.el | 21 ++++++++++++++------- > 1 files changed, 14 insertions(+), 7 deletions(-) > > diff --git a/emacs/notmuch.el b/emacs/notmuch.el > index b9db9ba..c354b05 100644 > --- a/emacs/notmuch.el > +++ b/emacs/notmuch.el > @@ -140,7 +140,7 @@ This is basically just `format-kbd-macro' but we also convert ESC to M-." > "M-" > (concat desc " ")))) > > -(defun notmuch-describe-keymap (keymap ua-keys &optional prefix tail) > +(defun notmuch-describe-keymap (keymap ua-keys &optional prefix tail remap) > "Return a list of cons cells, each describing one binding in KEYMAP. > > Each cons cell consists of a string giving a human-readable > @@ -150,16 +150,23 @@ documentation is extracted. > > UA-KEYS should be a key sequence bound to `universal-argument'. > It will be used to describe bindings of commands that support a > -prefix argument. PREFIX and TAIL are used internally." > +prefix argument. PREFIX, TAIL and REMAP are used internally." > (map-keymap > (lambda (key binding) > (cond ((mouse-event-p key) nil) > ((keymapp binding) I think it would be worth documenting what's going on with remaps here. I didn't get it until I wrote some of my own test code *and* chatted with you on IRC. Perhaps something like: ;; Remapping are represented as a binding whose first "event" is ;; 'remap. Hence, if the keymap has any remappings, it will have a ;; binding whose "key" is 'remap, and whose "binding" is itself a ;; keymap that maps not from keys to commands, but from old (remapped) ;; functions to the commands to use in their stead. > - (setq tail > - (notmuch-describe-keymap > - binding ua-keys (notmuch-prefix-key-description key) tail))) > + (if (equal key 'remap) `eq' would be more standard here. > + (setq tail > + (notmuch-describe-keymap > + binding ua-keys prefix tail t)) > + (setq tail > + (notmuch-describe-keymap > + binding ua-keys (notmuch-prefix-key-description key) tail)))) > (t > - (let ((key-string (concat prefix (format-kbd-macro (vector key))))) > + (let* ((actual-key (if remap > + (where-is-internal key nil t) Is it okay to use nil for the keymap to `where-is-internal' here? It seems like maybe we have to pass down a "root" keymap from `notmuch-substitute-command-keys' so it knows what to resolve the remappings relative to. Arguably the original caller should provide just the root keymap, while the "current" sub-keymap should be one of the internal recursive arguments. > + (vector key))) > + (key-string (concat prefix (format-kbd-macro actual-key)))) > ;; We don't include documentation if the key-binding is > ;; over-ridden. Note, over-riding a binding > ;; automatically hides the prefixed version too. > @@ -168,7 +175,7 @@ prefix argument. PREFIX and TAIL are used internally." > (get binding 'notmuch-prefix-doc)) > ;; Documentation for prefixed command > (let ((ua-desc (key-description ua-keys))) > - (push (cons (concat ua-desc " " prefix (format-kbd-macro (vector key))) > + (push (cons (concat ua-desc " " prefix (format-kbd-macro actual-key)) > (get binding 'notmuch-prefix-doc)) > tail))) > ;; Documentation for command > -- > 1.7.9.1 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch