On Fri, Dec 06, 2013 at 08:15:52AM +0700, Duy Nguyen wrote: > On Fri, Dec 6, 2013 at 4:28 AM, Jeff King <p...@peff.net> 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html