{"thread":{"id":"20328","subject":"[GITK PATCH] gitk: fix direction of --left-right triangles","startedAt":"2009-08-01T12:47:47Z","lastAt":"2009-08-08T16:34:26Z","messageCount":4,"participants":["Thomas Rast"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"119329","messageId":"0fd5fc0f09779bb04c02b54d6ec8f43087a51bca.1249130587.git.trast@student.ethz.ch","threadId":"20328","inReplyTo":null,"subject":"[GITK PATCH] gitk: fix direction of --left-right triangles","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-08-01T12:47:47Z","receivedAt":"2009-08-01T12:47:47Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"c961b22 (gitk: Use git log and add support for --left-right,\n2007-07-09) introduced --left-right support to gitk, but right from\nthe start, 'gitk --left-right A...B' oriented the triangles the wrong\nway: commits coming from A had a triangle to the right, and vice\nversa.  To fix this, we simply swap the triangles.  (Note that git-log\ndoes it right.)\n\nNoticed-by: Björn Steinbrink <B.Steinbrink@gmx.de>\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n\nSorry for the double mail, I forgot to Cc the list in the first mail.\n\n gitk |   12 ++++++------\n 1 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 4604c83..5be2a76 100755\n--- a/gitk\n+++ b/gitk\n@@ -5689,16 +5689,16 @@ proc drawcmittext {id row col} {\n     } elseif {$listed == 3} {\n \t# triangle pointing left for left-side commits\n \tset t [$canv create polygon \\\n-\t\t   [expr {$x - $orad}] $y \\\n-\t\t   [expr {$x + $orad - 1}] [expr {$y - $orad}] \\\n-\t\t   [expr {$x + $orad - 1}] [expr {$y + $orad - 1}] \\\n+\t\t   [expr {$x + $orad - 1}] $y \\\n+\t\t   [expr {$x - $orad}] [expr {$y - $orad}] \\\n+\t\t   [expr {$x - $orad}] [expr {$y + $orad - 1}] \\\n \t\t   -fill $ofill -outline $fgcolor -width 1 -tags circle]\n     } else {\n \t# triangle pointing right for right-side commits\n \tset t [$canv create polygon \\\n-\t\t   [expr {$x + $orad - 1}] $y \\\n-\t\t   [expr {$x - $orad}] [expr {$y - $orad}] \\\n-\t\t   [expr {$x - $orad}] [expr {$y + $orad - 1}] \\\n+\t\t   [expr {$x - $orad}] $y \\\n+\t\t   [expr {$x + $orad - 1}] [expr {$y - $orad}] \\\n+\t\t   [expr {$x + $orad - 1}] [expr {$y + $orad - 1}] \\\n \t\t   -fill $ofill -outline $fgcolor -width 1 -tags circle]\n     }\n     set circleitem($row) $t\n-- \n1.6.4.214.gb5b94\n"},{"id":"119695","messageId":"8a2113bfa2f1eaf6a13587cadfbaae81c8914947.1249506383.git.trast@student.ethz.ch","threadId":"20328","inReplyTo":"0fd5fc0f09779bb04c02b54d6ec8f43087a51bca.1249130587.git.trast@student.ethz.ch","subject":"[PATCH v2] gitk: fix direction of symmetric difference in optimized mode","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-08-05T21:15:36Z","receivedAt":"2009-08-05T21:15:36Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"ee66e08 (gitk: Make updates go faster, 2008-05-09) implemented an\noptimized mode where gitk parses the arguments with rev-parse, and\nmanually reads history in chunks.  As mentioned in the commit message,\nsymmetric differences are a problem there:\n\n    One wrinkle is that we have to turn symmetric diff arguments (of the\n    form a...b) back into symmetric diff form so that --left-right still\n    works, as git rev parse turns a...b into a b ^merge_base(a,b).\n\nHowever, git-rev-parse returns a...b in the swapped order\n\n    b a ^merge_base(a,b)\n\nThis has been the case since at least 1f8115b (the state of master at\nthe time of the abovementioned ee66e08; Merge branch 'maint',\n2008-05-08).  So gitk flipped the sides of symmetric differences\nwhenever it was in optimized mode.\n\nFix this by swapping the sides of the reconstruction code.\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n\nThis supersedes the older patch, which was simply wrong; the triangle\ndirections in the affected section are correct.\n\nThe confusing part of this, but also how I stumbled across the real\nbug, was that I was playing with --show-all and that flipped the\ndirection *again*.  Turns out the option is not recognized by gitk and\nlets it go back to unoptimized mode, where the bug does not exist.\n\n\n gitk |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 4604c83..e103dab 100755\n--- a/gitk\n+++ b/gitk\n@@ -288,7 +288,7 @@ proc parseviewrevs {view revs} {\n \t    if {$sdm != 2} {\n \t\tlappend ret $id\n \t    } else {\n-\t\tlset ret end [lindex $ret end]...$id\n+\t\tlset ret end $id...[lindex $ret end]\n \t    }\n \t    lappend pos $id\n \t}\n-- \n1.6.4.96.g577b\n"},{"id":"119756","messageId":"200908060919.27780.trast@student.ethz.ch","threadId":"20328","inReplyTo":"19066.8802.98042.957009@cargo.ozlabs.ibm.com","subject":"Re: [PATCH v2] gitk: fix direction of symmetric difference in optimized mode","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-08-06T07:19:26Z","receivedAt":"2009-08-06T07:19:26Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Paul Mackerras wrote:\n> Thomas Rast writes:\n> \n> > The confusing part of this, but also how I stumbled across the real\n> > bug, was that I was playing with --show-all and that flipped the\n> > direction *again*.  Turns out the option is not recognized by gitk and\n> > lets it go back to unoptimized mode, where the bug does not exist.\n> \n> What does --show-all do?  Maybe I need to add support for it to gitk.\n\nSupport for _displaying_ it was added to gitk in 1407ade, even before\nthe option was added to the revision walker.  Since it's a debugging\noption, I doubt it's worth handling this in the optimized code path.\n\nBut anyway, it does not do what I hoped :-(\n\nIt shows commits that were walked, but found uninteresting, with a ^\nin front.  See the long explanation in 3131b71; you can try\n\n  gitk --show-all origin/next..origin/pu\n\nfor a nice example in git.git.\n\nI was _actually_ looking for an option to make --cherry-pick\n--left-right history connected again, as I was trying to make sense of\nan SVN history basically by saying\n\n  gitk --left-right --cherry-pick svn/2.2...svn/trunk\n\n(Incidentally this SVN is publicly available at\nhttps://secure.a-eskwadraat.nl/svn/domjudge, but I doubt it's worth\nthe cloning.)\n\nThe problem with this is that it disconnects history, so I was looking\nfor an option to either get back the commits omitted by --cherry-pick\n(but of course flagged in some way that shows they're duplicated) or\nfix the parent pointers so that history becomes connected again.  Some\nof my guesses were --sparse, --full-history and --show-all, but none\nachieve this.\n\n[I *think* --sparse comes closest, but it's about TREESAME-type\nuninteresting commits, not about --cherry-pick.  --full-history is\nonly about the merges that are TREESAME, so that's out.  --show-all\napparently is something entirely different.]\n\nSadly, it's really the underlying git-rev-list that is \"broken\" in the\nsense that it does not fix the parent lists.  And git log --graph\nhandles it much worse than gitk.  I've added the authors of the\nrelevant features to Cc; maybe you can help?\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"119996","messageId":"50f4386635a3ab08ee7fd432d672f775cc760f39.1249749019.git.trast@student.ethz.ch","threadId":"20328","inReplyTo":"200908060919.27780.trast@student.ethz.ch","subject":"[RFC PATCH] Simplify away duplicate commits with --cherry-pick --parents","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-08-08T16:34:26Z","receivedAt":"2009-08-08T16:34:26Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The current --cherry-pick declares commits SHOWN that are found to be\nduplicates.  Unfortunately this disconnects the history at every such\nduplicate, making it quite hard to follow in graphical viewers.\n\nAdd an extra stage of parent rewriting after scanning for duplicates,\nwhich simplifies the history to omit all duplicate commits.  This\ncannot easily be shifted to the existing parent rewriting because\ncherry_pick_list() always comes last in the entire filtering process\n(presumably because it is the most expensive).\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n\nI wrote:\n> The problem with [gitk --left-right --cherry-pick A...B] is that it\n> disconnects history\n[...]\n> Sadly, it's really the underlying git-rev-list that is \"broken\" in the\n> sense that it does not fix the parent lists.  And git log --graph\n> handles it much worse than gitk.\n\nMaybe this is an approach.  It unfortunately breaks down if merges can\ndisappear because of patch-ids too.  Can they?\n\n(In the case where a merge is flagged SHOWN, it might have its parent\nlist reduced to one at some point, and then later filterings would\nsimplify it away whereas earlier ones didn't.)\n\nAlso, I'm not entirely sure we want to do this without any guards\nexcept rewrite_parents.\n\nOn the plus side, the issues with git log --graph vanish because\nhistory is again connected :-)\n\n\n revision.c |   21 +++++++++++++++++++++\n 1 files changed, 21 insertions(+), 0 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 9f5dac5..9e24514 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -517,6 +517,8 @@ static int add_parents_to_list(struct rev_info *revs, struct commit *commit,\n \treturn 0;\n }\n \n+static int remove_duplicate_parents(struct commit *commit);\n+\n static void cherry_pick_list(struct commit_list *list, struct rev_info *revs)\n {\n \tstruct commit_list *p;\n@@ -599,6 +601,25 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs)\n \t\tcommit->util = NULL;\n \t}\n \n+\tif (revs->rewrite_parents) {\n+\t\t/* Prune away commits we've just found to be duplicates */\n+\t\tfor (p = list; p; p = p->next) {\n+\t\t\tstruct commit *commit = p->item;\n+\t\t\tstruct commit_list *pp;\n+\n+\t\t\tfor (pp = commit->parents; pp; pp = pp->next) {\n+\t\t\t\tstruct commit *parent = pp->item;\n+\t\t\t\twhile (parent->object.flags & SHOWN\n+\t\t\t\t       && parent->parents\n+\t\t\t\t       && !parent->parents->next)\n+\t\t\t\t\tparent = parent->parents->item;\n+\t\t\t\tpp->item = parent;\n+\t\t\t}\n+\n+\t\t\tremove_duplicate_parents(commit);\n+\t\t}\n+\t}\n+\n \tfree_patch_ids(&ids);\n }\n \n-- \n1.6.4.199.g24c3\n"}]}