[PATCH] git gui: show diffs with a minimum of 1 context line

git apply does not handle diffs without context correctly. Configuring git
gui to show zero context lines therefore breaks staging.

Signed-off-by: Clemens Buchacher <redacted>
---

In reply to this patch I will send a first attempt at fixing this problem
instead of avoiding it. There does not seem to be a straightforward
solution, however, so this should hide the bug for now.

 git-gui/git-gui.sh     |    2 +-
 git-gui/lib/diff.tcl   |    2 +-
 git-gui/lib/option.tcl |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index ad65aaa..86402d4 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh

@@ -1932,7 +1932,7 @@ proc show_more_context {} {
 
 proc show_less_context {} {
 	global repo_config
-	if {$repo_config(gui.diffcontext) >= 1} {
+	if {$repo_config(gui.diffcontext) > 1} {
 		incr repo_config(gui.diffcontext) -1
 		reshow_diff
 	}

diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl
index 52b79e4..4a7138b 100644
--- a/git-gui/lib/diff.tcl
+++ b/git-gui/lib/diff.tcl

@@ -175,7 +175,7 @@ proc show_diff {path w {lno {}} {scroll_pos {}}} {
 
 	lappend cmd -p
 	lappend cmd --no-color
-	if {$repo_config(gui.diffcontext) >= 0} {
+	if {$repo_config(gui.diffcontext) >= 1} {
 		lappend cmd "-U$repo_config(gui.diffcontext)"
 	}
 	if {$w eq $ui_index} {

diff --git a/git-gui/lib/option.tcl b/git-gui/lib/option.tcl
index ffb3f00..5e1346e 100644
--- a/git-gui/lib/option.tcl
+++ b/git-gui/lib/option.tcl

@@ -125,7 +125,7 @@ proc do_options {} {
 		{b gui.matchtrackingbranch {mc "Match Tracking Branches"}}
 		{b gui.fastcopyblame {mc "Blame Copy Only On Changed Files"}}
 		{i-20..200 gui.copyblamethreshold {mc "Minimum Letters To Blame Copy On"}}
-		{i-0..99 gui.diffcontext {mc "Number of Diff Context Lines"}}
+		{i-1..99 gui.diffcontext {mc "Number of Diff Context Lines"}}
 		{i-0..99 gui.commitmsgwidth {mc "Commit Message Text Width"}}
 		{t gui.newbranchtemplate {mc "New Branch Name Template"}}
 		} {

-- 
1.6.0

git apply does not work correctly with zero-context patches. It does a
little better with --unidiff-zero.
---

This appears to fix staging hunks with zero context lines in the majority of
cases. Staging individual lines still is a problem frequently.

In any case, it's easy enough to break zero-context diff & patch like this:

echo a > victim
git add victim
echo b >> victim
git diff -U0 | git apply --cached --unidiff-zero
git diff

So before delving into this problem to deeply, I'd like to find out who
needs fixing exactly. Is there documentation defining how zero-context git
diff output should look like? Or is git apply the culprit in the bug above?

Or do we even want to support applying zero-context patches? If not, we
should detect and fail such attempts.

Clemens

 git-gui/lib/diff.tcl |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl
index 52b79e4..78c1b56 100644
--- a/git-gui/lib/diff.tcl
+++ b/git-gui/lib/diff.tcl

@@ -302,12 +302,15 @@ proc read_diff {fd scroll_pos} {
 
 proc apply_hunk {x y} {
 	global current_diff_path current_diff_header current_diff_side
-	global ui_diff ui_index file_states
+	global ui_diff ui_index file_states repo_config
 
 	if {$current_diff_path eq {} || $current_diff_header eq {}} return
 	if {![lock_index apply_hunk]} return
 
 	set apply_cmd {apply --cached --whitespace=nowarn}
+	if {$repo_config(gui.diffcontext) eq 0} {
+		lappend apply_cmd --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."]

@@ -375,12 +378,15 @@ proc apply_hunk {x y} {
 
 proc apply_line {x y} {
 	global current_diff_path current_diff_header current_diff_side
-	global ui_diff ui_index file_states
+	global ui_diff ui_index file_states repo_config
 
 	if {$current_diff_path eq {} || $current_diff_header eq {}} return
 	if {![lock_index apply_hunk]} return
 
 	set apply_cmd {apply --cached --whitespace=nowarn}
+	if {$repo_config(gui.diffcontext) eq 0} {
+		lappend apply_cmd --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 line."]

-- 
1.6.0

Clemens Buchacher schrieb:

git apply does not work correctly with zero-context patches. It does a
little better with --unidiff-zero.

No, NO, NOOOOO! This kills your data!

http://thread.gmane.org/gmane.comp.version-control.git/67854/focus=68127

-- Hannes

On Sat, Aug 30, 2008 at 09:43:19PM +0200, Johannes Sixt wrote:

Clemens Buchacher schrieb:

quoted

git apply does not work correctly with zero-context patches. It does a
little better with --unidiff-zero.

No, NO, NOOOOO! This kills your data!

Okay. Since we have 'Stage Line for Commit', supporting this would be almost
pointless anyways. So let's forget about trying to fix this and simply
disable zero-context diff in git-gui, as per my original patch

[PATCH] git gui: show diffs with a minimum of 1 context line

Clemens

Staging hunks without context does not work, because line number
information would have to be recomputed for individual hunks.  Since it is
already possible to stage individual lines using 'Stage Line for Commit',
zero context diffs are not really necessary for git gui, however.

Signed-off-by: Clemens Buchacher <redacted>
---

Same patch, different commit message.

 git-gui/git-gui.sh     |    2 +-
 git-gui/lib/diff.tcl   |    2 +-
 git-gui/lib/option.tcl |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index ad65aaa..86402d4 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh

@@ -1932,7 +1932,7 @@ proc show_more_context {} {
 
 proc show_less_context {} {
 	global repo_config
-	if {$repo_config(gui.diffcontext) >= 1} {
+	if {$repo_config(gui.diffcontext) > 1} {
 		incr repo_config(gui.diffcontext) -1
 		reshow_diff
 	}

diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl
index 52b79e4..4a7138b 100644
--- a/git-gui/lib/diff.tcl
+++ b/git-gui/lib/diff.tcl

@@ -175,7 +175,7 @@ proc show_diff {path w {lno {}} {scroll_pos {}}} {
 
 	lappend cmd -p
 	lappend cmd --no-color
-	if {$repo_config(gui.diffcontext) >= 0} {
+	if {$repo_config(gui.diffcontext) >= 1} {
 		lappend cmd "-U$repo_config(gui.diffcontext)"
 	}
 	if {$w eq $ui_index} {

diff --git a/git-gui/lib/option.tcl b/git-gui/lib/option.tcl
index ffb3f00..5e1346e 100644
--- a/git-gui/lib/option.tcl
+++ b/git-gui/lib/option.tcl

@@ -125,7 +125,7 @@ proc do_options {} {
 		{b gui.matchtrackingbranch {mc "Match Tracking Branches"}}
 		{b gui.fastcopyblame {mc "Blame Copy Only On Changed Files"}}
 		{i-20..200 gui.copyblamethreshold {mc "Minimum Letters To Blame Copy On"}}
-		{i-0..99 gui.diffcontext {mc "Number of Diff Context Lines"}}
+		{i-1..99 gui.diffcontext {mc "Number of Diff Context Lines"}}
 		{i-0..99 gui.commitmsgwidth {mc "Commit Message Text Width"}}
 		{t gui.newbranchtemplate {mc "New Branch Name Template"}}
 		} {

-- 
1.6.0

Clemens Buchacher [off-list ref] wrote:

git apply does not handle diffs without context correctly. Configuring git
gui to show zero context lines therefore breaks staging.

Signed-off-by: Clemens Buchacher <redacted>

Thanks, this is queued for 'maint'.
 
-- 
Shawn.

On Mon, Sep 01, 2008 at 12:33:59PM -0700, Shawn O. Pearce wrote:

Clemens Buchacher [off-list ref] wrote:

quoted

git apply does not handle diffs without context correctly. Configuring git
gui to show zero context lines therefore breaks staging.

Signed-off-by: Clemens Buchacher <redacted>

Thanks, this is queued for 'maint'.

Actually, if you don't mind, I changed the commit message because 'git
apply' is not really to blame here:

Staging hunks without context does not work, because line number
information would have to be recomputed for individual hunks.  Since it is
already possible to stage individual lines using 'Stage Line for Commit',
zero context diffs are not really necessary for git gui, however.

Signed-off-by: Clemens Buchacher <redacted>

Clemens Buchacher [off-list ref] wrote:

Actually, if you don't mind, I changed the commit message because 'git
apply' is not really to blame here:

Staging hunks without context does not work, because line number
information would have to be recomputed for individual hunks.  Since it is
already possible to stage individual lines using 'Stage Line for Commit',
zero context diffs are not really necessary for git gui, however.

Thanks, message updated.

-- 
Shawn.