Johannes Schindelin <johannes.schindelin@xxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html