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