{"thread":{"id":"62933","subject":"[PATCH] rebase: add `--update-refs=interactive`","startedAt":"2025-02-10T19:16:59Z","lastAt":"2025-02-19T14:52:36Z","messageCount":16,"participants":["Ivan Shapovalov","D. Ben Knoble","Phillip Wood","Junio C Hamano","phillip.wood123@gmail.com"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"512195","messageId":"20250210191650.316329-1-intelfx@intelfx.name","threadId":"62933","inReplyTo":null,"subject":"[PATCH] rebase: add `--update-refs=interactive`","fromName":"Ivan Shapovalov","fromEmail":"intelfx@intelfx.name","sentAt":"2025-02-10T19:16:44Z","receivedAt":"2025-02-10T19:16:59Z","isPatch":true,"sender":{"key":"intelfx@intelfx.name","avatar":"https://gravatar.com/avatar/fb0d6e45051c5d8cda450fd6d02ecba0216f5f3aae1ebcf5de522939ff18eef3?d=mp&s=160"},"body":"In rebase-heavy workflows involving multiple interdependent feature\nbranches, typing out `--update-refs` quickly becomes tiring, which\ncan be mitigated with setting the `rebase.updateRefs` git-config option\nto perform update-refs by default.\n\nHowever, the utility of `rebase.updateRefs` is somewhat limited because\nyou rarely want it in a non-interactive rebase (as it does not give you\nthe chance to review the update-refs candidates, likely leading to\nupdating refs that you didn't want updated -- I made quite an amount\nof mess by setting this option and subsequently forgetting about it).\n\nTry to find a middle ground by introducing a third value,\n`--update-refs=interactive` (and `rebase.updateRefs=interactive`)\nwhich means `--update-refs` when starting an interactive rebase and\n`--no-update-refs` otherwise. This option is primarily intended to be\nused in the gitconfig, but is also accepted on the command line\nfor completeness.\n\nSigned-off-by: Ivan Shapovalov <intelfx@intelfx.name>\n---\n Documentation/config/rebase.txt |  7 +++-\n Documentation/git-rebase.txt    |  8 +++-\n builtin/rebase.c                | 72 +++++++++++++++++++++++++++++----\n 3 files changed, 77 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt\nindex c6187ab28b..d8bbaba69a 100644\n--- a/Documentation/config/rebase.txt\n+++ b/Documentation/config/rebase.txt\n@@ -24,7 +24,12 @@ rebase.autoStash::\n \tDefaults to false.\n \n rebase.updateRefs::\n-\tIf set to true enable `--update-refs` option by default.\n+\tIf set to true, enable the `--update-refs` option of\n+\tlinkgit:git-rebase[1] by default. When set to 'interactive',\n+\tonly enable `--update-refs` by default for interactive mode\n+\t(equivalent to `--update-refs=interactive`).\n+\tThis option can be overridden by specifying any form of\n+\t`--update-refs` on the command line.\n \n rebase.missingCommitsCheck::\n \tIf set to \"warn\", git rebase -i will print a warning if some\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex b18cdbc023..ae6939588d 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -647,12 +647,18 @@ rebase --continue` is invoked. Currently, you cannot pass\n \n --update-refs::\n --no-update-refs::\n+--update-refs=interactive::\n \tAutomatically force-update any branches that point to commits that\n \tare being rebased. Any branches that are checked out in a worktree\n \tare not updated in this way.\n +\n+If `--update-refs=interactive` is specified, the behavior is equivalent to\n+`--update-refs` if the rebase is interactive and `--no-update-refs` otherwise.\n+(This is mainly useful as a configuration setting, although it might also be\n+of use in aliases.)\n++\n If the configuration variable `rebase.updateRefs` is set, then this option\n-can be used to override and disable this setting.\n+can be used to override or disable the configuration.\n +\n See also INCOMPATIBLE OPTIONS below.\n \ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 6c9eaf3788..57b456599b 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -129,10 +129,17 @@ struct rebase_options {\n \tint reschedule_failed_exec;\n \tint reapply_cherry_picks;\n \tint fork_point;\n-\tint update_refs;\n+\t// UPDATE_REFS_{UNKNOWN,NO,ALWAYS} numeric values must never\n+\t// change as post-option-parsing code works with {,config_}update_refs\n+\t// as if they were ints\n+\tenum {\n+\t\tUPDATE_REFS_UNKNOWN = -1,\n+\t\tUPDATE_REFS_NO = 0,\n+\t\tUPDATE_REFS_ALWAYS = 1,\n+\t\tUPDATE_REFS_INTERACTIVE,\n+\t} update_refs, config_update_refs;\n \tint config_autosquash;\n \tint config_rebase_merges;\n-\tint config_update_refs;\n };\n \n #define REBASE_OPTIONS_INIT {\t\t\t  \t\\\n@@ -150,8 +157,8 @@ struct rebase_options {\n \t\t.autosquash = -1,                       \\\n \t\t.rebase_merges = -1,                    \\\n \t\t.config_rebase_merges = -1,             \\\n-\t\t.update_refs = -1,                      \\\n-\t\t.config_update_refs = -1,               \\\n+\t\t.update_refs = UPDATE_REFS_UNKNOWN,     \\\n+\t\t.config_update_refs = UPDATE_REFS_UNKNOWN, \\\n \t\t.strategy_opts = STRING_LIST_INIT_NODUP,\\\n \t}\n \n@@ -412,6 +419,18 @@ static void imply_merge(struct rebase_options *opts, const char *option)\n \t}\n }\n \n+static int coerce_update_refs(const struct rebase_options *opts, int update_refs)\n+{\n+\t/* coerce \"=interactive\" into \"no\" rather than \"not set\" when not interactive\n+\t * this way, `git -c rebase.updateRefs=yes rebase --update-refs=interactive [without -i]`\n+\t * will not inherit the \"yes\" from the config */\n+\tif (update_refs == UPDATE_REFS_INTERACTIVE)\n+\t\treturn (opts->flags & REBASE_INTERACTIVE_EXPLICIT)\n+\t\t       ? UPDATE_REFS_ALWAYS\n+\t\t       : UPDATE_REFS_NO;\n+\treturn update_refs;\n+}\n+\n /* Returns the filename prefixed by the state_dir */\n static const char *state_dir_path(const char *filename, struct rebase_options *opts)\n {\n@@ -779,6 +798,17 @@ static void parse_rebase_merges_value(struct rebase_options *options, const char\n \t\tdie(_(\"Unknown rebase-merges mode: %s\"), value);\n }\n \n+static int parse_update_refs_value(const char *value, const char *desc)\n+{\n+\tint v = git_parse_maybe_bool(value);\n+\tif (v >= 0)\n+\t\treturn v ? UPDATE_REFS_ALWAYS : UPDATE_REFS_NO;\n+\telse if (!strcmp(\"interactive\", value))\n+\t\treturn UPDATE_REFS_INTERACTIVE;\n+\n+\tdie(_(\"bad %s value '%s'; valid values are boolean or \\\"interactive\\\"\"), desc, value);\n+}\n+\n static int rebase_config(const char *var, const char *value,\n \t\t\t const struct config_context *ctx, void *data)\n {\n@@ -821,7 +851,8 @@ static int rebase_config(const char *var, const char *value,\n \t}\n \n \tif (!strcmp(var, \"rebase.updaterefs\")) {\n-\t\topts->config_update_refs = git_config_bool(var, value);\n+\t\topts->config_update_refs = parse_update_refs_value(value,\n+\t\t\t\"rebase.updateRefs\");\n \t\treturn 0;\n \t}\n \n@@ -1042,6 +1073,19 @@ static int parse_opt_rebase_merges(const struct option *opt, const char *arg, in\n \treturn 0;\n }\n \n+static int parse_opt_update_refs(const struct option *opt, const char *arg, int unset)\n+{\n+\tstruct rebase_options *options = opt->value;\n+\n+\tif (arg)\n+\t\toptions->update_refs = parse_update_refs_value(arg,\n+\t\t\t\"--update-refs\");\n+\telse\n+\t\toptions->update_refs = unset ? UPDATE_REFS_NO : UPDATE_REFS_ALWAYS;\n+\n+\treturn 0;\n+}\n+\n static void NORETURN error_on_missing_default_upstream(void)\n {\n \tstruct branch *current_branch = branch_get(NULL);\n@@ -1187,9 +1231,11 @@ int cmd_rebase(int argc,\n \t\tOPT_BOOL(0, \"autosquash\", &options.autosquash,\n \t\t\t N_(\"move commits that begin with \"\n \t\t\t    \"squash!/fixup! under -i\")),\n-\t\tOPT_BOOL(0, \"update-refs\", &options.update_refs,\n-\t\t\t N_(\"update branches that point to commits \"\n-\t\t\t    \"that are being rebased\")),\n+\t\tOPT_CALLBACK_F(0, \"update-refs\", &options,\n+\t\t\tN_(\"(bool|interactive)\"),\n+\t\t\tN_(\"update branches that point to commits \"\n+\t\t\t   \"that are being rebased\"),\n+\t\t\tPARSE_OPT_OPTARG, parse_opt_update_refs),\n \t\t{ OPTION_STRING, 'S', \"gpg-sign\", &gpg_sign, N_(\"key-id\"),\n \t\t\tN_(\"GPG-sign commits\"),\n \t\t\tPARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n@@ -1528,6 +1574,16 @@ int cmd_rebase(int argc,\n \tif (isatty(2) && options.flags & REBASE_NO_QUIET)\n \t\tstrbuf_addstr(&options.git_format_patch_opt, \" --progress\");\n \n+\t/* coerce --update-refs=interactive into yes or no.\n+\t * we do it here because there's just too much code below that handles\n+\t * {,config_}update_refs in one way or another and modifying it to\n+\t * account for the new state would be too invasive.\n+\t * all further code uses {,config_}update_refs as a tristate. */\n+\toptions.update_refs =\n+\t\tcoerce_update_refs(&options, options.update_refs);\n+\toptions.config_update_refs =\n+\t\tcoerce_update_refs(&options, options.config_update_refs);\n+\n \tif (options.git_am_opts.nr || options.type == REBASE_APPLY) {\n \t\t/* all am options except -q are compatible only with --apply */\n \t\tfor (i = options.git_am_opts.nr - 1; i >= 0; i--)\n-- \n2.48.1.5.g9188e14f140\n\n"},{"id":"512197","messageId":"CALnO6CAM7WCOJV8s8ZARi3BAFwkh0TNTCod_YH9s+EpO7t-Qtg@mail.gmail.com","threadId":"62933","inReplyTo":"20250210191650.316329-1-intelfx@intelfx.name","subject":"Re: [PATCH] rebase: add `--update-refs=interactive`","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2025-02-10T20:22:09Z","receivedAt":"2025-02-10T20:22:23Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"On Mon, Feb 10, 2025 at 2:17 PM Ivan Shapovalov <intelfx@intelfx.name> wrote:\n>\n> In rebase-heavy workflows involving multiple interdependent feature\n> branches, typing out `--update-refs` quickly becomes tiring, which\n> can be mitigated with setting the `rebase.updateRefs` git-config option\n> to perform update-refs by default.\n>\n> However, the utility of `rebase.updateRefs` is somewhat limited because\n> you rarely want it in a non-interactive rebase (as it does not give you\n> the chance to review the update-refs candidates, likely leading to\n> updating refs that you didn't want updated -- I made quite an amount\n> of mess by setting this option and subsequently forgetting about it).\n>\n> Try to find a middle ground by introducing a third value,\n> `--update-refs=interactive` (and `rebase.updateRefs=interactive`)\n> which means `--update-refs` when starting an interactive rebase and\n> `--no-update-refs` otherwise. This option is primarily intended to be\n> used in the gitconfig, but is also accepted on the command line\n> for completeness.\n>\n> Signed-off-by: Ivan Shapovalov <intelfx@intelfx.name>\n> ---\n>  Documentation/config/rebase.txt |  7 +++-\n>  Documentation/git-rebase.txt    |  8 +++-\n>  builtin/rebase.c                | 72 +++++++++++++++++++++++++++++----\n>  3 files changed, 77 insertions(+), 10 deletions(-)\n>\n> diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt\n> index c6187ab28b..d8bbaba69a 100644\n> --- a/Documentation/config/rebase.txt\n> +++ b/Documentation/config/rebase.txt\n> @@ -24,7 +24,12 @@ rebase.autoStash::\n>         Defaults to false.\n>\n>  rebase.updateRefs::\n> -       If set to true enable `--update-refs` option by default.\n> +       If set to true, enable the `--update-refs` option of\n> +       linkgit:git-rebase[1] by default. When set to 'interactive',\n> +       only enable `--update-refs` by default for interactive mode\n> +       (equivalent to `--update-refs=interactive`).\n> +       This option can be overridden by specifying any form of\n> +       `--update-refs` on the command line.\n>\n>  rebase.missingCommitsCheck::\n>         If set to \"warn\", git rebase -i will print a warning if some\n> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\n> index b18cdbc023..ae6939588d 100644\n> --- a/Documentation/git-rebase.txt\n> +++ b/Documentation/git-rebase.txt\n> @@ -647,12 +647,18 @@ rebase --continue` is invoked. Currently, you cannot pass\n>\n>  --update-refs::\n>  --no-update-refs::\n> +--update-refs=interactive::\n\nBased on `git grep -e '--.*\\[=' Documentation/git-*.txt`, I think this\nshould be more like\n\n    --update-refs[=interactive]::\n    --no-update-refs::\n\nBut maybe that unintentionally suggests that `=interactive` is the default?\n\n>         Automatically force-update any branches that point to commits that\n>         are being rebased. Any branches that are checked out in a worktree\n>         are not updated in this way.\n>  +\n> +If `--update-refs=interactive` is specified, the behavior is equivalent to\n> +`--update-refs` if the rebase is interactive and `--no-update-refs` otherwise.\n> +(This is mainly useful as a configuration setting, although it might also be\n> +of use in aliases.)\n> ++\n>  If the configuration variable `rebase.updateRefs` is set, then this option\n> -can be used to override and disable this setting.\n> +can be used to override or disable the configuration.\n>  +\n>  See also INCOMPATIBLE OPTIONS below.\n>\n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index 6c9eaf3788..57b456599b 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -129,10 +129,17 @@ struct rebase_options {\n>         int reschedule_failed_exec;\n>         int reapply_cherry_picks;\n>         int fork_point;\n> -       int update_refs;\n> +       // UPDATE_REFS_{UNKNOWN,NO,ALWAYS} numeric values must never\n> +       // change as post-option-parsing code works with {,config_}update_refs\n> +       // as if they were ints\n> +       enum {\n> +               UPDATE_REFS_UNKNOWN = -1,\n> +               UPDATE_REFS_NO = 0,\n> +               UPDATE_REFS_ALWAYS = 1,\n> +               UPDATE_REFS_INTERACTIVE,\n> +       } update_refs, config_update_refs;\n>         int config_autosquash;\n>         int config_rebase_merges;\n> -       int config_update_refs;\n>  };\n>\n>  #define REBASE_OPTIONS_INIT {                          \\\n> @@ -150,8 +157,8 @@ struct rebase_options {\n>                 .autosquash = -1,                       \\\n>                 .rebase_merges = -1,                    \\\n>                 .config_rebase_merges = -1,             \\\n> -               .update_refs = -1,                      \\\n> -               .config_update_refs = -1,               \\\n> +               .update_refs = UPDATE_REFS_UNKNOWN,     \\\n> +               .config_update_refs = UPDATE_REFS_UNKNOWN, \\\n>                 .strategy_opts = STRING_LIST_INIT_NODUP,\\\n>         }\n>\n> @@ -412,6 +419,18 @@ static void imply_merge(struct rebase_options *opts, const char *option)\n>         }\n>  }\n>\n> +static int coerce_update_refs(const struct rebase_options *opts, int update_refs)\n> +{\n> +       /* coerce \"=interactive\" into \"no\" rather than \"not set\" when not interactive\n> +        * this way, `git -c rebase.updateRefs=yes rebase --update-refs=interactive [without -i]`\n> +        * will not inherit the \"yes\" from the config */\n> +       if (update_refs == UPDATE_REFS_INTERACTIVE)\n> +               return (opts->flags & REBASE_INTERACTIVE_EXPLICIT)\n> +                      ? UPDATE_REFS_ALWAYS\n> +                      : UPDATE_REFS_NO;\n> +       return update_refs;\n> +}\n> +\n>  /* Returns the filename prefixed by the state_dir */\n>  static const char *state_dir_path(const char *filename, struct rebase_options *opts)\n>  {\n> @@ -779,6 +798,17 @@ static void parse_rebase_merges_value(struct rebase_options *options, const char\n>                 die(_(\"Unknown rebase-merges mode: %s\"), value);\n>  }\n>\n> +static int parse_update_refs_value(const char *value, const char *desc)\n> +{\n> +       int v = git_parse_maybe_bool(value);\n> +       if (v >= 0)\n> +               return v ? UPDATE_REFS_ALWAYS : UPDATE_REFS_NO;\n> +       else if (!strcmp(\"interactive\", value))\n> +               return UPDATE_REFS_INTERACTIVE;\n> +\n> +       die(_(\"bad %s value '%s'; valid values are boolean or \\\"interactive\\\"\"), desc, value);\n> +}\n> +\n>  static int rebase_config(const char *var, const char *value,\n>                          const struct config_context *ctx, void *data)\n>  {\n> @@ -821,7 +851,8 @@ static int rebase_config(const char *var, const char *value,\n>         }\n>\n>         if (!strcmp(var, \"rebase.updaterefs\")) {\n> -               opts->config_update_refs = git_config_bool(var, value);\n> +               opts->config_update_refs = parse_update_refs_value(value,\n> +                       \"rebase.updateRefs\");\n>                 return 0;\n>         }\n>\n> @@ -1042,6 +1073,19 @@ static int parse_opt_rebase_merges(const struct option *opt, const char *arg, in\n>         return 0;\n>  }\n>\n> +static int parse_opt_update_refs(const struct option *opt, const char *arg, int unset)\n> +{\n> +       struct rebase_options *options = opt->value;\n> +\n> +       if (arg)\n> +               options->update_refs = parse_update_refs_value(arg,\n> +                       \"--update-refs\");\n> +       else\n> +               options->update_refs = unset ? UPDATE_REFS_NO : UPDATE_REFS_ALWAYS;\n> +\n> +       return 0;\n> +}\n> +\n>  static void NORETURN error_on_missing_default_upstream(void)\n>  {\n>         struct branch *current_branch = branch_get(NULL);\n> @@ -1187,9 +1231,11 @@ int cmd_rebase(int argc,\n>                 OPT_BOOL(0, \"autosquash\", &options.autosquash,\n>                          N_(\"move commits that begin with \"\n>                             \"squash!/fixup! under -i\")),\n> -               OPT_BOOL(0, \"update-refs\", &options.update_refs,\n> -                        N_(\"update branches that point to commits \"\n> -                           \"that are being rebased\")),\n> +               OPT_CALLBACK_F(0, \"update-refs\", &options,\n> +                       N_(\"(bool|interactive)\"),\n> +                       N_(\"update branches that point to commits \"\n> +                          \"that are being rebased\"),\n> +                       PARSE_OPT_OPTARG, parse_opt_update_refs),\n>                 { OPTION_STRING, 'S', \"gpg-sign\", &gpg_sign, N_(\"key-id\"),\n>                         N_(\"GPG-sign commits\"),\n>                         PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n> @@ -1528,6 +1574,16 @@ int cmd_rebase(int argc,\n>         if (isatty(2) && options.flags & REBASE_NO_QUIET)\n>                 strbuf_addstr(&options.git_format_patch_opt, \" --progress\");\n>\n> +       /* coerce --update-refs=interactive into yes or no.\n> +        * we do it here because there's just too much code below that handles\n> +        * {,config_}update_refs in one way or another and modifying it to\n> +        * account for the new state would be too invasive.\n> +        * all further code uses {,config_}update_refs as a tristate. */\n> +       options.update_refs =\n> +               coerce_update_refs(&options, options.update_refs);\n> +       options.config_update_refs =\n> +               coerce_update_refs(&options, options.config_update_refs);\n> +\n>         if (options.git_am_opts.nr || options.type == REBASE_APPLY) {\n>                 /* all am options except -q are compatible only with --apply */\n>                 for (i = options.git_am_opts.nr - 1; i >= 0; i--)\n> --\n> 2.48.1.5.g9188e14f140\n>\n>\n\nShould we add a test for this?\n\n-- \nD. Ben Knoble\n"},{"id":"512228","messageId":"bc0de52b59f289e1388f1581fcfa49453365e21a.camel@intelfx.name","threadId":"62933","inReplyTo":"CALnO6CAM7WCOJV8s8ZARi3BAFwkh0TNTCod_YH9s+EpO7t-Qtg@mail.gmail.com","subject":"Re: [PATCH] rebase: add `--update-refs=interactive`","fromName":"Ivan Shapovalov","fromEmail":"intelfx@intelfx.name","sentAt":"2025-02-11T11:33:15Z","receivedAt":"2025-02-11T11:33:20Z","isPatch":true,"sender":{"key":"intelfx@intelfx.name","avatar":"https://gravatar.com/avatar/fb0d6e45051c5d8cda450fd6d02ecba0216f5f3aae1ebcf5de522939ff18eef3?d=mp&s=160"},"body":"On 2025-02-10 at 15:22 -0500, D. Ben Knoble wrote:\n> On Mon, Feb 10, 2025 at 2:17 PM Ivan Shapovalov <intelfx@intelfx.name> wrote:\n> > \n> > In rebase-heavy workflows involving multiple interdependent feature\n> > branches, typing out `--update-refs` quickly becomes tiring, which\n> > can be mitigated with setting the `rebase.updateRefs` git-config option\n> > to perform update-refs by default.\n> > \n> > However, the utility of `rebase.updateRefs` is somewhat limited because\n> > you rarely want it in a non-interactive rebase (as it does not give you\n> > the chance to review the update-refs candidates, likely leading to\n> > updating refs that you didn't want updated -- I made quite an amount\n> > of mess by setting this option and subsequently forgetting about it).\n> > \n> > Try to find a middle ground by introducing a third value,\n> > `--update-refs=interactive` (and `rebase.updateRefs=interactive`)\n> > which means `--update-refs` when starting an interactive rebase and\n> > `--no-update-refs` otherwise. This option is primarily intended to be\n> > used in the gitconfig, but is also accepted on the command line\n> > for completeness.\n> > \n> > Signed-off-by: Ivan Shapovalov <intelfx@intelfx.name>\n> > ---\n> >  Documentation/config/rebase.txt |  7 +++-\n> >  Documentation/git-rebase.txt    |  8 +++-\n> >  builtin/rebase.c                | 72 +++++++++++++++++++++++++++++----\n> >  3 files changed, 77 insertions(+), 10 deletions(-)\n> > \n> > diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt\n> > index c6187ab28b..d8bbaba69a 100644\n> > --- a/Documentation/config/rebase.txt\n> > +++ b/Documentation/config/rebase.txt\n> > @@ -24,7 +24,12 @@ rebase.autoStash::\n> >         Defaults to false.\n> > \n> >  rebase.updateRefs::\n> > -       If set to true enable `--update-refs` option by default.\n> > +       If set to true, enable the `--update-refs` option of\n> > +       linkgit:git-rebase[1] by default. When set to 'interactive',\n> > +       only enable `--update-refs` by default for interactive mode\n> > +       (equivalent to `--update-refs=interactive`).\n> > +       This option can be overridden by specifying any form of\n> > +       `--update-refs` on the command line.\n> > \n> >  rebase.missingCommitsCheck::\n> >         If set to \"warn\", git rebase -i will print a warning if some\n> > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\n> > index b18cdbc023..ae6939588d 100644\n> > --- a/Documentation/git-rebase.txt\n> > +++ b/Documentation/git-rebase.txt\n> > @@ -647,12 +647,18 @@ rebase --continue` is invoked. Currently, you cannot pass\n> > \n> >  --update-refs::\n> >  --no-update-refs::\n> > +--update-refs=interactive::\n> \n> Based on `git grep -e '--.*\\[=' Documentation/git-*.txt`, I think this\n> should be more like\n> \n>     --update-refs[=interactive]::\n>     --no-update-refs::\n> \n> But maybe that unintentionally suggests that `=interactive` is the default?\n\nPerhaps --update-refs[=(yes|no|interactive)] then? Or is that too\nverbose? Anyway, I don't have a preference, I'll just do what I'm told\nhere.\n\n> \n> >         Automatically force-update any branches that point to commits that\n> >         are being rebased. Any branches that are checked out in a worktree\n> >         are not updated in this way.\n> >  +\n> > +If `--update-refs=interactive` is specified, the behavior is equivalent to\n> > +`--update-refs` if the rebase is interactive and `--no-update-refs` otherwise.\n> > +(This is mainly useful as a configuration setting, although it might also be\n> > +of use in aliases.)\n> > ++\n> >  If the configuration variable `rebase.updateRefs` is set, then this option\n> > -can be used to override and disable this setting.\n> > +can be used to override or disable the configuration.\n> >  +\n> >  See also INCOMPATIBLE OPTIONS below.\n> > \n> > diff --git a/builtin/rebase.c b/builtin/rebase.c\n> > index 6c9eaf3788..57b456599b 100644\n> > --- a/builtin/rebase.c\n> > +++ b/builtin/rebase.c\n> > @@ -129,10 +129,17 @@ struct rebase_options {\n> >         int reschedule_failed_exec;\n> >         int reapply_cherry_picks;\n> >         int fork_point;\n> > -       int update_refs;\n> > +       // UPDATE_REFS_{UNKNOWN,NO,ALWAYS} numeric values must never\n> > +       // change as post-option-parsing code works with {,config_}update_refs\n> > +       // as if they were ints\n> > +       enum {\n> > +               UPDATE_REFS_UNKNOWN = -1,\n> > +               UPDATE_REFS_NO = 0,\n> > +               UPDATE_REFS_ALWAYS = 1,\n> > +               UPDATE_REFS_INTERACTIVE,\n> > +       } update_refs, config_update_refs;\n> >         int config_autosquash;\n> >         int config_rebase_merges;\n> > -       int config_update_refs;\n> >  };\n> > \n> >  #define REBASE_OPTIONS_INIT {                          \\\n> > @@ -150,8 +157,8 @@ struct rebase_options {\n> >                 .autosquash = -1,                       \\\n> >                 .rebase_merges = -1,                    \\\n> >                 .config_rebase_merges = -1,             \\\n> > -               .update_refs = -1,                      \\\n> > -               .config_update_refs = -1,               \\\n> > +               .update_refs = UPDATE_REFS_UNKNOWN,     \\\n> > +               .config_update_refs = UPDATE_REFS_UNKNOWN, \\\n> >                 .strategy_opts = STRING_LIST_INIT_NODUP,\\\n> >         }\n> > \n> > @@ -412,6 +419,18 @@ static void imply_merge(struct rebase_options *opts, const char *option)\n> >         }\n> >  }\n> > \n> > +static int coerce_update_refs(const struct rebase_options *opts, int update_refs)\n> > +{\n> > +       /* coerce \"=interactive\" into \"no\" rather than \"not set\" when not interactive\n> > +        * this way, `git -c rebase.updateRefs=yes rebase --update-refs=interactive [without -i]`\n> > +        * will not inherit the \"yes\" from the config */\n> > +       if (update_refs == UPDATE_REFS_INTERACTIVE)\n> > +               return (opts->flags & REBASE_INTERACTIVE_EXPLICIT)\n> > +                      ? UPDATE_REFS_ALWAYS\n> > +                      : UPDATE_REFS_NO;\n> > +       return update_refs;\n> > +}\n> > +\n> >  /* Returns the filename prefixed by the state_dir */\n> >  static const char *state_dir_path(const char *filename, struct rebase_options *opts)\n> >  {\n> > @@ -779,6 +798,17 @@ static void parse_rebase_merges_value(struct rebase_options *options, const char\n> >                 die(_(\"Unknown rebase-merges mode: %s\"), value);\n> >  }\n> > \n> > +static int parse_update_refs_value(const char *value, const char *desc)\n> > +{\n> > +       int v = git_parse_maybe_bool(value);\n> > +       if (v >= 0)\n> > +               return v ? UPDATE_REFS_ALWAYS : UPDATE_REFS_NO;\n> > +       else if (!strcmp(\"interactive\", value))\n> > +               return UPDATE_REFS_INTERACTIVE;\n> > +\n> > +       die(_(\"bad %s value '%s'; valid values are boolean or \\\"interactive\\\"\"), desc, value);\n> > +}\n> > +\n> >  static int rebase_config(const char *var, const char *value,\n> >                          const struct config_context *ctx, void *data)\n> >  {\n> > @@ -821,7 +851,8 @@ static int rebase_config(const char *var, const char *value,\n> >         }\n> > \n> >         if (!strcmp(var, \"rebase.updaterefs\")) {\n> > -               opts->config_update_refs = git_config_bool(var, value);\n> > +               opts->config_update_refs = parse_update_refs_value(value,\n> > +                       \"rebase.updateRefs\");\n> >                 return 0;\n> >         }\n> > \n> > @@ -1042,6 +1073,19 @@ static int parse_opt_rebase_merges(const struct option *opt, const char *arg, in\n> >         return 0;\n> >  }\n> > \n> > +static int parse_opt_update_refs(const struct option *opt, const char *arg, int unset)\n> > +{\n> > +       struct rebase_options *options = opt->value;\n> > +\n> > +       if (arg)\n> > +               options->update_refs = parse_update_refs_value(arg,\n> > +                       \"--update-refs\");\n> > +       else\n> > +               options->update_refs = unset ? UPDATE_REFS_NO : UPDATE_REFS_ALWAYS;\n> > +\n> > +       return 0;\n> > +}\n> > +\n> >  static void NORETURN error_on_missing_default_upstream(void)\n> >  {\n> >         struct branch *current_branch = branch_get(NULL);\n> > @@ -1187,9 +1231,11 @@ int cmd_rebase(int argc,\n> >                 OPT_BOOL(0, \"autosquash\", &options.autosquash,\n> >                          N_(\"move commits that begin with \"\n> >                             \"squash!/fixup! under -i\")),\n> > -               OPT_BOOL(0, \"update-refs\", &options.update_refs,\n> > -                        N_(\"update branches that point to commits \"\n> > -                           \"that are being rebased\")),\n> > +               OPT_CALLBACK_F(0, \"update-refs\", &options,\n> > +                       N_(\"(bool|interactive)\"),\n> > +                       N_(\"update branches that point to commits \"\n> > +                          \"that are being rebased\"),\n> > +                       PARSE_OPT_OPTARG, parse_opt_update_refs),\n> >                 { OPTION_STRING, 'S', \"gpg-sign\", &gpg_sign, N_(\"key-id\"),\n> >                         N_(\"GPG-sign commits\"),\n> >                         PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n> > @@ -1528,6 +1574,16 @@ int cmd_rebase(int argc,\n> >         if (isatty(2) && options.flags & REBASE_NO_QUIET)\n> >                 strbuf_addstr(&options.git_format_patch_opt, \" --progress\");\n> > \n> > +       /* coerce --update-refs=interactive into yes or no.\n> > +        * we do it here because there's just too much code below that handles\n> > +        * {,config_}update_refs in one way or another and modifying it to\n> > +        * account for the new state would be too invasive.\n> > +        * all further code uses {,config_}update_refs as a tristate. */\n> > +       options.update_refs =\n> > +               coerce_update_refs(&options, options.update_refs);\n> > +       options.config_update_refs =\n> > +               coerce_update_refs(&options, options.config_update_refs);\n> > +\n> >         if (options.git_am_opts.nr || options.type == REBASE_APPLY) {\n> >                 /* all am options except -q are compatible only with --apply */\n> >                 for (i = options.git_am_opts.nr - 1; i >= 0; i--)\n> > --\n> > 2.48.1.5.g9188e14f140\n> > \n> > \n> \n> Should we add a test for this?\n> \n\nAny suggestions what exactly I should test here? I don't have much\nexperience testing interactive CLI tools, so I'd appreciate some hints.\n\n-- \nIvan Shapovalov / intelfx /\n"},{"id":"512234","messageId":"1279671f-4063-4347-b153-9f6ff079bd77@gmail.com","threadId":"62933","inReplyTo":"20250210191650.316329-1-intelfx@intelfx.name","subject":"Re: [PATCH] rebase: add `--update-refs=interactive`","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-02-11T14:36:03Z","receivedAt":"2025-02-11T14:36:06Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ivan\n\nOn 10/02/2025 19:16, Ivan Shapovalov wrote:\n> In rebase-heavy workflows involving multiple interdependent feature\n> branches, typing out `--update-refs` quickly becomes tiring, which\n> can be mitigated with setting the `rebase.updateRefs` git-config option\n> to perform update-refs by default.\n> \n> However, the utility of `rebase.updateRefs` is somewhat limited because\n> you rarely want it in a non-interactive rebase (as it does not give you\n> the chance to review the update-refs candidates, likely leading to\n> updating refs that you didn't want updated -- I made quite an amount\n> of mess by setting this option and subsequently forgetting about it).\n\nI'm a bit surprised by this - I'd have thought there is more scope for \nmessing things up by making a mistake when editing the todo list that \nfor the non-interactive case. Are you able to explain a in a bit more \ndetail the problem you have been experiencing please?\n\n> Try to find a middle ground by introducing a third value,\n> `--update-refs=interactive` (and `rebase.updateRefs=interactive`)\n> which means `--update-refs` when starting an interactive rebase and\n> `--no-update-refs` otherwise. This option is primarily intended to be\n> used in the gitconfig, but is also accepted on the command line\n> for completeness.\n\nI'm not convinced allowing \"--update-refs=interactive\" on the \ncommandline improves the usability - why wouldn't I just say \n\"--update-refs\" if I want to update all the branches or \n\"--no-update-refs\" if I don't? I also think supporting \n--update-refs=(true|false) is verbose and unnecessary as the user can \nalready specify their intent with the existing option.\n\n>   rebase.updateRefs::\n> -\tIf set to true enable `--update-refs` option by default.\n> +\tIf set to true, enable the `--update-refs` option of\n> +\tlinkgit:git-rebase[1] by default. When set to 'interactive',\n\nOur existing documentation is inconsistent in how it formats config \nvalues. rebase.backend uses \"apply\", rebase.rebaseMerges uses \n`rebase-cousins` which I think matches other commands and is therefore \nwhat we should use here and rebase.missingCommitCheck uses a mixture \nwith \"warn\" and `drop`.\n\n> +\tonly enable `--update-refs` by default for interactive mode\n> +\t(equivalent to `--update-refs=interactive`).\n> +\tThis option can be overridden by specifying any form of\n> +\t`--update-refs` on the command line.\n\n> @@ -129,10 +129,17 @@ struct rebase_options {\n>   \tint reschedule_failed_exec;\n>   \tint reapply_cherry_picks;\n>   \tint fork_point;\n> -\tint update_refs;\n> +\t// UPDATE_REFS_{UNKNOWN,NO,ALWAYS} numeric values must never\n> +\t// change as post-option-parsing code works with {,config_}update_refs\n> +\t// as if they were ints\n\nThis feels a bit fragile - why can't we update the code to use the enum? \nAlso note that comments should be formatted as\n\n/* single line comment */\n\nor\n\n/*\n  * multi-line\n  * comment\n  */\n\n> +\tenum {\n> +\t\tUPDATE_REFS_UNKNOWN = -1,\n> +\t\tUPDATE_REFS_NO = 0,\n> +\t\tUPDATE_REFS_ALWAYS = 1,\n> +\t\tUPDATE_REFS_INTERACTIVE,\n> +\t} update_refs, config_update_refs;\n\nI don't think we want to change the type of `update_refs` as I'm not \nconvinced we want to change the commandline option.\n\n> +static int coerce_update_refs(const struct rebase_options *opts, int update_refs)\n\nI'd be tempted to call this \"should_update_refs(...)\"\n\n> +{\n> +\t/* coerce \"=interactive\" into \"no\" rather than \"not set\" when not interactive\n> +\t * this way, `git -c rebase.updateRefs=yes rebase --update-refs=interactive [without -i]`\n> +\t * will not inherit the \"yes\" from the config */\n\nStyle - see above\n\n> +\tif (update_refs == UPDATE_REFS_INTERACTIVE)\n> +\t\treturn (opts->flags & REBASE_INTERACTIVE_EXPLICIT)\n> +\t\t       ? UPDATE_REFS_ALWAYS\n> +\t\t       : UPDATE_REFS_NO;\n> +\treturn update_refs;\n> +}\n> [...]   \n> +static int parse_update_refs_value(const char *value, const char *desc)\n> +{\n> +\tint v = git_parse_maybe_bool(value);\n\nStyle: there should be a blank line after the variable declarations at \nthe start of a function.\n\n> +\tif (v >= 0)\n> +\t\treturn v ? UPDATE_REFS_ALWAYS : UPDATE_REFS_NO;\n> +\telse if (!strcmp(\"interactive\", value))\n> +\t\treturn UPDATE_REFS_INTERACTIVE;\n> +\n> +\tdie(_(\"bad %s value '%s'; valid values are boolean or \\\"interactive\\\"\"), desc, value);\n\nI think we normally say \"invalid\" or \"unknown\" rather than \"bad\" in our \nerror messages. It'd be clearer just to list the possible values as \nthere are only three of them.\n\n> +\t/* coerce --update-refs=interactive into yes or no.\n> +\t * we do it here because there's just too much code below that handles\n> +\t * {,config_}update_refs in one way or another and modifying it to\n> +\t * account for the new state would be too invasive.\n> +\t * all further code uses {,config_}update_refs as a tristate. */\n\nI think we need to find a cleaner way of handling this. There are only \ntwo mentions of options.config_update_refs below this point - is it \nreally so difficult for those to use the enum?\n\nGiven a bit more detail I could be convinced that the config option is \nuseful but I don't think we should be changing the commandline option.\n\nBest Wishes\n\nPhillip\n\n"},{"id":"512238","messageId":"xmqqfrkk1l4i.fsf@gitster.g","threadId":"62933","inReplyTo":"bc0de52b59f289e1388f1581fcfa49453365e21a.camel@intelfx.name","subject":"Re: [PATCH] rebase: add `--update-refs=interactive`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-11T16:50:21Z","receivedAt":"2025-02-11T16:50:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ivan Shapovalov <intelfx@intelfx.name> writes:\n\n>> >  --update-refs::\n>> >  --no-update-refs::\n>> > +--update-refs=interactive::\n>> \n>> Based on `git grep -e '--.*\\[=' Documentation/git-*.txt`, I think this\n>> should be more like\n>> \n>>     --update-refs[=interactive]::\n>>     --no-update-refs::\n>> \n>> But maybe that unintentionally suggests that `=interactive` is the default?\n>\n> Perhaps --update-refs[=(yes|no|interactive)] then? Or is that too\n> verbose?\n\nIf `--update-refs` does take values that the git_parse_maybe_bool()\nhelper parses as a Boolean value, I do not think the above is\nverbose at all.  Rather, it is a disservice to the users if the\ndocumentation does not mention yes/no in such a case.  I'd say\nlisting other Boolean synonyms like yes/true/on/no/false/off is\ntoo verbose, though ;-).\n\n> Anyway, I don't have a preference, I'll just do what I'm told\n\nThat is not quite in line with how we'd like to operate.\n\nIt is your itch.  Others may give suggestions to help you polish it,\nbut ultimately, we would not want to accept a patch that the author\ndoes not agree with.\n\nThanks.\n"},{"id":"512242","messageId":"a761826ddafbadac6d2932f145316493298da33c.camel@intelfx.name","threadId":"62933","inReplyTo":"xmqqfrkk1l4i.fsf@gitster.g","subject":"Re: [PATCH] rebase: add `--update-refs=interactive`","fromName":"Ivan Shapovalov","fromEmail":"intelfx@intelfx.name","sentAt":"2025-02-11T17:36:13Z","receivedAt":"2025-02-11T17:36:20Z","isPatch":true,"sender":{"key":"intelfx@intelfx.name","avatar":"https://gravatar.com/avatar/fb0d6e45051c5d8cda450fd6d02ecba0216f5f3aae1ebcf5de522939ff18eef3?d=mp&s=160"},"body":"On 2025-02-11 at 08:50 -0800, Junio C Hamano wrote:\n> Ivan Shapovalov <intelfx@intelfx.name> writes:\n> \n> > > >  --update-refs::\n> > > >  --no-update-refs::\n> > > > +--update-refs=interactive::\n> > > \n> > > Based on `git grep -e '--.*\\[=' Documentation/git-*.txt`, I think this\n> > > should be more like\n> > > \n> > >     --update-refs[=interactive]::\n> > >     --no-update-refs::\n> > > \n> > > But maybe that unintentionally suggests that `=interactive` is the default?\n> > \n> > Perhaps --update-refs[=(yes|no|interactive)] then? Or is that too\n> > verbose?\n> \n> If `--update-refs` does take values that the git_parse_maybe_bool()\n> helper parses as a Boolean value, I do not think the above is\n> verbose at all.  Rather, it is a disservice to the users if the\n> documentation does not mention yes/no in such a case.  I'd say\n> listing other Boolean synonyms like yes/true/on/no/false/off is\n> too verbose, though ;-).\n> \n> > Anyway, I don't have a preference, I'll just do what I'm told\n> \n> That is not quite in line with how we'd like to operate.\n> \n> It is your itch.  Others may give suggestions to help you polish it,\n> but ultimately, we would not want to accept a patch that the author\n> does not agree with.\n\nOf course, I care about the patch and the feature; what I wanted to say\nis that I do not care (comparatively) about the formatting of the help\ntext: I couldn't figure it out on my own, so whatever you tell me is\nthe proper way of formatting it, I'll do.\n\n-- \nIvan Shapovalov / intelfx /\n"},{"id":"512249","messageId":"f689c263ead8104ec42f63f1e9ed10350a27ae1d.camel@intelfx.name","threadId":"62933","inReplyTo":"1279671f-4063-4347-b153-9f6ff079bd77@gmail.com","subject":"Re: [PATCH] rebase: add `--update-refs=interactive`","fromName":"Ivan Shapovalov","fromEmail":"intelfx@intelfx.name","sentAt":"2025-02-11T18:11:45Z","receivedAt":"2025-02-11T18:11:51Z","isPatch":true,"sender":{"key":"intelfx@intelfx.name","avatar":"https://gravatar.com/avatar/fb0d6e45051c5d8cda450fd6d02ecba0216f5f3aae1ebcf5de522939ff18eef3?d=mp&s=160"},"body":"On 2025-02-11 at 14:36 +0000, Phillip Wood wrote:\n> Hi Ivan\n> \n> On 10/02/2025 19:16, Ivan Shapovalov wrote:\n> > In rebase-heavy workflows involving multiple interdependent feature\n> > branches, typing out `--update-refs` quickly becomes tiring, which\n> > can be mitigated with setting the `rebase.updateRefs` git-config option\n> > to perform update-refs by default.\n> > \n> > However, the utility of `rebase.updateRefs` is somewhat limited because\n> > you rarely want it in a non-interactive rebase (as it does not give you\n> > the chance to review the update-refs candidates, likely leading to\n> > updating refs that you didn't want updated -- I made quite an amount\n> > of mess by setting this option and subsequently forgetting about it).\n> \n> I'm a bit surprised by this - I'd have thought there is more scope for \n> messing things up by making a mistake when editing the todo list that \n> for the non-interactive case. Are you able to explain a in a bit more \n> detail the problem you have been experiencing please?\n\nI often find myself managing multiple interdependent downstream patch\nbranches, rebasing them en masse from release to release. Eventually,\nI found myself typing `git rebase -i --update-refs` more often than\nnot, so I just stuck it into the config as `rebase.updateRefs=true`.\n\nHowever, sometimes I also maintain those patch branches for multiple\nreleases. Consider a (hypothetical) situation:\n\n- tag v1\n- tag v2\n- branch work/myfeature-v1 that is based on tag v1\n\nNow, I want to rebase myfeature onto v2, so I do this:\n\n$ git checkout work/myfeature-v1\n$ git checkout -b work/myfeature-v2\n$ git rebase --onto v2 v1 work/myfeature-v2\n\nWith `rebase.updateRefs=true`, this ends up silently updating _both_\nwork/myfeature-v2 and work/myfeature-v1.\n\nWith this in mind, I wrote this patch such that update-refs only\nhappens for interactive rebases, when I have the chance to inspect the\ntodo list and prune unwanted update-refs items.\n\nDoes this make sense? I made an attempt to explain this motivation in\nthe commit message, so if this does make sense but the commit message\ndoesn't, please tell me how to improve/expand the latter.\n\n> \n> > Try to find a middle ground by introducing a third value,\n> > `--update-refs=interactive` (and `rebase.updateRefs=interactive`)\n> > which means `--update-refs` when starting an interactive rebase and\n> > `--no-update-refs` otherwise. This option is primarily intended to be\n> > used in the gitconfig, but is also accepted on the command line\n> > for completeness.\n> \n> I'm not convinced allowing \"--update-refs=interactive\" on the \n> commandline improves the usability - why wouldn't I just say \n> \"--update-refs\" if I want to update all the branches or \n> \"--no-update-refs\" if I don't? I also think supporting \n> --update-refs=(true|false) is verbose and unnecessary as the user can \n> already specify their intent with the existing option.\n\nI make heavy use of aliases for various workflows, which invoke one\nanother (making use of the ability to override earlier command-line\noptions with the latter ones), and the ability to spell out\n`alias.myRebase = rebase ... --update-refs=interactive ...` was useful.\n\nRe: specifying `=(true|false)`, the intention was to avoid unnecessary\ndivergence, both in UX and code (and reuse the parser to simplify said\ncode). If you think it will be harmful, I'll remove that.\n\n> \n> >   rebase.updateRefs::\n> > -\tIf set to true enable `--update-refs` option by default.\n> > +\tIf set to true, enable the `--update-refs` option of\n> > +\tlinkgit:git-rebase[1] by default. When set to 'interactive',\n> \n> Our existing documentation is inconsistent in how it formats config \n> values. rebase.backend uses \"apply\", rebase.rebaseMerges uses \n> `rebase-cousins` which I think matches other commands and is therefore \n> what we should use here and rebase.missingCommitCheck uses a mixture \n> with \"warn\" and `drop`.\n\nApologies, I'm not sure I understood what exactly you were suggesting\nhere. Did you mean to suggest wrapping \"interactive\" in backticks\ninstead of single quotes?\n\n> \n> > +\tonly enable `--update-refs` by default for interactive mode\n> > +\t(equivalent to `--update-refs=interactive`).\n> > +\tThis option can be overridden by specifying any form of\n> > +\t`--update-refs` on the command line.\n> \n> > @@ -129,10 +129,17 @@ struct rebase_options {\n> >   \tint reschedule_failed_exec;\n> >   \tint reapply_cherry_picks;\n> >   \tint fork_point;\n> > -\tint update_refs;\n> > +\t// UPDATE_REFS_{UNKNOWN,NO,ALWAYS} numeric values must never\n> > +\t// change as post-option-parsing code works with {,config_}update_refs\n> > +\t// as if they were ints\n> \n> This feels a bit fragile - why can't we update the code to use the enum? \n\nIt'd just be a lot of code to update, incl. stylistically (especially\nthe implicit truthy/falsy coercions or `>= 0`). I opted to make the\nchange as non-invasive as possible.\n\n> Also note that comments should be formatted as\n> \n> /* single line comment */\n> \n> or\n> \n> /*\n>   * multi-line\n>   * comment\n>   */\n\nOK\n\n> \n> > +\tenum {\n> > +\t\tUPDATE_REFS_UNKNOWN = -1,\n> > +\t\tUPDATE_REFS_NO = 0,\n> > +\t\tUPDATE_REFS_ALWAYS = 1,\n> > +\t\tUPDATE_REFS_INTERACTIVE,\n> > +\t} update_refs, config_update_refs;\n> \n> I don't think we want to change the type of `update_refs` as I'm not \n> convinced we want to change the commandline option.\n> \n> > +static int coerce_update_refs(const struct rebase_options *opts, int update_refs)\n> \n> I'd be tempted to call this \"should_update_refs(...)\"\n\nOK\n\n> \n> > +{\n> > +\t/* coerce \"=interactive\" into \"no\" rather than \"not set\" when not interactive\n> > +\t * this way, `git -c rebase.updateRefs=yes rebase --update-refs=interactive [without -i]`\n> > +\t * will not inherit the \"yes\" from the config */\n> \n> Style - see above\n\nOK\n\n> \n> > +\tif (update_refs == UPDATE_REFS_INTERACTIVE)\n> > +\t\treturn (opts->flags & REBASE_INTERACTIVE_EXPLICIT)\n> > +\t\t       ? UPDATE_REFS_ALWAYS\n> > +\t\t       : UPDATE_REFS_NO;\n> > +\treturn update_refs;\n> > +}\n> > [...]   \n> > +static int parse_update_refs_value(const char *value, const char *desc)\n> > +{\n> > +\tint v = git_parse_maybe_bool(value);\n> \n> Style: there should be a blank line after the variable declarations at \n> the start of a function.\n\nOK\n\n> \n> > +\tif (v >= 0)\n> > +\t\treturn v ? UPDATE_REFS_ALWAYS : UPDATE_REFS_NO;\n> > +\telse if (!strcmp(\"interactive\", value))\n> > +\t\treturn UPDATE_REFS_INTERACTIVE;\n> > +\n> > +\tdie(_(\"bad %s value '%s'; valid values are boolean or \\\"interactive\\\"\"), desc, value);\n> \n> I think we normally say \"invalid\" or \"unknown\" rather than \"bad\" in our \n> error messages. It'd be clearer just to list the possible values as \n> there are only three of them.\n\nIt's not just three (see other review from Junio), otherwise OK\n\n> \n> > +\t/* coerce --update-refs=interactive into yes or no.\n> > +\t * we do it here because there's just too much code below that handles\n> > +\t * {,config_}update_refs in one way or another and modifying it to\n> > +\t * account for the new state would be too invasive.\n> > +\t * all further code uses {,config_}update_refs as a tristate. */\n> \n> I think we need to find a cleaner way of handling this. There are only \n> two mentions of options.config_update_refs below this point - is it \n> really so difficult for those to use the enum?\n\nSee above; I opted to make this change as non-invasive as possible and\nkeep the complex argument validation logic (lines 1599, 1606-1609)\nintact because I'm not even sure I understand it right.\n\nBesides, even if I convert those uses to use enumerators, I still\nwouldn't want to deal with non-tristate values beyond this point.\n\n-- \nIvan Shapovalov / intelfx /\n\n> \n> Given a bit more detail I could be convinced that the config option is \n> useful but I don't think we should be changing the commandline option.\n> \n> Best Wishes\n> \n> Phillip\n> \n"},{"id":"512252","messageId":"CALnO6CDN627+SUC6BWBvVjFnU5qKsBrfLkmX2okv8J8+wgDDRA@mail.gmail.com","threadId":"62933","inReplyTo":"bc0de52b59f289e1388f1581fcfa49453365e21a.camel@intelfx.name","subject":"Re: [PATCH] rebase: add `--update-refs=interactive`","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2025-02-11T19:28:57Z","receivedAt":"2025-02-11T19:29:10Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"On Tue, Feb 11, 2025 at 6:33 AM Ivan Shapovalov <intelfx@intelfx.name> wrote:\n>\n> On 2025-02-10 at 15:22 -0500, D. Ben Knoble wrote:\n> >\n> > Based on `git grep -e '--.*\\[=' Documentation/git-*.txt`, I think this\n> > should be more like\n> >\n> >     --update-refs[=interactive]::\n> >     --no-update-refs::\n> >\n> > But maybe that unintentionally suggests that `=interactive` is the default?\n>\n> Perhaps --update-refs[=(yes|no|interactive)] then? Or is that too\n> verbose? Anyway, I don't have a preference, I'll just do what I'm told\n> here.\n\nI don't have a strong opinion, and I think this is being discussed\nelsewhere in this thread.\n\n> > Should we add a test for this?\n> >\n>\n> Any suggestions what exactly I should test here? I don't have much\n> experience testing interactive CLI tools, so I'd appreciate some hints.\n>\n> --\n> Ivan Shapovalov / intelfx /\n\nGive t/README a glance; t3404 is probably a good place to start given\n\"git grep update-refs t.\"\n\n\n\n-- \nD. Ben Knoble\n"},{"id":"512253","messageId":"CALnO6CCNWz2o+qa+oMNFAAvkGY8n9tBMPkgdz8wah5eHo1hoTQ@mail.gmail.com","threadId":"62933","inReplyTo":"CALnO6CDN627+SUC6BWBvVjFnU5qKsBrfLkmX2okv8J8+wgDDRA@mail.gmail.com","subject":"Re: [PATCH] rebase: add `--update-refs=interactive`","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2025-02-11T19:29:43Z","receivedAt":"2025-02-11T19:29:56Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"On Tue, Feb 11, 2025 at 2:28 PM D. Ben Knoble <ben.knoble@gmail.com> wrote:\n>\n> On Tue, Feb 11, 2025 at 6:33 AM Ivan Shapovalov <intelfx@intelfx.name> wrote:\n> >\n> > On 2025-02-10 at 15:22 -0500, D. Ben Knoble wrote:\n> > > Should we add a test for this?\n> > >\n> >\n> > Any suggestions what exactly I should test here? I don't have much\n> > experience testing interactive CLI tools, so I'd appreciate some hints.\n> >\n> > --\n> > Ivan Shapovalov / intelfx /\n>\n> Give t/README a glance; t3404 is probably a good place to start given\n> \"git grep update-refs t.\"\n\nBut I really meant to write—let's hash out the other details before\nworrying about a test ;)\n\n-- \nD. Ben Knoble\n"},{"id":"512316","messageId":"5b605c3e-ef6a-433a-9637-1e8f277dfde9@gmail.com","threadId":"62933","inReplyTo":"f689c263ead8104ec42f63f1e9ed10350a27ae1d.camel@intelfx.name","subject":"Re: [PATCH] rebase: add `--update-refs=interactive`","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-02-12T14:26:52Z","receivedAt":"2025-02-12T14:26:56Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ivan\n\nOn 11/02/2025 18:11, Ivan Shapovalov wrote:\n> On 2025-02-11 at 14:36 +0000, Phillip Wood wrote:\n>> On 10/02/2025 19:16, Ivan Shapovalov wrote:\n>>\n>> I'm a bit surprised by this - I'd have thought there is more scope for\n>> messing things up by making a mistake when editing the todo list that\n>> for the non-interactive case. Are you able to explain a in a bit more\n>> detail the problem you have been experiencing please?\n> \n> I often find myself managing multiple interdependent downstream patch\n> branches, rebasing them en masse from release to release. Eventually,\n> I found myself typing `git rebase -i --update-refs` more often than\n> not, so I just stuck it into the config as `rebase.updateRefs=true`.\n> \n> However, sometimes I also maintain those patch branches for multiple\n> releases. Consider a (hypothetical) situation:\n> \n> - tag v1\n> - tag v2\n> - branch work/myfeature-v1 that is based on tag v1\n> \n> Now, I want to rebase myfeature onto v2, so I do this:\n> \n> $ git checkout work/myfeature-v1\n> $ git checkout -b work/myfeature-v2\n> $ git rebase --onto v2 v1 work/myfeature-v2\n> \n> With `rebase.updateRefs=true`, this ends up silently updating _both_\n> work/myfeature-v2 and work/myfeature-v1.\n\nThanks for the explanation. So this is about copying a branch and then \nrebasing the copy without updating the original. A while ago there was a \ndiscussion[1] about excluding branches that match HEAD from \n\"--update-refs\". Maybe we should revisit that with a view to adding a \nconfig setting that excludes copies of the current branch from \n\"--update-refs\".\n\nMaintaining multiple versions of the same branch sounds like a lot of \nwork - whats the advantage over merging a single branch into each release?\n\n[1] \nhttps://lore.kernel.org/git/adb7f680-5bfa-6fa5-6d8a-61323fee7f53@haller-berlin.de/\n\n> With this in mind, I wrote this patch such that update-refs only\n> happens for interactive rebases, when I have the chance to inspect the\n> todo list and prune unwanted update-refs items.\n> \n> Does this make sense? I made an attempt to explain this motivation in\n> the commit message, so if this does make sense but the commit message\n> doesn't, please tell me how to improve/expand the latter.\n\nI think having the example in the commit message would help - I feel \nlike I've now got a clear idea of the problem you are facing whereas I \ndidn't understand what the issue was just from the commit message.\n\n>>> Try to find a middle ground by introducing a third value,\n>>> `--update-refs=interactive` (and `rebase.updateRefs=interactive`)\n>>> which means `--update-refs` when starting an interactive rebase and\n>>> `--no-update-refs` otherwise. This option is primarily intended to be\n>>> used in the gitconfig, but is also accepted on the command line\n>>> for completeness.\n>>\n>> I'm not convinced allowing \"--update-refs=interactive\" on the\n>> commandline improves the usability - why wouldn't I just say\n>> \"--update-refs\" if I want to update all the branches or\n>> \"--no-update-refs\" if I don't? I also think supporting\n>> --update-refs=(true|false) is verbose and unnecessary as the user can\n>> already specify their intent with the existing option.\n> \n> I make heavy use of aliases for various workflows, which invoke one\n> another (making use of the ability to override earlier command-line\n> options with the latter ones), and the ability to spell out\n> `alias.myRebase = rebase ... --update-refs=interactive ...` was useful.\n\nYou can write your alias as\n\n    alias.myRebase = -c rebase.updaterefs=interactive rebase ...\n\ninstead. It is not quite as convenient but it means we don't have to add \ncomplexity to the command line interface that is only useful for aliases \n(I can't think of a use for \"--update-refs=interactive\" outside of an \nalias definition).\n\n> Re: specifying `=(true|false)`, the intention was to avoid unnecessary\n> divergence, both in UX and code (and reuse the parser to simplify said\n> code). If you think it will be harmful, I'll remove that.\n\nIt would be even simpler if we didn't change the command line interface ;)\n\n>>>    rebase.updateRefs::\n>>> -\tIf set to true enable `--update-refs` option by default.\n>>> +\tIf set to true, enable the `--update-refs` option of\n>>> +\tlinkgit:git-rebase[1] by default. When set to 'interactive',\n>>\n>> Our existing documentation is inconsistent in how it formats config\n>> values. rebase.backend uses \"apply\", rebase.rebaseMerges uses\n>> `rebase-cousins` which I think matches other commands and is therefore\n>> what we should use here and rebase.missingCommitCheck uses a mixture\n>> with \"warn\" and `drop`.\n> \n> Apologies, I'm not sure I understood what exactly you were suggesting\n> here. Did you mean to suggest wrapping \"interactive\" in backticks\n> instead of single quotes?\n\nSorry that wasn't very clear. Yes that is what I was trying to say.\n\n>>> +\tif (v >= 0)\n>>> +\t\treturn v ? UPDATE_REFS_ALWAYS : UPDATE_REFS_NO;\n>>> +\telse if (!strcmp(\"interactive\", value))\n>>> +\t\treturn UPDATE_REFS_INTERACTIVE;\n>>> +\n>>> +\tdie(_(\"bad %s value '%s'; valid values are boolean or \\\"interactive\\\"\"), desc, value);\n>>\n>> I think we normally say \"invalid\" or \"unknown\" rather than \"bad\" in our\n>> error messages. It'd be clearer just to list the possible values as\n>> there are only three of them.\n> \n> It's not just three (see other review from Junio), otherwise OK\n\nAs this is a hint in a error message I don't think we need to \nexhaustively list all the possible synonyms git accepts for \"true\" and \n\"false\"\n\n>>> +\t/* coerce --update-refs=interactive into yes or no.\n>>> +\t * we do it here because there's just too much code below that handles\n>>> +\t * {,config_}update_refs in one way or another and modifying it to\n>>> +\t * account for the new state would be too invasive.\n>>> +\t * all further code uses {,config_}update_refs as a tristate. */\n>>\n>> I think we need to find a cleaner way of handling this. There are only\n>> two mentions of options.config_update_refs below this point - is it\n>> really so difficult for those to use the enum?\n> \n> See above; I opted to make this change as non-invasive as possible and\n> keep the complex argument validation logic (lines 1599, 1606-1609)\n> intact because I'm not even sure I understand it right.\n> \n> Besides, even if I convert those uses to use enumerators, I still\n> wouldn't want to deal with non-tristate values beyond this point.\n\nWe could add a new boolean variable which is initalized here and use \nthat instead in the code below. Of the code below could just call \nshould_update_refs() to convert the enum to a boolean.\n\nBest Wishes\n\nPhillip\n\n\n"},{"id":"512318","messageId":"xmqqh64zumkw.fsf@gitster.g","threadId":"62933","inReplyTo":"5b605c3e-ef6a-433a-9637-1e8f277dfde9@gmail.com","subject":"Re: [PATCH] rebase: add `--update-refs=interactive`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-12T16:58:23Z","receivedAt":"2025-02-12T16:58:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> Maintaining multiple versions of the same branch sounds like a lot of\n> work - whats the advantage over merging a single branch into each\n> release?\n\nMaking a single branch that would merge to each release track\ncleanly, with preparing and maintaining semantic fixes necessary for\neach track, is probably equally a lot of work, if not more.  I try\nto do that for this project only because I am a perfectionist for\nthese things (and do so for fun), but I can understand if many\nothers (a pragmatist in me included) consider it not worth the\neffort.  After all, it stops mattering once the branch finally gets\nmerged.\n"},{"id":"512320","messageId":"f0fa961084281b1d5948f59c42cf0c87e731d9bc.camel@intelfx.name","threadId":"62933","inReplyTo":"5b605c3e-ef6a-433a-9637-1e8f277dfde9@gmail.com","subject":"Re: [PATCH] rebase: add `--update-refs=interactive`","fromName":"Ivan Shapovalov","fromEmail":"intelfx@intelfx.name","sentAt":"2025-02-12T17:18:36Z","receivedAt":"2025-02-12T17:18:41Z","isPatch":true,"sender":{"key":"intelfx@intelfx.name","avatar":"https://gravatar.com/avatar/fb0d6e45051c5d8cda450fd6d02ecba0216f5f3aae1ebcf5de522939ff18eef3?d=mp&s=160"},"body":"On 2025-02-12 at 14:26 +0000, Phillip Wood wrote:\n> Hi Ivan\n> \n> On 11/02/2025 18:11, Ivan Shapovalov wrote:\n> > On 2025-02-11 at 14:36 +0000, Phillip Wood wrote:\n> > > On 10/02/2025 19:16, Ivan Shapovalov wrote:\n> > > \n> > > I'm a bit surprised by this - I'd have thought there is more scope for\n> > > messing things up by making a mistake when editing the todo list that\n> > > for the non-interactive case. Are you able to explain a in a bit more\n> > > detail the problem you have been experiencing please?\n> > \n> > I often find myself managing multiple interdependent downstream patch\n> > branches, rebasing them en masse from release to release. Eventually,\n> > I found myself typing `git rebase -i --update-refs` more often than\n> > not, so I just stuck it into the config as `rebase.updateRefs=true`.\n> > \n> > However, sometimes I also maintain those patch branches for multiple\n> > releases. Consider a (hypothetical) situation:\n> > \n> > - tag v1\n> > - tag v2\n> > - branch work/myfeature-v1 that is based on tag v1\n> > \n> > Now, I want to rebase myfeature onto v2, so I do this:\n> > \n> > $ git checkout work/myfeature-v1\n> > $ git checkout -b work/myfeature-v2\n> > $ git rebase --onto v2 v1 work/myfeature-v2\n> > \n> > With `rebase.updateRefs=true`, this ends up silently updating _both_\n> > work/myfeature-v2 and work/myfeature-v1.\n> \n> Thanks for the explanation. So this is about copying a branch and then \n> rebasing the copy without updating the original. A while ago there was a \n> discussion[1] about excluding branches that match HEAD from \n> \"--update-refs\". Maybe we should revisit that with a view to adding a \n> config setting that excludes copies of the current branch from \n> \"--update-refs\".\n\nThis idea stops working once you have a bunch of interdependent feature\nbranches (consider two branches work/myfeatureA and work/myfeatureB,\nwith the latter based on the former, with each having two versions as\ndescribed above, and then you rebase work/myfeatureB-v2 from v1 onto v2\nand expect to update work/myfeatureA-v2 but not work/myfeatureA-v1).\nExcluding branches that match HEAD is a very narrow workaround that\nonly fixes one particular instance of one particular workflow.\n\nI don't understand the opposition, really — in my understanding, an\nability to restrict update-refs to interactive runs is a significantly\nuseful mechanism that does not impose any particular policy. It answers\nthe question of \"I want git to _suggest_ updating refs by default, but\nonly if I have a chance to confirm/reject each particular update\".\n\n> \n> Maintaining multiple versions of the same branch sounds like a lot of \n> work - whats the advantage over merging a single branch into each release?\n\nDifferent people, different workflows.\n\n-- \nIvan Shapovalov / intelfx /\n\n> \n> [1] \n> https://lore.kernel.org/git/adb7f680-5bfa-6fa5-6d8a-61323fee7f53@haller-berlin.de/\n> \n> > With this in mind, I wrote this patch such that update-refs only\n> > happens for interactive rebases, when I have the chance to inspect the\n> > todo list and prune unwanted update-refs items.\n> > \n> > Does this make sense? I made an attempt to explain this motivation in\n> > the commit message, so if this does make sense but the commit message\n> > doesn't, please tell me how to improve/expand the latter.\n> \n> I think having the example in the commit message would help - I feel \n> like I've now got a clear idea of the problem you are facing whereas I \n> didn't understand what the issue was just from the commit message.\n> \n> > > > Try to find a middle ground by introducing a third value,\n> > > > `--update-refs=interactive` (and `rebase.updateRefs=interactive`)\n> > > > which means `--update-refs` when starting an interactive rebase and\n> > > > `--no-update-refs` otherwise. This option is primarily intended to be\n> > > > used in the gitconfig, but is also accepted on the command line\n> > > > for completeness.\n> > > \n> > > I'm not convinced allowing \"--update-refs=interactive\" on the\n> > > commandline improves the usability - why wouldn't I just say\n> > > \"--update-refs\" if I want to update all the branches or\n> > > \"--no-update-refs\" if I don't? I also think supporting\n> > > --update-refs=(true|false) is verbose and unnecessary as the user can\n> > > already specify their intent with the existing option.\n> > \n> > I make heavy use of aliases for various workflows, which invoke one\n> > another (making use of the ability to override earlier command-line\n> > options with the latter ones), and the ability to spell out\n> > `alias.myRebase = rebase ... --update-refs=interactive ...` was useful.\n> \n> You can write your alias as\n> \n>     alias.myRebase = -c rebase.updaterefs=interactive rebase ...\n> \n> instead. It is not quite as convenient but it means we don't have to add \n> complexity to the command line interface that is only useful for aliases \n> (I can't think of a use for \"--update-refs=interactive\" outside of an \n> alias definition).\n> \n> > Re: specifying `=(true|false)`, the intention was to avoid unnecessary\n> > divergence, both in UX and code (and reuse the parser to simplify said\n> > code). If you think it will be harmful, I'll remove that.\n> \n> It would be even simpler if we didn't change the command line interface ;)\n> \n> > > >    rebase.updateRefs::\n> > > > -\tIf set to true enable `--update-refs` option by default.\n> > > > +\tIf set to true, enable the `--update-refs` option of\n> > > > +\tlinkgit:git-rebase[1] by default. When set to 'interactive',\n> > > \n> > > Our existing documentation is inconsistent in how it formats config\n> > > values. rebase.backend uses \"apply\", rebase.rebaseMerges uses\n> > > `rebase-cousins` which I think matches other commands and is therefore\n> > > what we should use here and rebase.missingCommitCheck uses a mixture\n> > > with \"warn\" and `drop`.\n> > \n> > Apologies, I'm not sure I understood what exactly you were suggesting\n> > here. Did you mean to suggest wrapping \"interactive\" in backticks\n> > instead of single quotes?\n> \n> Sorry that wasn't very clear. Yes that is what I was trying to say.\n> \n> > > > +\tif (v >= 0)\n> > > > +\t\treturn v ? UPDATE_REFS_ALWAYS : UPDATE_REFS_NO;\n> > > > +\telse if (!strcmp(\"interactive\", value))\n> > > > +\t\treturn UPDATE_REFS_INTERACTIVE;\n> > > > +\n> > > > +\tdie(_(\"bad %s value '%s'; valid values are boolean or \\\"interactive\\\"\"), desc, value);\n> > > \n> > > I think we normally say \"invalid\" or \"unknown\" rather than \"bad\" in our\n> > > error messages. It'd be clearer just to list the possible values as\n> > > there are only three of them.\n> > \n> > It's not just three (see other review from Junio), otherwise OK\n> \n> As this is a hint in a error message I don't think we need to \n> exhaustively list all the possible synonyms git accepts for \"true\" and \n> \"false\"\n> \n> > > > +\t/* coerce --update-refs=interactive into yes or no.\n> > > > +\t * we do it here because there's just too much code below that handles\n> > > > +\t * {,config_}update_refs in one way or another and modifying it to\n> > > > +\t * account for the new state would be too invasive.\n> > > > +\t * all further code uses {,config_}update_refs as a tristate. */\n> > > \n> > > I think we need to find a cleaner way of handling this. There are only\n> > > two mentions of options.config_update_refs below this point - is it\n> > > really so difficult for those to use the enum?\n> > \n> > See above; I opted to make this change as non-invasive as possible and\n> > keep the complex argument validation logic (lines 1599, 1606-1609)\n> > intact because I'm not even sure I understand it right.\n> > \n> > Besides, even if I convert those uses to use enumerators, I still\n> > wouldn't want to deal with non-tristate values beyond this point.\n> \n> We could add a new boolean variable which is initalized here and use \n> that instead in the code below. Of the code below could just call \n> should_update_refs() to convert the enum to a boolean.\n> \n> Best Wishes\n> \n> Phillip\n> \n> \n"},{"id":"512356","messageId":"8a259585-97f7-4756-a126-17a982da58d7@gmail.com","threadId":"62933","inReplyTo":"f0fa961084281b1d5948f59c42cf0c87e731d9bc.camel@intelfx.name","subject":"Re: [PATCH] rebase: add `--update-refs=interactive`","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-02-13T09:43:07Z","receivedAt":"2025-02-13T09:43:12Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ivan\n\nOn 12/02/2025 17:18, Ivan Shapovalov wrote:\n> On 2025-02-12 at 14:26 +0000, Phillip Wood wrote:\n>>\n>> Thanks for the explanation. So this is about copying a branch and then\n>> rebasing the copy without updating the original. A while ago there was a\n>> discussion[1] about excluding branches that match HEAD from\n>> \"--update-refs\". Maybe we should revisit that with a view to adding a\n>> config setting that excludes copies of the current branch from\n>> \"--update-refs\".\n> \n> This idea stops working once you have a bunch of interdependent feature\n> branches (consider two branches work/myfeatureA and work/myfeatureB,\n> with the latter based on the former, with each having two versions as\n> described above, and then you rebase work/myfeatureB-v2 from v1 onto v2\n> and expect to update work/myfeatureA-v2 but not work/myfeatureA-v1).\n> Excluding branches that match HEAD is a very narrow workaround that\n> only fixes one particular instance of one particular workflow.\n\nGood point\n\n> I don't understand the opposition, really — in my understanding, an\n> ability to restrict update-refs to interactive runs is a significantly\n> useful mechanism that does not impose any particular policy. It answers\n> the question of \"I want git to _suggest_ updating refs by default, but\n> only if I have a chance to confirm/reject each particular update\".\n\nI'm not opposed, I'm just trying to understand the problem and see if \nthere are synergies with other issues people have brought to the list in \nthe past. You've convinced me that supporting \n\"rebase.updateRefs=interactive\" is worthwhile but I do not think we want \nto change the commandline interface. I'd much rather reserve the \noptional argument to support filtering in the future so that\n\n    git rebase --update-refs='*-v2' --update-refs=^not-me-v2\n\nwould update all the branches ending in \"-v2\" except \"not-me-v2\". We'd \nwant configure any default patterns separately to whether \n\"--update-refs\" was enabled by default which means we can add \"rebase \n.updateRefs=interactive\" without boxing ourselves into a corner.\n\n>> Maintaining multiple versions of the same branch sounds like a lot of\n>> work - whats the advantage over merging a single branch into each release?\n> \n> Different people, different workflows.\n\nFair enough, from what Junio said it may actually be less work anyway.\n\nBest Wishes\n\nPhillip\n\n"},{"id":"512357","messageId":"27f4e010-e2fb-4e80-b64d-3843c9fc3f55@gmail.com","threadId":"62933","inReplyTo":"xmqqh64zumkw.fsf@gitster.g","subject":"Re: [PATCH] rebase: add `--update-refs=interactive`","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-02-13T09:43:20Z","receivedAt":"2025-02-13T09:43:22Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 12/02/2025 16:58, Junio C Hamano wrote:\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n> \n>> Maintaining multiple versions of the same branch sounds like a lot of\n>> work - whats the advantage over merging a single branch into each\n>> release?\n> \n> Making a single branch that would merge to each release track\n> cleanly, with preparing and maintaining semantic fixes necessary for\n> each track, is probably equally a lot of work, if not more.  I try\n> to do that for this project only because I am a perfectionist for\n> these things (and do so for fun), but I can understand if many\n> others (a pragmatist in me included) consider it not worth the\n> effort.  After all, it stops mattering once the branch finally gets\n> merged.\n\nThanks for that insight, I'd assumed the benefits of having a single \nsource of truth for each branch outweighed the costs but it seems that \nis not necessarily the case.\n\nBest Wishes\n\nPhillip\n"},{"id":"512362","messageId":"1c69cee93c7edf62579d8eb3f40b0a98f3a5d075.camel@intelfx.name","threadId":"62933","inReplyTo":"8a259585-97f7-4756-a126-17a982da58d7@gmail.com","subject":"Re: [PATCH] rebase: add `--update-refs=interactive`","fromName":"Ivan Shapovalov","fromEmail":"intelfx@intelfx.name","sentAt":"2025-02-13T12:04:04Z","receivedAt":"2025-02-13T12:04:09Z","isPatch":true,"sender":{"key":"intelfx@intelfx.name","avatar":"https://gravatar.com/avatar/fb0d6e45051c5d8cda450fd6d02ecba0216f5f3aae1ebcf5de522939ff18eef3?d=mp&s=160"},"body":"On 2025-02-13 at 09:43 +0000, phillip.wood123@gmail.com wrote:\n> Hi Ivan\n> \n> On 12/02/2025 17:18, Ivan Shapovalov wrote:\n> > On 2025-02-12 at 14:26 +0000, Phillip Wood wrote:\n> > > \n> > > Thanks for the explanation. So this is about copying a branch and then\n> > > rebasing the copy without updating the original. A while ago there was a\n> > > discussion[1] about excluding branches that match HEAD from\n> > > \"--update-refs\". Maybe we should revisit that with a view to adding a\n> > > config setting that excludes copies of the current branch from\n> > > \"--update-refs\".\n> > \n> > This idea stops working once you have a bunch of interdependent feature\n> > branches (consider two branches work/myfeatureA and work/myfeatureB,\n> > with the latter based on the former, with each having two versions as\n> > described above, and then you rebase work/myfeatureB-v2 from v1 onto v2\n> > and expect to update work/myfeatureA-v2 but not work/myfeatureA-v1).\n> > Excluding branches that match HEAD is a very narrow workaround that\n> > only fixes one particular instance of one particular workflow.\n> \n> Good point\n> \n> > I don't understand the opposition, really — in my understanding, an\n> > ability to restrict update-refs to interactive runs is a significantly\n> > useful mechanism that does not impose any particular policy. It answers\n> > the question of \"I want git to _suggest_ updating refs by default, but\n> > only if I have a chance to confirm/reject each particular update\".\n> \n> I'm not opposed, I'm just trying to understand the problem and see if \n> there are synergies with other issues people have brought to the list in \n> the past. You've convinced me that supporting \n> \"rebase.updateRefs=interactive\" is worthwhile but I do not think we want \n> to change the commandline interface. I'd much rather reserve the \n> optional argument to support filtering in the future so that\n> \n>     git rebase --update-refs='*-v2' --update-refs=^not-me-v2\n> \n> would update all the branches ending in \"-v2\" except \"not-me-v2\". We'd \n> want configure any default patterns separately to whether \n> \"--update-refs\" was enabled by default which means we can add \"rebase \n> .updateRefs=interactive\" without boxing ourselves into a corner.\n\nMakes sense, that's indeed a better use of the optional argument.\nAlright, I'll send a v2 with +stylistic changes and -CLI changes.\n\n-- \nIvan Shapovalov / intelfx /\n\n> \n> > > Maintaining multiple versions of the same branch sounds like a lot of\n> > > work - whats the advantage over merging a single branch into each release?\n> > \n> > Different people, different workflows.\n> \n> Fair enough, from what Junio said it may actually be less work anyway.\n> \n> Best Wishes\n> \n> Phillip\n> \n"},{"id":"512711","messageId":"d48cbeb9-2b88-43cc-98af-c0f5d597415c@gmail.com","threadId":"62933","inReplyTo":"1c69cee93c7edf62579d8eb3f40b0a98f3a5d075.camel@intelfx.name","subject":"Re: [PATCH] rebase: add `--update-refs=interactive`","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-02-19T14:52:31Z","receivedAt":"2025-02-19T14:52:36Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ivan\n\nOn 13/02/2025 12:04, Ivan Shapovalov wrote:\n>> would update all the branches ending in \"-v2\" except \"not-me-v2\". We'd\n>> want configure any default patterns separately to whether\n>> \"--update-refs\" was enabled by default which means we can add \"rebase\n>> .updateRefs=interactive\" without boxing ourselves into a corner.\n> \n> Makes sense, that's indeed a better use of the optional argument.\n> Alright, I'll send a v2 with +stylistic changes and -CLI changes.\n\nThat's great, glad we're in agreement\n\nThanks\n\nPhillip\n\n"}]}