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

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

[David]

On Dec 4, 2007 3:00 AM, Andy Parkins <andyparkins@gmail.com> wrote:
>
> Qt puts a common face on threading, process control, networking, file
> systems, internationalisation, rendering, openGL, and of course the GUI
> itself.  Tcl/Tk (to take the most wicked example) gives you applications
> that are much harder to make run on Windows than on UNIX.
>
> Anyway, I don't want to sound like a strange Qt fan boy; the above is simply
> my justification for putting "git-gui in Qt" on my wish list.
>
> Andy
> --
> Dr Andy Parkins, M Eng (hons), MIET
> andyparkins@gmail.com

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.

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

[Marco Costalba]

On Dec 11, 2007 2:48 PM, David <davvid@gmail.com> wrote:
>
> Of course there are some notable things missing (such as proper i18n),
> but it's not too bad for a first draft.
>

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

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

[Jason Sewall]

On Dec 11, 2007 1:20 PM, Marco Costalba <mcostalba@gmail.com> wrote:
> 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?

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

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

[Marco Costalba]

On Dec 11, 2007 8:14 PM, Jason Sewall <jasonsewall@gmail.com> wrote:
> On Dec 11, 2007 1:20 PM, Marco Costalba <mcostalba@gmail.com> wrote:

> ./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.
>

./configure --prefix=$HOME/bin

(half) worked thanks.



> 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.
>

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

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

[David]

On Dec 11, 2007 11:33 AM, Marco Costalba <mcostalba@gmail.com> wrote:
> On Dec 11, 2007 8:14 PM, Jason Sewall <jasonsewall@gmail.com> wrote:
> > On Dec 11, 2007 1:20 PM, Marco Costalba <mcostalba@gmail.com> wrote:
>
> > ./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.
> >
>
> ./configure --prefix=$HOME/bin
>
> (half) worked thanks.
>

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.


> > 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

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).

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

[Jason Sewall]

On Dec 11, 2007 3:54 PM, David <davvid@gmail.com> wrote:
> > On Dec 11, 2007 8:14 PM, Jason Sewall <jasonsewall@gmail.com> wrote:
> >
> > 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
>
> 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

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

[Alex Riesen]

David, Tue, Dec 11, 2007 14:48:32 +0100:
> 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.

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).

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

[Steffen Prohaska]

On Dec 11, 2007, at 11:37 PM, Alex Riesen wrote:

> David, Tue, Dec 11, 2007 14:48:32 +0100:
>> 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.
>
> 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

diff --git a/py/cmds.py b/py/cmds.py
index 5abf930..4a35ead 100644
--- a/py/cmds.py
+++ b/py/cmds.py
@@ -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)

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

[Jakub Narebski]

David <davvid@gmail.com> writes:

> 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).

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

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

[Shawn O. Pearce]

Jason Sewall <jasonsewall@gmail.com> wrote:
> 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).

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.

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

[Jason Sewall]

On Dec 11, 2007 11:10 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Jason Sewall <jasonsewall@gmail.com> wrote:
> > 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).
>
> 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?

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

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

[Shawn O. Pearce]

Jason Sewall <jasonsewall@gmail.com> wrote:
> 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.

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.  :)
 
> 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.

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.
 
> 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.

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.
 
> 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.

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.

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

[Jason Sewall]

On Dec 12, 2007 12:23 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> > 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.
>
> 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.

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

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

[Johannes Schindelin]

Hi,

On Wed, 12 Dec 2007, Jason Sewall wrote:

> On Dec 12, 2007 12:23 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> > > 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.
> >
> > 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.
> 
> On that note, did you see what I wrote above, about having "split hunk" 
> functionality?

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

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

[Jason Sewall]

On Dec 12, 2007 1:15 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Wed, 12 Dec 2007, Jason Sewall wrote:
> > On that note, did you see what I wrote above, about having "split hunk"
> > functionality?
>
> 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.

Jason

[PATCH] Teach git-gui to split hunks

[Johannes Schindelin]

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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Wed, 12 Dec 2007, Jason Sewall wrote:

	> On Dec 12, 2007 1:15 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> 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(-)

diff --git a/git-gui.sh b/git-gui.sh
index 1fca11f..0798d41 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -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"] \
diff --git a/lib/diff.tcl b/lib/diff.tcl
index 43565e4..0c3f790 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -289,6 +289,79 @@ proc read_diff {fd} {
 	}
 }
 
+proc split_hunk {x y} {
+	global current_diff_path current_diff_header current_diff_side
+	global ui_diff ui_index file_states
+
+	if {$current_diff_path eq {} || $current_diff_header eq {}} return
+	if {![lock_index apply_hunk]} return
+
+	set c_lno [lindex [split [$ui_diff index @$x,$y] .] 0]
+	set s_idx [$ui_diff search -backwards -regexp ^@@ $c_lno.0 0.0]
+	if {$s_idx eq {} || $s_idx >= [expr $c_lno - 1]} {
+		unlock_index
+		return
+	}
+	set s_lno [lindex [split $s_idx .] 0]
+
+	# the first hunk will look like this: @@ -$m1,$m2 +$p1,$p2 @@
+	# the second hunk will look like this: @@ -$m3,$m4 +$p3,$p4 @@
+
+	# get original hunk numbers
+	set hunk_line [$ui_diff get $s_idx "$s_idx lineend"]
+	set re "@@ +-(\[0-9\]+)(,(\[0-9\]+))? +\\+(\[0-9\]+)(,(\[0-9\]+))? *@@"
+	if {![regexp $re $hunk_line dummy m1 dummy m4 p1 dummy p4] ||
+			$m1 == 0 || $p1 == 0} { # create/delete file
+		unlock_index
+		return
+	}
+	if {$m4 == ""} {
+		set m4 1
+	}
+	if {$p4 == ""} {
+		set p4 1
+	}
+
+	# count changes
+	set m2 0
+	set p2 0
+	for {set l [expr $s_lno + 1]} {$l < $c_lno} {incr l} {
+		switch -exact -- [$ui_diff get $l.0] {
+			" " {
+				incr m2
+				incr p2
+			}
+			"+" {
+				incr p2
+			}
+			"-" {
+				incr m2
+			}
+		}
+	}
+
+	# We could check if {$m2 == $p2 && $m2 == [expr $c_lno - $s_lno]}
+	# and just remove the hunk.  But let's not be too clever here.
+
+	set m3 [expr $m1 + $m2]
+	set m4 [expr $m4 - $m2]
+	set p3 [expr $p1 + $p2]
+	set p4 [expr $p4 - $p2]
+
+	if {$m4 == 0 && $p4 == 0} {
+		index_unlock
+		return
+	}
+
+	$ui_diff configure -state normal
+	$ui_diff delete $s_idx "$s_idx lineend"
+	$ui_diff insert $s_idx "@@ -$m1,$m2 +$p1,$p2 @@" d_@
+	$ui_diff insert $c_lno.0 "@@ -$m3,$m4 +$p3,$p4 @@\n" d_@
+	$ui_diff configure -state disabled
+
+	unlock_index
+}
+
 proc apply_hunk {x y} {
 	global current_diff_path current_diff_header current_diff_side
 	global ui_diff ui_index file_states
@@ -296,7 +369,7 @@ proc apply_hunk {x y} {
 	if {$current_diff_path eq {} || $current_diff_header eq {}} return
 	if {![lock_index apply_hunk]} return
 
-	set apply_cmd {apply --cached --whitespace=nowarn}
+	set apply_cmd {apply --cached --whitespace=nowarn --unidiff-zero}
 	set mi [lindex $file_states($current_diff_path) 0]
 	if {$current_diff_side eq $ui_index} {
 		set failed_msg [mc "Failed to unstage selected hunk."]
-- 
1.5.3.7.2250.g3893

Re: [PATCH] Teach git-gui to split hunks

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> 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.
> ...

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

> @@ -296,7 +369,7 @@ proc apply_hunk {x y} {
>  	if {$current_diff_path eq {} || $current_diff_header eq {}} return
>  	if {![lock_index apply_hunk]} return
>  
> -	set apply_cmd {apply --cached --whitespace=nowarn}
> +	set apply_cmd {apply --cached --whitespace=nowarn --unidiff-zero}
>  	set mi [lindex $file_states($current_diff_path) 0]
>  	if {$current_diff_side eq $ui_index} {
>  		set failed_msg [mc "Failed to unstage selected hunk."]

... by clicking between the '-' and '+' lines, and apply only one half?

Well, the question was not very well stated.  I know what it means --
remove that old line, without replacing with the corrected/updated one.
The real question is how would that be useful?

Re: [PATCH] Teach git-gui to split hunks

[Johannes Schindelin]

Hi,

On Wed, 12 Dec 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > 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. ...
> 
> I still have conceptual problem with this whole thing.  For example, 
> what does that MEAN to split this hunk from your patch...
> 
> > @@ -296,7 +369,7 @@ proc apply_hunk {x y} {
> >  	if {$current_diff_path eq {} || $current_diff_header eq {}} return
> >  	if {![lock_index apply_hunk]} return
> >  
> > -	set apply_cmd {apply --cached --whitespace=nowarn}
> > +	set apply_cmd {apply --cached --whitespace=nowarn --unidiff-zero}
> >  	set mi [lindex $file_states($current_diff_path) 0]
> >  	if {$current_diff_side eq $ui_index} {
> >  		set failed_msg [mc "Failed to unstage selected hunk."]
> 
> ... by clicking between the '-' and '+' lines, and apply only one half?
> 
> Well, the question was not very well stated.  I know what it means -- 
> remove that old line, without replacing with the corrected/updated one. 
> The real question is how would that be useful?

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

Re: [PATCH] Teach git-gui to split hunks

[Jean-François Veillette]

> Well, the question was not very well stated.  I know what it means --
> remove that old line, without replacing with the corrected/updated  
> one.
> The real question is how would that be useful?

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

Re: [PATCH] Teach git-gui to split hunks

[Junio C Hamano]

Jean-François Veillette <jean_francois_veillette@yahoo.ca> writes:

>> Well, the question was not very well stated.  I know what it means --
>> remove that old line, without replacing with the corrected/updated
>> one.
>> The real question is how would that be useful?
>
> 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.

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.

Re: [PATCH] Teach git-gui to split hunks

[Wincent Colaiuta]

El 12/12/2007, a las 21:18, Junio C Hamano escribió:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> 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.
>> ...
>
> I still have conceptual problem with this whole thing.  For example,
> what does that MEAN to split this hunk from your patch...
>
>> @@ -296,7 +369,7 @@ proc apply_hunk {x y} {
>> 	if {$current_diff_path eq {} || $current_diff_header eq {}} return
>> 	if {![lock_index apply_hunk]} return
>>
>> -	set apply_cmd {apply --cached --whitespace=nowarn}
>> +	set apply_cmd {apply --cached --whitespace=nowarn --unidiff-zero}
>> 	set mi [lindex $file_states($current_diff_path) 0]
>> 	if {$current_diff_side eq $ui_index} {
>> 		set failed_msg [mc "Failed to unstage selected hunk."]
>
> ... by clicking between the '-' and '+' lines, and apply only one  
> half?
>
> Well, the question was not very well stated.  I know what it means --
> remove that old line, without replacing with the corrected/updated  
> one.
> The real question is how would that be useful?

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

Re: [PATCH] Teach git-gui to split hunks

[Johannes Sixt]

Johannes Schindelin schrieb:
> 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.

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

Re: [PATCH] Teach git-gui to split hunks

[Shawn O. Pearce]

Johannes Sixt <j.sixt@viscovery.net> wrote:
> Johannes Schindelin schrieb:
> > 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.
> 
> NACK! --unidiff-zero eats your data.

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.
 
> 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.

-- 
Shawn.

Re: [PATCH] Teach git-gui to split hunks

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

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

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

diff --git a/read-cache.c b/read-cache.c
index 7db5588..4d12073 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -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:

diff --git a/read-cache.c b/read-cache.c
index 7db5588..4d12073 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -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.
  */

Re: [PATCH] Teach git-gui to split hunks

[Johannes Sixt]

Junio C Hamano schrieb:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
>> For this to work, apply has to be called with --unidiff-zero, since
>> the new hunks can start or stop with a "-" or "+" line.
> 
> You do not have to do "unidiff zero".  Suppose you have this hunk you
> need to split.
> 
> diff --git a/read-cache.c b/read-cache.c
> index 7db5588..4d12073 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -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.
>   */
...
> 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.

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

Re: [PATCH] Teach git-gui to split hunks

[Johannes Schindelin]

Hi,

On Thu, 13 Dec 2007, Johannes Sixt wrote:

> Johannes Schindelin schrieb:
> > 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.
> 
> NACK! --unidiff-zero eats your data.

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

Re: [PATCH] Teach git-gui to split hunks

[Johannes Schindelin]

Hi,

On Thu, 13 Dec 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > For this to work, apply has to be called with --unidiff-zero, since
> > the new hunks can start or stop with a "-" or "+" line.
> 
> You do not have to do "unidiff zero".  Suppose you have this hunk you
> need to split.
>
> [describes to pick zero or more '-' lines and zero or more '+' lines]

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

Ciao,
Dscho

Re: [PATCH] Teach git-gui to split hunks

[Johannes Sixt]

Johannes Schindelin schrieb:
> On Thu, 13 Dec 2007, Junio C Hamano wrote:
> 
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>>> For this to work, apply has to be called with --unidiff-zero, since
>>> the new hunks can start or stop with a "-" or "+" line.
>> You do not have to do "unidiff zero".  Suppose you have this hunk you
>> need to split.
>>
>> [describes to pick zero or more '-' lines and zero or more '+' lines]
> 
> I thought about that, but the UI is not trivial.  The UI for my solution 
> is.

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

Re: [PATCH] Teach git-gui to split hunks

[Johannes Schindelin]

Hi,

On Thu, 13 Dec 2007, Johannes Sixt wrote:

> Johannes Schindelin schrieb:
> > On Thu, 13 Dec 2007, Junio C Hamano wrote:
> > 
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >>
> >>> For this to work, apply has to be called with --unidiff-zero, since 
> >>> the new hunks can start or stop with a "-" or "+" line.
> >> You do not have to do "unidiff zero".  Suppose you have this hunk you 
> >> need to split.
> >>
> >> [describes to pick zero or more '-' lines and zero or more '+' lines]
> > 
> > I thought about that, but the UI is not trivial.  The UI for my 
> > solution is.
> 
> 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.

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

[PATCH] git-gui: Move frequently used commands to the top of the context menu.

[Johannes Sixt]

From: Johannes Sixt <johannes.sixt@telecom.at>

"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.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 Johannes Schindelin schrieb:
 > On Thu, 13 Dec 2007, Johannes Sixt wrote:
 >> 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.
 >
 > And your hand falls off... ;-)

 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(-)

diff --git a/git-gui.sh b/git-gui.sh
index 95b9537..8e3751f 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -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

Re: [PATCH] git-gui: Move frequently used commands to the top of the context menu.

[Shawn O. Pearce]

Johannes Sixt <j.sixt@viscovery.net> wrote:
> "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.

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

-- 
Shawn.