From: Jani Nikula <jani@nikula.org>
To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH 0/3] argument parsing additions
In-Reply-To: <87vcmchf8c.fsf@qmul.ac.uk>
References: <87399iicit.fsf@qmul.ac.uk> <cover.1331329406.git.jani@nikula.org>
	<87vcmchf8c.fsf@qmul.ac.uk>
User-Agent: Notmuch/0.11.1+295~g780f284 (http://notmuchmail.org) Emacs/23.3.1
	(i686-pc-linux-gnu)
Date: Sun, 11 Mar 2012 21:26:07 +0200
Message-ID: <87ty1vym5s.fsf@nikula.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Sat, 10 Mar 2012 11:23:15 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> On Sat, 10 Mar 2012 00:33:27 +0200, Jani Nikula <jani@nikula.org> wrote:
> > Hi Mark -
> > 
> > I'm not sure which is worse, criticizing or rewriting other people's
> > patches. I already did the former, and now I'm doing the
> > latter. Apologies for both. I didn't really mean to write these patches,
> > but it turned out to be more fun writing a proper reply in C than in
> > English.
> > 
> > Patch 1 adds --arg=true and --arg=false support for booleans. It's not
> > strictly required for the --entire-thread support in patch 3, which uses
> > the extension of keyword arguments from patch 2, but it's for
> > consistency across boolean arguments.
> 
> Hi 
> 
> I like patch 1: I have an almost identical to my version (in the series
> I just sent to the list
> id:"1331377533-30262-1-git-send-email-markwalters1009@gmail.com>
> X-Mailer: git-send-email 1.7.9.1"). 

id:"1331377533-30262-1-git-send-email-markwalters1009@gmail.com" (fixed
reference)

Either of the patches should be pushed forward. The command line
interface is sound, in line with notmuch style, and allows boolean
parameters defaulting to true that you can switch off. And it needs no
changes to argument parser users.

> I am not sure about patch 2 and patch 3. Do you have a use case for
> --option except when option is a boolean?

For the sake of example, if some command produced some unformatted
output by default, you could enable some formatting with --format and
specify details with --format=foo or --format=bar. But admittedly it's a
bit contrived.

And using that for the bool case is also abusing it.

> Otherwise I think I prefer either my
> approach (abusing a notmuch_bool_t) or just adding an option
> NOTMUCH_OPT_BOOLEAN_AS_INT which does boolean parsing but returns an
> int. I guess I am saying that I think allowing boolean options which can
> sometimes default to true and sometimes to false is more useful than
> allowing --option for arbitrary keywords (*).
> 
> What do you think? 

I think we're somewhere between overengineering and bikeshedding, and we
should just fix the issue at hand with the "boolean" -1 value hack. Your
v2 series accomplishes what's needed. Let's go with it, and consider a
more general approach if another case comes up. (I actually wrote
patches to add generic support for getting info about which arguments
were set in the command line, but I think it's more trouble than its
worth.)


BR,
Jani.


> 
> Best wishes 
> 
> Mark
> 
> (*) Indeed, I was thinking of the former as a possibility for the
> exclude code, but I am erring towards just using keywords so I can allow
> more options as you suggested.
> 
> 
> > 
> > Please let me know what you think.
> > 
> > BR,
> > Jani.
> > 
> > 
> > Jani Nikula (3):
> >   command-line-arguments: allow true and false keywords for booleans
> >   command-line-arguments: support keyword arguments with default value
> >   cli: allow switching off entire thread mode in notmuch show json
> >     format
> > 
> >  command-line-arguments.c |   45 +++++++++++++++++++++++++++++++++++++++------
> >  command-line-arguments.h |    1 +
> >  notmuch-show.c           |   12 ++++++++++--
> >  3 files changed, 50 insertions(+), 8 deletions(-)
> > 
> > -- 
> > 1.7.5.4
> >