threads / patch / 7475

patchmergetool: Fix abort command when resolving symlinks and deleted files

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

## tl;dr

19 messages between Mar 29, 2007 and Mar 30, 2007. Diffs are folded; open one to read it.

replies: 18people: 4as markdown or json

Theodore Ts'o· Mar 29, 2007, 15:44 UTC · lore

[GIT PULL] git mergetool fixes

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

Theodore Ts'o· Mar 29, 2007, 15:44 UTC · re: Theodore Ts'o · lore

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

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

Show changes to Documentation/git-mergetool.txt +5 −5

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

Theodore Ts'o· Mar 29, 2007, 15:44 UTC · re: Theodore Ts'o · lore

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

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

Show changes to git-mergetool.sh +13 −13

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

Theodore Ts'o· Mar 29, 2007, 15:44 UTC · re: Theodore Ts'o · lore

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

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(-)

Show changes to git-mergetool.sh +5 −6

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

Theodore Ts'o· Mar 29, 2007, 15:44 UTC · re: Theodore Ts'o · lore

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

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

Show changes to git-mergetool.sh +2 −2

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

Theodore Ts'o· Mar 29, 2007, 15:44 UTC · re: Theodore Ts'o · lore

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

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

Show changes to git-mergetool.sh +5 −5

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

Theodore Ts'o· Mar 29, 2007, 15:44 UTC · re: Theodore Ts'o · lore

[PATCH] mergetool: factor out common code

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(-)

Show changes to git-mergetool.sh +37 −43

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

Theodore Ts'o· Mar 29, 2007, 15:44 UTC · re: Theodore Ts'o · lore

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

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

Show changes to git-mergetool.sh +1 −1

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

Theodore Ts'o· Mar 29, 2007, 15:44 UTC · re: Theodore Ts'o · lore

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

Show changes to git-mergetool.sh +4 −4

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

Theodore Ts'o· Mar 29, 2007, 15:44 UTC · re: Theodore Ts'o · lore

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

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(-)

Show changes to git-mergetool.sh +14 −2

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

Theodore Ts'o· Mar 29, 2007, 15:44 UTC · re: Theodore Ts'o · lore

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

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

Show changes to git-mergetool.sh +1 −1

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

Theodore Ts'o· Mar 29, 2007, 15:44 UTC · re: Theodore Ts'o · lore

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

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(-)

Show changes to git-mergetool.sh +15 −14

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

Matthias Lederhofer· Mar 29, 2007, 22:29 UTC · re: Theodore Ts'o · lore

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

Theodore Ts'o <tytso@mit.edu> wrote:

> +    printf "  {$branch}: "

This should use %s too.

Theodore Tso· Mar 30, 2007, 02:54 UTC · re: Matthias Lederhofer · lore

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

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

Randal L. Schwartz· Mar 30, 2007, 14:46 UTC · re: Theodore Tso · lore

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

>>>>> "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!

Matthias Lederhofer· Mar 29, 2007, 15:57 UTC · re: Theodore Ts'o · lore

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

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.

Theodore Tso· Mar 29, 2007, 17:09 UTC · re: Matthias Lederhofer · lore

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

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

Junio C Hamano· Mar 29, 2007, 22:02 UTC · re: Theodore Tso · lore

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

Theodore Tso <tytso@mit.edu> writes:

Show 5 quoted lines

> 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

Theodore Tso· Mar 29, 2007, 22:15 UTC · re: Junio C Hamano · lore

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

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