Re: [PATCH 4/5] rebase --keep-base: imply --reapply-cherry-picks

From: Jonathan Tan <jonathantanmy@google.com>
Date: Aug 24, 2022, 22:09 UTC
Message-ID: <20220824220945.1683473-1-jonathantanmy@google.com>
In-Reply-To: <9cd4c372ee4b3e5ba45c66a43ad0edaf52f0eed9.1660576283.git.gitgitgadget@gmail.com>

"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 10 quoted lines

> @@ -1240,6 +1241,12 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  		if (options.root)
>  			die(_("options '%s' and '%s' cannot be used together"), "--keep-base", "--root");
>  	}
> +	/*
> +	 * --keep-base defaults to --reapply-cherry-picks as it is confusing if
> +	 * commits disappear when using this option.
> +	 */
> +	if (options.reapply_cherry_picks < 0)
> +		options.reapply_cherry_picks = keep_base;

Here, we set options.reapply_cherry_picks to 1 if keep_base is 1, but...

Show 11 quoted lines

> @@ -1416,7 +1423,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  	if (options.empty != EMPTY_UNSPECIFIED)
>  		imply_merge(&options, "--empty");
>  
> -	if (options.reapply_cherry_picks)
> +	/*
> +	 * --keep-base implements --reapply-cherry-picks by altering upstream so
> +	 * it works with both backends.
> +	 */
> +	if (options.reapply_cherry_picks && !keep_base)
>  		imply_merge(&options, "--reapply-cherry-picks");

...if we implement --reapply-cherry-picks by altering upstream (and hence not need to rely on the "merge" backend), shouldn't we suppress the setting of options.reapply_cherry_picks too?