From: Jeff King <peff@peff.net>
Date: Mon, 15 Oct 2018 19:35:26 GMT
Subject: Re: [PATCH] branch: trivial style fix
Message-ID: <20181015193525.GA8349@sigill.intra.peff.net>
In-Reply-To: <87bm87x5z2.fsf@ls-a.me>

On Sat, Oct 06, 2018 at 08:40:33AM +0800, Tao Qingyun wrote:

> >> -	for (i = 0; i < argc; i++, strbuf_reset(&bname)) {
> >> +	for (i = 0; i < argc; i++) {
> >>  		char *target = NULL;
> >>  		int flags = 0;
> >>  
> >> +		strbuf_reset(&bname);
> >
> > This is not "trivial" nor "style fix", though.
> >
> > It is not "trivial" because it also changes the behaviour.  Instead
> > of resetting the strbuf each time after the loop body runs, the new
> > code resets it before the loop body, even for the 0-th iteration
> > where the strbuf hasn't even been used at all.  It is not a "style
> > fix" because we do not have a particular reason to avoid using the
> > comma operator to perform two simple expressions with side effects
> > inside a single expression.
> >
> Thank you and Jeff for your explanation. I think I get the point now.
> 
> The third part of `for` statement is normally for a step. I think it's
> improve readability even it does nothing in the first iteration.
> 
> And, should I drop this part and resend the patch? I'm a newbie :).

Sorry for the slow reply. For some reason I do not think your message
here made it to the list (but I don't see anything obviously wrong with
it).

Anyway, yes, I think it is worth dropping this hunk and re-sending the
else-if style fix as a separate patch (you may choose to re-send this
hunk as its own patch, too, if you want to argue for its inclusion, but
there is no sense in holding the actual style fix hostage).

-Peff