[GITK PATCH] gitk: fix direction of --left-right triangles

[GITK PATCH] gitk: fix direction of --left-right triangles

[Thomas Rast]

c961b22 (gitk: Use git log and add support for --left-right,
2007-07-09) introduced --left-right support to gitk, but right from
the start, 'gitk --left-right A...B' oriented the triangles the wrong
way: commits coming from A had a triangle to the right, and vice
versa.  To fix this, we simply swap the triangles.  (Note that git-log
does it right.)

Noticed-by: Björn Steinbrink <B.Steinbrink@gmx.de>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Sorry for the double mail, I forgot to Cc the list in the first mail.

 gitk |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/gitk b/gitk
index 4604c83..5be2a76 100755
--- a/gitk
+++ b/gitk
@@ -5689,16 +5689,16 @@ proc drawcmittext {id row col} {
     } elseif {$listed == 3} {
 	# triangle pointing left for left-side commits
 	set t [$canv create polygon \
-		   [expr {$x - $orad}] $y \
-		   [expr {$x + $orad - 1}] [expr {$y - $orad}] \
-		   [expr {$x + $orad - 1}] [expr {$y + $orad - 1}] \
+		   [expr {$x + $orad - 1}] $y \
+		   [expr {$x - $orad}] [expr {$y - $orad}] \
+		   [expr {$x - $orad}] [expr {$y + $orad - 1}] \
 		   -fill $ofill -outline $fgcolor -width 1 -tags circle]
     } else {
 	# triangle pointing right for right-side commits
 	set t [$canv create polygon \
-		   [expr {$x + $orad - 1}] $y \
-		   [expr {$x - $orad}] [expr {$y - $orad}] \
-		   [expr {$x - $orad}] [expr {$y + $orad - 1}] \
+		   [expr {$x - $orad}] $y \
+		   [expr {$x + $orad - 1}] [expr {$y - $orad}] \
+		   [expr {$x + $orad - 1}] [expr {$y + $orad - 1}] \
 		   -fill $ofill -outline $fgcolor -width 1 -tags circle]
     }
     set circleitem($row) $t
-- 
1.6.4.214.gb5b94

[PATCH v2] gitk: fix direction of symmetric difference in optimized mode

[Thomas Rast]

ee66e08 (gitk: Make updates go faster, 2008-05-09) implemented an
optimized mode where gitk parses the arguments with rev-parse, and
manually reads history in chunks.  As mentioned in the commit message,
symmetric differences are a problem there:

    One wrinkle is that we have to turn symmetric diff arguments (of the
    form a...b) back into symmetric diff form so that --left-right still
    works, as git rev parse turns a...b into a b ^merge_base(a,b).

However, git-rev-parse returns a...b in the swapped order

    b a ^merge_base(a,b)

This has been the case since at least 1f8115b (the state of master at
the time of the abovementioned ee66e08; Merge branch 'maint',
2008-05-08).  So gitk flipped the sides of symmetric differences
whenever it was in optimized mode.

Fix this by swapping the sides of the reconstruction code.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

This supersedes the older patch, which was simply wrong; the triangle
directions in the affected section are correct.

The confusing part of this, but also how I stumbled across the real
bug, was that I was playing with --show-all and that flipped the
direction *again*.  Turns out the option is not recognized by gitk and
lets it go back to unoptimized mode, where the bug does not exist.


 gitk |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitk b/gitk
index 4604c83..e103dab 100755
--- a/gitk
+++ b/gitk
@@ -288,7 +288,7 @@ proc parseviewrevs {view revs} {
 	    if {$sdm != 2} {
 		lappend ret $id
 	    } else {
-		lset ret end [lindex $ret end]...$id
+		lset ret end $id...[lindex $ret end]
 	    }
 	    lappend pos $id
 	}
-- 
1.6.4.96.g577b

Re: [PATCH v2] gitk: fix direction of symmetric difference in optimized mode

[Thomas Rast]

Paul Mackerras wrote:
> Thomas Rast writes:
> 
> > The confusing part of this, but also how I stumbled across the real
> > bug, was that I was playing with --show-all and that flipped the
> > direction *again*.  Turns out the option is not recognized by gitk and
> > lets it go back to unoptimized mode, where the bug does not exist.
> 
> What does --show-all do?  Maybe I need to add support for it to gitk.

Support for _displaying_ it was added to gitk in 1407ade, even before
the option was added to the revision walker.  Since it's a debugging
option, I doubt it's worth handling this in the optimized code path.

But anyway, it does not do what I hoped :-(

It shows commits that were walked, but found uninteresting, with a ^
in front.  See the long explanation in 3131b71; you can try

  gitk --show-all origin/next..origin/pu

for a nice example in git.git.

I was _actually_ looking for an option to make --cherry-pick
--left-right history connected again, as I was trying to make sense of
an SVN history basically by saying

  gitk --left-right --cherry-pick svn/2.2...svn/trunk

(Incidentally this SVN is publicly available at
https://secure.a-eskwadraat.nl/svn/domjudge, but I doubt it's worth
the cloning.)

The problem with this is that it disconnects history, so I was looking
for an option to either get back the commits omitted by --cherry-pick
(but of course flagged in some way that shows they're duplicated) or
fix the parent pointers so that history becomes connected again.  Some
of my guesses were --sparse, --full-history and --show-all, but none
achieve this.

[I *think* --sparse comes closest, but it's about TREESAME-type
uninteresting commits, not about --cherry-pick.  --full-history is
only about the merges that are TREESAME, so that's out.  --show-all
apparently is something entirely different.]

Sadly, it's really the underlying git-rev-list that is "broken" in the
sense that it does not fix the parent lists.  And git log --graph
handles it much worse than gitk.  I've added the authors of the
relevant features to Cc; maybe you can help?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

[RFC PATCH] Simplify away duplicate commits with --cherry-pick --parents

[Thomas Rast]

The current --cherry-pick declares commits SHOWN that are found to be
duplicates.  Unfortunately this disconnects the history at every such
duplicate, making it quite hard to follow in graphical viewers.

Add an extra stage of parent rewriting after scanning for duplicates,
which simplifies the history to omit all duplicate commits.  This
cannot easily be shifted to the existing parent rewriting because
cherry_pick_list() always comes last in the entire filtering process
(presumably because it is the most expensive).

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

I wrote:
> The problem with [gitk --left-right --cherry-pick A...B] is that it
> disconnects history
[...]
> Sadly, it's really the underlying git-rev-list that is "broken" in the
> sense that it does not fix the parent lists.  And git log --graph
> handles it much worse than gitk.

Maybe this is an approach.  It unfortunately breaks down if merges can
disappear because of patch-ids too.  Can they?

(In the case where a merge is flagged SHOWN, it might have its parent
list reduced to one at some point, and then later filterings would
simplify it away whereas earlier ones didn't.)

Also, I'm not entirely sure we want to do this without any guards
except rewrite_parents.

On the plus side, the issues with git log --graph vanish because
history is again connected :-)


 revision.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/revision.c b/revision.c
index 9f5dac5..9e24514 100644
--- a/revision.c
+++ b/revision.c
@@ -517,6 +517,8 @@ static int add_parents_to_list(struct rev_info *revs, struct commit *commit,
 	return 0;
 }
 
+static int remove_duplicate_parents(struct commit *commit);
+
 static void cherry_pick_list(struct commit_list *list, struct rev_info *revs)
 {
 	struct commit_list *p;
@@ -599,6 +601,25 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs)
 		commit->util = NULL;
 	}
 
+	if (revs->rewrite_parents) {
+		/* Prune away commits we've just found to be duplicates */
+		for (p = list; p; p = p->next) {
+			struct commit *commit = p->item;
+			struct commit_list *pp;
+
+			for (pp = commit->parents; pp; pp = pp->next) {
+				struct commit *parent = pp->item;
+				while (parent->object.flags & SHOWN
+				       && parent->parents
+				       && !parent->parents->next)
+					parent = parent->parents->item;
+				pp->item = parent;
+			}
+
+			remove_duplicate_parents(commit);
+		}
+	}
+
 	free_patch_ids(&ids);
 }
 
-- 
1.6.4.199.g24c3