{"thread":{"id":"9230","subject":"[PATCH] Teach git-gui to split hunks","startedAt":"2007-07-26T05:32:07Z","lastAt":"2007-07-26T14:34:22Z","messageCount":6,"participants":["Johannes Schindelin","David Kastrup","Shawn O. Pearce","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"48641","messageId":"Pine.LNX.4.64.0707260630570.14781@racer.site","threadId":"9230","inReplyTo":null,"subject":"[PATCH] Teach git-gui to split hunks","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-26T05:32:07Z","receivedAt":"2007-07-26T05:32:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nWhen you select the context menu item \"Split Hunk\" in the diff area,\ngit-gui will now split the current hunk so that a new hunk starts at\nthe current position.\n\nFor this to work, apply has to be called with --unidiff-zero, since\nthe new hunks can start or stop with a \"-\" or \"+\" line.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tNo more complaining from me about the lack of that feature.\n\n\tAnd I did not even need a C compiler to do that.\n\n git-gui.sh | 4 +++\n lib/diff.tcl | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 78 insertions(+), 1 deletions(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex a38293a..a0f7617 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -2400,6 +2400,10 @@ $ctxm add command \\\n \t-command {apply_hunk $cursorX $cursorY}\n set ui_diff_applyhunk [$ctxm index last]\n lappend diff_actions [list $ctxm entryconf $ui_diff_applyhunk -state]\n+$ctxm add command \\\n+\t-label [mc \"Split Hunk\"] \\\n+\t-command {split_hunk $cursorX $cursorY}\n+lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]\n $ctxm add separator\n $ctxm add command \\\n \t-label {Decrease Font Size} \\\ndiff --git a/lib/diff.tcl b/lib/diff.tcl\nindex e09e125..88ec8f3 100644\n--- a/lib/diff.tcl\n+++ b/lib/diff.tcl\n@@ -265,6 +265,79 @@ proc read_diff {fd} {\n \t}\n }\n \n+proc split_hunk {x y} {\n+\tglobal current_diff_path current_diff_header current_diff_side\n+\tglobal ui_diff ui_index file_states\n+\n+\tif {$current_diff_path eq {} || $current_diff_header eq {}} return\n+\tif {![lock_index apply_hunk]} return\n+\n+\tset c_lno [lindex [split [$ui_diff index @$x,$y] .] 0]\n+\tset s_idx [$ui_diff search -backwards -regexp ^@@ $c_lno.0 0.0]\n+\tif {$s_idx eq {} || $s_idx >= [expr $c_lno - 1]} {\n+\t\tunlock_index\n+\t\treturn\n+\t}\n+\tset s_lno [lindex [split $s_idx .] 0]\n+\n+\t# the first hunk will look like this: @@ -$m1,$m2 +$p1,$p2 @@\n+\t# the second hunk will look like this: @@ -$m3,$m4 +$p3,$p4 @@\n+\n+\t# get original hunk numbers\n+\tset hunk_line [$ui_diff get $s_idx \"$s_idx lineend\"]\n+\tset re \"@@ +-(\\[0-9\\]+)(,(\\[0-9\\]+))? +\\\\+(\\[0-9\\]+)(,(\\[0-9\\]+))? *@@\"\n+\tif {![regexp $re $hunk_line dummy m1 dummy m4 p1 dummy p4] ||\n+\t\t\t$m1 == 0 || $p1 == 0} { # create/delete file\n+\t\tunlock_index\n+\t\treturn\n+\t}\n+\tif {$m4 == \"\"} {\n+\t\tset m4 1\n+\t}\n+\tif {$p4 == \"\"} {\n+\t\tset p4 1\n+\t}\n+\n+\t# count changes\n+\tset m2 0\n+\tset p2 0\n+\tfor {set l [expr $s_lno + 1]} {$l < $c_lno} {incr l} {\n+\t\tswitch -exact -- [$ui_diff get $l.0] {\n+\t\t\t\" \" {\n+\t\t\t\tincr m2\n+\t\t\t\tincr p2\n+\t\t\t}\n+\t\t\t\"+\" {\n+\t\t\t\tincr p2\n+\t\t\t}\n+\t\t\t\"-\" {\n+\t\t\t\tincr m2\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t# We could check if {$m2 == $p2 && $m2 == [expr $c_lno - $s_lno]}\n+\t# and just remove the hunk. But let's not be too clever here.\n+\n+\tset m3 [expr $m1 + $m2]\n+\tset m4 [expr $m4 - $m2]\n+\tset p3 [expr $p1 + $p2]\n+\tset p4 [expr $p4 - $p2]\n+\n+\tif {$m4 == 0 && $p4 == 0} {\n+\t\tindex_unlock\n+\t\treturn\n+\t}\n+\n+\t$ui_diff configure -state normal\n+\t$ui_diff delete $s_idx \"$s_idx lineend\"\n+\t$ui_diff insert $s_idx \"@@ -$m1,$m2 +$p1,$p2 @@\" d_@\n+\t$ui_diff insert $c_lno.0 \"@@ -$m3,$m4 +$p3,$p4 @@\\n\" d_@\n+\t$ui_diff configure -state disabled\n+\n+\tunlock_index\n+}\n+\n proc apply_hunk {x y} {\n \tglobal current_diff_path current_diff_header current_diff_side\n \tglobal ui_diff ui_index file_states\n@@ -272,7 +345,7 @@ proc apply_hunk {x y} {\n \tif {$current_diff_path eq {} || $current_diff_header eq {}} return\n \tif {![lock_index apply_hunk]} return\n \n-\tset apply_cmd {apply --cached --whitespace=nowarn}\n+\tset apply_cmd {apply --cached --whitespace=nowarn --unidiff-zero}\n \tset mi [lindex $file_states($current_diff_path) 0]\n \tif {$current_diff_side eq $ui_index} {\n \t\tset mode unstage\n-- \n1.5.3.rc2.42.gda8d-dirty\n"},{"id":"48644","messageId":"85ir8790lj.fsf@lola.goethe.zz","threadId":"9230","inReplyTo":"Pine.LNX.4.64.0707260630570.14781@racer.site","subject":"Re: [PATCH] Teach git-gui to split hunks","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-07-26T05:48:40Z","receivedAt":"2007-07-26T05:48:40Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Johannes Schindelin writes:\n\n> When you select the context menu item \"Split Hunk\" in the diff area,\n> git-gui will now split the current hunk so that a new hunk starts at\n> the current position.\n>\n> For this to work, apply has to be called with --unidiff-zero, since\n> the new hunks can start or stop with a \"-\" or \"+\" line.\n\nUnless one splits right at the beginning or end of an existing hunk,\nwouldn't there be context which one could use? Or does it confuse\npatch when adjacent hunks have overlapping contexts? At least if the\nfirst hunk patches what is to be used as context in the second hunk, I\ncould imagine this. And there is really no danger of losing synch in\nthis situation, anyhow. So it would be more of a convenience thing\nthan anything else to be able to omit --unidiff-zero.\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"48668","messageId":"20070726070744.GD18114@spearce.org","threadId":"9230","inReplyTo":"85ir8790lj.fsf@lola.goethe.zz","subject":"Re: [PATCH] Teach git-gui to split hunks","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-07-26T07:07:44Z","receivedAt":"2007-07-26T07:07:44Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"David Kastrup wrote:\n> Johannes Schindelin writes:\n> \n> > When you select the context menu item \"Split Hunk\" in the diff area,\n> > git-gui will now split the current hunk so that a new hunk starts at\n> > the current position.\n> >\n> > For this to work, apply has to be called with --unidiff-zero, since\n> > the new hunks can start or stop with a \"-\" or \"+\" line.\n> \n> Unless one splits right at the beginning or end of an existing hunk,\n> wouldn't there be context which one could use? Or does it confuse\n> patch when adjacent hunks have overlapping contexts? At least if the\n> first hunk patches what is to be used as context in the second hunk, I\n> could imagine this. And there is really no danger of losing synch in\n> this situation, anyhow. So it would be more of a convenience thing\n> than anything else to be able to omit --unidiff-zero.\n\nYea, there's context there. Junio and I talked about this patch on\n#git a few minutes ago. I really appreciate that Dscho wrote it,\nespecially given that he hasn't really been into Tcl hacking for\nGit much before. But I'd really like to save/create context, like\n`git add -i` does, so that we don't have to use --unidiff-zero here.\n\nIt won't matter if git-apply rejected overlapping context in this\ncase. git-gui will only ever feed one hunk at a time to git-apply.\nAnd if things get really f'd in the diff buffer the user can easily\nregenerate it (right click, Refresh).\n\nRight now git-gui's apply doesn't correctly update the other hunk\nheaders when you apply a hunk. I've seen git-apply fail on some\nhunks just for this reason. Refreshing the diff (so git recomputes\nthe headers) works around the issue. So I'm a little worried about\nusing --unidiff-zero here.\n\n-- \nShawn.\n"},{"id":"48673","messageId":"46A84E06.2ED4CFBC@eudaptics.com","threadId":"9230","inReplyTo":"Pine.LNX.4.64.0707260630570.14781@racer.site","subject":"Re: [PATCH] Teach git-gui to split hunks","fromName":"Johannes Sixt","fromEmail":"j.sixt@eudaptics.com","sentAt":"2007-07-26T07:32:22Z","receivedAt":"2007-07-26T07:32:22Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin wrote:\n> \n> When you select the context menu item \"Split Hunk\" in the diff area,\n> git-gui will now split the current hunk so that a new hunk starts at\n> the current position.\n> \n> For this to work, apply has to be called with --unidiff-zero, since\n> the new hunks can start or stop with a \"-\" or \"+\" line.\n\nFor chrissake, NO!\n\nI tried this already, and it immediately corrupted my data.\n\nThe problem case is when the hunk you want to apply is not the first one\nand the first one does not add and remove the same number of lines. In\nthis case, all that git-apply can do is to rely on line numbers. But\nthey are WRONG and apply the patch at the WRONG spot.\n\nFirst, I didn't believe Linus when he preached that --unidiff-zero is\nbad; it took only a day to become a follower. ;)\n\n-- Hannes\n"},{"id":"48706","messageId":"Pine.LNX.4.64.0707261526090.14781@racer.site","threadId":"9230","inReplyTo":"46A84E06.2ED4CFBC@eudaptics.com","subject":"Re: [PATCH] Teach git-gui to split hunks","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-26T14:26:57Z","receivedAt":"2007-07-26T14:26:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 26 Jul 2007, Johannes Sixt wrote:\n\n> Johannes Schindelin wrote:\n> > \n> > When you select the context menu item \"Split Hunk\" in the diff area,\n> > git-gui will now split the current hunk so that a new hunk starts at\n> > the current position.\n> > \n> > For this to work, apply has to be called with --unidiff-zero, since\n> > the new hunks can start or stop with a \"-\" or \"+\" line.\n> \n> For chrissake, NO!\n> \n> I tried this already, and it immediately corrupted my data.\n> \n> The problem case is when the hunk you want to apply is not the first one\n> and the first one does not add and remove the same number of lines. In\n> this case, all that git-apply can do is to rely on line numbers. But\n> they are WRONG and apply the patch at the WRONG spot.\n> \n> First, I didn't believe Linus when he preached that --unidiff-zero is\n> bad; it took only a day to become a follower. ;)\n\nOkay, convinced. But I have some issues there, which I will outline in \nthe reply to Shawn.\n\nCiao,\nDscho\n"},{"id":"48707","messageId":"Pine.LNX.4.64.0707261527040.14781@racer.site","threadId":"9230","inReplyTo":"20070726070744.GD18114@spearce.org","subject":"Re: [PATCH] Teach git-gui to split hunks","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-26T14:34:22Z","receivedAt":"2007-07-26T14:34:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 26 Jul 2007, Shawn O. Pearce wrote:\n\n> [talk about the --unidiff-zero stuff]\n> \n> Yea, there's context there. Junio and I talked about this patch on #git \n> a few minutes ago. I really appreciate that Dscho wrote it, especially \n> given that he hasn't really been into Tcl hacking for Git much before.\n\nHeh. I did my share of Tcl hacking. In one (closed source) project, I \nwas in constant awe why the devs chose Tcl as a scripting language, but Qt \nas windowing library (back when it was pretty expensive for a startup to \nuse Qt).\n\n> But I'd really like to save/create context, like `git add -i` does, so \n> that we don't have to use --unidiff-zero here.\n\nSee below.\n\n> It won't matter if git-apply rejected overlapping context in this case. \n> git-gui will only ever feed one hunk at a time to git-apply. And if \n> things get really f'd in the diff buffer the user can easily regenerate \n> it (right click, Refresh).\n\nBut you're right, the other hunk headers should be updated. I'll have a \nlook at it this weekend.\n\n> Right now git-gui's apply doesn't correctly update the other hunk \n> headers when you apply a hunk. I've seen git-apply fail on some hunks \n> just for this reason. Refreshing the diff (so git recomputes the \n> headers) works around the issue. So I'm a little worried about using \n> --unidiff-zero here.\n\nOkay, but just using more context means that you also have to _update_ the \ncontext of another hunk. Imagine this:\n\n--- a/x\n+++ a/x\n one line\n-a removed one\n+an added one\n another line\n\nNow you split between the \"-\" and \"+\" line. If you stage the first hunk, \nnot only do you have to update the hunk header of the second hunk (now the \nonly one shown), which you already hinted should be done; you _also_ have \nto update the context in the second hunk, since it changed.\n\nThis is just tricky enough that I am tempted to have a go at it. Not \ntoday, though, since I have other work to do, and the issues are easier to \nwork around for the moment than to work around the lack of \"Split Hunk\" \nfor me.\n\nBTW did you think about any kind of integrity checking, i.e. see if the \nfiles involved still have the same hashes as when the diff was generated? \nIt _is_ possible to use git-gui _and_ the command line...\n\nCiao,\nDscho\n"}]}