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

Jeff King [off-list ref] writes:

On Fri, Feb 26, 2016 at 06:29:57PM -0500, Jeff King wrote:

quoted

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

Yup.

But why do you even need to run local-env-vars from outside a
repository in the first place?