threads / patch / 24300

patch, 2 partsrebase-i minifixes

Subject: [PATCH 0/2] rebase-i minifixes

## tl;dr

4 messages between Jul 6, 2010 and Jul 6, 2010. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Junio C Hamano· Jul 6, 2010, 06:27 UTC · lore

The first one is just a (long overdue) style fix. The second one changes "read" to "read -r" to work around commit log messages that have backslash characters in them, the issue reported by Joshua ben Jore

Junio C Hamano (2):
  rebase-i: style fix
  rebase-i: do not get fooled by a log message ending with backslash
 git-rebase--interactive.sh    |   19 ++++++++++---------
 t/t3404-rebase-interactive.sh |   18 ++++++++++++++++++
 2 files changed, 28 insertions(+), 9 deletions(-)
-- 
1.7.2.rc1.209.g3868f
Junio C Hamano· Jul 6, 2010, 06:27 UTC · re: Junio C Hamano · lore

[PATCH 1/2] rebase-i: style fix

Case arms should align with "case" and "esac".

Do not cat a file into a pipeline; just make the downstream command read from the file.

Having a while statement as a downstream of a pipe is fine, but the loop should begin on its own line.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-rebase--interactive.sh |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)
Show changes to git-rebase--interactive.sh +6 −5
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 6b86abc..ed57552 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -263,10 +263,10 @@ pick_one_preserving_merges () {
 	then
 		if test "$fast_forward" = t
 		then
-			cat "$DOTEST"/current-commit | while read current_commit
+			while read current_commit
 			do
 				git rev-parse HEAD > "$REWRITTEN"/$current_commit
-			done
+			done <"$DOTEST"/current-commit
 			rm "$DOTEST"/current-commit ||
 			die "Cannot write current commit's replacement sha1"
 		fi
@@ -440,9 +440,9 @@ record_in_rewritten() {
 	echo "$oldsha1" >> "$REWRITTEN_PENDING"
 
 	case "$(peek_next_command)" in
-	    squash|s|fixup|f)
+	squash|s|fixup|f)
 		;;
-	    *)
+	*)
 		flush_rewritten_pending
 		;;
 	esac
@@ -890,7 +890,8 @@ first and then run 'git rebase --continue' again."
 		git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
 			--abbrev=7 --reverse --left-right --topo-order \
 			$REVISIONS | \
-			sed -n "s/^>//p" | while read shortsha1 rest
+			sed -n "s/^>//p" |
+		while read shortsha1 rest
 		do
 			if test t != "$PRESERVE_MERGES"
 			then
-- 
1.7.2.rc1.209.g3868f
Junio C Hamano· Jul 6, 2010, 06:27 UTC · re: Junio C Hamano · lore

[PATCH 2/2] rebase-i: do not get fooled by a log message ending with backslash

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-rebase--interactive.sh    |   10 +++++-----
 t/t3404-rebase-interactive.sh |   18 ++++++++++++++++++
 2 files changed, 23 insertions(+), 5 deletions(-)
Show changes to 2 files +23 −5

git-rebase--interactive.sh, t/t3404-rebase-interactive.sh

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ed57552..31e6860 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -450,7 +450,7 @@ record_in_rewritten() {
 
 do_next () {
 	rm -f "$MSG" "$AUTHOR_SCRIPT" "$AMEND" || exit
-	read command sha1 rest < "$TODO"
+	read -r command sha1 rest < "$TODO"
 	case "$command" in
 	'#'*|''|noop)
 		mark_action_done
@@ -591,7 +591,7 @@ do_rest () {
 # skip picking commits whose parents are unchanged
 skip_unnecessary_picks () {
 	fd=3
-	while read command sha1 rest
+	while read -r command sha1 rest
 	do
 		# fd=3 means we skip the command
 		case "$fd,$command,$(git rev-parse --verify --quiet $sha1^)" in
@@ -644,13 +644,13 @@ rearrange_squash () {
 	test -s "$1.sq" || return
 
 	used=
-	while read pick sha1 message
+	while read -r pick sha1 message
 	do
 		case " $used" in
 		*" $sha1 "*) continue ;;
 		esac
 		echo "$pick $sha1 $message"
-		while read squash action msg
+		while read -r squash action msg
 		do
 			case "$message" in
 			"$msg"*)
@@ -891,7 +891,7 @@ first and then run 'git rebase --continue' again."
 			--abbrev=7 --reverse --left-right --topo-order \
 			$REVISIONS | \
 			sed -n "s/^>//p" |
-		while read shortsha1 rest
+		while read -r shortsha1 rest
 		do
 			if test t != "$PRESERVE_MERGES"
 			then
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index ee9a1b2..47ca88f 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -630,4 +630,22 @@ test_expect_success 'always cherry-pick with --no-ff' '
 	test_cmp empty out
 '
 
+test_expect_success 'set up commits with funny messages' '
+	git checkout -b funny A &&
+	echo >>file1 &&
+	test_tick &&
+	git commit -a -m "end with slash\\" &&
+	echo >>file1 &&
+	test_tick &&
+	git commit -a -m "another commit"
+'
+
+test_expect_success 'rebase-i history with funny messages' '
+	git rev-list A..funny >expect &&
+	test_tick &&
+	FAKE_LINES="1 2" git rebase -i A &&
+	git rev-list A.. >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.2.rc1.209.g3868f
Jonathan Nieder· Jul 6, 2010, 18:57 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] rebase-i: do not get fooled by a log message ending with backslash

Junio C Hamano wrote:
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Posix mandates the -r option to replace the less convenient ‘line’ utility. read -r is part of SVR4.2[1] so any vaguely Posix-ish sh should have it (i.e., not Solaris /bin/sh, but that’s okay).

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

(Not tested, since I don’t have access to any non-Linux installations nowadays.)

[1] http://www.in-ulm.de/~mascheck/bourne/

← back to recent threads