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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html