From: Pierre Habouzit Date: Thu, 31 Jul 2008 09:15:15 GMT Subject: Re: git blame not respecting --find-copies-harder ? Message-ID: <20080731091515.GC12867@artemis.madism.org> In-Reply-To: <20080731090348.GB20691@sigill.intra.peff.net> On Thu, Jul 31, 2008 at 09:03:49AM +0000, Jeff King wrote: > 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. --no-walk alters how add_pending_object_with_mode works, which is called by handle_revision_arg. IOW, --no-walk is not the same as --no-walk . IOW reference arguments and --no-walk ordering must be preserved, IOW --no-walk is a pseudo-argument like --all --tags or --remote are. It's okay for parse_revision_opt to take out any option that doesn't alter handle_revision_arg (as for parse_option based parsers it's the sole thing setup_revision will be doing: consuming revisions in a loop with handle_revision_arg), but only those. The full list (that I made when I wrote parse_revision_opt) is actually: pseudo arguments: --all --branches --tags --remotes --reflog combining operator: --not behavior modifying: --no-walk --do-walk -- ·O· Pierre Habouzit ··O madcoder@debian.org OOO http://www.madism.org