Hi,
On Mon, 23 Jun 2008, Linus Torvalds wrote:
> 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.
> 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.
> 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.
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
