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

From: Jeff King <peff@peff.net>
Date: Jun 23, 2008, 18:33 UTC
Message-ID: <20080623183358.GA28941@sigill.intra.peff.net>
In-Reply-To: <alpine.LFD.1.10.0806231114180.2926@woody.linux-foundation.org>

On Mon, Jun 23, 2008 at 11:20:46AM -0700, Linus Torvalds wrote:

> Actually, I guess "--default" does, but if you try to mix that up with (a) 
> a default head that starts with a dash and (b) git-blame, you're doing 
> something pretty odd.

Yes, and I think that is why "in practice" this works with git-blame.

> And yes, "-n" does too, but if you pass it negative numbers you get what 
> you deserve.

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. Now again, as it happens, git-blame seems to ignore "-n" entirely (though I would have expected it to limit the depth of the blame traversal), so maybe people shouldn't be using it.

Show 12 quoted lines

> The point being, we really _do_ have a real-life existing case for 
> PARSE_OPT_CONTINUE_ON_UNKNOWN, which is hard to handle any other way. 
> Currently you can literally do
> 
> 	git blame --since=April -b Makefile
> 
> and while it's a totally made-up example, it's one I've picked to show 
> exactly what does *not* work with my patch that started this whole thread.
> 
> And guess what you need to fix it?
> 
> If you guessed PARSE_OPT_CONTINUE_ON_UNKNOWN, you win a prize.

I guessed "to convert the revision and diff options to a format that parse_options understands, so we can add them as appropriate to the options tables of the various commands." Do I win anything?

-Peff