Re: [PATCH 10/14] pull: set reflog message

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

>>  /**
>> + * Sets the GIT_REFLOG_ACTION environment variable to the concatenation of argv
>> + */
>> +static void set_reflog_message(int argc, const char **argv)
>> +{
>> +	int i;
>> +	struct strbuf msg = STRBUF_INIT;
>> +
>> +	for (i = 0; i < argc; i++) {
>> +		strbuf_addstr(&msg, argv[i]);
>> +		strbuf_addch(&msg, ' ');
>> +	}
>> +
>> +	strbuf_rtrim(&msg);
>
> Since this is not performance critical code, I would be slightly in
> favor of simplifying thusly:
>
> /* Join the argument list, separated by spaces */
> for (i = 0; i < argc; i++)
> 	strbuf_addf(&msg, "%s%s", i ? " " : "", argv[i]);

Yeah, either that, or "insert separator only before adding to
something else" pattern, i.e.

	for (i = 0; i < argc; i++) {
        	if (i)
                	addch(&msg, ' ');
		addstr(&msg, argv[i]);
	}

Both are easier to read than "always append SP and trim at the end".
Besides, trimming at the end makes readers wonder if there are cases
where argv[argc-1] ends with whitespaces and the code on purpose
removes them.