git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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.

Previous: Matthieu Moy
Message 9 of 9 in “Bug in "git log --graph -p -m" (version 1.7.7.6)”
  1. Dale R. WorleyFeb 5, 2013
  2. Junio C HamanoFeb 5, 2013
  3. Matthieu MoyFeb 5, 2013
  4. Dale R. WorleyFeb 6, 2013
  5. John KeepingFeb 6, 2013
  6. Matthieu MoyFeb 6, 2013
  7. graph: output padding for merge subsequent parentsJohn Keeping, Feb 6, 2013
  8. Matthieu MoyFeb 8, 2013
  9. Junio C HamanoFeb 8, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.