Re: [PATCH v2 1/2] rebase -i -p: handle index and workdir correctly

From: Stephan Beyer <s-beyer@gmx.net>
Date: Aug 13, 2008, 20:58 UTC
Message-ID: <20080813205806.GC10758@leksak.fem-net>
In-Reply-To: <23b4780a3a7bbe06f4157b6843d1d9dab26d7087.1218628444.git.trast@student.ethz.ch>

Hi,

Thomas Rast wrote:

Show 24 quoted lines

> Also, it attempted to do a fast forward even if it was instructed not
> to commit (via -n).  Fall back to the cherry-pick code path and let
> that handle the issue for us.
> 
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
>  git-rebase--interactive.sh    |   13 ++++++++++++-
>  t/t3404-rebase-interactive.sh |    6 ++++++
>  2 files changed, 18 insertions(+), 1 deletions(-)
> 
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 4e334ba..1dc24b1 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -145,7 +145,16 @@ pick_one () {
>  }
>  
>  pick_one_preserving_merges () {
> -	case "$1" in -n) sha1=$2 ;; *) sha1=$1 ;; esac
> +	fast_forward=t
> +	case "$1" in
> +	-n)
> +		fast_forward=f
> +		sha1=$2

The intention of setting fast_forward=f is clear, but it is overwritten later on line 169 that is not shown in this patch:

168: # rewrite parents; if none were rewritten, we can fast-forward. 169: fast_forward=t

Clearly, this is due to your patch split, because after the second patch, this is fixed.

The rest of this patch seems to be ok to me.

Regards.

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F