Re: [PATCH 2/2] Revert "rev-parse: remove restrictions on some options"

> On Fri, Feb 26, 2016 at 06:29:57PM -0500, Jeff King wrote:
>
>> The best solution here would be to have a full parsing loop
>> that handles all options, but only calls setup_git_directory
>> as appropriate. Unfortunately, that's a bit complicated to
>> implement. We _have_ to handle each option in the order it
>> appears on the command line. If the caller asked for:
>> 
>>   git rev-parse --resolve-git-dir foo/.git --show-toplevel
>> 
>> then it must receive the two lines of output in the correct
>> to know which is which. But asking for:
>> 
>>   git rev-parse --show-toplevel --resolve-git-dir foo/.git
>> 
>> is weird; we have to setup_git_directory() for the first
>> option.
>> 
>> So any implementation would probably have to either:
>> 
>>   - make two passes over the options, first figuring out
>>     whether we need a git-dir, and then actually handling
>>     the options. That's possible, but it's probably not
>>     worth the trouble.
>> 
>>   - call setup_git_directory() on the fly when an option
>>     needs it; that requires annotating all of the options,
>>     and there are a considerable number of them.
>
> Having just sent this, of course, it occurs to me that a loop like:
>
>    setup_git_directory_gently(&nongit);
>    for (i = 0; i < argc; i++) {
>   	if (!strcmp(argv[i], "--local-env-vars"))
>   	... and other nongit-ok options ...
>   
>   	if (nongit)
>   		die("need a repo");
>   
>   	if (!strcmp(argv[i], "--git-dir"))
>   	... and other options ...
>    }
>
> would probably work. It does still leave things like --parseopt and
> --sq-quote as one-offs, though, because they consume the rest of the
> command line. And the fact remains that --local-repo-env isn't really
> suitable for use with other options.
>
> So I'm still tempted by this "lazy" approach.
>
> -Peff