Re: [PATCH] graph: output padding for merge subsequent parents
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 8, 2013, 19:40 UTC
- Message-ID
- <7vd2wa1tno.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <vpqtxpmae1p.fsf@grenoble-inp.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
Show 46 quoted lines
> John Keeping <john@keeping.me.uk> writes:
>
>> diff --git a/graph.c b/graph.c
>> index 391a712..2a3fc5c 100644
>> --- a/graph.c
>> +++ b/graph.c
>> @@ -1227,6 +1227,16 @@ void graph_show_commit(struct git_graph *graph)
>> if (!graph)
>> return;
>>
>> + /*
>> + * When showing a diff of a merge against each of its parents, we
>> + * are called once for each parent without graph_update having been
>> + * called. In this case, simply output a single padding line.
>> + */
>> + if (graph_is_commit_finished(graph)) {
>> + graph_show_padding(graph);
>> + shown_commit_line = 1;
>> + }
>> +
>> while (!shown_commit_line && !graph_is_commit_finished(graph)) {
>
> This works, but if we know we're not going to enter the while loop, it
> seams even easier to do this:
>
> --- a/graph.c
> +++ b/graph.c
> @@ -1227,7 +1227,17 @@ void graph_show_commit(struct git_graph *graph)
> if (!graph)
> return;
>
> - while (!shown_commit_line && !graph_is_commit_finished(graph)) {
> + /*
> + * When showing a diff of a merge against each of its parents, we
> + * are called once for each parent without graph_update having been
> + * called. In this case, simply output a single padding line.
> + */
> + if (graph_is_commit_finished(graph)) {
> + graph_show_padding(graph);
> + return;
> + }
> +
> + while (!shown_commit_line) {
> shown_commit_line = graph_next_line(graph, &msgbuf);
> fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout);
> if (!shown_commit_line)In this particular case, with the current state of this function, it is probably OK, but an early return like this tend to be a source of future bugs in the longer term, to make the codeflow skip whatever necessary clean-up that needs to be done after the loop exits.