Hi,

On Mon, 23 Jun 2008, Linus Torvalds wrote:

quoted text
> On Mon, 23 Jun 2008, Johannes Schindelin wrote:
> > 
> > Thinking about the recursive approach again, I came up with this POC:
> 
> "recursive" is pointless.

Nope, it is not.

To keep things _simple_ a callback is not good.  Sure, you can work around 
the limitations of callbacks for aggregation, but the code change looks 
horrible.  And the same holds true for the help message.

Just compare that to the recursive approach.

Okay, now for the "granted, your approach has merits" part.

quoted text
> Look at cmd_apply() in builtin-apply.c. Notice how it currently 
> absolutely CANNOT sanely be turned into using "parse_options()", not 
> because it needs any "recursive" handling, but simply because it wants 
> to do *incremental* handling.

That is a totally independent issue from the one I discussed, namely sane 
handling of the diff (and rev) options.

quoted text
> 	for (;;) {
> 		const char *arg;
> 		argc = parse_options(argc, argv,
> 			options, usage, PARSE_OPT_STOP_AT_UNKNOWN);

We do have PARSE_OPT_STOP_AT_NON_OPTION since a0ec9d25(parseopt: add flag 
to stop on first non option), which tries to solve a _similar_ problem, 
and it should be not hard at all to get PARSE_OPT_STOP_AT_UNKNOWN without 
changing the loop as you suggested.

Heck, we could just as easily introduce PARSE_OPT_IGNORE_UNKNOWN.

quoted text
> Could you handle the "recursive" use of parse_options() in builtin-blame.c 
> by teaching it about recursion? Yes. But again, it's just _simpler_ to 
> just teach parse_options() to parse the things it knows about, and leave 
> the other things in place.

And here I disagree.  You might not need a nice "--help" output, but most 
mortals do.  And this is not easy with your approach.

In contrast, by using my approach of having an option_table for a bundle 
of common options, which just set variables in a certain struct, you can 
have a relatively painless migration, and you get all the benefits of 
parse-options.

But I guess the approach of whoever has more time to work on it will 
win... ;-)

Ciao,
Dscho

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html