[PATCH v4 0/7] emacs: help: remap keybindings

[PATCH v4 0/7] emacs: help: remap keybindings

[Mark Walters]

This is v4 of this patch set. v3 is at id:1383932419-12533-1-git-send-email-markwalters1009@gmail.com.

David found a bug in v3 which I tracked down to some match-data being
over-written. This included the bugfix
id:1384076682-16872-1-git-send-email-markwalters1009@gmail.com which
is folded into this series as the second patch.

The diff v3+bugfix is below: it does the save-match-data as Austin
suggested, and it removes an accidental added comment which I put in
when finding the bug.

I have also left off the last "testing patch" (a patch that added some
functionality to make it easier to test the help routine but not
something that should be added to mainline)

Best wishes

Mark

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 20ed1fb..0655bdd 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -305,16 +305,15 @@ prefix argument.  PREFIX and TAIL are used internally."
 (defun notmuch-substitute-command-keys (doc)
   "Like `substitute-command-keys' but with documentation, not function names."
   (let ((beg 0))
-    (while (string-match "\\\\{\\([^}[:space:]]*\\)}" doc beg) ;; matches \{not-space}
+    (while (string-match "\\\\{\\([^}[:space:]]*\\)}" doc beg)
       (let ((desc
 	     (save-match-data
 	       (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 keymap))
-		      (desc-list (mapcar (lambda (arg) (concat (car arg) "\t" (cdr arg))) desc-alist))
-		      (desc (mapconcat #'identity desc-list "\n")))
-		 desc))))
+		      (desc-list (mapcar (lambda (arg) (concat (car arg) "\t" (cdr arg))) desc-alist)))
+		 (mapconcat #'identity desc-list "\n")))))
 	(setq doc (replace-match desc 1 1 doc)))
       (setq beg (match-end 0)))
     doc))




Mark Walters (7):
  emacs: help: check for nil key binding
  emacs: help: save-match-data
  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/notmuch-lib.el  |   85 ++++++++++++++++++++++++++++++++++--------------
 emacs/notmuch-tree.el |    8 ++--
 2 files changed, 64 insertions(+), 29 deletions(-)

-- 
1.7.9.1

[PATCH v4 1/7] emacs: help: check for nil key binding

[Mark Walters]

A standard way to unset a key binding is local-unset-key which is equivalent to
  (define-key (current-local-map) key nil)

Currently notmuch-help gives an error and fails if a user has done this.

To fix this we only add a help line if the binding is non-nil.
---
 emacs/notmuch-lib.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 08a86fd..5e734b3 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -254,7 +254,7 @@ prefix argument.  PREFIX and TAIL are used internally."
 	    (setq tail
 		  (notmuch-describe-keymap
 		   binding ua-keys (notmuch-prefix-key-description key) tail)))
-	   (t
+	   (binding
 	    (when (and ua-keys (symbolp binding)
 		       (get binding 'notmuch-prefix-doc))
 	      ;; Documentation for prefixed command
-- 
1.7.9.1

[PATCH v4 2/7] emacs: help: save-match-data

[Mark Walters]

The routines that construct the help page in notmuch-lib rely on
match-data being preserved across some fairly complicated code. This
is currently valid but will not be when this series is finished. Thus
place everything between the string-match and replace-match inside a
save-match-data.
---
 emacs/notmuch-lib.el |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 5e734b3..6abc4ea 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -274,11 +274,13 @@ prefix argument.  PREFIX and TAIL are used internally."
   "Like `substitute-command-keys' but with documentation, not function names."
   (let ((beg 0))
     (while (string-match "\\\\{\\([^}[:space:]]*\\)}" doc beg)
-      (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-list (notmuch-describe-keymap keymap ua-keys))
-	     (desc (mapconcat #'identity desc-list "\n")))
+      (let ((desc
+	     (save-match-data
+	       (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-list (notmuch-describe-keymap keymap ua-keys)))
+		 (mapconcat #'identity desc-list "\n")))))
 	(setq doc (replace-match desc 1 1 doc)))
       (setq beg (match-end 0)))
     doc))
-- 
1.7.9.1

[PATCH v4 3/7] emacs: help: remove duplicate bindings

[Mark Walters]

If the user (or a mode) overrides a keybinding from the common keymap
in one of the modes then both help lines appear in the help screen
even though only one of them is applicable.

Fix this by checking if we already have that key binding. We do this
by constructing an list of (key . docstring) pairs so it is easy to
check if we have already had that binding. Then the actual print help
routine changes these pairs into strings "key \t docstring"
---
 emacs/notmuch-lib.el |   41 ++++++++++++++++++++++++-----------------
 1 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 6abc4ea..dde636a 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -238,11 +238,12 @@ This is basically just `format-kbd-macro' but we also convert ESC to M-."
       (concat desc " "))))
 
 (defun notmuch-describe-keymap (keymap ua-keys &optional prefix tail)
-  "Return a list of strings, each describing one binding in KEYMAP.
+  "Return a list of cons cells, each describing one binding in KEYMAP.
 
-Each string gives a human-readable description of the key and a
-one-line description of the bound function.  See `notmuch-help'
-for an overview of how this documentation is extracted.
+Each cons cell consists of a string giving a human-readable
+description of the key, and a one-line description of the bound
+function.  See `notmuch-help' for an overview of how this
+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
@@ -255,18 +256,23 @@ prefix argument.  PREFIX and TAIL are used internally."
 		  (notmuch-describe-keymap
 		   binding ua-keys (notmuch-prefix-key-description key) tail)))
 	   (binding
-	    (when (and ua-keys (symbolp binding)
-		       (get binding 'notmuch-prefix-doc))
-	      ;; Documentation for prefixed command
-	      (let ((ua-desc (key-description ua-keys)))
-		(push (concat ua-desc " " prefix (format-kbd-macro (vector key))
-			      "\t" (get binding 'notmuch-prefix-doc))
-		      tail)))
-	    ;; Documentation for command
-	    (push (concat prefix (format-kbd-macro (vector key)) "\t"
-			  (or (and (symbolp binding) (get binding 'notmuch-doc))
-			      (notmuch-documentation-first-line binding)))
-		  tail))))
+	    (let ((key-string (concat prefix (format-kbd-macro (vector key)))))
+	      ;; We don't include documentation if the key-binding is
+	      ;; over-ridden. Note, over-riding a binding
+	      ;; automatically hides the prefixed version too.
+	      (unless (assoc key-string tail)
+		(when (and ua-keys (symbolp binding)
+			   (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)))
+				(get binding 'notmuch-prefix-doc))
+			  tail)))
+		;; Documentation for command
+		(push (cons key-string
+			    (or (and (symbolp binding) (get binding 'notmuch-doc))
+				(notmuch-documentation-first-line binding)))
+		      tail))))))
    keymap)
   tail)
 
@@ -279,7 +285,8 @@ 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-list (notmuch-describe-keymap keymap ua-keys)))
+		      (desc-alist (notmuch-describe-keymap keymap ua-keys))
+		      (desc-list (mapcar (lambda (arg) (concat (car arg) "\t" (cdr arg))) desc-alist)))
 		 (mapconcat #'identity desc-list "\n")))))
 	(setq doc (replace-match desc 1 1 doc)))
       (setq beg (match-end 0)))
-- 
1.7.9.1

[PATCH v4 4/7] emacs: help: split out notmuch-describe-key as a function

[Mark Walters]

The actual documentation function notmuch-describe-keymap was getting
rather complicated so split out the code for a single key into its own
function notmuch-describe-key.
---
 emacs/notmuch-lib.el |   42 +++++++++++++++++++++++++-----------------
 1 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index dde636a..33790e2 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -237,6 +237,30 @@ This is basically just `format-kbd-macro' but we also convert ESC to M-."
 	"M-"
       (concat desc " "))))
 
+
+(defun notmuch-describe-key (actual-key binding prefix ua-keys tail)
+  "Prepend cons cells describing prefix-arg ACTUAL-KEY and ACTUAL-KEY to TAIL
+
+It does not prepend if ACTUAL-KEY is already listed in TAIL."
+  (let ((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.
+    (unless (assoc key-string tail)
+      (when (and ua-keys (symbolp binding)
+		 (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 actual-key))
+		      (get binding 'notmuch-prefix-doc))
+		tail)))
+      ;; Documentation for command
+      (push (cons key-string
+		  (or (and (symbolp binding) (get binding 'notmuch-doc))
+		      (notmuch-documentation-first-line binding)))
+	    tail)))
+    tail)
+
 (defun notmuch-describe-keymap (keymap ua-keys &optional prefix tail)
   "Return a list of cons cells, each describing one binding in KEYMAP.
 
@@ -256,23 +280,7 @@ prefix argument.  PREFIX and TAIL are used internally."
 		  (notmuch-describe-keymap
 		   binding ua-keys (notmuch-prefix-key-description key) tail)))
 	   (binding
-	    (let ((key-string (concat prefix (format-kbd-macro (vector key)))))
-	      ;; We don't include documentation if the key-binding is
-	      ;; over-ridden. Note, over-riding a binding
-	      ;; automatically hides the prefixed version too.
-	      (unless (assoc key-string tail)
-		(when (and ua-keys (symbolp binding)
-			   (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)))
-				(get binding 'notmuch-prefix-doc))
-			  tail)))
-		;; Documentation for command
-		(push (cons key-string
-			    (or (and (symbolp binding) (get binding 'notmuch-doc))
-				(notmuch-documentation-first-line binding)))
-		      tail))))))
+	    (setq tail (notmuch-describe-key (vector key) binding prefix ua-keys tail)))))
    keymap)
   tail)
 
-- 
1.7.9.1

[PATCH v4 5/7] emacs: help: add base-keymap

[Mark Walters]

To support key remapping in emacs help we need to know the base keymap
when looking at the remapping. keep track of this while we recurse
down the sub-keymaps in help.
---
 emacs/notmuch-lib.el |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 33790e2..7b1fc33 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-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
@@ -278,7 +278,7 @@ prefix argument.  PREFIX and TAIL are used internally."
 	   ((keymapp binding)
 	    (setq 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)
@@ -293,7 +293,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)))
 		 (mapconcat #'identity desc-list "\n")))))
 	(setq doc (replace-match desc 1 1 doc)))
-- 
1.7.9.1

[PATCH v4 6/7] emacs: help: add a special function to deal with remaps

[Mark Walters]

remaps are a rather unusual keymap consisting of "first key" 'remap
and then "second-key" the remapped-function. Thus we do the
documentation for it separately.
---
 emacs/notmuch-lib.el |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 7b1fc33..33211c8 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -261,6 +261,21 @@ It does not prepend if ACTUAL-KEY is already listed in TAIL."
 	    tail)))
     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
+  ;; keymap that maps not from keys to commands, but from old (remapped)
+  ;; functions to the commands to use in their stead.
+  (map-keymap
+   (lambda (command binding)
+     (mapc
+      (lambda (actual-key)
+	(setq tail (notmuch-describe-key actual-key binding prefix ua-keys tail)))
+      (where-is-internal command base-keymap)))
+   remap-keymap)
+  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.
 
@@ -277,8 +292,11 @@ prefix argument.  PREFIX and TAIL are used internally."
      (cond ((mouse-event-p key) nil)
 	   ((keymapp binding)
 	    (setq tail
-		  (notmuch-describe-keymap
-		   binding ua-keys base-keymap (notmuch-prefix-key-description key) tail)))
+		  (if (eq key 'remap)
+		      (notmuch-describe-remaps
+		       binding ua-keys base-keymap prefix tail)
+		    (notmuch-describe-keymap
+		     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)
-- 
1.7.9.1

[PATCH v4 7/7] emacs: tree: use remap for the over-ridden global bindings

[Mark Walters]

Following a suggestion by Austin in id:20130915153642.GY1426@mit.edu
we use remap for the over-riding bindings in pick. This means that if
the user modifies the global keymap these modifications will happen in
the tree-view versions of them too.

[tree-view overrides these to do things like close the message pane
before doing the action, so the functionality is very close to the
original common keymap function.]
---
 emacs/notmuch-tree.el |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-tree.el b/emacs/notmuch-tree.el
index f13b41f..8d59e65 100644
--- a/emacs/notmuch-tree.el
+++ b/emacs/notmuch-tree.el
@@ -220,13 +220,13 @@ FUNC."
     (set-keymap-parent map notmuch-common-keymap)
     ;; The following override the global keymap.
     ;; Override because we want to close message pane first.
-    (define-key map "?" (notmuch-tree-close-message-pane-and #'notmuch-help))
+    (define-key map [remap notmuch-help] (notmuch-tree-close-message-pane-and #'notmuch-help))
     ;; Override because we first close message pane and then close tree buffer.
-    (define-key map "q" 'notmuch-tree-quit)
+    (define-key map [remap notmuch-kill-this-buffer] 'notmuch-tree-quit)
     ;; Override because we close message pane after the search query is entered.
-    (define-key map "s" 'notmuch-tree-to-search)
+    (define-key map [remap notmuch-search] 'notmuch-tree-to-search)
     ;; Override because we want to close message pane first.
-    (define-key map "m" (notmuch-tree-close-message-pane-and #'notmuch-mua-new-mail))
+    (define-key map [remap notmuch-mua-new-mail] (notmuch-tree-close-message-pane-and #'notmuch-mua-new-mail))
 
     ;; these use notmuch-show functions directly
     (define-key map "|" 'notmuch-show-pipe-message)
-- 
1.7.9.1

Re: [PATCH v4 0/7] emacs: help: remap keybindings

[Austin Clements]

LGTM.

Quoth Mark Walters on Nov 12 at  8:10 pm:
> This is v4 of this patch set. v3 is at id:1383932419-12533-1-git-send-email-markwalters1009@gmail.com.
> 
> David found a bug in v3 which I tracked down to some match-data being
> over-written. This included the bugfix
> id:1384076682-16872-1-git-send-email-markwalters1009@gmail.com which
> is folded into this series as the second patch.
> 
> The diff v3+bugfix is below: it does the save-match-data as Austin
> suggested, and it removes an accidental added comment which I put in
> when finding the bug.
> 
> I have also left off the last "testing patch" (a patch that added some
> functionality to make it easier to test the help routine but not
> something that should be added to mainline)
> 
> Best wishes
> 
> Mark
> 
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 20ed1fb..0655bdd 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -305,16 +305,15 @@ prefix argument.  PREFIX and TAIL are used internally."
>  (defun notmuch-substitute-command-keys (doc)
>    "Like `substitute-command-keys' but with documentation, not function names."
>    (let ((beg 0))
> -    (while (string-match "\\\\{\\([^}[:space:]]*\\)}" doc beg) ;; matches \{not-space}
> +    (while (string-match "\\\\{\\([^}[:space:]]*\\)}" doc beg)
>        (let ((desc
>  	     (save-match-data
>  	       (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 keymap))
> -		      (desc-list (mapcar (lambda (arg) (concat (car arg) "\t" (cdr arg))) desc-alist))
> -		      (desc (mapconcat #'identity desc-list "\n")))
> -		 desc))))
> +		      (desc-list (mapcar (lambda (arg) (concat (car arg) "\t" (cdr arg))) desc-alist)))
> +		 (mapconcat #'identity desc-list "\n")))))
>  	(setq doc (replace-match desc 1 1 doc)))
>        (setq beg (match-end 0)))
>      doc))
> 
> 
> 
> 
> Mark Walters (7):
>   emacs: help: check for nil key binding
>   emacs: help: save-match-data
>   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/notmuch-lib.el  |   85 ++++++++++++++++++++++++++++++++++--------------
>  emacs/notmuch-tree.el |    8 ++--
>  2 files changed, 64 insertions(+), 29 deletions(-)
> 

Re: [PATCH v4 0/7] emacs: help: remap keybindings

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> This is v4 of this patch set. v3 is at id:1383932419-12533-1-git-send-email-markwalters1009@gmail.com.
>
> David found a bug in v3 which I tracked down to some match-data being
> over-written. This included the bugfix
> id:1384076682-16872-1-git-send-email-markwalters1009@gmail.com which
> is folded into this series as the second patch.

pushed v4

d