Subject: Re: [PATCH 4/5] rebase -i: learn to abbreviate command names

From: Junio C Hamano <[email-masked]>
Date: Sat, 02 Dec 2017 17:18:50 -0800

Johannes Schindelin <[email-masked]> writes:

> I am a fan of not relying too heavily on compiler optimization and e.g.
> extract code from loops when it does not need to be evaluated every single
> iteration. In this case:
>
> 	const char *pick = abbreviate_commands ? "p" : "pick";
> 	...
> 		strbuf_addf(&buf, "%s %s ", pick,
> 			    oid_to_hex(&commit->object.oid));

I would have called that variable "pick_cmd", not just "pick"; this
preference is minor enough that I would probably reject a patch to
rename from one to the other if the above were already part of the
existing codebase.

I find that the code suggested above easier to follow, simply
because it expresses clearly the flow of thought and that flow of
thought matches how I personally think: we decide how this command
is spelled in the output upfront, and then use that same spelling
consistently throughout the loop.

I do not think it matters performance-wise either way, but I value
how easy it is to follow the code for humans, and it matters much
more in the longer run.  If a compiler does a poor job, we can
eventually notice and help it to produce better code that still does
what we wanted it to do (or it may not be performance critical and
we may not even notice).  If a code is hard to follow, on the other
hand, what we wanted it to do in the first place becomes harder to
figure out.