From: Jeff King <peff@peff.net>
Date: Tue, 10 Mar 2009 17:59:04 GMT
Subject: Re: [PATCH 2/3] config: set help text for --bool-or-int
Message-ID: <20090310175904.GB26351@sigill.intra.peff.net>
In-Reply-To: <94a0d4530903091450gdcad625g95cd9550cfb724fa@mail.gmail.com>

On Mon, Mar 09, 2009 at 11:50:09PM +0200, Felipe Contreras wrote:

> Yeah, I meant usage_with_help. I don't know what should be done, but I
> think two things should be achieved:
> 
> a) don't crash
> b) encourage the options to always have a description

I don't know of a good way to check at compile time that the struct
member is non-NULL. So I think we are stuck with parse_options dying, or
doing something to cover it up, like:

> Perhaps not showing the option at all, or perhaps showing "**EMPTY**".

But the problem there is that if you _did_ mean for it to be shown, such
a bug can easily escape notice. Tests will generally still pass, so
unless you are scanning the output manually, nothing will tell you what
happened.

I think our best bet is to simply try to catch such things in code
review. This one slipped through, but it was still caught in 'next',
most because it _did_ crash on a non-glibc platform.

> Minor nitpick: "value is interpreted either as bool or int"
> 
> The value is what it is, the --boo-or-int option doesn't change the
> value, just how it is interpreted.

Fair enough. My patch is is in 'next' now, so please go ahead and submit
a new patch with your suggested change. You might want to change --bool
and --int at the same time, though, as they use the same wording.

-Peff