Date: Fri, 8 Nov 2013 15:00:16 -0500
From: Austin Clements <amdragon@MIT.EDU>
To: Mark Walters <markwalters1009@gmail.com>
Subject: Re: [PATCH v3 0/7] emacs: help: remap keybindings
Message-ID: <20131108200016.GN29298@mit.edu>
References: <1383932419-12533-1-git-send-email-markwalters1009@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1383932419-12533-1-git-send-email-markwalters1009@gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch@notmuchmail.org
Precedence: list

LGTM.

Quoth Mark Walters on Nov 08 at  5:40 pm:
> This is version 3 of the series. v2 is at
> id:1383870096-14627-1-git-send-email-markwalters1009@gmail.com
> 
> The changes are in response to Austin's review of v2. I have not
> changed notmuch-describe-key.
> 
> The other change is to add a base-keymap to look up the remapped
> commands in (since notmuch-substitute-command-keys could refer to
> something other than the current keymap). Currently this never happens
> but this makes the code more robust.
> 
> The final patch of the series is (probably) just for testing
> purposes. It lets you call notmuch-help and specify which mode you
> want to display help for. This means you can call it without having
> the mode as current-mode and hence test the base-keymap code.
> 
> Finally the diff from v2 below (with the testing patch omitted) is below.
> 
> Best wishes
> 
> Mark
> 
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index ef616d5..4b3a86e 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -261,7 +261,7 @@ It does not prepend if ACTUAL-KEY is already listed in TAIL."
>  	    tail)))
>      tail)
>  
> -(defun notmuch-describe-remaps (remap-keymap ua-keys prefix tail)
> +(defun notmuch-describe-remaps (remap-keymap ua-keys base-keymap prefix tail)
>    ;; Remappings 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
> @@ -272,11 +272,11 @@ It does not prepend if ACTUAL-KEY is already listed in TAIL."
>       (mapc
>        (lambda (actual-key)
>  	(setq tail (notmuch-describe-key actual-key binding prefix ua-keys tail)))
> -      (where-is-internal command)))
> +      (where-is-internal command base-keymap)))
>     remap-keymap)
>    tail)
>  
> -(defun notmuch-describe-keymap (keymap ua-keys &optional prefix tail)
> +(defun notmuch-describe-keymap (keymap ua-keys base-keymap &optional prefix tail)
>    "Return a list of cons cells, each describing one binding in KEYMAP.
>  
>  Each cons cell consists of a string giving a human-readable
> @@ -291,13 +291,12 @@ prefix argument.  PREFIX and TAIL are used internally."
>     (lambda (key binding)
>       (cond ((mouse-event-p key) nil)
>  	   ((keymapp binding)
> -	    (if (eq key 'remap)
> -		(setq tail
> +	    (setq tail
> +		  (if (eq key 'remap)
>  		      (notmuch-describe-remaps
> -		       binding ua-keys prefix tail))
> -	      (setq tail
> +		       binding ua-keys base-keymap prefix tail)
>  		    (notmuch-describe-keymap
> -		     binding ua-keys (notmuch-prefix-key-description key) tail))))
> +		     binding ua-keys base-keymap (notmuch-prefix-key-description key) tail))))
>  	   (binding
>  	    (setq tail (notmuch-describe-key (vector key) binding prefix ua-keys tail)))))
>     keymap)
> @@ -310,7 +309,7 @@ prefix argument.  PREFIX and TAIL are used internally."
>        (let* ((keymap-name (substring doc (match-beginning 1) (match-end 1)))
>  	     (keymap (symbol-value (intern keymap-name)))
>  	     (ua-keys (where-is-internal 'universal-argument keymap t))
> -	     (desc-alist (notmuch-describe-keymap keymap ua-keys))
> +	     (desc-alist (notmuch-describe-keymap keymap ua-keys keymap))
>  	     (desc-list (mapcar (lambda (arg) (concat (car arg) "\t" (cdr arg))) desc-alist))
>  	     (desc (mapconcat #'identity desc-list "\n")))
>  	(setq doc (replace-match desc 1 1 doc)))
> 
> 
> 
> Mark Walters (7):
>   emacs: help: check for nil key binding
>   emacs: help: remove duplicate bindings
>   emacs: help: split out notmuch-describe-key as a function
>   emacs: help: add base-keymap
>   emacs: help: add a special function to deal with remaps
>   emacs: tree: use remap for the over-ridden global bindings
>   emacs: help: base-keymap-test-help
> 
>  emacs/notmuch-lib.el  |   79 ++++++++++++++++++++++++++++++++++--------------
>  emacs/notmuch-tree.el |    8 ++--
>  2 files changed, 60 insertions(+), 27 deletions(-)