[PATCH 0/6] emacs: Built-in help improvements and clean up doc strings

[PATCH 0/6] emacs: Built-in help improvements and clean up doc strings

[Austin Clements]

The first three patches in this series simply fix some function
documentation string problems I found while making the other patches.
I'm marking them trivial because they don't affect any code.

The rest of this series improves a bunch of the built-in Emacs help,
accessed through `notmuch-help' (bound to "?" by default).  In
addition to improving some documentation strings, it introduces a new
facility to `notmuch-help' to explicitly specify interactive
documentation and documentation for prefixed commands.  For example,
where notmuch-help currently says

m       Invoke the notmuch mail composition window.

with this series, it says

m       Compose new mail.
C-u m   ... and prompt for sender

Feel free to point out commands I missed (or post patches for them).
I went through all bound commands in hello, search, and show, as well
as all prefixed commands, but didn't go through unbound, non-prefixed
commands.

[PATCH 1/6] emacs: `notmuch-mua-new-forward-message' is not interactive

[Austin Clements]

`notmuch-mua-new-forward-message' must be called from a buffer
containing a raw RFC2822-formatted message to forward.  Hence, it's
intended to be invoked programmatically through something else that
sets up this buffer (like `notmuch-show-forward-message'), not
interactively.

Remove its interactive specification and update the documentation
string to mention the requirements on the current buffer.
---
 emacs/notmuch-mua.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index d41c0b3..dbb0dae 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -327,9 +327,10 @@ the From: address first."
 (defun notmuch-mua-new-forward-message (&optional prompt-for-sender)
   "Invoke the notmuch message forwarding window.
 
+The current buffer must contain an RFC2822 message to forward.
+
 If PROMPT-FOR-SENDER is non-nil, the user will be prompted for
 the From: address first."
-  (interactive "P")
   (if (or prompt-for-sender notmuch-always-prompt-for-sender)
       (let* ((sender (notmuch-mua-prompt-for-sender))
 	     (address-components (mail-extract-address-components sender))
-- 
1.8.4.rc3

[PATCH 2/6] emacs: `notmuch-mua-new-reply' is also not interactive

[Austin Clements]

Like `notmuch-mua-new-forward-message', this is meant to be invoked
programmatically by something that can provide a reasonable query
string.

In fact, `notmuch-mua-new-reply's interactive specification didn't
match its arguments, so it wouldn't have worked interactively.
---
 emacs/notmuch-mua.el | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index dbb0dae..f6006ee 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -340,8 +340,11 @@ the From: address first."
     (notmuch-mua-forward-message)))
 
 (defun notmuch-mua-new-reply (query-string &optional prompt-for-sender reply-all)
-  "Invoke the notmuch reply window."
-  (interactive "P")
+  "Compose a reply to the message identified by QUERY-STRING.
+
+If PROMPT-FOR-SENDER is non-nil, the user will be prompted for
+the From: address first.  If REPLY-ALL is non-nil, the message
+will be addressed to all recipients of the source message."
   (let ((sender
 	 (when prompt-for-sender
 	   (notmuch-mua-prompt-for-sender))))
-- 
1.8.4.rc3

[PATCH 3/6] emacs: Clean up a few documentation strings

[Austin Clements]

Correct some grammatical errors, fix some violations of standard
documentation string formatting conventions, and be more precise.
---
 emacs/notmuch-show.el | 18 ++++++++++--------
 emacs/notmuch.el      | 10 +++++-----
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5d7e24b..7758eaf 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1575,7 +1575,7 @@ shown."
       (notmuch-show-archive-thread-then-next)))
 
 (defun notmuch-show-rewind ()
-  "Backup through the thread, (reverse scrolling compared to \\[notmuch-show-advance-and-archive]).
+  "Backup through the thread (reverse scrolling compared to \\[notmuch-show-advance-and-archive]).
 
 Specifically, if the beginning of the previous email is fewer
 than `window-height' lines from the current point, move to it
@@ -1723,15 +1723,16 @@ to show, nil otherwise."
     (view-buffer buf 'kill-buffer-if-not-modified)))
 
 (defun notmuch-show-pipe-message (entire-thread command)
-  "Pipe the contents of the current message (or thread) to the given command.
+  "Pipe the contents of the current message (or thread) to COMMAND.
 
-The given command will be executed with the raw contents of the
-current email message as stdin. Anything printed by the command
-to stdout or stderr will appear in the *notmuch-pipe* buffer.
+COMMAND will be executed with the raw contents of the current
+email message as stdin. Anything printed by the command to stdout
+or stderr will appear in the *notmuch-pipe* buffer.
 
-When invoked with a prefix argument, the command will receive all
-open messages in the current thread (formatted as an mbox) rather
-than only the current message."
+If ENTIRE-THREAD is non-nil (or when invoked with a prefix
+argument), COMMAND will receive all open messages in the current
+thread (formatted as an mbox) rather than only the current
+message."
   (interactive (let ((query-string (if current-prefix-arg
 				       "Pipe all open messages to command: "
 				     "Pipe message to command: ")))
@@ -1823,6 +1824,7 @@ See `notmuch-tag' for information on the format of TAG-CHANGES."
 
 (defun notmuch-show-open-or-close-all ()
   "Set the visibility all of the messages in the current thread.
+
 By default make all of the messages visible. With a prefix
 argument, hide all of the messages."
   (interactive)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 0ff248b..a36849f 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -869,15 +869,15 @@ PROMPT is the string to prompt with."
 
 ;;;###autoload
 (defun notmuch-search (&optional query oldest-first target-thread target-line)
-  "Run \"notmuch search\" with the given `query' and display results.
+  "Display threads matching QUERY in a notmuch-search buffer.
 
-If `query' is nil, it is read interactively from the minibuffer.
+If QUERY is nil, it is read interactively from the minibuffer.
 Other optional parameters are used as follows:
 
-  oldest-first: A Boolean controlling the sort order of returned threads
-  target-thread: A thread ID (without the thread: prefix) that will be made
+  OLDEST-FIRST: A Boolean controlling the sort order of returned threads
+  TARGET-THREAD: A thread ID (without the thread: prefix) that will be made
                  current if it appears in the search results.
-  target-line: The line number to move to if the target thread does not
+  TARGET-LINE: The line number to move to if the target thread does not
                appear in the search results.
 
 When called interactively, this will prompt for a query and use
-- 
1.8.4.rc3

[PATCH 4/6] emacs: Support overriding help and describing prefix action

[Austin Clements]

Traditionally, function documentation strings are intended primarily
for programmers, rather than users.  They're written from the
perspective of calling the function, not interactively invoking it.
They're only ever displayed along with the function prototype (and
often refer to argument names).  And built-in help commands like
`describe-bindings' show the name of the command, not its
documentation.

The notmuch help system is like `describe-bindings', but tries to be
more user-friendly by displaying documentation strings, rather than
Elisp command names.  For most commands, this is fine, but for some
the "programmer description" is inappropriate for interactive use.
This is particularly noticeable for commands that take an optional
prefix argument.

This patch adds support for two symbol properties: notmuch-doc and
notmuch-prefix-doc, which let a command override its interactive
documentation and provide separate documentation for its prefixed
invocation.  If notmuch-prefix-doc is present, we add an extra line to
the help giving the prefixed key sequence along with the documentation
for the prefixed command.
---
 emacs/notmuch.el | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index a36849f..278bd35 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 &optional prefix tail)
+(defun notmuch-describe-keymap (keymap ua-keys &optional prefix tail)
   "Return a list of strings, each describing one key in KEYMAP.
 
 Each string gives a human-readable description of the key and the
@@ -151,10 +151,19 @@ first line of documentation for the bound function."
 	   ((keymapp binding)
 	    (setq tail
 		  (notmuch-describe-keymap
-		   binding (notmuch-prefix-key-description key) tail)))
+		   binding ua-keys (notmuch-prefix-key-description key) tail)))
 	   (t
+	    (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"
-			  (notmuch-documentation-first-line binding))
+			  (or (and (symbolp binding) (get binding 'notmuch-doc))
+			      (notmuch-documentation-first-line binding)))
 		  tail))))
    keymap)
   tail)
@@ -165,14 +174,24 @@ first line of documentation for the bound function."
     (while (string-match "\\\\{\\([^}[:space:]]*\\)}" doc beg)
       (let* ((keymap-name (substring doc (match-beginning 1) (match-end 1)))
 	     (keymap (symbol-value (intern keymap-name)))
-	     (desc-list (notmuch-describe-keymap keymap))
+	     (ua-keys (where-is-internal 'universal-argument keymap t))
+	     (desc-list (notmuch-describe-keymap keymap ua-keys))
 	     (desc (mapconcat #'identity desc-list "\n")))
 	(setq doc (replace-match desc 1 1 doc)))
       (setq beg (match-end 0)))
     doc))
 
 (defun notmuch-help ()
-  "Display help for the current notmuch mode."
+  "Display help for the current notmuch mode.
+
+This is similar to `describe-function' for the current major
+mode, but bindings tables are shown with documentation strings
+rather than command names.  By default, this uses the first line
+of each command's documentation string.  A command can override
+this by setting the 'notmuch-doc property of its command symbol.
+A command that supports a prefix argument can explicitly document
+its prefixed behavior by setting the 'notmuch-prefix-doc property
+of its command symbol."
   (interactive)
   (let* ((mode major-mode)
 	 (doc (substitute-command-keys (notmuch-substitute-command-keys (documentation mode t)))))
-- 
1.8.4.rc3

[PATCH 5/6] emacs: Improve interactive use documentation

[Austin Clements]

This improves the function documentation for many interactive
commands, either by improving their documentation string where the
improvement also makes sense for programmatic use or by adding a
'notmuch-doc property where it doesn't.

For nearly all commands that support a prefix argument, this adds a
'notmuch-prefix-doc property to document their prefixed behavior This
omits prefix documentation for a few commands where I thought the
prefixed behavior was too obscure (or too complex to fit in one line).
---
 emacs/notmuch-mua.el  |  3 ++-
 emacs/notmuch-show.el | 26 ++++++++++++++++++++++++--
 emacs/notmuch.el      | 11 +++++++++--
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index f6006ee..53802d2 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -313,8 +313,9 @@ the From: header is already filled in by notmuch."
       (ido-completing-read "Send mail From: " notmuch-identities
 			   nil nil nil 'notmuch-mua-sender-history (car notmuch-identities)))))
 
+(put 'notmuch-mua-new-mail 'notmuch-prefix-doc "... and prompt for sender")
 (defun notmuch-mua-new-mail (&optional prompt-for-sender)
-  "Invoke the notmuch mail composition window.
+  "Compose new mail.
 
 If PROMPT-FOR-SENDER is non-nil, the user will be prompted for
 the From: address first."
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 7758eaf..7325792 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1488,6 +1488,8 @@ current thread."
   "Are the headers of the current message visible?"
   (notmuch-show-get-prop :headers-visible))
 
+(put 'notmuch-show-mark-read 'notmuch-prefix-doc
+     "Mark the current message as unread.")
 (defun notmuch-show-mark-read (&optional unread)
   "Mark the current message as read.
 
@@ -1609,16 +1611,20 @@ any effects from previous calls to
       ;; Move to the previous message.
       (notmuch-show-previous-message)))))
 
+(put 'notmuch-show-reply 'notmuch-prefix-doc "... and prompt for sender")
 (defun notmuch-show-reply (&optional prompt-for-sender)
   "Reply to the sender and all recipients of the current message."
   (interactive "P")
   (notmuch-mua-new-reply (notmuch-show-get-message-id) prompt-for-sender t))
 
+(put 'notmuch-show-reply-sender 'notmuch-prefix-doc "... and prompt for sender")
 (defun notmuch-show-reply-sender (&optional prompt-for-sender)
   "Reply to the sender of the current message."
   (interactive "P")
   (notmuch-mua-new-reply (notmuch-show-get-message-id) prompt-for-sender nil))
 
+(put 'notmuch-show-forward-message 'notmuch-prefix-doc
+     "... and prompt for sender")
 (defun notmuch-show-forward-message (&optional prompt-for-sender)
   "Forward the current message."
   (interactive "P")
@@ -1722,6 +1728,10 @@ to show, nil otherwise."
     (set-buffer-modified-p nil)
     (view-buffer buf 'kill-buffer-if-not-modified)))
 
+(put 'notmuch-show-pipe-message 'notmuch-doc
+     "Pipe the contents of the current message to a command.")
+(put 'notmuch-show-pipe-message 'notmuch-prefix-doc
+     "Pipe the thread as an mbox to a command.")
 (defun notmuch-show-pipe-message (entire-thread command)
   "Pipe the contents of the current message (or thread) to COMMAND.
 
@@ -1795,12 +1805,16 @@ See `notmuch-tag' for information on the format of TAG-CHANGES."
 	 (notmuch-show-set-tags new-tags))))))
 
 (defun notmuch-show-add-tag ()
-  "Same as `notmuch-show-tag' but sets initial input to '+'."
+  "Change tags for the current message (defaulting to add).
+
+Same as `notmuch-show-tag' but sets initial input to '+'."
   (interactive)
   (notmuch-show-tag "+"))
 
 (defun notmuch-show-remove-tag ()
-  "Same as `notmuch-show-tag' but sets initial input to '-'."
+  "Change tags for the current message (defaulting to remove).
+
+Same as `notmuch-show-tag' but sets initial input to '-'."
   (interactive)
   (notmuch-show-tag "-"))
 
@@ -1822,6 +1836,8 @@ See `notmuch-tag' for information on the format of TAG-CHANGES."
      (not (plist-get props :message-visible))))
   (force-window-update))
 
+(put 'notmuch-show-open-or-close-all 'notmuch-doc "Show all messages.")
+(put 'notmuch-show-open-or-close-all 'notmuch-prefix-doc "Hide all messages.")
 (defun notmuch-show-open-or-close-all ()
   "Set the visibility all of the messages in the current thread.
 
@@ -1873,6 +1889,8 @@ search results instead."
   (interactive)
   (notmuch-show-next-thread t t))
 
+(put 'notmuch-show-archive-thread 'notmuch-prefix-doc
+     "Un-archive each message in thread.")
 (defun notmuch-show-archive-thread (&optional unarchive)
   "Archive each message in thread.
 
@@ -1902,6 +1920,8 @@ buffer."
   (notmuch-show-archive-thread)
   (notmuch-show-next-thread))
 
+(put 'notmuch-show-archive-message 'notmuch-prefix-doc
+     "Un-archive the current message.")
 (defun notmuch-show-archive-message (&optional unarchive)
   "Archive the current message.
 
@@ -1953,6 +1973,8 @@ thread from search."
   (interactive)
   (notmuch-common-do-stash (notmuch-show-get-from)))
 
+(put 'notmuch-show-stash-message-id 'notmuch-prefix-doc
+     "Copy thread: query matching current thread to kill-ring.")
 (defun notmuch-show-stash-message-id (&optional stash-thread-id)
   "Copy id: query matching the current message to kill-ring.
 
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 278bd35..e559472 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -575,15 +575,21 @@ See `notmuch-tag' for information on the format of TAG-CHANGES."
     (notmuch-search-tag-region beg end tag-changes)))
 
 (defun notmuch-search-add-tag ()
-  "Same as `notmuch-search-tag' but sets initial input to '+'."
+  "Change tags for the current thread (defaulting to add).
+
+Same as `notmuch-search-tag' but sets initial input to '+'."
   (interactive)
   (notmuch-search-tag "+"))
 
 (defun notmuch-search-remove-tag ()
-  "Same as `notmuch-search-tag' but sets initial input to '-'."
+  "Change tags for the current thread (defaulting to remove).
+
+Same as `notmuch-search-tag' but sets initial input to '-'."
   (interactive)
   (notmuch-search-tag "-"))
 
+(put 'notmuch-search-archive-thread 'notmuch-prefix-doc
+     "Un-archive the currently selected thread.")
 (defun notmuch-search-archive-thread (&optional unarchive)
   "Archive the currently selected thread.
 
@@ -887,6 +893,7 @@ PROMPT is the string to prompt with."
 			      'notmuch-search-history nil nil)))))
 
 ;;;###autoload
+(put 'notmuch-search 'notmuch-doc "Search for messages.")
 (defun notmuch-search (&optional query oldest-first target-thread target-line)
   "Display threads matching QUERY in a notmuch-search buffer.
 
-- 
1.8.4.rc3

[PATCH 6/6] News for Emacs help improvements

[Austin Clements]

---
 NEWS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/NEWS b/NEWS
index 2e845f4..4f3b98a 100644
--- a/NEWS
+++ b/NEWS
@@ -17,6 +17,14 @@ New options to better support handling duplicate messages
   bigger than the number of matching messages due to duplicates
   (i.e. multiple files having the same message-id).
 
+Emacs Interface
+---------------
+
+Built-in help improvements
+
+  Documentation for many commands has been improved, as displayed by
+  `notmuch-help` (usually bound to "?").  The bindings listed by
+  `notmuch-help` also now include descriptions of prefixed commands.
 
 Notmuch 0.16 (2013-08-03)
 =========================
-- 
1.8.4.rc3

Re: [PATCH 4/6] emacs: Support overriding help and describing prefix action

[Mark Walters]

This whole series looks good to me. If you are rolling another version
for any reason I have one trivial comment

On Sun, 06 Oct 2013, Austin Clements <amdragon@MIT.EDU> wrote:
> Traditionally, function documentation strings are intended primarily
> for programmers, rather than users.  They're written from the
> perspective of calling the function, not interactively invoking it.
> They're only ever displayed along with the function prototype (and
> often refer to argument names).  And built-in help commands like
> `describe-bindings' show the name of the command, not its
> documentation.
>
> The notmuch help system is like `describe-bindings', but tries to be
> more user-friendly by displaying documentation strings, rather than
> Elisp command names.  For most commands, this is fine, but for some
> the "programmer description" is inappropriate for interactive use.
> This is particularly noticeable for commands that take an optional
> prefix argument.
>
> This patch adds support for two symbol properties: notmuch-doc and
> notmuch-prefix-doc, which let a command override its interactive
> documentation and provide separate documentation for its prefixed
> invocation.  If notmuch-prefix-doc is present, we add an extra line to
> the help giving the prefixed key sequence along with the documentation
> for the prefixed command.
> ---
>  emacs/notmuch.el | 29 ++++++++++++++++++++++++-----
>  1 file changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index a36849f..278bd35 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 &optional prefix tail)
> +(defun notmuch-describe-keymap (keymap ua-keys &optional prefix tail)
>    "Return a list of strings, each describing one key in KEYMAP.
>  
>  Each string gives a human-readable description of the key and the

It would be nice to document the ua-keys variable here. It took me some
time to work out what was going in (and I worked out based on the
caller).

Best wishes

Mark

> @@ -151,10 +151,19 @@ first line of documentation for the bound function."
>  	   ((keymapp binding)
>  	    (setq tail
>  		  (notmuch-describe-keymap
> -		   binding (notmuch-prefix-key-description key) tail)))
> +		   binding ua-keys (notmuch-prefix-key-description key) tail)))
>  	   (t
> +	    (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"
> -			  (notmuch-documentation-first-line binding))
> +			  (or (and (symbolp binding) (get binding 'notmuch-doc))
> +			      (notmuch-documentation-first-line binding)))
>  		  tail))))
>     keymap)
>    tail)
> @@ -165,14 +174,24 @@ first line of documentation for the bound function."
>      (while (string-match "\\\\{\\([^}[:space:]]*\\)}" doc beg)
>        (let* ((keymap-name (substring doc (match-beginning 1) (match-end 1)))
>  	     (keymap (symbol-value (intern keymap-name)))
> -	     (desc-list (notmuch-describe-keymap keymap))
> +	     (ua-keys (where-is-internal 'universal-argument keymap t))
> +	     (desc-list (notmuch-describe-keymap keymap ua-keys))
>  	     (desc (mapconcat #'identity desc-list "\n")))
>  	(setq doc (replace-match desc 1 1 doc)))
>        (setq beg (match-end 0)))
>      doc))
>  
>  (defun notmuch-help ()
> -  "Display help for the current notmuch mode."
> +  "Display help for the current notmuch mode.
> +
> +This is similar to `describe-function' for the current major
> +mode, but bindings tables are shown with documentation strings
> +rather than command names.  By default, this uses the first line
> +of each command's documentation string.  A command can override
> +this by setting the 'notmuch-doc property of its command symbol.
> +A command that supports a prefix argument can explicitly document
> +its prefixed behavior by setting the 'notmuch-prefix-doc property
> +of its command symbol."
>    (interactive)
>    (let* ((mode major-mode)
>  	 (doc (substitute-command-keys (notmuch-substitute-command-keys (documentation mode t)))))
> -- 
> 1.8.4.rc3
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 4/6] emacs: Support overriding help and describing prefix action

[Tomi Ollila]

On Sun, Oct 06 2013, Mark Walters <markwalters1009@gmail.com> wrote:

> This whole series looks good to me. If you are rolling another version
> for any reason I have one trivial comment
>
> On Sun, 06 Oct 2013, Austin Clements <amdragon@MIT.EDU> wrote:
>> Traditionally, function documentation strings are intended primarily
>> for programmers, rather than users.  They're written from the
>> perspective of calling the function, not interactively invoking it.
>> They're only ever displayed along with the function prototype (and
>> often refer to argument names).  And built-in help commands like
>> `describe-bindings' show the name of the command, not its
>> documentation.
>>
>> The notmuch help system is like `describe-bindings', but tries to be
>> more user-friendly by displaying documentation strings, rather than
>> Elisp command names.  For most commands, this is fine, but for some
>> the "programmer description" is inappropriate for interactive use.
>> This is particularly noticeable for commands that take an optional
>> prefix argument.
>>
>> This patch adds support for two symbol properties: notmuch-doc and
>> notmuch-prefix-doc, which let a command override its interactive
>> documentation and provide separate documentation for its prefixed
>> invocation.  If notmuch-prefix-doc is present, we add an extra line to
>> the help giving the prefixed key sequence along with the documentation
>> for the prefixed command.
>> ---
>>  emacs/notmuch.el | 29 ++++++++++++++++++++++++-----
>>  1 file changed, 24 insertions(+), 5 deletions(-)
>>
>> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
>> index a36849f..278bd35 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 &optional prefix tail)
>> +(defun notmuch-describe-keymap (keymap ua-keys &optional prefix tail)
>>    "Return a list of strings, each describing one key in KEYMAP.
>>  
>>  Each string gives a human-readable description of the key and the

This series Looks Good To Me, too.

> It would be nice to document the ua-keys variable here. It took me some
> time to work out what was going in (and I worked out based on the
> caller).

I did not have any problems understanding this -- probably due to me
having played so much with where-is-internal and such commands...
... just that this looks to me as 'first-ua-key' (ua being unversal 
argument, not user agent ;)

If there is need for new version then documenting this would not hurt...
if not followup-patch to add this documentation (w/ one line commit
message for least review effort! ;) could do)

Tomi

>
> Best wishes
>
> Mark
>
>> @@ -151,10 +151,19 @@ first line of documentation for the bound function."
>>  	   ((keymapp binding)
>>  	    (setq tail
>>  		  (notmuch-describe-keymap
>> -		   binding (notmuch-prefix-key-description key) tail)))
>> +		   binding ua-keys (notmuch-prefix-key-description key) tail)))
>>  	   (t
>> +	    (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"
>> -			  (notmuch-documentation-first-line binding))
>> +			  (or (and (symbolp binding) (get binding 'notmuch-doc))
>> +			      (notmuch-documentation-first-line binding)))
>>  		  tail))))
>>     keymap)
>>    tail)
>> @@ -165,14 +174,24 @@ first line of documentation for the bound function."
>>      (while (string-match "\\\\{\\([^}[:space:]]*\\)}" doc beg)
>>        (let* ((keymap-name (substring doc (match-beginning 1) (match-end 1)))
>>  	     (keymap (symbol-value (intern keymap-name)))
>> -	     (desc-list (notmuch-describe-keymap keymap))
>> +	     (ua-keys (where-is-internal 'universal-argument keymap t))
>> +	     (desc-list (notmuch-describe-keymap keymap ua-keys))
>>  	     (desc (mapconcat #'identity desc-list "\n")))
>>  	(setq doc (replace-match desc 1 1 doc)))
>>        (setq beg (match-end 0)))
>>      doc))
>>  
>>  (defun notmuch-help ()
>> -  "Display help for the current notmuch mode."
>> +  "Display help for the current notmuch mode.
>> +
>> +This is similar to `describe-function' for the current major
>> +mode, but bindings tables are shown with documentation strings
>> +rather than command names.  By default, this uses the first line
>> +of each command's documentation string.  A command can override
>> +this by setting the 'notmuch-doc property of its command symbol.
>> +A command that supports a prefix argument can explicitly document
>> +its prefixed behavior by setting the 'notmuch-prefix-doc property
>> +of its command symbol."
>>    (interactive)
>>    (let* ((mode major-mode)
>>  	 (doc (substitute-command-keys (notmuch-substitute-command-keys (documentation mode t)))))
>> -- 
>> 1.8.4.rc3
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 7/6] emacs: Improved `notmuch-describe-keymap' documentation

[Austin Clements]

---
 emacs/notmuch.el | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index e559472..c47c6b5 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -141,10 +141,15 @@ 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 key in KEYMAP.
+  "Return a list of strings, each describing one binding in KEYMAP.
 
-Each string gives a human-readable description of the key and the
-first line of documentation for the bound function."
+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.
+
+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."
   (map-keymap
    (lambda (key binding)
      (cond ((mouse-event-p key) nil)
-- 
1.8.4.rc3

Re: [PATCH 4/6] emacs: Support overriding help and describing prefix action

[Austin Clements]

Quoth Mark Walters on Oct 06 at  9:14 pm:
> 
> This whole series looks good to me. If you are rolling another version
> for any reason I have one trivial comment
> 
> On Sun, 06 Oct 2013, Austin Clements <amdragon@MIT.EDU> wrote:
> > Traditionally, function documentation strings are intended primarily
> > for programmers, rather than users.  They're written from the
> > perspective of calling the function, not interactively invoking it.
> > They're only ever displayed along with the function prototype (and
> > often refer to argument names).  And built-in help commands like
> > `describe-bindings' show the name of the command, not its
> > documentation.
> >
> > The notmuch help system is like `describe-bindings', but tries to be
> > more user-friendly by displaying documentation strings, rather than
> > Elisp command names.  For most commands, this is fine, but for some
> > the "programmer description" is inappropriate for interactive use.
> > This is particularly noticeable for commands that take an optional
> > prefix argument.
> >
> > This patch adds support for two symbol properties: notmuch-doc and
> > notmuch-prefix-doc, which let a command override its interactive
> > documentation and provide separate documentation for its prefixed
> > invocation.  If notmuch-prefix-doc is present, we add an extra line to
> > the help giving the prefixed key sequence along with the documentation
> > for the prefixed command.
> > ---
> >  emacs/notmuch.el | 29 ++++++++++++++++++++++++-----
> >  1 file changed, 24 insertions(+), 5 deletions(-)
> >
> > diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> > index a36849f..278bd35 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 &optional prefix tail)
> > +(defun notmuch-describe-keymap (keymap ua-keys &optional prefix tail)
> >    "Return a list of strings, each describing one key in KEYMAP.
> >  
> >  Each string gives a human-readable description of the key and the
> 
> It would be nice to document the ua-keys variable here. It took me some
> time to work out what was going in (and I worked out based on the
> caller).

Just sent a follow-up patch that improves this function documentation.

> Best wishes
> 
> Mark
> 
> > @@ -151,10 +151,19 @@ first line of documentation for the bound function."
> >  	   ((keymapp binding)
> >  	    (setq tail
> >  		  (notmuch-describe-keymap
> > -		   binding (notmuch-prefix-key-description key) tail)))
> > +		   binding ua-keys (notmuch-prefix-key-description key) tail)))
> >  	   (t
> > +	    (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"
> > -			  (notmuch-documentation-first-line binding))
> > +			  (or (and (symbolp binding) (get binding 'notmuch-doc))
> > +			      (notmuch-documentation-first-line binding)))
> >  		  tail))))
> >     keymap)
> >    tail)
> > @@ -165,14 +174,24 @@ first line of documentation for the bound function."
> >      (while (string-match "\\\\{\\([^}[:space:]]*\\)}" doc beg)
> >        (let* ((keymap-name (substring doc (match-beginning 1) (match-end 1)))
> >  	     (keymap (symbol-value (intern keymap-name)))
> > -	     (desc-list (notmuch-describe-keymap keymap))
> > +	     (ua-keys (where-is-internal 'universal-argument keymap t))
> > +	     (desc-list (notmuch-describe-keymap keymap ua-keys))
> >  	     (desc (mapconcat #'identity desc-list "\n")))
> >  	(setq doc (replace-match desc 1 1 doc)))
> >        (setq beg (match-end 0)))
> >      doc))
> >  
> >  (defun notmuch-help ()
> > -  "Display help for the current notmuch mode."
> > +  "Display help for the current notmuch mode.
> > +
> > +This is similar to `describe-function' for the current major
> > +mode, but bindings tables are shown with documentation strings
> > +rather than command names.  By default, this uses the first line
> > +of each command's documentation string.  A command can override
> > +this by setting the 'notmuch-doc property of its command symbol.
> > +A command that supports a prefix argument can explicitly document
> > +its prefixed behavior by setting the 'notmuch-prefix-doc property
> > +of its command symbol."
> >    (interactive)
> >    (let* ((mode major-mode)
> >  	 (doc (substitute-command-keys (notmuch-substitute-command-keys (documentation mode t)))))

Re: [PATCH 6/6] News for Emacs help improvements

[David Bremner]

Austin Clements <amdragon@MIT.EDU> writes:

> ---
>  NEWS | 8 ++++++++
>  1 file changed, 8 insertions(+)

first 6 pushed.

d

Re: [PATCH 7/6] emacs: Improved `notmuch-describe-keymap' documentation

[Mark Walters]

Hello

This looks good to me +1

Best wishes

Mark

On Mon, 07 Oct 2013, Austin Clements <amdragon@MIT.EDU> wrote:
> ---
>  emacs/notmuch.el | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index e559472..c47c6b5 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -141,10 +141,15 @@ 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 key in KEYMAP.
> +  "Return a list of strings, each describing one binding in KEYMAP.
>  
> -Each string gives a human-readable description of the key and the
> -first line of documentation for the bound function."
> +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.
> +
> +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."
>    (map-keymap
>     (lambda (key binding)
>       (cond ((mouse-event-p key) nil)
> -- 
> 1.8.4.rc3
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 7/6] emacs: Improved `notmuch-describe-keymap' documentation

[David Bremner]

Austin Clements <amdragon@MIT.EDU> writes:

> +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.
> +
> +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."

pushed, 

d