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

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

Hi,

On Mon, 5 Nov 2007, Linus Torvalds wrote:

quoted

On Mon, 5 Nov 2007, Johannes Schindelin wrote:

quoted

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.

quoted

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