{"thread":{"id":"13701","subject":"log --graph: extra space with --pretty=oneline","startedAt":"2008-05-28T11:24:05Z","lastAt":"2008-06-02T04:41:11Z","messageCount":12,"participants":["Teemu Likonen","Wincent Colaiuta","Adam Simpkins","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"77928","messageId":"20080528112405.GA12065@mithlond.arda.local","threadId":"13701","inReplyTo":null,"subject":"log --graph: extra space with --pretty=oneline","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2008-05-28T11:24:05Z","receivedAt":"2008-05-28T11:24:05Z","isPatch":false,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"Sometimes \"log --graph --pretty=oneline\" prints a sort of broken graph\nline. In the git repository try this:\n\n$ git log --graph --pretty=oneline --abbrev-commit -4 8366b7b\n\n\nM   8366b7b... Merge branch 'maint'\n|\\  \n| M   a2f5be5... Merge branch 'jk/maint-send-email-compose' into maint\n| |\\  \n| M  \\  93c7b9c... Merge branch 'hb/maint-send-email-quote-recipients' into maint\n| |\\  | \n| M  \\ \\  6abf189... Merge branch 'maint-1.5.4' into maint\n| |\\  | |\n    ^\n\nExtra spaces there. I don't mind that myself but to some users it may\nlook like a bug. Maybe one would expect output like this:\n\n\nM   8366b7b... Merge branch 'maint'\n|\\  \n| M   a2f5be5... Merge branch 'jk/maint-send-email-compose' into maint\n| |\\  \n| | \\\n| M  \\  93c7b9c... Merge branch 'hb/maint-send-email-quote-recipients' into maint\n| |\\  \\ \n| | \\  \\\n| M  \\  |  6abf189... Merge branch 'maint-1.5.4' into maint\n| |\\  | |\n\n\nIt requires more lines though.\n"},{"id":"77929","messageId":"D3963BB7-4F3C-4D4F-9423-D03F05100F10@wincent.com","threadId":"13701","inReplyTo":"20080528112405.GA12065@mithlond.arda.local","subject":"Re: log --graph: extra space with --pretty=oneline","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2008-05-28T11:34:47Z","receivedAt":"2008-05-28T11:34:47Z","isPatch":false,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 28/5/2008, a las 13:24, Teemu Likonen escribió:\n> Sometimes \"log --graph --pretty=oneline\" prints a sort of broken graph\n> line. In the git repository try this:\n>\n> $ git log --graph --pretty=oneline --abbrev-commit -4 8366b7b\n>\n>\n> M   8366b7b... Merge branch 'maint'\n> |\\\n> | M   a2f5be5... Merge branch 'jk/maint-send-email-compose' into maint\n> | |\\\n> | M  \\  93c7b9c... Merge branch 'hb/maint-send-email-quote- \n> recipients' into maint\n> | |\\  |\n> | M  \\ \\  6abf189... Merge branch 'maint-1.5.4' into maint\n> | |\\  | |\n>    ^\n>\n> Extra spaces there. I don't mind that myself but to some users it may\n> look like a bug. Maybe one would expect output like this:\n>\n>\n> M   8366b7b... Merge branch 'maint'\n> |\\\n> | M   a2f5be5... Merge branch 'jk/maint-send-email-compose' into maint\n> | |\\\n> | | \\\n> | M  \\  93c7b9c... Merge branch 'hb/maint-send-email-quote- \n> recipients' into maint\n> | |\\  \\\n> | | \\  \\\n> | M  \\  |  6abf189... Merge branch 'maint-1.5.4' into maint\n> | |\\  | |\n>\n>\n> It requires more lines though.\n\nYes it does, but it definitely looks more readable to me.\n\nW\n"},{"id":"78024","messageId":"20080529085752.GA31865@adamsimpkins.net","threadId":"13701","inReplyTo":"20080528112405.GA12065@mithlond.arda.local","subject":"Re: log --graph: extra space with --pretty=oneline","fromName":"Adam Simpkins","fromEmail":"adam@adamsimpkins.net","sentAt":"2008-05-29T08:57:53Z","receivedAt":"2008-05-29T08:57:53Z","isPatch":false,"sender":{"key":"adam@adamsimpkins.net","avatar":"https://gravatar.com/avatar/d3fd2c0b3e2d2136b56e95726ee03227bee4eb562f627dcd3ebca0623fa05054?d=mp&s=160"},"body":"On Wed, May 28, 2008 at 02:24:05PM +0300, Teemu Likonen wrote:\n> Sometimes \"log --graph --pretty=oneline\" prints a sort of broken graph\n> line. In the git repository try this:\n> \n> $ git log --graph --pretty=oneline --abbrev-commit -4 8366b7b\n> \n> [current graph output removed]\n> \n> Extra spaces there. I don't mind that myself but to some users it may\n> look like a bug. Maybe one would expect output like this:\n> \n> \n> M   8366b7b... Merge branch 'maint'\n> |\\  \n> | M   a2f5be5... Merge branch 'jk/maint-send-email-compose' into maint\n> | |\\  \n> | | \\\n> | M  \\  93c7b9c... Merge branch 'hb/maint-send-email-quote-recipients' into maint\n> | |\\  \\ \n> | | \\  \\\n> | M  \\  |  6abf189... Merge branch 'maint-1.5.4' into maint\n> | |\\  | |\n> \n> It requires more lines though.\n\n\nHmm.  Yes, the output could definitely be improved.  This problem\noccurs for merges with 2 parents (not octopus merges), and only when\nthe branch line to the right of the merge was moving right at the end\nof the previous commit (i.e., it was displayed with '\\' instead of '|'\nor '/').\n\nNote that this doesn't require extra lines to fix:\n\nM   8366b7b... Merge branch 'maint'\n|\\  \n| M   a2f5be5... Merge branch 'jk/maint-send-email-compose' into maint\n| |\\  \n| M \\  93c7b9c... Merge branch 'hb/maint-send-email-quote-recipients' into maint\n| |\\ \\ \n| M \\ \\  6abf189... Merge branch 'maint-1.5.4' into maint\n| |\\ \\ \\\n\nThis can easily be implemented by changing the output for 2-way merges\nfrom something like this:\n\n| M  \\\n| |\\  |\n\nto this:\n\n| M \\\n| |\\ \\\n\nHowever, I find this makes the graph slightly uglier when the incoming\nbranch to the right of the merge wasn't '\\' on the previous line.  The\nfollowing change seems to look better when the branch line was '|' or\n'/' on the previous line:\n\n| M |\n| |\\ \\\n\nFor comparison, here's a comparison of several scenarios of how the\noutput looks now, and how it would look with these fixes.\n\nCurrent behavior        Option 1                 Option 2\n\n| |\\                    | |\\                     | |\\\n| M  \\                  | M \\                    | M |\n| |\\  |                 | |\\ \\                   | |\\ \\\n\n| | |                   | | |                    | | |\n| M  \\                  | M \\                    | M |\n| |\\  |                 | |\\ \\                   | |\\ \\\n\n| |  /                  | |  /                   | |  /\n| M  \\                  | M \\                    | M |\n| |\\  |                 | |\\ \\                   | |\\ \\\n\n| | |/                  | | |/                   | | |/\n| M  \\                  | M \\                    | M |\n| |\\  |                 | |\\ \\                   | |\\ \\\n\nNote that in the case of octopus merges, the current code already\nproduces output like that of option 1.\n\nHowever, I find that both options 1 and 2 look a little uglier than\nthe current behavior when the branch lines need to be collapsed again\nafter the merge.  The graph has more pointy angles, but it is still\nreadable in all cases:\n\nCurrent behavior        Option 1                 Option 2\n\n| * | |                 | * | |                  | * | |\n| M  \\ \\                | M \\ \\                  | M | |\n| |\\  | |               | |\\ \\ \\                 | |\\ \\ \\\n|/ / / /                |/ / / /                 |/ / / /\n\nFor sections of the graph where there are several merge commits in a\nrow, I think option 1 looks the best:\n\nCurrent behavior        Option 1                 Option 2\n\n* |                     * |                      * |\nM  \\                    M \\                      M |\n|\\  |                   |\\ \\                     |\\ \\\nM  \\ \\                  M \\ \\                    M | |\n|\\  | |                 |\\ \\ \\                   |\\ \\ \\\nM  \\ \\ \\                M \\ \\ \\                  M | | |\n|\\  | | |               |\\ \\ \\ \\                 |\\ \\ \\ \\\n\n\nWhat is everyone's preference between the 3 options?  Personally, I'm\nleaning towards Option 2.\n\nI'll send out some informal patches of both option 1 and option 2, for\ncomparison.\n\nIn the future, the code could even be improved to dynamically choose\nbetween these three options based on the output printed for the\nprevious commit.  Currently it doesn't store enough information from\nthe previous commit to do this.\n\n-- \nAdam Simpkins\nadam@adamsimpkins.net\n"},{"id":"78025","messageId":"1212051825-32102-1-git-send-email-adam@adamsimpkins.net","threadId":"13701","inReplyTo":"20080529085752.GA31865@adamsimpkins.net","subject":"[PATCH] graph API: improve output for merge commits (option 1)","fromName":"Adam Simpkins","fromEmail":"adam@adamsimpkins.net","sentAt":"2008-05-29T09:03:45Z","receivedAt":"2008-05-29T09:03:45Z","isPatch":true,"sender":{"key":"adam@adamsimpkins.net","avatar":"https://gravatar.com/avatar/d3fd2c0b3e2d2136b56e95726ee03227bee4eb562f627dcd3ebca0623fa05054?d=mp&s=160"},"body":"This eliminates the extra space that sometimes appeared in branch lines\nto the right of 2-way merge commits.  (It appeared when the branch line was\ndisplayed as '\\' on the line just before the merge commit.)\n\nFor example,\n\n| |\\\n| M  \\\n| |\\  |\n\nis now displayed as\n\n| |\\\n| M \\\n| |\\ \\\n\nSigned-off-by: Adam Simpkins <adam@adamsimpkins.net>\n---\n graph.c |    8 ++------\n 1 files changed, 2 insertions(+), 6 deletions(-)\n\ndiff --git a/graph.c b/graph.c\nindex 26b8c52..c3babcb 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -625,10 +625,8 @@ void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)\n \t\t\tseen_this = 1;\n \t\t\tgraph_output_commit_char(graph, sb);\n \n-\t\t\tif (graph->num_parents < 2)\n+\t\t\tif (graph->num_parents < 3)\n \t\t\t\tstrbuf_addch(sb, ' ');\n-\t\t\telse if (graph->num_parents == 2)\n-\t\t\t\tstrbuf_addstr(sb, \"  \");\n \t\t\telse {\n \t\t\t\tint num_dashes =\n \t\t\t\t\t((graph->num_parents - 2) * 2) - 1;\n@@ -679,9 +677,7 @@ void graph_output_post_merge_line(struct git_graph *graph, struct strbuf *sb)\n \t\t\tstrbuf_addch(sb, '|');\n \t\t\tfor (j = 0; j < graph->num_parents - 1; j++)\n \t\t\t\tstrbuf_addstr(sb, \"\\\\ \");\n-\t\t\tif (graph->num_parents == 2)\n-\t\t\t\tstrbuf_addch(sb, ' ');\n-\t\t} else if (seen_this && (graph->num_parents > 2)) {\n+\t\t} else if (seen_this) {\n \t\t\tstrbuf_addstr(sb, \"\\\\ \");\n \t\t} else {\n \t\t\tstrbuf_addstr(sb, \"| \");\n-- \n1.5.6.rc0.46.gd2b3.dirty\n"},{"id":"78026","messageId":"1212051856-32138-1-git-send-email-adam@adamsimpkins.net","threadId":"13701","inReplyTo":"20080529085752.GA31865@adamsimpkins.net","subject":"[PATCH] graph API: improve output for merge commits (option 2)","fromName":"Adam Simpkins","fromEmail":"adam@adamsimpkins.net","sentAt":"2008-05-29T09:04:16Z","receivedAt":"2008-05-29T09:04:16Z","isPatch":true,"sender":{"key":"adam@adamsimpkins.net","avatar":"https://gravatar.com/avatar/d3fd2c0b3e2d2136b56e95726ee03227bee4eb562f627dcd3ebca0623fa05054?d=mp&s=160"},"body":"This eliminates the extra space that sometimes appeared in branch lines\nto the right of 2-way merge commits.  (It appeared when the branch line was\ndisplayed as '\\' on the line just before the merge commit.)\n\nFor example,\n\n| |\\\n| M  \\\n| |\\  |\n\nis now displayed as\n\n| |\\\n| M |\n| |\\ \\\n\nThe output for octopus merges was also updated to be more\nsimilar to that for 2-way merges.\n\nSigned-off-by: Adam Simpkins <adam@adamsimpkins.net>\n---\n graph.c |   12 ++++--------\n 1 files changed, 4 insertions(+), 8 deletions(-)\n\ndiff --git a/graph.c b/graph.c\nindex 26b8c52..92f5b1a 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -535,7 +535,7 @@ static void graph_output_pre_commit_line(struct git_graph *graph,\n \t\tif (col->commit == graph->commit) {\n \t\t\tseen_this = 1;\n \t\t\tstrbuf_addf(sb, \"| %*s\", graph->expansion_row, \"\");\n-\t\t} else if (seen_this) {\n+\t\t} else if (seen_this && (graph->expansion_row > 0)) {\n \t\t\tstrbuf_addstr(sb, \"\\\\ \");\n \t\t} else {\n \t\t\tstrbuf_addstr(sb, \"| \");\n@@ -625,10 +625,8 @@ void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)\n \t\t\tseen_this = 1;\n \t\t\tgraph_output_commit_char(graph, sb);\n \n-\t\t\tif (graph->num_parents < 2)\n+\t\t\tif (graph->num_parents < 3)\n \t\t\t\tstrbuf_addch(sb, ' ');\n-\t\t\telse if (graph->num_parents == 2)\n-\t\t\t\tstrbuf_addstr(sb, \"  \");\n \t\t\telse {\n \t\t\t\tint num_dashes =\n \t\t\t\t\t((graph->num_parents - 2) * 2) - 1;\n@@ -636,7 +634,7 @@ void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)\n \t\t\t\t\tstrbuf_addch(sb, '-');\n \t\t\t\tstrbuf_addstr(sb, \". \");\n \t\t\t}\n-\t\t} else if (seen_this && (graph->num_parents > 1)) {\n+\t\t} else if (seen_this && (graph->num_parents > 2)) {\n \t\t\tstrbuf_addstr(sb, \"\\\\ \");\n \t\t} else {\n \t\t\tstrbuf_addstr(sb, \"| \");\n@@ -679,9 +677,7 @@ void graph_output_post_merge_line(struct git_graph *graph, struct strbuf *sb)\n \t\t\tstrbuf_addch(sb, '|');\n \t\t\tfor (j = 0; j < graph->num_parents - 1; j++)\n \t\t\t\tstrbuf_addstr(sb, \"\\\\ \");\n-\t\t\tif (graph->num_parents == 2)\n-\t\t\t\tstrbuf_addch(sb, ' ');\n-\t\t} else if (seen_this && (graph->num_parents > 2)) {\n+\t\t} else if (seen_this) {\n \t\t\tstrbuf_addstr(sb, \"\\\\ \");\n \t\t} else {\n \t\t\tstrbuf_addstr(sb, \"| \");\n-- \n1.5.6.rc0.46.gd2b3.dirty\n"},{"id":"78030","messageId":"20080529102549.GA11074@mithlond.arda.local","threadId":"13701","inReplyTo":"20080529085752.GA31865@adamsimpkins.net","subject":"Re: log --graph: extra space with --pretty=oneline","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2008-05-29T10:25:49Z","receivedAt":"2008-05-29T10:25:49Z","isPatch":false,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"\nAdam Simpkins wrote (2008-05-29 01:57 -0700):\n\n> What is everyone's preference between the 3 options?  Personally, I'm\n> leaning towards Option 2.\n\nI prefer the option 2 too. To me it never looks too ugly whereas the\nother two have some broken line or buggy-ish cases.\n\nDamn, this log --graph is so nice feature. I like it more than gitk,\nregardless of the fact that I have two monitors and could easily have\ngitk open and visible all the time.\n"},{"id":"78307","messageId":"1212353818-7031-1-git-send-email-adam@adamsimpkins.net","threadId":"13701","inReplyTo":"20080529085752.GA31865@adamsimpkins.net","subject":"[PATCH 0/2] graph API: improve printing of merges","fromName":"Adam Simpkins","fromEmail":"adam@adamsimpkins.net","sentAt":"2008-06-01T20:56:56Z","receivedAt":"2008-06-01T20:56:56Z","isPatch":true,"sender":{"key":"adam@adamsimpkins.net","avatar":"https://gravatar.com/avatar/d3fd2c0b3e2d2136b56e95726ee03227bee4eb562f627dcd3ebca0623fa05054?d=mp&s=160"},"body":"This is two minor patches to improve the display of merge commits in the\ngraph output.  The first fixes the \"extra space\" that appears sometimes,\nas pointed out by Teemu Likonen.  I had previously posted 2 simple\noptions for fixing the problem, but neither one was best in all cases.\nThis patch is an improved version that dynamically chooses how the merge\ncommit should be displayed, based on the last line of the previous\ncommit's output.\n\nFor example, with the new changes, the code now prints:\n\n$ git log --graph --pretty=format:%h -10 8d6afc1\nM   8d6afc1\n|\\  \n| M   f2fea68\n| |\\  \n| M \\   21dbe12\n| |\\ \\  \nM | \\ \\   41094b8\n|\\ \\ \\ \\  \n| M \\ \\ \\   061ad5f\n| |\\ \\ \\ \\  \n| M \\ \\ \\ \\   fe041ad\n| |\\ \\ \\ \\ \\  \n| | \\ \\ \\ \\ \\     \n| |  \\ \\ \\ \\ \\    \n| M-. \\ \\ \\ \\ \\   cd1333d\n| |\\ \\ \\ \\ \\ \\ \\  \n| | * | | | | | | cfcbd34\n| | * | | | | | | 5398fed\n| M | | | | | | |   539d84f\n| |\\ \\ \\ \\ \\ \\ \\ \\  \n\n\nThe second patch improves the output for octopus merges, by avoiding\nprinting unnecessary padding lines before the commit when there aren't\nany existing branch lines to the right of the merge.\n\nAdam Simpkins (2):\n  graph API: improve display of merge commits\n  graph API: avoid printing unnecessary padding before some octopus\n    merges\n\n graph.c |  123 +++++++++++++++++++++++++++++++++++++++++++++++++++-----------\n 1 files changed, 101 insertions(+), 22 deletions(-)\n"},{"id":"78309","messageId":"1212353818-7031-2-git-send-email-adam@adamsimpkins.net","threadId":"13701","inReplyTo":"1212353818-7031-1-git-send-email-adam@adamsimpkins.net","subject":"[PATCH 1/2] graph API: improve display of merge commits","fromName":"Adam Simpkins","fromEmail":"adam@adamsimpkins.net","sentAt":"2008-06-01T20:56:57Z","receivedAt":"2008-06-01T20:56:57Z","isPatch":true,"sender":{"key":"adam@adamsimpkins.net","avatar":"https://gravatar.com/avatar/d3fd2c0b3e2d2136b56e95726ee03227bee4eb562f627dcd3ebca0623fa05054?d=mp&s=160"},"body":"This change improves the way merge commits are displayed, to eliminate a\nfew visual artifacts.  Previously, merge commits were displayed as:\n\n| M  \\\n| |\\  |\n\nAs pointed out by Teemu Likonen, this didn't look nice if the rightmost\nbranch line was displayed as '\\' on the previous line, as it then\nappeared to have an extra space in it:\n\n| |\\\n| M  \\\n| |\\  |\n\nThis change updates the code so that branch lines to the right of merge\ncommits are printed slightly differently depending on how the previous\nline was displayed:\n\n| |\\          | | |        | |  /\n| M \\         | M |        | M |\n| |\\ \\        | |\\ \\       | |\\ \\\n\nSigned-off-by: Adam Simpkins <adam@adamsimpkins.net>\n---\n graph.c |  110 +++++++++++++++++++++++++++++++++++++++++++++++++++++----------\n 1 files changed, 93 insertions(+), 17 deletions(-)\n\ndiff --git a/graph.c b/graph.c\nindex 26b8c52..332d1e8 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -81,6 +81,27 @@ struct git_graph {\n \t */\n \tenum graph_state state;\n \t/*\n+\t * The output state for the previous line of output.\n+\t * This is primarily used to determine how the first merge line\n+\t * should appear, based on the last line of the previous commit.\n+\t */\n+\tenum graph_state prev_state;\n+\t/*\n+\t * The index of the column that refers to this commit.\n+\t *\n+\t * If none of the incoming columns refer to this commit,\n+\t * this will be equal to num_columns.\n+\t */\n+\tint commit_index;\n+\t/*\n+\t * The commit_index for the previously displayed commit.\n+\t *\n+\t * This is used to determine how the first line of a merge\n+\t * graph output should appear, based on the last line of the\n+\t * previous commit.\n+\t */\n+\tint prev_commit_index;\n+\t/*\n \t * The maximum number of columns that can be stored in the columns\n \t * and new_columns arrays.  This is also half the number of entries\n \t * that can be stored in the mapping and new_mapping arrays.\n@@ -137,6 +158,9 @@ struct git_graph *graph_init(struct rev_info *opt)\n \tgraph->num_parents = 0;\n \tgraph->expansion_row = 0;\n \tgraph->state = GRAPH_PADDING;\n+\tgraph->prev_state = GRAPH_PADDING;\n+\tgraph->commit_index = 0;\n+\tgraph->prev_commit_index = 0;\n \tgraph->num_columns = 0;\n \tgraph->num_new_columns = 0;\n \tgraph->mapping_size = 0;\n@@ -164,6 +188,12 @@ void graph_release(struct git_graph *graph)\n \tfree(graph);\n }\n \n+static void graph_update_state(struct git_graph *graph, enum graph_state s)\n+{\n+\tgraph->prev_state = graph->state;\n+\tgraph->state = s;\n+}\n+\n static void graph_ensure_capacity(struct git_graph *graph, int num_columns)\n {\n \tif (graph->column_capacity >= num_columns)\n@@ -342,6 +372,7 @@ static void graph_update_columns(struct git_graph *graph)\n \t\tif (col_commit == graph->commit) {\n \t\t\tint old_mapping_idx = mapping_idx;\n \t\t\tseen_this = 1;\n+\t\t\tgraph->commit_index = i;\n \t\t\tfor (parent = graph->commit->parents;\n \t\t\t     parent;\n \t\t\t     parent = parent->next) {\n@@ -395,6 +426,13 @@ void graph_update(struct git_graph *graph, struct commit *commit)\n \t}\n \n \t/*\n+\t * Store the old commit_index in prev_commit_index.\n+\t * graph_update_columns() will update graph->commit_index for this\n+\t * commit.\n+\t */\n+\tgraph->prev_commit_index = graph->commit_index;\n+\n+\t/*\n \t * Call graph_update_columns() to update\n \t * columns, new_columns, and mapping.\n \t */\n@@ -404,6 +442,9 @@ void graph_update(struct git_graph *graph, struct commit *commit)\n \n \t/*\n \t * Update graph->state.\n+\t * Note that we don't call graph_update_state() here, since\n+\t * we don't want to update graph->prev_state.  No line for\n+\t * graph->state was ever printed.\n \t *\n \t * If the previous commit didn't get to the GRAPH_PADDING state,\n \t * it never finished its output.  Goto GRAPH_SKIP, to print out\n@@ -498,9 +539,9 @@ static void graph_output_skip_line(struct git_graph *graph, struct strbuf *sb)\n \tgraph_pad_horizontally(graph, sb);\n \n \tif (graph->num_parents >= 3)\n-\t\tgraph->state = GRAPH_PRE_COMMIT;\n+\t\tgraph_update_state(graph, GRAPH_PRE_COMMIT);\n \telse\n-\t\tgraph->state = GRAPH_COMMIT;\n+\t\tgraph_update_state(graph, GRAPH_COMMIT);\n }\n \n static void graph_output_pre_commit_line(struct git_graph *graph,\n@@ -535,7 +576,22 @@ static void graph_output_pre_commit_line(struct git_graph *graph,\n \t\tif (col->commit == graph->commit) {\n \t\t\tseen_this = 1;\n \t\t\tstrbuf_addf(sb, \"| %*s\", graph->expansion_row, \"\");\n-\t\t} else if (seen_this) {\n+\t\t} else if (seen_this && (graph->expansion_row == 0)) {\n+\t\t\t/*\n+\t\t\t * This is the first line of the pre-commit output.\n+\t\t\t * If the previous commit was a merge commit and\n+\t\t\t * ended in the GRAPH_POST_MERGE state, all branch\n+\t\t\t * lines after graph->prev_commit_index were\n+\t\t\t * printed as \"\\\" on the previous line.  Continue\n+\t\t\t * to print them as \"\\\" on this line.  Otherwise,\n+\t\t\t * print the branch lines as \"|\".\n+\t\t\t */\n+\t\t\tif (graph->prev_state == GRAPH_POST_MERGE &&\n+\t\t\t    graph->prev_commit_index < i)\n+\t\t\t\tstrbuf_addstr(sb, \"\\\\ \");\n+\t\t\telse\n+\t\t\t\tstrbuf_addstr(sb, \"| \");\n+\t\t} else if (seen_this && (graph->expansion_row > 0)) {\n \t\t\tstrbuf_addstr(sb, \"\\\\ \");\n \t\t} else {\n \t\t\tstrbuf_addstr(sb, \"| \");\n@@ -550,7 +606,7 @@ static void graph_output_pre_commit_line(struct git_graph *graph,\n \t */\n \tgraph->expansion_row++;\n \tif (graph->expansion_row >= num_expansion_rows)\n-\t\tgraph->state = GRAPH_COMMIT;\n+\t\tgraph_update_state(graph, GRAPH_COMMIT);\n }\n \n static void graph_output_commit_char(struct git_graph *graph, struct strbuf *sb)\n@@ -625,10 +681,8 @@ void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)\n \t\t\tseen_this = 1;\n \t\t\tgraph_output_commit_char(graph, sb);\n \n-\t\t\tif (graph->num_parents < 2)\n+\t\t\tif (graph->num_parents < 3)\n \t\t\t\tstrbuf_addch(sb, ' ');\n-\t\t\telse if (graph->num_parents == 2)\n-\t\t\t\tstrbuf_addstr(sb, \"  \");\n \t\t\telse {\n \t\t\t\tint num_dashes =\n \t\t\t\t\t((graph->num_parents - 2) * 2) - 1;\n@@ -636,8 +690,27 @@ void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)\n \t\t\t\t\tstrbuf_addch(sb, '-');\n \t\t\t\tstrbuf_addstr(sb, \". \");\n \t\t\t}\n-\t\t} else if (seen_this && (graph->num_parents > 1)) {\n+\t\t} else if (seen_this && (graph->num_parents > 2)) {\n \t\t\tstrbuf_addstr(sb, \"\\\\ \");\n+\t\t} else if (seen_this && (graph->num_parents == 2)) {\n+\t\t\t/*\n+\t\t\t * This is a 2-way merge commit.\n+\t\t\t * There is no GRAPH_PRE_COMMIT stage for 2-way\n+\t\t\t * merges, so this is the first line of output\n+\t\t\t * for this commit.  Check to see what the previous\n+\t\t\t * line of output was.\n+\t\t\t *\n+\t\t\t * If it was GRAPH_POST_MERGE, the branch line\n+\t\t\t * coming into this commit may have been '\\',\n+\t\t\t * and not '|' or '/'.  If so, output the branch\n+\t\t\t * line as '\\' on this line, instead of '|'.  This\n+\t\t\t * makes the output look nicer.\n+\t\t\t */\n+\t\t\tif (graph->prev_state == GRAPH_POST_MERGE &&\n+\t\t\t    graph->prev_commit_index < i)\n+\t\t\t\tstrbuf_addstr(sb, \"\\\\ \");\n+\t\t\telse\n+\t\t\t\tstrbuf_addstr(sb, \"| \");\n \t\t} else {\n \t\t\tstrbuf_addstr(sb, \"| \");\n \t\t}\n@@ -649,11 +722,11 @@ void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)\n \t * Update graph->state\n \t */\n \tif (graph->num_parents > 1)\n-\t\tgraph->state = GRAPH_POST_MERGE;\n+\t\tgraph_update_state(graph, GRAPH_POST_MERGE);\n \telse if (graph_is_mapping_correct(graph))\n-\t\tgraph->state = GRAPH_PADDING;\n+\t\tgraph_update_state(graph, GRAPH_PADDING);\n \telse\n-\t\tgraph->state = GRAPH_COLLAPSING;\n+\t\tgraph_update_state(graph, GRAPH_COLLAPSING);\n }\n \n void graph_output_post_merge_line(struct git_graph *graph, struct strbuf *sb)\n@@ -679,9 +752,7 @@ void graph_output_post_merge_line(struct git_graph *graph, struct strbuf *sb)\n \t\t\tstrbuf_addch(sb, '|');\n \t\t\tfor (j = 0; j < graph->num_parents - 1; j++)\n \t\t\t\tstrbuf_addstr(sb, \"\\\\ \");\n-\t\t\tif (graph->num_parents == 2)\n-\t\t\t\tstrbuf_addch(sb, ' ');\n-\t\t} else if (seen_this && (graph->num_parents > 2)) {\n+\t\t} else if (seen_this) {\n \t\t\tstrbuf_addstr(sb, \"\\\\ \");\n \t\t} else {\n \t\t\tstrbuf_addstr(sb, \"| \");\n@@ -694,9 +765,9 @@ void graph_output_post_merge_line(struct git_graph *graph, struct strbuf *sb)\n \t * Update graph->state\n \t */\n \tif (graph_is_mapping_correct(graph))\n-\t\tgraph->state = GRAPH_PADDING;\n+\t\tgraph_update_state(graph, GRAPH_PADDING);\n \telse\n-\t\tgraph->state = GRAPH_COLLAPSING;\n+\t\tgraph_update_state(graph, GRAPH_COLLAPSING);\n }\n \n void graph_output_collapsing_line(struct git_graph *graph, struct strbuf *sb)\n@@ -801,7 +872,7 @@ void graph_output_collapsing_line(struct git_graph *graph, struct strbuf *sb)\n \t * Otherwise, we need to collapse some branch lines together.\n \t */\n \tif (graph_is_mapping_correct(graph))\n-\t\tgraph->state = GRAPH_PADDING;\n+\t\tgraph_update_state(graph, GRAPH_PADDING);\n }\n \n int graph_next_line(struct git_graph *graph, struct strbuf *sb)\n@@ -865,6 +936,11 @@ void graph_padding_line(struct git_graph *graph, struct strbuf *sb)\n \t}\n \n \tgraph_pad_horizontally(graph, sb);\n+\n+\t/*\n+\t * Update graph->prev_state since we have output a padding line\n+\t */\n+\tgraph->prev_state = GRAPH_PADDING;\n }\n \n int graph_is_commit_finished(struct git_graph const *graph)\n-- \n1.5.6.rc0.54.g04bfd\n"},{"id":"78308","messageId":"1212353818-7031-3-git-send-email-adam@adamsimpkins.net","threadId":"13701","inReplyTo":"1212353818-7031-2-git-send-email-adam@adamsimpkins.net","subject":"[PATCH 2/2] graph API: avoid printing unnecessary padding before some octopus merges","fromName":"Adam Simpkins","fromEmail":"adam@adamsimpkins.net","sentAt":"2008-06-01T20:56:58Z","receivedAt":"2008-06-01T20:56:58Z","isPatch":true,"sender":{"key":"adam@adamsimpkins.net","avatar":"https://gravatar.com/avatar/d3fd2c0b3e2d2136b56e95726ee03227bee4eb562f627dcd3ebca0623fa05054?d=mp&s=160"},"body":"When an octopus merge is printed, several lines are printed before it to\nmove over existing branch lines to its right.  This is needed to make\nroom for the children of the octopus merge.  For example:\n\n| | | |\n| |  \\ \\\n| |   \\ \\\n| |    \\ \\\n| M---. \\ \\\n| |\\ \\ \\ \\ \\\n\nHowever, this step isn't necessary if there are no branch lines to the\nright of the octopus merge.  Therefore, skip this step when it is not\nneeded, to avoid printing extra lines that don't really serve any\npurpose.\n\nSigned-off-by: Adam Simpkins <adam@adamsimpkins.net>\n---\n graph.c |   13 ++++++++-----\n 1 files changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/graph.c b/graph.c\nindex 332d1e8..0531716 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -450,16 +450,18 @@ void graph_update(struct git_graph *graph, struct commit *commit)\n \t * it never finished its output.  Goto GRAPH_SKIP, to print out\n \t * a line to indicate that portion of the graph is missing.\n \t *\n-\t * Otherwise, if there are 3 or more parents, we need to print\n-\t * extra rows before the commit, to expand the branch lines around\n-\t * it and make room for it.\n+\t * If there are 3 or more parents, we may need to print extra rows\n+\t * before the commit, to expand the branch lines around it and make\n+\t * room for it.  We need to do this unless there aren't any branch\n+\t * rows to the right of this commit.\n \t *\n \t * If there are less than 3 parents, we can immediately print the\n \t * commit line.\n \t */\n \tif (graph->state != GRAPH_PADDING)\n \t\tgraph->state = GRAPH_SKIP;\n-\telse if (graph->num_parents >= 3)\n+\telse if (graph->num_parents >= 3 &&\n+\t\t graph->commit_index < (graph->num_columns - 1))\n \t\tgraph->state = GRAPH_PRE_COMMIT;\n \telse\n \t\tgraph->state = GRAPH_COMMIT;\n@@ -538,7 +540,8 @@ static void graph_output_skip_line(struct git_graph *graph, struct strbuf *sb)\n \tstrbuf_addstr(sb, \"...\");\n \tgraph_pad_horizontally(graph, sb);\n \n-\tif (graph->num_parents >= 3)\n+\tif (graph->num_parents >= 3 &&\n+\t    graph->commit_index < (graph->num_columns - 1))\n \t\tgraph_update_state(graph, GRAPH_PRE_COMMIT);\n \telse\n \t\tgraph_update_state(graph, GRAPH_COMMIT);\n-- \n1.5.6.rc0.54.g04bfd\n"},{"id":"78311","messageId":"7vskvwakph.fsf@gitster.siamese.dyndns.org","threadId":"13701","inReplyTo":"1212353818-7031-3-git-send-email-adam@adamsimpkins.net","subject":"Re: [PATCH 2/2] graph API: avoid printing unnecessary padding before some octopus merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-01T21:50:34Z","receivedAt":"2008-06-01T21:50:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Simpkins <adam@adamsimpkins.net> writes:\n\n> When an octopus merge is printed, several lines are printed before it to\n> move over existing branch lines to its right.  This is needed to make\n> room for the children of the octopus merge.  For example:\n>\n> | | | |\n> | |  \\ \\\n> | |   \\ \\\n> | |    \\ \\\n> | M---. \\ \\\n> | |\\ \\ \\ \\ \\\n>\n> However, this step isn't necessary if there are no branch lines to the\n> right of the octopus merge.  Therefore, skip this step when it is not\n> needed, to avoid printing extra lines that don't really serve any\n> purpose.\n>\n> Signed-off-by: Adam Simpkins <adam@adamsimpkins.net>\n> ---\n>  graph.c |   13 ++++++++-----\n>  1 files changed, 8 insertions(+), 5 deletions(-)\n>\n> diff --git a/graph.c b/graph.c\n> index 332d1e8..0531716 100644\n> --- a/graph.c\n> +++ b/graph.c\n> @@ -450,16 +450,18 @@ void graph_update(struct git_graph *graph, struct commit *commit)\n>  \t * it never finished its output.  Goto GRAPH_SKIP, to print out\n>  \t * a line to indicate that portion of the graph is missing.\n>  \t *\n> -\t * Otherwise, if there are 3 or more parents, we need to print\n> -\t * extra rows before the commit, to expand the branch lines around\n> -\t * it and make room for it.\n> +\t * If there are 3 or more parents, we may need to print extra rows\n> +\t * before the commit, to expand the branch lines around it and make\n> +\t * room for it.  We need to do this unless there aren't any branch\n> +\t * rows to the right of this commit.\n\nDouble negation like this is confusing, isn't it?\n\n\"We do not have to do this if there isn't any branch row to the right of\nthis commit\" may be better.  \"We need to do this only if there is a branch\nrow (or more) to the right of this commit\" would probably be better.\n\nOther than that, the code looks sane to me.\n"},{"id":"78322","messageId":"20080602000441.GA9291@adamsimpkins.net","threadId":"13701","inReplyTo":"7vskvwakph.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] graph API: avoid printing unnecessary padding before some octopus merges","fromName":"Adam Simpkins","fromEmail":"adam@adamsimpkins.net","sentAt":"2008-06-02T00:04:42Z","receivedAt":"2008-06-02T00:04:42Z","isPatch":true,"sender":{"key":"adam@adamsimpkins.net","avatar":"https://gravatar.com/avatar/d3fd2c0b3e2d2136b56e95726ee03227bee4eb562f627dcd3ebca0623fa05054?d=mp&s=160"},"body":"On Sun, Jun 01, 2008 at 02:50:34PM -0700, Junio C Hamano wrote:\n> Adam Simpkins <adam@adamsimpkins.net> writes:\n> \n> > diff --git a/graph.c b/graph.c\n> > index 332d1e8..0531716 100644\n> > --- a/graph.c\n> > +++ b/graph.c\n> > @@ -450,16 +450,18 @@ void graph_update(struct git_graph *graph, struct commit *commit)\n> >  \t * it never finished its output.  Goto GRAPH_SKIP, to print out\n> >  \t * a line to indicate that portion of the graph is missing.\n> >  \t *\n> > -\t * Otherwise, if there are 3 or more parents, we need to print\n> > -\t * extra rows before the commit, to expand the branch lines around\n> > -\t * it and make room for it.\n> > +\t * If there are 3 or more parents, we may need to print extra rows\n> > +\t * before the commit, to expand the branch lines around it and make\n> > +\t * room for it.  We need to do this unless there aren't any branch\n> > +\t * rows to the right of this commit.\n> \n> Double negation like this is confusing, isn't it?\n> \n> \"We do not have to do this if there isn't any branch row to the right of\n> this commit\" may be better.  \"We need to do this only if there is a branch\n> row (or more) to the right of this commit\" would probably be better.\n\nYes, I agree it is less confusing without the double negation.  Your\nsecond choice of wording sounds best.\n\nHow do you prefer to fix simple things like this?  Do you want to just\napply the fix yourself, or is it easier for you if I submit an amended\npatch?\n\n-- \nAdam Simpkins\nadam@adamsimpkins.net\n"},{"id":"78326","messageId":"7vod6ka1p4.fsf@gitster.siamese.dyndns.org","threadId":"13701","inReplyTo":"20080602000441.GA9291@adamsimpkins.net","subject":"Re: [PATCH 2/2] graph API: avoid printing unnecessary padding before some octopus merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-02T04:41:11Z","receivedAt":"2008-06-02T04:41:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Simpkins <adam@adamsimpkins.net> writes:\n\n> How do you prefer to fix simple things like this?  Do you want to just\n> apply the fix yourself, or is it easier for you if I submit an amended\n> patch?\n\nFor a small thing like this, it's probably easiest if you said: \"Yeah, use\nthat phrasing\" (or \"It would be even better to say this way: ...\") would\nbe good enough.  I know how to operate my editor ;-).\n"}]}