One philosophical nit below, but not enough to hold this up. Quoth Dmitry Kurochkin on Jan 28 at 8:41 am: > Before the change, tag format validation was done in > `notmuch-search-operate-all' function only. The patch moves it down > to `notmuch-tag', so that all users of that function get input > validation. > --- > emacs/notmuch.el | 12 ++++++------ > 1 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/emacs/notmuch.el b/emacs/notmuch.el > index 72f78ed..84d7d0a 100644 > --- a/emacs/notmuch.el > +++ b/emacs/notmuch.el > @@ -522,6 +522,12 @@ 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 > + (when (null tags) (error "No tags given")) Since this is a non-interactive function and hence is meant to be invoked programmatically, I would expect it to accept zero tags. Unlike the following check, this is a UI-level check and thus, I believe, belongs in interactive functions (even if that requires a little duplication). > + (mapc (lambda (tag) > + (unless (string-match-p "^[-+][-+_.[:word:]]+$" tag) > + (error "Tag must be of the form `+this_tag' or `-that_tag'"))) > + tags) > (run-hooks 'notmuch-before-tag-hook) > (apply 'notmuch-call-notmuch-process > (append (list "tag") tags (list "--" query))) > @@ -890,12 +896,6 @@ characters as well as `_.+-'. > (interactive (notmuch-select-tags-with-completion > "Operations (+add -drop): notmuch tag " > '("+" "-"))) > - ;; Perform some validation > - (when (null actions) (error "No operations given")) > - (mapc (lambda (action) > - (unless (string-match-p "^[-+][-+_.[:word:]]+$" action) > - (error "Action must be of the form `+this_tag' or `-that_tag'"))) > - actions) > (apply 'notmuch-tag notmuch-search-query-string actions)) > > (defun notmuch-search-buffer-title (query)