From: Mirko Faina <mroik@delayed.space>
Date: Mon, 20 Apr 2026 10:30:15 GMT
Subject: Re: [PATCH] revision.c: implement --reverse=before for walks
Message-ID: <aeX_3tJicFsmDfCX@exploit>
In-Reply-To: <aeXxC8eR0Mn3dGEn@exploit>

On Mon, Apr 20, 2026 at 11:33:25AM +0200, Mirko Faina wrote:
> > diff --git a/revision.c b/revision.c
> > index 599b3a66c3..89a58a65b7 100644
> > --- a/revision.c
> > +++ b/revision.c
> > @@ -2686,7 +2686,20 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
> >  			git_log_output_encoding = xstrdup("");
> >  		return argcount;
> >  	} else if (!strcmp(arg, "--reverse")) {
> > -		revs->reverse ^= 1;
> > +		/*
> > +		 * This relies on "do not reverse" being the 0 value for our
> > +		 * enum, and historical "reverse after" having value 1.
> > +		 */
> > +		revs->reverse = !revs->reverse;
> > +	} else if (!strcmp(arg, "--no-reverse")) {
> > +		revs->reverse = 0;
> > +	} else if (skip_prefix(arg, "--reverse=", &optarg)) {
> > +		if (!strcmp(optarg, "after"))
> > +			revs->reverse = REVS_REVERSE_AFTER;
> > +		else if (!strcmp(optarg, "before"))
> > +			revs->reverse = REVS_REVERSE_BEFORE;
> > +		else
> > +			die(_("unknown value for --reverse: %s"), optarg);
> >  	} else if (!strcmp(arg, "--children")) {
> >  		revs->children.name = "children";
> >  		revs->limited = 1;
> 
> This unfortunately wouldn't work as the first condition is a prefix of
> the third, so no free copy-paste for me.
> 
> Will have separate parsing for omitted and explicit forms in v2.

Just realized it's a strcmp and not start_with, so this should work
fine.

Thank you