ugit: a pyqt-based git gui // was: Re: If you would write git from scratch now, what would you change?

For whatever it's worth, I've written a PyQt4-based git gui. For lack of a better name I call it ugit, as in "I git, you git, we all git with ugit" (or something silly like that).

Though there's still a few things remaining to be implemented, the bulk of the initial groundwork is already done. All you need to build/run it is python and pyqt4 (pyuic4). I've deliberately tried to keep the interface similar to git-gui for now since it is obviously based on it, but that's not a requirement.

Of course there are some notable things missing (such as proper i18n), but it's not too bad for a first draft.

For more details (and the code) see:
http://repo.or.cz/w/ugit.git

Enjoy,
-- David A.

David, Tue, Dec 11, 2007 14:48:32 +0100:

Interesting. I had to start it like this:

	$ export PYTHONPATH=$(pwd)/build/default:$(pwd)/build/default/ui
	$ python ./build/default/bin/ugit.pyc

It has some problem with merges in "Git Commit Browser": takes a lot of CPU and very slowly generates a very big diff.

The diff view is very ... dark. Out of place, when the rest of the interface corresponds to system theme (mine is rather light).

Yeah, it's dark here (Mac OS X), too -- doesn't look very friendly.

It took me some time to get the full Qt, SIP, PyQT toolchain running ... below's my first (tiny) fix that makes "Branch > Create ..." work if you don't select track.

Steffen

index 5abf930..4a35ead 100644
@@ -119,8 +119,8 @@ def git_create_branch (name, base, track=False):
        '''Creates a branch starting from base.  Pass track=True
        to create a remote tracking branch.'''
        cmd = 'git branch'
-       if track: cmd += ' --track '
-       cmd += '%s %s' % ( utils.shell_quote (name),
+       if track: cmd += ' --track'
+       cmd += ' %s %s' % ( utils.shell_quote (name),
                        utils.shell_quote (base))
        return commands.getoutput (cmd)

Cannot start the thing...

$ python bin/ugit.py
Traceback (most recent call last):
  File "bin/ugit.py", line 6, in <module>
    from ugitlibs.models import GitModel
ImportError: No module named ugitlibs.models
$

Some hints?

Thanks
Marco

Where did you install it?

Because I had the same problem when I did
./configure
because it puts the python stuff in /usr/local/python2.5/site-packages/... and python doesn't look there by default.
./configure --prefix=/usr fixed that for me (and I'm sure there's a way to tell python to look in /usr/local... too, but I can't be bothered with that.

I re-installed without the prefix and that error disappeared, but now I get Traceback (most recent call last):
File "/usr/local/bin/ugit", line 12, in <module>
view = GitView (app.activeWindow())
File "../py/views.py", line 15, in __init__
File "default/ui/Window.py", line 43, in setupUi
AttributeError: setLeftMargin

I'm too busy to poke around and see why that's happening, but hopefully someone can.

This with Python 2.5.1 and PyQt4 4.2-8 (from an up-to-date Fedora 8 install)

Jason

./configure --prefix=$HOME/bin

(half) worked thanks.

I have only $HOME/bin in path so I manually moved main 4 files and
directory ugitlibs under bin/

$HOME/bin -> ugit ugit.py uit.pyc ugit.pyo

                   -> ugitlibs/ -> with remaining files

And it worked.

Marco

Hello

To allow python to find the libs you just have to set $PYTHONPATH to include $PREFIX/lib/python2.4/site-packages (change that 2.4 to 2.5 if you're on python2.5). Of course in a packaged form that wouldn't be an issue since $PREFIX=/usr, but for test-driving it you'd probably need to set PYTHONPATH. Are there any distros that don't use $PREFIX/lib/python2.x/site-packages? If not, it wouldn't hurt to add an assumption about the installation layout into the main script.

As for the setLeftMarginError -- That could be because you have py/qt 4.2. The ui files were generated with designer-qt4 (4.3.x) so you might need a more recent pyqt4. I'll see if I can grab an older version of pyqt and use it for all of the ui designs (.ui files are probably forward but not backwards compatible).

That was it - as a matter of fact, the package updater for my distro was asking me to upgrade.

Well done, though I don't think I'm going to abandon git-gui quite yet.

The most valuable thing git-gui does, IMHO, is give you fine control over what you stage in a commit. The two-paned view of staged and unstaged changes with the view of the actual changes makes it really easy to see exactly what you are committing. And the graphical "{Un}stage hunk for commit" business, which ugit seems to lack so far, is really excellent - it's much easier to use than git add -i for partial adds.

I would like to see git add -i's hunk-splitting functionality in a graphical tool for that matter.

Anyway, ugit is very good for a first draft; its text display beats whats in git-gui in a big way (and I would *hope* qt4 would beat Tcl/Tk at that at least).

One suggestion:
For Westerners like me, having "staged" on the left and "unstaged" on the right seems a little unnatural; I'd be curious to hear others' opinions on that.

Jason

Are you just using the wrong fonts under git-gui? I mean both Tk and qt4 are drawing text through your windowing system, from the same pool of font files... if qt4 can draw nice text then so can Tk, right?

Shawn.

I don't know much about graphical toolkits and the like, but I think that the more modern ones have fancy features like antialiasing and subpixel rendering, which makes a big difference when you're working on a laptop with a tiny screen.

Take a look for yourself:
http://img441.imageshack.us/img441/492/comparejd6.png

They are obviously using different fonts there (because I can't figure out what font ugit is using) but there is a difference in rendering quality to be sure.

The qt stuff fits better with the rest of my system better too (even though I'm using gnome) - it's entirely the result of Tk being lightweight and a million years old, when UI conventions were different (like every menu being detachable, and antique scrollbars). I'm not here to start a toolkit flame war (we had a toolkit dogpile on the list last week, I think) I'm just pointing out that Tk is from a different era.

I use git-gui and gitk for my git graphical needs because they rock and at the end of the day, the fonts and antialiasing aren't that big of a deal, especially since I'm usually doing quick scans and searches over the information those tools display, not reading novels in them.

Jason

Oh, that's a good point. On my Mac OS X system with the aqua port of Tk the fonts render just as good as anything else on this box. I guess the Aqua port of Tk is just better than the X11 port of Tk is. :)

Be nice to know what ugit is using, or really how its guessing the default font. I wonder what font you are using with your git-gui. The default Tk picks on X11 is basically crap, but git-gui goes with your system default as its own default.

Yes. The tile extension in 8.5 should actually improve this quite a bit; as I understand it there is a GTK backend for Tk with that set of extensions, making the UI look more modern on X11, assuming GTK was available when Tk was compiled, etc...

I have yet to make git-gui use the tile extension. Its however planned to happen in the near-ish future.

Good points. Features win over pretty most of the time. But at some point pretty is important; especially to new user adoption. Plus if you are looking at it all day long it shouldn't be jarring to the eyes. But git-gui still isn't even where I want it ot be feature-wise. E.g. I'd *love* to teach it inotify support, so you don't even need to have that Rescan button.

Shawn.

On that note, did you see what I wrote above, about having "split hunk" functionality? That would be killer. I know, I know, I should write a patch... well, I've got a paper deadline coming up, and the time it takes me to run git add -i over some clicks in git-gui plus the time to figure out how to add that feature doesn't balance...

Jason

Hi,

I had a patch for splitting hunks in git-gui in August, but there were some issues that I did not yet resolve. If you want to work on it, I'll gladly share that patch with you.

Ciao,
Dscho

Sure, send it along. I'm not going to make any promises, but I could probably find time poke around in there.

Jason

When you select the context menu item "Split Hunk" in the diff area, git-gui will now split the current hunk so that a new hunk starts at the current position.

For this to work, apply has to be called with --unidiff-zero, since the new hunks can start or stop with a "-" or "+" line.

---

On Wed, 12 Dec 2007, Jason Sewall wrote:

> On Dec 12, 2007 1:15 PM, Johannes Schindelin <[email protected]> wrote:
> >
> > I had a patch for splitting hunks in git-gui in August, but
> > there were some issues that I did not yet resolve. If you
> > want to work on it, I'll gladly share that patch with you.
>
> Sure, send it along. I'm not going to make any promises, but I
> could probably find time poke around in there.

This was the next step that I did not yet quite complete:

+proc get_context_lines {line_number context_count direction variable} {
+ global ui_diff
+ upvar result $variable
+

+       set count 0
+       set result ""
+       for {set i $line_number} {$count < $context_count} {incr i $direction} {

+ switch -exact -- [$ui_diff get $i.0] {
+ " " {
+ append result [$ui_diff get $i.0 "$i.lineend"]
+ incr count
+ }
+ "-" {
+ append result [$ui_diff get $i.0 "$i.lineend"]
+ incr count
+ }
+ "@" {
+ return $count
+ }
+ "" {
+ return $count
+ }
+ }
+ }
+}

Of course, this function would be used to add some context to the new hunk boundaries (if needed), and to adjust the contexts of the remaining hunks whenever some hunk was applied.

... and then get rid of that ugly unidiff-zero option.

 git-gui.sh   |    4 +++
 lib/diff.tcl |   75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 78 insertions(+), 1 deletions(-)

index 1fca11f..0798d41 100755
@@ -2567,6 +2567,10 @@ $ctxm add command \
 	-command {apply_hunk $cursorX $cursorY}
 set ui_diff_applyhunk [$ctxm index last]
 lappend diff_actions [list $ctxm entryconf $ui_diff_applyhunk -state]
+$ctxm add command \
+	-label [mc "Split Hunk"] \
+	-command {split_hunk $cursorX $cursorY}
+lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
 $ctxm add separator
 $ctxm add command \
 	-label [mc "Decrease Font Size"] \

1.5.3.7.2250.g3893

NACK! --unidiff-zero eats your data.

1. Prepare a modification that adds 2 lines that are *not* adjacent, like this:

@@ -6,6 +6,8 @@ git-checkout [options] [<branch>] [<paths>...]
--
b= create a new branch started at <branch>
+first
l create the new branch's reflog
track arrange that the new branch tracks the remote branch
+after track
f proceed even if the index or working tree is not HEAD
m merge local modifications into the new branch

2. Reduce context to zero.
3. Stage *second* hunk.

Result: It is staged at the wrong place:

@@ -9,4 +9,5 @@ l create the new branch's reflog
track arrange that the new branch tracks the remote branch
f proceed even if the index or working tree is not HEAD
+after track
m merge local modifications into the new branch
q,quiet be quiet

Reason: --unidiff-zero can only look at the line numbers. And those are wrong because it doesn't account for the shift in line numbers caused by the first hunk.

-- Hannes

Yea, don't worry about that, I won't be applying any patch to git-gui that feeds data to git-apply with --undiff-zero. Not unless its completely bullet-proof that the hunk headers will *never* be wrong.

I'd rather always apply with context and let git-apply do its thing to validate the hunks. If you can get the hunk headers computed right you can also get the context computed right, which means git-apply can actually verify the patch can be applied, thus double checking the splitter.

Shawn.

Hi,

Did I not make crystal clear that I intended this patch to be cleaned up first, to not need unidiff-zero?

If so, my sincerest apologies.

THIS PATCH IS NOT MEANT FOR APPLICATION, BUT FOR JASON TO PLAY WITH.

Ciao,
Dscho

You do not have to do "unidiff zero". Suppose you have this hunk you need to split.

index 7db5588..4d12073 100644
@@ -12,8 +12,8 @@
 /* Index extensions.
  *
  * The first letter should be 'A'..'Z' for extensions that are not
- * necessary for a correct operation (i.e. optimization data).
- * When new extensions are added that _needs_ to be understood in
+ * necessary for a correct operation (that is, optimization data).
+ * When new extensions are added that needs to be understood in
  * order to correctly interpret the index file, pick character that
  * is outside the range, to cause the reader to abort.
  */

Think about taking the s/i.e./that is,/ substitution without taking the other s/_needs_/needs/ substitution. You do not split the hunk between two '-' lines, but effectively make it into this hunk instead:

index 7db5588..4d12073 100644
@@ -12,8 +12,8 @@
 /* Index extensions.
  *
  * The first letter should be 'A'..'Z' for extensions that are not
- * necessary for a correct operation (i.e. optimization data).
+ * necessary for a correct operation (that is, optimization data).
  * When new extensions are added that _needs_ to be understood in
  * order to correctly interpret the index file, pick character that
  * is outside the range, to cause the reader to abort.
  */

That is, , if you want to do finer grained hunk splitting than what "git add -p" lets you do, you do _not_ let user specify "I want to split the hunk into two, before this point and after this point". Instead, let the user pick zero or more '-' line and zero or more '+' line, and adjust the context around it. An unpicked '-' line becomes the common context, and an unpicked '+' line disappears. After that, you recount the diff. That way, you do not have to do any "unidiff zero" cop-out.

At the same time, you can stash away what was _not_ picked, creating two variants to be applied on top of the result of applying (or not applying) the picked patch, if you want to allow "undo".

(variant one: applies after the above is applied)
@@ -12,8 +12,8 @@
/* Index extensions.
*
* The first letter should be 'A'..'Z' for extensions that are not
* necessary for a correct operation (that is, optimization data).
- * When new extensions are added that _needs_ to be understood in
+ * When new extensions are added that needs to be understood in
* order to correctly interpret the index file, pick character that
* is outside the range, to cause the reader to abort. */

(variant two: applies if the above is not applied)
@@ -12,8 +12,8 @@
/* Index extensions.
*
* The first letter should be 'A'..'Z' for extensions that are not
* necessary for a correct operation (i.e. optimization data).
- * When new extensions are added that _needs_ to be understood in
+ * When new extensions are added that needs to be understood in
* order to correctly interpret the index file, pick character that
* is outside the range, to cause the reader to abort. */

...

In this case I would expect two adjacent hunks: one that covers the selected changes, another one with the remaining changes, but each against the original:

@@ -12,7 +12,7 @@
/* Index extensions.
*
* The first letter should be 'A'..'Z' for extensions that are not
- * necessary for a correct operation (i.e. optimization data).
+ * necessary for a correct operation (that is, optimization data).
* When new extensions are added that _needs_ to be understood in
* order to correctly interpret the index file, pick character that
* is outside the range, to cause the reader to abort.
@@ -13,7 +13,7 @@
*
* The first letter should be 'A'..'Z' for extensions that are not
* necessary for a correct operation (i.e. optimization data).
- * When new extensions are added that _needs_ to be understood in
+ * When new extensions are added that needs to be understood in
* order to correctly interpret the index file, pick character that
* is outside the range, to cause the reader to abort. */

Then I can stage either one. After that operation, git-gui refreshes the patch display. This is now the time where the hunk that was not staged should be updated to reflect the correct diff against the staged hunk.

-- Hannes

Hi,

I thought about that, but the UI is not trivial. The UI for my solution is.

Ciao,
Dscho

It's probably sufficient to have an option "Stage this Line": Once you have staged enough lines, the hunk will be split automatically by the current number-of-context-lines setting.

-- Hannes

Hi,

And your hand falls off... ;-)

Seriously, I often have a big chunk of changes, for example with an indentation change, where I want to stage everything _but_ one line in the middle. Just staging that, "git checkout <file>" and fixing the indentation of that single line speeds up my procedure vastly.

Ciao,
Dscho

From: Johannes Sixt <[email protected]>

"Stage/Unstage Hunk" is probably the most frequently used command of the patch context menu *and* it is not available in some other form than the context menu. Therefore, it should go to the top. "Less Context" and "More Context" entries are also not easily available otherwise, and are therefore, moved second. The other entries are available via key strokes (Copy, Paste, Refresh) or rarly used (Font Size, Options) and can go last.

---

Not with this patch.

And, obviously, "Stage this Line" is accompanied by "Unstage this Line". So when you want to stage a lot *except* one line, then you better stage the lot, then *unstage* one line.

-- Hannes

PS: Warning, Shawn: [ab]/git-gui.sh below is forged; you won't have blob 95b9537.

 git-gui.sh |   42 +++++++++++++++++++++---------------------
 1 files changed, 21 insertions(+), 21 deletions(-)

index 95b9537..8e3751f 100755
@@ -2542,6 +2542,27 @@ $ui_diff tag raise sel
 set ctxm .vpane.lower.diff.body.ctxm
 menu $ctxm -tearoff 0
 $ctxm add command \
+	-label [mc "Apply/Reverse Hunk"] \
+	-command {apply_hunk $cursorX $cursorY}
+set ui_diff_applyhunk [$ctxm index last]
+lappend diff_actions [list $ctxm entryconf $ui_diff_applyhunk -state]
+$ctxm add separator
+$ctxm add command \
+	-label [mc "Show Less Context"] \
+	-command {if {$repo_config(gui.diffcontext) >= 1} {
+		incr repo_config(gui.diffcontext) -1
+		reshow_diff
+	}}
+lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
+$ctxm add command \
+	-label [mc "Show More Context"] \
+	-command {if {$repo_config(gui.diffcontext) < 99} {
+		incr repo_config(gui.diffcontext)
+		reshow_diff
+	}}
+lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
+$ctxm add separator
+$ctxm add command \
 	-label [mc Refresh] \
 	-command reshow_diff
 lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]

@@ -2563,12 +2584,6 @@ $ctxm add command \
 lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
 $ctxm add separator
 $ctxm add command \
-	-label [mc "Apply/Reverse Hunk"] \
-	-command {apply_hunk $cursorX $cursorY}
-set ui_diff_applyhunk [$ctxm index last]
-lappend diff_actions [list $ctxm entryconf $ui_diff_applyhunk -state]
-$ctxm add separator
-$ctxm add command \
 	-label [mc "Decrease Font Size"] \
 	-command {incr_font_size font_diff -1}
 lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]

@@ -2577,21 +2592,6 @@ $ctxm add command \
 	-command {incr_font_size font_diff 1}
 lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
 $ctxm add separator
-$ctxm add command \
-	-label [mc "Show Less Context"] \
-	-command {if {$repo_config(gui.diffcontext) >= 1} {
-		incr repo_config(gui.diffcontext) -1
-		reshow_diff
-	}}
-lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
-$ctxm add command \
-	-label [mc "Show More Context"] \
-	-command {if {$repo_config(gui.diffcontext) < 99} {
-		incr repo_config(gui.diffcontext)
-		reshow_diff
-	}}
-lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
-$ctxm add separator
 $ctxm add command -label [mc "Options..."] \
 	-command do_options
 proc popup_diff_menu {ctxm x y X Y} {

1.5.3.7.1929.gf9f0a

Thanks. This will be in my repo.or.cz tree shortly.

Shawn.

I still have conceptual problem with this whole thing. For example, what does that MEAN to split this hunk from your patch...

I don't know if it would be useful, but I think the more important concern here is consistency. ie. it should split hunks the same way "git add -i" does. Both "git gui" and "git add -i" are official parts of Git, so in the interests of coherency they should share the same concept of "what it means to split a hunk".

"git add -i" considers any hunk where a there are multiple groups of deletions and/or insertions separated by context lines to be "splittable" (on the boundaries defined by those intervening context line), and all others to be unsplittable. I think this is a fairly intuitive way to conceptualize splitting, so if it comes down to making "git gui" split like "git add -i", or making "git add -i" split like this patch proposes that "git gui" should do it, then I'd vote for the former.

Cheers,
Wincent

I often get big hunk just because I modified whitespaces around relevent pieces of code, the ability to segment the changes and only pick isolated and specific lines for a commit (not commiting whitespaces surrounding real code changes) would be very welcome. Maybe I should know better, but the actual hunk selection in git gui is quite good already, but the ability to be more precise on how a hunk is defined is a welcome change.

- jfv

Oh, I wasn't questioning the usefulness of hunk splitting in general. It is sometimes useful and that is why we have "add -i".

If you have something like this:

        @@ -j,k +l,m @@
         common 1
         common 2
        -preimage
        +postimage
         common 3
        -deleted
         common 4
         common 5

I think it makes sense to split it into two logical (overlapping) hunks:

        @@ -j,(k-3) +l,(m-2) @@
         common 1
         common 2
        -preimage
        +postimage
         common 3

and

        @@ -j,(k-3) +l,(m-3) @@
         common 3
        -deleted
         common 4
         common 5

and being able to apply one of them independent from the other, or re-combine them back into one hunk.

I was just questioning if it makes sense to split a hunk like this in the middle of -/+ lines:

	@@ -j,k +l,m @@
	 common
	 common
	-pre 1
	-pre 2
        -pre 3
        +post 1
	+post 2
	 common

You could split between "-pre 2" and "-pre 3", but I do not think that would be so useful. It is a different story if you allowed the above to first be transformed into this way (assuming that "pre 1" and "pre 2" corresponds to "post 1"):

	@@ -j,k +l,m @@
	 common
	 common
	-pre 1
	-pre 2
        +post 1
        -pre 3
	+post 2
	 common

and then be split between "+post 1" and "-pre 3". That may make sense in some context.

Hi,

The thing is: sometimes there is a patch which contains just one garbage line. (I am talking about my current working tree, so you are not allowed to be offended by my language in this case.)

The thing I would like to do is right click on that line, start a new hunk, then right click on the next line to start yet another hunk, and apply this and the first hunk.

It is a lazy way to edit a patch.

Ciao,
Dscho

I have added it to http://git.or.cz/gitwiki/InterfacesFrontendsAndTools

Funny that both (h)gct and Qct, which are commit tools too (although multi-SCM) are also written in PyQt...

Jakub Narebski
Poland
ShadeHawk on #git