Re: git blame not respecting --find-copies-harder ?

From: Jeff King <peff@peff.net>
Date: Jul 31, 2008, 09:03 UTC
Message-ID: <20080731090348.GB20691@sigill.intra.peff.net>
In-Reply-To: <7v8wvizc16.fsf@gitster.siamese.dyndns.org>

On Thu, Jul 31, 2008 at 01:35:33AM -0700, Junio C Hamano wrote:

Show 17 quoted lines

> > @@ -1002,7 +1002,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
> >  	    !strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk"))
> >  	{
> >  		unkv[(*unkc)++] = arg;
> > -		return 0;
> > +		return 1;
> >  	}
> >  
> >  	if (!prefixcmp(arg, "--max-count=")) {
> >
> > That is, handle_revision_opt says "yes we parsed this, and it should be
> > gone" even though it still gets stuck in the "unknown" section to be
> > parsed by setup_revisions later.
> 
> Hmm, wouldn't that suggest it needs to return 1 when an option candidate
> given to diff_opt_parse() turns out not to be a diff option and stuffed
> back to unkv[] at the end of this function?

Not necessarily. The logic there goes:

  1. it's not a revision option, so pass to diff
  2. it's not a diff opt, so it is unknown; we parsed no options
  3. the caller sees we failed to parse, so it barfs

but the logic here is:

  1. it _is_ a revision option. Our handling of it is just a little odd,
     in that we need to parse it later, when we are in setup_revisions.
     So put it aside for now as a "revision" that just happens to look
     like an option.
  2. caller sees we parsed, and doesn't complain
  3. caller later passes the "revision" to setup_revisions, which knows
     what to do

Now my patch doesn't just operate on "--all", but rather several other options including --no-walk. And maybe that is not right, and --all should be handled separately (though I think --remotes would follow the same logic). I'm not really sure why --no-walk is separated like that.

-Peff