Re: git rebase -i error message interprets \t in commit message

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: git rebase -i error message interprets \t in commit message

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:58:20

Matthieu Moy [off-list ref] writes:
quoted hunk
David Kastrup [off-list ref] writes:
quoted
Could not apply 16de9d2... Make tempo range 	empo 20~30 be input as 	empo 20-30 instead
Indeed. The source of the problem is that our "die" shell function
interprets \t (because it uses "echo").

A simple fix would be this:
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 7a964ad..97258d5 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -53,7 +53,7 @@ die () {
 die_with_status () {
        status=$1
        shift
-       echo >&2 "$*"
+       printf >&2 "%s\n" "$*"
        exit "$status"
 }
 
It does not sound crazy as the shell function "say" right below uses the
same printf "%s\n" "$*", but I'm wondering whether this could have other
bad implications (e.g. if there are escape sequences in the commit
message, aren't we going to screw up the terminal?).
I gave a quick look at "git grep -e 'die ' -- \*.sh" output, and I
do not think this change will break things (i.e. no caller expects
the non-portable behaviour of the shell such as "\c" at the end to
omit the trailing newline).

Thanks, care to roll it into a patch with a test or two?

[PATCH] die_with_status: use "printf '%s\n'", not "echo"

From: Matthieu Moy <hidden>
Date: 2016-06-15 22:58:20

At least GNU echo interprets backslashes in its arguments.

This triggered at least one bug: the error message of "rebase -i" was
turning \t in commit messages into actual tabulations. There may be
others.

Using "printf '%s\n'" instead avoids this bad behavior, and is the form
used by the "say" function.

Noticed-by: David Kastrup [off-list ref]
Signed-off-by: Matthieu Moy <redacted>
---
 git-sh-setup.sh               |  2 +-
 t/t3404-rebase-interactive.sh | 13 +++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 7a964ad..e15be51 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -53,7 +53,7 @@ die () {
 die_with_status () {
 	status=$1
 	shift
-	echo >&2 "$*"
+	printf >&2 '%s\n' "$*"
 	exit "$status"
 }
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 49ccb38..074deb1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -976,4 +976,17 @@ test_expect_success 'rebase -i with --strategy and -X' '
 	test $(cat file1) = Z
 '
 
+test_expect_success 'rebase -i error on commits with \ in message' '
+	current_head=$(git rev-parse HEAD)
+	test_when_finished "git rebase --abort; git reset --hard $current_head; rm -f error" &&
+	test_commit TO-REMOVE will-conflict old-content &&
+	test_commit "\temp" will-conflict new-content dummy &&
+	(
+	EDITOR=true &&
+	export EDITOR &&
+	test_must_fail git rebase -i HEAD^ --onto HEAD^^ 2>error
+	) &&
+	grep -v "	" error
+'
+
 test_done
-- 
1.8.3.3.797.gb72c616
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help