Re: [PATCH 2/3] parseopt: introduce OPT_RECURSE to specify shared options

From: Pierre Habouzit <madcoder@debian.org>
Date: Nov 5, 2007, 16:53 UTC
Message-ID: <20071105165359.GC6205@artemis.corp>
In-Reply-To: <Pine.LNX.4.64.0711051623450.4362@racer.site>

On Mon, Nov 05, 2007 at 04:29:43PM +0000, Johannes Schindelin wrote:

Show 33 quoted lines

> Hi,
> 
> On Mon, 5 Nov 2007, Linus Torvalds wrote:
> 
> > On Mon, 5 Nov 2007, Johannes Schindelin wrote:
> > > 
> > > After kicking this around a bit more on IRC, we had another idea.  
> > > Instead of introducing OPT_RECURSE(), do something like OPT__QUIET(), 
> > > only this time in diff.h: ....
> > 
> > I think the preprocessor approach would tend to be simpler, which is an 
> > advantage. But whichever approach is chosen, I think one important issue 
> > is to make sure that options that *hide* other options are correctly 
> > handled in the help printout..
> 
> Yep. See my patch 3/3, which just used a char[256] for the short names, 
> and a path-list for the long names.
> 
> > But that's an implementation issue. The same certainly *can* be done 
> > with a recursive setup, just passing a linked list of what the earlier 
> > levels were (which is what we do in other places). And it's not like the 
> > recursion is going to be very deep or complex.
> 
> Exactly.
> 
> The more pressing issue is that we have pointers in the option structure, 
> which point back to the variables expected to hold the option values.
> 
> The recurse approach would need fixing up those (or some ugly copying of 
> a struct diff_options).
> 
> But the preprocessor approach means wasting space (since we basically have 
> the same options in different builtins),

The "lost" space is the number of options x sizeof(struct option), the latter being (if I'm correct):

on i386: 9 * 4 = 36 octets on amd64: 4 x 2 + 8 * 4 + 8 (padding) + 8 * 2 = 64 octets.

It's not even near being an issue :)

>                                          and it means that the callback 
> functions needed to parse e.g. the diff colour names need to be public.  
> Which is not the worst thing, of course.

  Well it's certainly less ugly than copying the diff_options or
reseting it or anything like that. I don't care if we need to make a
couple of opt-parsing function public more than what we could have
needed.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org