threads / patch / 15290

patch, 8 partsAdd mergetool functionality to git-gui.

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

## tl;dr

21 messages between Aug 30, 2008 and Sep 24, 2008. Diffs are folded; open one to read it.

replies: 20people: 3as markdown or json

Alexander Gavrilov· Aug 30, 2008, 20:52 UTC · lore

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(-)
Alexander Gavrilov· Aug 30, 2008, 20:54 UTC · re: Alexander Gavrilov · lore

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

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(-)
Show changes to git-gui.sh +8 −0
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
Alexander Gavrilov· Aug 30, 2008, 20:55 UTC · re: Alexander Gavrilov · lore

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

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
Show changes to 2 files +190 −60

git-gui.sh, 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
Alexander Gavrilov· Aug 30, 2008, 20:56 UTC · re: Alexander Gavrilov · lore

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

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(-)
Show changes to 3 files +260 −0

git-gui.sh, lib/mergetool.tcl, lib/option.tcl

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
Alexander Gavrilov· Aug 30, 2008, 20:59 UTC · re: Alexander Gavrilov · lore

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

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(-)
Show changes to lib/mergetool.tcl +27 −0
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
Alexander Gavrilov· Aug 30, 2008, 21:00 UTC · re: Alexander Gavrilov · lore

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

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(-)
Show changes to 4 files +7 −3

git-gui.sh, lib/commit.tcl, lib/diff.tcl, lib/index.tcl

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
Alexander Gavrilov· Aug 30, 2008, 21:02 UTC · re: Alexander Gavrilov · lore

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

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(-)
Show changes to 3 files +109 −39

git-gui.sh, lib/diff.tcl, lib/mergetool.tcl

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
Alexander Gavrilov· Aug 30, 2008, 21:04 UTC · re: Alexander Gavrilov · lore

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

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(-)
Show changes to git-gui.sh +35 −9
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
Alexander Gavrilov· Aug 30, 2008, 21:05 UTC · re: Alexander Gavrilov · lore

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

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(-)
Show changes to lib/diff.tcl +89 −5
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
Johannes Sixt· Sep 8, 2008, 12:10 UTC · re: Alexander Gavrilov · lore

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

Alexander Gavrilov schrieb:
Show 9 quoted lines
> 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
Alexander Gavrilov· Sep 8, 2008, 12:25 UTC · re: Johannes Sixt · lore

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

Show 14 quoted lines
> 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
Johannes Sixt· Sep 17, 2008, 11:40 UTC · re: Alexander Gavrilov · lore

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

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

Johannes Sixt· Sep 17, 2008, 11:40 UTC · re: Johannes Sixt · lore

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

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(-)
Show changes to 2 files +0 −16

git-gui.sh, lib/mergetool.tcl

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
Johannes Sixt· Sep 17, 2008, 11:40 UTC · re: Johannes Sixt · lore

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

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(-)
Show changes to lib/mergetool.tcl +0 −2
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
Alexander Gavrilov· Sep 17, 2008, 12:25 UTC · re: Johannes Sixt · lore

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

On Wed, Sep 17, 2008 at 3:40 PM, Johannes Sixt <johannes.sixt@telecom.at> wrote:
Show 7 quoted lines
> 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.

Shawn O. Pearce· Sep 24, 2008, 17:50 UTC · re: Alexander Gavrilov · lore

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

Alexander Gavrilov <angavrilov@gmail.com> wrote:
Show 15 quoted lines
> 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.
Johannes Sixt· Sep 24, 2008, 19:08 UTC · re: Shawn O. Pearce · lore

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

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:
Show 5 quoted lines
> 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(-)
Show changes to lib/mergetool.tcl +1 −9
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
Alexander Gavrilov· Sep 17, 2008, 12:50 UTC · re: Johannes Sixt · lore

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

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.
Show 6 quoted lines
> 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
Johannes Sixt· Sep 17, 2008, 21:40 UTC · re: Alexander Gavrilov · lore

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

On Mittwoch, 17. September 2008, Alexander Gavrilov wrote:
> On Wed, Sep 17, 2008 at 3:40 PM, Johannes Sixt <johannes.sixt@telecom.at> 
wrote:
Show 9 quoted lines
> > 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.
Show 7 quoted lines
> 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
Alexander Gavrilov· Sep 17, 2008, 22:24 UTC · re: Johannes Sixt · lore

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

On Thursday 18 September 2008 01:40:16 Johannes Sixt wrote:
Show 7 quoted lines
> 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
Shawn O. Pearce· Sep 24, 2008, 17:48 UTC · re: Alexander Gavrilov · lore

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

Alexander Gavrilov <angavrilov@gmail.com> wrote:
Show 13 quoted lines
> 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.
Show 7 quoted lines
> 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.

← back to recent threads