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

[Pratyush Yadav <me@yadavpratyush.com>]

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