On Fri, Dec 06, 2013 at 08:15:52AM +0700, Duy Nguyen wrote:
> On Fri, Dec 6, 2013 at 4:28 AM, Jeff King <peff@xxxxxxxx> wrote:
> > BTW, the raw looping to find "--" made me wonder how we handle:
> >
> > git log --grep -- HEAD
> >
> > I'd expect it to be equivalent to:
> >
> > git log --grep=-- HEAD
> >
> > but it's not; we truncate the arguments and complain that --grep is
> > missing its argument. Which is probably good enough, given that the
> > alternative is doing a pass that understands all of the options. But it
> > does mean that the "--long-opt=arg" form is safer than the split form if
> > you are passing along an arbitrary "arg".
>
> Maybe we could make setup_revisions() use parse_options() someday,
> which understands about arguments and dashdash.
>
> $ ./git grep -e -- foo
> fatal: ambiguous argument 'foo': both revision and filename
> Use '--' to separate paths from revisions, like this:
> 'git <command> [<revision>...] -- [<file>...]'
> $ ./git grep -e -- -- foo
Yes, although we use it some in handle_revision_opt, I believe. The
problem isn't inherent to parse_options or not, though. To do it
correctly, we need to either:
1. make two passes with the code that actually understands the options
(be it parse_options or not); the first looking for "--", and the
second to do the actual parsing. Right now our first pass does not
understand the options at all.
2. store the non-option arguments (including "--"), and only resolve
and verify them after we have gone through the whole command-line
and know whether we hit a "--" or not.
I suspect the second option would be simpler, as neither parse-options
nor the current revision code is safe to run through twice (e.g.,
parse-options may have callbacks that add to a list, and we would need
to add some kind of "dry-run" flag).
It's something that would be nice to fix, but I don't see myself working
on it anytime soon. It's a lot of work for very little benefit.
-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