{"thread":{"id":"13792","subject":"log --graph --first-parent weirdness","startedAt":"2008-06-04T15:00:42Z","lastAt":"2008-06-05T18:31:10Z","messageCount":11,"participants":["Teemu Likonen","Junio C Hamano","Adam Simpkins","Ping Yin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"78628","messageId":"20080604150042.GA3038@mithlond.arda.local","threadId":"13792","inReplyTo":null,"subject":"log --graph --first-parent weirdness","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2008-06-04T15:00:42Z","receivedAt":"2008-06-04T15:00:42Z","isPatch":false,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"The output of \"git log --graph --first-parent\" seems weird. Maybe it's\nbecause I don't understand everything but here's an example anyway:\n\n\n$ git log --graph --pretty=oneline --abbrev-commit --decorate\n\nM   2ba1dba... (refs/heads/master) Merge branch 'topic'\n|\\  \n| * 432e062... (refs/heads/topic) Change from branch 'topic'\n* | b762236... Change from branch 'master'\n|/  \n* d7fb80a... Initial commit\n\n\nNormal --first-parent prints this:\n\n$ git log --first-parent --pretty=oneline --abbrev-commit\n\n2ba1dba... Merge branch 'topic'\nb762236... Change from branch 'master'\nd7fb80a... Initial commit\n\n\nWith --graph it goes:\n\n$ git log --graph --first-parent --pretty=oneline --abbrev-commit\n\nM   2ba1dba... Merge branch 'topic'\n|\\  \n* | b762236... Change from branch 'master'\n* | d7fb80a... Initial commit\n /\n\n\nSo, it prints the second parent line but it leads to nowhere. Try the\nsame with the git repository and you'll see a _lots_ of parallel branch\nlines which seem to go nowhere.\n"},{"id":"78629","messageId":"20080604150826.GB3038@mithlond.arda.local","threadId":"13792","inReplyTo":"20080604150042.GA3038@mithlond.arda.local","subject":"Re: log --graph --first-parent weirdness","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2008-06-04T15:08:26Z","receivedAt":"2008-06-04T15:08:26Z","isPatch":false,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"Teemu Likonen wrote (2008-06-04 18:00 +0300):\n\n> $ git log --graph --first-parent --pretty=oneline --abbrev-commit\n> \n> M   2ba1dba... Merge branch 'topic'\n> |\\  \n> * | b762236... Change from branch 'master'\n> * | d7fb80a... Initial commit\n>  /\n> \n> \n> So, it prints the second parent line but it leads to nowhere. Try the\n> same with the git repository and you'll see a _lots_ of parallel\n> branch lines which seem to go nowhere.\n\nAnd by the way, I'm pretty sure that some earlier log --graph version\nprinted only this with --first-parent:\n\nM   2ba1dba... Merge branch 'topic'\n|\\  \n*   b762236... Change from branch 'master'\n*   d7fb80a... Initial commit\n"},{"id":"78638","messageId":"7vmym1xgy4.fsf@gitster.siamese.dyndns.org","threadId":"13792","inReplyTo":"20080604150042.GA3038@mithlond.arda.local","subject":"Re: log --graph --first-parent weirdness","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-04T17:12:19Z","receivedAt":"2008-06-04T17:12:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Teemu Likonen <tlikonen@iki.fi> writes:\n\n> The output of \"git log --graph --first-parent\" seems weird.\n\nHeh, --first-parent means \"I'll view everything as a single strand of\npearls\".  Who in the right mind would use --graph at the same time to\nbegin with ;-)?\n\nWe could turn --graph automatically off if --first-parent is given, but\nI tend to agree with you that the right behaviour is to show the same\n\"everything prefixed with '| ', wasting two columns without good reason\"\noutput as you would see on a true linear history.\n"},{"id":"78640","messageId":"20080604173820.GA3038@mithlond.arda.local","threadId":"13792","inReplyTo":"7vmym1xgy4.fsf@gitster.siamese.dyndns.org","subject":"Re: log --graph --first-parent weirdness","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2008-06-04T17:38:20Z","receivedAt":"2008-06-04T17:38:20Z","isPatch":false,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"Junio C Hamano wrote (2008-06-04 10:12 -0700):\n\n> Teemu Likonen <tlikonen@iki.fi> writes:\n> \n> > The output of \"git log --graph --first-parent\" seems weird.\n> \n> Heh, --first-parent means \"I'll view everything as a single strand of\n> pearls\".  Who in the right mind would use --graph at the same time to\n> begin with ;-)?\n\nExactly :) I have an alias \"lg = log --graph\" and I almost always use\nthat instead of the normal log. Then I accidentally noticed that my \"git\nlg\" doesn't quite fit with --first-parent.\n\n> We could turn --graph automatically off if --first-parent is given,\n> but I tend to agree with you that the right behaviour is to show the\n> same \"everything prefixed with '| ', wasting two columns without good\n> reason\" output as you would see on a true linear history.\n\nTo me it's perfectly fine to turn off --graph when used with\n--first-parent, but yes, generally users might expect to see a line of\nM's, *'s and |'s there. At least it would clearly show which commits are\nmerges and which are not.\n"},{"id":"78644","messageId":"20080604180432.GA31437@adamsimpkins.net","threadId":"13792","inReplyTo":"20080604173820.GA3038@mithlond.arda.local","subject":"Re: log --graph --first-parent weirdness","fromName":"Adam Simpkins","fromEmail":"adam@adamsimpkins.net","sentAt":"2008-06-04T18:04:33Z","receivedAt":"2008-06-04T18:04:33Z","isPatch":false,"sender":{"key":"adam@adamsimpkins.net","avatar":"https://gravatar.com/avatar/d3fd2c0b3e2d2136b56e95726ee03227bee4eb562f627dcd3ebca0623fa05054?d=mp&s=160"},"body":"On Wed, Jun 04, 2008 at 08:38:20PM +0300, Teemu Likonen wrote:\n> Junio C Hamano wrote (2008-06-04 10:12 -0700):\n> \n> > Teemu Likonen <tlikonen@iki.fi> writes:\n> > \n> > > The output of \"git log --graph --first-parent\" seems weird.\n> \n> <snip>\n> \n> > We could turn --graph automatically off if --first-parent is given,\n> > but I tend to agree with you that the right behaviour is to show the\n> > same \"everything prefixed with '| ', wasting two columns without good\n> > reason\" output as you would see on a true linear history.\n> \n> To me it's perfectly fine to turn off --graph when used with\n> --first-parent, but yes, generally users might expect to see a line of\n> M's, *'s and |'s there. At least it would clearly show which commits are\n> merges and which are not.\n\nIt should be pretty simple to fix this as suggested.  There are two\nplaces in graph.c where we loop over the current commit's parents.\nChanging those to break out after the first commit when\nrevs->first_parent_only is set should result in the desired behavior.\n\nI'll try to get some time this evening or tomorrow to create a patch.\n\n-- \nAdam Simpkins\nadam@adamsimpkins.net\n"},{"id":"78645","messageId":"7v1w3dxeh9.fsf@gitster.siamese.dyndns.org","threadId":"13792","inReplyTo":"20080604173820.GA3038@mithlond.arda.local","subject":"Re: log --graph --first-parent weirdness","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-04T18:05:38Z","receivedAt":"2008-06-04T18:05:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Teemu Likonen <tlikonen@iki.fi> writes:\n\n> To me it's perfectly fine to turn off --graph when used with\n> --first-parent, but yes, generally users might expect to see a line of\n> M's, *'s and |'s there. At least it would clearly show which commits are\n> merges and which are not.\n\nI disagree.  If you are doing --first-parent, you do not _care_ what is\nmerge and what is not.  Besides, you can easily see that from the log\nmessage if you cared enough.\n\nAnd if the graph actually draws the real ancestry graph (i.e. without\n--first-parent), the lines visually show which is merge and which is not,\nso the \"M\" gets very distracting.\n\nI'd really suggest changing the \"M\" and use \"*\" everywhere.\n"},{"id":"78684","messageId":"20080605013719.GA1433@kooxoo235","threadId":"13792","inReplyTo":"7v1w3dxeh9.fsf@gitster.siamese.dyndns.org","subject":"Re: log --graph --first-parent weirdness","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-06-05T01:37:19Z","receivedAt":"2008-06-05T01:37:19Z","isPatch":false,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"* Junio C Hamano <gitster@pobox.com> [2008-06-04 11:05:38 -0700]:\n\n> And if the graph actually draws the real ancestry graph (i.e. without\n> --first-parent), the lines visually show which is merge and which is not,\n> so the \"M\" gets very distracting.\n> \n> I'd really suggest changing the \"M\" and use \"*\" everywhere.\n\nI vote for this change.\n"},{"id":"78725","messageId":"1212656179-13637-1-git-send-email-adam@adamsimpkins.net","threadId":"13792","inReplyTo":"20080604180432.GA31437@adamsimpkins.net","subject":"[PATCH] graph API: fix \"git log --graph --first-parent\"","fromName":"Adam Simpkins","fromEmail":"adam@adamsimpkins.net","sentAt":"2008-06-05T08:56:19Z","receivedAt":"2008-06-05T08:56:19Z","isPatch":true,"sender":{"key":"adam@adamsimpkins.net","avatar":"https://gravatar.com/avatar/d3fd2c0b3e2d2136b56e95726ee03227bee4eb562f627dcd3ebca0623fa05054?d=mp&s=160"},"body":"This change teaches the graph API that only the first parent of each\ncommit is interesting when \"--first-parent\" was specified.\n\nThis change also consolidates the graph parent walking logic into two\nnew internal functions, first_interesting_parent() and\nnext_interesting_parent().  A simpler fix would have been to simply\nbreak at the end of the 2 existing for loops when\ngraph->revs->first_parent_only is set.  However, this change seems\nnicer, especially if we ever need to add any new loops over the parent\nlist in the future.\n\nSigned-off-by: Adam Simpkins <adam@adamsimpkins.net>\n---\n graph.c |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++----------\n 1 files changed, 53 insertions(+), 11 deletions(-)\n\ndiff --git a/graph.c b/graph.c\nindex edfab2d..283b137 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -237,6 +237,52 @@ static int graph_is_interesting(struct git_graph *graph, struct commit *commit)\n \treturn (commit->object.flags & (UNINTERESTING | TREESAME)) ? 0 : 1;\n }\n \n+static struct commit_list *next_interesting_parent(struct git_graph *graph,\n+\t\t\t\t\t\t   struct commit_list *orig)\n+{\n+\tstruct commit_list *list;\n+\n+\t/*\n+\t * If revs->first_parent_only is set, only the first\n+\t * parent is interesting.  None of the others are.\n+\t */\n+\tif (graph->revs->first_parent_only)\n+\t\treturn NULL;\n+\n+\t/*\n+\t * Return the next interesting commit after orig\n+\t */\n+\tfor (list = orig->next; list; list = list->next) {\n+\t\tif (graph_is_interesting(graph, list->item))\n+\t\t\treturn list;\n+\t}\n+\n+\treturn NULL;\n+}\n+\n+static struct commit_list *first_interesting_parent(struct git_graph *graph)\n+{\n+\tstruct commit_list *parents = graph->commit->parents;\n+\n+\t/*\n+\t * If this commit has no parents, ignore it\n+\t */\n+\tif (!parents)\n+\t\treturn NULL;\n+\n+\t/*\n+\t * If the first parent is interesting, return it\n+\t */\n+\tif (graph_is_interesting(graph, parents->item))\n+\t\treturn parents;\n+\n+\t/*\n+\t * Otherwise, call next_interesting_parent() to get\n+\t * the next interesting parent\n+\t */\n+\treturn next_interesting_parent(graph, parents);\n+}\n+\n static void graph_insert_into_new_columns(struct git_graph *graph,\n \t\t\t\t\t  struct commit *commit,\n \t\t\t\t\t  int *mapping_index)\n@@ -244,12 +290,6 @@ static void graph_insert_into_new_columns(struct git_graph *graph,\n \tint i;\n \n \t/*\n-\t * Ignore uinteresting commits\n-\t */\n-\tif (!graph_is_interesting(graph, commit))\n-\t\treturn;\n-\n-\t/*\n \t * If the commit is already in the new_columns list, we don't need to\n \t * add it.  Just update the mapping correctly.\n \t */\n@@ -373,9 +413,9 @@ static void graph_update_columns(struct git_graph *graph)\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\tfor (parent = first_interesting_parent(graph);\n \t\t\t     parent;\n-\t\t\t     parent = parent->next) {\n+\t\t\t     parent = next_interesting_parent(graph, parent)) {\n \t\t\t\tgraph_insert_into_new_columns(graph,\n \t\t\t\t\t\t\t      parent->item,\n \t\t\t\t\t\t\t      &mapping_idx);\n@@ -420,9 +460,11 @@ void graph_update(struct git_graph *graph, struct commit *commit)\n \t * Count how many interesting parents this commit has\n \t */\n \tgraph->num_parents = 0;\n-\tfor (parent = commit->parents; parent; parent = parent->next) {\n-\t\tif (graph_is_interesting(graph, parent->item))\n-\t\t\tgraph->num_parents++;\n+\tfor (parent = first_interesting_parent(graph);\n+\t     parent;\n+\t     parent = next_interesting_parent(graph, parent))\n+\t{\n+\t\tgraph->num_parents++;\n \t}\n \n \t/*\n-- \n1.5.6.rc1.13.g14be6\n"},{"id":"78729","messageId":"20080605092812.GA14116@adamsimpkins.net","threadId":"13792","inReplyTo":"7v1w3dxeh9.fsf@gitster.siamese.dyndns.org","subject":"Re: log --graph --first-parent weirdness","fromName":"Adam Simpkins","fromEmail":"adam@adamsimpkins.net","sentAt":"2008-06-05T09:28:13Z","receivedAt":"2008-06-05T09:28:13Z","isPatch":false,"sender":{"key":"adam@adamsimpkins.net","avatar":"https://gravatar.com/avatar/d3fd2c0b3e2d2136b56e95726ee03227bee4eb562f627dcd3ebca0623fa05054?d=mp&s=160"},"body":"On Wed, Jun 04, 2008 at 11:05:38AM -0700, Junio C Hamano wrote:\n> \n> I'd really suggest changing the \"M\" and use \"*\" everywhere.\n\nThat's fine with me.  Here's a simple patch to change the behavior.\n\n\n-- >8 --\n\"git log --graph\": print '*' for all commits, including merges\n\nPreviously, merge commits were printed with 'M' instead of '*'.  This\nhad the potential to confuse users when not all parents of the merge\ncommit were included in the log output.\n\nAs Junio has pointed out, merge commits can almost always be easily\nidentified from the log message, anyway.\n\nSigned-off-by: Adam Simpkins <adam@adamsimpkins.net>\n---\n graph.c |   14 --------------\n 1 files changed, 0 insertions(+), 14 deletions(-)\n\ndiff --git a/graph.c b/graph.c\nindex edfab2d..c50adcd 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -638,20 +638,6 @@ static void graph_output_commit_char(struct git_graph *graph, struct strbuf *sb)\n \t}\n \n \t/*\n-\t * Print 'M' for merge commits\n-\t *\n-\t * Note that we don't check graph->num_parents to determine if the\n-\t * commit is a merge, since that only tracks the number of\n-\t * \"interesting\" parents.  We want to print 'M' for merge commits\n-\t * even if they have less than 2 interesting parents.\n-\t */\n-\tif (graph->commit->parents != NULL &&\n-\t    graph->commit->parents->next != NULL) {\n-\t\tstrbuf_addch(sb, 'M');\n-\t\treturn;\n-\t}\n-\n-\t/*\n \t * Print '*' in all other cases\n \t */\n \tstrbuf_addch(sb, '*');\n-- \n1.5.6.rc1.13.g14be6\n"},{"id":"78734","messageId":"20080605095033.GC5946@mithlond.arda.local","threadId":"13792","inReplyTo":"7v1w3dxeh9.fsf@gitster.siamese.dyndns.org","subject":"Re: log --graph --first-parent weirdness","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2008-06-05T09:50:33Z","receivedAt":"2008-06-05T09:50:33Z","isPatch":false,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"Junio C Hamano wrote (2008-06-04 11:05 -0700):\n\n> Teemu Likonen <tlikonen@iki.fi> writes:\n> \n> > To me it's perfectly fine to turn off --graph when used with\n> > --first-parent, but yes, generally users might expect to see a line\n> > of M's, *'s and |'s there. At least it would clearly show which\n> > commits are merges and which are not.\n> \n> I disagree.  If you are doing --first-parent, you do not _care_ what\n> is merge and what is not.  Besides, you can easily see that from the\n> log message if you cared enough.\n> \n> And if the graph actually draws the real ancestry graph (i.e. without\n> --first-parent), the lines visually show which is merge and which is\n> not, so the \"M\" gets very distracting.\n> \n> I'd really suggest changing the \"M\" and use \"*\" everywhere.\n\nWell, I disagree :-) Merges are interesting points in history (they\nintroduce features etc.) and for a \"--graph --first-parent\" user\na certain already known merge is easier to find if there is a stable\nidentifier for them (like \"M\"). Commit messages are not stable in that\nsense and it helps if user can just keep an eye on the graph when\nsearching for a certain merge (helps to skip other commits). Once the\ncorrect merge is found, one would perhaps do \"git log -p\n<the-SHA1-of-that-merge>^2\".\n\nSo I like having separate identifiers for merge commits. However, I do\nrealize that in the bigger picture those M's are not at all essential\nfor finding wanted information from project's history. So this question\nis not something I'd go arguing too seriously.\n"},{"id":"78791","messageId":"7vbq2f3f9t.fsf@gitster.siamese.dyndns.org","threadId":"13792","inReplyTo":"20080605095033.GC5946@mithlond.arda.local","subject":"Re: log --graph --first-parent weirdness","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-05T18:31:10Z","receivedAt":"2008-06-05T18:31:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Teemu Likonen <tlikonen@iki.fi> writes:\n\n> Well, I disagree :-) Merges are interesting points in history (they\n> introduce features etc.) and for a \"--graph --first-parent\" user\n> a certain already known merge is easier to find if there is a stable\n> identifier for them.\n\nStep back a bit.  Regular commits also introduce features.  If you want to\nargue for marking a merge as more significant than single parent commits,\nyou need to justify the reason why a bit better.\n\nWhen you are looking at a history (be it 'first-parent' or regular), each\ntransition introduces changes, but especially when you are talking about\nfirst-parent, a merge is merely a squashed commit of everything that\nhappened on the side branch, which may be trivial one-liner fix or an\naddition of full new command.  Why a merge of trivial one-liner fix should\nbe treated as more significant than a more involved change that directly\nwas done on the master branch?\n\nA full and perfect implementation of a new command may have happened on a\nside branch as a single commit.  If the master branch was dormant while it\nwas being done, the final merge of that side branch will result in a\nfast-forward, and the introduction of the new command would appear as a\nnon-merge, regular commit.  If on the other hand there were activities on\nmaster since the side branch forked, the introduction of the new command\nwould appear as a merge.  Why do you paint the latter as more significant\nthan the former?\n\nIf somebody argues for making the marking different (perhaps by color-code\nthe asterisk differently) depending on how much each commit changes the\ntree relative to its parents, I would say it might be a great feature.\nSuch a display would treat the two cases I mentioned above equally.\n\nI however do not think the number of recorded parents deserves such a\nspecial treatment to clutter the output and distract people, especially\nwhen \"is it a merge?\" can be easily seen by two other means (log message\nand graph lines).\n"}]}