On Sun, 29 Jan 2012 18:16:50 -0500, Austin Clements <amdragon@MIT.EDU> wrote: > Quoth Dmitry Kurochkin on Jan 30 at 2:54 am: > > 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. > > What about not giving any error for no tags? As a user, if I delete > the whole tags prompt including the +/- operator, that's a very > explicit action and it's very clear what it should do (nothing). I > don't need Emacs wagging its finger at me for doing something with a > clear meaning. Sure, let's try it. I am always hesitant to do changes like this to avoid boring discussions on what is better. I hope nobody would argue with this change :) Regards, Dmitry