On Sat, 14 Apr 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote: > notmuch-tag is extended to accept various formats of the tag changes. > In particular, user prompting for tag changes is now incorporated > here, so it is common for modes. > > The tag binary and the notmuch-{before,after}-tag-hooks are only > called if tag changes is non-nil. > > The actual tag-changes applied are returned by the function. > --- > emacs/notmuch-tag.el | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el > index c25cff8..dd7f9d7 100644 > --- a/emacs/notmuch-tag.el > +++ b/emacs/notmuch-tag.el > @@ -108,18 +108,26 @@ from TAGS if present." > (error "Changed tag must be of the form `+this_tag' or `-that_tag'"))))) > (sort result-tags 'string<))) > > -(defun notmuch-tag (query &rest tag-changes) > +(defun notmuch-tag (query &optional tag-changes) > "Add/remove tags in TAG-CHANGES to messages matching QUERY. > > -TAG-CHANGES should be a list of strings of the form \"+tag\" or > -\"-tag\" and QUERY should be a string containing the > -search-query. > +QUERY should be a string containing the search-terms. > +TAG-CHANGES can take multiple forms. If TAG-CHANGES is a list of > +strings of the form \"+tag\" or \"-tag\" then those are the tag > +changes applied. If TAG-CHANGES is a string then it is > +interpreted as a single tag change. If TAG-CHANGES is the string > +\"-\" or \"+\", or null, then the user is prompted to enter the > +tag changes. > > Note: Other code should always use this function alter tags of > messages instead of running (notmuch-call-notmuch-process \"tag\" ..) > directly, so that hooks specified in notmuch-before-tag-hook and > notmuch-after-tag-hook will be run." > ;; Perform some validation > + (if (string-or-null-p tag-changes) > + (if (or (string= tag-changes "-") (string= tag-changes "+") (null tag-changes)) > + (setq tag-changes (notmuch-read-tag-changes tag-changes query)) > + (setq tag-changes (list tag-changes)))) > (mapc (lambda (tag-change) > (unless (string-match-p "^[-+]\\S-+$" tag-change) > (error "Tag must be of the form `+this_tag' or `-that_tag'"))) > @@ -128,7 +136,9 @@ notmuch-after-tag-hook will be run." > (run-hooks 'notmuch-before-tag-hook) > (apply 'notmuch-call-notmuch-process "tag" > (append tag-changes (list "--" query))) > - (run-hooks 'notmuch-after-tag-hook))) > + (run-hooks 'notmuch-after-tag-hook)) Hi The series looks good to me with one minor point: > + ;; return the list of actual changed tags > + tag-changes) I found the comment confusing: I read it as the function looked at the tags the message had before and after and returned the difference. Perhaps something like "in all cases we return tag-changes as a list" (and a similar comment for the commit message). Best wishes Mark