# [PATCH 0/1] legacy-rebase: fix "regression"

5 messages from 2018-11-20 to 2018-11-21. Participants: Johannes Schindelin via GitGitGadget, Carlo Arenas, Junio C Hamano, Johannes Schindelin.
Thread: https://gitlist.dev/t/49869

## Johannes Schindelin via GitGitGadget, 2018-11-20 20:01

Subject: [PATCH 0/1] legacy-rebase: fix "regression"
Message-ID: <pull.86.git.gitgitgadget@gmail.com>
URL: https://gitlist.dev/e/pull.86.git.gitgitgadget%40gmail.com

```
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
-- 
gitgitgadget

```

## Johannes Schindelin via GitGitGadget, 2018-11-20 20:02

Subject: [PATCH 1/1] legacy-rebase: backport -C<n> and --whitespace=<option> checks
Message-ID: <84e65a4efb3d58ca777cd59c52fe86d63060ea9e.1542744118.git.gitgitgadget@gmail.com>
URL: https://gitlist.dev/e/84e65a4efb3d58ca777cd59c52fe86d63060ea9e.1542744118.git.gitgitgadget%40gmail.com
In-Reply-To: <pull.86.git.gitgitgadget@gmail.com>

```
From: Johannes Schindelin <johannes.schindelin@gmx.de>

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.

Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 git-legacy-rebase.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh
index 75a08b2683..ced0635326 100755
--- a/git-legacy-rebase.sh
+++ b/git-legacy-rebase.sh
@@ -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"
 		;;
-- 
gitgitgadget

```

## Carlo Arenas, 2018-11-20 23:45

Subject: Re: [PATCH 1/1] legacy-rebase: backport -C<n> and --whitespace=<option> checks
Message-ID: <CAPUEspgxNm9oEA-fuT7kBegEp5F2za5Nm6CSvFkMB+Kz_pNbrw@mail.gmail.com>
URL: https://gitlist.dev/e/CAPUEspgxNm9oEA-fuT7kBegEp5F2za5Nm6CSvFkMB%2BKz_pNbrw%40mail.gmail.com
In-Reply-To: <84e65a4efb3d58ca777cd59c52fe86d63060ea9e.1542744118.git.gitgitgadget@gmail.com>

```
Tested-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>

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

Carlo

```

## Junio C Hamano, 2018-11-21 04:40

Subject: Re: [PATCH 1/1] legacy-rebase: backport -C<n> and --whitespace=<option> checks
Message-ID: <xmqqin0r82mu.fsf@gitster-ct.c.googlers.com>
URL: https://gitlist.dev/e/xmqqin0r82mu.fsf%40gitster-ct.c.googlers.com
In-Reply-To: <CAPUEspgxNm9oEA-fuT7kBegEp5F2za5Nm6CSvFkMB+Kz_pNbrw@mail.gmail.com>

```
Carlo Arenas <carenas@gmail.com> writes:

> Tested-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
>
> 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(-)

diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh
index ced0635326..b97ffdc9dd 100755
--- a/git-legacy-rebase.sh
+++ b/git-legacy-rebase.sh
@@ -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"



```

## Johannes Schindelin, 2018-11-21 14:06

Subject: Re: [PATCH 1/1] legacy-rebase: backport -C<n> and --whitespace=<option> checks
Message-ID: <nycvar.QRO.7.76.6.1811211506120.41@tvgsbejvaqbjf.bet>
URL: https://gitlist.dev/e/nycvar.QRO.7.76.6.1811211506120.41%40tvgsbejvaqbjf.bet
In-Reply-To: <xmqqin0r82mu.fsf@gitster-ct.c.googlers.com>

```
Hi Junio,

On Wed, 21 Nov 2018, Junio C Hamano wrote:

> Carlo Arenas <carenas@gmail.com> writes:
> 
> > Tested-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> >
> > 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

> 
> 
>  git-legacy-rebase.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh
> index ced0635326..b97ffdc9dd 100755
> --- a/git-legacy-rebase.sh
> +++ b/git-legacy-rebase.sh
> @@ -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"
> 
> 
> 
```