{"thread":{"id":"35200","subject":"[PATCH] graph.c: visual difference on subsequent series","startedAt":"2013-10-25T16:07:48Z","lastAt":"2014-01-03T20:16:03Z","messageCount":11,"participants":["Milton Soares Filho","Junio C Hamano","Keshav Kini","Thomas Rast"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"229520","messageId":"1382717268-21884-1-git-send-email-milton.soares.filho@gmail.com","threadId":"35200","inReplyTo":null,"subject":"[PATCH] graph.c: visual difference on subsequent series","fromName":"Milton Soares Filho","fromEmail":"milton.soares.filho@gmail.com","sentAt":"2013-10-25T16:07:48Z","receivedAt":"2013-10-25T16:07:48Z","isPatch":true,"sender":{"key":"milton.soares.filho@gmail.com","avatar":"https://gravatar.com/avatar/d31bcd8aa5b1556462c04ce486389a9daf6096b741aa6f546e5a6f0010f1f4c4?d=mp&s=160"},"body":"For projects with separate history lines and, thus, multiple root-commits, the\nlinear arrangement of `git log --graph --oneline` does not allow the user to\nspot where the sequence ends, giving the impression that it's a contiguous\nhistory. E.g.\n\nHistory sequence A: a1 -- a2 -- a3 (root-commit)\nHistory sequence B: b1 -- b2 -- b3 (root-commit)\n\n git log --graph --oneline\n * a1\n * a2\n * a3\n * b1\n * b2\n * b3\n\nIn a GUI tool, the root-commit of each series would stand out on the graph.\n\nThis modification changes the commit char to a different symbol ('x'), so users\nof the command-line graph tool can easily identify root-commits and make sense\nof where each series is limited to.\n\n git log --graph --oneline\n * a1\n * a2\n x a3\n * b1\n * b2\n x b3\n\nSigned-off-by: Milton Soares Filho \n---\n graph.c | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/graph.c b/graph.c\nindex b24d04c..ec8e960 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -780,6 +780,15 @@ static void graph_output_commit_char(struct git_graph *graph, struct strbuf *sb)\n \t}\n \n \t/*\n+\t * Out-stand parentless commits to enforce non-continuity on subsequent\n+\t * but separate series\n+\t */\n+\tif (graph->commit->parents == NULL) {\n+\t\tstrbuf_addch(sb, 'x');\n+\t\treturn;\n+\t}\n+\n+\t/*\n \t * get_revision_mark() handles all other cases without assert()\n \t */\n \tstrbuf_addstr(sb, get_revision_mark(graph->revs, graph->commit));\n-- \n1.8.1.2\n"},{"id":"229523","messageId":"xmqqeh79jmtr.fsf@gitster.dls.corp.google.com","threadId":"35200","inReplyTo":"1382717268-21884-1-git-send-email-milton.soares.filho@gmail.com","subject":"Re: [PATCH] graph.c: visual difference on subsequent series","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-25T17:13:20Z","receivedAt":"2013-10-25T17:13:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Milton Soares Filho writes:\n\n> git log --graph --oneline\n> * a1\n> * a2\n> x a3\n> * b1\n> * b2\n> x b3\n\nI agree that the problem you are trying to solve is a good thing to\ntackle, and I also agree that marking a root commit differently from\nother commits is one way to solve it, but I am not sure if that is\nthe best way. If the stretches of a's and b's in your history are\nvery long, wouldn't it be easier to spot if they are painted in\ndifferent colours, in addition to or instead of marking the roots\ndifferently [*1*], for example?\n\n> \t/*\n> +\t * Out-stand parentless commits to enforce non-continuity on subsequent\n> +\t * but separate series\n> +\t */\n> +\tif (graph->commit->parents == NULL) {\n> +\t\tstrbuf_addch(sb, 'x');\n> +\t\treturn;\n> +\t}\n> +\n> +\t/*\n> \t * get_revision_mark() handles all other cases without assert()\n> \t */\n> \tstrbuf_addstr(sb, get_revision_mark(graph->revs, graph->commit));\n\nIt is unclear why the update goes to this function. At the first\nglance, I feel that it would be more sensible to add the equivalent\ncode to get_revision_mark()---we do not have to worry about what\nelse, other than calling get_revision_mark() and adding it to sb,\nwould be skipped by the added \"return\" when we later have to update\nthis function and add more code after the existing strbuf_addstr().\n\nThe change implemented your way will lose other information when a\nroot commit is at the boundary, marked as uninteresting, or on the\nleft/right side of traversal (when --left-right is requested). I\nthink these pieces of information your patch seems to be losing are\na lot more relevant than \"have we hit the root?\", especially in the\nmajority of repositories where there is only one root commit.\n\nThanks.\n\n\n[Footnote]\n\n*1* Note that I am not saying \"the change the patch introduces is\nnot sufficient and you have to paint the commits in different\ncolors\" here. I myself think it would be a lot more work to do so,\nand I even suspect that it may be asking for the moon---you may not\neven know what root \"a1\" (and \"b1\") came from when you are showing\nthese commits without first digging down to the roots and then\nwalking the history backwards, which may not be practically\nfeasible.\n"},{"id":"229525","messageId":"CAPNngRMP29s9gZg9R987yRd2qJ=UuaMWnFphtQdGDRgG_SCxsQ@mail.gmail.com","threadId":"35200","inReplyTo":"xmqqeh79jmtr.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] graph.c: visual difference on subsequent series","fromName":"Milton Soares Filho","fromEmail":"milton.soares.filho@gmail.com","sentAt":"2013-10-25T20:49:43Z","receivedAt":"2013-10-25T20:49:43Z","isPatch":true,"sender":{"key":"milton.soares.filho@gmail.com","avatar":"https://gravatar.com/avatar/d31bcd8aa5b1556462c04ce486389a9daf6096b741aa6f546e5a6f0010f1f4c4?d=mp&s=160"},"body":"On 25 October 2013 15:13, Junio C Hamano wrote:\n> Milton Soares Filho writes:\n>\n>> git log --graph --oneline\n>> * a1\n>> * a2\n>> x a3\n>> * b1\n>> * b2\n>> x b3\n>\n> I agree that the problem you are trying to solve is a good thing to\n> tackle, and I also agree that marking a root commit differently from\n> other commits is one way to solve it, but I am not sure if that is\n> the best way. If the stretches of a's and b's in your history are\n> very long, wouldn't it be easier to spot if they are painted in\n> different colours, in addition to or instead of marking the roots\n> differently [*1*], for example?\n\nThanks for taking your time reviewing this patch, Junio. I didn't really thought\nit would get any attention since multiple root-commits is not a very common\nuse-case[1]. However, if most people got excited with git-subtree new\nfeatures as I did, there is a good chance that multiple root-commits are\ngoing to become a common-place in the near future ;-)\n\nThat said, I completely agree that painting with different colors would be\na much better fix, however I believe that it can be done in a separate\nchangeset by someone that understands better the impact on the rest\nof the system. Personally, changing only the mark is sufficient because:\n\na) it'll work on terminal types without coloring support and configurations\n whose explicitly disable it\nb) it'll spare myself of running a separate GUI program just\n to spot where each series begin\nc) it won't require any visual design skills from a developer (me)\n without a minimal sense for it :-)\n\nBy the way, is there a visual or design guideline document for building\ndecorated log graphs? From where comes the inspiration of it?\n\n> The change implemented your way will lose other information when a\n> root commit is at the boundary, marked as uninteresting, or on the\n> left/right side of traversal (when --left-right is requested). I\n> think these pieces of information your patch seems to be losing are\n> a lot more relevant than \"have we hit the root?\", especially in the\n> majority of repositories where there is only one root commit.\n\nNice. I'll try to move the logic into get_revision_mark() and hope\nthe priority on handling it is better suited.\n\n> [...]\n> and I even suspect that it may be asking for the moon---you may not\n> even know what root \"a1\" (and \"b1\") came from when you are showing\n> these commits without first digging down to the roots and then\n> walking the history backwards, which may not be practically\n> feasible.\n\nIt'd be nice to figure out a test-case to emerge it.\n\n[]s, milton\n\n[1]: In git repository itself I could find only seven of them (root-commis)\n"},{"id":"229543","messageId":"87mwlwn4e0.fsf@gmail.com","threadId":"35200","inReplyTo":"CAPNngRMP29s9gZg9R987yRd2qJ=UuaMWnFphtQdGDRgG_SCxsQ@mail.gmail.com","subject":"Re: [PATCH] graph.c: visual difference on subsequent series","fromName":"Keshav Kini","fromEmail":"keshav.kini@gmail.com","sentAt":"2013-10-26T02:37:59Z","receivedAt":"2013-10-26T02:37:59Z","isPatch":true,"sender":{"key":"keshav.kini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/691290?v=4"},"body":"Milton Soares Filho writes:\n> On 25 October 2013 15:13, Junio C Hamano wrote:\n>> Milton Soares Filho writes:\n>>\n>>> git log --graph --oneline\n>>> * a1\n>>> * a2\n>>> x a3\n>>> * b1\n>>> * b2\n>>> x b3\n>>\n>> I agree that the problem you are trying to solve is a good thing to\n>> tackle, and I also agree that marking a root commit differently from\n>> other commits is one way to solve it, but I am not sure if that is\n>> the best way. If the stretches of a's and b's in your history are\n>> very long, wouldn't it be easier to spot if they are painted in\n>> different colours, in addition to or instead of marking the roots\n>> differently [*1*], for example?\n>\n> Thanks for taking your time reviewing this patch, Junio. I didn't really thought\n> it would get any attention since multiple root-commits is not a very common\n> use-case[1]. However, if most people got excited with git-subtree new\n> features as I did, there is a good chance that multiple root-commits are\n> going to become a common-place in the near future ;-)\n\nI don't think this is that obscure. I've often thought there should be\nsome way to distinguish root commits as well. In fact when dealing with\nmultiple root commits I usually just don't use --oneline and instead use\nthe full --graph view so I can find root commits by grepping for '^ ' :)\n\nI should also mention that there are lots of situations where you might\nsee multiple \"root commits\" not because there are truly multiple commits\nwith no parent in the repository, but because you're looking at some\nsubgraph of the history graph -- that is, you have multiple commits in\nyour display whose parents are purposely excluded. For example, you\nmight be looking at a revision list like 'C ^A ^B':\n\n master\n | .---------------B\n | / `-------------.\n O< .---`--C\n | \\ /\n | `---------------A\n\nThe commits you were looking at would be these ones:\n\n `-------------.\n .---`--C\n /\n\nSo multiple \"roots\" can appear easily in such cases.\n\n> That said, I completely agree that painting with different colors would be\n> a much better fix, however I believe that it can be done in a separate\n> changeset by someone that understands better the impact on the rest\n> of the system. Personally, changing only the mark is sufficient because:\n>\n> a) it'll work on terminal types without coloring support and configurations\n> whose explicitly disable it\n> b) it'll spare myself of running a separate GUI program just\n> to spot where each series begin\n> c) it won't require any visual design skills from a developer (me)\n> without a minimal sense for it :-)\n\nI'm a bit worried that if someone is parsing `git log --graph` output\nlooking for `*` lines they might suddenly start missing the root commits\nthat they were previously able to find. I mean, not that anyone should\nbe doing that, but if we can avoid breaking that, why not do so?\n\nWhat about just putting an extra blank line after every root commit line\n(possibly except the last one)? That should make it plenty easy to see\nwhere the root commits are in --oneline mode. I think it would actually\nbe easier to spot at a glance than replacing `*` with `x` because it\ncreates a gap in all columns of the output, rather than only in column\n1. Also, this is very subjective but I think it looks kind of ugly to\nuse \"x\" :P\n\nBy the by, you might want to use the `-v` argument to `git send-email`\nso that people reading the list can tell at a glance which patch\nversions are newer than which other patch versions.\n\n-Keshav\n"},{"id":"229643","messageId":"xmqqeh75h087.fsf@gitster.dls.corp.google.com","threadId":"35200","inReplyTo":"87mwlwn4e0.fsf@gmail.com","subject":"Re: [PATCH] graph.c: visual difference on subsequent series","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-28T15:41:12Z","receivedAt":"2013-10-28T15:41:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"[administrivia: please avoid culling addresses from To:/Cc: lines]\n\nKeshav Kini writes:\n\n> What about just putting an extra blank line after every root commit line\n> (possibly except the last one)? That should make it plenty easy to see\n> where the root commits are in --oneline mode. I think it would actually\n> be easier to spot at a glance than replacing `*` with `x` because it\n> creates a gap in all columns of the output, rather than only in column\n> 1. Also, this is very subjective but I think it looks kind of ugly to\n> use \"x\" :P\n\nI agree to all of the above, including the ugliness of 'x' ;-)\n\nA \"blank\" may however be hard to spot, if the range is limited,\nthough. For example,\n\n $ git log --graph --oneline a4..\n * HEAD\n /* a1\n | * a2\n | * a3\n * b1\n * b2\n * b3\n\nwhere \"a4\", which is a root, is the sole parent of \"a3\" and HEAD is\na merge between \"a1\" and \"b1\" might produce something like this,\nwhile we may get this from the same history, when shown unlimited:\n\n $ git log --graph --oneline\n * HEAD\n /* a1\n | * a2\n | * a3\n | * a4\n |\n * b1\n * b2\n * b3\n\nA divider line might make it visually a lot more strong, i.e.\n\n $ git log --graph --oneline\n * HEAD\n /* a1\n | * a2\n | * a3\n | * a4\n | ~~~~~~~~~~~~~~~~~~~~~~~\n * b1\n * b2\n * b3\n\nbut I am not sure if it is too distracting.\n"},{"id":"229649","messageId":"87fvrljpq4.fsf@gmail.com","threadId":"35200","inReplyTo":"xmqqeh75h087.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] graph.c: visual difference on subsequent series","fromName":"Keshav Kini","fromEmail":"keshav.kini@gmail.com","sentAt":"2013-10-28T16:59:47Z","receivedAt":"2013-10-28T16:59:47Z","isPatch":true,"sender":{"key":"keshav.kini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/691290?v=4"},"body":"Junio C Hamano writes:\n> [administrivia: please avoid culling addresses from To:/Cc: lines]\n\nYikes, sorry about that. I've been sending messages through Gmane\nrather than via email, and I didn't realize the list didn't\nautomatically send messages to the appropriate people who are only\nreading the list via actual email (as I am not such a person).\n\n> Keshav Kini writes:\n>> What about just putting an extra blank line after every root commit line\n>> (possibly except the last one)? That should make it plenty easy to see\n>> where the root commits are in --oneline mode. I think it would actually\n>> be easier to spot at a glance than replacing `*` with `x` because it\n>> creates a gap in all columns of the output, rather than only in column\n>> 1. Also, this is very subjective but I think it looks kind of ugly to\n>> use \"x\" :P\n>\n> I agree to all of the above, including the ugliness of 'x' ;-)\n>\n> A \"blank\" may however be hard to spot, if the range is limited,\n> though. For example,\n>\n> $ git log --graph --oneline a4..\n> * HEAD\n> /* a1\n> | * a2\n> | * a3\n> * b1\n> * b2\n> * b3\n>\n> where \"a4\", which is a root, is the sole parent of \"a3\" and HEAD is\n> a merge between \"a1\" and \"b1\" might produce something like this,\n> while we may get this from the same history, when shown unlimited:\n>\n> $ git log --graph --oneline\n> * HEAD\n> /* a1\n> | * a2\n> | * a3\n> | * a4\n> |\n> * b1\n> * b2\n> * b3\n>\n> A divider line might make it visually a lot more strong, i.e.\n>\n> $ git log --graph --oneline\n> * HEAD\n> /* a1\n> | * a2\n> | * a3\n> | * a4\n> | ~~~~~~~~~~~~~~~~~~~~~~~\n> * b1\n> * b2\n> * b3\n>\n> but I am not sure if it is too distracting.\n\nI would be fine with that, fwiw. We can also turn it on and off with a\nconfig option if people really don't like it, I suppose...\n\n-Keshav\n"},{"id":"229651","messageId":"CAPNngRMprE3QwDn3y74QqitAs+-DCBm1oO33uKRHsn9jLrNSnA@mail.gmail.com","threadId":"35200","inReplyTo":"xmqqeh75h087.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] graph.c: visual difference on subsequent series","fromName":"Milton Soares Filho","fromEmail":"milton.soares.filho@gmail.com","sentAt":"2013-10-28T17:18:31Z","receivedAt":"2013-10-28T17:18:31Z","isPatch":true,"sender":{"key":"milton.soares.filho@gmail.com","avatar":"https://gravatar.com/avatar/d31bcd8aa5b1556462c04ce486389a9daf6096b741aa6f546e5a6f0010f1f4c4?d=mp&s=160"},"body":"On 28 October 2013 13:41, Junio C Hamano wrote:\n> I agree to all of the above, including the ugliness of 'x' ;-)\n>\n> A \"blank\" may however be hard to spot, if the range is limited,\n> though. For example,\n\nA 'x' looks like termination points in some specification languages\nsuch as SDL and MSC and thus translates directly to the idea of a\nroot-commit, at least IMO. For sure it does not stand out as blatantly\nas it should, but it gives a general idea without further\ndistractions, which seems to be the idea of a simple 'git log --graph\n--oneline'.\n\nAn idea that have just come to mind is to have a decorator to enforce\nthis property, like this.\n\n * HEAD\n /* a1\n | * a2\n | * a3\n | x a4 (root-commit)\n * b1\n * b2\n x b3 (root-commit)\n\nThis way the user only gets 'distracted' if he explicitly asks for it\n(--decorate), with all its colors and whatnot. What do you think?\nShould I aim for it?\n\nBesides anything else, this discussion is becoming very subjective.\nI've received private feedbacks thanking for the changeset and not a\nword against the poor 'x'. Maybe it's time to talk to a UI designer or\nlet a benevolent dictator set this quarrel off ;-)\n\n[]s, milton\n"},{"id":"229652","messageId":"xmqqsivlfg6z.fsf@gitster.dls.corp.google.com","threadId":"35200","inReplyTo":"CAPNngRMprE3QwDn3y74QqitAs+-DCBm1oO33uKRHsn9jLrNSnA@mail.gmail.com","subject":"Re: [PATCH] graph.c: visual difference on subsequent series","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-28T17:39:16Z","receivedAt":"2013-10-28T17:39:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Milton Soares Filho writes:\n\n> On 28 October 2013 13:41, Junio C Hamano wrote:\n>> I agree to all of the above, including the ugliness of 'x' ;-)\n>>\n>> A \"blank\" may however be hard to spot, if the range is limited,\n>> though. For example,\n>\n> A 'x' looks like termination points in some specification languages\n> such as SDL and MSC and thus translates directly to the idea of a\n> root-commit, at least IMO. For sure it does not stand out as blatantly\n> as it should, but it gives a general idea without further\n> distractions, which seems to be the idea of a simple 'git log --graph\n> --oneline'.\n>\n> An idea that have just come to mind is to have a decorator to enforce\n> this property, like this.\n>\n> * HEAD\n> /* a1\n> | * a2\n> | * a3\n> | x a4 (root-commit)\n> * b1\n> * b2\n> x b3 (root-commit)\n>\n> This way the user only gets 'distracted' if he explicitly asks for it\n> (--decorate), with all its colors and whatnot. What do you think?\n> Should I aim for it?\n>\n> Besides anything else, this discussion is becoming very subjective.\n\nIf I have to choose, I'd rather avoid using 'x' or anything that\nhave to override '*', not just 'x' being ugly, but the approach to\n_replace_ the \"revision-mark\" (usually '*' but sometimes '<', '^',\netc) forces us to give priority between \"root-ness\" and other kinds\nof information (e.g. \"left-ness\"). That was the primary reason I\nliked Keshav's suggestion to use one extra line _below_ the root,\nwhich will allow us to still keep the existing information unlike\nwhat we discussed in our back-and-forth during the initial review.\n\nI also think a blank (or divider) below the root commits does make\nit visually obvious that nothing comes _before_ the root commit in\nthe history, which probably even removes the need to paint the\ntracks of histories leading to different roots in different colours.\n\nI hope the above shows that my reaction was much less subjective\nthan my response sounded ;-)\n\nThanks.\n"},{"id":"232292","messageId":"xmqqbo0be0hc.fsf_-_@gitster.dls.corp.google.com","threadId":"35200","inReplyTo":"xmqqsivlfg6z.fsf@gitster.dls.corp.google.com","subject":"[RFH/PATCH] graph: give an extra gap after showing root commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-20T20:22:39Z","receivedAt":"2013-12-20T20:22:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With a history with more than one root commit, if a root commit\nfalls on one display column and another commit that is unrelated to\nthat root's history is shown on the next line on the same column,\nthe resulting graph would appear as if the latter is a parent of the\nformer, like this (there are two histories a1-a2 & b1-b2):\n\n ------------------------------------------\n $ git log --graph --oneline a2 b2\n * a2\n * a1\n * b2\n * b1\n ------------------------------------------\n\nwhich is misleading. b2 is a tip of a history unrelated to the\nhistory that has a1 as a root.\n\nForce a gap line immediately before showing next unrelated commit if\nwe showed a root from one history, to make the above display look\nlike this instead:\n\n ------------------------------------------\n $ git log --graph --oneline a2 b2\n * a2\n * a1\n\n * b2\n * b1\n ------------------------------------------\n\nbut do not waste line when we do not have to. E.g. with a history\nthat merges a2 and b2,\n\n ------------------------------------------\n $ git log --graph --oneline m\n * Merge a2 and b2\n |\\\n | * a2\n | * a1\n * b2\n * b1\n ------------------------------------------\n\nthere is no need to show an extra blank line after showing a1, as\nit is clear that it has no parents.\n\nThis takes inspiration from Milton Soares Filho's \"graph.c: mark\nroot commit differently\" from a few months ago ($gmane/236708),\nwhich tried to show a root commit as 'x', but only if it would have\nbeen shown as '*' in the original, but uses a different approach so\nthat a root that may have been painted differently from '*'\n(e.g. '<' for \"left root\") can also be made distinguishable.\n\nSigned-off-by: Junio C Hamano \n\n---\n\n Note that this still does not work very well for --boundary case\n (see the last test added to t6016).\n\n It may actually make sense to force the \"next\" commit after showing\n a root always occupy a different column, instead of wasting a blank\n line. If we did so, the output from the first example may look like\n this:\n\n ------------------------------------------\n $ git log --graph --oneline a2 b2\n * a2\n * a1\n * b2\n * b1\n ------------------------------------------\n\n or it may even look like this:\n\n ------------------------------------------\n $ git log --graph --oneline a2 b2\n * a2\n * a1\n * b2\n /\n * b1\n ------------------------------------------\n\n I tried to follow graph_update_columns() logic but gave up for now;\n avoiding to place a commit whose descendant has already been seen\n to the same column as the root commit we are processing there is\n easy, but the current commit may not yet be in columns[], and\n graph_output_commit_line() needs to show the current commit beyond\n the end of the columns[] list in such a case, so teaching\n graph_update_columns() to tweak placement of the next line is not\n sufficient.\n\n graph.c | 20 ++++++++++++++++--\n t/t6016-rev-list-graph-simplify-history.sh | 34 ++++++++++++++++++++++++++++++\n 2 files changed, 52 insertions(+), 2 deletions(-)\n\ndiff --git a/graph.c b/graph.c\nindex b24d04c..2c3f141 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -187,6 +187,10 @@ struct git_graph {\n \t * stored as an index into the array column_colors.\n \t */\n \tunsigned short default_column_color;\n+\t/* The last one was a root and we haven't emitted an extra blank */\n+\tunsigned need_post_root_gap : 1;\n+\t/* Are we looking at the root? */\n+\tunsigned is_root : 1;\n };\n \n static struct strbuf *diff_output_prefix_callback(struct diff_options *opt, void *data)\n@@ -205,7 +209,7 @@ static struct strbuf *diff_output_prefix_callback(struct diff_options *opt, void\n \n struct git_graph *graph_init(struct rev_info *opt)\n {\n-\tstruct git_graph *graph = xmalloc(sizeof(struct git_graph));\n+\tstruct git_graph *graph = xcalloc(1, sizeof(struct git_graph));\n \n \tif (!column_colors)\n \t\tgraph_set_column_colors(column_colors_ansi,\n@@ -552,11 +556,14 @@ static void graph_update_columns(struct git_graph *graph)\n void graph_update(struct git_graph *graph, struct commit *commit)\n {\n \tstruct commit_list *parent;\n+\tint was_root = graph->is_root;\n \n \t/*\n \t * Set the new commit\n \t */\n \tgraph->commit = commit;\n+\tgraph->is_root = !commit->parents;\n+\tgraph->need_post_root_gap = 0;\n \n \t/*\n \t * Count how many interesting parents this commit has\n@@ -607,8 +614,12 @@ void graph_update(struct git_graph *graph, struct commit *commit)\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+\telse {\n \t\tgraph->state = GRAPH_COMMIT;\n+\t\tif (was_root &&\n+\t\t graph->prev_commit_index == graph->commit_index)\n+\t\t\tgraph->need_post_root_gap = 1;\n+\t}\n }\n \n static int graph_is_mapping_correct(struct git_graph *graph)\n@@ -814,6 +825,11 @@ static void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)\n \tint seen_this = 0;\n \tint i, chars_written;\n \n+\tif (graph->need_post_root_gap) {\n+\t\tgraph->need_post_root_gap = 0;\n+\t\tstrbuf_addch(sb, '\\n');\n+\t}\n+\n \t/*\n \t * Output the row containing this commit\n \t * Iterate up to and including graph->num_columns,\ndiff --git a/t/t6016-rev-list-graph-simplify-history.sh b/t/t6016-rev-list-graph-simplify-history.sh\nindex f7181d1..ca53a80 100755\n--- a/t/t6016-rev-list-graph-simplify-history.sh\n+++ b/t/t6016-rev-list-graph-simplify-history.sh\n@@ -264,4 +264,38 @@ test_expect_success '--graph --boundary ^C3' '\n \ttest_cmp expected actual\n \t'\n \n+one_independent_branch () {\n+\tgit checkout --orphan root$1 A1 &&\n+\ttest_commit root_$1 &&\n+\ttest_commit then_$1 &&\n+\ttest_commit further_$1\n+}\n+\n+test_expect_success 'multi-root setup' '\n+\tone_independent_branch 0 &&\n+\tone_independent_branch 1 &&\n+\tone_independent_branch 2 &&\n+\n+\tgit checkout -b merge210 root2 &&\n+\ttest_tick &&\n+\tgit merge -s ours root1 &&\n+\ttest_tick &&\n+\tgit merge -s ours root0\n+'\n+\n+test_expect_success 'multi-root does not emit unnecessary post-root gap' '\n+\tgit log --oneline --graph >actual &&\n+\t! grep \"^$\" actual\n+'\n+\n+test_expect_success 'multi-root does show necessary post-root gap' '\n+\tgit log --oneline --graph root0 root1 root2 >actual &&\n+\ttest $(grep -c \"^$\" actual) = 2\n+'\n+\n+test_expect_failure 'multi-root does not emit unnecessary post-root gap' '\n+\tgit log --oneline --graph merge210~1...merge210~1^2~2 >actual &&\n+\t! grep \"^$\" actual\n+'\n+\n test_done\n-- \n1.8.5.2-297-g3e57c29\n"},{"id":"232295","messageId":"xmqqy53fch96.fsf@gitster.dls.corp.google.com","threadId":"35200","inReplyTo":"xmqqbo0be0hc.fsf_-_@gitster.dls.corp.google.com","subject":"Re: [RFH/PATCH] graph: give an extra gap after showing root commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-20T22:03:17Z","receivedAt":"2013-12-20T22:03:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Note that this still does not work very well for --boundary case\n> (see the last test added to t6016).\n> ...\n> +test_expect_failure 'multi-root does not emit unnecessary post-root gap' '\n> +\tgit log --oneline --graph merge210~1...merge210~1^2~2 >actual &&\n> +\t! grep \"^$\" actual\n> +'\n\nObviously, this needs to be\n\n git log --oneline --graph --boundary merge210~1...merge210~1^2~2 >actual &&\n\nfor it to fail.\n"},{"id":"232638","messageId":"87sit4rfcs.fsf@thomasrast.ch","threadId":"35200","inReplyTo":"xmqqbo0be0hc.fsf_-_@gitster.dls.corp.google.com","subject":"Re: [RFH/PATCH] graph: give an extra gap after showing root commit","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2014-01-03T20:16:03Z","receivedAt":"2014-01-03T20:16:03Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Hi Junio,\n\nI briefly looked at d84a3da (jc/graph-post-root-gap) in pu, and have\nthis nit:\n\n> diff --git a/t/t6016-rev-list-graph-simplify-history.sh b/t/t6016-rev-list-graph-simplify-history.sh\n> [...]\n> +one_independent_branch () {\n> +\tgit checkout --orphan root$1 A1 &&\n> +\ttest_commit root_$1 &&\n\nThe naming of root0 etc. makes the test below rather confusing to read,\nbecause test_commit root_0 also creates a tag called root_0. So you set\nup history that has a tag root_0 that points *only* at the root, and a\nbranch root0 that includes two more commits.\n\n> +test_expect_failure 'multi-root does show necessary post-root gap' '\n> +\tsed -e \"s/ #$/ /\" >expect <<-\\EOF &&\n> +\t* further_2\n> +\t* then_2\n> +\t* root_2\n> +\t * further_1\n> +\t * then_1\n> +\t * root_1\n> +\t* further_0\n> +\t* then_0\n> +\t* root_0\n> +\tEOF\n> +\tgit log --graph --format=%s root0 root1 root2 >actual &&\n> +\ttest_cmp expect actual\n> +'\n\n-- \nThomas Rast\ntr@thomasrast.ch\n"}]}