From: Pablo Sabater <pabloosabaterr@gmail.com>
Date: Thu, 14 May 2026 10:19:01 GMT
Subject: Re: [GSoC PATCH v3 1/1] graph: add indentation for commits preceded by a parentless commit
Message-ID: <CAN5EUNSxyT5EyTf8b4evbW+JbDeRms91zQEn_JgiinOgvpe6mQ@mail.gmail.com>
In-Reply-To: <20260513230216.GA1378627@coredump.intra.peff.net>

El jue, 14 may 2026 a las 1:02, Jeff King (<peff@peff.net>) escribió:
>
> 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

It is dead code. The behaviour for placeholder at
"graph_output_commit_line()" and "graph_output_post_merge_line()" is
the same, if it's a placeholder print a padding instead of an edge,
but I didn't give it a second thought, graph_output_commit_line() can
have a placeholder at its right (that's why it needs the continue to
avoid extra padding) but post merge can't and as it is dead code I
didn't notice.
I'll drop the dead code.

Thanks,

--
Pablo