Junio C Hamano writes:
> Rafael Silva <rafaeloliveira.cs@xxxxxxxxx> writes:
>
>> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
>> index b887413d8d..fb15587af8 100644
>> --- a/builtin/bisect--helper.c
>> +++ b/builtin/bisect--helper.c
>> @@ -988,11 +988,8 @@ static int process_replay_file(FILE *fp, struct bisect_terms *terms)
>> struct strbuf word = STRBUF_INIT;
>> int res = 0;
>>
>> - while (strbuf_getline(&line, fp) != EOF) {
>> + while (!res && strbuf_getline(&line, fp) != EOF)
>> res = process_line(terms, &line, &word);
>> - if (res)
>> - break;
>> - }
>
> I do not mind shorter and crisper code, but I somehow find that the
> original more cleanly expresses the intent.
>
> "We'll grab input lines one by one until the input runs out" and "we
> stop when we see a line that process_line() likes" are conditions
> that the loop may stop at two logically distinct levels. You can
> conflate them into a single boolean, making it "unless we found a
> line the process_line() liked in the previous round, grab the next
> line but stop when we ran out the input", and it may make the result
> shorter, but it may be easier to follow by normal readers if we kept
> them separate, like the original does.
That's a good point (and nice explanation, by the way). Before I was
thinking more on the line "while we do not found a good line from
process_line() and we do not finish processing the file, let's go to
the next line" which lead me to proposed changes for shorten the code.
However, after your explanation, I can see now and agree the original
does seems easier to follow and we can as it is.
--
Thanks
Rafael
