Re: [PATCH 3/3] revision: handle pseudo-opts in `--stdin` mode

From: Patrick Steinhardt <ps@pks.im>
Date: Jun 15, 2023, 14:25 UTC
Message-ID: <2ot3z23uzikdrgsl7wdqwzpxe4xihkpekqn2rg5y3vfq5yklv6@njx2gf3wwcab>
In-Reply-To: <xmqq352uyr5b.fsf@gitster.g>

On Wed, Jun 14, 2023 at 08:56:00AM -0700, Junio C Hamano wrote:

Show 29 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> 
> >  		if (sb.buf[0] == '-') {
> > +			const char *argv[2] = { sb.buf, NULL };
> > +
> >  			if (!strcmp(sb.buf, "--")) {
> >  				seen_dashdash = 1;
> >  				break;
> >  			}
> >  
> > -			die("options not supported in --stdin mode");
> > +			if (handle_revision_pseudo_opt(revs, argv, flags))
> > +				continue;
> > +
> > +			die(_("option '%s' not supported in --stdin mode"), sb.buf);
> >  		}
> 
> The reason why we had the double-dash checking inside this block is
> because we rejected any dashed options other than double-dash, but
> with this step, that is no longer true, so it may make more sense to
> move the "seen_dashdash" code outside the block.  Also unlike the
> command line options that allows "--end-of-options" marker to allow
> tweaking how a failing handle_revision_arg() call is handled, this
> codepath does not seem to have any matching provision.  In the
> original code, which did not accept any options, it was perfectly
> understandable that it was unaware of "--end-of-options", but now we
> do allow dashed options, shouldn't we be supporting it here, too?
> 
> Thanks.

Good point, we should. Most importantly, the code would currently refuse to enumerate references with a leading dash, and there would be no way to work around this issue.

E.g. this works:

$ git rev-list --end-of-options -branch

Whereas neither of these do in the current version:

$ printf "%s\n" -branch | git rev-list $ printf "%s\n" --end-of-options -branch | git rev-list

Will send a v2 with `--end-of-options` handling, thanks!

Patrick