[PATCH 6/8 v2] sh-tools: add a run_merge_tool function

[PATCH 6/8 v2] sh-tools: add a run_merge_tool function

[David Aguilar]

This function launches merge tools and will be used to refactor
git-(diff|merge)tool.

Signed-off-by: David Aguilar <davvid@gmail.com>
---

This fixes the problems spotted by Markus.

 Documentation/git-sh-tools.txt |    3 +
 git-sh-tools.sh                |  160 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 154 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-sh-tools.txt b/Documentation/git-sh-tools.txt
index 055a10c..68d1b37 100644
--- a/Documentation/git-sh-tools.txt
+++ b/Documentation/git-sh-tools.txt
@@ -36,6 +36,9 @@ init_merge_tool_path::
 	sets up `$merge_tool_path` according to '(diff|merge)tool.<tool>.path'
 	configurations.
 
+run_merge_tool::
+	runs the specified merge tool.
+
 Author
 ------
 Written by David Aguilar <davvid@gmail.com>
diff --git a/git-sh-tools.sh b/git-sh-tools.sh
index 234bac7..e8593fc 100644
--- a/git-sh-tools.sh
+++ b/git-sh-tools.sh
@@ -2,7 +2,12 @@
 # Built-in merge tools are always valid.
 valid_tool() {
 	case "$1" in
-	kdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge)
+	kompare)
+		if mergetool_mode; then
+			return 1
+		fi
+		;; # happy
+	kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge)
 		;; # happy
 	*)
 		if ! valid_custom_tool "$1"; then
@@ -12,29 +17,34 @@ valid_tool() {
 	esac
 }
 
+# Test whether we're in merge mode
+mergetool_mode()
+{
+	test "$TOOL_MODE" = "merge"
+}
+
 # Verifies that (difftool|mergetool).<tool>.cmd exists
 # Requires $TOOL_MODE to be set.
 valid_custom_tool() {
-	if test "$TOOL_MODE" = "diff"; then
-		merge_tool_cmd="$(git config difftool.$1.cmd)"
-		test -z "$merge_tool_cmd" &&
+	if mergetool_mode; then
 		merge_tool_cmd="$(git config mergetool.$1.cmd)"
 		test -n "$merge_tool_cmd"
-	elif test "$TOOL_MODE" = "merge"; then
+	else
+		merge_tool_cmd="$(git config difftool.$1.cmd)"
+		test -z "$merge_tool_cmd" &&
 		merge_tool_cmd="$(git config mergetool.$1.cmd)"
 		test -n "$merge_tool_cmd"
 	fi
 }
 
-
 # Set up $merge_tool_path for (diff|merge)tool.<tool>.path configurations
 init_merge_tool_path() {
-	if test "$TOOL_MODE" = "diff"; then
+	if mergetool_mode; then
+		merge_tool_path=$(git config mergetool."$1".path)
+	else
 		merge_tool_path=$(git config difftool."$1".path)
 		test -z "$merge_tool_path" &&
 		merge_tool_path=$(git config mergetool."$1".path)
-	elif test "$TOOL_MODE" = "merge"; then
-		merge_tool_path=$(git config mergetool."$1".path)
 	fi
 
 	if test -z "$merge_tool_path" ; then
@@ -48,3 +58,135 @@ init_merge_tool_path() {
 		esac
 	fi
 }
+
+# Runs a side-by-side merge tool
+run_merge_tool()
+{
+	merge_tool="$1"
+
+	# base_present is always false when !mergetool_mode
+	case "$merge_tool" in
+	kdiff3)
+		if mergetool_mode; then
+			base=Base
+			local=Local
+			remote=Remote
+		else
+			base=A
+			local=A
+			remote=B
+		fi
+		if base_present; then
+			("$merge_tool_path" --auto \
+				--L1 "$MERGED ($base)" \
+				--L2 "$MERGED ($local)" \
+				--L3 "$MERGED ($remote)" \
+				-o "$MERGED" "$BASE" "$LOCAL" "$REMOTE" \
+			 > /dev/null 2>&1)
+		else
+			("$merge_tool_path" --auto \
+				--L1 "$MERGED ($local)" \
+				--L2 "$MERGED ($remote)" \
+				-o "$MERGED" "$LOCAL" "$REMOTE" \
+			 > /dev/null 2>&1)
+		fi
+		status=$?
+		;;
+
+	kompare)
+		"$merge_tool_path" "$LOCAL" "$REMOTE"
+		status=$?
+		;;
+
+	tkdiff)
+		if base_present; then
+			"$merge_tool_path" -a "$BASE" -o "$MERGED" "$LOCAL" "$REMOTE"
+		else
+			"$merge_tool_path" -o "$MERGED" "$LOCAL" "$REMOTE"
+		fi
+		status=$?
+		;;
+	meld)
+		mergetool_mode && touch "$BACKUP"
+		"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+		mergetool_mode && check_unchanged
+		;;
+	vimdiff)
+		mergetool_mode && touch "$BACKUP"
+		"$merge_tool_path" -c "wincmd l" "$LOCAL" "$MERGED" "$REMOTE"
+		mergetool_mode && check_unchanged
+		;;
+	gvimdiff)
+		mergetool_mode && touch "$BACKUP"
+		"$merge_tool_path" -c "wincmd l" -f "$LOCAL" "$MERGED" "$REMOTE"
+		mergetool_mode && check_unchanged
+		;;
+	xxdiff)
+		if mergetool_mode; then
+			touch "$BACKUP"
+			xtra_args='--show-merged-pane'
+		else
+			xtra_args=
+		fi
+		if base_present; then
+			"$merge_tool_path" -X $xtra_args \
+				-R 'Accel.SaveAsMerged: "Ctrl-S"' \
+				-R 'Accel.Search: "Ctrl+F"' \
+				-R 'Accel.SearchForward: "Ctrl-G"' \
+				--merged-file "$MERGED" "$LOCAL" "$BASE" "$REMOTE"
+		else
+			"$merge_tool_path" -X $xtra_args \
+				-R 'Accel.SaveAsMerged: "Ctrl-S"' \
+				-R 'Accel.Search: "Ctrl+F"' \
+				-R 'Accel.SearchForward: "Ctrl-G"' \
+				--merged-file "$MERGED" "$LOCAL" "$REMOTE"
+		fi
+		mergetool_mode && check_unchanged
+		;;
+	opendiff)
+		mergetool_mode && touch "$BACKUP"
+		if base_present; then
+			"$merge_tool_path" "$LOCAL" "$REMOTE" \
+				-ancestor "$BASE" -merge "$MERGED" | cat
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" \
+				-merge "$MERGED" | cat
+		fi
+		mergetool_mode && check_unchanged
+		;;
+	ecmerge)
+		mergetool_mode && touch "$BACKUP"
+		if base_present; then
+			"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
+				--default --mode=merge3 --to="$MERGED"
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" \
+				--default --mode=merge2 --to="$MERGED"
+		fi
+		mergetool_mode && check_unchanged
+		;;
+	emerge)
+		if base_present ; then
+			"$merge_tool_path" -f emerge-files-with-ancestor-command \
+				"$LOCAL" "$REMOTE" "$BASE" "$(basename "$MERGED")"
+		else
+			"$merge_tool_path" -f emerge-files-command \
+				"$LOCAL" "$REMOTE" "$(basename "$MERGED")"
+		fi
+		status=$?
+		;;
+	*)
+		if test -n "$merge_tool_cmd"; then
+			if test "$merge_tool_trust_exit_code" = "false"; then
+				mergetool_mode && touch "$BACKUP"
+				( eval $merge_tool_cmd )
+				mergetool_mode && check_unchanged
+			else
+				( eval $merge_tool_cmd )
+				status=$?
+			fi
+		fi
+		;;
+	esac
+	return $status
+}
-- 
1.6.2.1.303.g63699

Re: [PATCH 6/8 v2] sh-tools: add a run_merge_tool function

[James Pickens]

On Mon, Mar 30, 2009 at 1:11 AM, David Aguilar <davvid@gmail.com> wrote:
> This function launches merge tools and will be used to refactor
> git-(diff|merge)tool.

Thanks for writing difftool; I find it quite useful.  I tried it with
tkdiff, and noticed that it shows the 'merge preview' window even though it
isn't doing a merge.  If a user with unstaged changes were to carelessly
click the 'save and exit' button, his changes could be lost.  So I think
it's a good idea to stop the merge preview window from showing up under
difftool.  To do that I think you just have to remove the '-o "$MERGED"'
option to tkdiff.

James

Re: [PATCH 6/8 v2] sh-tools: add a run_merge_tool function

[Markus Heidelberg]

James Pickens, 31.03.2009:
> On Mon, Mar 30, 2009 at 1:11 AM, David Aguilar <davvid@gmail.com> wrote:
> > This function launches merge tools and will be used to refactor
> > git-(diff|merge)tool.
> 
> Thanks for writing difftool; I find it quite useful.  I tried it with
> tkdiff, and noticed that it shows the 'merge preview' window even though it
> isn't doing a merge.  If a user with unstaged changes were to carelessly
> click the 'save and exit' button, his changes could be lost.  So I think
> it's a good idea to stop the merge preview window from showing up under
> difftool.  To do that I think you just have to remove the '-o "$MERGED"'
> option to tkdiff.

This mail made me see an issue with your patch series. Sorry, I haven't
seen this earlier, my review was just scratching the surface, I merely
applied it and looked through it, but didn't actually test it. Lack of
time.

The invocations seem to be appropriate only for mergetool, it is just
the invocations from the old git-mergetool.sh, not from the old
git-difftool-helper.sh. This means, git-difftool opens 3 files instead
of 2.

I think there are preset diff tools, which opened 3 files instead of 2
before this series (I just tested kdiff3, it opened 3 files). Seems to
be originated from the fact, that they were initially copied from
git-mergetool.sh.

Markus

Re: [PATCH 6/8 v2] sh-tools: add a run_merge_tool function

[David Aguilar]

On  0, Markus Heidelberg <markus.heidelberg@web.de> wrote:
> James Pickens, 31.03.2009:
> > On Mon, Mar 30, 2009 at 1:11 AM, David Aguilar <davvid@gmail.com> wrote:
> > > This function launches merge tools and will be used to refactor
> > > git-(diff|merge)tool.

sorry this whole series is being rewriten...


> > 
> > Thanks for writing difftool; I find it quite useful.  I tried it with
> > tkdiff, and noticed that it shows the 'merge preview' window even though it
> > isn't doing a merge.  If a user with unstaged changes were to carelessly
> > click the 'save and exit' button, his changes could be lost.  So I think
> > it's a good idea to stop the merge preview window from showing up under
> > difftool.  To do that I think you just have to remove the '-o "$MERGED"'
> > option to tkdiff.
> 
> This mail made me see an issue with your patch series. Sorry, I haven't
> seen this earlier, my review was just scratching the surface, I merely
> applied it and looked through it, but didn't actually test it. Lack of
> time.
> 
> The invocations seem to be appropriate only for mergetool, it is just
> the invocations from the old git-mergetool.sh, not from the old
> git-difftool-helper.sh. This means, git-difftool opens 3 files instead
> of 2.
> 
> I think there are preset diff tools, which opened 3 files instead of 2
> before this series (I just tested kdiff3, it opened 3 files). Seems to
> be originated from the fact, that they were initially copied from
> git-mergetool.sh.
> 
> Markus
> 

-- 
		David

Re: [PATCH 6/8 v2] sh-tools: add a run_merge_tool function

[David Aguilar]

On  0, James Pickens <jepicken@gmail.com> wrote:
> On Mon, Mar 30, 2009 at 1:11 AM, David Aguilar <davvid@gmail.com> wrote:
> > This function launches merge tools and will be used to refactor
> > git-(diff|merge)tool.
> 
> Thanks for writing difftool; I find it quite useful.  I tried it with
> tkdiff, and noticed that it shows the 'merge preview' window even though it
> isn't doing a merge.  If a user with unstaged changes were to carelessly
> click the 'save and exit' button, his changes could be lost.  So I think
> it's a good idea to stop the merge preview window from showing up under
> difftool.  To do that I think you just have to remove the '-o "$MERGED"'
> option to tkdiff.
> 
> James

Hi James

I included your suggestion in my latest patch series.
Once things settle down with the current series I'll
add diffuse to {diff,merge}tool and friends.

So, yup, this shouldn't be an issue in future versions of
difftool.

Thanks

-- 

	David