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

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Liam,

On Tue, 28 Nov 2017, liam Beguin wrote:

> On 27/11/17 06:04 PM, Johannes Schindelin wrote:
> > 
> > On Sun, 26 Nov 2017, Liam Beguin wrote:
> > 
> >> @@ -2483,7 +2491,9 @@ int sequencer_make_script(int keep_empty, FILE *out,
> >>  		strbuf_reset(&buf);
> >>  		if (!keep_empty && is_original_commit_empty(commit))
> >>  			strbuf_addf(&buf, "%c ", comment_line_char);
> >> -		strbuf_addf(&buf, "pick %s ", oid_to_hex(&commit->object.oid));
> >> +		strbuf_addf(&buf, "%s %s ",
> >> +			    abbreviate_commands ? "p" : "pick",
> >> +			    oid_to_hex(&commit->object.oid));
> > 
> > I guess the compiler will optimize this code so that the conditional
> > is evaluated only once. Not that this is performance critical ;-)
> 
> Is your guess enough? :-) If not, how could I make sure this is
> optimized?  Should I do that check before the while() loop?

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));

But given Junio's comment that the assignment of `first` was too far away
from the line where it is used for his taste, I guess he will argue (once
again) the exact opposite of me.

Ciao,
Dscho