Skip to contentSkip to conversation

legacy-rebase: fix "regression"

PATCH In review Johannes Schindelin via GitGitGadget 4 participants 5 messages Nov 20, 2018 – Nov 21, 2018

lore
Share & tools
lore
#0 lore raw

cover legacy-rebase: fix "regression"

This is a backport, really, to accommodate a new regression test that was introduced when the built-in rebase learned to validate the -C<n> and --whitespace=<option> arguments early.

Johannes Schindelin (1):
legacy-rebase: backport -C<n> and --whitespace=<option> checks

 git-legacy-rebase.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

base-commit: bb75be6cb916297f271c846f2f9caa3daaaec718
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-86%2Fdscho%2Fscripted-rebase-Cn-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-86/dscho/scripted-rebase-Cn-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/86

Signature · 1 line
gitgitgadget
#1 lore raw

PATCH 1/1 legacy-rebase: backport -C<n> and --whitespace=<option> checks

From: Johannes Schindelin <[email protected]>

Since 04519d720114 (rebase: validate -C<n> and --whitespace=<mode> parameters early, 2018-11-14), the built-in rebase validates the -C and --whitespace arguments early. As this commit also introduced a regression test for this, and as a later commit introduced the GIT_TEST_REBASE_USE_BUILTIN mode to run tests, we now have a "regression" in the scripted version of git rebase on our hands.

Backport the validation to fix this.

---
 git-legacy-rebase.sh | 8 ++++++++
 1 file changed, 8 insertions(+)
git-legacy-rebase.sh +8 -0
index 75a08b2683..ced0635326 100755
@@ -337,6 +337,11 @@ do
 		fix|strip)
 			force_rebase=t
 			;;
+		warn|nowarn|error|error-all)
+			;; # okay, known whitespace option
+		*)
+			die "Invalid whitespace option: '${1%*=}'"
+			;;
 		esac
 		;;
 	--ignore-whitespace)
@@ -352,6 +357,9 @@ do
 		git_am_opt="$git_am_opt $1"
 		force_rebase=t
 		;;
+	-C*[!0-9]*)
+		die "switch \`C' expects a numerical value"
+		;;
 	-C*)
 		git_am_opt="$git_am_opt $1"
 		;;
Signature · 1 line
gitgitgadget
#3 lore raw

Re: [PATCH 1/1] legacy-rebase: backport -C<n> and --whitespace=<option> checks

Quoted 4 lines · Carlo Arenas

the C version prepends: "fatal: " unlike the shell version for both error messages

In addition, "Invalid whitespace option" says 'bad', not '--whitespace=bad', in the builtin version.

I think the following would address both issues.

 git-legacy-rebase.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
git-legacy-rebase.sh +2 -2
index ced0635326..b97ffdc9dd 100755
@@ -340,7 +340,7 @@ do
 		warn|nowarn|error|error-all)
 			;; # okay, known whitespace option
 		*)
-			die "Invalid whitespace option: '${1%*=}'"
+			die "fatal: Invalid whitespace option: '${1#*=}'"
 			;;
 		esac
 		;;
@@ -358,7 +358,7 @@ do
 		force_rebase=t
 		;;
 	-C*[!0-9]*)
-		die "switch \`C' expects a numerical value"
+		die "fatal: switch \`C' expects a numerical value"
 		;;
 	-C*)
 		git_am_opt="$git_am_opt $1"
#4 lore raw

Re: [PATCH 1/1] legacy-rebase: backport -C<n> and --whitespace=<option> checks

Hi Junio,

Quoted 11 lines · Junio C Hamano
Quoted 4 lines · Carlo Arenas

the C version prepends: "fatal: " unlike the shell version for both error messages

In addition, "Invalid whitespace option" says 'bad', not '--whitespace=bad', in the builtin version.

I think the following would address both issues.

Yes! Thank you. Can you squash it in?

Thanks,
Dscho

Quoted 30 lines

 git-legacy-rebase.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
git-legacy-rebase.sh +2 -2
index ced0635326..b97ffdc9dd 100755
@@ -340,7 +340,7 @@ do
 		warn|nowarn|error|error-all)
 			;; # okay, known whitespace option
 		*)
-			die "Invalid whitespace option: '${1%*=}'"
+			die "fatal: Invalid whitespace option: '${1#*=}'"
 			;;
 		esac
 		;;
@@ -358,7 +358,7 @@ do
 		force_rebase=t
 		;;
 	-C*[!0-9]*)
-		die "switch \`C' expects a numerical value"
+		die "fatal: switch \`C' expects a numerical value"
 		;;
 	-C*)
 		git_am_opt="$git_am_opt $1"