From: Jeff King <peff@peff.net>
Date: Wed, 13 May 2026 23:02:16 GMT
Subject: Re: [GSoC PATCH v3 1/1] graph: add indentation for commits preceded by a parentless commit
Message-ID: <20260513230216.GA1378627@coredump.intra.peff.net>
In-Reply-To: <20260427102838.44867-2-pabloosabaterr@gmail.com>

On Mon, Apr 27, 2026 at 12:28:38PM +0200, Pablo Sabater wrote:

> @@ -1135,7 +1227,18 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
>  				graph_line_write_column(line, col, '|');
>  			graph_line_addch(line, ' ');
>  		} else {
> -			graph_line_write_column(line, col, '|');
> +			if (col->is_placeholder) {
> +				/*
> +				 * Same placeholder handling as in
> +				 * graph_output_commit_line().
> +				 */
> +				if (seen_this)
> +					continue;
> +				graph_line_write_column(line, col, ' ');
> +			} else {
> +				graph_line_write_column(line, col, '|');
> +			}

I haven't looked closely at the patch, but Coverity complained that
the "if (seen_this)" check here is dead code, because this whole else
block follows:

                  } else if (seen_this) {
                          if (graph->edges_added > 0)
                                  graph_line_write_column(line, col, '\\');
                          else
                                  graph_line_write_column(line, col, '|');
                          graph_line_addch(line, ' ');
                  } else {
			...the code above...

I don't know if that just means the continue here is redundant and can
be removed, or if it's a sign of a larger logic error.

-Peff