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

[Linus Torvalds <torvalds@linux-foundation.org>]

On Mon, 23 Jun 2008, Jeff King wrote:
> 
> It's worse than that. We assume by default that the option has no
> argument, so the argument becomes a non-option parameter to the original
> command. Try (with current git-blame, but I think your patch doesn't
> change this):
> 
>   $ git blame -n 1 git.c
>   fatal: bad revision '1'
> 
>   $ git blame --default HEAD git.c
>   fatal: cannot stat path HEAD: No such file or directory
> 
> Oops.

Oops. And then, how would you fix this most easily?

Be honest now.

Hint: it _still_ involves PARSE_OPT_CONTINUE_ON_UNKNOWN. It would fix both 
cases.

IOW, the whole builtin-blame.c option parsing *should* look like this:

	argc = parse_options(argc, argv, options, usage, PARSE_OPT_CONTINUE_ON_UNKNOWN);
	init_revisions(&revs, NULL);
	setup_revisions(argc, argv, &revs, NULL);

and it should just work.

But we should *also* have some way to do things like the code in 
builtin-ls-files.c which have a few options that don't work well with the 
current parse_options(). Yes, you can make all of them work with 
callbacks, but that often ends up requiring moving arguments around. 
There's no way to make a trivial conversion for 90% of the cases, and then 
leaving the 10% that need other changes.

Example: many arguments cause multiple option variables to change. 
parse_options() simply can't handle that well - you can do it with a 
callback, but then you need to make the option variables global or make 
them a structure or something. All of which just makes it nasty to do 
partial conversions for the simple cases.

And I guarantee that just adding PARSE_OPT_{CONTINUE|STOP}_ON_UNKNOWN is 
going to be the smallest patch, and make for the easiest usage case. It 
may not be "pretty", but I can whip up a patch in five minutes.

Or are we going to sit around discussing this for another five months?

			Linus