[PATCH 1/6] emacs: move tag format validation to `notmuch-tag' function
Dmitry Kurochkin
dmitry.kurochkin at gmail.com
Sun Jan 29 14:54:32 PST 2012
Hi Austin.
On Sun, 29 Jan 2012 16:34:27 -0500, Austin Clements <amdragon at 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)
More information about the notmuch
mailing list