All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Pablo Sabater <pabloosabaterr@gmail.com>
Cc: git@vger.kernel.org, gitster@pobox.com,
	christian.couder@gmail.com, karthik.188@gmail.com,
	jltobler@gmail.com, ayu.chandekar@gmail.com,
	siddharthasthana31@gmail.com, chandrapratap3519@gmail.com
Subject: Re: [GSoC PATCH v3 1/1] graph: add indentation for commits preceded by a parentless commit
Date: Wed, 13 May 2026 19:02:16 -0400	[thread overview]
Message-ID: <20260513230216.GA1378627@coredump.intra.peff.net> (raw)
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

  reply	other threads:[~2026-05-13 23:02 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-02 21:17 [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root Pablo Sabater
2026-04-02 21:17 ` [GSoC RFC PATCH 1/1] " Pablo Sabater
2026-04-03 17:55   ` Junio C Hamano
2026-04-03 18:07     ` Pablo
2026-04-03  5:04 ` [GSoC RFC PATCH 0/1] " Junio C Hamano
2026-04-03  8:25   ` Pablo
2026-04-04  9:24 ` [GSoC RFC PATCH v2 0/1] graph: add indentation for commits preceded by a parentless commit Pablo Sabater
2026-04-04  9:24   ` [GSoC RFC PATCH v2 1/1] " Pablo Sabater
2026-04-10 16:25   ` [GSoC RFC PATCH v2 0/1] " Pablo
2026-04-10 16:54     ` Junio C Hamano
2026-04-27 10:28   ` [GSoC PATCH v3 " Pablo Sabater
2026-04-27 10:28     ` [GSoC PATCH v3 1/1] " Pablo Sabater
2026-05-13 23:02       ` Jeff King [this message]
2026-05-14 10:19         ` Pablo Sabater
2026-04-27 10:35     ` [GSoC PATCH v3 0/1] " Pablo
2026-05-14 15:15 ` [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root Phillip Wood
2026-05-14 17:45   ` Pablo Sabater
2026-05-15  9:33     ` Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260513230216.GA1378627@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=ayu.chandekar@gmail.com \
    --cc=chandrapratap3519@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jltobler@gmail.com \
    --cc=karthik.188@gmail.com \
    --cc=pabloosabaterr@gmail.com \
    --cc=siddharthasthana31@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.