Re: [BUG] A part of an edge from an octopus merge gets colored, even with --color=never

[Noam Postavsky <npostavs@users.sourceforge.net>]

[-- Attachment #1: Type: text/plain, Size: 486 bytes --]

On 6 August 2018 at 17:26, Jeff King <peff@peff.net> wrote:

> I suspect it still has a bug, which is that it is handling this
> first-parent-goes-left case, but probably gets the straight-parent case
> wrong. But at least in this form, I think it is obvious to see where
> that bug is (the "three" in the comment is not accurate in that latter
> case, and it should be two).

Yes, thanks, it makes a lot more sense this way. I believe the
attached handles both parent types correctly.

[-- Attachment #2: v3-0001-log-Fix-coloring-of-certain-octupus-merge-shapes.patch --]
[-- Type: text/x-diff, Size: 2819 bytes --]

From a841a50b016c0cfc9183384e6c3ca85a23d1e11f Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@users.sourceforge.net>
Date: Sat, 1 Sep 2018 20:07:16 -0400
Subject: [PATCH v3] log: Fix coloring of certain octupus merge shapes

For octopus merges where the first parent edge immediately merges into
the next column to the left:

| *-.
| |\ \
|/ / /

then the number of columns should be one less than the usual case:

| *-.
| |\ \
| | | *

Also refactor the code to iterate over columns rather than dashes,
building from an initial patch suggestion by Jeff King.

Signed-off-by: Noam Postavsky <npostavs@users.sourceforge.net>
---
 graph.c | 48 ++++++++++++++++++++++++++++++++++++------------
 1 file changed, 36 insertions(+), 12 deletions(-)

diff --git a/graph.c b/graph.c
index e1f6d3bdd..478c86dfb 100644
--- a/graph.c
+++ b/graph.c
@@ -848,21 +848,45 @@ static int graph_draw_octopus_merge(struct git_graph *graph,
 				    struct strbuf *sb)
 {
 	/*
-	 * Here dashless_commits represents the number of parents
-	 * which don't need to have dashes (because their edges fit
-	 * neatly under the commit).
+	 * Here dashless_commits represents the number of parents which don't
+	 * need to have dashes (because their edges fit neatly under the
+	 * commit).  And dashful_commits are the remaining ones.
 	 */
 	const int dashless_commits = 2;
-	int col_num, i;
-	int num_dashes =
-		((graph->num_parents - dashless_commits) * 2) - 1;
-	for (i = 0; i < num_dashes; i++) {
-		col_num = (i / 2) + dashless_commits + graph->commit_index;
-		strbuf_write_column(sb, &graph->new_columns[col_num], '-');
+	int dashful_commits = graph->num_parents - dashless_commits;
+
+	/*
+	 * Usually, each parent gets its own column, like this:
+	 *
+	 * | *-.
+	 * | |\ \
+	 * | | | *
+	 *
+	 * Sometimes the first parent goes into an existing column, like this:
+	 *
+	 * | *-.
+	 * | |\ \
+	 * |/ / /
+	 *
+	 */
+	int parent_in_existing_cols = graph->num_parents -
+		(graph->num_new_columns - graph->num_columns);
+
+	/*
+	 * Draw the dashes.  We start in the column following the
+	 * dashless_commits, but subtract out the parent which goes to an
+	 * existing column: we've already counted that column in commit_index.
+	 */
+	int first_col = graph->commit_index + dashless_commits
+		- parent_in_existing_cols;
+	int i;
+
+	for (i = 0; i < dashful_commits; i++) {
+		strbuf_write_column(sb, &graph->new_columns[i+first_col], '-');
+		strbuf_write_column(sb, &graph->new_columns[i+first_col],
+				    i == dashful_commits-1 ? '.' : '-');
 	}
-	col_num = (i / 2) + dashless_commits + graph->commit_index;
-	strbuf_write_column(sb, &graph->new_columns[col_num], '.');
-	return num_dashes + 1;
+	return 2 * dashful_commits;
 }
 
 static void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)
-- 
2.11.0