{"thread":{"id":"3221","subject":"gitk changing line color for no reason after merge","startedAt":"2006-02-02T17:21:37Z","lastAt":"2006-02-08T21:06:23Z","messageCount":8,"participants":["Pavel Roskin","Junio C Hamano","Paul Mackerras"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"15495","messageId":"1138900897.28967.18.camel@dv","threadId":"3221","inReplyTo":null,"subject":"gitk changing line color for no reason after merge","fromName":"Pavel Roskin","fromEmail":"proski@gnu.org","sentAt":"2006-02-02T17:21:37Z","receivedAt":"2006-02-02T17:21:37Z","isPatch":false,"sender":{"key":"proski@gnu.org","avatar":null},"body":"Hello, Paul!\n\nI have noticed that gitk changes the line color at a commit following a\nmerge (i.e. a commit with many parents).  I made this screenshot to\ndemonstrate the problem:\n\nhttp://red-bean.com/proski/gitk/gitk.png\n\nIf you follow the leftmost line, you'll see how green becomes red, gray\nbecomes brown and red becomes blue.\n\nI think it would be much better if line colors only change at\nnon-trivial nodes, i.e. those with more than one child or parent.\n\nThe fix is trivial:\n\ndiff --git a/gitk b/gitk\nindex f12b3ce..14ff570 100755\n--- a/gitk\n+++ b/gitk\n@@ -770,7 +770,7 @@ proc assigncolor {id} {\n \n     if [info exists colormap($id)] return\n     set ncolors [llength $colors]\n-    if {$nparents($id) <= 1 && $nchildren($id) == 1} {\n+    if {$nchildren($id) == 1} {\n \tset child [lindex $children($id) 0]\n \tif {[info exists colormap($child)]\n \t    && $nparents($child) == 1} {\n\n\nI checked the history of gitk and could trace this code back to the\nfirst version of gitk in the git repository.  Maybe it was intended this\nway, but I think it would be better to fix it.\n\n-- \nRegards,\nPavel Roskin\n"},{"id":"15662","messageId":"1139289517.15955.23.camel@dv","threadId":"3221","inReplyTo":"1138900897.28967.18.camel@dv","subject":"Re: gitk changing line color for no reason after merge","fromName":"Pavel Roskin","fromEmail":"proski@gnu.org","sentAt":"2006-02-07T05:18:37Z","receivedAt":"2006-02-07T05:18:37Z","isPatch":false,"sender":{"key":"proski@gnu.org","avatar":null},"body":"Hello!\n\nSorry for replying to myself, but there is nobody else to reply to.\n\n> I think it would be much better if line colors only change at\n> non-trivial nodes, i.e. those with more than one child or parent.\n\nI didn't realize that the colors correspond to nodes, not branches.\nEvery node has one color that is used for lines to all of its children.\nIt would be much better to assign colors to \"branches\" consisting of\nindividual lines connecting nodes, but changing that would require many\nchanges in gitk.\n\n> diff --git a/gitk b/gitk\n> index f12b3ce..14ff570 100755\n> --- a/gitk\n> +++ b/gitk\n> @@ -770,7 +770,7 @@ proc assigncolor {id} {\n>  \n>      if [info exists colormap($id)] return\n>      set ncolors [llength $colors]\n> -    if {$nparents($id) <= 1 && $nchildren($id) == 1} {\n> +    if {$nchildren($id) == 1} {\n>  \tset child [lindex $children($id) 0]\n>  \tif {[info exists colormap($child)]\n>  \t    && $nparents($child) == 1} {\n> \n> \n\nI still stand behind this patch because it eliminates color changes on\nthe nodes that have exactly one child and parent.  $nparents($id) is\nirrelevant here, because it characterizes the current node, but the code\ndecides whether the line should change color at the child node.\n\nHowever, further changes to reduce color changes didn't produce nice\nresults for me.  If I try to keep one color running as long as possible,\nI get branches of the same color because, as I said, gitk uses the same\ncolor for connections to all children.  So, every node on the branch\nspurs branching lines of the same color, which can intersect or run\nside-by-side.\n\nI can submit this patch formally, but I hope to get some comments first.\n\n-- \nRegards,\nPavel Roskin\n"},{"id":"15669","messageId":"17384.24784.898327.669875@cargo.ozlabs.ibm.com","threadId":"3221","inReplyTo":"1139289517.15955.23.camel@dv","subject":"Re: gitk changing line color for no reason after merge","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2006-02-07T08:56:48Z","receivedAt":"2006-02-07T08:56:48Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Pavel Roskin writes:\n> Hello!\n\n> I didn't realize that the colors correspond to nodes, not branches.\n> Every node has one color that is used for lines to all of its children.\n> It would be much better to assign colors to \"branches\" consisting of\n> individual lines connecting nodes, but changing that would require many\n\nWhy would that be better?\n\n> > -    if {$nparents($id) <= 1 && $nchildren($id) == 1} {\n> > +    if {$nchildren($id) == 1} {\n> >  \tset child [lindex $children($id) 0]\n> >  \tif {[info exists colormap($child)]\n> >  \t    && $nparents($child) == 1} {\n> > \n> > \n> \n> I still stand behind this patch because it eliminates color changes on\n> the nodes that have exactly one child and parent.  $nparents($id) is\n\nUmmm... I don't see that you have changed anything for nodes that have\nexactly one parent?\n\n> However, further changes to reduce color changes didn't produce nice\n> results for me.  If I try to keep one color running as long as possible,\n> I get branches of the same color because, as I said, gitk uses the same\n> color for connections to all children.  So, every node on the branch\n> spurs branching lines of the same color, which can intersect or run\n> side-by-side.\n\nThe colors are really just to make the lines visually distinct.  They\n(the colors) have no semantic meaning.  (I did try coloring the lines\naccording to the committer, but I just ended up with an awful lot of\nlines being the same color - corresponding to one Linus Torvalds. :)\n\nPaul.\n"},{"id":"15666","messageId":"7vlkwnxz5t.fsf@assigned-by-dhcp.cox.net","threadId":"3221","inReplyTo":"1139289517.15955.23.camel@dv","subject":"Re: gitk changing line color for no reason after merge","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-07T10:04:46Z","receivedAt":"2006-02-07T10:04:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pavel Roskin <proski@gnu.org> writes:\n\n> I still stand behind this patch because it eliminates color changes on\n> the nodes that have exactly one child and parent.  $nparents($id) is\n> irrelevant here, because it characterizes the current node, but the code\n> decides whether the line should change color at the child node.\n\nIt all depends on what you are trying to achieve with colours.\nBeing a bit colour-challenged, I am not in a good position to\ncomment on how much gitk's use of different colours is helping\nthe readability, but I suspect not very much.  Use of too many\ncolours just makes things distracting.  Especially weaker\ncolours like light yellow are very hard to see on a white\nbackground, at least to me.\n\nTrying to differenciate \"trunk\" with \"side branches\" may be\nsometimes useful in a small one-man project, but it quickly\nbreaks down once you start merging from all over the place,\nsince merges in distributed development do not have inherent\ndistinction between \"trunk\" and \"side branches\".\n\nOne thing it _could_ do is to assign different colours to edges\ncoming into a merge node, and match the colour of the edge that\nleads to a parent to the colour in which the diff text from\nthat parent is shown.  Then the colouring becomes somewhat\nmeaningful.\n\nMaybe gitk is already doing it, but it makes me suspect it\ndoesn't, to read the way the code initialises \"$ctext tag conf\nm$N\" and uses the m$N tag for each diff output line, which is\ndone pretty much independently from the procedure that paint\nedges (the one you touch in your patch).\n"},{"id":"15704","messageId":"1139360047.13646.22.camel@dv","threadId":"3221","inReplyTo":"7vlkwnxz5t.fsf@assigned-by-dhcp.cox.net","subject":"Re: gitk changing line color for no reason after merge","fromName":"Pavel Roskin","fromEmail":"proski@gnu.org","sentAt":"2006-02-08T00:54:07Z","receivedAt":"2006-02-08T00:54:07Z","isPatch":false,"sender":{"key":"proski@gnu.org","avatar":null},"body":"On Tue, 2006-02-07 at 02:04 -0800, Junio C Hamano wrote:\n> Pavel Roskin <proski@gnu.org> writes:\n> \n> > I still stand behind this patch because it eliminates color changes on\n> > the nodes that have exactly one child and parent.  $nparents($id) is\n> > irrelevant here, because it characterizes the current node, but the code\n> > decides whether the line should change color at the child node.\n> \n> It all depends on what you are trying to achieve with colours.\n\nI'm trying to make it easier to follow a line.  It's easier if its color\nis not changing, especially on trivial nodes (one parent, one child).\n\n> Being a bit colour-challenged, I am not in a good position to\n> comment on how much gitk's use of different colours is helping\n> the readability, but I suspect not very much.  Use of too many\n> colours just makes things distracting.  Especially weaker\n> colours like light yellow are very hard to see on a white\n> background, at least to me.\n\nMy point is not to use to many colors.  There is one case when gitk\nchanges line color for no reason, and that case should be fixed.  Since\nthe colors rotate as gitk picks them, extra color switch means that the\ncolors repeat more often.\n\nBy the way, there are some nice colors to add (lightblue darkcyan pink),\nbut let's use effectively what we've got.\n\n> Trying to differenciate \"trunk\" with \"side branches\" may be\n> sometimes useful in a small one-man project, but it quickly\n> breaks down once you start merging from all over the place,\n> since merges in distributed development do not have inherent\n> distinction between \"trunk\" and \"side branches\".\n\nIt's not my point.  My point is to make it easier to follow the lines,\nwithout making any assumptions about \"real\" branches.\n\n> One thing it _could_ do is to assign different colours to edges\n> coming into a merge node, and match the colour of the edge that\n> leads to a parent to the colour in which the diff text from\n> that parent is shown.  Then the colouring becomes somewhat\n> meaningful.\n\nYes, that would be great.\n\n> Maybe gitk is already doing it, but it makes me suspect it\n> doesn't, to read the way the code initialises \"$ctext tag conf\n> m$N\" and uses the m$N tag for each diff output line, which is\n> done pretty much independently from the procedure that paint\n> edges (the one you touch in your patch).\n\nCorrect.\n\nTo illustrate my point, I put some examples online:\n\nhttp://red-bean.com/proski/gitk/gitk-before.png - what gitk is showing\nnow.  There are color changes on trivial nodes both below and above\nmerges.\n\nhttp://red-bean.com/proski/gitk/gitk-after.png - with my patch.  Color\nchanges on trivial nodes only happen below merges.\n\nhttp://red-bean.com/proski/gitk/gitk-ideal.png - made in GIMP.  Trivial\nnodes never change line color, because it changes as soon as the line\nforks.\n\n-- \nRegards,\nPavel Roskin\n"},{"id":"15706","messageId":"1139361014.13646.37.camel@dv","threadId":"3221","inReplyTo":"17384.24784.898327.669875@cargo.ozlabs.ibm.com","subject":"Re: gitk changing line color for no reason after merge","fromName":"Pavel Roskin","fromEmail":"proski@gnu.org","sentAt":"2006-02-08T01:10:14Z","receivedAt":"2006-02-08T01:10:14Z","isPatch":false,"sender":{"key":"proski@gnu.org","avatar":null},"body":"On Tue, 2006-02-07 at 19:56 +1100, Paul Mackerras wrote:\n> Pavel Roskin writes:\n> > Hello!\n> \n> > I didn't realize that the colors correspond to nodes, not branches.\n> > Every node has one color that is used for lines to all of its children.\n> > It would be much better to assign colors to \"branches\" consisting of\n> > individual lines connecting nodes, but changing that would require many\n> \n> Why would that be better?\n\nBecause you would get consistent colors between line forks.  The color\nwould help to tack the line.  Anyway, I don't think I'll ever implement\nit myself.\n\n> > > -    if {$nparents($id) <= 1 && $nchildren($id) == 1} {\n> > > +    if {$nchildren($id) == 1} {\n> > >  \tset child [lindex $children($id) 0]\n> > >  \tif {[info exists colormap($child)]\n> > >  \t    && $nparents($child) == 1} {\n> > > \n> > > \n> > \n> > I still stand behind this patch because it eliminates color changes on\n> > the nodes that have exactly one child and parent.  $nparents($id) is\n> \n> Ummm... I don't see that you have changed anything for nodes that have\n> exactly one parent?\n\nNodes with one parent are fine.  It's nodes with multiple parents that\nare the problem - they fail to inherit the color assigned to their\nchild.\n\nLet's see what assigncolor does:\n\n|  ----------- color assigned to the child, possibly to be inherited\n|\nchild\n|\n|  ----------- we are assigning color to this line\n|\nid ----------- we are processing this node\n|\\\n| \\--------\\\n|           |\nparent1   parent2\n\nWhy should color at the child change because its single parent has\nseveral parents?\n\n> > However, further changes to reduce color changes didn't produce nice\n> > results for me.  If I try to keep one color running as long as possible,\n> > I get branches of the same color because, as I said, gitk uses the same\n> > color for connections to all children.  So, every node on the branch\n> > spurs branching lines of the same color, which can intersect or run\n> > side-by-side.\n> \n> The colors are really just to make the lines visually distinct.  They\n> (the colors) have no semantic meaning.  (I did try coloring the lines\n> according to the committer, but I just ended up with an awful lot of\n> lines being the same color - corresponding to one Linus Torvalds. :)\n\nI guess I wasn't clear about my intentions.  My only intention is\nexactly that - \"to make the lines visually distinct\".  There is one\ncorner case where the color changes for no reason.  There are cases that\nare harder to fix, but this one is easy, and I have found a working\nsolution.\n\n-- \nRegards,\nPavel Roskin\n"},{"id":"15707","messageId":"17385.22468.218755.833713@cargo.ozlabs.ibm.com","threadId":"3221","inReplyTo":"1139360047.13646.22.camel@dv","subject":"Re: gitk changing line color for no reason after merge","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2006-02-08T02:30:28Z","receivedAt":"2006-02-08T02:30:28Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Pavel Roskin writes:\n\n> I'm trying to make it easier to follow a line.  It's easier if its color\n> is not changing, especially on trivial nodes (one parent, one child).\n\nOK, you're using \"line\" to mean something a bit different from the\nconnection between a commit and its children, which is how I use it.\nYou seem to be using it more as a \"line of development\", or as a\nseries of related patches.  Which is fine, if you can find a way to\nidentify lines of development automatically.  (I know it looks obvious\nwhen you look at the gitk display, but that's a lot different from\nwriting down an algorithm to do it.)\n\n> http://red-bean.com/proski/gitk/gitk-ideal.png - made in GIMP.  Trivial\n> nodes never change line color, because it changes as soon as the line\n> forks.\n\nMy problem with that is that it isn't clear that e.g. the green and\nbrown lines near the bottom actually represent the same parent - and\nthat will get worse with more complex graphs.\n\nPaul.\n"},{"id":"15728","messageId":"1139432783.16999.37.camel@dv","threadId":"3221","inReplyTo":"17385.22468.218755.833713@cargo.ozlabs.ibm.com","subject":"Re: gitk changing line color for no reason after merge","fromName":"Pavel Roskin","fromEmail":"proski@gnu.org","sentAt":"2006-02-08T21:06:23Z","receivedAt":"2006-02-08T21:06:23Z","isPatch":false,"sender":{"key":"proski@gnu.org","avatar":null},"body":"On Wed, 2006-02-08 at 13:30 +1100, Paul Mackerras wrote:\n> Pavel Roskin writes:\n> \n> > I'm trying to make it easier to follow a line.  It's easier if its color\n> > is not changing, especially on trivial nodes (one parent, one child).\n> \n> OK, you're using \"line\" to mean something a bit different from the\n> connection between a commit and its children, which is how I use it.\n\nI see.  Actually, your choice seems to me quite random and\nnon-intuitive.  You group together changes that have the same parent,\nlikely made independently by different people.  In fact, only those\nchanges are shown that would lead to the current revision of the\nrepository, unless \"--all\" is used.  Changes on unmerged branches are\nnot shown.\n\nIf you prefer \"horizontal\" grouping, it would be more logical to turn it\nupside down, i.e. group commits with their parents.  In this case, the\nline group would represent one act of merging, performed by one person.\nNo parents are hidden from view even without \"--all\".\n\n> You seem to be using it more as a \"line of development\", or as a\n> series of related patches.  Which is fine, if you can find a way to\n> identify lines of development automatically.  (I know it looks obvious\n> when you look at the gitk display, but that's a lot different from\n> writing down an algorithm to do it.)\n\nAs usually, let's go from the newest commits to the root of the tree.\nThe idea is to assign branch ID to changesets, i.e. to combinations of\nsha1 and parent number.  Branch ID should be inherited from the children\nby the first parent.  Other parents get new branch ID.  There should be\na list of active branches, i.e. those branch ID with yet to be seen\nparents.  Color should be assigned to branch ID at the creation time.\nThe color should be selected according to two rules, whenever possible.\nIt should be unique among the already assigned colors for the same\nchild, and is should avoid colors of the active branches.\n\nActually, qgit does a pretty reasonable thing.  I haven't used gitk for\nmonths, but I had to inspect a Mercurial repository using hgk.  I was\nsurprised by its \"crazy\" color changes (or so it seemed to me after\nqgit), then I found that gitk had the same problem, then I fixed it and\nstarted this thread :-)\n\n> > http://red-bean.com/proski/gitk/gitk-ideal.png - made in GIMP.  Trivial\n> > nodes never change line color, because it changes as soon as the line\n> > forks.\n> \n> My problem with that is that it isn't clear that e.g. the green and\n> brown lines near the bottom actually represent the same parent - and\n> that will get worse with more complex graphs.\n\nYou are right.  qgit only uses vertical and horizontal lines, so it's\neasier to find the parent.\n\n-- \nRegards,\nPavel Roskin\n"}]}