[PATCH (GIT-GUI) 0/8] Add mergetool functionality to git-gui.

[PATCH (GIT-GUI) 0/8] Add mergetool functionality to git-gui.

[Alexander Gavrilov]

The original git-mergetool program is intended to be used from
the command line, and thus does not integrate well with GUI
usage. For instance, it presents the user with its own interactive
interface, based on text-mode prompts.

This series implements mergetool functionality within git-gui,
replacing the default 'Stage Hunk/Line' context menu items
for unresolved files with conflict resolution commands.

It also enhances auto-selection behavior to minimize the
necessary number of clicks on the items of the file lists,
and adds special handling of diffs for conflicts involving
deletion or symlinks.


Custom merge tools are not supported as I'm not sure of
the best way to evaluate command line patterns that are
intended for the shell from Tcl.



PATCHES:

	git-gui: Don't allow staging files with conflicts.
	---
	 git-gui.sh |    8 ++++++++
	 1 files changed, 8 insertions(+), 0 deletions(-)

	git-gui: Support resolving conflicts via the diff context menu.
	---
	 git-gui.sh        |  152 ++++++++++++++++++++++++++++++++---------------------
	 lib/mergetool.tcl |   98 ++++++++++++++++++++++++++++++++++
	 2 files changed, 190 insertions(+), 60 deletions(-)
	 create mode 100644 lib/mergetool.tcl

	git-gui: Support calling merge tools.
	---
	 git-gui.sh        |    7 ++
	 lib/mergetool.tcl |  252 +++++++++++++++++++++++++++++++++++++++++++++++++++++
	 lib/option.tcl    |    1 +
	 3 files changed, 260 insertions(+), 0 deletions(-)

	git-gui: Support more merge tools.
	---
	 lib/mergetool.tcl |   27 +++++++++++++++++++++++++++
	 1 files changed, 27 insertions(+), 0 deletions(-)

	git-gui: Support conflict states _U & UT.
	---
	 git-gui.sh     |    6 ++++--
	 lib/commit.tcl |    1 +
	 lib/diff.tcl   |    2 +-
	 lib/index.tcl  |    1 +
	 4 files changed, 7 insertions(+), 3 deletions(-)

	git-gui: Reimplement and enhance auto-selection of diffs.
	---
	 git-gui.sh        |  122 +++++++++++++++++++++++++++++++++++++++++------------
	 lib/diff.tcl      |   18 +++++---
	 lib/mergetool.tcl |    8 +---
	 3 files changed, 109 insertions(+), 39 deletions(-)

	git-gui: Make F5 reselect a diff, if an untracked file is selected.
	---
	 git-gui.sh |   44 +++++++++++++++++++++++++++++++++++---------
	 1 files changed, 35 insertions(+), 9 deletions(-)

	git-gui: Show special diffs for complex conflict cases.
	---
	 lib/diff.tcl |   94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
	 1 files changed, 89 insertions(+), 5 deletions(-)

[PATCH (GIT-GUI) 1/8] git-gui: Don't allow staging files with conflicts.

[Alexander Gavrilov]

Prevent staging files with conflict markers by clicking
on the icon in the 'Unstaged Changes' list. Instead, pretend
that the user clicked the name, and show the diff.

Originally it made some sense to allow staging conflicting
files, because git-gui did not provide any tools to resolve
them from within the GUI. But now that I'm adding mergetool
capabilities, it is more likely to cause accidental and
non-undoable errors.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 git-gui.sh |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index ec08b5a..9df4f8c 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1838,6 +1838,14 @@ proc toggle_or_diff {w x y} {
 	$ui_index tag remove in_sel 0.0 end
 	$ui_workdir tag remove in_sel 0.0 end
 
+	# Do not stage files with conflicts
+	if {[info exists file_states($path)]} {
+		set state [lindex $file_states($path) 0]
+		if {[string index $state 0] eq {U}} {
+			set col 1
+		}
+	}
+
 	if {$col == 0 && $y > 1} {
 		set i [expr {$lno-1}]
 		set ll [expr {[llength $file_lists($w)]-1}]
-- 
1.6.0.20.g6148bc

[PATCH (GIT-GUI) 2/8] git-gui: Support resolving conflicts via the diff context menu.

[Alexander Gavrilov]

If the file has merge conflicts, show a special version of the
diff context menu, which includes conflict resolution commands
instead of Stage Hunk/Line. This patch only supports resolving
by discarding all sides except one.

Discarding is the only way to resolve conflicts involving symlinks
and/or deletion, excluding manual editing.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 git-gui.sh        |  152 ++++++++++++++++++++++++++++++++---------------------
 lib/mergetool.tcl |   98 ++++++++++++++++++++++++++++++++++
 2 files changed, 190 insertions(+), 60 deletions(-)
 create mode 100644 lib/mergetool.tcl

diff --git a/git-gui.sh b/git-gui.sh
index 9df4f8c..90d597e 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2721,6 +2721,51 @@ $ui_diff tag raise sel
 
 # -- Diff Body Context Menu
 #
+
+proc create_common_diff_popup {ctxm} {
+	$ctxm add command \
+		-label [mc "Show Less Context"] \
+		-command show_less_context
+	lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
+	$ctxm add command \
+		-label [mc "Show More Context"] \
+		-command show_more_context
+	lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
+	$ctxm add separator
+	$ctxm add command \
+		-label [mc Refresh] \
+		-command reshow_diff
+	lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
+	$ctxm add command \
+		-label [mc Copy] \
+		-command {tk_textCopy $ui_diff}
+	lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
+	$ctxm add command \
+		-label [mc "Select All"] \
+		-command {focus $ui_diff;$ui_diff tag add sel 0.0 end}
+	lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
+	$ctxm add command \
+		-label [mc "Copy All"] \
+		-command {
+			$ui_diff tag add sel 0.0 end
+			tk_textCopy $ui_diff
+			$ui_diff tag remove sel 0.0 end
+		}
+	lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
+	$ctxm add separator
+	$ctxm add command \
+		-label [mc "Decrease Font Size"] \
+		-command {incr_font_size font_diff -1}
+	lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
+	$ctxm add command \
+		-label [mc "Increase Font Size"] \
+		-command {incr_font_size font_diff 1}
+	lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
+	$ctxm add separator
+	$ctxm add command -label [mc "Options..."] \
+		-command do_options
+}
+
 set ctxm .vpane.lower.diff.body.ctxm
 menu $ctxm -tearoff 0
 $ctxm add command \
@@ -2734,73 +2779,60 @@ $ctxm add command \
 set ui_diff_applyline [$ctxm index last]
 lappend diff_actions [list $ctxm entryconf $ui_diff_applyline -state]
 $ctxm add separator
-$ctxm add command \
-	-label [mc "Show Less Context"] \
-	-command show_less_context
-lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
-$ctxm add command \
-	-label [mc "Show More Context"] \
-	-command show_more_context
-lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
-$ctxm add separator
-$ctxm add command \
-	-label [mc Refresh] \
-	-command reshow_diff
-lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
-$ctxm add command \
-	-label [mc Copy] \
-	-command {tk_textCopy $ui_diff}
-lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
-$ctxm add command \
-	-label [mc "Select All"] \
-	-command {focus $ui_diff;$ui_diff tag add sel 0.0 end}
-lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
-$ctxm add command \
-	-label [mc "Copy All"] \
-	-command {
-		$ui_diff tag add sel 0.0 end
-		tk_textCopy $ui_diff
-		$ui_diff tag remove sel 0.0 end
-	}
-lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
-$ctxm add separator
-$ctxm add command \
-	-label [mc "Decrease Font Size"] \
-	-command {incr_font_size font_diff -1}
-lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
-$ctxm add command \
-	-label [mc "Increase Font Size"] \
-	-command {incr_font_size font_diff 1}
-lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
-$ctxm add separator
-$ctxm add command -label [mc "Options..."] \
-	-command do_options
-proc popup_diff_menu {ctxm x y X Y} {
+create_common_diff_popup $ctxm
+
+set ctxmmg .vpane.lower.diff.body.ctxmmg
+menu $ctxmmg -tearoff 0
+$ctxmmg add command \
+	-label [mc "Use Remote Version"] \
+	-command {merge_resolve_one 3}
+lappend diff_actions [list $ctxmmg entryconf [$ctxmmg index last] -state]
+$ctxmmg add command \
+	-label [mc "Use Local Version"] \
+	-command {merge_resolve_one 2}
+lappend diff_actions [list $ctxmmg entryconf [$ctxmmg index last] -state]
+$ctxmmg add command \
+	-label [mc "Revert To Base"] \
+	-command {merge_resolve_one 1}
+lappend diff_actions [list $ctxmmg entryconf [$ctxmmg index last] -state]
+$ctxmmg add separator
+create_common_diff_popup $ctxmmg
+
+proc popup_diff_menu {ctxm ctxmmg x y X Y} {
 	global current_diff_path file_states
 	set ::cursorX $x
 	set ::cursorY $y
-	if {$::ui_index eq $::current_diff_side} {
-		set l [mc "Unstage Hunk From Commit"]
-		set t [mc "Unstage Line From Commit"]
+	if {[info exists file_states($current_diff_path)]} {
+		set state [lindex $file_states($current_diff_path) 0]
 	} else {
-		set l [mc "Stage Hunk For Commit"]
-		set t [mc "Stage Line For Commit"]
-	}
-	if {$::is_3way_diff
-		|| $current_diff_path eq {}
-		|| ![info exists file_states($current_diff_path)]
-		|| {_O} eq [lindex $file_states($current_diff_path) 0]
-		|| {_T} eq [lindex $file_states($current_diff_path) 0]
-		|| {T_} eq [lindex $file_states($current_diff_path) 0]} {
-		set s disabled
+		set state {__}
+	}
+	if {[string index $state 0] eq {U}} {
+		tk_popup $ctxmmg $X $Y
 	} else {
-		set s normal
+		if {$::ui_index eq $::current_diff_side} {
+			set l [mc "Unstage Hunk From Commit"]
+			set t [mc "Unstage Line From Commit"]
+		} else {
+			set l [mc "Stage Hunk For Commit"]
+			set t [mc "Stage Line For Commit"]
+		}
+		if {$::is_3way_diff
+			|| $current_diff_path eq {}
+			|| {__} eq $state
+			|| {_O} eq $state
+			|| {_T} eq $state
+			|| {T_} eq $state} {
+			set s disabled
+		} else {
+			set s normal
+		}
+		$ctxm entryconf $::ui_diff_applyhunk -state $s -label $l
+		$ctxm entryconf $::ui_diff_applyline -state $s -label $t
+		tk_popup $ctxm $X $Y
 	}
-	$ctxm entryconf $::ui_diff_applyhunk -state $s -label $l
-	$ctxm entryconf $::ui_diff_applyline -state $s -label $t
-	tk_popup $ctxm $X $Y
 }
-bind_button3 $ui_diff [list popup_diff_menu $ctxm %x %y %X %Y]
+bind_button3 $ui_diff [list popup_diff_menu $ctxm $ctxmmg %x %y %X %Y]
 
 # -- Status Bar
 #
diff --git a/lib/mergetool.tcl b/lib/mergetool.tcl
new file mode 100644
index 0000000..7945d74
--- /dev/null
+++ b/lib/mergetool.tcl
@@ -0,0 +1,98 @@
+# git-gui merge conflict resolution
+# parts based on git-mergetool (c) 2006 Theodore Y. Ts'o
+
+proc merge_resolve_one {stage} {
+	global current_diff_path
+
+	switch -- $stage {
+		1 { set target [mc "the base version"] }
+		2 { set target [mc "this branch"] }
+		3 { set target [mc "the other branch"] }
+	}
+
+	set op_question [mc "Force resolution to %s?
+Note that the diff shows only conflicting changes.
+
+%s will be overwritten.
+
+This operation can be undone only by restarting the merge." \
+		$target [short_path $current_diff_path]]
+
+	if {[ask_popup $op_question] eq {yes}} {
+		merge_load_stages $current_diff_path [list merge_force_stage $stage]
+	}
+}
+
+proc merge_add_resolution {path} {
+	global current_diff_path
+
+	if {$path eq $current_diff_path} {
+		set after {reshow_diff;}
+	} else {
+		set after {}
+	}
+
+	update_index \
+		[mc "Adding resolution for %s" [short_path $path]] \
+		[list $path] \
+		[concat $after [list ui_ready]]
+}
+
+proc merge_force_stage {stage} {
+	global current_diff_path merge_stages
+
+	if {$merge_stages($stage) ne {}} {
+		git checkout-index -f --stage=$stage -- $current_diff_path
+	} else {
+		file delete -- $current_diff_path
+	}
+
+	merge_add_resolution $current_diff_path
+}
+
+proc merge_load_stages {path cont} {
+	global merge_stages_fd merge_stages merge_stages_buf
+
+	if {[info exists merge_stages_fd]} {
+		catch { kill_file_process $merge_stages_fd }
+		catch { close $merge_stages_fd }
+	}
+
+	set merge_stages(0) {}
+	set merge_stages(1) {}
+	set merge_stages(2) {}
+	set merge_stages(3) {}
+	set merge_stages_buf {}
+
+	set merge_stages_fd [eval git_read ls-files -u -z -- $path]
+
+	fconfigure $merge_stages_fd -blocking 0 -translation binary -encoding binary
+	fileevent $merge_stages_fd readable [list read_merge_stages $merge_stages_fd $cont]
+}
+
+proc read_merge_stages {fd cont} {
+	global merge_stages_buf merge_stages_fd merge_stages
+
+	append merge_stages_buf [read $fd]
+	set pck [split $merge_stages_buf "\0"]
+	set merge_stages_buf [lindex $pck end]
+
+	if {[eof $fd] && $merge_stages_buf ne {}} {
+		lappend pck {}
+		set merge_stages_buf {}
+	}
+
+	foreach p [lrange $pck 0 end-1] {
+		set fcols [split $p "\t"]
+		set cols  [split [lindex $fcols 0] " "]
+		set stage [lindex $cols 2]
+		
+		set merge_stages($stage) [lrange $cols 0 1]
+	}
+
+	if {[eof $fd]} {
+		close $fd
+		unset merge_stages_fd
+		eval $cont
+	}
+}
-- 
1.6.0.20.g6148bc

[PATCH (GIT-GUI) 3/8] git-gui: Support calling merge tools.

[Alexander Gavrilov]

Adds an item to the diff context menu in conflict mode,
which invokes a merge tool for the selected file. Tool
command-line handling code was ported from git-mergetool.

Automatic default tool selection and custom merge tools
are not supported. If merge.tool is not set, git-gui
defaults to meld.

This implementation uses a checkout-index hack in order
to retrieve all stages with autocrlf and filters properly
applied. It requires temporarily moving the original
conflict file out of the way.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 git-gui.sh        |    7 ++
 lib/mergetool.tcl |  252 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/option.tcl    |    1 +
 3 files changed, 260 insertions(+), 0 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 90d597e..8d4f715 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -657,6 +657,8 @@ proc apply_config {} {
 }
 
 set default_config(branch.autosetupmerge) true
+set default_config(merge.tool) {}
+set default_config(merge.keepbackup) true
 set default_config(merge.diffstat) true
 set default_config(merge.summary) false
 set default_config(merge.verbosity) 2
@@ -2784,6 +2786,11 @@ create_common_diff_popup $ctxm
 set ctxmmg .vpane.lower.diff.body.ctxmmg
 menu $ctxmmg -tearoff 0
 $ctxmmg add command \
+	-label [mc "Run Merge Tool"] \
+	-command {merge_resolve_tool}
+lappend diff_actions [list $ctxmmg entryconf [$ctxmmg index last] -state]
+$ctxmmg add separator
+$ctxmmg add command \
 	-label [mc "Use Remote Version"] \
 	-command {merge_resolve_one 3}
 lappend diff_actions [list $ctxmmg entryconf [$ctxmmg index last] -state]
diff --git a/lib/mergetool.tcl b/lib/mergetool.tcl
index 7945d74..ba9e8ce 100644
--- a/lib/mergetool.tcl
+++ b/lib/mergetool.tcl
@@ -96,3 +96,255 @@ proc read_merge_stages {fd cont} {
 		eval $cont
 	}
 }
+
+proc merge_resolve_tool {} {
+	global current_diff_path
+
+	merge_load_stages $current_diff_path [list merge_resolve_tool2]
+}
+
+proc merge_resolve_tool2 {} {
+	global current_diff_path merge_stages
+
+	# Validate the stages
+	if {$merge_stages(2) eq {} ||
+	    [lindex $merge_stages(2) 0] eq {120000} ||
+	    [lindex $merge_stages(2) 0] eq {160000} ||
+	    $merge_stages(3) eq {} ||
+	    [lindex $merge_stages(3) 0] eq {120000} ||
+	    [lindex $merge_stages(3) 0] eq {160000}
+	} {
+		error_popup [mc "Cannot resolve deletion or link conflicts using a tool"]
+		return
+	}
+
+	if {![file exists $current_diff_path]} {
+		error_popup [mc "Conflict file does not exist"]
+		return
+	}
+
+	# Determine the tool to use
+	set tool [get_config merge.tool]
+	if {$tool eq {}} { set tool meld }
+
+	set merge_tool_path [get_config "mergetool.$tool.path"]
+	if {$merge_tool_path eq {}} {
+		switch -- $tool {
+		emerge { set merge_tool_path "emacs" }
+		default { set merge_tool_path $tool }
+		}
+	}
+
+	# Make file names
+	set filebase [file rootname $current_diff_path]
+	set fileext  [file extension $current_diff_path]
+	set basename [lindex [file split $current_diff_path] end]
+
+	set MERGED   $current_diff_path
+	set BASE     "./$MERGED.BASE$fileext"
+	set LOCAL    "./$MERGED.LOCAL$fileext"
+	set REMOTE   "./$MERGED.REMOTE$fileext"
+	set BACKUP   "./$MERGED.BACKUP$fileext"
+
+	set base_stage $merge_stages(1)
+
+	# Build the command line
+	switch -- $tool {
+	kdiff3 {
+		if {$base_stage ne {}} {
+			set cmdline [list "$merge_tool_path" --auto --L1 "$MERGED (Base)" \
+				--L2 "$MERGED (Local)" --L3 "$MERGED (Remote)" -o "$MERGED" "$BASE" "$LOCAL" "$REMOTE"]
+		} else {
+			set cmdline [list "$merge_tool_path" --auto --L1 "$MERGED (Local)" \
+				--L2 "$MERGED (Remote)" -o "$MERGED" "$LOCAL" "$REMOTE"]
+		}
+	}
+	tkdiff {
+		if {$base_stage ne {}} {
+			set cmdline [list "$merge_tool_path" -a "$BASE" -o "$MERGED" "$LOCAL" "$REMOTE"]
+		} else {
+			set cmdline [list "$merge_tool_path" -o "$MERGED" "$LOCAL" "$REMOTE"]
+		}
+	}
+	meld {
+		set cmdline [list "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"]
+	}
+	gvimdiff {
+		set cmdline [list "$merge_tool_path" -f "$LOCAL" "$MERGED" "$REMOTE"]
+	}
+	xxdiff {
+		if {$base_stage ne {}} {
+			set cmdline [list "$merge_tool_path" -X --show-merged-pane \
+					    -R {Accel.SaveAsMerged: "Ctrl-S"} \
+					    -R {Accel.Search: "Ctrl+F"} \
+					    -R {Accel.SearchForward: "Ctrl-G"} \
+					    --merged-file "$MERGED" "$LOCAL" "$BASE" "$REMOTE"]
+		} else {
+			set cmdline [list "$merge_tool_path" -X --show-merged-pane \
+					    -R {Accel.SaveAsMerged: "Ctrl-S"} \
+					    -R {Accel.Search: "Ctrl+F"} \
+					    -R {Accel.SearchForward: "Ctrl-G"} \
+					    --merged-file "$MERGED" "$LOCAL" "$REMOTE"]
+		}
+	}
+	opendiff {
+		if {$base_stage ne {}} {
+			set cmdline [list "$merge_tool_path" "$LOCAL" "$REMOTE" -ancestor "$BASE" -merge "$MERGED"]
+		} else {
+			set cmdline [list "$merge_tool_path" "$LOCAL" "$REMOTE" -merge "$MERGED"]
+		}
+	}
+	ecmerge {
+		if {$base_stage ne {}} {
+			set cmdline [list "$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" --default --mode=merge3 --to="$MERGED"]
+		} else {
+			set cmdline [list "$merge_tool_path" "$LOCAL" "$REMOTE" --default --mode=merge2 --to="$MERGED"]
+		}
+	}
+	emerge {
+		if {$base_stage ne {}} {
+			set cmdline [list "$merge_tool_path" -f emerge-files-with-ancestor-command \
+					"$LOCAL" "$REMOTE" "$BASE" "$basename"]
+		} else {
+			set cmdline [list "$merge_tool_path" -f emerge-files-command \
+					"$LOCAL" "$REMOTE" "$basename"]
+		}
+	}
+	vimdiff {
+		error_popup [mc "Not a GUI merge tool: '%s'" $tool]
+		return
+	}
+	default {
+		error_popup [mc "Unsupported merge tool '%s'" $tool]
+		return
+	}
+	}
+
+	merge_tool_start $cmdline $MERGED $BACKUP [list $BASE $LOCAL $REMOTE]
+}
+
+proc delete_temp_files {files} {
+	foreach fname $files {
+		file delete $fname
+	}
+}
+
+proc merge_tool_get_stages {target stages} {
+	global merge_stages
+
+	set i 1
+	foreach fname $stages {
+		if {$merge_stages($i) eq {}} {
+			file delete $fname
+		} else {
+			# A hack to support autocrlf properly
+			git checkout-index -f --stage=$i -- $target
+			file rename -force -- $target $fname
+		}
+		incr i
+	}
+}
+
+proc merge_tool_start {cmdline target backup stages} {
+	global merge_stages mtool_target mtool_tmpfiles mtool_fd mtool_mtime
+
+	if {[info exists mtool_fd]} {
+		if {[ask_popup [mc "Merge tool is already running, terminate it?"]] eq {yes}} {
+			catch { kill_file_process $mtool_fd }
+			catch { close $mtool_fd }
+			unset mtool_fd
+
+			set old_backup [lindex $mtool_tmpfiles end]
+			file rename -force -- $old_backup $mtool_target
+			delete_temp_files $mtool_tmpfiles
+		} else {
+			return
+		}
+	}
+
+	# Save the original file
+	file rename -force -- $target $backup
+
+	# Get the blobs; it destroys $target
+	if {[catch {merge_tool_get_stages $target $stages} err]} {
+		file rename -force -- $backup $target
+		delete_temp_files $stages
+		error_popup [mc "Error retrieving versions:\n%s" $err]
+		return
+	}
+
+	# Restore the conflict file
+	file copy -force -- $backup $target
+
+	# Initialize global state
+	set mtool_target $target
+	set mtool_mtime [file mtime $target]
+	set mtool_tmpfiles $stages
+
+	lappend mtool_tmpfiles $backup
+
+	# Force redirection to avoid interpreting output on stderr
+	# as an error, and launch the tool
+	lappend cmdline {2>@1}
+
+	if {[catch { set mtool_fd [_open_stdout_stderr $cmdline] } err]} {
+		delete_temp_files $mtool_tmpfiles
+		error_popup [mc "Could not start the merge tool:\n\n%s" $err]
+		return
+	}
+
+	ui_status [mc "Running merge tool..."]
+
+	fconfigure $mtool_fd -blocking 0 -translation binary -encoding binary
+	fileevent $mtool_fd readable [list read_mtool_output $mtool_fd]
+}
+
+proc read_mtool_output {fd} {
+	global mtool_fd mtool_tmpfiles
+
+	read $fd
+	if {[eof $fd]} {
+		unset mtool_fd
+
+		fconfigure $fd -blocking 1
+		merge_tool_finish $fd
+	}
+}
+
+proc merge_tool_finish {fd} {
+	global mtool_tmpfiles mtool_target mtool_mtime
+
+	set backup [lindex $mtool_tmpfiles end]
+	set failed 0
+
+	# Check the return code
+	if {[catch {close $fd} err]} {
+		set failed 1
+		if {$err ne {child process exited abnormally}} {
+			error_popup [strcat [mc "Merge tool failed."] "\n\n$err"]
+		}
+	}
+
+	# Check the modification time of the target file
+	if {!$failed && [file mtime $mtool_target] eq $mtool_mtime} {
+		if {[ask_popup [mc "File %s unchanged, still accept as resolved?" \
+				[short_path $mtool_target]]] ne {yes}} {
+			set failed 1
+		}
+	}
+
+	# Finish
+	if {$failed} {
+		file rename -force -- $backup $mtool_target
+		delete_temp_files $mtool_tmpfiles
+		ui_status [mc "Merge tool failed."]
+	} else {
+		if {[is_config_true merge.keepbackup]} {
+			file rename -force -- $backup "$mtool_target.orig"
+		}
+
+		delete_temp_files $mtool_tmpfiles
+
+		merge_add_resolution $mtool_target
+	}
+}
diff --git a/lib/option.tcl b/lib/option.tcl
index eb958ff..dd979e2 100644
--- a/lib/option.tcl
+++ b/lib/option.tcl
@@ -119,6 +119,7 @@ proc do_options {} {
 		{b merge.summary {mc "Summarize Merge Commits"}}
 		{i-1..5 merge.verbosity {mc "Merge Verbosity"}}
 		{b merge.diffstat {mc "Show Diffstat After Merge"}}
+		{t merge.tool {mc "Use Merge Tool"}}
 
 		{b gui.trustmtime  {mc "Trust File Modification Timestamps"}}
 		{b gui.pruneduringfetch {mc "Prune Tracking Branches During Fetch"}}
-- 
1.6.0.20.g6148bc

[PATCH (GIT-GUI) 4/8] git-gui: Support more merge tools.

[Alexander Gavrilov]

Add native support for Araxis Merge, WinMerge and Perforce merge.

Custom merge tools are not implemented by mergetool.tcl; besides,
native support allows constructing the command lines in a more
intelligent way.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 lib/mergetool.tcl |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/lib/mergetool.tcl b/lib/mergetool.tcl
index ba9e8ce..5f3da1f 100644
--- a/lib/mergetool.tcl
+++ b/lib/mergetool.tcl
@@ -131,6 +131,7 @@ proc merge_resolve_tool2 {} {
 	if {$merge_tool_path eq {}} {
 		switch -- $tool {
 		emerge { set merge_tool_path "emacs" }
+		araxis { set merge_tool_path "compare" }
 		default { set merge_tool_path $tool }
 		}
 	}
@@ -210,6 +211,31 @@ proc merge_resolve_tool2 {} {
 					"$LOCAL" "$REMOTE" "$basename"]
 		}
 	}
+	winmerge {
+		if {$base_stage ne {}} {
+			# This tool does not support 3-way merges.
+			# Use the 'conflict file' resolution feature instead.
+			set cmdline [list "$merge_tool_path" -e -ub "$MERGED"]
+		} else {
+			set cmdline [list "$merge_tool_path" -e -ub -wl \
+				-dl "Theirs File" -dr "Mine File" "$REMOTE" "$LOCAL" "$MERGED"]
+		}
+	}
+	araxis {
+		if {$base_stage ne {}} {
+			set cmdline [list "$merge_tool_path" -wait -merge -3 -a1 \
+				-title1:"'$MERGED (Base)'" -title2:"'$MERGED (Local)'" \
+				-title3:"'$MERGED (Remote)'" \
+				"$BASE" "$LOCAL" "$REMOTE" "$MERGED"]
+		} else {
+			set cmdline [list "$merge_tool_path" -wait -2 \
+				 -title1:"'$MERGED (Local)'" -title2:"'$MERGED (Remote)'" \
+				 "$LOCAL" "$REMOTE" "$MERGED"]
+		}
+	}
+	p4merge {
+		set cmdline [list "$merge_tool_path" "$BASE" "$REMOTE" "$LOCAL" "$MERGED"]
+	}
 	vimdiff {
 		error_popup [mc "Not a GUI merge tool: '%s'" $tool]
 		return
@@ -236,6 +262,7 @@ proc merge_tool_get_stages {target stages} {
 	foreach fname $stages {
 		if {$merge_stages($i) eq {}} {
 			file delete $fname
+			catch { close [open $fname w] }
 		} else {
 			# A hack to support autocrlf properly
 			git checkout-index -f --stage=$i -- $target
-- 
1.6.0.20.g6148bc

[PATCH (GIT-GUI) 5/8] git-gui: Support conflict states _U & UT.

[Alexander Gavrilov]

Support _U (local deleted, remote modified) and
UT (file type changed in conflict) modes.

Note that 'file type changed' does not refer to
changes in the executable bit, instead it denotes
replacing a file with a link, or vice versa.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 git-gui.sh     |    6 ++++--
 lib/commit.tcl |    1 +
 lib/diff.tcl   |    2 +-
 lib/index.tcl  |    1 +
 4 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 8d4f715..a3000b6 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1669,10 +1669,12 @@ foreach i {
 		{D_ {mc "Staged for removal"}}
 		{DO {mc "Staged for removal, still present"}}
 
+		{_U {mc "Requires merge resolution"}}
 		{U_ {mc "Requires merge resolution"}}
 		{UU {mc "Requires merge resolution"}}
 		{UM {mc "Requires merge resolution"}}
 		{UD {mc "Requires merge resolution"}}
+		{UT {mc "Requires merge resolution"}}
 	} {
 	set text [eval [lindex $i 1]]
 	if {$max_status_desc < [string length $text]} {
@@ -1843,7 +1845,7 @@ proc toggle_or_diff {w x y} {
 	# Do not stage files with conflicts
 	if {[info exists file_states($path)]} {
 		set state [lindex $file_states($path) 0]
-		if {[string index $state 0] eq {U}} {
+		if {[string first {U} $state] >= 0} {
 			set col 1
 		}
 	}
@@ -2814,7 +2816,7 @@ proc popup_diff_menu {ctxm ctxmmg x y X Y} {
 	} else {
 		set state {__}
 	}
-	if {[string index $state 0] eq {U}} {
+	if {[string first {U} $state] >= 0} {
 		tk_popup $ctxmmg $X $Y
 	} else {
 		if {$::ui_index eq $::current_diff_side} {
diff --git a/lib/commit.tcl b/lib/commit.tcl
index f4ab707..2977315 100644
--- a/lib/commit.tcl
+++ b/lib/commit.tcl
@@ -151,6 +151,7 @@ The rescan will be automatically started now.
 		D? -
 		T_ -
 		M? {set files_ready 1}
+		_U -
 		U? {
 			error_popup [mc "Unmerged files cannot be committed.
 
diff --git a/lib/diff.tcl b/lib/diff.tcl
index 52b79e4..a7fcdc6 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -166,7 +166,7 @@ proc show_diff {path w {lno {}} {scroll_pos {}}} {
 		lappend cmd diff-index
 		lappend cmd --cached
 	} elseif {$w eq $ui_workdir} {
-		if {[string index $m 0] eq {U}} {
+		if {[string first {U} $m] >= 0} {
 			lappend cmd diff
 		} else {
 			lappend cmd diff-files
diff --git a/lib/index.tcl b/lib/index.tcl
index 7c27f2a..06a4fb0 100644
--- a/lib/index.tcl
+++ b/lib/index.tcl
@@ -164,6 +164,7 @@ proc write_update_index {fd pathList totalCnt batch after} {
 		_O -
 		AM {set new A_}
 		_T {set new T_}
+		_U -
 		U? {
 			if {[file exists $path]} {
 				set new M_
-- 
1.6.0.20.g6148bc

[PATCH (GIT-GUI) 6/8] git-gui: Reimplement and enhance auto-selection of diffs.

[Alexander Gavrilov]

Generalize the next_diff system, and implement auto-reselection
for merge tool resolution and reshow_diff. Also add auto-selection
of diffs after rescan, if no diff is already selected.

New auto-select rules:

- Rescan auto-selects the first conflicting file, or if none
  a modified tracked file, if nothing was selected previously.
- Resolving a conflict auto-selects the nearest conflicting
  file, or nothing if everything is resolved.
- Staging the last remaining hunk auto-selects the nearest
  modified staged file.
- Staging a file through its icon auto-selects the nearest file.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 git-gui.sh        |  122 +++++++++++++++++++++++++++++++++++++++++------------
 lib/diff.tcl      |   18 +++++---
 lib/mergetool.tcl |    8 +---
 3 files changed, 109 insertions(+), 39 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index a3000b6..f84f436 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1326,6 +1326,8 @@ proc rescan_done {fd buf after} {
 	unlock_index
 	display_all_files
 	if {$current_diff_path ne {}} reshow_diff
+	if {$current_diff_path eq {}} select_first_diff
+
 	uplevel #0 $after
 }
 
@@ -1821,7 +1823,98 @@ proc do_commit {} {
 
 proc next_diff {} {
 	global next_diff_p next_diff_w next_diff_i
-	show_diff $next_diff_p $next_diff_w $next_diff_i
+	show_diff $next_diff_p $next_diff_w {}
+}
+
+proc find_anchor_pos {lst name} {
+	set lid [lsearch -sorted -exact $lst $name]
+
+	if {$lid == -1} {
+		set lid 0
+		foreach lname $lst {
+			if {$lname >= $name} break
+			incr lid
+		}
+	}
+
+	return $lid
+}
+
+proc find_file_from {flist idx delta path mmask} {
+	global file_states
+
+	set len [llength $flist]
+	while {$idx >= 0 && $idx < $len} {
+		set name [lindex $flist $idx]
+
+		if {$name ne $path && [info exists file_states($name)]} {
+			set state [lindex $file_states($name) 0]
+
+			if {$mmask eq {} || [regexp $mmask $state]} {
+				return $idx
+			}
+		}
+
+		incr idx $delta
+	}
+
+	return {}
+}
+
+proc find_next_diff {w path {lno {}} {mmask {}}} {
+	global next_diff_p next_diff_w next_diff_i
+	global file_lists ui_index ui_workdir
+
+	set flist $file_lists($w)
+	if {$lno eq {}} {
+		set lno [find_anchor_pos $flist $path]
+	} else {
+		incr lno -1
+	}
+
+	if {$mmask ne {} && ![regexp {(^\^)|(\$$)} $mmask]} {
+		if {$w eq $ui_index} {
+			set mmask "^$mmask"
+		} else {
+			set mmask "$mmask\$"
+		}
+	}
+
+	set idx [find_file_from $flist $lno 1 $path $mmask]
+	if {$idx eq {}} {
+		incr lno -1
+		set idx [find_file_from $flist $lno -1 $path $mmask]
+	}
+
+	if {$idx ne {}} {
+		set next_diff_w $w
+		set next_diff_p [lindex $flist $idx]
+		set next_diff_i [expr {$idx+1}]
+		return 1
+	} else {
+		return 0
+	}
+}
+
+proc next_diff_after_action {w path {lno {}} {mmask {}}} {
+	global current_diff_path
+
+	if {$path ne $current_diff_path} {
+		return {}
+	} elseif {[find_next_diff $w $path $lno $mmask]} {
+		return {next_diff;}
+	} else {
+		return {reshow_diff;}
+	}
+}
+
+proc select_first_diff {} {
+	global ui_workdir
+
+	if {[find_next_diff $ui_workdir {} 1 {^_?U}] ||
+	    [find_next_diff $ui_workdir {} 1 {[^O]$}]} {
+		next_diff
+	}
 }
 
 proc toggle_or_diff {w x y} {
@@ -1851,32 +1944,7 @@ proc toggle_or_diff {w x y} {
 	}
 
 	if {$col == 0 && $y > 1} {
-		set i [expr {$lno-1}]
-		set ll [expr {[llength $file_lists($w)]-1}]
-
-		if {$i == $ll && $i == 0} {
-			set after {reshow_diff;}
-		} else {
-			global next_diff_p next_diff_w next_diff_i
-
-			set next_diff_w $w
-
-			if {$i < $ll} {
-				set i [expr {$i + 1}]
-				set next_diff_i $i
-			} else {
-				set next_diff_i $i
-				set i [expr {$i - 1}]
-			}
-
-			set next_diff_p [lindex $file_lists($w) $i]
-
-			if {$next_diff_p ne {} && $current_diff_path ne {}} {
-				set after {next_diff;}
-			} else {
-				set after {}
-			}
-		}
+		set after [next_diff_after_action $w $path $lno]
 
 		if {$w eq $ui_index} {
 			update_indexinfo \
diff --git a/lib/diff.tcl b/lib/diff.tcl
index a7fcdc6..95998b4 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -24,10 +24,16 @@ proc reshow_diff {} {
 	set p $current_diff_path
 	if {$p eq {}} {
 		# No diff is being shown.
-	} elseif {$current_diff_side eq {}
-		|| [catch {set s $file_states($p)}]
-		|| [lsearch -sorted -exact $file_lists($current_diff_side) $p] == -1} {
+	} elseif {$current_diff_side eq {}} {
 		clear_diff
+	} elseif {[catch {set s $file_states($p)}]
+		|| [lsearch -sorted -exact $file_lists($current_diff_side) $p] == -1} {
+
+		if {[find_next_diff $current_diff_side $p {} {[^O]}]} {
+			next_diff
+		} else {
+			clear_diff
+		}
 	} else {
 		set save_pos [lindex [$ui_diff yview] 0]
 		show_diff $p $current_diff_side {} $save_pos
@@ -71,6 +77,7 @@ proc show_diff {path w {lno {}} {scroll_pos {}}} {
 	}
 	if {$lno >= 1} {
 		$w tag add in_diff $lno.0 [expr {$lno + 1}].0
+		$w see $lno.0
 	}
 
 	set s $file_states($path)
@@ -366,10 +373,9 @@ proc apply_hunk {x y} {
 	}
 	unlock_index
 	display_file $current_diff_path $mi
+	# This should trigger shift to the next changed file
 	if {$o eq {_}} {
-		clear_diff
-	} else {
-		set current_diff_path $current_diff_path
+		reshow_diff
 	}
 }
 
diff --git a/lib/mergetool.tcl b/lib/mergetool.tcl
index 5f3da1f..79c58bc 100644
--- a/lib/mergetool.tcl
+++ b/lib/mergetool.tcl
@@ -24,13 +24,9 @@ This operation can be undone only by restarting the merge." \
 }
 
 proc merge_add_resolution {path} {
-	global current_diff_path
+	global current_diff_path ui_workdir
 
-	if {$path eq $current_diff_path} {
-		set after {reshow_diff;}
-	} else {
-		set after {}
-	}
+	set after [next_diff_after_action $ui_workdir $path {} {^_?U}]
 
 	update_index \
 		[mc "Adding resolution for %s" [short_path $path]] \
-- 
1.6.0.20.g6148bc

[PATCH (GIT-GUI) 7/8] git-gui: Make F5 reselect a diff, if an untracked file is selected.

[Alexander Gavrilov]

If an untracked file is selected, F5 and other manual rescan synonyms
would try to select a tracked file instead. Also, clicking on an icon
in the unstaged changes list skips over untracked files, unless the
file clicked is untracked itself.

The objective is to make it easier to ignore untracked files showing
up in the Unstaged Changes list, and ensure that no modifications
to tracked objects are left unstaged.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 git-gui.sh |   44 +++++++++++++++++++++++++++++++++++---------
 1 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index f84f436..fde44a6 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1817,6 +1817,10 @@ proc do_rescan {} {
 	rescan ui_ready
 }
 
+proc ui_do_rescan {} {
+	rescan {force_first_diff; ui_ready}
+}
+
 proc do_commit {} {
 	commit_tree
 }
@@ -1917,6 +1921,18 @@ proc select_first_diff {} {
 	}
 }
 
+proc force_first_diff {} {
+	global current_diff_path
+
+	if {[info exists file_states($current_diff_path)]} {
+		set state [lindex $file_states($current_diff_path) 0]
+
+		if {[string index $state 1] ne {O}} return
+	}
+
+	select_first_diff
+}
+
 proc toggle_or_diff {w x y} {
 	global file_states file_lists current_diff_path ui_index ui_workdir
 	global last_clicked selected_paths
@@ -1938,13 +1954,23 @@ proc toggle_or_diff {w x y} {
 	# Do not stage files with conflicts
 	if {[info exists file_states($path)]} {
 		set state [lindex $file_states($path) 0]
-		if {[string first {U} $state] >= 0} {
-			set col 1
-		}
+	} else {
+		set state {__}
+	}
+
+	if {[string first {U} $state] >= 0} {
+		set col 1
 	}
 
+	# Restage the file, or simply show the diff
 	if {$col == 0 && $y > 1} {
-		set after [next_diff_after_action $w $path $lno]
+		if {[string index $state 1] eq {O}} {
+			set mmask {}
+		} else {
+			set mmask {[^O]}
+		}
+
+		set after [next_diff_after_action $w $path $lno $mmask]
 
 		if {$w eq $ui_index} {
 			update_indexinfo \
@@ -2208,7 +2234,7 @@ if {[is_enabled multicommit] || [is_enabled singlecommit]} {
 	.mbar.commit add separator
 
 	.mbar.commit add command -label [mc Rescan] \
-		-command do_rescan \
+		-command ui_do_rescan \
 		-accelerator F5
 	lappend disable_on_lock \
 		[list .mbar.commit entryconf [.mbar.commit index last] -state]
@@ -2564,7 +2590,7 @@ pack .vpane.lower.commarea.buttons.l -side top -fill x
 pack .vpane.lower.commarea.buttons -side left -fill y
 
 button .vpane.lower.commarea.buttons.rescan -text [mc Rescan] \
-	-command do_rescan
+	-command ui_do_rescan
 pack .vpane.lower.commarea.buttons.rescan -side top -fill x
 lappend disable_on_lock \
 	{.vpane.lower.commarea.buttons.rescan conf -state}
@@ -2985,9 +3011,9 @@ if {[is_enabled transport]} {
 	bind . <$M1B-Key-P> do_push_anywhere
 }
 
-bind .   <Key-F5>     do_rescan
-bind .   <$M1B-Key-r> do_rescan
-bind .   <$M1B-Key-R> do_rescan
+bind .   <Key-F5>     ui_do_rescan
+bind .   <$M1B-Key-r> ui_do_rescan
+bind .   <$M1B-Key-R> ui_do_rescan
 bind .   <$M1B-Key-s> do_signoff
 bind .   <$M1B-Key-S> do_signoff
 bind .   <$M1B-Key-t> do_add_selection
-- 
1.6.0.20.g6148bc

[PATCH (GIT-GUI) 8/8] git-gui: Show special diffs for complex conflict cases.

[Alexander Gavrilov]

Add special handling for displaying diffs of modified/deleted,
and symlink/mode conflicts. Currently the display is completely
unusable for deciding how to resolve the conflict.

New display modes:

1) Deleted/Modified conflict: e.g.
	LOCAL: deleted
	REMOTE:
	[diff :1:$path :3:$path]

2) Conflict involving symlinks:
	LOCAL:
	[diff :1:$path :2:$path]
	REMOTE:
	[diff :1:$path :3:$path]

In order to be able to display multiple diffs, this
patch adds a queue of commands to call.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 lib/diff.tcl |   94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 89 insertions(+), 5 deletions(-)

diff --git a/lib/diff.tcl b/lib/diff.tcl
index 95998b4..c67b020 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -65,6 +65,7 @@ proc show_diff {path w {lno {}} {scroll_pos {}}} {
 	global is_3way_diff diff_active repo_config
 	global ui_diff ui_index ui_workdir
 	global current_diff_path current_diff_side current_diff_header
+	global current_diff_queue
 
 	if {$diff_active || ![lock_index read]} return
 
@@ -82,13 +83,69 @@ proc show_diff {path w {lno {}} {scroll_pos {}}} {
 
 	set s $file_states($path)
 	set m [lindex $s 0]
-	set is_3way_diff 0
-	set diff_active 1
 	set current_diff_path $path
 	set current_diff_side $w
-	set current_diff_header {}
+	set current_diff_queue {}
 	ui_status [mc "Loading diff of %s..." [escape_path $path]]
 
+	if {[string first {U} $m] >= 0} {
+		merge_load_stages $path [list show_unmerged_diff $scroll_pos]
+	} elseif {$m eq {_O}} {
+		show_other_diff $path $w $m $scroll_pos
+	} else {
+		start_show_diff $scroll_pos
+	}
+}
+
+proc show_unmerged_diff {scroll_pos} {
+	global current_diff_path current_diff_side
+	global merge_stages ui_diff
+	global current_diff_queue
+
+	if {$merge_stages(2) eq {}} {
+		lappend current_diff_queue \
+			[list "LOCAL: deleted\nREMOTE:\n" d======= \
+			    [list ":1:$current_diff_path" ":3:$current_diff_path"]]
+	} elseif {$merge_stages(3) eq {}} {
+		lappend current_diff_queue \
+			[list "REMOTE: deleted\nLOCAL:\n" d======= \
+			    [list ":1:$current_diff_path" ":2:$current_diff_path"]]
+	} elseif {[lindex $merge_stages(1) 0] eq {120000}
+		|| [lindex $merge_stages(2) 0] eq {120000}
+		|| [lindex $merge_stages(3) 0] eq {120000}} {
+		lappend current_diff_queue \
+			[list "LOCAL:\n" d======= \
+			    [list ":1:$current_diff_path" ":2:$current_diff_path"]]
+		lappend current_diff_queue \
+			[list "REMOTE:\n" d======= \
+			    [list ":1:$current_diff_path" ":3:$current_diff_path"]]
+	} else {
+		start_show_diff $scroll_pos
+		return
+	}
+
+	advance_diff_queue $scroll_pos
+}
+
+proc advance_diff_queue {scroll_pos} {
+	global current_diff_queue ui_diff
+
+	set item [lindex $current_diff_queue 0]
+	set current_diff_queue [lrange $current_diff_queue 1 end]
+
+	$ui_diff conf -state normal
+	$ui_diff insert end [lindex $item 0] [lindex $item 1]
+	$ui_diff conf -state disabled
+
+	start_show_diff $scroll_pos [lindex $item 2]
+}
+
+proc show_other_diff {path w m scroll_pos} {
+	global file_states file_lists
+	global is_3way_diff diff_active repo_config
+	global ui_diff ui_index ui_workdir
+	global current_diff_path current_diff_side current_diff_header
+
 	# - Git won't give us the diff, there's nothing to compare to!
 	#
 	if {$m eq {_O}} {
@@ -167,6 +224,22 @@ proc show_diff {path w {lno {}} {scroll_pos {}}} {
 		ui_ready
 		return
 	}
+}
+
+proc start_show_diff {scroll_pos {add_opts {}}} {
+	global file_states file_lists
+	global is_3way_diff diff_active repo_config
+	global ui_diff ui_index ui_workdir
+	global current_diff_path current_diff_side current_diff_header
+
+	set path $current_diff_path
+	set w $current_diff_side
+
+	set s $file_states($path)
+	set m [lindex $s 0]
+	set is_3way_diff 0
+	set diff_active 1
+	set current_diff_header {}
 
 	set cmd [list]
 	if {$w eq $ui_index} {
@@ -188,8 +261,12 @@ proc show_diff {path w {lno {}} {scroll_pos {}}} {
 	if {$w eq $ui_index} {
 		lappend cmd [PARENT]
 	}
-	lappend cmd --
-	lappend cmd $path
+	if {$add_opts ne {}} {
+		eval lappend cmd $add_opts
+	} else {
+		lappend cmd --
+		lappend cmd $path
+	}
 
 	if {[catch {set fd [eval git_read --nice $cmd]} err]} {
 		set diff_active 0
@@ -209,6 +286,7 @@ proc show_diff {path w {lno {}} {scroll_pos {}}} {
 proc read_diff {fd scroll_pos} {
 	global ui_diff diff_active
 	global is_3way_diff current_diff_header
+	global current_diff_queue
 
 	$ui_diff conf -state normal
 	while {[gets $fd line] >= 0} {
@@ -293,6 +371,12 @@ proc read_diff {fd scroll_pos} {
 
 	if {[eof $fd]} {
 		close $fd
+
+		if {$current_diff_queue ne {}} {
+			advance_diff_queue $scroll_pos
+			return
+		}
+
 		set diff_active 0
 		unlock_index
 		if {$scroll_pos ne {}} {
-- 
1.6.0.20.g6148bc

Re: [PATCH (GIT-GUI) 1/8] git-gui: Don't allow staging files with conflicts.

[Johannes Sixt]

Alexander Gavrilov schrieb:
> Prevent staging files with conflict markers by clicking
> on the icon in the 'Unstaged Changes' list. Instead, pretend
> that the user clicked the name, and show the diff.
> 
> Originally it made some sense to allow staging conflicting
> files, because git-gui did not provide any tools to resolve
> them from within the GUI. But now that I'm adding mergetool
> capabilities, it is more likely to cause accidental and
> non-undoable errors.

I know I'm a bit late... I just got the first impression of the
consequences of this patch. And I don't like them.

I did a merge with conflicts, and the result was resolved to my liking by
rerere. (And even if rerere did not kick in - I'd have resolved the
conflicts in an external editor anyway.) Now I want to stage the file -
but I can't. :-(

No, I can't "Run merge tool", because I don't have one.
No, I don't want to "Use remote version" or "Use local version" or "Revert
to base".
Yes, I want to stage the file _as_is_!!

Please help.

-- Hannes

Re: [PATCH (GIT-GUI) 1/8] git-gui: Don't allow staging files with conflicts.

[Alexander Gavrilov]

> I know I'm a bit late... I just got the first impression of the
> consequences of this patch. And I don't like them.
>
> I did a merge with conflicts, and the result was resolved to my liking by
> rerere. (And even if rerere did not kick in - I'd have resolved the
> conflicts in an external editor anyway.) Now I want to stage the file -
> but I can't. :-(
>
> No, I can't "Run merge tool", because I don't have one.
> No, I don't want to "Use remote version" or "Use local version" or "Revert
> to base".
> Yes, I want to stage the file _as_is_!!
>
> Please help.

I'll make a patch today to add a new item to the popup menu.

I don't use rerere much, so I didn't notice this problem myself until
yesterday evening.

Alexander

[PATCH/RFC 0/2] git-gui: issues with merge tool series

[Johannes Sixt]

I've now worked a while with the latest git-gui that includes the
capability to run a merge tool, and I must say that there is room
for improvement.

I greatly appreciate that a merge tool can be invoked. This is really
helpful.

But...  I've nevertheless two *big* issues with that:

1. The inability to stage a conflicted file by clicking the icon is
   *very* disruptive. The new menu entry "Stage Working Copy" is
   really only a workaround, and it shows.

2. The fact that a file was automatically staged after the merge
   tool was exited has caused headaches that the merge tool should
   actually cure. Because of this behavior, I could not postpone a
   conflict resolution once I have started the merge tool, nor can
   I re-run the tool if I forgot to resolve a conflict.

This patch series addresses these issues.

Furthermore, the operations "Use local version" and "Use remote
version" could be much more useful. They could serve as "merge
shortcuts": These operations should only use the "local" or "remote"
part inside the conflict markers, and not also reset the part that
is not visible. In the current form I found them useful only in the
modify/delete conflict cases, where I picked the "delete" part.

HTH,
-- Hannes

[PATCH/RFC 1/2] Revert "git-gui: Don't allow staging files with conflicts."

[Johannes Sixt]

This reverts commits 617ceee653bd7377f662bfc6d3085f321efab7e4 (Don't allow
staging files with conflicts) and 2fe5b2ee42897a3acc78e5ddaace3775eb2713ca
(Restore ability to Stage Working Copy for conflicts).

The inability to use the icon in front of the file name to stage a file,
even a conflicted one, is too disruptive for the workflow. It was intended
as a safety valve against accidentally staging the conflicted file, which
would remove the conflict stages from the index and inhibit the use of
the merge tool. The new menu item "Stage Working Copy" that the second
reverted commit introduced helped a bit, but it clearly was just a
workaround.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 git-gui.sh        |   10 ----------
 lib/mergetool.tcl |    6 ------
 2 files changed, 0 insertions(+), 16 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 1044ab9..3b1532c 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1954,17 +1954,12 @@ proc toggle_or_diff {w x y} {
 	$ui_index tag remove in_sel 0.0 end
 	$ui_workdir tag remove in_sel 0.0 end
 
-	# Do not stage files with conflicts
 	if {[info exists file_states($path)]} {
 		set state [lindex $file_states($path) 0]
 	} else {
 		set state {__}
 	}
 
-	if {[string first {U} $state] >= 0} {
-		set col 1
-	}
-
 	# Restage the file, or simply show the diff
 	if {$col == 0 && $y > 1} {
 		if {[string index $state 1] eq {O}} {
@@ -2902,11 +2897,6 @@ $ctxmmg add command \
 	-command {merge_resolve_one 1}
 lappend diff_actions [list $ctxmmg entryconf [$ctxmmg index last] -state]
 $ctxmmg add separator
-$ctxmmg add command \
-	-label [mc "Stage Working Copy"] \
-	-command {merge_resolve_one 0}
-lappend diff_actions [list $ctxmmg entryconf [$ctxmmg index last] -state]
-$ctxmmg add separator
 create_common_diff_popup $ctxmmg
 
 proc popup_diff_menu {ctxm ctxmmg x y X Y} {
diff --git a/lib/mergetool.tcl b/lib/mergetool.tcl
index a44a725..965cfe4 100644
--- a/lib/mergetool.tcl
+++ b/lib/mergetool.tcl
@@ -5,12 +5,6 @@ proc merge_resolve_one {stage} {
 	global current_diff_path
 
 	switch -- $stage {
-		0 {	# Stage without confirmation, to minimize
-			# disruption of the rerere workflow
-			merge_add_resolution $current_diff_path
-			return
-		}
-
 		1 { set targetquestion [mc "Force resolution to the base version?"] }
 		2 { set targetquestion [mc "Force resolution to this branch?"] }
 		3 { set targetquestion [mc "Force resolution to the other branch?"] }
-- 
1.6.0.1.1210.gb7ffe

[PATCH/RFC 2/2] git-gui: Do not automatically stage file after merge tool finishes

[Johannes Sixt]

If a merge tool was invoked on a conflicted file and the tool completed,
then the conflicted file was staged automatically. However, the fact that
the user closed the merge tool cannot be understood as the unequivocal
sign that the conflict was completely resolved. For example, the user
could have decided to postpone the resolution of the conflict, or could
have accidentally closed the tool. We better leave the file unstaged and
let the user stage it explicitly.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 lib/mergetool.tcl |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/lib/mergetool.tcl b/lib/mergetool.tcl
index 965cfe4..1a96189 100644
--- a/lib/mergetool.tcl
+++ b/lib/mergetool.tcl
@@ -367,7 +367,5 @@ proc merge_tool_finish {fd} {
 		}
 
 		delete_temp_files $mtool_tmpfiles
-
-		merge_add_resolution $mtool_target
 	}
 }
-- 
1.6.0.1.1210.gb7ffe

Re: [PATCH/RFC 2/2] git-gui: Do not automatically stage file after merge tool finishes

[Alexander Gavrilov]

On Wed, Sep 17, 2008 at 3:40 PM, Johannes Sixt <johannes.sixt@telecom.at> wrote:
> If a merge tool was invoked on a conflicted file and the tool completed,
> then the conflicted file was staged automatically. However, the fact that
> the user closed the merge tool cannot be understood as the unequivocal
> sign that the conflict was completely resolved. For example, the user
> could have decided to postpone the resolution of the conflict, or could
> have accidentally closed the tool. We better leave the file unstaged and
> let the user stage it explicitly.

It completely reproduces the logic that git-mergetool uses. Namely, if
the file is unchanged, it asks explicitly, and if the tool returns a
non-zero exit code, it does not stage at all.

You also cannot simply remove merge_add_resolution, because then it
would leave the diff view stale.

Re: [PATCH/RFC 0/2] git-gui: issues with merge tool series

[Alexander Gavrilov]

On Wed, Sep 17, 2008 at 3:40 PM, Johannes Sixt <johannes.sixt@telecom.at> wrote:
> 1. The inability to stage a conflicted file by clicking the icon is
>   *very* disruptive. The new menu entry "Stage Working Copy" is
>   really only a workaround, and it shows.

I can see two ways to fix it:

1) Allow that icon to work only if the diff is currently displayed,
and also ask for confirmation if there are any conflict markers
present.

   Problem: What should it do with modify/delete conflicts, which
don't have any conflict markers?

2) Much harder: implement complete one-click undo. This involves
saving information from the index somewhere, and forcing such items to
remain in the 'staged' list, even if the index isn't different from
the tree version any more.

   By the way, is there a simple way to re-create a conflict file from
the saved multistage index entries?


Probably the best fix is some combination of these two.


> Furthermore, the operations "Use local version" and "Use remote
> version" could be much more useful. They could serve as "merge
> shortcuts": These operations should only use the "local" or "remote"
> part inside the conflict markers, and not also reset the part that
> is not visible. In the current form I found them useful only in the
> modify/delete conflict cases, where I picked the "delete" part.

That was simply a reimplementation of git-mergetool with proper GUI.
It cannot do per-hunk resolution either...


Alexander

Re: [PATCH/RFC 0/2] git-gui: issues with merge tool series

[Johannes Sixt]

On Mittwoch, 17. September 2008, Alexander Gavrilov wrote:
> On Wed, Sep 17, 2008 at 3:40 PM, Johannes Sixt <johannes.sixt@telecom.at> 
wrote:
> > 1. The inability to stage a conflicted file by clicking the icon is
> >   *very* disruptive. The new menu entry "Stage Working Copy" is
> >   really only a workaround, and it shows.
>
> I can see two ways to fix it:
>
> 1) Allow that icon to work only if the diff is currently displayed,
> and also ask for confirmation if there are any conflict markers
> present.

Good.

>    Problem: What should it do with modify/delete conflicts, which
> don't have any conflict markers?

Stage the file, i.e. pick the 'modify' part (perhaps with confirmation). 
Rationale: That's what the user sees in the diff pane. Additionally, keep 
the "Use local/remote version" entries so that the 'delete' part can be 
chosen from the context menu.

> 2) Much harder: implement complete one-click undo. This involves
> saving information from the index somewhere, and forcing such items to
> remain in the 'staged' list, even if the index isn't different from
> the tree version any more.
>
>    By the way, is there a simple way to re-create a conflict file from
> the saved multistage index entries?

There's 0cf8581e (checkout -m: recreate merge when checking out of unmerged 
index) in 'next'. But that does not address how the unmerged entries get into 
the index.

-- Hannes

Re: [PATCH/RFC 0/2] git-gui: issues with merge tool series

[Alexander Gavrilov]

On Thursday 18 September 2008 01:40:16 Johannes Sixt wrote:
> On Mittwoch, 17. September 2008, Alexander Gavrilov wrote:
> >    By the way, is there a simple way to re-create a conflict file from
> > the saved multistage index entries?
> 
> There's 0cf8581e (checkout -m: recreate merge when checking out of unmerged 
> index) in 'next'. But that does not address how the unmerged entries get into 
> the index.

If everything else fails, the entries can be fetched from the index and
cached in RAM by git-gui itself. Then it will be able to put them back
via update-index --index-info.

Alexander

Re: [PATCH/RFC 0/2] git-gui: issues with merge tool series

[Shawn O. Pearce]

Alexander Gavrilov <angavrilov@gmail.com> wrote:
> On Wed, Sep 17, 2008 at 3:40 PM, Johannes Sixt <johannes.sixt@telecom.at> wrote:
> > 1. The inability to stage a conflicted file by clicking the icon is
> >   *very* disruptive. The new menu entry "Stage Working Copy" is
> >   really only a workaround, and it shows.
> 
> I can see two ways to fix it:
> 
> 1) Allow that icon to work only if the diff is currently displayed,
> and also ask for confirmation if there are any conflict markers
> present.
> 
>    Problem: What should it do with modify/delete conflicts, which
> don't have any conflict markers?

I'm inclined to say we should just let the icon work like it used
to, stage the whole working copy.  That's exactly what it does on
a non-merge, and thus is what it should do on a merge.

As a safety feature we can scan the working copy for merge marks if
we are currently in a merge and prompt the user for confirmation
if it looks like they are merging an unmerged working copy.
 
> 2) Much harder: implement complete one-click undo. This involves
> saving information from the index somewhere, and forcing such items to
> remain in the 'staged' list, even if the index isn't different from
> the tree version any more.
> 
>    By the way, is there a simple way to re-create a conflict file from
> the saved multistage index entries?

Run git-merge-file after extracting the stages to temporary files.
You can also jam the stages back into the index with update-index
--index-info.

-- 
Shawn.

Re: [PATCH/RFC 2/2] git-gui: Do not automatically stage file after merge tool finishes

[Shawn O. Pearce]

Alexander Gavrilov <angavrilov@gmail.com> wrote:
> On Wed, Sep 17, 2008 at 3:40 PM, Johannes Sixt <johannes.sixt@telecom.at> wrote:
> > If a merge tool was invoked on a conflicted file and the tool completed,
> > then the conflicted file was staged automatically. However, the fact that
> > the user closed the merge tool cannot be understood as the unequivocal
> > sign that the conflict was completely resolved. For example, the user
> > could have decided to postpone the resolution of the conflict, or could
> > have accidentally closed the tool. We better leave the file unstaged and
> > let the user stage it explicitly.
> 
> It completely reproduces the logic that git-mergetool uses. Namely, if
> the file is unchanged, it asks explicitly, and if the tool returns a
> non-zero exit code, it does not stage at all.
> 
> You also cannot simply remove merge_add_resolution, because then it
> would leave the diff view stale.

Is it just a matter of calling reshow_diff instead?

-- 
Shawn.

[PATCH/RFC 2/2 v2] git-gui: Do not automatically stage file after merge tool finishes

[Johannes Sixt]

If a merge tool was invoked on a conflicted file and the tool completed,
then the conflicted file was staged automatically. However, the fact that
the user closed the merge tool cannot be understood as the unequivocal
sign that the conflict was completely resolved. For example, the user
could have decided to postpone the resolution of the conflict, or could
have accidentally closed the tool. We better leave the file unstaged and
let the user stage it explicitly.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
On Mittwoch, 24. September 2008, Shawn O. Pearce wrote:
> Alexander Gavrilov <angavrilov@gmail.com> wrote:
> > You also cannot simply remove merge_add_resolution, because then it
> > would leave the diff view stale.
>
> Is it just a matter of calling reshow_diff instead?

Indeed. I'd tried do_rescan, but reshow_diff does it, too, and it doesn't
mess with the file lists!

This version of the patch also removes the check whether the merge tool
had modified the file, because now we don't do anything with the file
anyway.

-- Hannes

 lib/mergetool.tcl |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/lib/mergetool.tcl b/lib/mergetool.tcl
index 965cfe4..f292f30 100644
--- a/lib/mergetool.tcl
+++ b/lib/mergetool.tcl
@@ -348,14 +348,6 @@ proc merge_tool_finish {fd} {
 		}
 	}
 
-	# Check the modification time of the target file
-	if {!$failed && [file mtime $mtool_target] eq $mtool_mtime} {
-		if {[ask_popup [mc "File %s unchanged, still accept as resolved?" \
-				[short_path $mtool_target]]] ne {yes}} {
-			set failed 1
-		}
-	}
-
 	# Finish
 	if {$failed} {
 		file rename -force -- $backup $mtool_target
@@ -368,6 +360,6 @@ proc merge_tool_finish {fd} {
 
 		delete_temp_files $mtool_tmpfiles
 
-		merge_add_resolution $mtool_target
+		reshow_diff
 	}
 }
-- 
1.6.0.2.295.g3898d