[PATCH 2/2] git-gui: support for diff3 conflict style

[PATCH 2/2] git-gui: support for diff3 conflict style

[Bert Wesarg]

This adds highlight support for the diff3 conflict style.

The common pre-image will be reversed to --, because it has been removed
and either replaced with our or their side.

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>

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

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 38362fa..0134438 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -3354,6 +3354,9 @@ $ui_diff tag conf d_s- \
 $ui_diff tag conf d< \
 	-foreground orange \
 	-font font_diffbold
+$ui_diff tag conf d| \
+	-foreground orange \
+	-font font_diffbold
 $ui_diff tag conf d= \
 	-foreground orange \
 	-font font_diffbold
diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl
index d4e2ce3..ccd4c70 100644
--- a/git-gui/lib/diff.tcl
+++ b/git-gui/lib/diff.tcl
@@ -339,6 +339,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
 	}
 
 	set ::current_diff_inheader 1
+	set ::in_conflict_pre_image 0
 	fconfigure $fd \
 		-blocking 0 \
 		-encoding [get_path_encoding $path] \
@@ -420,6 +421,15 @@ proc read_diff {fd conflict_size cont_info} {
 					set is_conflict_diff 1
 					set line [string replace $line 0 1 {  }]
 					set tags d$op
+					set ::in_conflict_pre_image 0
+				} elseif {[regexp {^\+\+\|{$conflict_size}(?: |$)} $line]} {
+					set is_conflict_diff 1
+					set line [string replace $line 0 1 {  }]
+					set tags d|
+					set ::in_conflict_pre_image 1
+				} elseif ($::in_conflict_pre_image) {
+					set line [string replace $line 0 1 {--}]
+					set tags d_--
 				} else {
 					set tags d_++
 				}
-- 
tg: (fba7c96..) bw/git-gui/support-diff3 (depends on: master bw/git-gui/respect-conflict-marker-size)

Re: [PATCH 2/2] git-gui: support for diff3 conflict style

[Pat Thoyts]

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

>This adds highlight support for the diff3 conflict style.
>
>The common pre-image will be reversed to --, because it has been removed
>and either replaced with our or their side.
>
>Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>
>---
> git-gui/git-gui.sh   |    3 +++
> git-gui/lib/diff.tcl |   10 ++++++++++
> 2 files changed, 13 insertions(+), 0 deletions(-)
>
>diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
>index 38362fa..0134438 100755
>--- a/git-gui/git-gui.sh
>+++ b/git-gui/git-gui.sh
>@@ -3354,6 +3354,9 @@ $ui_diff tag conf d_s- \
> $ui_diff tag conf d< \
> 	-foreground orange \
> 	-font font_diffbold
>+$ui_diff tag conf d| \
>+	-foreground orange \
>+	-font font_diffbold
> $ui_diff tag conf d= \
> 	-foreground orange \
> 	-font font_diffbold
>diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl
>index d4e2ce3..ccd4c70 100644
>--- a/git-gui/lib/diff.tcl
>+++ b/git-gui/lib/diff.tcl
>@@ -339,6 +339,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
> 	}
> 
> 	set ::current_diff_inheader 1
>+	set ::in_conflict_pre_image 0
> 	fconfigure $fd \
> 		-blocking 0 \
> 		-encoding [get_path_encoding $path] \
>@@ -420,6 +421,15 @@ proc read_diff {fd conflict_size cont_info} {
> 					set is_conflict_diff 1
> 					set line [string replace $line 0 1 {  }]
> 					set tags d$op
>+					set ::in_conflict_pre_image 0
>+				} elseif {[regexp {^\+\+\|{$conflict_size}(?: |$)} $line]} {
>+					set is_conflict_diff 1
>+					set line [string replace $line 0 1 {  }]
>+					set tags d|
>+					set ::in_conflict_pre_image 1
>+				} elseif ($::in_conflict_pre_image) {
>+					set line [string replace $line 0 1 {--}]
>+					set tags d_--
> 				} else {
> 					set tags d_++
> 				}

This has the same issue as the last patch with variable substitution
into the regexp. Replaced the regexp expression with
   [regexp "^\\+\\+\\|{$conflict_size}(?: |\$)" $line]

I configured a test repository with
'merge.conflictstyle diff3' and could test this. Looks fine.
-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD

Re: [PATCH 2/2] git-gui: support for diff3 conflict style

[Bert Wesarg]

On Fri, Nov 19, 2010 at 12:41, Pat Thoyts
<patthoyts@users.sourceforge.net> wrote:
> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>
>>This adds highlight support for the diff3 conflict style.
>>
>>The common pre-image will be reversed to --, because it has been removed
>>and either replaced with our or their side.
>>
>>Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>>
>>---
>> git-gui/git-gui.sh   |    3 +++
>> git-gui/lib/diff.tcl |   10 ++++++++++
>> 2 files changed, 13 insertions(+), 0 deletions(-)
>>
>>diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
>>index 38362fa..0134438 100755
>>--- a/git-gui/git-gui.sh
>>+++ b/git-gui/git-gui.sh
>>@@ -3354,6 +3354,9 @@ $ui_diff tag conf d_s- \
>> $ui_diff tag conf d< \
>>       -foreground orange \
>>       -font font_diffbold
>>+$ui_diff tag conf d| \
>>+      -foreground orange \
>>+      -font font_diffbold
>> $ui_diff tag conf d= \
>>       -foreground orange \
>>       -font font_diffbold
>>diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl
>>index d4e2ce3..ccd4c70 100644
>>--- a/git-gui/lib/diff.tcl
>>+++ b/git-gui/lib/diff.tcl
>>@@ -339,6 +339,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
>>       }
>>
>>       set ::current_diff_inheader 1
>>+      set ::in_conflict_pre_image 0
>>       fconfigure $fd \
>>               -blocking 0 \
>>               -encoding [get_path_encoding $path] \
>>@@ -420,6 +421,15 @@ proc read_diff {fd conflict_size cont_info} {
>>                                       set is_conflict_diff 1
>>                                       set line [string replace $line 0 1 {  }]
>>                                       set tags d$op
>>+                                      set ::in_conflict_pre_image 0
>>+                              } elseif {[regexp {^\+\+\|{$conflict_size}(?: |$)} $line]} {
>>+                                      set is_conflict_diff 1
>>+                                      set line [string replace $line 0 1 {  }]
>>+                                      set tags d|
>>+                                      set ::in_conflict_pre_image 1
>>+                              } elseif ($::in_conflict_pre_image) {
>>+                                      set line [string replace $line 0 1 {--}]
>>+                                      set tags d_--
>>                               } else {
>>                                       set tags d_++
>>                               }
>
> This has the same issue as the last patch with variable substitution
> into the regexp. Replaced the regexp expression with
>   [regexp "^\\+\\+\\|{$conflict_size}(?: |\$)" $line]
>
> I configured a test repository with
> 'merge.conflictstyle diff3' and could test this. Looks fine.

Thanks Pat.

By looking over the patch (which by itself is now some month old) I
wondered whether git-gui has some policy to show the diff output as
verbatim as possible. Which I break with this patch, because I replace
the ++ line prefix in the pre-image with -- and in the |||| lines with
"  ". Shawn, does such policy exists?

Bert

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

Re: [PATCH 2/2] git-gui: support for diff3 conflict style

[Bert Wesarg]

On Fri, Nov 19, 2010 at 12:41, Pat Thoyts
<patthoyts@users.sourceforge.net> wrote:
> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>
>>This adds highlight support for the diff3 conflict style.
>>
>>The common pre-image will be reversed to --, because it has been removed
>>and either replaced with our or their side.
>>
>>Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>>
>>---
>> git-gui/git-gui.sh   |    3 +++
>> git-gui/lib/diff.tcl |   10 ++++++++++
>> 2 files changed, 13 insertions(+), 0 deletions(-)
>>
>>diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
>>index 38362fa..0134438 100755
>>--- a/git-gui/git-gui.sh
>>+++ b/git-gui/git-gui.sh
>>@@ -3354,6 +3354,9 @@ $ui_diff tag conf d_s- \
>> $ui_diff tag conf d< \
>>       -foreground orange \
>>       -font font_diffbold
>>+$ui_diff tag conf d| \
>>+      -foreground orange \
>>+      -font font_diffbold
>> $ui_diff tag conf d= \
>>       -foreground orange \
>>       -font font_diffbold
>>diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl
>>index d4e2ce3..ccd4c70 100644
>>--- a/git-gui/lib/diff.tcl
>>+++ b/git-gui/lib/diff.tcl
>>@@ -339,6 +339,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
>>       }
>>
>>       set ::current_diff_inheader 1
>>+      set ::in_conflict_pre_image 0
>>       fconfigure $fd \
>>               -blocking 0 \
>>               -encoding [get_path_encoding $path] \
>>@@ -420,6 +421,15 @@ proc read_diff {fd conflict_size cont_info} {
>>                                       set is_conflict_diff 1
>>                                       set line [string replace $line 0 1 {  }]
>>                                       set tags d$op
>>+                                      set ::in_conflict_pre_image 0
>>+                              } elseif {[regexp {^\+\+\|{$conflict_size}(?: |$)} $line]} {
>>+                                      set is_conflict_diff 1
>>+                                      set line [string replace $line 0 1 {  }]
>>+                                      set tags d|
>>+                                      set ::in_conflict_pre_image 1
>>+                              } elseif ($::in_conflict_pre_image) {
>>+                                      set line [string replace $line 0 1 {--}]
>>+                                      set tags d_--
>>                               } else {
>>                                       set tags d_++
>>                               }
>
> This has the same issue as the last patch with variable substitution
> into the regexp. Replaced the regexp expression with
>   [regexp "^\\+\\+\\|{$conflict_size}(?: |\$)" $line]
>
> I configured a test repository with
> 'merge.conflictstyle diff3' and could test this. Looks fine.

I miss this one in git-gui.git

Thanks,
Bert

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

[PATCH 2/2] git-gui: support for diff3 conflict style

[Bert Wesarg]

This adds highlight support for the diff3 conflict style.

The common pre-image will be reversed to --, because it has been removed
and either replaced with our or their side.

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---

Maybe it's not too late for the next release.

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

diff --git a/git-gui.sh b/git-gui.sh
index d5c1535..6adcda6 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3388,6 +3388,9 @@ $ui_diff tag conf d_s- \
 $ui_diff tag conf d< \
 	-foreground orange \
 	-font font_diffbold
+$ui_diff tag conf d| \
+	-foreground orange \
+	-font font_diffbold
 $ui_diff tag conf d= \
 	-foreground orange \
 	-font font_diffbold
diff --git a/lib/diff.tcl b/lib/diff.tcl
index 39e4d90..11f9e16 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -339,6 +339,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
 	}
 
 	set ::current_diff_inheader 1
+	set ::in_conflict_pre_image 0
 	fconfigure $fd \
 		-blocking 0 \
 		-encoding [get_path_encoding $path] \
@@ -439,10 +440,21 @@ proc read_diff {fd conflict_size cont_info} {
 			{++} {
 				set regexp [string map [list %conflict_size $conflict_size]\
 								{^\+\+([<>=]){%conflict_size}(?: |$)}]
+				set regexp_pre_image [string map [list %conflict_size $conflict_size]\
+								{^\+\+\|{%conflict_size}(?: |$)]
 				if {[regexp $regexp $line _g op]} {
 					set is_conflict_diff 1
 					set line [string replace $line 0 1 {  }]
 					set tags d$op
+					set ::in_conflict_pre_image 0
+				} elseif {[regexp $regexp_pre_image $line]} {
+					set is_conflict_diff 1
+					set line [string replace $line 0 1 {  }]
+					set tags d|
+					set ::in_conflict_pre_image 1
+				} elseif ($::in_conflict_pre_image) {
+					set line [string replace $line 0 1 {--}]
+					set tags d_--
 				} else {
 					set tags d_++
 				}
-- 
1.7.4.1.1319.ga04f7.dirty

[PATCH 1/2] git-gui: use existing interface to query a path's attribute

[Bert Wesarg]

Replace the hand-coded call to git check-attr with the already provided one.

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

diff --git a/lib/diff.tcl b/lib/diff.tcl
index 958a0fa..0fd4600 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -270,19 +270,6 @@ proc show_other_diff {path w m cont_info} {
 	}
 }
 
-proc get_conflict_marker_size {path} {
-	set size 7
-	catch {
-		set fd_rc [eval [list git_read check-attr "conflict-marker-size" -- $path]]
-		set ret [gets $fd_rc line]
-		close $fd_rc
-		if {$ret > 0} {
-			regexp {.*: conflict-marker-size: (\d+)$} $line line size
-		}
-	}
-	return $size
-}
-
 proc start_show_diff {cont_info {add_opts {}}} {
 	global file_states file_lists
 	global is_3way_diff is_submodule_diff diff_active repo_config
@@ -298,7 +285,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
 	set is_submodule_diff 0
 	set diff_active 1
 	set current_diff_header {}
-	set conflict_size [get_conflict_marker_size $path]
+	set conflict_size [gitattr $path conflict-marker-size 7]
 
 	set cmd [list]
 	if {$w eq $ui_index} {
-- 
2.21.0.789.ga095d9d866

[PATCH 2/2] git-gui: support for diff3 conflict style

[Bert Wesarg]

This adds highlight support for the diff3 conflict style.

The common pre-image will be reversed to --, because it has been removed
and either replaced with ours or theirs side.

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

diff --git a/git-gui.sh b/git-gui.sh
index fd476b6..6d80f82 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3581,6 +3581,9 @@ $ui_diff tag conf d_s- \
 $ui_diff tag conf d< \
 	-foreground orange \
 	-font font_diffbold
+$ui_diff tag conf d| \
+	-foreground orange \
+	-font font_diffbold
 $ui_diff tag conf d= \
 	-foreground orange \
 	-font font_diffbold
diff --git a/lib/diff.tcl b/lib/diff.tcl
index 0fd4600..6caf4e7 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -347,6 +347,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
 	}
 
 	set ::current_diff_inheader 1
+	set ::conflict_state {CONTEXT}
 	fconfigure $fd \
 		-blocking 0 \
 		-encoding [get_path_encoding $path] \
@@ -450,10 +451,28 @@ proc read_diff {fd conflict_size cont_info} {
 			{++} {
 				set regexp [string map [list %conflict_size $conflict_size]\
 								{^\+\+([<>=]){%conflict_size}(?: |$)}]
+				set regexp_pre_image [string map [list %conflict_size $conflict_size]\
+								{^\+\+\|{%conflict_size}(?: |$)}]
 				if {[regexp $regexp $line _g op]} {
 					set is_conflict_diff 1
 					set line [string replace $line 0 1 {  }]
+					set markup {}
 					set tags d$op
+					switch -exact -- $op {
+					< { set ::conflict_state {OURS} }
+					= { set ::conflict_state {THEIRS} }
+					> { set ::conflict_state {CONTEXT} }
+					}
+				} elseif {[regexp $regexp_pre_image $line]} {
+					set is_conflict_diff 1
+					set line [string replace $line 0 1 {  }]
+					set markup {}
+					set tags d|
+					set ::conflict_state {BASE}
+				} elseif {$::conflict_state eq {BASE}} {
+					set line [string replace $line 0 1 {--}]
+					set markup {}
+					set tags d_--
 				} else {
 					set tags d_++
 				}
@@ -505,6 +524,9 @@ proc read_diff {fd conflict_size cont_info} {
 			}
 		}
 		set mark [$ui_diff index "end - 1 line linestart"]
+		if {[llength $markup] > 0} {
+			set tags {}
+		}
 		$ui_diff insert end $line $tags
 		if {[string index $line end] eq "\r"} {
 			$ui_diff tag add d_cr {end - 2c}
-- 
2.21.0.789.ga095d9d866

Re: [PATCH 1/2] git-gui: use existing interface to query a path's attribute

[Pratyush Yadav]

Looks good. Thanks.

On 25/09/19 10:38PM, Bert Wesarg wrote:
> Replace the hand-coded call to git check-attr with the already provided one.
> 
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> ---
>  lib/diff.tcl | 15 +--------------
>  1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/lib/diff.tcl b/lib/diff.tcl
> index 958a0fa..0fd4600 100644
> --- a/lib/diff.tcl
> +++ b/lib/diff.tcl
> @@ -270,19 +270,6 @@ proc show_other_diff {path w m cont_info} {
>  	}
>  }
>  
> -proc get_conflict_marker_size {path} {
> -	set size 7
> -	catch {
> -		set fd_rc [eval [list git_read check-attr "conflict-marker-size" -- $path]]
> -		set ret [gets $fd_rc line]
> -		close $fd_rc
> -		if {$ret > 0} {
> -			regexp {.*: conflict-marker-size: (\d+)$} $line line size
> -		}
> -	}
> -	return $size
> -}
> -
>  proc start_show_diff {cont_info {add_opts {}}} {
>  	global file_states file_lists
>  	global is_3way_diff is_submodule_diff diff_active repo_config
> @@ -298,7 +285,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
>  	set is_submodule_diff 0
>  	set diff_active 1
>  	set current_diff_header {}
> -	set conflict_size [get_conflict_marker_size $path]
> +	set conflict_size [gitattr $path conflict-marker-size 7]
>  
>  	set cmd [list]
>  	if {$w eq $ui_index} {
> -- 
> 2.21.0.789.ga095d9d866
> 

-- 
Regards,
Pratyush Yadav

Re: [PATCH 2/2] git-gui: support for diff3 conflict style

[Pratyush Yadav]

Hi Philip, Bert,

Is there any way I can test this change? Philip, I ran the rebase you 
mention in the GitHub issue [0], and I get that '9c8cba6862abe5ac821' is 
an unknown revision.

Is there any quick way I can reproduce this (maybe on a sample repo)?

[0] https://github.com/git-for-windows/git/issues/2340

On 25/09/19 10:38PM, Bert Wesarg wrote:
> This adds highlight support for the diff3 conflict style.
> 
> The common pre-image will be reversed to --, because it has been removed
> and either replaced with ours or theirs side.
> 
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> ---
>  git-gui.sh   |  3 +++
>  lib/diff.tcl | 22 ++++++++++++++++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index fd476b6..6d80f82 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -3581,6 +3581,9 @@ $ui_diff tag conf d_s- \
>  $ui_diff tag conf d< \
>  	-foreground orange \
>  	-font font_diffbold
> +$ui_diff tag conf d| \
> +	-foreground orange \
> +	-font font_diffbold
>  $ui_diff tag conf d= \
>  	-foreground orange \
>  	-font font_diffbold
> diff --git a/lib/diff.tcl b/lib/diff.tcl
> index 0fd4600..6caf4e7 100644
> --- a/lib/diff.tcl
> +++ b/lib/diff.tcl
> @@ -347,6 +347,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
>  	}
>  
>  	set ::current_diff_inheader 1
> +	set ::conflict_state {CONTEXT}
>  	fconfigure $fd \
>  		-blocking 0 \
>  		-encoding [get_path_encoding $path] \
> @@ -450,10 +451,28 @@ proc read_diff {fd conflict_size cont_info} {
>  			{++} {
>  				set regexp [string map [list %conflict_size $conflict_size]\
>  								{^\+\+([<>=]){%conflict_size}(?: |$)}]
> +				set regexp_pre_image [string map [list %conflict_size $conflict_size]\
> +								{^\+\+\|{%conflict_size}(?: |$)}]
>  				if {[regexp $regexp $line _g op]} {
>  					set is_conflict_diff 1
>  					set line [string replace $line 0 1 {  }]
> +					set markup {}
>  					set tags d$op
> +					switch -exact -- $op {
> +					< { set ::conflict_state {OURS} }
> +					= { set ::conflict_state {THEIRS} }
> +					> { set ::conflict_state {CONTEXT} }
> +					}
> +				} elseif {[regexp $regexp_pre_image $line]} {
> +					set is_conflict_diff 1
> +					set line [string replace $line 0 1 {  }]
> +					set markup {}
> +					set tags d|
> +					set ::conflict_state {BASE}
> +				} elseif {$::conflict_state eq {BASE}} {
> +					set line [string replace $line 0 1 {--}]
> +					set markup {}
> +					set tags d_--

I'm afraid I don't follow what this hunk is supposed to do.

You set the variable ::conflict_state to the values like OURS, THEIRS, 
CONTEXT, but I don't see those values being used anywhere. A quick 
search for these words shows me that you only set them, never read them.

Is there some extra code that you have and I don't?

Also, this function is long and complicated already. A comment 
explaining what this code is doing would be nice, since it is not at all 
obvious at first read-through.

>  				} else {
>  					set tags d_++
>  				}
> @@ -505,6 +524,9 @@ proc read_diff {fd conflict_size cont_info} {
>  			}
>  		}
>  		set mark [$ui_diff index "end - 1 line linestart"]
> +		if {[llength $markup] > 0} {
> +			set tags {}
> +		}
>  		$ui_diff insert end $line $tags
>  		if {[string index $line end] eq "\r"} {
>  			$ui_diff tag add d_cr {end - 2c}
> -- 
> 2.21.0.789.ga095d9d866
> 

-- 
Regards,
Pratyush Yadav

Re: [PATCH 2/2] git-gui: support for diff3 conflict style

[Bert Wesarg]

[-- Attachment #1: Type: text/plain, Size: 7133 bytes --]

Pratyush,

On Sun, Sep 29, 2019 at 5:04 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
>
> Hi Philip, Bert,
>
> Is there any way I can test this change? Philip, I ran the rebase you
> mention in the GitHub issue [0], and I get that '9c8cba6862abe5ac821' is
> an unknown revision.
>
> Is there any quick way I can reproduce this (maybe on a sample repo)?

The easiest way to produce a merge conflict, is to change the same
line differently in two branches and try to merge them. I added a
fast-import file to demonstrate this for you.

$ git init foo
$ cd foo
$ git fast-import <../conflict-merge.fi
$ git reset --hard master
$ git merge branch

this gets you into the conflict, probably the usual style. Which looks
in liek this on the terminal:

@@@ -2,7 -2,7 +2,11 @@@ Lorem ipsum dolor sit amet, consectetu
  Sed feugiat nisl eget efficitur ultrices.
  Nunc cursus metus rutrum, mollis lorem vitae, hendrerit mi.
  Aenean vestibulum ante ac libero venenatis, non hendrerit orci pharetra.
++<<<<<<< HEAD
 +Proin bibendum purus ut est tristique, non pharetra dui consectetur.
++=======
+ Proin placerat leo malesuada lacinia lobortis.
++>>>>>>> branch
  Pellentesque aliquam libero et nisi scelerisque commodo.
  Quisque id velit sed magna molestie porttitor.
  Vivamus sed urna in risus molestie ultricies.

and this in git gui: https://kgab.selfhost.eu/s/gHHaQqowGp7mXEb

Git gui removes the '++' in front of the marker lines. It therefor
already 'changes' the 'diff'. Though git-apply cannot handle such
'diffs' anyway.

To get the diff3 style do:

$ git merge --abort
$ git -c merge.conflictStyle=diff3 merge branch

This is how it looks in the terminal now:

@@@ -2,7 -2,7 +2,13 @@@ Lorem ipsum dolor sit amet, consectetu
  Sed feugiat nisl eget efficitur ultrices.
  Nunc cursus metus rutrum, mollis lorem vitae, hendrerit mi.
  Aenean vestibulum ante ac libero venenatis, non hendrerit orci pharetra.
++<<<<<<< HEAD
 +Proin bibendum purus ut est tristique, non pharetra dui consectetur.
++||||||| merged common ancestors
++Proin in felis eu elit suscipit rhoncus vel ut metus.
++=======
+ Proin placerat leo malesuada lacinia lobortis.
++>>>>>>> branch
  Pellentesque aliquam libero et nisi scelerisque commodo.
  Quisque id velit sed magna molestie porttitor.
  Vivamus sed urna in risus molestie ultricies.

As you can see, there is not the usual 'I removed this, and added
that' experience, everything is 'added'. Thus I inverted the pre-image
to '--'. Here is how it looks in the gui:
https://kgab.selfhost.eu/s/5c8Tosra7WRfjwJ

> [0] https://github.com/git-for-windows/git/issues/2340
>
> On 25/09/19 10:38PM, Bert Wesarg wrote:
> > This adds highlight support for the diff3 conflict style.
> >
> > The common pre-image will be reversed to --, because it has been removed
> > and either replaced with ours or theirs side.
> >
> > Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> > ---
> >  git-gui.sh   |  3 +++
> >  lib/diff.tcl | 22 ++++++++++++++++++++++
> >  2 files changed, 25 insertions(+)
> >
> > diff --git a/git-gui.sh b/git-gui.sh
> > index fd476b6..6d80f82 100755
> > --- a/git-gui.sh
> > +++ b/git-gui.sh
> > @@ -3581,6 +3581,9 @@ $ui_diff tag conf d_s- \
> >  $ui_diff tag conf d< \
> >       -foreground orange \
> >       -font font_diffbold
> > +$ui_diff tag conf d| \
> > +     -foreground orange \
> > +     -font font_diffbold
> >  $ui_diff tag conf d= \
> >       -foreground orange \
> >       -font font_diffbold
> > diff --git a/lib/diff.tcl b/lib/diff.tcl
> > index 0fd4600..6caf4e7 100644
> > --- a/lib/diff.tcl
> > +++ b/lib/diff.tcl
> > @@ -347,6 +347,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
> >       }
> >
> >       set ::current_diff_inheader 1
> > +     set ::conflict_state {CONTEXT}
> >       fconfigure $fd \
> >               -blocking 0 \
> >               -encoding [get_path_encoding $path] \
> > @@ -450,10 +451,28 @@ proc read_diff {fd conflict_size cont_info} {
> >                       {++} {
> >                               set regexp [string map [list %conflict_size $conflict_size]\
> >                                                               {^\+\+([<>=]){%conflict_size}(?: |$)}]
> > +                             set regexp_pre_image [string map [list %conflict_size $conflict_size]\
> > +                                                             {^\+\+\|{%conflict_size}(?: |$)}]
> >                               if {[regexp $regexp $line _g op]} {
> >                                       set is_conflict_diff 1
> >                                       set line [string replace $line 0 1 {  }]
> > +                                     set markup {}
> >                                       set tags d$op
> > +                                     switch -exact -- $op {
> > +                                     < { set ::conflict_state {OURS} }
> > +                                     = { set ::conflict_state {THEIRS} }
> > +                                     > { set ::conflict_state {CONTEXT} }
> > +                                     }
> > +                             } elseif {[regexp $regexp_pre_image $line]} {
> > +                                     set is_conflict_diff 1
> > +                                     set line [string replace $line 0 1 {  }]
> > +                                     set markup {}
> > +                                     set tags d|
> > +                                     set ::conflict_state {BASE}
> > +                             } elseif {$::conflict_state eq {BASE}} {
> > +                                     set line [string replace $line 0 1 {--}]
> > +                                     set markup {}
> > +                                     set tags d_--
>
> I'm afraid I don't follow what this hunk is supposed to do.
>
> You set the variable ::conflict_state to the values like OURS, THEIRS,
> CONTEXT, but I don't see those values being used anywhere. A quick
> search for these words shows me that you only set them, never read them.

the last elseif depends on it.

I actually only need to detect the pre-image lines, which starts with
the ||| conflict-marker and ends with the === conflict-marker, instead
of all possible states.

>
> Is there some extra code that you have and I don't?
>
> Also, this function is long and complicated already. A comment
> explaining what this code is doing would be nice, since it is not at all
> obvious at first read-through.

Will re-send.

Bert

>
> >                               } else {
> >                                       set tags d_++
> >                               }
> > @@ -505,6 +524,9 @@ proc read_diff {fd conflict_size cont_info} {
> >                       }
> >               }
> >               set mark [$ui_diff index "end - 1 line linestart"]
> > +             if {[llength $markup] > 0} {
> > +                     set tags {}
> > +             }
> >               $ui_diff insert end $line $tags
> >               if {[string index $line end] eq "\r"} {
> >                       $ui_diff tag add d_cr {end - 2c}
> > --
> > 2.21.0.789.ga095d9d866
> >
>
> --
> Regards,
> Pratyush Yadav

[-- Attachment #2: conflict-merge.fi --]
[-- Type: application/octet-stream, Size: 2023 bytes --]

blob
mark :1
data 436
Lorem ipsum dolor sit amet, consectetur adipiscing elit.
Sed feugiat nisl eget efficitur ultrices.
Nunc cursus metus rutrum, mollis lorem vitae, hendrerit mi.
Aenean vestibulum ante ac libero venenatis, non hendrerit orci pharetra.
Proin in felis eu elit suscipit rhoncus vel ut metus.
Pellentesque aliquam libero et nisi scelerisque commodo.
Quisque id velit sed magna molestie porttitor.
Vivamus sed urna in risus molestie ultricies.

reset refs/heads/branch
commit refs/heads/branch
mark :2
author Bert Wesarg <bert.wesarg@googlemail.com> 1569822514 +0200
committer Bert Wesarg <bert.wesarg@googlemail.com> 1569822891 +0200
data 5
init
M 100644 :1 lorem

blob
mark :3
data 451
Lorem ipsum dolor sit amet, consectetur adipiscing elit.
Sed feugiat nisl eget efficitur ultrices.
Nunc cursus metus rutrum, mollis lorem vitae, hendrerit mi.
Aenean vestibulum ante ac libero venenatis, non hendrerit orci pharetra.
Proin bibendum purus ut est tristique, non pharetra dui consectetur.
Pellentesque aliquam libero et nisi scelerisque commodo.
Quisque id velit sed magna molestie porttitor.
Vivamus sed urna in risus molestie ultricies.

commit refs/heads/master
mark :4
author Bert Wesarg <bert.wesarg@googlemail.com> 1569822667 +0200
committer Bert Wesarg <bert.wesarg@googlemail.com> 1569823044 +0200
data 8
changed
from :2
M 100644 :3 lorem

blob
mark :5
data 429
Lorem ipsum dolor sit amet, consectetur adipiscing elit.
Sed feugiat nisl eget efficitur ultrices.
Nunc cursus metus rutrum, mollis lorem vitae, hendrerit mi.
Aenean vestibulum ante ac libero venenatis, non hendrerit orci pharetra.
Proin placerat leo malesuada lacinia lobortis.
Pellentesque aliquam libero et nisi scelerisque commodo.
Quisque id velit sed magna molestie porttitor.
Vivamus sed urna in risus molestie ultricies.

commit refs/heads/branch
mark :6
author Bert Wesarg <bert.wesarg@googlemail.com> 1569823077 +0200
committer Bert Wesarg <bert.wesarg@googlemail.com> 1569823077 +0200
data 9
conflict
from :2
M 100644 :5 lorem

[PATCH v2 1/2] git-gui: use existing interface to query a path's attribute

[Bert Wesarg]

Replace the hand-coded call to git check-attr with the already provided one.

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

diff --git a/lib/diff.tcl b/lib/diff.tcl
index 958a0fa..0fd4600 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -270,19 +270,6 @@ proc show_other_diff {path w m cont_info} {
 	}
 }
 
-proc get_conflict_marker_size {path} {
-	set size 7
-	catch {
-		set fd_rc [eval [list git_read check-attr "conflict-marker-size" -- $path]]
-		set ret [gets $fd_rc line]
-		close $fd_rc
-		if {$ret > 0} {
-			regexp {.*: conflict-marker-size: (\d+)$} $line line size
-		}
-	}
-	return $size
-}
-
 proc start_show_diff {cont_info {add_opts {}}} {
 	global file_states file_lists
 	global is_3way_diff is_submodule_diff diff_active repo_config
@@ -298,7 +285,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
 	set is_submodule_diff 0
 	set diff_active 1
 	set current_diff_header {}
-	set conflict_size [get_conflict_marker_size $path]
+	set conflict_size [gitattr $path conflict-marker-size 7]
 
 	set cmd [list]
 	if {$w eq $ui_index} {
-- 
2.23.0.11.g242cf7f110

[PATCH v2 2/2] git-gui: support for diff3 conflict style

[Bert Wesarg]

This adds highlight support for the diff3 conflict style.

The common pre-image will be reversed to --, because it has been removed
and replaced with ours or theirs side respectively.

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

diff --git a/git-gui.sh b/git-gui.sh
index fd476b6..6d80f82 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3581,6 +3581,9 @@ $ui_diff tag conf d_s- \
 $ui_diff tag conf d< \
 	-foreground orange \
 	-font font_diffbold
+$ui_diff tag conf d| \
+	-foreground orange \
+	-font font_diffbold
 $ui_diff tag conf d= \
 	-foreground orange \
 	-font font_diffbold
diff --git a/lib/diff.tcl b/lib/diff.tcl
index 0fd4600..6459cfb 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -347,6 +347,10 @@ proc start_show_diff {cont_info {add_opts {}}} {
 	}
 
 	set ::current_diff_inheader 1
+	# detect pre-image lines of the diff3 conflict-style, they are just '++'
+	# lines which is not bijective, thus we need to maintain a state across
+	# lines
+	set ::conflict_in_pre_image 0
 	fconfigure $fd \
 		-blocking 0 \
 		-encoding [get_path_encoding $path] \
@@ -449,11 +453,22 @@ proc read_diff {fd conflict_size cont_info} {
 			{--} {set tags d_--}
 			{++} {
 				set regexp [string map [list %conflict_size $conflict_size]\
-								{^\+\+([<>=]){%conflict_size}(?: |$)}]
+								{^\+\+([<>=|]){%conflict_size}(?: |$)}]
 				if {[regexp $regexp $line _g op]} {
 					set is_conflict_diff 1
 					set line [string replace $line 0 1 {  }]
 					set tags d$op
+					# the ||| conflict-marker marks the start of the pre-image,
+					# all those lines are also prefixed with '++', thus we need
+					# to maintain this state
+					set ::conflict_in_pre_image [expr {$op eq {|}}
+				} elseif {$::conflict_in_pre_image} {
+					# this is a pre-image line, it is the one which both sides
+					# are based on. As it has also the '++' line start, it is
+					# normally shown as 'added', invert this to '--' to make
+					# it a 'removed' line
+					set line [string replace $line 0 1 {--}]
+					set tags d_--
 				} else {
 					set tags d_++
 				}
-- 
2.23.0.11.g242cf7f110

[PATCH v3 1/2] git-gui: use existing interface to query a path's attribute

[Bert Wesarg]

Replace the hand-coded call to git check-attr with the already provided one.

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

diff --git a/lib/diff.tcl b/lib/diff.tcl
index 958a0fa..0fd4600 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -270,19 +270,6 @@ proc show_other_diff {path w m cont_info} {
 	}
 }
 
-proc get_conflict_marker_size {path} {
-	set size 7
-	catch {
-		set fd_rc [eval [list git_read check-attr "conflict-marker-size" -- $path]]
-		set ret [gets $fd_rc line]
-		close $fd_rc
-		if {$ret > 0} {
-			regexp {.*: conflict-marker-size: (\d+)$} $line line size
-		}
-	}
-	return $size
-}
-
 proc start_show_diff {cont_info {add_opts {}}} {
 	global file_states file_lists
 	global is_3way_diff is_submodule_diff diff_active repo_config
@@ -298,7 +285,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
 	set is_submodule_diff 0
 	set diff_active 1
 	set current_diff_header {}
-	set conflict_size [get_conflict_marker_size $path]
+	set conflict_size [gitattr $path conflict-marker-size 7]
 
 	set cmd [list]
 	if {$w eq $ui_index} {
-- 
2.23.0.11.g242cf7f110

Re: [PATCH v3 1/2] git-gui: use existing interface to query a path's attribute

[Pratyush Yadav]

Hi,

I don't see any difference between v3 and v2 of this patch. What changed 
in this version?

On 30/09/19 09:54PM, Bert Wesarg wrote:
> Replace the hand-coded call to git check-attr with the already provided one.
> 
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> ---
>  lib/diff.tcl | 15 +--------------
>  1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/lib/diff.tcl b/lib/diff.tcl
> index 958a0fa..0fd4600 100644
> --- a/lib/diff.tcl
> +++ b/lib/diff.tcl
> @@ -270,19 +270,6 @@ proc show_other_diff {path w m cont_info} {
>  	}
>  }
>  
> -proc get_conflict_marker_size {path} {
> -	set size 7
> -	catch {
> -		set fd_rc [eval [list git_read check-attr "conflict-marker-size" -- $path]]
> -		set ret [gets $fd_rc line]
> -		close $fd_rc
> -		if {$ret > 0} {
> -			regexp {.*: conflict-marker-size: (\d+)$} $line line size
> -		}
> -	}
> -	return $size
> -}
> -
>  proc start_show_diff {cont_info {add_opts {}}} {
>  	global file_states file_lists
>  	global is_3way_diff is_submodule_diff diff_active repo_config
> @@ -298,7 +285,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
>  	set is_submodule_diff 0
>  	set diff_active 1
>  	set current_diff_header {}
> -	set conflict_size [get_conflict_marker_size $path]
> +	set conflict_size [gitattr $path conflict-marker-size 7]
>  
>  	set cmd [list]
>  	if {$w eq $ui_index} {
> -- 
> 2.23.0.11.g242cf7f110
> 

-- 
Regards,
Pratyush Yadav

Re: [PATCH v3 1/2] git-gui: use existing interface to query a path's attribute

[Bert Wesarg]

On Tue, Oct 1, 2019 at 4:24 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
>
> Hi,
>
> I don't see any difference between v3 and v2 of this patch. What changed
> in this version?

nothing, but 2/2 changed.

Bert

>
> On 30/09/19 09:54PM, Bert Wesarg wrote:
> > Replace the hand-coded call to git check-attr with the already provided one.
> >
> > Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> > ---
> >  lib/diff.tcl | 15 +--------------
> >  1 file changed, 1 insertion(+), 14 deletions(-)
> >
> > diff --git a/lib/diff.tcl b/lib/diff.tcl
> > index 958a0fa..0fd4600 100644
> > --- a/lib/diff.tcl
> > +++ b/lib/diff.tcl
> > @@ -270,19 +270,6 @@ proc show_other_diff {path w m cont_info} {
> >       }
> >  }
> >
> > -proc get_conflict_marker_size {path} {
> > -     set size 7
> > -     catch {
> > -             set fd_rc [eval [list git_read check-attr "conflict-marker-size" -- $path]]
> > -             set ret [gets $fd_rc line]
> > -             close $fd_rc
> > -             if {$ret > 0} {
> > -                     regexp {.*: conflict-marker-size: (\d+)$} $line line size
> > -             }
> > -     }
> > -     return $size
> > -}
> > -
> >  proc start_show_diff {cont_info {add_opts {}}} {
> >       global file_states file_lists
> >       global is_3way_diff is_submodule_diff diff_active repo_config
> > @@ -298,7 +285,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
> >       set is_submodule_diff 0
> >       set diff_active 1
> >       set current_diff_header {}
> > -     set conflict_size [get_conflict_marker_size $path]
> > +     set conflict_size [gitattr $path conflict-marker-size 7]
> >
> >       set cmd [list]
> >       if {$w eq $ui_index} {
> > --
> > 2.23.0.11.g242cf7f110
> >
>
> --
> Regards,
> Pratyush Yadav

Re: [PATCH v3 1/2] git-gui: use existing interface to query a path's attribute

[Pratyush Yadav]

On 01/10/19 05:22PM, Bert Wesarg wrote:
> On Tue, Oct 1, 2019 at 4:24 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
> >
> > Hi,
> >
> > I don't see any difference between v3 and v2 of this patch. What changed
> > in this version?
> 
> nothing, but 2/2 changed.

I don't see a v3 of 2/2 in my inbox. A search on public-inbox yields 
nothing either. Can you please check if the patch was sent properly? Or 
if you _can_ find it on public-inbox.org, a link to that would do just 
fine.

I _do_ have v2 of both patches, but the v3 of the second patch is the 
one missing.

-- 
Regards,
Pratyush Yadav

Re: [PATCH v3 1/2] git-gui: use existing interface to query a path's attribute

[Bert Wesarg]

Pratyush,

On Tue, Oct 1, 2019 at 7:31 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
>
> On 01/10/19 05:22PM, Bert Wesarg wrote:
> > On Tue, Oct 1, 2019 at 4:24 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
> > >
> > > Hi,
> > >
> > > I don't see any difference between v3 and v2 of this patch. What changed
> > > in this version?
> >
> > nothing, but 2/2 changed.
>
> I don't see a v3 of 2/2 in my inbox. A search on public-inbox yields
> nothing either. Can you please check if the patch was sent properly? Or
> if you _can_ find it on public-inbox.org, a link to that would do just
> fine.
>
> I _do_ have v2 of both patches, but the v3 of the second patch is the
> one missing.

I noticed this already, while in contact with Johannes on GitHub and I
found my error. While pasting messages ids for References into the v3
patch, I missed to renamed Message-Id to References for the v2 patch,
thus the v3 has the same Message-Id than the v2 patch. Will resend
now.

Bert

>
> --
> Regards,
> Pratyush Yadav

[PATCH v3 2/2] git-gui: support for diff3 conflict style

[Bert Wesarg]

This adds highlight support for the diff3 conflict style.

The common pre-image will be reversed to --, because it has been removed
and replaced with ours or theirs side respectively.

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

--- 

v3: Fixed a syntax error

diff --git a/git-gui.sh b/git-gui.sh
index fd476b6..6d80f82 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3581,6 +3581,9 @@ $ui_diff tag conf d_s- \
 $ui_diff tag conf d< \
 	-foreground orange \
 	-font font_diffbold
+$ui_diff tag conf d| \
+	-foreground orange \
+	-font font_diffbold
 $ui_diff tag conf d= \
 	-foreground orange \
 	-font font_diffbold
diff --git a/lib/diff.tcl b/lib/diff.tcl
index 0fd4600..dacdda2 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -347,6 +347,10 @@ proc start_show_diff {cont_info {add_opts {}}} {
 	}
 
 	set ::current_diff_inheader 1
+	# detect pre-image lines of the diff3 conflict-style, they are just '++'
+	# lines which is not bijective, thus we need to maintain a state across
+	# lines
+	set ::conflict_in_pre_image 0
 	fconfigure $fd \
 		-blocking 0 \
 		-encoding [get_path_encoding $path] \
@@ -449,11 +453,22 @@ proc read_diff {fd conflict_size cont_info} {
 			{--} {set tags d_--}
 			{++} {
 				set regexp [string map [list %conflict_size $conflict_size]\
-								{^\+\+([<>=]){%conflict_size}(?: |$)}]
+								{^\+\+([<>=|]){%conflict_size}(?: |$)}]
 				if {[regexp $regexp $line _g op]} {
 					set is_conflict_diff 1
 					set line [string replace $line 0 1 {  }]
 					set tags d$op
+					# the ||| conflict-marker marks the start of the pre-image,
+					# all those lines are also prefixed with '++', thus we need
+					# to maintain this state
+					set ::conflict_in_pre_image [expr {$op eq {|}}]
+				} elseif {$::conflict_in_pre_image} {
+					# this is a pre-image line, it is the one which both sides
+					# are based on. As it has also the '++' line start, it is
+					# normally shown as 'added', invert this to '--' to make
+					# it a 'removed' line
+					set line [string replace $line 0 1 {--}]
+					set tags d_--
 				} else {
 					set tags d_++
 				}
-- 
2.23.0.11.g242cf7f110

Re: [PATCH v3 2/2] git-gui: support for diff3 conflict style

[Pratyush Yadav]

Hi,

I made some fixes to the punctuation and capitalization in the comments 
you added. You can take a look at [0].

Just to be sure, I get the following text in git-gui when I ran your 
example:

  <<<<<<< HEAD
 +Proin bibendum purus ut est tristique, non pharetra dui consectetur.
  ||||||| merged common ancestors
--Proin in felis eu elit suscipit rhoncus vel ut metus.
  =======
+ Proin placerat leo malesuada lacinia lobortis.
  >>>>>>> branch

I noticed that the line after '<<<<<<< HEAD' starts with ' +' and the 
line after '=======' starts with '+ '.

So on the "HEAD" version, the space is before the '+', and on the 
"branch" version, the space is after the '+'. This is the intended 
behaviour, right?

It is not strictly related to your patch because it happens without 
diff3 conflict style enabled as well, but I just want to make sure this 
is not a bug.

The patch looks good. Will queue. Thanks.

[0] https://github.com/prati0100/git-gui/commit/d6e413c7cff6d09a0089d7a5de115ad438b42e81

On 02/10/19 09:36AM, Bert Wesarg wrote:
> This adds highlight support for the diff3 conflict style.
> 
> The common pre-image will be reversed to --, because it has been removed
> and replaced with ours or theirs side respectively.
> 
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> ---
>  git-gui.sh   |  3 +++
>  lib/diff.tcl | 17 ++++++++++++++++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> --- 
> 
> v3: Fixed a syntax error
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index fd476b6..6d80f82 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -3581,6 +3581,9 @@ $ui_diff tag conf d_s- \
>  $ui_diff tag conf d< \
>  	-foreground orange \
>  	-font font_diffbold
> +$ui_diff tag conf d| \
> +	-foreground orange \
> +	-font font_diffbold
>  $ui_diff tag conf d= \
>  	-foreground orange \
>  	-font font_diffbold
> diff --git a/lib/diff.tcl b/lib/diff.tcl
> index 0fd4600..dacdda2 100644
> --- a/lib/diff.tcl
> +++ b/lib/diff.tcl
> @@ -347,6 +347,10 @@ proc start_show_diff {cont_info {add_opts {}}} {
>  	}
>  
>  	set ::current_diff_inheader 1
> +	# detect pre-image lines of the diff3 conflict-style, they are just '++'
> +	# lines which is not bijective, thus we need to maintain a state across
> +	# lines
> +	set ::conflict_in_pre_image 0
>  	fconfigure $fd \
>  		-blocking 0 \
>  		-encoding [get_path_encoding $path] \
> @@ -449,11 +453,22 @@ proc read_diff {fd conflict_size cont_info} {
>  			{--} {set tags d_--}
>  			{++} {
>  				set regexp [string map [list %conflict_size $conflict_size]\
> -								{^\+\+([<>=]){%conflict_size}(?: |$)}]
> +								{^\+\+([<>=|]){%conflict_size}(?: |$)}]
>  				if {[regexp $regexp $line _g op]} {
>  					set is_conflict_diff 1
>  					set line [string replace $line 0 1 {  }]
>  					set tags d$op
> +					# the ||| conflict-marker marks the start of the pre-image,
> +					# all those lines are also prefixed with '++', thus we need
> +					# to maintain this state
> +					set ::conflict_in_pre_image [expr {$op eq {|}}]
> +				} elseif {$::conflict_in_pre_image} {
> +					# this is a pre-image line, it is the one which both sides
> +					# are based on. As it has also the '++' line start, it is
> +					# normally shown as 'added', invert this to '--' to make
> +					# it a 'removed' line
> +					set line [string replace $line 0 1 {--}]
> +					set tags d_--
>  				} else {
>  					set tags d_++
>  				}

-- 
Regards,
Pratyush Yadav

Re: [PATCH 2/2] git-gui: support for diff3 conflict style

[Philip Oakley]

On 30/09/2019 13:17, Bert Wesarg wrote:
> Pratyush,
>
> On Sun, Sep 29, 2019 at 5:04 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
>> Hi Philip, Bert,
>>
>> Is there any way I can test this change? Philip, I ran the rebase you
>> mention in the GitHub issue [0], and I get that '9c8cba6862abe5ac821' is
>> an unknown revision.
>>
>> Is there any quick way I can reproduce this (maybe on a sample repo)?
> The easiest way to produce a merge conflict, is to change the same
> line differently in two branches and try to merge them. I added a
> fast-import file to demonstrate this for you.
>
> $ git init foo
> $ cd foo
> $ git fast-import <../conflict-merge.fi
> $ git reset --hard master
> $ git merge branch
>
> this gets you into the conflict, probably the usual style. Which looks
> in liek this on the terminal:
>
> @@@ -2,7 -2,7 +2,11 @@@ Lorem ipsum dolor sit amet, consectetu
>    Sed feugiat nisl eget efficitur ultrices.
>    Nunc cursus metus rutrum, mollis lorem vitae, hendrerit mi.
>    Aenean vestibulum ante ac libero venenatis, non hendrerit orci pharetra.
> ++<<<<<<< HEAD
>   +Proin bibendum purus ut est tristique, non pharetra dui consectetur.
> ++=======
> + Proin placerat leo malesuada lacinia lobortis.
> ++>>>>>>> branch
>    Pellentesque aliquam libero et nisi scelerisque commodo.
>    Quisque id velit sed magna molestie porttitor.
>    Vivamus sed urna in risus molestie ultricies.
>
> and this in git gui: https://kgab.selfhost.eu/s/gHHaQqowGp7mXEb

The snapshot of the Gui looks just the thing! (I've been away).

I'm sure this would solve my immediate issue.

My only remaining bikeshed question it prompted was to check which parts 
would be committed as part of committing the whole "hunk". But haven't 
had time to look at all!

>
> Git gui removes the '++' in front of the marker lines. It therefor
> already 'changes' the 'diff'. Though git-apply cannot handle such
> 'diffs' anyway.
>
> To get the diff3 style do:
>
> $ git merge --abort
> $ git -c merge.conflictStyle=diff3 merge branch
>
> This is how it looks in the terminal now:
>
> @@@ -2,7 -2,7 +2,13 @@@ Lorem ipsum dolor sit amet, consectetu
>    Sed feugiat nisl eget efficitur ultrices.
>    Nunc cursus metus rutrum, mollis lorem vitae, hendrerit mi.
>    Aenean vestibulum ante ac libero venenatis, non hendrerit orci pharetra.
> ++<<<<<<< HEAD
>   +Proin bibendum purus ut est tristique, non pharetra dui consectetur.
> ++||||||| merged common ancestors
> ++Proin in felis eu elit suscipit rhoncus vel ut metus.
> ++=======
> + Proin placerat leo malesuada lacinia lobortis.
> ++>>>>>>> branch
>    Pellentesque aliquam libero et nisi scelerisque commodo.
>    Quisque id velit sed magna molestie porttitor.
>    Vivamus sed urna in risus molestie ultricies.
>
> As you can see, there is not the usual 'I removed this, and added
> that' experience, everything is 'added'. Thus I inverted the pre-image
> to '--'. Here is how it looks in the gui:
> https://kgab.selfhost.eu/s/5c8Tosra7WRfjwJ
>
>> [0] https://github.com/git-for-windows/git/issues/2340
>>
>> On 25/09/19 10:38PM, Bert Wesarg wrote:
>>> This adds highlight support for the diff3 conflict style.
>>>
>>> The common pre-image will be reversed to --, because it has been removed
>>> and either replaced with ours or theirs side.
>>>
>>> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>>> ---
>>>   git-gui.sh   |  3 +++
>>>   lib/diff.tcl | 22 ++++++++++++++++++++++
>>>   2 files changed, 25 insertions(+)
>>>
>>> diff --git a/git-gui.sh b/git-gui.sh
>>> index fd476b6..6d80f82 100755
>>> --- a/git-gui.sh
>>> +++ b/git-gui.sh
>>> @@ -3581,6 +3581,9 @@ $ui_diff tag conf d_s- \
>>>   $ui_diff tag conf d< \
>>>        -foreground orange \
>>>        -font font_diffbold
>>> +$ui_diff tag conf d| \
>>> +     -foreground orange \
>>> +     -font font_diffbold
>>>   $ui_diff tag conf d= \
>>>        -foreground orange \
>>>        -font font_diffbold
>>> diff --git a/lib/diff.tcl b/lib/diff.tcl
>>> index 0fd4600..6caf4e7 100644
>>> --- a/lib/diff.tcl
>>> +++ b/lib/diff.tcl
>>> @@ -347,6 +347,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
>>>        }
>>>
>>>        set ::current_diff_inheader 1
>>> +     set ::conflict_state {CONTEXT}
>>>        fconfigure $fd \
>>>                -blocking 0 \
>>>                -encoding [get_path_encoding $path] \
>>> @@ -450,10 +451,28 @@ proc read_diff {fd conflict_size cont_info} {
>>>                        {++} {
>>>                                set regexp [string map [list %conflict_size $conflict_size]\
>>>                                                                {^\+\+([<>=]){%conflict_size}(?: |$)}]
>>> +                             set regexp_pre_image [string map [list %conflict_size $conflict_size]\
>>> +                                                             {^\+\+\|{%conflict_size}(?: |$)}]
>>>                                if {[regexp $regexp $line _g op]} {
>>>                                        set is_conflict_diff 1
>>>                                        set line [string replace $line 0 1 {  }]
>>> +                                     set markup {}
>>>                                        set tags d$op
>>> +                                     switch -exact -- $op {
>>> +                                     < { set ::conflict_state {OURS} }
>>> +                                     = { set ::conflict_state {THEIRS} }
>>> +                                     > { set ::conflict_state {CONTEXT} }
>>> +                                     }
>>> +                             } elseif {[regexp $regexp_pre_image $line]} {
>>> +                                     set is_conflict_diff 1
>>> +                                     set line [string replace $line 0 1 {  }]
>>> +                                     set markup {}
>>> +                                     set tags d|
>>> +                                     set ::conflict_state {BASE}
>>> +                             } elseif {$::conflict_state eq {BASE}} {
>>> +                                     set line [string replace $line 0 1 {--}]
>>> +                                     set markup {}
>>> +                                     set tags d_--
>> I'm afraid I don't follow what this hunk is supposed to do.
>>
>> You set the variable ::conflict_state to the values like OURS, THEIRS,
>> CONTEXT, but I don't see those values being used anywhere. A quick
>> search for these words shows me that you only set them, never read them.
> the last elseif depends on it.
>
> I actually only need to detect the pre-image lines, which starts with
> the ||| conflict-marker and ends with the === conflict-marker, instead
> of all possible states.
>
>> Is there some extra code that you have and I don't?
>>
>> Also, this function is long and complicated already. A comment
>> explaining what this code is doing would be nice, since it is not at all
>> obvious at first read-through.
> Will re-send.
>
> Bert
>
>>>                                } else {
>>>                                        set tags d_++
>>>                                }
>>> @@ -505,6 +524,9 @@ proc read_diff {fd conflict_size cont_info} {
>>>                        }
>>>                }
>>>                set mark [$ui_diff index "end - 1 line linestart"]
>>> +             if {[llength $markup] > 0} {
>>> +                     set tags {}
>>> +             }
>>>                $ui_diff insert end $line $tags
>>>                if {[string index $line end] eq "\r"} {
>>>                        $ui_diff tag add d_cr {end - 2c}
>>> --
>>> 2.21.0.789.ga095d9d866
>>>
>> --
>> Regards,
>> Pratyush Yadav

Re: [PATCH 2/2] git-gui: support for diff3 conflict style

[Pratyush Yadav]

On 03/10/19 09:02PM, Philip Oakley wrote:
> On 30/09/2019 13:17, Bert Wesarg wrote:
> > Pratyush,
> > 
> > On Sun, Sep 29, 2019 at 5:04 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
> > > Hi Philip, Bert,
> > > 
> > > Is there any way I can test this change? Philip, I ran the rebase you
> > > mention in the GitHub issue [0], and I get that '9c8cba6862abe5ac821' is
> > > an unknown revision.
> > > 
> > > Is there any quick way I can reproduce this (maybe on a sample repo)?
> > The easiest way to produce a merge conflict, is to change the same
> > line differently in two branches and try to merge them. I added a
> > fast-import file to demonstrate this for you.
> > 
> > $ git init foo
> > $ cd foo
> > $ git fast-import <../conflict-merge.fi
> > $ git reset --hard master
> > $ git merge branch
> > 
> > this gets you into the conflict, probably the usual style. Which looks
> > in liek this on the terminal:
> > 
> > @@@ -2,7 -2,7 +2,11 @@@ Lorem ipsum dolor sit amet, consectetu
> >    Sed feugiat nisl eget efficitur ultrices.
> >    Nunc cursus metus rutrum, mollis lorem vitae, hendrerit mi.
> >    Aenean vestibulum ante ac libero venenatis, non hendrerit orci pharetra.
> > ++<<<<<<< HEAD
> >   +Proin bibendum purus ut est tristique, non pharetra dui consectetur.
> > ++=======
> > + Proin placerat leo malesuada lacinia lobortis.
> > ++>>>>>>> branch
> >    Pellentesque aliquam libero et nisi scelerisque commodo.
> >    Quisque id velit sed magna molestie porttitor.
> >    Vivamus sed urna in risus molestie ultricies.
> > 
> > and this in git gui: https://kgab.selfhost.eu/s/gHHaQqowGp7mXEb
> 
> The snapshot of the Gui looks just the thing! (I've been away).
> 
> I'm sure this would solve my immediate issue.
> 
> My only remaining bikeshed question it prompted was to check which parts
> would be committed as part of committing the whole "hunk". But haven't had
> time to look at all!

I'm not sure what you mean by "committing the whole hunk". In a merge 
conflict state, you don't get the usual "Stage hunk" and "Stage lines" 
options, but instead get 3 options:

  Use Remote Version
  Use Local Version
  Revert To Base

You can use these to choose how you want to resolve the conflict.

These 3 options seem to work fine on my quick testing.

-- 
Regards,
Pratyush Yadav

Re: [PATCH 2/2] git-gui: support for diff3 conflict style

[Philip Oakley]

On 03/10/2019 21:54, Pratyush Yadav wrote:
>> My only remaining bikeshed question it prompted was to check which parts
>> would be committed as part of committing the whole "hunk". But haven't had
>> time to look at all!
> I'm not sure what you mean by "committing the whole hunk". In a merge
> conflict state, you don't get the usual "Stage hunk" and "Stage lines"
> options, but instead get 3 options:
>
>    Use Remote Version
>    Use Local Version
>    Revert To Base
>
> You can use these to choose how you want to resolve the conflict.
>
> These 3 options seem to work fine on my quick testing.
That looks like just the answer I was hoping for!

Thanks.

-- 
Philip

Re: [PATCH 2/2] git-gui: support for diff3 conflict style

[Philip Oakley]

On 04/10/2019 16:22, Bert Wesarg wrote:
> On Thu, Oct 3, 2019 at 11:40 PM Philip Oakley <philipoakley@iee.email> wrote:
>> On 03/10/2019 21:54, Pratyush Yadav wrote:
>>>> My only remaining bikeshed question it prompted was to check which parts
>>>> would be committed as part of committing the whole "hunk". But haven't had
>>>> time to look at all!
>>> I'm not sure what you mean by "committing the whole hunk". In a merge
>>> conflict state, you don't get the usual "Stage hunk" and "Stage lines"
>>> options, but instead get 3 options:
>>>
>>>     Use Remote Version
>>>     Use Local Version
>>>     Revert To Base
>>>
>>> You can use these to choose how you want to resolve the conflict.
>>>
>>> These 3 options seem to work fine on my quick testing.
>> That looks like just the answer I was hoping for!
> note, that these three options apply to the whole file, not for
> individual conflicts.
>
>
Hmm, maybe not as great as hoped, but it's a start.
Philip