Date: Sun, 29 Jan 2012 18:16:50 -0500
From: Austin Clements <amdragon@MIT.EDU>
To: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
Subject: Re: [PATCH 1/6] emacs: move tag format validation to `notmuch-tag'
	function
Message-ID: <20120129231650.GK17991@mit.edu>
References: <1327725684-5887-1-git-send-email-dmitry.kurochkin@gmail.com>
	<20120129213427.GF17991@mit.edu> <87fwey86hj.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87fwey86hj.fsf@gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch@notmuchmail.org
Precedence: list

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.