Hi Austin. On Sun, 29 Jan 2012 16:34:27 -0500, Austin Clements <amdragon@MIT.EDU> wrote: > 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). > Agreed. Though I would hate to add the same check to each tag operation. Perhaps this check can go to `notmuch-select-tags-with-completion'? This is not the main patch in the series. So I think I would prefer not to make v2 because of this issue. If we come up with a good (i.e. no duplication) solution, I will prepare a separate patch for it. Regards, Dmitry > > + (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)