[PATCH 1/8] git-gui: rework handling of diff header

[PATCH 1/8] git-gui: rework handling of diff header

[Bert Wesarg]

The fix in ca53c3f (Fix diff parsing for lines starting with "--" or "++",
2008-09-05) got a bug report from Johannes Sixt, that new files in the
index now looks like:

new file mode 100644
--- /dev/null
+++ b/foo
@@ -0,0 +1 @@
+foo

The introduced problem was that the 'in-diff-header'-flag was unconditially
disabled. Now it is only disabled when a hunk line is detected. And also
re-enabled when we encounter a new diff header.

The second part solves also the issue reported by me for diffs with file
type changes (i.e. the ''error: Unhandled 2 way diff marker: {d}"', which
comes from the second 'diff --git' line).

Reported-by: Johannes Sixt <j.sixt@viscovery.net>
Reported-by: Bert Wesarg <bert.wesarg@googlemail.com>
Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>

---

Cc: Johannes Sixt <j.sixt@viscovery.net>
---
 foo          |    1 +
 lib/diff.tcl |   18 +++++++++++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)
 create mode 100644 foo

diff --git a/foo b/foo
new file mode 100644
index 0000000..257cc56
--- /dev/null
+++ b/foo
@@ -0,0 +1 @@
+foo
diff --git a/lib/diff.tcl b/lib/diff.tcl
index 9d0dc07..dae6ca6 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -382,23 +382,35 @@ proc read_diff {fd conflict_size cont_info} {
 		foreach {line markup} [parse_color_line $line] break
 		set line [string map {\033 ^} $line]
 
-		# -- Cleanup uninteresting diff header lines.
+		# -- Check for start of diff header.
+		if {   [string match {diff --git *}      $line]
+		    || [string match {diff --cc *}       $line]
+		    || [string match {diff --combined *} $line]} {
+			set ::current_diff_inheader 1
+		}
+
+		# -- Check for end of diff header (any hunk line will do this).
 		#
+		if {[regexp {^@@+ } $line]} {set ::current_diff_inheader 0}
+
 		if {$::current_diff_inheader} {
+			append current_diff_header $line "\n"
+
+			# -- Cleanup uninteresting diff header lines.
+			#
 			if {   [string match {diff --git *}      $line]
 			    || [string match {diff --cc *}       $line]
 			    || [string match {diff --combined *} $line]
 			    || [string match {--- *}             $line]
 			    || [string match {+++ *}             $line]} {
-				append current_diff_header $line "\n"
 				continue
 			}
 		}
+
 		if {[string match {index *} $line]} continue
 		if {$line eq {deleted file mode 120000}} {
 			set line "deleted symlink"
 		}
-		set ::current_diff_inheader 0
 
 		# -- Automatically detect if this is a 3 way diff.
 		#
-- 
1.7.3.2.1200.ge4bf6

[PATCH 2/8] git-gui: handle index lines only in the diff header

[Bert Wesarg]

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
 lib/diff.tcl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/diff.tcl b/lib/diff.tcl
index dae6ca6..0b72924 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -402,12 +402,12 @@ proc read_diff {fd conflict_size cont_info} {
 			    || [string match {diff --cc *}       $line]
 			    || [string match {diff --combined *} $line]
 			    || [string match {--- *}             $line]
-			    || [string match {+++ *}             $line]} {
+			    || [string match {+++ *}             $line]
+			    || [string match {index *}           $line]} {
 				continue
 			}
 		}
 
-		if {[string match {index *} $line]} continue
 		if {$line eq {deleted file mode 120000}} {
 			set line "deleted symlink"
 		}
-- 
1.7.3.2.1200.ge4bf6

[PATCH 3/8] git-gui: name also new symlinks so

[Bert Wesarg]

and rename them only in the diff header

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
 lib/diff.tcl |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/diff.tcl b/lib/diff.tcl
index 0b72924..aa30089 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -406,12 +406,13 @@ proc read_diff {fd conflict_size cont_info} {
 			    || [string match {index *}           $line]} {
 				continue
 			}
-		}
 
-		if {$line eq {deleted file mode 120000}} {
-			set line "deleted symlink"
+			# -- Name it symlink, not 120000
+			#    Note, that the original line is in $current_diff_header
+			regsub {^(deleted|new) file mode 120000} $line {\1 symlink} line
 		}
 
+
 		# -- Automatically detect if this is a 3 way diff.
 		#
 		if {[string match {@@@ *} $line]} {set is_3way_diff 1}
@@ -421,6 +422,7 @@ proc read_diff {fd conflict_size cont_info} {
 			|| [regexp {^(old|new) mode *} $line]
 			|| [string match {deleted file *} $line]
 			|| [string match {deleted symlink} $line]
+			|| [string match {new symlink} $line]
 			|| [string match {Binary files * and * differ} $line]
 			|| $line eq {\ No newline at end of file}
 			|| [regexp {^\* Unmerged path } $line]} {
-- 
1.7.3.2.1200.ge4bf6

[PATCH 4/8] git-gui: there is no "mode *" diff header line

[Bert Wesarg]

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
 lib/diff.tcl |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/lib/diff.tcl b/lib/diff.tcl
index aa30089..20f50dd 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -417,8 +417,7 @@ proc read_diff {fd conflict_size cont_info} {
 		#
 		if {[string match {@@@ *} $line]} {set is_3way_diff 1}
 
-		if {[string match {mode *} $line]
-			|| [string match {new file *} $line]
+		if {[string match {new file *} $line]
 			|| [regexp {^(old|new) mode *} $line]
 			|| [string match {deleted file *} $line]
 			|| [string match {deleted symlink} $line]
-- 
1.7.3.2.1200.ge4bf6

[PATCH 5/8] git-gui: move 3way diff autodetect up

[Bert Wesarg]

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
 lib/diff.tcl |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/diff.tcl b/lib/diff.tcl
index 20f50dd..2290919 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -393,6 +393,10 @@ proc read_diff {fd conflict_size cont_info} {
 		#
 		if {[regexp {^@@+ } $line]} {set ::current_diff_inheader 0}
 
+		# -- Automatically detect if this is a 3 way diff.
+		#
+		if {[string match {@@@ *} $line]} {set is_3way_diff 1}
+
 		if {$::current_diff_inheader} {
 			append current_diff_header $line "\n"
 
@@ -413,9 +417,6 @@ proc read_diff {fd conflict_size cont_info} {
 		}
 
 
-		# -- Automatically detect if this is a 3 way diff.
-		#
-		if {[string match {@@@ *} $line]} {set is_3way_diff 1}
 
 		if {[string match {new file *} $line]
 			|| [regexp {^(old|new) mode *} $line]
-- 
1.7.3.2.1200.ge4bf6

[PATCH 6/8] git-gui: always reset the current tag

[Bert Wesarg]

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
 lib/diff.tcl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/lib/diff.tcl b/lib/diff.tcl
index 2290919..91ed7c6 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -382,6 +382,8 @@ proc read_diff {fd conflict_size cont_info} {
 		foreach {line markup} [parse_color_line $line] break
 		set line [string map {\033 ^} $line]
 
+		set tags {}
+
 		# -- Check for start of diff header.
 		if {   [string match {diff --git *}      $line]
 		    || [string match {diff --cc *}       $line]
@@ -426,7 +428,6 @@ proc read_diff {fd conflict_size cont_info} {
 			|| [string match {Binary files * and * differ} $line]
 			|| $line eq {\ No newline at end of file}
 			|| [regexp {^\* Unmerged path } $line]} {
-			set tags {}
 		} elseif {$is_3way_diff} {
 			set op [string range $line 0 1]
 			switch -- $op {
-- 
1.7.3.2.1200.ge4bf6

[PATCH 7/8] git-gui: handle special content lines only in the diff header section

[Bert Wesarg]

These two also stop the diff header.

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
 lib/diff.tcl |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/lib/diff.tcl b/lib/diff.tcl
index 91ed7c6..4eaf7e7 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -400,7 +400,14 @@ proc read_diff {fd conflict_size cont_info} {
 		if {[string match {@@@ *} $line]} {set is_3way_diff 1}
 
 		if {$::current_diff_inheader} {
-			append current_diff_header $line "\n"
+
+			# -- These two lines stop a diff header and shouldn't be in there
+			if {   [string match {Binary files * and * differ} $line]
+			    || [regexp {^\* Unmerged path }                $line]} {
+				set ::current_diff_inheader 0
+			} else {
+				append current_diff_header $line "\n"
+			}
 
 			# -- Cleanup uninteresting diff header lines.
 			#
@@ -418,16 +425,12 @@ proc read_diff {fd conflict_size cont_info} {
 			regsub {^(deleted|new) file mode 120000} $line {\1 symlink} line
 		}
 
-
-
 		if {[string match {new file *} $line]
 			|| [regexp {^(old|new) mode *} $line]
 			|| [string match {deleted file *} $line]
 			|| [string match {deleted symlink} $line]
 			|| [string match {new symlink} $line]
-			|| [string match {Binary files * and * differ} $line]
-			|| $line eq {\ No newline at end of file}
-			|| [regexp {^\* Unmerged path } $line]} {
+			|| $line eq {\ No newline at end of file}} {
 		} elseif {$is_3way_diff} {
 			set op [string range $line 0 1]
 			switch -- $op {
-- 
1.7.3.2.1200.ge4bf6

[PATCH 8/8] git-gui: handle meta diff header lines only in the header section

[Bert Wesarg]

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
 lib/diff.tcl |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/lib/diff.tcl b/lib/diff.tcl
index 4eaf7e7..cf8a95e 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -423,14 +423,9 @@ proc read_diff {fd conflict_size cont_info} {
 			# -- Name it symlink, not 120000
 			#    Note, that the original line is in $current_diff_header
 			regsub {^(deleted|new) file mode 120000} $line {\1 symlink} line
-		}
 
-		if {[string match {new file *} $line]
-			|| [regexp {^(old|new) mode *} $line]
-			|| [string match {deleted file *} $line]
-			|| [string match {deleted symlink} $line]
-			|| [string match {new symlink} $line]
-			|| $line eq {\ No newline at end of file}} {
+		} elseif {   $line eq {\ No newline at end of file}} {
+			# -- Handle some special lines
 		} elseif {$is_3way_diff} {
 			set op [string range $line 0 1]
 			switch -- $op {
-- 
1.7.3.2.1200.ge4bf6

Re: [PATCH 1/8] git-gui: rework handling of diff header

[Pat Thoyts]

Bert Wesarg <bert.wesarg@googlemail.com> writes:

>The fix in ca53c3f (Fix diff parsing for lines starting with "--" or "++",
>2008-09-05) got a bug report from Johannes Sixt, that new files in the
>index now looks like:
>
>new file mode 100644
>--- /dev/null
>+++ b/foo
>@@ -0,0 +1 @@
>+foo

Inlining this in the commit comment causes 'git am' to terminate the
comment text early. You should add some leading space when putting text
that looks like patch header into this section.
>
>The introduced problem was that the 'in-diff-header'-flag was unconditially
>disabled. Now it is only disabled when a hunk line is detected. And also
>re-enabled when we encounter a new diff header.
>
>The second part solves also the issue reported by me for diffs with file
>type changes (i.e. the ''error: Unhandled 2 way diff marker: {d}"', which
>comes from the second 'diff --git' line).
>
>Reported-by: Johannes Sixt <j.sixt@viscovery.net>
>Reported-by: Bert Wesarg <bert.wesarg@googlemail.com>
>Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>
>---
>
>Cc: Johannes Sixt <j.sixt@viscovery.net>
>---
> foo          |    1 +
> lib/diff.tcl |   18 +++++++++++++++---
> 2 files changed, 16 insertions(+), 3 deletions(-)
> create mode 100644 foo
>
>diff --git a/foo b/foo
>new file mode 100644
>index 0000000..257cc56
>--- /dev/null
>+++ b/foo
>@@ -0,0 +1 @@
>+foo

We don't need a 'foo' file :)

>diff --git a/lib/diff.tcl b/lib/diff.tcl
>index 9d0dc07..dae6ca6 100644
>--- a/lib/diff.tcl
>+++ b/lib/diff.tcl
>@@ -382,23 +382,35 @@ proc read_diff {fd conflict_size cont_info} {
> 		foreach {line markup} [parse_color_line $line] break
> 		set line [string map {\033 ^} $line]
> 
>-		# -- Cleanup uninteresting diff header lines.
>+		# -- Check for start of diff header.
>+		if {   [string match {diff --git *}      $line]
>+		    || [string match {diff --cc *}       $line]
>+		    || [string match {diff --combined *} $line]} {
>+			set ::current_diff_inheader 1
>+		}
>+
>+		# -- Check for end of diff header (any hunk line will do this).
> 		#
>+		if {[regexp {^@@+ } $line]} {set ::current_diff_inheader 0}
>+
> 		if {$::current_diff_inheader} {
>+			append current_diff_header $line "\n"
>+
>+			# -- Cleanup uninteresting diff header lines.
>+			#
> 			if {   [string match {diff --git *}      $line]
> 			    || [string match {diff --cc *}       $line]
> 			    || [string match {diff --combined *} $line]
> 			    || [string match {--- *}             $line]
> 			    || [string match {+++ *}             $line]} {
>-				append current_diff_header $line "\n"
> 				continue
> 			}
> 		}
>+
> 		if {[string match {index *} $line]} continue
> 		if {$line eq {deleted file mode 120000}} {
> 			set line "deleted symlink"
> 		}
>-		set ::current_diff_inheader 0
> 
> 		# -- Automatically detect if this is a 3 way diff.
> 		#

The patch series 1 to 8 are fine. I've taken them without the foo file
mentioned above and adding some spaces to the commit comment as noted
above. This replaces a couple of tentative patches we've discussed
previously for errors on stderr when symlinks replace files.
Thank you - applying to master.

-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD