Display of merges in gitk

I have reworked the way gitk displays merges. Previously it would display all the diffs between the child and each parent. That displayed a lot of unnecessary stuff; for example, for the famous octopus (pentapus?) merge, each diff was shown 4 times. It also shows diffs when the merge is perfectly straightforward, i.e. when any given file has been modified in at most one of the parents, and the child's version is the same as the parent's.

Now gitk will only list a file as having a difference in a merge if one or more of the parents has a version of the file that is different both from the child and from the common ancestor of the parents (if a common ancestor exists). If there is no common ancestor, then a file is listed if it is different in the child from all of the parents.

I now also display the diffs for a file in one unified difference listing. Lines are color-coded according to which parent they come from, and are in bold with a + at the start of the line if they ended up in the merge result (the child), or in normal font with a - at the start of the line if they didn't. In the usual case of two parents, lines from the first parent are in red and lines from the second parent are in blue. Lines in the result that don't correspond to either parent are in bold black.

Linus, could you do a pull from the usual place to pick this up? (rsync://rsync.kernel.org/pub/scm/gitk/gitk.git). I also included a patch from Junio.

Paul.

Paul Mackerras <paulus@samba.org> writes:

In case you did not notice, /pub/scm/git/git.git/ repository is under the care of yours truly starting this week. I've merged from your tree up to this commit:

    commit c8a4acbf4a7b0f9da3e320877b2b2d12cd58700b
    Author: Paul Mackerras <paulus@samba.org>
    Date:   Fri Jul 29 09:23:03 2005 -0500

      Improve the merge display when the result differs from all parents.

      Now we see if the result is quite similar to one of the parents, and
      if it is, display the result as a diff from that parent.  If the result
      is similar to more than one parent, pick the one that it's most
      similar to.

Thanks for the great tool.

It appears that gitk gets wider test coverage only after it is pulled into git.git repository. I think it would be a good idea for me to pull from you often.

Recently there was a discussion with binary packaging folks. While I do not mind, and actually I would prefer, shipping gitk as part of the core GIT, I have never heard about your preference. As long as gitk is just a single file (or even a handful files in the future) project that does not have a filename that overlaps with core GIT, I can continue pulling from you and I think the binary packaging folks can produce separate git-core and gitk package out of git.git tree without problems. However, once you start wanting to have your own Makefile and maybe debian/rules file for packaging, for example, I suspect the way currently things are set up would break miserably. It's all Linus' fault to have merged with your tree in the first place ;-).

Anyhow, I have one bug to report. I selected one rev, and then said "diff this -> selected" from right-click menu on an adjacent one, and I got this:

    wrong # args: should be "startdiff ids"
    wrong # args: should be "startdiff ids"
        while executing
    "startdiff $newid [list $oldid]"
        (procedure "diffvssel" line 28)
        invoked from within
    "diffvssel 0"
        invoked from within
    ".rowctxmenu invoke active"
        ("uplevel" body line 1)
        invoked from within
    "uplevel #0 [list $w invoke active]"
        (procedure "tk::MenuInvoke" line 47)
        invoked from within
    "tk::MenuInvoke .rowctxmenu 1"
        (command bound to event)

The following seems to fix it. ------------ Fix diff invoked from the right-click menu.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
# - master: [PATCH] Making it easier to find which change introduced a bug
# + (working tree)

Junio C Hamano writes:

Yes, I agree. I'm happy to send you an email when I have committed changes to gitk if that will help.

He did ask me first, and I said he could :). It makes things easier for me, having gitk in the core git, because it means that I don't have to worry about making a proper package out of it. I don't see any reason why gitk would grow to be more than just the script.

I am also thinking of doing a "gitool", somewhat like bk citool, to make it easier to create commits. I guess we can decide later whether to make it part of the core git, although it seems more like porcelain than gitk.

Thanks for the patch. I have committed that fix plus fixes for some other bugs that people have reported, and pushed it to master.kernel.org. Could you do another pull please?

Regards, Paul.

Paul Mackerras <paulus@samba.org> writes:

Please do not waste your time on that doing it regularly, but do not hesitate to drop me a note if you have something newsworthy. I always fetch from (not necessarily merge with) a couple of repositories before starting my git day, in order to sanity check the status of my private work repository. Your repository is among them, so I'll usually notice.

Understood.  So let's keep the current arrangement.

Sounds like it.

-jc

On Fri, 29 Jul 2005 20:51:40 -0500 Paul Mackerras wrote:

I have found a reproducible bug in gitk which seems to be in that new code for merges. Run

	gitk f4b3a4c30b5ea3a5de2a2597a3c53266017d02ba

on the git or cogito repository (that commit is from 2005-07-05), then click the topmost commit ("Merge with Linus' current tree"), then click it again - you get an error popup with this trace:

can't read "filelines(f13bbe7f56e49a11f6bfc3b73a463c741f969c9c,0,137)": no such element in array
can't read "filelines(f13bbe7f56e49a11f6bfc3b73a463c741f969c9c,0,137)": no such element in array
    while executing
"$ctext insert end "-$filelines($p,$f,$ol)\n" m$pnum"
    (procedure "processgroup" line 143)
    invoked from within
"processgroup"
    (procedure "processhunks" line 47)
    invoked from within
"processhunks"
    (procedure "getmergediffline" line 81)
    invoked from within
"getmergediffline file10 {f4b3a4c30b5ea3a5de2a2597a3c53266017d02ba f13bbe7f56e49a11f6bfc3b73a463c741f969c9c} f4b3a4c30b5ea3a5de2a2597a3c53266017d02ba"

Another badness which appears with the same repository: run

	gitk f13bbe7f56e49a11f6bfc3b73a463c741f969c9c

then repeated clicks on the topmost commit result in a different diff shown (no error popups, however).

Also if I quickly move between commits, sometimes I get error popups like:

can't unset "treepending": no such variable
    while executing
"unset treepending"
    (procedure "gettreediffline" line 9)
    invoked from within
"gettreediffline file30 {429a9358763dfb98cd063e40dc49cbc049e2a268 154d3d2dd2656c23ea04e9d1c6dd4e576a7af6de}"

This error does not happen again if the same commit is selected.

All this happens both with current cogito (134b78a167b5f77725cd8435849de7298b6de477) and git (9e44f8980b8f41fbeea0a68035865e5778e4dc7d).

On Fri, 29 Jul 2005, Paul Mackerras wrote:

Ok, goodie. It works fine in my environment, with most merges showing up as not interesting. But a merge like

	3e0777b8fa96f7073ed5d13d3bc1d573b766bef9

shows an example of where there was actually both real content merges, and some real clashes.

However, most of the content merges were trivial, and they often hide the real clashes. For example, if you click on that merge, a _lot_ of it looks like it might be clashes, even though most of it was auto-merged. This is not really a problem, but I get the feeling that it could be improved somehow - maybe a button that hides the parts that don't have clashes?

To get the alternate output, maybe something like:
 - run "merge" on the original/parent1/parent2 (the same way the
   git-merge-one-file-script does)
 - anything that merged fine is in black (regardless of which parent it 
   came from), and then mark the merge rejects are in red/blue depending 
   on parent?

I don't know how doable that would be.

		Linus

[ Also Kay Sievers, because the clickability thing sounds like a 
  potentially good thing for webgit too.. ]

For 2.6.13 we've been reverting some stuff lately, to make sure we get a stable release. That's fine, and when I revert something I try to mention the commit ID of the thing I revert in the message. Apparently others do too, as indicated by a patch I just got from Petr Vandovec. So we've got for example:

	889371f61fd5bb914d0331268f12432590cf7e85:
	Author: Linus Torvalds <torvalds@g5.osdl.org>  2005-07-30 13:41:56
	Committer: Linus Torvalds <torvalds@g5.osdl.org>  2005-07-30 13:41:56

    Revert "yenta free_irq on suspend"
    
    ACPI is wrong.  Devices should not release their IRQ's on suspend and
    re-aquire them on resume.  ACPI should just re-init the IRQ controller
    instead of breaking most drivers very subtly.
    
    Breakage reported by Hugh Dickins <hugh@veritas.com>
    
    Undo: d8c4b4195c7d664baf296818bf756775149232d3
    
    Signed-off-by: Linus Torvalds <torvalds@osdl.org>

and

	403fe5ae57c831968c3dbbaba291ae825a1c5aaa:
	Author: Petr Vandrovec <vandrove@vc.cvut.cz>  2005-08-05 06:50:07
	Committer: Linus Torvalds <torvalds@g5.osdl.org>  2005-08-05 06:57:44

    [PATCH] rtc: msleep() cannot be used from interrupt
    
    Since the beginning of July my Opteron box was randomly crashing and
    being rebooted by hardware watchdog.  Today it finally did it in front
    of me, and this patch will hopefully fix it.
    
    The problem is that at the end of June (the 28th, to be exact: commit
    47f176fdaf8924bc83fddcf9658f2fd3ef60d573, "[PATCH] Using msleep()
    instead of HZ") rtc_get_rtc_time ...

and when I use gitk, it would be just too damn cool for words if I could easily follow the SHA1's mentioned in the commit message.

I can just cut-and-paste the SHA1, and I've verified that it works fine. However, as you'v enoticed, I'm of the whiny kind, and I thought it could be easier. So I'm whining again.

<whine>Mommy, mommy, can you make my life easier</whine>

So I noticed that I really would like two things:

 - "clickable" SHA1's in commit messages would be really really cool if 
   something like that is even possible with tcl/tk.

   Now, if you can highlight them when showing the message, that would be 
   extra cool, but even without any highlighing, the thing actually 
   _almost_ works fine already: you can double-click the SHA1, and it will 
   select it. You then have to move the mouse to the "goto" window, and 
   paste in the SHA1 there. And this is where it would be good if this 
   sequence could be simplified a bit.

   Even if it's something as simple as accepting the SHA1 paste into the 
   same window (not having to go to the "goto" window: just double-click 
   on the SHA1, and then right-click to "paste it back").

 - I'd like to have a "back button". Not just for the above kind of thing, 
   but in general too: when searching for something, it would just be very 
   nice if gitk just kept a list of the <n> last commits that have 
   been selected, and there was a web-browser-like button that went 
   back/forward in history.

   But especially when looking at a revert, I just want to first go to the 
   thing we revert, see what's going on there (get the "historical 
   perspective" - commit log for why the original was done etc), and then 
   I'd want to go back (and possibly forth and back again ;). And while 
   the revert mentioned the thing it reverted (so I could cut-and-paste), 
   the thing it reverted obviously does _not_ mention the thing that 
   reverted it, so now I have to manually just scroll back.

   This same thing happens for a failed search (I search for xyz, and it 
   actually finds it, and I realize that that was the wrong search, but
   now I'm two months back..)

<whine>Mommy, mommy, pleeeease</whine>

			Linus

On Fri, Aug 05, 2005 at 07:37:41AM -0700, Linus Torvalds wrote:

[snipped]

The following code worked for me on a toy commit. I'm not sure the regular expression in linkcommits is right if the SHA1 is followed by a colon or a comma, as I noticed it was in your examples. If it doesn, then removing the [[:<:]] and [[:>:]] will probably fix it.

Things that look like SHA1s are highlighted even if they don't actually exist.

There is probably a "more right" place to do the '$ctext tag bind Commit' commands but I didn't find it right away.

On Fri, Aug 05, 2005 at 07:37:41AM -0700, Linus Torvalds wrote:

...

...

...

Damn cool? No problem. :)
  http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=403fe5ae57c831968c3dbbaba291ae825a1c5aaa

Kay

On Sat, 6 Aug 2005, Kay Sievers wrote:

Goodie. Although when I looked at it first, it wasn't obvious - the link is same font, same color as the rest. Maybe make them stand out a _bit_ more?

But yes, works well. Thanks,

		Linus

On Sat, Aug 06, 2005 at 08:44:10AM -0700, Linus Torvalds wrote:

It's blue now, to invite you to click on it. :)

Kay

A git tag object can have its own text contents, but I do not see how I can get to it from gitweb.

For example, I have "junio-gpg-pub" tag in my git.git repository. This is a tag to a "blob" which is my public GPG key. The "tag" object itself says:

    object b92c9c07fe2d0d89c4f692573583c4753b5355d2
    type blob
    tag junio-gpg-pub
    tagger Junio C Hamano <junkio@cox.net> 1123226972 -0700

    This is the GPG key I use to sign GIT releases.
    ...

to tell people that they can use it to verify the tags signed by me. I would appreciate that this description is visible somewhere from gitweb. Clicking on the link just spews out the blob contents, which is the ascii armored public key.

Of course I _could_ add textual description outside the ascii armor in this particular case, but that approach would not work in general --- my next funky tag _could_ point to a JPEG picture with the tag description that says "my beautiful wife" ;-).

I realize that I am going tangent, but it would be very cool if gitweb understood a tag payload that said something like this:

    object b02c0c07fe2d0d80c4f602573583c4753b5355d2
    type blob
    tag my-beautiful-wife
    tagger Junio C Hamano <junkio@cox.net> 1123226072 -0700

    object-content-type: image/jpeg
    Here is a textual description of the blob being pointed at
    by this tag.  For this example, it should say "this is my
    wife's picture" or something like that.

No, I am not going to create that tag.

Junio C Hamano <junkio@cox.net> writes:

I just realized that this is also unavailable in gitk, so please consider this as a feature request to gitk as well.

If you can pop-up a temporary window that shows the tag contents when I hover over a tag icon for 2 seconds, and remove that temporary window when step outside it would be ideal. It is up to you to implement the part to show my wife's picture, reading "object-content-type: image/jpeg" thing ;-). That one was not a serious request.

I have two more requests to gitk, which are related to each other but not related to the "tag contents" one above:

 - if "gitk --all" slurped not just refs/heads and refs/tags but
   everything under refs/* recursively, that would help
   visualizing the bisect status.  bisect creates bunch of
   commit object names in refs/bisect.

 - I have not looked at the code closely enough, but I cannot
   find how to re-read references.  I would appreciate it if it
   allowed it.  This relates to the bisect status visualization,
   where the set of references changes _after_ the user started
   gitk.

Junio C Hamano writes:

I did something a little easier - if you click on the tag, it now displays the contents of the tag in the details pane. Is that good enough?

Well, Tk can display inline images in text widgets... :)

I implemented these two. There is now a "Reread references" button in the File menu. References other than tags and heads get displayed in a light blue box.

Hmmm... now I suppose we want a way to use gitk to drive the git bisection process... :)

Paul.

Hi,

On Sat, 20 Aug 2005, Paul Mackerras wrote:

Ssshh! Let sleeping dogs lie! ;-)

Ciao, Dscho

Paul Mackerras <paulus@samba.org> writes:

Looks very nice, and a lot more sensible than those unsolicited popups I hate very much.

Wonderful.

Yes!  That was the direction I was implying.

On Sat, Aug 06, 2005 at 11:27:54AM -0700, Junio C Hamano wrote:

Check it out. But it still has no "picture tag" support. :)

Thanks, Kay

Linus Torvalds writes:

Done, and it was even pretty easy.  It took only about a dozen lines.

Good idea. Also done. :) It's on master.kernel.org now in my gitk.git directory. Hopefully Junio will pull it into git soon. The current version also squishes the graph horizontally if it gets too wide (i.e. more than half the width of the top-left pane).

Paul.

On Sat, 6 Aug 2005, Paul Mackerras wrote:

Looks good also. I assume the mouse can't change when it hovers? 

Yeah, that looks weird when the lines start turning soft ans squiggly.

		Linus

On Sat, Aug 06, 2005 at 09:20:16AM -0700, Linus Torvalds wrote:

In gitk? This is be possible. My patch[1] does it. The way it's done stinks a little bit, though. <Enter> and <Leave> are bound on a tag common to all the "hyperlinks", and change the -cursor of the whole widget---either to hand2 or the empty string.

+    $ctext tag bind Commit <Enter> { %W configure -cursor hand2 }
+    $ctext tag bind Commit <Leave> { %W configure -cursor {} }

My patch also underlines links, as requested in a different message in this thread.

+    $ctext tag configure Commit -underline yes -foreground blue

Jeff [1] http://www.gelato.unsw.edu.au/archives/git/0508/7253.html

This makes the cursor change when you hover over a SHA1 link with the new "hypertext" gitk commit ID linking feature.

All credit goes to Jeff Epler <jepler@unpythonic.net> and bugs are mine. I don't actually know any tcl/tk, I'm just acting as a random monkey that looks at what others do and mix it up.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
----

Linus Torvalds writes:

I committed something based on this but with extra stuff to make the cursor changes work with the change from the normal cursor to the watch cursor and back.

Paul.

Paul Mackerras <paulus@samba.org> writes:

I did, and will push it out shortly, but I think you need this patch. To make later merges from you easier, I will not put this in my "master" branch.

------------ [PATCH] gitk proposed fix: handle more than one SHA1 links.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

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

c423b62a66d4c4f2a7856cada507f973e9140590

Junio C Hamano writes:

I have committed this plus the hand cursor for the links plus a small change to make gitk display commit messages correctly according to the current locale.

Paul.