Date: Wed, 11 Jan 2012 13:26:12 -0500
From: Austin Clements <amdragon@MIT.EDU>
To: Jani Nikula <jani@nikula.org>
Subject: Re: [PATCH 1/3] count: Convert to new-style argument parsing
Message-ID: <20120111182453.GS20796@mit.edu>
References: <20120109043101.GH20796@mit.edu>
	<1326258173-21163-1-git-send-email-amdragon@mit.edu>
	<1326258173-21163-2-git-send-email-amdragon@mit.edu>
	<87zkdulkla.fsf@nikula.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87zkdulkla.fsf@nikula.org>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch@notmuchmail.org
Precedence: list

Quoth Jani Nikula on Jan 11 at  8:17 am:
> On Wed, 11 Jan 2012 00:02:51 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > ---
> >  notmuch-count.c |   53 +++++++++++++++++++++++++----------------------------
> >  1 files changed, 25 insertions(+), 28 deletions(-)
> > 
> > diff --git a/notmuch-count.c b/notmuch-count.c
> > index 20ce334..fb7401b 100644
> > --- a/notmuch-count.c
> > +++ b/notmuch-count.c
> > @@ -21,6 +21,11 @@
> >  
> >  #include "notmuch-client.h"
> >  
> > +typedef enum {
> > +    OUTPUT_THREADS,
> > +    OUTPUT_MESSAGES,
> > +} output_t;
> > +
> >  int
> >  notmuch_count_command (void *ctx, int argc, char *argv[])
> >  {
> > @@ -28,35 +33,23 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
> >      notmuch_database_t *notmuch;
> >      notmuch_query_t *query;
> >      char *query_str;
> > -    int i;
> > -    notmuch_bool_t output_messages = TRUE;
> > +    int opt_index;
> > +    output_t output = OUTPUT_MESSAGES;
> >  
> > -    argc--; argv++; /* skip subcommand argument */
> > +    notmuch_opt_desc_t options[] = {
> > +	{ NOTMUCH_OPT_KEYWORD, &output, "output", 'o',
> > +	  (notmuch_keyword_t []){ { "threads", OUTPUT_THREADS },
> > +				  { "messages", OUTPUT_MESSAGES },
> > +				  { 0, 0 } } },
> 
> To be pedantic, parse_arguments() expects 'output_var' to be a pointer
> to int for NOTMUCH_OPT_KEYWORD. sizeof(enum) is implementation
> dependent. I would forget about output_t typedef, use plain enum, and
> int type for 'output'.

Pedantic or not, that's a good point.  We should fix this in
notmuch-search.c, too (which is where I shamelessly copied this from).
Or make the argument parser more type-safe.