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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Nov 5, 2007, 16:29 UTC
Message-ID: <Pine.LNX.4.64.0711051623450.4362@racer.site>
In-Reply-To: <alpine.LFD.0.999.0711050755340.15101@woody.linux-foundation.org>

Hi,

On Mon, 5 Nov 2007, Linus Torvalds wrote:

Show 10 quoted lines

> 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), 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.

Ciao, Dscho