git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthieu Moy <Matthieu.Moy@imag.fr>
To: git@vger.kernel.org, gitster@pobox.com
Cc: Thomas Rast <trast@inf.ethz.ch>, Matthieu Moy <Matthieu.Moy@imag.fr>
Subject: [PATCH v2] die_with_status: use "printf '%s\n'", not "echo"
Date: Wed,  7 Aug 2013 11:26:05 +0200	[thread overview]
Message-ID: <1375867565-21127-1-git-send-email-Matthieu.Moy@imag.fr> (raw)
In-Reply-To: <vpqy58dj1ly.fsf@anie.imag.fr>

Some implementations of 'echo' (e.g. dash's built-in) interpret
backslash sequences in their 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 <dak@gnu.org>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Changed the "grep" function to be more accurate. The test still
catches the old failure and pass after the fix.

 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..4dbeddb 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
+	) &&
+	test_expect_code 1 grep  "	emp" error
+'
+
 test_done
-- 
1.8.3.3.797.gb72c616

  reply	other threads:[~2013-08-07  9:26 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-06 15:44 git rebase -i error message interprets \t in commit message David Kastrup
2013-08-06 16:01 ` Ramkumar Ramachandra
2013-08-06 17:47   ` Junio C Hamano
2013-08-06 16:05 ` Matthieu Moy
2013-08-06 17:07   ` David Kastrup
2013-08-06 17:19     ` Matthieu Moy
2013-08-06 19:23       ` David Kastrup
2013-08-06 19:35         ` Junio C Hamano
2013-08-06 17:52   ` Junio C Hamano
2013-08-06 18:26     ` [PATCH] die_with_status: use "printf '%s\n'", not "echo" Matthieu Moy
2013-08-07  8:58       ` Thomas Rast
2013-08-07  9:23         ` Matthieu Moy
2013-08-07  9:26           ` Matthieu Moy [this message]
2013-08-07  9:48           ` Thomas Rast

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1375867565-21127-1-git-send-email-Matthieu.Moy@imag.fr \
    --to=matthieu.moy@imag.fr \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=trast@inf.ethz.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).