Re: [PATCH 5/7] grep: fix "--" rev/pathspec disambiguation

From: Jeff King <peff@peff.net>
Date: Feb 14, 2017, 19:51 UTC
Message-ID: <20170214195127.q6gs6ayp6w7uxi72@sigill.intra.peff.net>
In-Reply-To: <20170214185621.GC44208@google.com>

On Tue, Feb 14, 2017 at 10:56:21AM -0800, Brandon Williams wrote:

Show 16 quoted lines

> On 02/14, Jeff King wrote:
> > -	/* Check revs and then paths */
> > +	/*
> > +	 * We have to find "--" in a separate pass, because its presence
> > +	 * influences how we will parse arguments that come before it.
> > +	 */
> > +	for (i = 0; i < argc; i++) {
> > +		if (!strcmp(argv[i], "--")) {
> > +			seen_dashdash = 1;
> > +			break;
> > +		}
> > +	}
> 
> So this simply checks if "--" is an argument that was provided.  This
> then allows grep to know ahead of time how to handle revs/paths
> preceding a "--" or in the absences of the "--".  Seems sensible to me.

By the way, we have to check again later for "--" when parsing the revs themselves. In theory you could set seen_dashdash to the offset of the dashdash in the array, and do the iteration more like:

  for (i = 0; i < dashdash_pos; i++)
          handle_rev(argv[i]);
  for (i = dashdash_pos + 1; i < argc; i++)
          handle_path(argv[i]);

But our loops also handle the case where there is no "--" at all, and I think that approach ends up convoluting the logic. I didn't go very far in that direction before giving it up, though.

-Peff