git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2 v3] mergetool
@ 2007-10-17 17:16 Steffen Prohaska
  2007-10-17 17:16 ` [PATCH 1/2 v3] mergetool: use path to mergetool in config var mergetool.<tool>.path Steffen Prohaska
  2007-10-17 17:20 ` [PATCH 0/2 v3] mergetool Steffen Prohaska
  0 siblings, 2 replies; 10+ messages in thread
From: Steffen Prohaska @ 2007-10-17 17:16 UTC (permalink / raw)
  To: git

So here they are again.

    Steffen

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2 v3] mergetool: use path to mergetool in config var mergetool.<tool>.path
  2007-10-17 17:16 [PATCH 0/2 v3] mergetool Steffen Prohaska
@ 2007-10-17 17:16 ` Steffen Prohaska
  2007-10-17 17:16   ` [PATCH 2/2 v3] mergetool: add support for ECMerge Steffen Prohaska
  2007-10-18  5:27   ` [PATCH 1/2 v3] mergetool: use path to mergetool in config var mergetool.<tool>.path Shawn O. Pearce
  2007-10-17 17:20 ` [PATCH 0/2 v3] mergetool Steffen Prohaska
  1 sibling, 2 replies; 10+ messages in thread
From: Steffen Prohaska @ 2007-10-17 17:16 UTC (permalink / raw)
  To: git; +Cc: Steffen Prohaska

This commit adds a mechanism to provide absolute paths to the
external programs called by 'git mergetool'. A path can be
specified in the configuation variable mergetool.<tool>.path.
The configuration variable is similar to how we name branches
and remotes. It is extensible if we need to specify more details
about a tool.

The mechanism is especially useful on Windows, where external
programs are unlikely to be in PATH.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Documentation/git-mergetool.txt |    6 ++
 git-mergetool.sh                |   97 +++++++++++++++++++++-----------------
 2 files changed, 60 insertions(+), 43 deletions(-)

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 6c32c6d..78b2f43 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -31,6 +31,12 @@ If a merge resolution program is not specified, 'git mergetool'
 will use the configuration variable merge.tool.  If the
 configuration variable merge.tool is not set, 'git mergetool'
 will pick a suitable default.
++
+You can explicitly provide a full path to the tool by setting the
+configuration variable mergetool.<tool>.path. For example, you
+can configure the absolute path to kdiff3 by setting
+mergetool.kdiff3.path. Otherwise, 'git mergetool' assumes the tool
+is available in PATH.
 
 Author
 ------
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 9f4f313..1e4583f 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -192,10 +192,10 @@ merge_file () {
     case "$merge_tool" in
 	kdiff3)
 	    if base_present ; then
-		(kdiff3 --auto --L1 "$path (Base)" --L2 "$path (Local)" --L3 "$path (Remote)" \
+		("$merge_tool_path" --auto --L1 "$path (Base)" --L2 "$path (Local)" --L3 "$path (Remote)" \
 		    -o "$path" -- "$BASE" "$LOCAL" "$REMOTE" > /dev/null 2>&1)
 	    else
-		(kdiff3 --auto --L1 "$path (Local)" --L2 "$path (Remote)" \
+		("$merge_tool_path" --auto --L1 "$path (Local)" --L2 "$path (Remote)" \
 		    -o "$path" -- "$LOCAL" "$REMOTE" > /dev/null 2>&1)
 	    fi
 	    status=$?
@@ -203,35 +203,35 @@ merge_file () {
 	    ;;
 	tkdiff)
 	    if base_present ; then
-		tkdiff -a "$BASE" -o "$path" -- "$LOCAL" "$REMOTE"
+		"$merge_tool_path" -a "$BASE" -o "$path" -- "$LOCAL" "$REMOTE"
 	    else
-		tkdiff -o "$path" -- "$LOCAL" "$REMOTE"
+		"$merge_tool_path" -o "$path" -- "$LOCAL" "$REMOTE"
 	    fi
 	    status=$?
 	    save_backup
 	    ;;
 	meld|vimdiff)
 	    touch "$BACKUP"
-	    $merge_tool -- "$LOCAL" "$path" "$REMOTE"
+	    "$merge_tool_path" -- "$LOCAL" "$path" "$REMOTE"
 	    check_unchanged
 	    save_backup
 	    ;;
 	gvimdiff)
 		touch "$BACKUP"
-		gvimdiff -f -- "$LOCAL" "$path" "$REMOTE"
+		"$merge_tool_path" -f -- "$LOCAL" "$path" "$REMOTE"
 		check_unchanged
 		save_backup
 		;;
 	xxdiff)
 	    touch "$BACKUP"
 	    if base_present ; then
-		xxdiff -X --show-merged-pane \
+		"$merge_tool_path" -X --show-merged-pane \
 		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
 		    -R 'Accel.Search: "Ctrl+F"' \
 		    -R 'Accel.SearchForward: "Ctrl-G"' \
 		    --merged-file "$path" -- "$LOCAL" "$BASE" "$REMOTE"
 	    else
-		xxdiff -X --show-merged-pane \
+		"$merge_tool_path" -X --show-merged-pane \
 		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
 		    -R 'Accel.Search: "Ctrl+F"' \
 		    -R 'Accel.SearchForward: "Ctrl-G"' \
@@ -243,18 +243,18 @@ merge_file () {
 	opendiff)
 	    touch "$BACKUP"
 	    if base_present; then
-		opendiff "$LOCAL" "$REMOTE" -ancestor "$BASE" -merge "$path" | cat
+		"$merge_tool_path" "$LOCAL" "$REMOTE" -ancestor "$BASE" -merge "$path" | cat
 	    else
-		opendiff "$LOCAL" "$REMOTE" -merge "$path" | cat
+		"$merge_tool_path" "$LOCAL" "$REMOTE" -merge "$path" | cat
 	    fi
 	    check_unchanged
 	    save_backup
 	    ;;
 	emerge)
 	    if base_present ; then
-		emacs -f emerge-files-with-ancestor-command "$LOCAL" "$REMOTE" "$BASE" "$(basename "$path")"
+		"$merge_tool_path" -f emerge-files-with-ancestor-command "$LOCAL" "$REMOTE" "$BASE" "$(basename "$path")"
 	    else
-		emacs -f emerge-files-command "$LOCAL" "$REMOTE" "$(basename "$path")"
+		"$merge_tool_path" -f emerge-files-command "$LOCAL" "$REMOTE" "$(basename "$path")"
 	    fi
 	    status=$?
 	    save_backup
@@ -297,17 +297,38 @@ do
     shift
 done
 
+valid_tool() {
+	case "$1" in
+		kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff)
+			;; # happy
+		*)
+			return 1
+			;;
+	esac
+}
+
+init_merge_tool_path() {
+	merge_tool_path=`git config mergetool.$1.path`
+	if test -z "$merge_tool_path" ; then
+		case "$merge_tool" in
+			emerge)
+				merge_tool_path=emacs
+				;;
+			*)
+				merge_tool_path=$merge_tool
+				;;
+		esac
+	fi
+}
+
+
 if test -z "$merge_tool"; then
     merge_tool=`git config merge.tool`
-    case "$merge_tool" in
-	kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | "")
-	    ;; # happy
-	*)
+    test -n "$merge_tool" || valid_tool "$merge_tool" || {
 	    echo >&2 "git config option merge.tool set to unknown tool: $merge_tool"
 	    echo >&2 "Resetting to default..."
 	    unset merge_tool
-	    ;;
-    esac
+    }
 fi
 
 if test -z "$merge_tool" ; then
@@ -329,40 +350,30 @@ if test -z "$merge_tool" ; then
     merge_tool_candidates="$merge_tool_candidates opendiff emerge vimdiff"
     echo "merge tool candidates: $merge_tool_candidates"
     for i in $merge_tool_candidates; do
-        if test $i = emerge ; then
-            cmd=emacs
-        else
-            cmd=$i
-        fi
-        if type $cmd > /dev/null 2>&1; then
+        init_merge_tool_path $i
+        if type "$merge_tool_path" > /dev/null 2>&1; then
             merge_tool=$i
             break
         fi
     done
     if test -z "$merge_tool" ; then
-	echo "No available merge resolution programs available."
+	echo "No known merge resolution program available."
 	exit 1
     fi
+else
+    valid_tool "$merge_tool" || {
+        echo >&2 "Unknown merge_tool $merge_tool"
+        exit 1
+    }
+
+    init_merge_tool_path "$merge_tool"
+
+    if ! type "$merge_tool_path" > /dev/null 2>&1; then
+        echo "The merge tool $merge_tool is not available as '$merge_tool_path'"
+        exit 1
+    fi
 fi
 
-case "$merge_tool" in
-    kdiff3|tkdiff|meld|xxdiff|vimdiff|gvimdiff|opendiff)
-	if ! type "$merge_tool" > /dev/null 2>&1; then
-	    echo "The merge tool $merge_tool is not available"
-	    exit 1
-	fi
-	;;
-    emerge)
-	if ! type "emacs" > /dev/null 2>&1; then
-	    echo "Emacs is not available"
-	    exit 1
-	fi
-	;;
-    *)
-	echo "Unknown merge tool: $merge_tool"
-	exit 1
-	;;
-esac
 
 if test $# -eq 0 ; then
 	files=`git ls-files -u | sed -e 's/^[^	]*	//' | sort -u`
-- 
1.5.3.4.222.g2830

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2 v3] mergetool: add support for ECMerge
  2007-10-17 17:16 ` [PATCH 1/2 v3] mergetool: use path to mergetool in config var mergetool.<tool>.path Steffen Prohaska
@ 2007-10-17 17:16   ` Steffen Prohaska
  2007-10-18  5:27   ` [PATCH 1/2 v3] mergetool: use path to mergetool in config var mergetool.<tool>.path Shawn O. Pearce
  1 sibling, 0 replies; 10+ messages in thread
From: Steffen Prohaska @ 2007-10-17 17:16 UTC (permalink / raw)
  To: git; +Cc: Steffen Prohaska

Add support to mergetool for ECMerge available from
http://www.elliecomputing.com/Products/merge_overview.asp

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Documentation/git-mergetool.txt |    2 +-
 git-mergetool.sh                |   12 +++++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 78b2f43..a26c260 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -25,7 +25,7 @@ OPTIONS
 -t or --tool=<tool>::
 	Use the merge resolution program specified by <tool>.
 	Valid merge tools are:
-	kdiff3, tkdiff, meld, xxdiff, emerge, vimdiff, gvimdiff, and opendiff
+	kdiff3, tkdiff, meld, xxdiff, emerge, vimdiff, gvimdiff, ecmerge, and opendiff
 +
 If a merge resolution program is not specified, 'git mergetool'
 will use the configuration variable merge.tool.  If the
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 1e4583f..fdf4912 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -250,6 +250,16 @@ merge_file () {
 	    check_unchanged
 	    save_backup
 	    ;;
+	ecmerge)
+	    touch "$BACKUP"
+	    if base_present; then
+		"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" --mode=merge3 --to="$path"
+	    else
+		"$merge_tool_path" "$LOCAL" "$REMOTE" --mode=merge2 --to="$path"
+	    fi
+	    check_unchanged
+	    save_backup
+	    ;;
 	emerge)
 	    if base_present ; then
 		"$merge_tool_path" -f emerge-files-with-ancestor-command "$LOCAL" "$REMOTE" "$BASE" "$(basename "$path")"
@@ -299,7 +309,7 @@ done
 
 valid_tool() {
 	case "$1" in
-		kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff)
+		kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge)
 			;; # happy
 		*)
 			return 1
-- 
1.5.3.4.222.g2830

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2 v3] mergetool
  2007-10-17 17:16 [PATCH 0/2 v3] mergetool Steffen Prohaska
  2007-10-17 17:16 ` [PATCH 1/2 v3] mergetool: use path to mergetool in config var mergetool.<tool>.path Steffen Prohaska
@ 2007-10-17 17:20 ` Steffen Prohaska
  1 sibling, 0 replies; 10+ messages in thread
From: Steffen Prohaska @ 2007-10-17 17:20 UTC (permalink / raw)
  To: Git Mailing List


On Oct 17, 2007, at 7:16 PM, Steffen Prohaska wrote:

> So here they are again.

[ half of the message is missing. First time I used compose ;) ]

I haven't found these two patches in Shawn's nor Lars' repo.
I believe they would be pretty useful on Windows. They apply
to spearce/master.

	Steffen

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2 v3] mergetool: use path to mergetool in config var mergetool.<tool>.path
  2007-10-17 17:16 ` [PATCH 1/2 v3] mergetool: use path to mergetool in config var mergetool.<tool>.path Steffen Prohaska
  2007-10-17 17:16   ` [PATCH 2/2 v3] mergetool: add support for ECMerge Steffen Prohaska
@ 2007-10-18  5:27   ` Shawn O. Pearce
  2007-10-18  7:52     ` Steffen Prohaska
  1 sibling, 1 reply; 10+ messages in thread
From: Shawn O. Pearce @ 2007-10-18  5:27 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git

Steffen Prohaska <prohaska@zib.de> wrote:
> This commit adds a mechanism to provide absolute paths to the
> external programs called by 'git mergetool'.  ...
...
> @@ -297,17 +297,38 @@ do
>      shift
>  done
>  
> +valid_tool() {
> +	case "$1" in
> +		kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff)
> +			;; # happy
> +		*)
> +			return 1
> +			;;
> +	esac
> +}
...
> +    test -n "$merge_tool" || valid_tool "$merge_tool" || {

Wouldn't an empty $merge_tool string be caught above in the
valid_tool function where it falls through and returns 1?
So isn't test -n here redundant?

-- 
Shawn.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2 v3] mergetool: use path to mergetool in config var mergetool.<tool>.path
  2007-10-18  5:27   ` [PATCH 1/2 v3] mergetool: use path to mergetool in config var mergetool.<tool>.path Shawn O. Pearce
@ 2007-10-18  7:52     ` Steffen Prohaska
  2007-10-18  7:53       ` [PATCH 1/2 v4] " Steffen Prohaska
  2007-10-18  8:00       ` [PATCH 1/2 v3] " Shawn O. Pearce
  0 siblings, 2 replies; 10+ messages in thread
From: Steffen Prohaska @ 2007-10-18  7:52 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git


On Oct 18, 2007, at 7:27 AM, Shawn O. Pearce wrote:

> Steffen Prohaska <prohaska@zib.de> wrote:
>> This commit adds a mechanism to provide absolute paths to the
>> external programs called by 'git mergetool'.  ...
> ...
>> @@ -297,17 +297,38 @@ do
>>      shift
>>  done
>>
>> +valid_tool() {
>> +	case "$1" in
>> +		kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff |  
>> gvimdiff)
>> +			;; # happy
>> +		*)
>> +			return 1
>> +			;;
>> +	esac
>> +}
> ...
>> +    test -n "$merge_tool" || valid_tool "$merge_tool" || {
>
> Wouldn't an empty $merge_tool string be caught above in the
> valid_tool function where it falls through and returns 1?
> So isn't test -n here redundant?

Sharp eyes, thanks.


Correct is

test -z "$merge_tool" || valid_tool "$merge_tool" || {

No merge tool or a valid merge tool is allowed at this place.
If the tool's already empty there's no need to tell the user
that it will be reset to empty.

I'll send a v4 version.

	Steffen

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2 v4] mergetool: use path to mergetool in config var mergetool.<tool>.path
  2007-10-18  7:52     ` Steffen Prohaska
@ 2007-10-18  7:53       ` Steffen Prohaska
  2007-10-18  8:00       ` [PATCH 1/2 v3] " Shawn O. Pearce
  1 sibling, 0 replies; 10+ messages in thread
From: Steffen Prohaska @ 2007-10-18  7:53 UTC (permalink / raw)
  To: spearce; +Cc: git, Steffen Prohaska

This commit adds a mechanism to provide absolute paths to the
external programs called by 'git mergetool'. A path can be
specified in the configuation variable mergetool.<tool>.path.
The configuration variable is similar to how we name branches
and remotes. It is extensible if we need to specify more details
about a tool.

The mechanism is especially useful on Windows, where external
programs are unlikely to be in PATH.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Documentation/git-mergetool.txt |    6 ++
 git-mergetool.sh                |   97 +++++++++++++++++++++-----------------
 2 files changed, 60 insertions(+), 43 deletions(-)

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 6c32c6d..78b2f43 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -31,6 +31,12 @@ If a merge resolution program is not specified, 'git mergetool'
 will use the configuration variable merge.tool.  If the
 configuration variable merge.tool is not set, 'git mergetool'
 will pick a suitable default.
++
+You can explicitly provide a full path to the tool by setting the
+configuration variable mergetool.<tool>.path. For example, you
+can configure the absolute path to kdiff3 by setting
+mergetool.kdiff3.path. Otherwise, 'git mergetool' assumes the tool
+is available in PATH.
 
 Author
 ------
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 9f4f313..987aeda 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -192,10 +192,10 @@ merge_file () {
     case "$merge_tool" in
 	kdiff3)
 	    if base_present ; then
-		(kdiff3 --auto --L1 "$path (Base)" --L2 "$path (Local)" --L3 "$path (Remote)" \
+		("$merge_tool_path" --auto --L1 "$path (Base)" --L2 "$path (Local)" --L3 "$path (Remote)" \
 		    -o "$path" -- "$BASE" "$LOCAL" "$REMOTE" > /dev/null 2>&1)
 	    else
-		(kdiff3 --auto --L1 "$path (Local)" --L2 "$path (Remote)" \
+		("$merge_tool_path" --auto --L1 "$path (Local)" --L2 "$path (Remote)" \
 		    -o "$path" -- "$LOCAL" "$REMOTE" > /dev/null 2>&1)
 	    fi
 	    status=$?
@@ -203,35 +203,35 @@ merge_file () {
 	    ;;
 	tkdiff)
 	    if base_present ; then
-		tkdiff -a "$BASE" -o "$path" -- "$LOCAL" "$REMOTE"
+		"$merge_tool_path" -a "$BASE" -o "$path" -- "$LOCAL" "$REMOTE"
 	    else
-		tkdiff -o "$path" -- "$LOCAL" "$REMOTE"
+		"$merge_tool_path" -o "$path" -- "$LOCAL" "$REMOTE"
 	    fi
 	    status=$?
 	    save_backup
 	    ;;
 	meld|vimdiff)
 	    touch "$BACKUP"
-	    $merge_tool -- "$LOCAL" "$path" "$REMOTE"
+	    "$merge_tool_path" -- "$LOCAL" "$path" "$REMOTE"
 	    check_unchanged
 	    save_backup
 	    ;;
 	gvimdiff)
 		touch "$BACKUP"
-		gvimdiff -f -- "$LOCAL" "$path" "$REMOTE"
+		"$merge_tool_path" -f -- "$LOCAL" "$path" "$REMOTE"
 		check_unchanged
 		save_backup
 		;;
 	xxdiff)
 	    touch "$BACKUP"
 	    if base_present ; then
-		xxdiff -X --show-merged-pane \
+		"$merge_tool_path" -X --show-merged-pane \
 		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
 		    -R 'Accel.Search: "Ctrl+F"' \
 		    -R 'Accel.SearchForward: "Ctrl-G"' \
 		    --merged-file "$path" -- "$LOCAL" "$BASE" "$REMOTE"
 	    else
-		xxdiff -X --show-merged-pane \
+		"$merge_tool_path" -X --show-merged-pane \
 		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
 		    -R 'Accel.Search: "Ctrl+F"' \
 		    -R 'Accel.SearchForward: "Ctrl-G"' \
@@ -243,18 +243,18 @@ merge_file () {
 	opendiff)
 	    touch "$BACKUP"
 	    if base_present; then
-		opendiff "$LOCAL" "$REMOTE" -ancestor "$BASE" -merge "$path" | cat
+		"$merge_tool_path" "$LOCAL" "$REMOTE" -ancestor "$BASE" -merge "$path" | cat
 	    else
-		opendiff "$LOCAL" "$REMOTE" -merge "$path" | cat
+		"$merge_tool_path" "$LOCAL" "$REMOTE" -merge "$path" | cat
 	    fi
 	    check_unchanged
 	    save_backup
 	    ;;
 	emerge)
 	    if base_present ; then
-		emacs -f emerge-files-with-ancestor-command "$LOCAL" "$REMOTE" "$BASE" "$(basename "$path")"
+		"$merge_tool_path" -f emerge-files-with-ancestor-command "$LOCAL" "$REMOTE" "$BASE" "$(basename "$path")"
 	    else
-		emacs -f emerge-files-command "$LOCAL" "$REMOTE" "$(basename "$path")"
+		"$merge_tool_path" -f emerge-files-command "$LOCAL" "$REMOTE" "$(basename "$path")"
 	    fi
 	    status=$?
 	    save_backup
@@ -297,17 +297,38 @@ do
     shift
 done
 
+valid_tool() {
+	case "$1" in
+		kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff)
+			;; # happy
+		*)
+			return 1
+			;;
+	esac
+}
+
+init_merge_tool_path() {
+	merge_tool_path=`git config mergetool.$1.path`
+	if test -z "$merge_tool_path" ; then
+		case "$merge_tool" in
+			emerge)
+				merge_tool_path=emacs
+				;;
+			*)
+				merge_tool_path=$merge_tool
+				;;
+		esac
+	fi
+}
+
+
 if test -z "$merge_tool"; then
     merge_tool=`git config merge.tool`
-    case "$merge_tool" in
-	kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | "")
-	    ;; # happy
-	*)
+    test -z "$merge_tool" || valid_tool "$merge_tool" || {
 	    echo >&2 "git config option merge.tool set to unknown tool: $merge_tool"
 	    echo >&2 "Resetting to default..."
 	    unset merge_tool
-	    ;;
-    esac
+    }
 fi
 
 if test -z "$merge_tool" ; then
@@ -329,40 +350,30 @@ if test -z "$merge_tool" ; then
     merge_tool_candidates="$merge_tool_candidates opendiff emerge vimdiff"
     echo "merge tool candidates: $merge_tool_candidates"
     for i in $merge_tool_candidates; do
-        if test $i = emerge ; then
-            cmd=emacs
-        else
-            cmd=$i
-        fi
-        if type $cmd > /dev/null 2>&1; then
+        init_merge_tool_path $i
+        if type "$merge_tool_path" > /dev/null 2>&1; then
             merge_tool=$i
             break
         fi
     done
     if test -z "$merge_tool" ; then
-	echo "No available merge resolution programs available."
+	echo "No known merge resolution program available."
 	exit 1
     fi
+else
+    valid_tool "$merge_tool" || {
+        echo >&2 "Unknown merge_tool $merge_tool"
+        exit 1
+    }
+
+    init_merge_tool_path "$merge_tool"
+
+    if ! type "$merge_tool_path" > /dev/null 2>&1; then
+        echo "The merge tool $merge_tool is not available as '$merge_tool_path'"
+        exit 1
+    fi
 fi
 
-case "$merge_tool" in
-    kdiff3|tkdiff|meld|xxdiff|vimdiff|gvimdiff|opendiff)
-	if ! type "$merge_tool" > /dev/null 2>&1; then
-	    echo "The merge tool $merge_tool is not available"
-	    exit 1
-	fi
-	;;
-    emerge)
-	if ! type "emacs" > /dev/null 2>&1; then
-	    echo "Emacs is not available"
-	    exit 1
-	fi
-	;;
-    *)
-	echo "Unknown merge tool: $merge_tool"
-	exit 1
-	;;
-esac
 
 if test $# -eq 0 ; then
 	files=`git ls-files -u | sed -e 's/^[^	]*	//' | sort -u`
-- 
1.5.3.4.222.g2830

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2 v3] mergetool: use path to mergetool in config var mergetool.<tool>.path
  2007-10-18  7:52     ` Steffen Prohaska
  2007-10-18  7:53       ` [PATCH 1/2 v4] " Steffen Prohaska
@ 2007-10-18  8:00       ` Shawn O. Pearce
  2007-10-18  8:40         ` Steffen Prohaska
  1 sibling, 1 reply; 10+ messages in thread
From: Shawn O. Pearce @ 2007-10-18  8:00 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git

Steffen Prohaska <prohaska@zib.de> wrote:
> On Oct 18, 2007, at 7:27 AM, Shawn O. Pearce wrote:
> >...
> >>+    test -n "$merge_tool" || valid_tool "$merge_tool" || {
> >
> >Wouldn't an empty $merge_tool string be caught above in the
> >valid_tool function where it falls through and returns 1?
> >So isn't test -n here redundant?
> 
> Sharp eyes, thanks.
> 
> Correct is
> 
> test -z "$merge_tool" || valid_tool "$merge_tool" || {
> 
> No merge tool or a valid merge tool is allowed at this place.
> If the tool's already empty there's no need to tell the user
> that it will be reset to empty.
> 
> I'll send a v4 version.

Thanks but its a little late.  I'm about to push maint/master/next/pu
and this series is in next.  While testing it I found another
bug related to init_tool_merge_tool_path() not having $merge_tool
initialized and thus causing it to never select a default tool if
the user didn't have one configured.

Here's what I'm actually about to push out:

--8>--
From e3fa2c761fdc490494e8e0855bcee4d7e58ada6a Mon Sep 17 00:00:00 2001
From: Steffen Prohaska <prohaska@zib.de>
Date: Wed, 17 Oct 2007 19:16:11 +0200
Subject: [PATCH] mergetool: use path to mergetool in config var mergetool.<tool>.path

This commit adds a mechanism to provide absolute paths to the
external programs called by 'git mergetool'. A path can be
specified in the configuation variable mergetool.<tool>.path.
The configuration variable is similar to how we name branches
and remotes. It is extensible if we need to specify more details
about a tool.

The mechanism is especially useful on Windows, where external
programs are unlikely to be in PATH.

[sp: Fixed a few minor issues prior to applying]

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Documentation/git-mergetool.txt |    6 ++
 git-mergetool.sh                |   97 +++++++++++++++++++++-----------------
 2 files changed, 60 insertions(+), 43 deletions(-)

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 6c32c6d..78b2f43 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -31,6 +31,12 @@ If a merge resolution program is not specified, 'git mergetool'
 will use the configuration variable merge.tool.  If the
 configuration variable merge.tool is not set, 'git mergetool'
 will pick a suitable default.
++
+You can explicitly provide a full path to the tool by setting the
+configuration variable mergetool.<tool>.path. For example, you
+can configure the absolute path to kdiff3 by setting
+mergetool.kdiff3.path. Otherwise, 'git mergetool' assumes the tool
+is available in PATH.
 
 Author
 ------
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 9f4f313..4f89cbe 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -192,10 +192,10 @@ merge_file () {
     case "$merge_tool" in
 	kdiff3)
 	    if base_present ; then
-		(kdiff3 --auto --L1 "$path (Base)" --L2 "$path (Local)" --L3 "$path (Remote)" \
+		("$merge_tool_path" --auto --L1 "$path (Base)" --L2 "$path (Local)" --L3 "$path (Remote)" \
 		    -o "$path" -- "$BASE" "$LOCAL" "$REMOTE" > /dev/null 2>&1)
 	    else
-		(kdiff3 --auto --L1 "$path (Local)" --L2 "$path (Remote)" \
+		("$merge_tool_path" --auto --L1 "$path (Local)" --L2 "$path (Remote)" \
 		    -o "$path" -- "$LOCAL" "$REMOTE" > /dev/null 2>&1)
 	    fi
 	    status=$?
@@ -203,35 +203,35 @@ merge_file () {
 	    ;;
 	tkdiff)
 	    if base_present ; then
-		tkdiff -a "$BASE" -o "$path" -- "$LOCAL" "$REMOTE"
+		"$merge_tool_path" -a "$BASE" -o "$path" -- "$LOCAL" "$REMOTE"
 	    else
-		tkdiff -o "$path" -- "$LOCAL" "$REMOTE"
+		"$merge_tool_path" -o "$path" -- "$LOCAL" "$REMOTE"
 	    fi
 	    status=$?
 	    save_backup
 	    ;;
 	meld|vimdiff)
 	    touch "$BACKUP"
-	    $merge_tool -- "$LOCAL" "$path" "$REMOTE"
+	    "$merge_tool_path" -- "$LOCAL" "$path" "$REMOTE"
 	    check_unchanged
 	    save_backup
 	    ;;
 	gvimdiff)
 		touch "$BACKUP"
-		gvimdiff -f -- "$LOCAL" "$path" "$REMOTE"
+		"$merge_tool_path" -f -- "$LOCAL" "$path" "$REMOTE"
 		check_unchanged
 		save_backup
 		;;
 	xxdiff)
 	    touch "$BACKUP"
 	    if base_present ; then
-		xxdiff -X --show-merged-pane \
+		"$merge_tool_path" -X --show-merged-pane \
 		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
 		    -R 'Accel.Search: "Ctrl+F"' \
 		    -R 'Accel.SearchForward: "Ctrl-G"' \
 		    --merged-file "$path" -- "$LOCAL" "$BASE" "$REMOTE"
 	    else
-		xxdiff -X --show-merged-pane \
+		"$merge_tool_path" -X --show-merged-pane \
 		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
 		    -R 'Accel.Search: "Ctrl+F"' \
 		    -R 'Accel.SearchForward: "Ctrl-G"' \
@@ -243,18 +243,18 @@ merge_file () {
 	opendiff)
 	    touch "$BACKUP"
 	    if base_present; then
-		opendiff "$LOCAL" "$REMOTE" -ancestor "$BASE" -merge "$path" | cat
+		"$merge_tool_path" "$LOCAL" "$REMOTE" -ancestor "$BASE" -merge "$path" | cat
 	    else
-		opendiff "$LOCAL" "$REMOTE" -merge "$path" | cat
+		"$merge_tool_path" "$LOCAL" "$REMOTE" -merge "$path" | cat
 	    fi
 	    check_unchanged
 	    save_backup
 	    ;;
 	emerge)
 	    if base_present ; then
-		emacs -f emerge-files-with-ancestor-command "$LOCAL" "$REMOTE" "$BASE" "$(basename "$path")"
+		"$merge_tool_path" -f emerge-files-with-ancestor-command "$LOCAL" "$REMOTE" "$BASE" "$(basename "$path")"
 	    else
-		emacs -f emerge-files-command "$LOCAL" "$REMOTE" "$(basename "$path")"
+		"$merge_tool_path" -f emerge-files-command "$LOCAL" "$REMOTE" "$(basename "$path")"
 	    fi
 	    status=$?
 	    save_backup
@@ -297,17 +297,38 @@ do
     shift
 done
 
+valid_tool() {
+	case "$1" in
+		kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff)
+			;; # happy
+		*)
+			return 1
+			;;
+	esac
+}
+
+init_merge_tool_path() {
+	merge_tool_path=`git config mergetool.$1.path`
+	if test -z "$merge_tool_path" ; then
+		case "$1" in
+			emerge)
+				merge_tool_path=emacs
+				;;
+			*)
+				merge_tool_path=$1
+				;;
+		esac
+	fi
+}
+
+
 if test -z "$merge_tool"; then
     merge_tool=`git config merge.tool`
-    case "$merge_tool" in
-	kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | "")
-	    ;; # happy
-	*)
+    if ! valid_tool "$merge_tool"; then
 	    echo >&2 "git config option merge.tool set to unknown tool: $merge_tool"
 	    echo >&2 "Resetting to default..."
 	    unset merge_tool
-	    ;;
-    esac
+    fi
 fi
 
 if test -z "$merge_tool" ; then
@@ -329,40 +350,30 @@ if test -z "$merge_tool" ; then
     merge_tool_candidates="$merge_tool_candidates opendiff emerge vimdiff"
     echo "merge tool candidates: $merge_tool_candidates"
     for i in $merge_tool_candidates; do
-        if test $i = emerge ; then
-            cmd=emacs
-        else
-            cmd=$i
-        fi
-        if type $cmd > /dev/null 2>&1; then
+        init_merge_tool_path $i
+        if type "$merge_tool_path" > /dev/null 2>&1; then
             merge_tool=$i
             break
         fi
     done
     if test -z "$merge_tool" ; then
-	echo "No available merge resolution programs available."
+	echo "No known merge resolution program available."
 	exit 1
     fi
+else
+    if ! valid_tool "$merge_tool"; then
+        echo >&2 "Unknown merge_tool $merge_tool"
+        exit 1
+    fi
+
+    init_merge_tool_path "$merge_tool"
+
+    if ! type "$merge_tool_path" > /dev/null 2>&1; then
+        echo "The merge tool $merge_tool is not available as '$merge_tool_path'"
+        exit 1
+    fi
 fi
 
-case "$merge_tool" in
-    kdiff3|tkdiff|meld|xxdiff|vimdiff|gvimdiff|opendiff)
-	if ! type "$merge_tool" > /dev/null 2>&1; then
-	    echo "The merge tool $merge_tool is not available"
-	    exit 1
-	fi
-	;;
-    emerge)
-	if ! type "emacs" > /dev/null 2>&1; then
-	    echo "Emacs is not available"
-	    exit 1
-	fi
-	;;
-    *)
-	echo "Unknown merge tool: $merge_tool"
-	exit 1
-	;;
-esac
 
 if test $# -eq 0 ; then
 	files=`git ls-files -u | sed -e 's/^[^	]*	//' | sort -u`
-- 
1.5.3.4.1231.gac645


-- 
Shawn.

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2 v3] mergetool: use path to mergetool in config var mergetool.<tool>.path
  2007-10-18  8:00       ` [PATCH 1/2 v3] " Shawn O. Pearce
@ 2007-10-18  8:40         ` Steffen Prohaska
  2007-10-18 10:25           ` [PATCH] mergetool: avoid misleading message "Resetting to default..." Steffen Prohaska
  0 siblings, 1 reply; 10+ messages in thread
From: Steffen Prohaska @ 2007-10-18  8:40 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git


On Oct 18, 2007, at 10:00 AM, Shawn O. Pearce wrote:

> Steffen Prohaska <prohaska@zib.de> wrote:
>> On Oct 18, 2007, at 7:27 AM, Shawn O. Pearce wrote:
>>> ...
>>>> +    test -n "$merge_tool" || valid_tool "$merge_tool" || {
>>>
>>> Wouldn't an empty $merge_tool string be caught above in the
>>> valid_tool function where it falls through and returns 1?
>>> So isn't test -n here redundant?
>>
>> Sharp eyes, thanks.
>>
>> Correct is
>>
>> test -z "$merge_tool" || valid_tool "$merge_tool" || {
>>
>> No merge tool or a valid merge tool is allowed at this place.
>> If the tool's already empty there's no need to tell the user
>> that it will be reset to empty.
>>
>> I'll send a v4 version.
>
> Thanks but its a little late.

Sorry.


> I'm about to push maint/master/next/pu
> and this series is in next.  While testing it I found another
> bug related to init_tool_merge_tool_path() not having $merge_tool
> initialized and thus causing it to never select a default tool if
> the user didn't have one configured.

I see. Thanks for fixing this.


> Here's what I'm actually about to push out:

One remark ...

[...]
>
>  if test -z "$merge_tool"; then
>      merge_tool=`git config merge.tool`
> -    case "$merge_tool" in
> -	kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff |  
> gvimdiff | "")
> -	    ;; # happy
> -	*)
> +    if ! valid_tool "$merge_tool"; then

I think its ok to have an empty tool here ...

>  	    echo >&2 "git config option merge.tool set to unknown tool:  
> $merge_tool"
>  	    echo >&2 "Resetting to default..."

... So we should not print this message if the tool is empty.

But an empty tool is not valid. Therefore a check if "$merge_tool"
is not empty should be added to the if.

	Steffen

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] mergetool: avoid misleading message "Resetting to default..."
  2007-10-18  8:40         ` Steffen Prohaska
@ 2007-10-18 10:25           ` Steffen Prohaska
  0 siblings, 0 replies; 10+ messages in thread
From: Steffen Prohaska @ 2007-10-18 10:25 UTC (permalink / raw)
  To: spearce; +Cc: git, Steffen Prohaska

If no mergetool is configured in the configuration variable
merge.tool the resetting message should not be printed.

This is fixed. The message is only printed if a tool is configured
but the entry in merge.tool is invalid.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 git-mergetool.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 94511f9..a68b403 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -334,7 +334,7 @@ init_merge_tool_path() {
 
 if test -z "$merge_tool"; then
     merge_tool=`git config merge.tool`
-    if ! valid_tool "$merge_tool"; then
+    if test -n "$merge_tool" && ! valid_tool "$merge_tool"; then
 	    echo >&2 "git config option merge.tool set to unknown tool: $merge_tool"
 	    echo >&2 "Resetting to default..."
 	    unset merge_tool
-- 
1.5.3.4.222.g2830

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2007-10-18 10:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-17 17:16 [PATCH 0/2 v3] mergetool Steffen Prohaska
2007-10-17 17:16 ` [PATCH 1/2 v3] mergetool: use path to mergetool in config var mergetool.<tool>.path Steffen Prohaska
2007-10-17 17:16   ` [PATCH 2/2 v3] mergetool: add support for ECMerge Steffen Prohaska
2007-10-18  5:27   ` [PATCH 1/2 v3] mergetool: use path to mergetool in config var mergetool.<tool>.path Shawn O. Pearce
2007-10-18  7:52     ` Steffen Prohaska
2007-10-18  7:53       ` [PATCH 1/2 v4] " Steffen Prohaska
2007-10-18  8:00       ` [PATCH 1/2 v3] " Shawn O. Pearce
2007-10-18  8:40         ` Steffen Prohaska
2007-10-18 10:25           ` [PATCH] mergetool: avoid misleading message "Resetting to default..." Steffen Prohaska
2007-10-17 17:20 ` [PATCH 0/2 v3] mergetool Steffen Prohaska

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).