On Mon, Nov 05, 2007 at 04:29:43PM +0000, Johannes Schindelin wrote: > 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@xxxxxxxxxx OOO http://www.madism.org
Attachment:
pgpPbmdvZa7tU.pgp
Description: PGP signature
