Re: [RFC] Re: Convert 'git blame' to parse_options()

On Mon, 23 Jun 2008, Jeff King wrote:

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