Re: [PATCH v2 3/4] rebase: stop accepting --rebase-merges=""

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Alex

On 21/02/2023 05:58, Alex Henrie wrote:
The unusual syntax --rebase-merges="" (that is, --rebase-merges with an
empty string argument) has been an undocumented synonym of
--rebase-merges=no-rebase-cousins. Stop accepting that syntax to avoid
confusion when a rebase.merges config option is introduced, where
rebase.merges="" will be equivalent to not passing --rebase-merges.

I think this is sensible in the context of adding a config option. It is a backwards incompatible change though, lets hope no one was relying on it. Is there a particular reason you decided to redo the option parsing rather than just calling parse_merges_value() from the existing "if (rebase_merges)" block? I don't think it really matters, I'm just curious.

Best Wishes

Phillip

Signed-off-by: Alex Henrie <alexhenrie24@xxxxxxxxx>
---
  builtin/rebase.c         | 42 +++++++++++++++++++++++++++-------------
  t/t3430-rebase-merges.sh |  6 ++++++
  2 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 6635f10d52..0a8366f30f 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -771,6 +771,20 @@ static int run_specific_rebase(struct rebase_options *opts)
  	return status ? -1 : 0;
  }
+static void parse_merges_value(struct rebase_options *options, const char *value)
+{
+	if (value) {
+		if (!strcmp("no-rebase-cousins", value))
+			options->rebase_cousins = 0;
+		else if (!strcmp("rebase-cousins", value))
+			options->rebase_cousins = 1;
+		else
+			die(_("Unknown mode: %s"), value);
+	}
+
+	options->rebase_merges = 1;
+}
+
  static int rebase_config(const char *var, const char *value, void *data)
  {
  	struct rebase_options *opts = data;
@@ -980,6 +994,18 @@ static int parse_opt_empty(const struct option *opt, const char *arg, int unset)
  	return 0;
  }
+static int parse_opt_merges(const struct option *opt, const char *arg, int unset)
+{
+	struct rebase_options *options = opt->value;
+
+	if (unset)
+		options->rebase_merges = 0;
+	else
+		parse_merges_value(options, arg);
+
+	return 0;
+}
+
  static void NORETURN error_on_missing_default_upstream(void)
  {
  	struct branch *current_branch = branch_get(NULL);
@@ -1035,7 +1061,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
  	struct object_id branch_base;
  	int ignore_whitespace = 0;
  	const char *gpg_sign = NULL;
-	const char *rebase_merges = NULL;
  	struct string_list strategy_options = STRING_LIST_INIT_NODUP;
  	struct object_id squash_onto;
  	char *squash_onto_name = NULL;
@@ -1137,10 +1162,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
  			   &options.allow_empty_message,
  			   N_("allow rebasing commits with empty messages"),
  			   PARSE_OPT_HIDDEN),
-		{OPTION_STRING, 'r', "rebase-merges", &rebase_merges,
-			N_("mode"),
+		OPT_CALLBACK_F('r', "rebase-merges", &options, N_("mode"),
  			N_("try to rebase merges instead of skipping them"),
-			PARSE_OPT_OPTARG, NULL, (intptr_t)""},
+			PARSE_OPT_OPTARG, parse_opt_merges),
  		OPT_BOOL(0, "fork-point", &options.fork_point,
  			 N_("use 'merge-base --fork-point' to refine upstream")),
  		OPT_STRING('s', "strategy", &options.strategy,
@@ -1436,16 +1460,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
  	if (options.exec.nr)
  		imply_merge(&options, "--exec");
- if (rebase_merges) {
-		if (!*rebase_merges)
-			; /* default mode; do nothing */
-		else if (!strcmp("rebase-cousins", rebase_merges))
-			options.rebase_cousins = 1;
-		else if (strcmp("no-rebase-cousins", rebase_merges))
-			die(_("Unknown mode: %s"), rebase_merges);
-		options.rebase_merges = 1;
+	if (options.rebase_merges)
  		imply_merge(&options, "--rebase-merges");
-	}
if (options.type == REBASE_APPLY) {
  		if (ignore_whitespace)
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index e0d910c229..b8ba323dbc 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -293,6 +293,12 @@ test_expect_success 'do not rebase cousins unless asked for' '
  	EOF
  '
+test_expect_success '--rebase-merges="" is invalid syntax' '
+	echo "fatal: Unknown mode: " >expect &&
+	! git rebase --rebase-merges="" HEAD^ 2>actual &&
+	test_cmp expect actual
+'
+
  test_expect_success 'refs/rewritten/* is worktree-local' '
  	git worktree add wt &&
  	cat >wt/script-from-scratch <<-\EOF &&



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux