Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
>> diff --git a/sequencer.c b/sequencer.c
>> @@ -2849,10 +2849,14 @@ static int do_reset(const char *name, int len, struct replay_opts *opts)
>> /* Determine the length of the label */
>> + for (i = 0; i < len; i++) {
>> + if (isspace(name[i])) {
>> len = i;
>> + break;
>> + }
>> + }
>> strbuf_addf(&ref_name, "refs/rewritten/%.*s", len, name);
>
> At least for me, this would be more idiomatic if it was coded like this:
>
> for (i = 0; i < len; i++)
> if (isspace(name[i]))
> break;
> strbuf_addf(..., i, name);
>
> or, perhaps (less concise):
>
> for (i = 0; i < len; i++)
> if (isspace(name[i]))
> break;
> len = i;
> strbuf_addf(..., len, name);
Yup, the former is more idiomatic between these two; the latter is
also fine.
The result of Martin's patch shares the same "Huh?" factor as the
original that mucks with the "supposedly constant" side of the
termination condition, and from that point of view, it is not that
much of a readability improvement, I would think.
