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

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

From
Noam Postavsky <npostavs@users.sourceforge.net>
Date
Sep 2, 2018, 00:34 UTC
Message-ID
<CAM-tV-_=4WuMGemm6RTB902-m8JfMKGp_OkQFuJMagPE8bOOtg@mail.gmail.com>
In-Reply-To
<20180806212603.GA21026@sigill.intra.peff.net>
On 6 August 2018 at 17:26, Jeff King <peff@peff.net> wrote:
Show 5 quoted lines
> 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.

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
Previous: Jeff KingNext: Jeff King
Message 13 of 22 in “[BUG] A part of an edge from an octopus merge gets colored, even with --color=never”
  1. Noam PostavskyMay 15, 2016
  2. Johannes SixtMay 17, 2016
  3. Jeff KingMay 17, 2016
  4. Jeff KingMay 17, 2016
  5. Jeff KingMay 17, 2016
  6. Noam PostavskyMay 20, 2016
  7. Noam PostavskyJun 23, 2018
  8. Jeff KingJun 25, 2018
  9. Noam PostavskyJun 30, 2018
  10. Noam PostavskyAug 6, 2018
  11. Jeff KingAug 6, 2018
  12. Jeff KingAug 6, 2018
  13. Noam PostavskySep 2, 2018
  14. Jeff KingSep 8, 2018
  15. Noam PostavskySep 25, 2018
  16. Noam PostavskyOct 3, 2018
  17. Jeff KingOct 3, 2018
  18. Noam PostavskyOct 3, 2018
  19. Jeff KingOct 9, 2018
  20. Noam PostavskyOct 10, 2018
  21. Johannes SixtMay 17, 2016
  22. Jeff KingMay 17, 2016

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.