{"thread":{"id":"35157","subject":"[PATCH] graph: Fix log's graph's colors when merging branches.","startedAt":"2013-10-17T06:52:09Z","lastAt":"2013-10-17T17:16:01Z","messageCount":3,"participants":["Hemmo Nieminen","John Keeping"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"229113","messageId":"20131017064853.GA7419@duunidell","threadId":"35157","inReplyTo":null,"subject":"[PATCH] graph: Fix log's graph's colors when merging branches.","fromName":"Hemmo Nieminen","fromEmail":"hemmo.nieminen@iki.fi","sentAt":"2013-10-17T06:52:09Z","receivedAt":"2013-10-17T06:52:09Z","isPatch":true,"sender":{"key":"hemmo.nieminen@iki.fi","avatar":null},"body":"\n\n\n>From bdbefc100ceba66a70f540dfe2b1b09b0869f7ab Mon Sep 17 00:00:00 2001\nFrom: Hemmo Nieminen <hemmo.nieminen@iki.fi>\nDate: Wed, 16 Oct 2013 11:28:50 +0300\nSubject: [PATCH] graph: Fix log's graph's colors when merging branches.\n\nThe log's graph's colors were off sometimes when merging several\nbranches. For example in the graph depicted below, the '-----.' part\nfollowing the asterisk was colored with incorrect colors. This was\ncaused by the fact that the leftmost branches, not part of the merge,\nwere not taken into account when calculating the column numbers\n(colors).\n\n  | |     Merge of http://members.cox.net/junkio/git-jc.git\n  | |\n  | *-----.   commit 211232bae64bcc60bbf5d1b5e5b2344c22ed767e\n  | |\\ \\ \\ \\  Merge: fc54a9c 9e30dd7 c4b83e6 6602659 b28858b\n  | | | | | | Author: Junio C Hamano <junkio@cox.net>\n\nSigned-off-by: Hemmo Nieminen <hemmo.nieminen@iki.fi>\n---\n graph.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/graph.c b/graph.c\nindex b24d04c..6404331 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -801,10 +801,10 @@ static int graph_draw_octopus_merge(struct git_graph *graph,\n \tint num_dashes =\n \t\t((graph->num_parents - dashless_commits) * 2) - 1;\n \tfor (i = 0; i < num_dashes; i++) {\n-\t\tcol_num = (i / 2) + dashless_commits;\n+\t\tcol_num = (i / 2) + dashless_commits + graph->commit_index;\n \t\tstrbuf_write_column(sb, &graph->new_columns[col_num], '-');\n \t}\n-\tcol_num = (i / 2) + dashless_commits;\n+\tcol_num = (i / 2) + dashless_commits + graph->commit_index;\n \tstrbuf_write_column(sb, &graph->new_columns[col_num], '.');\n \treturn num_dashes + 1;\n }\n-- \n1.8.4.1\n\n"},{"id":"229114","messageId":"20131017081336.GS27238@serenity.lan","threadId":"35157","inReplyTo":"20131017064853.GA7419@duunidell","subject":"Re: [PATCH] graph: Fix log's graph's colors when merging branches.","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-10-17T08:13:36Z","receivedAt":"2013-10-17T08:13:36Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Thu, Oct 17, 2013 at 09:52:09AM +0300, Hemmo Nieminen wrote:\n> The log's graph's colors were off sometimes when merging several\n> branches. For example in the graph depicted below, the '-----.' part\n> following the asterisk was colored with incorrect colors. This was\n> caused by the fact that the leftmost branches, not part of the merge,\n> were not taken into account when calculating the column numbers\n> (colors).\n> \n>   | |     Merge of http://members.cox.net/junkio/git-jc.git\n>   | |\n>   | *-----.   commit 211232bae64bcc60bbf5d1b5e5b2344c22ed767e\n>   | |\\ \\ \\ \\  Merge: fc54a9c 9e30dd7 c4b83e6 6602659 b28858b\n>   | | | | | | Author: Junio C Hamano <junkio@cox.net>\n> \n> Signed-off-by: Hemmo Nieminen <hemmo.nieminen@iki.fi>\n\nIt took me a minute to spot the problem when I tested this, but you're\nright that there is a bug and I agree that the patch below is the right\nfix.\n\nPerhaps a better commit message will help others looking at this, maybe\nsomething like this?\n\n    graph: fix coloring around octopus merges\n\n    When drawing the graph of an octopus merge, we draw a horizontal\n    line from parents 3 and above into the asterisk representing the\n    commit.  The sections of this line should be colored to match the\n    graph lines coming in from above.\n\n    However, if the commit is not in the left-most column we do not take\n    into account the columns to the left of the commit when calculating\n    these colors.  Fix this by adding the appropriate offset to the\n    column index used for calculating the color.\n    ---\n\n    Commit 211232b (Octopus merge of the following five patches.,\n    2005-05-05) in git.git's history exhibits this problem.\n\nThe section below the \"---\" does not become part of the commit message,\nwhen the patch is added to Git, but is used for notes that might help\nreviewers.\n\n> ---\n>  graph.c | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/graph.c b/graph.c\n> index b24d04c..6404331 100644\n> --- a/graph.c\n> +++ b/graph.c\n> @@ -801,10 +801,10 @@ static int graph_draw_octopus_merge(struct git_graph *graph,\n>  \tint num_dashes =\n>  \t\t((graph->num_parents - dashless_commits) * 2) - 1;\n>  \tfor (i = 0; i < num_dashes; i++) {\n> -\t\tcol_num = (i / 2) + dashless_commits;\n> +\t\tcol_num = (i / 2) + dashless_commits + graph->commit_index;\n>  \t\tstrbuf_write_column(sb, &graph->new_columns[col_num], '-');\n>  \t}\n> -\tcol_num = (i / 2) + dashless_commits;\n> +\tcol_num = (i / 2) + dashless_commits + graph->commit_index;\n>  \tstrbuf_write_column(sb, &graph->new_columns[col_num], '.');\n>  \treturn num_dashes + 1;\n>  }\n"},{"id":"229123","messageId":"20131017171601.GB3747@duunidell","threadId":"35157","inReplyTo":"20131017081336.GS27238@serenity.lan","subject":"Re: [PATCH] graph: Fix log's graph's colors when merging branches.","fromName":"Hemmo Nieminen","fromEmail":"hemmo.nieminen@iki.fi","sentAt":"2013-10-17T17:16:01Z","receivedAt":"2013-10-17T17:16:01Z","isPatch":true,"sender":{"key":"hemmo.nieminen@iki.fi","avatar":null},"body":"Hi,\n\nOn 09:13 Thu 17 Oct, John Keeping wrote:\n> It took me a minute to spot the problem when I tested this, but you're\n> right that there is a bug and I agree that the patch below is the right\n> fix.\n> \n> Perhaps a better commit message will help others looking at this, maybe\n> something like this?\n\nUpdated the patch. Attaching it here.\n\n\n-- \nBest Regards\nHemmo Nieminen\n\n\n\n>From 46b7ae56e08bc8ca9c29697da2c1210bd2a242b0 Mon Sep 17 00:00:00 2001\nFrom: Hemmo Nieminen <hemmo.nieminen@iki.fi>\nDate: Wed, 16 Oct 2013 11:28:50 +0300\nSubject: [PATCH v2] graph: fix coloring around octopus merges\n\nWhen drawing the graph of an octopus merge, we draw a horizontal line\nfrom parents 3 and above into the asterisk representing the commit. The\nsections of this line should be colored to match the graph lines coming\nin from above.\n\nHowever, if the commit is not in the left-most column we do not take\ninto account the columns to the left of the commit when calculating\nthese colors. Fix this by adding the appropriate offset to the column\nindex used for calculating the color.\n\nSigned-off-by: Hemmo Nieminen <hemmo.nieminen@iki.fi>\n---\n\nNotes:\n    Commit 211232b (Octopus merge of the following five patches.,\n    2005-05-05) in git.git's history exhibits this problem.\n\n graph.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/graph.c b/graph.c\nindex b24d04c..6404331 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -801,10 +801,10 @@ static int graph_draw_octopus_merge(struct git_graph *graph,\n \tint num_dashes =\n \t\t((graph->num_parents - dashless_commits) * 2) - 1;\n \tfor (i = 0; i < num_dashes; i++) {\n-\t\tcol_num = (i / 2) + dashless_commits;\n+\t\tcol_num = (i / 2) + dashless_commits + graph->commit_index;\n \t\tstrbuf_write_column(sb, &graph->new_columns[col_num], '-');\n \t}\n-\tcol_num = (i / 2) + dashless_commits;\n+\tcol_num = (i / 2) + dashless_commits + graph->commit_index;\n \tstrbuf_write_column(sb, &graph->new_columns[col_num], '.');\n \treturn num_dashes + 1;\n }\n-- \n1.8.4.1\n\n"}]}