On Mon, 23 Jun 2008, Jeff King wrote:
> On Mon, Jun 23, 2008 at 09:25:10AM -0700, Linus Torvalds wrote:
>
> > 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.
>
> If I know that I have option "-a", what is the correct partial parsing
> of:
>
> git foo -b -a
>
> ?
You'd start off with argv[] looking like [ "foo" "-b" "-a" ] and then
after calling parse_options with that, depending on whether it has
PARSE_OPT_CONTINUE_ON_UNKNOWN or not, you'd either end up with the "-a"
handled (and argv[] now being just [ "foo" "-b" ]), or if you have
PARSE_OPT_STOP_ON_UNKNOWN then parse_options() would return without having
done anything, and expecting you to handle the unknown option first and
then restarting the argument parsing.
The problem with parse_options() right now is:
- it cannot do this at all (it can stop or ignore *non*arguments, but
not things that start with "-" and will always error out)
- it actually puts the result somewhere different than the source, which
makes it very annoying to work with together with anything else that
also wants to look at, or change, argv/argc.
IOW, right now it takes it's arguments from argv[1...], but then puts
back the remainder into argv[0...]. Similarly, it takes a "count plus
one" value of argc, but then _returns_ a "count plus zero".
That second thing is an annoyance, and could be handled either with a new
flag (PARSE_OPT_PUT_BACK_IDENTICALLY), or by just changing the calling
convention. The latter is the "right thing", but it needs trivial changes
in all existing callers.
I actually suspect that the best fix for the second issue is to yes,
change the calling convention so that it puts things back where it found
them, and returns a "count plus one" count, *but* have a special case
where "if all arguments are used, return zero".
[ IOW, it would never ever return "1". If there is one argument left in
argv[1], it returns 2 because it counts "argv[0]" even if it doesn't
_use_ it. That's exactly the same way argv/argc works normally. But
then, if it actually used up everything, it wouldn't return 1, but
return 0 to make it
(a) easier and more obvious to test for "done" in general
(b) easier to convert existing users that basically expect a zero
return value to mean "I ate all the arguments". Many existing users
of "parse_options()" don't care about anything else, and wouldn't
need any changes at all.
but I haven't actually looked too closely yet to be able to say whether
this would avoid the bulk of the problems with changing the existing
practice of 'parse_option()' callers. ]
Hmm?
Linus
--
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
