[GIT PULL] git mergetool fixes

[GIT PULL] git mergetool fixes

[Theodore Ts'o]

Hi Junio,

Please pull from:

        git://repo.or.cz/git/mergetool.git

To receive the following fixes (see below).  They are mostly cleanups
and portability fixes.  They also include Arjen Laarjoven's support
for Apple Mac OS X's opendiff GUI merge tool.

----

Theodore Ts'o (11):
      Fix minor formatting issue in man page for git-mergetool
      mergetool: Replace use of "echo -n" with printf(3) to be more portable
      mergetool: Don't error out in the merge case where the local file is deleted
      mergetool: portability fix: don't assume true is in /bin
      mergetool: portability fix: don't use reserved word function
      mergetool: factor out common code
      mergetool: Remove spurious error message if merge.tool config option not set
      mergetool: Fix abort command when resolving symlinks and deleted files
      mergetool: Add support for Apple Mac OS X's opendiff command
      mergetool: Make git-rm quiet when resolving a deleted file conflict
      mergetool: Clean up description of files and prompts for merge resolutions

                                                - Ted

[PATCH] Fix minor formatting issue in man page for git-mergetool

[Theodore Ts'o]

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 Documentation/git-mergetool.txt |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 5baaaca..34288fe 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -26,11 +26,11 @@ OPTIONS
 	Use the merge resolution program specified by <tool>.
 	Valid merge tools are:
 	kdiff3, tkdiff, meld, xxdiff, emerge, and vimdiff.
-
-	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.
++
+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.
 
 Author
 ------
-- 
1.5.1.rc2.1.g8afe-dirty

[PATCH] mergetool: Replace use of "echo -n" with printf(3) to be more portable

[Theodore Ts'o]

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 git-mergetool.sh |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 7942fd0..3cc428c 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -44,18 +44,18 @@ function describe_file () {
     branch="$2"
     file="$3"
 
-    echo -n "    "
+    printf "    "
     if test -z "$mode"; then
-	echo -n "'$path' was deleted"
+	printf  "'$path' was deleted"
     elif is_symlink "$mode" ; then
-	echo -n "'$path' is a symlink containing '"
+	printf "'$path' is a symlink containing '"
 	cat "$file"
-	echo -n "'"
+	printf "'"
     else
 	if base_present; then
-	    echo -n "'$path' was created"
+	    printf "'%s' was created" "$path"
 	else
-	    echo -n "'$path' was modified"
+	    printf "'%s' was modified" "$path"
 	fi
     fi
     echo " in the $branch branch"
@@ -64,7 +64,7 @@ function describe_file () {
 
 resolve_symlink_merge () {
     while /bin/true; do
-	echo -n "Use (r)emote or (l)ocal, or (a)bort? "
+	printf "Use (r)emote or (l)ocal, or (a)bort? "
 	read ans
 	case "$ans" in
 	    [lL]*)
@@ -88,7 +88,7 @@ resolve_symlink_merge () {
 
 resolve_deleted_merge () {
     while /bin/true; do
-	echo -n "Use (m)odified or (d)eleted file, or (a)bort? "
+	printf "Use (m)odified or (d)eleted file, or (a)bort? "
 	read ans
 	case "$ans" in
 	    [mM]*)
@@ -157,7 +157,7 @@ merge_file () {
     echo "Normal merge conflict for $path:"
     describe_file "$local_mode" "local" "$LOCAL"
     describe_file "$remote_mode" "remote" "$REMOTE"
-    echo -n "Hit return to start merge resolution tool ($merge_tool): "
+    printf "Hit return to start merge resolution tool ($merge_tool): "
     read ans
 
     case "$merge_tool" in
@@ -193,7 +193,7 @@ merge_file () {
 	    else
 		while true; do
 		    echo "$path seems unchanged."
-		    echo -n "Was the merge successful? [y/n] "
+		    printf "Was the merge successful? [y/n] "
 		    read answer < /dev/tty
 		    case "$answer" in
 			y*|Y*) status=0; break ;;
@@ -225,7 +225,7 @@ merge_file () {
 	    else
 		while true; do
 		    echo "$path seems unchanged."
-		    echo -n "Was the merge successful? [y/n] "
+		    printf "Was the merge successful? [y/n] "
 		    read answer < /dev/tty
 		    case "$answer" in
 			y*|Y*) status=0; break ;;
@@ -346,12 +346,12 @@ if test $# -eq 0 ; then
 	echo Merging the files: $files
 	git ls-files -u | sed -e 's/^[^	]*	//' | sort -u | while read i
 	do
-		echo ""
+		printf "\n"
 		merge_file "$i" < /dev/tty > /dev/tty
 	done
 else
 	while test $# -gt 0; do
-		echo ""
+		printf "\n"
 		merge_file "$1"
 		shift
 	done
-- 
1.5.1.rc2.1.g8afe-dirty

[PATCH] mergetool: Don't error out in the merge case where the local file is deleted

[Theodore Ts'o]

If the file we are trying to merge resolve is in git-ls-files -u, then
skip the file existence test.  If the file isn't reported in
git-ls-files, then check to see if the file exists or not to give an
appropriate error message.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 git-mergetool.sh |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 3cc428c..9d959a9 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -111,14 +111,13 @@ resolve_deleted_merge () {
 merge_file () {
     path="$1"
 
-    if test ! -f "$path" ; then
-	echo "$path: file not found"
-	exit 1
-    fi
-
     f=`git-ls-files -u -- "$path"`
     if test -z "$f" ; then
-	echo "$path: file does not need merging"
+        if test ! -f "$path" ; then
+	    echo "$path: file not found"
+	else
+	    echo "$path: file does not need merging"
+	fi
 	exit 1
     fi
 
-- 
1.5.1.rc2.1.g8afe-dirty

[PATCH] mergetool: portability fix: don't assume true is in /bin

[Theodore Ts'o]

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 git-mergetool.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 9d959a9..7d2dadd 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -63,7 +63,7 @@ function describe_file () {
 
 
 resolve_symlink_merge () {
-    while /bin/true; do
+    while true; do
 	printf "Use (r)emote or (l)ocal, or (a)bort? "
 	read ans
 	case "$ans" in
@@ -87,7 +87,7 @@ resolve_symlink_merge () {
 }
 
 resolve_deleted_merge () {
-    while /bin/true; do
+    while true; do
 	printf "Use (m)odified or (d)eleted file, or (a)bort? "
 	read ans
 	case "$ans" in
-- 
1.5.1.rc2.1.g8afe-dirty

[PATCH] mergetool: portability fix: don't use reserved word function

[Theodore Ts'o]

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 git-mergetool.sh |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 7d2dadd..8a87f5e 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -14,19 +14,19 @@ SUBDIRECTORY_OK=Yes
 require_work_tree
 
 # Returns true if the mode reflects a symlink
-function is_symlink () {
+is_symlink () {
     test "$1" = 120000
 }
 
-function local_present () {
+local_present () {
     test -n "$local_mode"
 }
 
-function remote_present () {
+remote_present () {
     test -n "$remote_mode"
 }
 
-function base_present () {
+base_present () {
     test -n "$base_mode"
 }
 
@@ -39,7 +39,7 @@ cleanup_temp_files () {
     fi
 }
 
-function describe_file () {
+describe_file () {
     mode="$1"
     branch="$2"
     file="$3"
-- 
1.5.1.rc2.1.g8afe-dirty

[PATCH] mergetool: factor out common code

[Theodore Ts'o]

Create common function check_unchanged(), save_backup() and
remove_backup().

Also fix some minor whitespace issues while we're at it.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 git-mergetool.sh |   80 +++++++++++++++++++++++++-----------------------------
 1 files changed, 37 insertions(+), 43 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 8a87f5e..aeb32ef 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -46,7 +46,7 @@ describe_file () {
 
     printf "    "
     if test -z "$mode"; then
-	printf  "'$path' was deleted"
+	printf "'$path' was deleted"
     elif is_symlink "$mode" ; then
 	printf "'$path' is a symlink containing '"
 	cat "$file"
@@ -108,12 +108,40 @@ resolve_deleted_merge () {
 	done
 }
 
+check_unchanged () {
+    if test "$path" -nt "$BACKUP" ; then
+	status=0;
+    else
+	while true; do
+	    echo "$path seems unchanged."
+	    printf "Was the merge successful? [y/n] "
+	    read answer < /dev/tty
+	    case "$answer" in
+		y*|Y*) status=0; break ;;
+		n*|N*) status=1; break ;;
+	    esac
+	done
+    fi
+}
+
+save_backup () {
+    if test "$status" -eq 0; then
+	mv -- "$BACKUP" "$path.orig"
+    fi
+}
+
+remove_backup () {
+    if test "$status" -eq 0; then
+	rm "$BACKUP"
+    fi
+}
+
 merge_file () {
     path="$1"
 
     f=`git-ls-files -u -- "$path"`
     if test -z "$f" ; then
-        if test ! -f "$path" ; then
+	if test ! -f "$path" ; then
 	    echo "$path: file not found"
 	else
 	    echo "$path: file does not need merging"
@@ -169,9 +197,7 @@ merge_file () {
 		    -o "$path" -- "$LOCAL" "$REMOTE" > /dev/null 2>&1)
 	    fi
 	    status=$?
-	    if test "$status" -eq 0; then
-		rm "$BACKUP"
-	    fi
+	    remove_backup
 	    ;;
 	tkdiff)
 	    if base_present ; then
@@ -180,29 +206,13 @@ merge_file () {
 		tkdiff -o "$path" -- "$LOCAL" "$REMOTE"
 	    fi
 	    status=$?
-	    if test "$status" -eq 0; then
-		mv -- "$BACKUP" "$path.orig"
-	    fi
+	    save_backup
 	    ;;
 	meld|vimdiff)
 	    touch "$BACKUP"
 	    $merge_tool -- "$LOCAL" "$path" "$REMOTE"
-	    if test "$path" -nt "$BACKUP" ; then
-		status=0;
-	    else
-		while true; do
-		    echo "$path seems unchanged."
-		    printf "Was the merge successful? [y/n] "
-		    read answer < /dev/tty
-		    case "$answer" in
-			y*|Y*) status=0; break ;;
-			n*|N*) status=1; break ;;
-		    esac
-		done
-	    fi
-	    if test "$status" -eq 0; then
-		mv -- "$BACKUP" "$path.orig"
-	    fi
+	    check_unchanged
+	    save_backup
 	    ;;
 	xxdiff)
 	    touch "$BACKUP"
@@ -219,22 +229,8 @@ merge_file () {
 		    -R 'Accel.SearchForward: "Ctrl-G"' \
 		    --merged-file "$path" -- "$LOCAL" "$REMOTE"
 	    fi
-	    if test "$path" -nt "$BACKUP" ; then
-		status=0;
-	    else
-		while true; do
-		    echo "$path seems unchanged."
-		    printf "Was the merge successful? [y/n] "
-		    read answer < /dev/tty
-		    case "$answer" in
-			y*|Y*) status=0; break ;;
-			n*|N*) status=1; break ;;
-		    esac
-		done
-	    fi
-	    if test "$status" -eq 0; then
-		mv -- "$BACKUP" "$path.orig"
-	    fi
+	    check_unchanged
+	    save_backup
 	    ;;
 	emerge)
 	    if base_present ; then
@@ -243,9 +239,7 @@ merge_file () {
 		emacs -f emerge-files-command "$LOCAL" "$REMOTE" "$path"
 	    fi
 	    status=$?
-	    if test "$status" -eq 0; then
-		mv -- "$BACKUP" "$path.orig"
-	    fi
+	    save_backup
 	    ;;
     esac
     if test "$status" -ne 0; then
-- 
1.5.1.rc2.1.g8afe-dirty

[PATCH] mergetool: Remove spurious error message if merge.tool config option not set

[Theodore Ts'o]

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 git-mergetool.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index aeb32ef..5de2433 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -282,7 +282,7 @@ done
 if test -z "$merge_tool"; then
     merge_tool=`git-config merge.tool`
     case "$merge_tool" in
-	kdiff3 | tkdiff | xxdiff | meld | emerge | vimdiff)
+	kdiff3 | tkdiff | xxdiff | meld | emerge | vimdiff | "")
 	    ;; # happy
 	*)
 	    echo >&2 "git config option merge.tool set to unknown tool: $merge_tool"
-- 
1.5.1.rc2.1.g8afe-dirty

[PATCH] mergetool: Fix abort command when resolving symlinks and deleted files

[Theodore Ts'o]

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 git-mergetool.sh |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 5de2433..f73072a 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -73,13 +73,13 @@ resolve_symlink_merge () {
 		cleanup_temp_files --save-backup
 		return
 		;;
-	   [rR]*)
+	    [rR]*)
 		git-checkout-index -f --stage=3 -- "$path"
 		git-add -- "$path"
 		cleanup_temp_files --save-backup
 		return
 		;;
-	    [qQ]*)
+	    [aA]*)
 		exit 1
 		;;
 	    esac
@@ -96,12 +96,12 @@ resolve_deleted_merge () {
 		cleanup_temp_files --save-backup
 		return
 		;;
-	   [dD]*)
+	    [dD]*)
 		git-rm -- "$path"
 		cleanup_temp_files
 		return
 		;;
-	    [qQ]*)
+	    [aA]*)
 		exit 1
 		;;
 	    esac
-- 
1.5.1.rc2.1.g8afe-dirty

[PATCH] mergetool: Add support for Apple Mac OS X's opendiff command

[Theodore Ts'o]

Signed-off-by: Arjen Laarhoven <arjen@yaph.org>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 git-mergetool.sh |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index f73072a..b0d28fd 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -232,6 +232,16 @@ merge_file () {
 	    check_unchanged
 	    save_backup
 	    ;;
+	opendiff)
+	    touch "$BACKUP"
+	    if base_present; then
+		opendiff "$LOCAL" "$REMOTE" -ancestor "$BASE" -merge "$path" | cat
+	    else
+		opendiff "$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" "$path"
@@ -282,7 +292,7 @@ done
 if test -z "$merge_tool"; then
     merge_tool=`git-config merge.tool`
     case "$merge_tool" in
-	kdiff3 | tkdiff | xxdiff | meld | emerge | vimdiff | "")
+	kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | "")
 	    ;; # happy
 	*)
 	    echo >&2 "git config option merge.tool set to unknown tool: $merge_tool"
@@ -301,6 +311,8 @@ if test -z "$merge_tool" ; then
 	merge_tool=xxdiff
     elif type meld >/dev/null 2>&1 && test -n "$DISPLAY"; then
 	merge_tool=meld
+    elif type opendiff >/dev/null 2>&1; then
+	merge_tool=opendiff
     elif type emacs >/dev/null 2>&1; then
 	merge_tool=emerge
     elif type vimdiff >/dev/null 2>&1; then
@@ -312,7 +324,7 @@ if test -z "$merge_tool" ; then
 fi
 
 case "$merge_tool" in
-    kdiff3|tkdiff|meld|xxdiff|vimdiff)
+    kdiff3|tkdiff|meld|xxdiff|vimdiff|opendiff)
 	if ! type "$merge_tool" > /dev/null 2>&1; then
 	    echo "The merge tool $merge_tool is not available"
 	    exit 1
-- 
1.5.1.rc2.1.g8afe-dirty

[PATCH] mergetool: Make git-rm quiet when resolving a deleted file conflict

[Theodore Ts'o]

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 git-mergetool.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index b0d28fd..b9d81f5 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -97,7 +97,7 @@ resolve_deleted_merge () {
 		return
 		;;
 	    [dD]*)
-		git-rm -- "$path"
+		git-rm -- "$path" > /dev/null
 		cleanup_temp_files
 		return
 		;;
-- 
1.5.1.rc2.1.g8afe-dirty

[PATCH] mergetool: Clean up description of files and prompts for merge resolutions

[Theodore Ts'o]

This fixes complaints from Junio for how messages and prompts are
printed when resolving symlink and deleted file merges.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 git-mergetool.sh |   29 +++++++++++++++--------------
 1 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index b9d81f5..9b824ac 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -44,27 +44,24 @@ describe_file () {
     branch="$2"
     file="$3"
 
-    printf "    "
+    printf "  {$branch}: "
     if test -z "$mode"; then
-	printf "'$path' was deleted"
+	echo "deleted"
     elif is_symlink "$mode" ; then
-	printf "'$path' is a symlink containing '"
-	cat "$file"
-	printf "'"
+	echo "a symbolic link -> '$(cat "$file")'"
     else
 	if base_present; then
-	    printf "'%s' was created" "$path"
+	    echo "modified"
 	else
-	    printf "'%s' was modified" "$path"
+	    echo "created"
 	fi
     fi
-    echo " in the $branch branch"
 }
 
 
 resolve_symlink_merge () {
     while true; do
-	printf "Use (r)emote or (l)ocal, or (a)bort? "
+	printf "Use (l)ocal or (r)emote, or (a)bort? "
 	read ans
 	case "$ans" in
 	    [lL]*)
@@ -88,10 +85,14 @@ resolve_symlink_merge () {
 
 resolve_deleted_merge () {
     while true; do
-	printf "Use (m)odified or (d)eleted file, or (a)bort? "
+	if base_present; then
+	    printf "Use (m)odified or (d)eleted file, or (a)bort? "
+	else
+	    printf "Use (c)reated or (d)eleted file, or (a)bort? "
+	fi
 	read ans
 	case "$ans" in
-	    [mM]*)
+	    [mMcC]*)
 		git-add -- "$path"
 		cleanup_temp_files --save-backup
 		return
@@ -166,7 +167,7 @@ merge_file () {
     remote_present && git cat-file blob ":3:$path" > "$REMOTE" 2>/dev/null
 
     if test -z "$local_mode" -o -z "$remote_mode"; then
-	echo "Deleted merge conflict for $path:"
+	echo "Deleted merge conflict for '$path':"
 	describe_file "$local_mode" "local" "$LOCAL"
 	describe_file "$remote_mode" "remote" "$REMOTE"
 	resolve_deleted_merge
@@ -174,14 +175,14 @@ merge_file () {
     fi
 
     if is_symlink "$local_mode" || is_symlink "$remote_mode"; then
-	echo "Symlink merge conflict for $path:"
+	echo "Symbolic link merge conflict for '$path':"
 	describe_file "$local_mode" "local" "$LOCAL"
 	describe_file "$remote_mode" "remote" "$REMOTE"
 	resolve_symlink_merge
 	return
     fi
 
-    echo "Normal merge conflict for $path:"
+    echo "Normal merge conflict for '$path':"
     describe_file "$local_mode" "local" "$LOCAL"
     describe_file "$remote_mode" "remote" "$REMOTE"
     printf "Hit return to start merge resolution tool ($merge_tool): "
-- 
1.5.1.rc2.1.g8afe-dirty

Re: [PATCH] mergetool: Clean up description of files and prompts for merge resolutions

[Matthias Lederhofer]

Theodore Ts'o <tytso@mit.edu> wrote:
> +    printf "  {$branch}: "
This should use %s too.

Re: [PATCH] mergetool: Clean up description of files and prompts for merge resolutions

[Theodore Tso]

On Fri, Mar 30, 2007 at 12:29:05AM +0200, Matthias Lederhofer wrote:
> Theodore Ts'o <tytso@mit.edu> wrote:
> > +    printf "  {$branch}: "
> This should use %s too.

It's strictly not necessary since $branch is either "local" or "remote".

						- Ted

Re: [PATCH] mergetool: Clean up description of files and prompts for merge resolutions

[Randal L. Schwartz]

>>>>> "Theodore" == Theodore Tso <tytso@mit.edu> writes:

Theodore> On Fri, Mar 30, 2007 at 12:29:05AM +0200, Matthias Lederhofer wrote:
>> Theodore Ts'o <tytso@mit.edu> wrote:
>> > +    printf "  {$branch}: "
>> This should use %s too.

Theodore> It's strictly not necessary since $branch is either "local" or "remote".

... this week. :)

Best to be safe, and provide a good model for later cut-n-pasters, I've
always said.

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

Re: [PATCH] mergetool: Replace use of "echo -n" with printf(3) to be more portable

[Matthias Lederhofer]

The subject should probably refer to printf(1) instead of printf(3).

Theodore Ts'o <tytso@mit.edu> wrote:
> -	echo -n "'$path' was deleted"
> +	printf  "'$path' was deleted"
Here is one space too much after printf.

> -	echo -n "'$path' is a symlink containing '"
> +	printf "'$path' is a symlink containing '"
You should use printf "'%s' [..]" "$path" as you do in some other
places (in case $path contains conversion specifiers).

> -    echo -n "Hit return to start merge resolution tool ($merge_tool): "
> +    printf "Hit return to start merge resolution tool ($merge_tool): "
Here it is much more unlikely that $merge_tool contains a conversion
specifier but anyway I'd prefer to use %s.

Re: [PATCH] mergetool: Replace use of "echo -n" with printf(3) to be more portable

[Theodore Tso]

On Thu, Mar 29, 2007 at 05:57:41PM +0200, Matthias Lederhofer wrote:
> The subject should probably refer to printf(1) instead of printf(3).

Yep.

> Theodore Ts'o <tytso@mit.edu> wrote:
> > +	printf  "'$path' was deleted"
> Here is one space too much after printf.

This was fixed in a later patch.

> > +	printf "'$path' is a symlink containing '"
> You should use printf "'%s' [..]" "$path" as you do in some other
> places (in case $path contains conversion specifiers).

This was also obsoleted in a later patch, but you're right.

> > +    printf "Hit return to start merge resolution tool ($merge_tool): "
> Here it is much more unlikely that $merge_tool contains a conversion
> specifier but anyway I'd prefer to use %s.

Yep, agreed.

Junio, I've prepared an alterate patch series.  If you haven't pulled
from repo.or.cz, I'd appreciate it if you could pull from the
mergetool branch (instead of master) instead:

	http://repo.or.cz/w/git/mergetool.git

The differences between the patch series in the master and mergetool
branch are:

	* Change the description of the printf patch to say: 
		printf(3)->printf(1)
	* Fix the double space after printf and symlink %s conversion
		earlier in the patch series (only to have it disappear
		in the last patch, so this is a no-op at the end of
		the day, but more of an excuse to go play with stgit
		and in the name of anal-rententive correctness)
	* Use a %s printf conversion for the "Hit return to start merge..."
		statement as noted above in the last patch.

Thanks!!

(Apparently there's no way to forcibly reset the pointer of the master
head on repo.or.cz, probably for security/sanity reasons, without
going through an administrator.)

						- Ted

Re: [PATCH] mergetool: Replace use of "echo -n" with printf(3) to be more portable

[Junio C Hamano]

Theodore Tso <tytso@mit.edu> writes:

> Junio, I've prepared an alterate patch series.  If you haven't pulled
> from repo.or.cz, I'd appreciate it if you could pull from the
> mergetool branch (instead of master) instead:
>
> 	http://repo.or.cz/w/git/mergetool.git

Thanks; will do later tonight after I got home.

> (Apparently there's no way to forcibly reset the pointer of the master
> head on repo.or.cz, probably for security/sanity reasons, without
> going through an administrator.)

Really?  I push with '+' (aka "single force") all the time to
alt-git.git repository and haven't noticed that:

    [remote "repo"]
            url = repo.or.cz:srv/git/alt-git.git/
            push = refs/tags/ko-master:refs/heads/master
            push = refs/tags/ko-next:refs/heads/next
            push = +refs/tags/ko-pu:refs/heads/pu
            push = refs/tags/ko-maint:refs/heads/maint

Here, tags/ko-* keeps track of what I pushed out the official
k.org repositories, and my workflow has been:

    $ git push ko ;# push to k.org refs/heads/{master,next,...}
    $ git fetch ko ;# fetch them back to refs/tags/ko-*
    $ git push repo ;# update alt-git.git

Re: [PATCH] mergetool: Replace use of "echo -n" with printf(3) to be more portable

[Theodore Tso]

On Thu, Mar 29, 2007 at 03:02:42PM -0700, Junio C Hamano wrote:
> Really?  I push with '+' (aka "single force") all the time to
> alt-git.git repository and haven't noticed that:

Oh, cool, it never occurred to me that it would work with push on a
remote repository (but that makes perfect sense in retrospect :-).

						- Ted