unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH v2 0/5] emacs: help: and remap keybindings
@ 2013-11-08  0:21 Mark Walters
  2013-11-08  0:21 ` [PATCH v2 1/5] emacs: help: check for nil key binding Mark Walters
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Mark Walters @ 2013-11-08  0:21 UTC (permalink / raw)
  To: notmuch

This is version 2 of the series id:1382821479-23384-1-git-send-email-markwalters1009@gmail.com

It fixes most of the comments Austin raised in his review. 

The first patch is now trivial and fixes a bug with unset keys in
help. (This is Austin's solution which is obviously better than mine
was)

The second patch is unchanged. The third patch is new and just
separates out a helper function. This will make the remapping help
code clearer, but is probably good in its own right.

The fourth patch deals with help for the remapping case. It is
completely separate from the main help so does not clutter it up like
it did before. The logic is slightly different from before: it now
puts all remapped bindings in rather than just the first (if the
command being remapped was bound to multiple keys).

The fifth patch adds remappings to tree and is unchanged.

It does not address Austin's query about passing down the base keymap
for the remapping.

Best wishes

Mark


Mark Walters (5):
  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 a special function to deal with remaps
  emacs: tree: use remap for the over-ridden global bindings

 emacs/notmuch-lib.el  |   76 +++++++++++++++++++++++++++++++++++-------------
 emacs/notmuch-tree.el |    8 ++--
 2 files changed, 59 insertions(+), 25 deletions(-)

-- 
1.7.9.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/5] emacs: help: check for nil key binding
  2013-11-08  0:21 [PATCH v2 0/5] emacs: help: and remap keybindings Mark Walters
@ 2013-11-08  0:21 ` Mark Walters
  2013-11-08  0:21 ` [PATCH v2 2/5] emacs: help: remove duplicate bindings Mark Walters
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Mark Walters @ 2013-11-08  0:21 UTC (permalink / raw)
  To: notmuch

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 921ed20..ec5a2cb 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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/5] emacs: help: remove duplicate bindings
  2013-11-08  0:21 [PATCH v2 0/5] emacs: help: and remap keybindings Mark Walters
  2013-11-08  0:21 ` [PATCH v2 1/5] emacs: help: check for nil key binding Mark Walters
@ 2013-11-08  0:21 ` Mark Walters
  2013-11-08  3:09   ` Austin Clements
  2013-11-08  0:21 ` [PATCH v2 3/5] emacs: help: split out notmuch-describe-key as a function Mark Walters
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Mark Walters @ 2013-11-08  0:21 UTC (permalink / raw)
  To: notmuch

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 the 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 ec5a2cb..2195166 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)
 
@@ -277,7 +283,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))
 	     (desc (mapconcat #'identity desc-list "\n")))
 	(setq doc (replace-match desc 1 1 doc)))
       (setq beg (match-end 0)))
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/5] emacs: help: split out notmuch-describe-key as a function
  2013-11-08  0:21 [PATCH v2 0/5] emacs: help: and remap keybindings Mark Walters
  2013-11-08  0:21 ` [PATCH v2 1/5] emacs: help: check for nil key binding Mark Walters
  2013-11-08  0:21 ` [PATCH v2 2/5] emacs: help: remove duplicate bindings Mark Walters
@ 2013-11-08  0:21 ` Mark Walters
  2013-11-08  3:30   ` Austin Clements
  2013-11-08  0:21 ` [PATCH v2 4/5] emacs: help: add a special function to deal with remaps Mark Walters
  2013-11-08  0:21 ` [PATCH v2 5/5] emacs: tree: use remap for the over-ridden global bindings Mark Walters
  4 siblings, 1 reply; 9+ messages in thread
From: Mark Walters @ 2013-11-08  0:21 UTC (permalink / raw)
  To: notmuch

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 2195166..8852703 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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 4/5] emacs: help: add a special function to deal with remaps
  2013-11-08  0:21 [PATCH v2 0/5] emacs: help: and remap keybindings Mark Walters
                   ` (2 preceding siblings ...)
  2013-11-08  0:21 ` [PATCH v2 3/5] emacs: help: split out notmuch-describe-key as a function Mark Walters
@ 2013-11-08  0:21 ` Mark Walters
  2013-11-08  3:35   ` Austin Clements
  2013-11-08  0:21 ` [PATCH v2 5/5] emacs: tree: use remap for the over-ridden global bindings Mark Walters
  4 siblings, 1 reply; 9+ messages in thread
From: Mark Walters @ 2013-11-08  0:21 UTC (permalink / raw)
  To: notmuch

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 |   25 ++++++++++++++++++++++---
 1 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 8852703..ef616d5 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 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)))
+   remap-keymap)
+  tail)
+
 (defun notmuch-describe-keymap (keymap ua-keys &optional prefix tail)
   "Return a list of cons cells, each describing one binding in KEYMAP.
 
@@ -276,9 +291,13 @@ prefix argument.  PREFIX and TAIL are used internally."
    (lambda (key binding)
      (cond ((mouse-event-p key) nil)
 	   ((keymapp binding)
-	    (setq tail
-		  (notmuch-describe-keymap
-		   binding ua-keys (notmuch-prefix-key-description key) tail)))
+	    (if (eq key 'remap)
+		(setq tail
+		      (notmuch-describe-remaps
+		       binding ua-keys prefix tail))
+	      (setq tail
+		    (notmuch-describe-keymap
+		     binding ua-keys (notmuch-prefix-key-description key) tail))))
 	   (binding
 	    (setq tail (notmuch-describe-key (vector key) binding prefix ua-keys tail)))))
    keymap)
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 5/5] emacs: tree: use remap for the over-ridden global bindings
  2013-11-08  0:21 [PATCH v2 0/5] emacs: help: and remap keybindings Mark Walters
                   ` (3 preceding siblings ...)
  2013-11-08  0:21 ` [PATCH v2 4/5] emacs: help: add a special function to deal with remaps Mark Walters
@ 2013-11-08  0:21 ` Mark Walters
  4 siblings, 0 replies; 9+ messages in thread
From: Mark Walters @ 2013-11-08  0:21 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/5] emacs: help: remove duplicate bindings
  2013-11-08  0:21 ` [PATCH v2 2/5] emacs: help: remove duplicate bindings Mark Walters
@ 2013-11-08  3:09   ` Austin Clements
  0 siblings, 0 replies; 9+ messages in thread
From: Austin Clements @ 2013-11-08  3:09 UTC (permalink / raw)
  To: Mark Walters; +Cc: notmuch

Quoth Mark Walters on Nov 08 at 12:21 am:
> 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 the is applicable.

s/the/them/

> 
> 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 ec5a2cb..2195166 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)
>  
> @@ -277,7 +283,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))
>  	     (desc (mapconcat #'identity desc-list "\n")))
>  	(setq doc (replace-match desc 1 1 doc)))
>        (setq beg (match-end 0)))

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 3/5] emacs: help: split out notmuch-describe-key as a function
  2013-11-08  0:21 ` [PATCH v2 3/5] emacs: help: split out notmuch-describe-key as a function Mark Walters
@ 2013-11-08  3:30   ` Austin Clements
  0 siblings, 0 replies; 9+ messages in thread
From: Austin Clements @ 2013-11-08  3:30 UTC (permalink / raw)
  To: Mark Walters; +Cc: notmuch

Quoth Mark Walters on Nov 08 at 12:21 am:
> 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 2195166..8852703 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."

This obviously works, but I wonder if the interface would be a bit
clearer if, instead of extending tail, this returned a list of just
the descriptions of ACTUAL-KEY.  It would still have to take the
existing bindings to eliminate duplicates, but that becomes just the
list of existing bindings, and not *also* the tail to prepend bindings
to.  I'm imagining something like,

(defun notmuch-describe-key (actual-key binding prefix ua-keys existing)
  ...
  (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)
      (cons
	;; Documentation for command
	(cons key-string
	      (or (and (symbolp binding) (get binding 'notmuch-doc))
		  (notmuch-documentation-first-line binding)))
	;; Documentation for prefixed command
	(when (and ua-keys (symbolp binding)
		   (get binding 'notmuch-prefix-doc))
	  (list
	    (let ((ua-desc (key-description ua-keys)))
	      (cons (concat ua-desc " " prefix (format-kbd-macro actual-key))
		    (get binding 'notmuch-prefix-doc)))))))))

Then below,

  (setq tail (nconc (notmuch-describe-key ...) tail))

This patch is also fine as is.

> +  (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)
>  

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 4/5] emacs: help: add a special function to deal with remaps
  2013-11-08  0:21 ` [PATCH v2 4/5] emacs: help: add a special function to deal with remaps Mark Walters
@ 2013-11-08  3:35   ` Austin Clements
  0 siblings, 0 replies; 9+ messages in thread
From: Austin Clements @ 2013-11-08  3:35 UTC (permalink / raw)
  To: Mark Walters; +Cc: notmuch

I like the way you separated this out.

Quoth Mark Walters on Nov 08 at 12:21 am:
> 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 |   25 ++++++++++++++++++++++---
>  1 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 8852703..ef616d5 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 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)))
> +   remap-keymap)
> +  tail)
> +

Hmm.  Maybe holistically the interface change I suggested to the
previous patch isn't the right thing to do, since there ends up being
a lot of symmetry between these three functions.

>  (defun notmuch-describe-keymap (keymap ua-keys &optional prefix tail)
>    "Return a list of cons cells, each describing one binding in KEYMAP.
>  
> @@ -276,9 +291,13 @@ prefix argument.  PREFIX and TAIL are used internally."
>     (lambda (key binding)
>       (cond ((mouse-event-p key) nil)
>  	   ((keymapp binding)
> -	    (setq tail
> -		  (notmuch-describe-keymap
> -		   binding ua-keys (notmuch-prefix-key-description key) tail)))
> +	    (if (eq key 'remap)
> +		(setq tail
> +		      (notmuch-describe-remaps
> +		       binding ua-keys prefix tail))
> +	      (setq tail
> +		    (notmuch-describe-keymap
> +		     binding ua-keys (notmuch-prefix-key-description key) tail))))

Maybe swap the if and the setq?

(setq tail (if (eq key 'remap) ...))

>  	   (binding
>  	    (setq tail (notmuch-describe-key (vector key) binding prefix ua-keys tail)))))
>     keymap)

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-11-08  3:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-08  0:21 [PATCH v2 0/5] emacs: help: and remap keybindings Mark Walters
2013-11-08  0:21 ` [PATCH v2 1/5] emacs: help: check for nil key binding Mark Walters
2013-11-08  0:21 ` [PATCH v2 2/5] emacs: help: remove duplicate bindings Mark Walters
2013-11-08  3:09   ` Austin Clements
2013-11-08  0:21 ` [PATCH v2 3/5] emacs: help: split out notmuch-describe-key as a function Mark Walters
2013-11-08  3:30   ` Austin Clements
2013-11-08  0:21 ` [PATCH v2 4/5] emacs: help: add a special function to deal with remaps Mark Walters
2013-11-08  3:35   ` Austin Clements
2013-11-08  0:21 ` [PATCH v2 5/5] emacs: tree: use remap for the over-ridden global bindings Mark Walters

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).