Re: [PATCH v2 2/7] bisect--helper: reimplement `bisect_replay` shell function in C

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

Rafael Silva <rafaeloliveira.cs@gmail.com> 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.