* [PATCH 0/8] history: introduce "split" subcommand
@ 2026-03-02 12:13 Patrick Steinhardt
2026-03-02 12:13 ` [PATCH 1/8] add-patch: split out header from "add-interactive.h" Patrick Steinhardt
` (9 more replies)
0 siblings, 10 replies; 16+ messages in thread
From: Patrick Steinhardt @ 2026-03-02 12:13 UTC (permalink / raw)
To: git; +Cc: Elijah Newren
Hi,
this patch series introduces `git history split` as an easy way to split
up one commit into multiple commits. This subcommand has already been
introduced in earlier versions of my git-history(1) patch series, but I
eventually decided to evict them from this series so that we can rather
focus more on basic decisions.
In any case, the current version of this patch series matches (to the
best of my knowledge) the latest agreements on the mailing list around
its behaviour. Most importantly:
- It will ask for commit messages for both commits, not only the first
commit, which has been a bit of a discussion point.
- It is not possible to edit hunks. This results in a mode where
conflicts are not possible as the tree of the second commit will
always match the tree of the original commit. Conflict handling for
subsequent subcommands will be a bigger topic, as it probably
depends on support for first-class conflicts.
- We also update dependent branches, same as with the latest iteration
of `git history reword`.
The patch series is built on top of 2cc7191751 (The 8th batch,
2026-02-27) with aa/add-p-no-auto-advance at 417b181f99 (add-patch:
allow interfile navigation when selecting hunks, 2026-02-14) merged into
it.
Thanks!
Patrick
---
Patrick Steinhardt (8):
add-patch: split out header from "add-interactive.h"
add-patch: split out `struct interactive_options`
add-patch: remove dependency on "add-interactive" subsystem
add-patch: add support for in-memory index patching
add-patch: allow disabling editing of hunks
cache-tree: allow writing in-memory index as tree
builtin/history: split out extended function to create commits
builtin/history: implement "split" subcommand
Documentation/git-history.adoc | 62 ++++
add-interactive.c | 177 ++--------
add-interactive.h | 48 +--
add-patch.c | 363 ++++++++++++++++----
add-patch.h | 74 ++++
builtin/add.c | 26 +-
builtin/checkout.c | 9 +-
builtin/commit.c | 16 +-
builtin/history.c | 317 +++++++++++++++--
builtin/reset.c | 20 +-
builtin/stash.c | 54 +--
cache-tree.c | 4 +-
cache-tree.h | 3 +-
commit.h | 2 +-
t/meson.build | 1 +
t/t3452-history-split.sh | 757 +++++++++++++++++++++++++++++++++++++++++
16 files changed, 1592 insertions(+), 341 deletions(-)
---
base-commit: caec355702b7a86dadec53344ba65ae33558cfa8
change-id: 20260202-pks-history-split-fb3a93df61ba
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/8] add-patch: split out header from "add-interactive.h" 2026-03-02 12:13 [PATCH 0/8] history: introduce "split" subcommand Patrick Steinhardt @ 2026-03-02 12:13 ` Patrick Steinhardt 2026-03-02 12:13 ` [PATCH 2/8] add-patch: split out `struct interactive_options` Patrick Steinhardt ` (8 subsequent siblings) 9 siblings, 0 replies; 16+ messages in thread From: Patrick Steinhardt @ 2026-03-02 12:13 UTC (permalink / raw) To: git; +Cc: Elijah Newren While we have a "add-patch.c" code file, its declarations are part of "add-interactive.h". This makes it somewhat harder than necessary to find relevant code and to identify clear boundaries between the two subsystems. Split up concerns and move declarations that relate to "add-patch.c" into a new "add-patch.h" header. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- add-interactive.h | 24 +++--------------------- add-patch.c | 1 + add-patch.h | 27 +++++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 21 deletions(-) diff --git a/add-interactive.h b/add-interactive.h index 7843397775..6c62489bfe 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -1,15 +1,11 @@ #ifndef ADD_INTERACTIVE_H #define ADD_INTERACTIVE_H +#include "add-patch.h" #include "color.h" -struct add_p_opt { - int context; - int interhunkcontext; - int auto_advance; -}; - -#define ADD_P_OPT_INIT { .context = -1, .interhunkcontext = -1, .auto_advance = 1 } +struct pathspec; +struct repository; struct add_i_state { struct repository *r; @@ -37,21 +33,7 @@ void init_add_i_state(struct add_i_state *s, struct repository *r, struct add_p_opt *add_p_opt); void clear_add_i_state(struct add_i_state *s); -struct repository; -struct pathspec; int run_add_i(struct repository *r, const struct pathspec *ps, struct add_p_opt *add_p_opt); -enum add_p_mode { - ADD_P_ADD, - ADD_P_STASH, - ADD_P_RESET, - ADD_P_CHECKOUT, - ADD_P_WORKTREE, -}; - -int run_add_p(struct repository *r, enum add_p_mode mode, - struct add_p_opt *o, const char *revision, - const struct pathspec *ps); - #endif diff --git a/add-patch.c b/add-patch.c index 8c03f710d3..8ce2fc02f6 100644 --- a/add-patch.c +++ b/add-patch.c @@ -3,6 +3,7 @@ #include "git-compat-util.h" #include "add-interactive.h" +#include "add-patch.h" #include "advice.h" #include "editor.h" #include "environment.h" diff --git a/add-patch.h b/add-patch.h new file mode 100644 index 0000000000..88b00ca788 --- /dev/null +++ b/add-patch.h @@ -0,0 +1,27 @@ +#ifndef ADD_PATCH_H +#define ADD_PATCH_H + +struct pathspec; +struct repository; + +struct add_p_opt { + int context; + int interhunkcontext; + int auto_advance; +}; + +#define ADD_P_OPT_INIT { .context = -1, .interhunkcontext = -1, .auto_advance = 1 } + +enum add_p_mode { + ADD_P_ADD, + ADD_P_STASH, + ADD_P_RESET, + ADD_P_CHECKOUT, + ADD_P_WORKTREE, +}; + +int run_add_p(struct repository *r, enum add_p_mode mode, + struct add_p_opt *o, const char *revision, + const struct pathspec *ps); + +#endif -- 2.53.0.697.g625c4fb2da.dirty ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/8] add-patch: split out `struct interactive_options` 2026-03-02 12:13 [PATCH 0/8] history: introduce "split" subcommand Patrick Steinhardt 2026-03-02 12:13 ` [PATCH 1/8] add-patch: split out header from "add-interactive.h" Patrick Steinhardt @ 2026-03-02 12:13 ` Patrick Steinhardt 2026-03-02 12:13 ` [PATCH 3/8] add-patch: remove dependency on "add-interactive" subsystem Patrick Steinhardt ` (7 subsequent siblings) 9 siblings, 0 replies; 16+ messages in thread From: Patrick Steinhardt @ 2026-03-02 12:13 UTC (permalink / raw) To: git; +Cc: Elijah Newren The `struct add_p_opt` is reused both by our infra for "git add -p" and "git add -i". Users of `run_add_i()` for example are expected to pass `struct add_p_opt`. This is somewhat confusing and raises the question of which options apply to what part of the stack. But things are even more confusing than that: while callers are expected to pass in `struct add_p_opt`, these options ultimately get used to initialize a `struct add_i_state` that is used by both subsystems. So we are basically going full circle here. Refactor the code and split out a new `struct interactive_options` that hosts common options used by both. These options are then applied to a `struct interactive_config` that hosts common configuration. This refactoring doesn't yet fully detangle the two subsystems from one another, as we still end up calling `init_add_i_state()` in the "git add -p" subsystem. This will be fixed in a subsequent commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- add-interactive.c | 177 +++++++++++--------------------------------------- add-interactive.h | 24 +------ add-patch.c | 187 +++++++++++++++++++++++++++++++++++++++++++---------- add-patch.h | 38 ++++++++++- builtin/add.c | 26 ++++---- builtin/checkout.c | 4 +- builtin/commit.c | 16 ++--- builtin/reset.c | 20 +++--- builtin/stash.c | 54 ++++++++-------- commit.h | 2 +- 10 files changed, 290 insertions(+), 258 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 1580639682..152e2a0297 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -3,7 +3,6 @@ #include "git-compat-util.h" #include "add-interactive.h" #include "color.h" -#include "config.h" #include "diffcore.h" #include "gettext.h" #include "hash.h" @@ -20,120 +19,18 @@ #include "prompt.h" #include "tree.h" -static void init_color(struct repository *r, enum git_colorbool use_color, - const char *section_and_slot, char *dst, - const char *default_color) -{ - char *key = xstrfmt("color.%s", section_and_slot); - const char *value; - - if (!want_color(use_color)) - dst[0] = '\0'; - else if (repo_config_get_value(r, key, &value) || - color_parse(value, dst)) - strlcpy(dst, default_color, COLOR_MAXLEN); - - free(key); -} - -static enum git_colorbool check_color_config(struct repository *r, const char *var) -{ - const char *value; - enum git_colorbool ret; - - if (repo_config_get_value(r, var, &value)) - ret = GIT_COLOR_UNKNOWN; - else - ret = git_config_colorbool(var, value); - - /* - * Do not rely on want_color() to fall back to color.ui for us. It uses - * the value parsed by git_color_config(), which may not have been - * called by the main command. - */ - if (ret == GIT_COLOR_UNKNOWN && - !repo_config_get_value(r, "color.ui", &value)) - ret = git_config_colorbool("color.ui", value); - - return ret; -} - void init_add_i_state(struct add_i_state *s, struct repository *r, - struct add_p_opt *add_p_opt) + struct interactive_options *opts) { s->r = r; - s->context = -1; - s->interhunkcontext = -1; - s->auto_advance = add_p_opt->auto_advance; - - s->use_color_interactive = check_color_config(r, "color.interactive"); - - init_color(r, s->use_color_interactive, "interactive.header", - s->header_color, GIT_COLOR_BOLD); - init_color(r, s->use_color_interactive, "interactive.help", - s->help_color, GIT_COLOR_BOLD_RED); - init_color(r, s->use_color_interactive, "interactive.prompt", - s->prompt_color, GIT_COLOR_BOLD_BLUE); - init_color(r, s->use_color_interactive, "interactive.error", - s->error_color, GIT_COLOR_BOLD_RED); - strlcpy(s->reset_color_interactive, - want_color(s->use_color_interactive) ? GIT_COLOR_RESET : "", COLOR_MAXLEN); - - s->use_color_diff = check_color_config(r, "color.diff"); - - init_color(r, s->use_color_diff, "diff.frag", s->fraginfo_color, - diff_get_color(s->use_color_diff, DIFF_FRAGINFO)); - init_color(r, s->use_color_diff, "diff.context", s->context_color, - "fall back"); - if (!strcmp(s->context_color, "fall back")) - init_color(r, s->use_color_diff, "diff.plain", - s->context_color, - diff_get_color(s->use_color_diff, DIFF_CONTEXT)); - init_color(r, s->use_color_diff, "diff.old", s->file_old_color, - diff_get_color(s->use_color_diff, DIFF_FILE_OLD)); - init_color(r, s->use_color_diff, "diff.new", s->file_new_color, - diff_get_color(s->use_color_diff, DIFF_FILE_NEW)); - strlcpy(s->reset_color_diff, - want_color(s->use_color_diff) ? GIT_COLOR_RESET : "", COLOR_MAXLEN); - - FREE_AND_NULL(s->interactive_diff_filter); - repo_config_get_string(r, "interactive.difffilter", - &s->interactive_diff_filter); - - FREE_AND_NULL(s->interactive_diff_algorithm); - repo_config_get_string(r, "diff.algorithm", - &s->interactive_diff_algorithm); - - if (!repo_config_get_int(r, "diff.context", &s->context)) - if (s->context < 0) - die(_("%s cannot be negative"), "diff.context"); - if (!repo_config_get_int(r, "diff.interHunkContext", &s->interhunkcontext)) - if (s->interhunkcontext < 0) - die(_("%s cannot be negative"), "diff.interHunkContext"); - - repo_config_get_bool(r, "interactive.singlekey", &s->use_single_key); - if (s->use_single_key) - setbuf(stdin, NULL); - - if (add_p_opt->context != -1) { - if (add_p_opt->context < 0) - die(_("%s cannot be negative"), "--unified"); - s->context = add_p_opt->context; - } - if (add_p_opt->interhunkcontext != -1) { - if (add_p_opt->interhunkcontext < 0) - die(_("%s cannot be negative"), "--inter-hunk-context"); - s->interhunkcontext = add_p_opt->interhunkcontext; - } + interactive_config_init(&s->cfg, r, opts); } void clear_add_i_state(struct add_i_state *s) { - FREE_AND_NULL(s->interactive_diff_filter); - FREE_AND_NULL(s->interactive_diff_algorithm); + interactive_config_clear(&s->cfg); memset(s, 0, sizeof(*s)); - s->use_color_interactive = GIT_COLOR_UNKNOWN; - s->use_color_diff = GIT_COLOR_UNKNOWN; + interactive_config_clear(&s->cfg); } /* @@ -287,7 +184,7 @@ static void list(struct add_i_state *s, struct string_list *list, int *selected, return; if (opts->header) - color_fprintf_ln(stdout, s->header_color, + color_fprintf_ln(stdout, s->cfg.header_color, "%s", opts->header); for (i = 0; i < list->nr; i++) { @@ -355,7 +252,7 @@ static ssize_t list_and_choose(struct add_i_state *s, list(s, &items->items, items->selected, &opts->list_opts); - color_fprintf(stdout, s->prompt_color, "%s", opts->prompt); + color_fprintf(stdout, s->cfg.prompt_color, "%s", opts->prompt); fputs(singleton ? "> " : ">> ", stdout); fflush(stdout); @@ -433,7 +330,7 @@ static ssize_t list_and_choose(struct add_i_state *s, if (from < 0 || from >= items->items.nr || (singleton && from + 1 != to)) { - color_fprintf_ln(stderr, s->error_color, + color_fprintf_ln(stderr, s->cfg.error_color, _("Huh (%s)?"), p); break; } else if (singleton) { @@ -993,7 +890,7 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps, free(files->items.items[i].string); } else if (item->index.unmerged || item->worktree.unmerged) { - color_fprintf_ln(stderr, s->error_color, + color_fprintf_ln(stderr, s->cfg.error_color, _("ignoring unmerged: %s"), files->items.items[i].string); free(item); @@ -1015,10 +912,10 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps, opts->prompt = N_("Patch update"); count = list_and_choose(s, files, opts); if (count > 0) { - struct add_p_opt add_p_opt = { - .context = s->context, - .interhunkcontext = s->interhunkcontext, - .auto_advance = s->auto_advance + struct interactive_options opts = { + .context = s->cfg.context, + .interhunkcontext = s->cfg.interhunkcontext, + .auto_advance = s->cfg.auto_advance, }; struct strvec args = STRVEC_INIT; struct pathspec ps_selected = { 0 }; @@ -1030,7 +927,7 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps, parse_pathspec(&ps_selected, PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL, PATHSPEC_LITERAL_PATH, "", args.v); - res = run_add_p(s->r, ADD_P_ADD, &add_p_opt, NULL, &ps_selected); + res = run_add_p(s->r, ADD_P_ADD, &opts, NULL, &ps_selected); strvec_clear(&args); clear_pathspec(&ps_selected); } @@ -1066,10 +963,10 @@ static int run_diff(struct add_i_state *s, const struct pathspec *ps, struct child_process cmd = CHILD_PROCESS_INIT; strvec_pushl(&cmd.args, "git", "diff", "-p", "--cached", NULL); - if (s->context != -1) - strvec_pushf(&cmd.args, "--unified=%i", s->context); - if (s->interhunkcontext != -1) - strvec_pushf(&cmd.args, "--inter-hunk-context=%i", s->interhunkcontext); + if (s->cfg.context != -1) + strvec_pushf(&cmd.args, "--unified=%i", s->cfg.context); + if (s->cfg.interhunkcontext != -1) + strvec_pushf(&cmd.args, "--inter-hunk-context=%i", s->cfg.interhunkcontext); strvec_pushl(&cmd.args, oid_to_hex(!is_initial ? &oid : s->r->hash_algo->empty_tree), "--", NULL); for (i = 0; i < files->items.nr; i++) @@ -1087,17 +984,17 @@ static int run_help(struct add_i_state *s, const struct pathspec *ps UNUSED, struct prefix_item_list *files UNUSED, struct list_and_choose_options *opts UNUSED) { - color_fprintf_ln(stdout, s->help_color, "status - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "status - %s", _("show paths with changes")); - color_fprintf_ln(stdout, s->help_color, "update - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "update - %s", _("add working tree state to the staged set of changes")); - color_fprintf_ln(stdout, s->help_color, "revert - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "revert - %s", _("revert staged set of changes back to the HEAD version")); - color_fprintf_ln(stdout, s->help_color, "patch - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "patch - %s", _("pick hunks and update selectively")); - color_fprintf_ln(stdout, s->help_color, "diff - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "diff - %s", _("view diff between HEAD and index")); - color_fprintf_ln(stdout, s->help_color, "add untracked - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "add untracked - %s", _("add contents of untracked files to the staged set of changes")); return 0; @@ -1105,21 +1002,21 @@ static int run_help(struct add_i_state *s, const struct pathspec *ps UNUSED, static void choose_prompt_help(struct add_i_state *s) { - color_fprintf_ln(stdout, s->help_color, "%s", + color_fprintf_ln(stdout, s->cfg.help_color, "%s", _("Prompt help:")); - color_fprintf_ln(stdout, s->help_color, "1 - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "1 - %s", _("select a single item")); - color_fprintf_ln(stdout, s->help_color, "3-5 - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "3-5 - %s", _("select a range of items")); - color_fprintf_ln(stdout, s->help_color, "2-3,6-9 - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "2-3,6-9 - %s", _("select multiple ranges")); - color_fprintf_ln(stdout, s->help_color, "foo - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "foo - %s", _("select item based on unique prefix")); - color_fprintf_ln(stdout, s->help_color, "-... - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "-... - %s", _("unselect specified items")); - color_fprintf_ln(stdout, s->help_color, "* - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "* - %s", _("choose all items")); - color_fprintf_ln(stdout, s->help_color, " - %s", + color_fprintf_ln(stdout, s->cfg.help_color, " - %s", _("(empty) finish selecting")); } @@ -1154,7 +1051,7 @@ static void print_command_item(int i, int selected UNUSED, static void command_prompt_help(struct add_i_state *s) { - const char *help_color = s->help_color; + const char *help_color = s->cfg.help_color; color_fprintf_ln(stdout, help_color, "%s", _("Prompt help:")); color_fprintf_ln(stdout, help_color, "1 - %s", _("select a numbered item")); @@ -1165,7 +1062,7 @@ static void command_prompt_help(struct add_i_state *s) } int run_add_i(struct repository *r, const struct pathspec *ps, - struct add_p_opt *add_p_opt) + struct interactive_options *interactive_opts) { struct add_i_state s = { NULL }; struct print_command_item_data data = { "[", "]" }; @@ -1208,15 +1105,15 @@ int run_add_i(struct repository *r, const struct pathspec *ps, ->util = util; } - init_add_i_state(&s, r, add_p_opt); + init_add_i_state(&s, r, interactive_opts); /* * When color was asked for, use the prompt color for * highlighting, otherwise use square brackets. */ - if (want_color(s.use_color_interactive)) { - data.color = s.prompt_color; - data.reset = s.reset_color_interactive; + if (want_color(s.cfg.use_color_interactive)) { + data.color = s.cfg.prompt_color; + data.reset = s.cfg.reset_color_interactive; } print_file_item_data.color = data.color; print_file_item_data.reset = data.reset; diff --git a/add-interactive.h b/add-interactive.h index 6c62489bfe..eefa2edc7c 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -2,38 +2,20 @@ #define ADD_INTERACTIVE_H #include "add-patch.h" -#include "color.h" struct pathspec; struct repository; struct add_i_state { struct repository *r; - enum git_colorbool use_color_interactive; - enum git_colorbool use_color_diff; - char header_color[COLOR_MAXLEN]; - char help_color[COLOR_MAXLEN]; - char prompt_color[COLOR_MAXLEN]; - char error_color[COLOR_MAXLEN]; - char reset_color_interactive[COLOR_MAXLEN]; - - char fraginfo_color[COLOR_MAXLEN]; - char context_color[COLOR_MAXLEN]; - char file_old_color[COLOR_MAXLEN]; - char file_new_color[COLOR_MAXLEN]; - char reset_color_diff[COLOR_MAXLEN]; - - int use_single_key; - char *interactive_diff_filter, *interactive_diff_algorithm; - int context, interhunkcontext; - int auto_advance; + struct interactive_config cfg; }; void init_add_i_state(struct add_i_state *s, struct repository *r, - struct add_p_opt *add_p_opt); + struct interactive_options *opts); void clear_add_i_state(struct add_i_state *s); int run_add_i(struct repository *r, const struct pathspec *ps, - struct add_p_opt *add_p_opt); + struct interactive_options *opts); #endif diff --git a/add-patch.c b/add-patch.c index 8ce2fc02f6..756143eb84 100644 --- a/add-patch.c +++ b/add-patch.c @@ -5,6 +5,8 @@ #include "add-interactive.h" #include "add-patch.h" #include "advice.h" +#include "config.h" +#include "diff.h" #include "editor.h" #include "environment.h" #include "gettext.h" @@ -279,6 +281,123 @@ struct add_p_state { const char *revision; }; +static void init_color(struct repository *r, + enum git_colorbool use_color, + const char *section_and_slot, char *dst, + const char *default_color) +{ + char *key = xstrfmt("color.%s", section_and_slot); + const char *value; + + if (!want_color(use_color)) + dst[0] = '\0'; + else if (repo_config_get_value(r, key, &value) || + color_parse(value, dst)) + strlcpy(dst, default_color, COLOR_MAXLEN); + + free(key); +} + +static enum git_colorbool check_color_config(struct repository *r, const char *var) +{ + const char *value; + enum git_colorbool ret; + + if (repo_config_get_value(r, var, &value)) + ret = GIT_COLOR_UNKNOWN; + else + ret = git_config_colorbool(var, value); + + /* + * Do not rely on want_color() to fall back to color.ui for us. It uses + * the value parsed by git_color_config(), which may not have been + * called by the main command. + */ + if (ret == GIT_COLOR_UNKNOWN && + !repo_config_get_value(r, "color.ui", &value)) + ret = git_config_colorbool("color.ui", value); + + return ret; +} + +void interactive_config_init(struct interactive_config *cfg, + struct repository *r, + struct interactive_options *opts) +{ + cfg->context = -1; + cfg->interhunkcontext = -1; + cfg->auto_advance = opts->auto_advance; + + cfg->use_color_interactive = check_color_config(r, "color.interactive"); + + init_color(r, cfg->use_color_interactive, "interactive.header", + cfg->header_color, GIT_COLOR_BOLD); + init_color(r, cfg->use_color_interactive, "interactive.help", + cfg->help_color, GIT_COLOR_BOLD_RED); + init_color(r, cfg->use_color_interactive, "interactive.prompt", + cfg->prompt_color, GIT_COLOR_BOLD_BLUE); + init_color(r, cfg->use_color_interactive, "interactive.error", + cfg->error_color, GIT_COLOR_BOLD_RED); + strlcpy(cfg->reset_color_interactive, + want_color(cfg->use_color_interactive) ? GIT_COLOR_RESET : "", COLOR_MAXLEN); + + cfg->use_color_diff = check_color_config(r, "color.diff"); + + init_color(r, cfg->use_color_diff, "diff.frag", cfg->fraginfo_color, + diff_get_color(cfg->use_color_diff, DIFF_FRAGINFO)); + init_color(r, cfg->use_color_diff, "diff.context", cfg->context_color, + "fall back"); + if (!strcmp(cfg->context_color, "fall back")) + init_color(r, cfg->use_color_diff, "diff.plain", + cfg->context_color, + diff_get_color(cfg->use_color_diff, DIFF_CONTEXT)); + init_color(r, cfg->use_color_diff, "diff.old", cfg->file_old_color, + diff_get_color(cfg->use_color_diff, DIFF_FILE_OLD)); + init_color(r, cfg->use_color_diff, "diff.new", cfg->file_new_color, + diff_get_color(cfg->use_color_diff, DIFF_FILE_NEW)); + strlcpy(cfg->reset_color_diff, + want_color(cfg->use_color_diff) ? GIT_COLOR_RESET : "", COLOR_MAXLEN); + + FREE_AND_NULL(cfg->interactive_diff_filter); + repo_config_get_string(r, "interactive.difffilter", + &cfg->interactive_diff_filter); + + FREE_AND_NULL(cfg->interactive_diff_algorithm); + repo_config_get_string(r, "diff.algorithm", + &cfg->interactive_diff_algorithm); + + if (!repo_config_get_int(r, "diff.context", &cfg->context)) + if (cfg->context < 0) + die(_("%s cannot be negative"), "diff.context"); + if (!repo_config_get_int(r, "diff.interHunkContext", &cfg->interhunkcontext)) + if (cfg->interhunkcontext < 0) + die(_("%s cannot be negative"), "diff.interHunkContext"); + + repo_config_get_bool(r, "interactive.singlekey", &cfg->use_single_key); + if (cfg->use_single_key) + setbuf(stdin, NULL); + + if (opts->context != -1) { + if (opts->context < 0) + die(_("%s cannot be negative"), "--unified"); + cfg->context = opts->context; + } + if (opts->interhunkcontext != -1) { + if (opts->interhunkcontext < 0) + die(_("%s cannot be negative"), "--inter-hunk-context"); + cfg->interhunkcontext = opts->interhunkcontext; + } +} + +void interactive_config_clear(struct interactive_config *cfg) +{ + FREE_AND_NULL(cfg->interactive_diff_filter); + FREE_AND_NULL(cfg->interactive_diff_algorithm); + memset(cfg, 0, sizeof(*cfg)); + cfg->use_color_interactive = GIT_COLOR_UNKNOWN; + cfg->use_color_diff = GIT_COLOR_UNKNOWN; +} + static void add_p_state_clear(struct add_p_state *s) { size_t i; @@ -299,9 +418,9 @@ static void err(struct add_p_state *s, const char *fmt, ...) va_list args; va_start(args, fmt); - fputs(s->s.error_color, stdout); + fputs(s->s.cfg.error_color, stdout); vprintf(fmt, args); - puts(s->s.reset_color_interactive); + puts(s->s.cfg.reset_color_interactive); va_end(args); } @@ -424,12 +543,12 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) int res; strvec_pushv(&args, s->mode->diff_cmd); - if (s->s.context != -1) - strvec_pushf(&args, "--unified=%i", s->s.context); - if (s->s.interhunkcontext != -1) - strvec_pushf(&args, "--inter-hunk-context=%i", s->s.interhunkcontext); - if (s->s.interactive_diff_algorithm) - strvec_pushf(&args, "--diff-algorithm=%s", s->s.interactive_diff_algorithm); + if (s->s.cfg.context != -1) + strvec_pushf(&args, "--unified=%i", s->s.cfg.context); + if (s->s.cfg.interhunkcontext != -1) + strvec_pushf(&args, "--inter-hunk-context=%i", s->s.cfg.interhunkcontext); + if (s->s.cfg.interactive_diff_algorithm) + strvec_pushf(&args, "--diff-algorithm=%s", s->s.cfg.interactive_diff_algorithm); if (s->revision) { struct object_id oid; strvec_push(&args, @@ -458,9 +577,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) } strbuf_complete_line(plain); - if (want_color_fd(1, s->s.use_color_diff)) { + if (want_color_fd(1, s->s.cfg.use_color_diff)) { struct child_process colored_cp = CHILD_PROCESS_INIT; - const char *diff_filter = s->s.interactive_diff_filter; + const char *diff_filter = s->s.cfg.interactive_diff_filter; setup_child_process(s, &colored_cp, NULL); xsnprintf((char *)args.v[color_arg_index], 8, "--color"); @@ -693,7 +812,7 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk, hunk->colored_end - hunk->colored_start); return; } else { - strbuf_addstr(out, s->s.fraginfo_color); + strbuf_addstr(out, s->s.cfg.fraginfo_color); p = s->colored.buf + header->colored_extra_start; len = header->colored_extra_end - header->colored_extra_start; @@ -715,7 +834,7 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk, if (len) strbuf_add(out, p, len); else if (colored) - strbuf_addf(out, "%s\n", s->s.reset_color_diff); + strbuf_addf(out, "%s\n", s->s.cfg.reset_color_diff); else strbuf_addch(out, '\n'); } @@ -1104,12 +1223,12 @@ static void recolor_hunk(struct add_p_state *s, struct hunk *hunk) strbuf_addstr(&s->colored, plain[current] == '-' ? - s->s.file_old_color : + s->s.cfg.file_old_color : plain[current] == '+' ? - s->s.file_new_color : - s->s.context_color); + s->s.cfg.file_new_color : + s->s.cfg.context_color); strbuf_add(&s->colored, plain + current, eol - current); - strbuf_addstr(&s->colored, s->s.reset_color_diff); + strbuf_addstr(&s->colored, s->s.cfg.reset_color_diff); if (next > eol) strbuf_add(&s->colored, plain + eol, next - eol); current = next; @@ -1238,7 +1357,7 @@ static int run_apply_check(struct add_p_state *s, static int read_single_character(struct add_p_state *s) { - if (s->s.use_single_key) { + if (s->s.cfg.use_single_key) { int res = read_key_without_echo(&s->answer); printf("%s\n", res == EOF ? "" : s->answer.buf); return res; @@ -1252,7 +1371,7 @@ static int read_single_character(struct add_p_state *s) static int prompt_yesno(struct add_p_state *s, const char *prompt) { for (;;) { - color_fprintf(stdout, s->s.prompt_color, "%s", _(prompt)); + color_fprintf(stdout, s->s.cfg.prompt_color, "%s", _(prompt)); fflush(stdout); if (read_single_character(s) == EOF) return -1; @@ -1541,7 +1660,7 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) /* Everything decided? */ if (undecided_previous < 0 && undecided_next < 0 && hunk->use != UNDECIDED_HUNK) { - if (!s->s.auto_advance) + if (!s->s.cfg.auto_advance) all_decided = 1; else { patch_update_resp++; @@ -1595,11 +1714,11 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) permitted |= ALLOW_EDIT; strbuf_addstr(&s->buf, ",e"); } - if (!s->s.auto_advance && s->file_diff_nr > 1) { + if (!s->s.cfg.auto_advance && s->file_diff_nr > 1) { permitted |= ALLOW_GOTO_NEXT_FILE; strbuf_addstr(&s->buf, ",>"); } - if (!s->s.auto_advance && s->file_diff_nr > 1) { + if (!s->s.cfg.auto_advance && s->file_diff_nr > 1) { permitted |= ALLOW_GOTO_PREVIOUS_FILE; strbuf_addstr(&s->buf, ",<"); } @@ -1614,7 +1733,7 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) else prompt_mode_type = PROMPT_HUNK; - printf("%s(%"PRIuMAX"/%"PRIuMAX") ", s->s.prompt_color, + printf("%s(%"PRIuMAX"/%"PRIuMAX") ", s->s.cfg.prompt_color, (uintmax_t)hunk_index + 1, (uintmax_t)(file_diff->hunk_nr ? file_diff->hunk_nr @@ -1627,8 +1746,8 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) } printf(_(s->mode->prompt_mode[prompt_mode_type]), hunk_use_decision, s->buf.buf); - if (*s->s.reset_color_interactive) - fputs(s->s.reset_color_interactive, stdout); + if (*s->s.cfg.reset_color_interactive) + fputs(s->s.cfg.reset_color_interactive, stdout); fflush(stdout); if (read_single_character(s) == EOF) { patch_update_resp = s->file_diff_nr; @@ -1679,7 +1798,7 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) } else if (ch == 'q') { patch_update_resp = s->file_diff_nr; break; - } else if (!s->s.auto_advance && s->answer.buf[0] == '>') { + } else if (!s->s.cfg.auto_advance && s->answer.buf[0] == '>') { if (permitted & ALLOW_GOTO_NEXT_FILE) { if (patch_update_resp == s->file_diff_nr - 1) patch_update_resp = 0; @@ -1690,7 +1809,7 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) err(s, _("No next file")); continue; } - } else if (!s->s.auto_advance && s->answer.buf[0] == '<') { + } else if (!s->s.cfg.auto_advance && s->answer.buf[0] == '<') { if (permitted & ALLOW_GOTO_PREVIOUS_FILE) { if (patch_update_resp == 0) patch_update_resp = s->file_diff_nr - 1; @@ -1813,7 +1932,7 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) err(s, _("Sorry, cannot split this hunk")); } else if (!split_hunk(s, file_diff, hunk - file_diff->hunk)) { - color_fprintf_ln(stdout, s->s.header_color, + color_fprintf_ln(stdout, s->s.cfg.header_color, _("Split into %d hunks."), (int)splittable_into); rendered_hunk_index = -1; @@ -1831,7 +1950,7 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) } else if (s->answer.buf[0] == '?') { const char *p = _(help_patch_remainder), *eol = p; - color_fprintf(stdout, s->s.help_color, "%s", + color_fprintf(stdout, s->s.cfg.help_color, "%s", _(s->mode->help_patch_text)); /* @@ -1855,13 +1974,13 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) if (file_diff->hunk[i].use == SKIP_HUNK) skipped += 1; } - color_fprintf_ln(stdout, s->s.help_color, _(p), + color_fprintf_ln(stdout, s->s.cfg.help_color, _(p), total, used, skipped); } if (*p != '?' && !strchr(s->buf.buf, *p)) continue; - color_fprintf_ln(stdout, s->s.help_color, + color_fprintf_ln(stdout, s->s.cfg.help_color, "%.*s", (int)(eol - p), p); } } else { @@ -1870,7 +1989,7 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) } } - if (s->s.auto_advance) + if (s->s.cfg.auto_advance) apply_patch(s, file_diff); putchar('\n'); @@ -1878,7 +1997,7 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) } int run_add_p(struct repository *r, enum add_p_mode mode, - struct add_p_opt *o, const char *revision, + struct interactive_options *opts, const char *revision, const struct pathspec *ps) { struct add_p_state s = { @@ -1886,7 +2005,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode, }; size_t i, binary_count = 0; - init_add_i_state(&s.s, r, o); + init_add_i_state(&s.s, r, opts); if (mode == ADD_P_STASH) s.mode = &patch_mode_stash; @@ -1932,7 +2051,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode, if ((i = patch_update_file(&s, i)) == s.file_diff_nr) break; } - if (!s.s.auto_advance) + if (!s.s.cfg.auto_advance) for (i = 0; i < s.file_diff_nr; i++) apply_patch(&s, s.file_diff + i); diff --git a/add-patch.h b/add-patch.h index 88b00ca788..e6868c60a2 100644 --- a/add-patch.h +++ b/add-patch.h @@ -1,16 +1,48 @@ #ifndef ADD_PATCH_H #define ADD_PATCH_H +#include "color.h" + struct pathspec; struct repository; -struct add_p_opt { +struct interactive_options { int context; int interhunkcontext; int auto_advance; }; -#define ADD_P_OPT_INIT { .context = -1, .interhunkcontext = -1, .auto_advance = 1 } +#define INTERACTIVE_OPTIONS_INIT { \ + .context = -1, \ + .interhunkcontext = -1, \ + .auto_advance = 1, \ +} + +struct interactive_config { + enum git_colorbool use_color_interactive; + enum git_colorbool use_color_diff; + char header_color[COLOR_MAXLEN]; + char help_color[COLOR_MAXLEN]; + char prompt_color[COLOR_MAXLEN]; + char error_color[COLOR_MAXLEN]; + char reset_color_interactive[COLOR_MAXLEN]; + + char fraginfo_color[COLOR_MAXLEN]; + char context_color[COLOR_MAXLEN]; + char file_old_color[COLOR_MAXLEN]; + char file_new_color[COLOR_MAXLEN]; + char reset_color_diff[COLOR_MAXLEN]; + + int use_single_key; + char *interactive_diff_filter, *interactive_diff_algorithm; + int context, interhunkcontext; + int auto_advance; +}; + +void interactive_config_init(struct interactive_config *cfg, + struct repository *r, + struct interactive_options *opts); +void interactive_config_clear(struct interactive_config *cfg); enum add_p_mode { ADD_P_ADD, @@ -21,7 +53,7 @@ enum add_p_mode { }; int run_add_p(struct repository *r, enum add_p_mode mode, - struct add_p_opt *o, const char *revision, + struct interactive_options *opts, const char *revision, const struct pathspec *ps); #endif diff --git a/builtin/add.c b/builtin/add.c index 4357f87b7f..84f9bcb789 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -31,7 +31,7 @@ static const char * const builtin_add_usage[] = { NULL }; static int patch_interactive, add_interactive, edit_interactive; -static struct add_p_opt add_p_opt = ADD_P_OPT_INIT; +static struct interactive_options interactive_opts = INTERACTIVE_OPTIONS_INIT; static int take_worktree_changes; static int add_renormalize; static int pathspec_file_nul; @@ -160,7 +160,7 @@ static int refresh(struct repository *repo, int verbose, const struct pathspec * int interactive_add(struct repository *repo, const char **argv, const char *prefix, - int patch, struct add_p_opt *add_p_opt) + int patch, struct interactive_options *interactive_opts) { struct pathspec pathspec; int ret; @@ -172,9 +172,9 @@ int interactive_add(struct repository *repo, prefix, argv); if (patch) - ret = !!run_add_p(repo, ADD_P_ADD, add_p_opt, NULL, &pathspec); + ret = !!run_add_p(repo, ADD_P_ADD, interactive_opts, NULL, &pathspec); else - ret = !!run_add_i(repo, &pathspec, add_p_opt); + ret = !!run_add_i(repo, &pathspec, interactive_opts); clear_pathspec(&pathspec); return ret; @@ -256,10 +256,10 @@ static struct option builtin_add_options[] = { OPT_GROUP(""), OPT_BOOL('i', "interactive", &add_interactive, N_("interactive picking")), OPT_BOOL('p', "patch", &patch_interactive, N_("select hunks interactively")), - OPT_BOOL(0, "auto-advance", &add_p_opt.auto_advance, + OPT_BOOL(0, "auto-advance", &interactive_opts.auto_advance, N_("auto advance to the next file when selecting hunks interactively")), - OPT_DIFF_UNIFIED(&add_p_opt.context), - OPT_DIFF_INTERHUNK_CONTEXT(&add_p_opt.interhunkcontext), + OPT_DIFF_UNIFIED(&interactive_opts.context), + OPT_DIFF_INTERHUNK_CONTEXT(&interactive_opts.interhunkcontext), OPT_BOOL('e', "edit", &edit_interactive, N_("edit current diff and apply")), OPT__FORCE(&ignored_too, N_("allow adding otherwise ignored files"), 0), OPT_BOOL('u', "update", &take_worktree_changes, N_("update tracked files")), @@ -402,9 +402,9 @@ int cmd_add(int argc, prepare_repo_settings(repo); repo->settings.command_requires_full_index = 0; - if (add_p_opt.context < -1) + if (interactive_opts.context < -1) die(_("'%s' cannot be negative"), "--unified"); - if (add_p_opt.interhunkcontext < -1) + if (interactive_opts.interhunkcontext < -1) die(_("'%s' cannot be negative"), "--inter-hunk-context"); if (patch_interactive) @@ -414,13 +414,13 @@ int cmd_add(int argc, die(_("options '%s' and '%s' cannot be used together"), "--dry-run", "--interactive/--patch"); if (pathspec_from_file) die(_("options '%s' and '%s' cannot be used together"), "--pathspec-from-file", "--interactive/--patch"); - exit(interactive_add(repo, argv + 1, prefix, patch_interactive, &add_p_opt)); + exit(interactive_add(repo, argv + 1, prefix, patch_interactive, &interactive_opts)); } else { - if (add_p_opt.context != -1) + if (interactive_opts.context != -1) die(_("the option '%s' requires '%s'"), "--unified", "--interactive/--patch"); - if (add_p_opt.interhunkcontext != -1) + if (interactive_opts.interhunkcontext != -1) die(_("the option '%s' requires '%s'"), "--inter-hunk-context", "--interactive/--patch"); - if (!add_p_opt.auto_advance) + if (!interactive_opts.auto_advance) die(_("the option '%s' requires '%s'"), "--no-auto-advance", "--interactive/--patch"); } diff --git a/builtin/checkout.c b/builtin/checkout.c index eefefd5d0f..bebe18c1d9 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -532,7 +532,7 @@ static int checkout_paths(const struct checkout_opts *opts, if (opts->patch_mode) { enum add_p_mode patch_mode; - struct add_p_opt add_p_opt = { + struct interactive_options interactive_opts = { .context = opts->patch_context, .interhunkcontext = opts->patch_interhunk_context, .auto_advance = opts->auto_advance @@ -562,7 +562,7 @@ static int checkout_paths(const struct checkout_opts *opts, else BUG("either flag must have been set, worktree=%d, index=%d", opts->checkout_worktree, opts->checkout_index); - return !!run_add_p(the_repository, patch_mode, &add_p_opt, + return !!run_add_p(the_repository, patch_mode, &interactive_opts, rev, &opts->pathspec); } diff --git a/builtin/commit.c b/builtin/commit.c index 9e3a09d532..a1d64dd699 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -123,7 +123,7 @@ static const char *edit_message, *use_message; static char *fixup_message, *fixup_commit, *squash_message; static const char *fixup_prefix; static int all, also, interactive, patch_interactive, only, amend, signoff; -static struct add_p_opt add_p_opt = ADD_P_OPT_INIT; +static struct interactive_options interactive_opts = INTERACTIVE_OPTIONS_INIT; static int edit_flag = -1; /* unspecified */ static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship; static int config_commit_verbose = -1; /* unspecified */ @@ -357,9 +357,9 @@ static const char *prepare_index(const char **argv, const char *prefix, const char *ret; char *path = NULL; - if (add_p_opt.context < -1) + if (interactive_opts.context < -1) die(_("'%s' cannot be negative"), "--unified"); - if (add_p_opt.interhunkcontext < -1) + if (interactive_opts.interhunkcontext < -1) die(_("'%s' cannot be negative"), "--inter-hunk-context"); if (is_status) @@ -408,7 +408,7 @@ static const char *prepare_index(const char **argv, const char *prefix, old_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT)); setenv(INDEX_ENVIRONMENT, the_repository->index_file, 1); - if (interactive_add(the_repository, argv, prefix, patch_interactive, &add_p_opt) != 0) + if (interactive_add(the_repository, argv, prefix, patch_interactive, &interactive_opts) != 0) die(_("interactive add failed")); the_repository->index_file = old_repo_index_file; @@ -433,9 +433,9 @@ static const char *prepare_index(const char **argv, const char *prefix, ret = get_lock_file_path(&index_lock); goto out; } else { - if (add_p_opt.context != -1) + if (interactive_opts.context != -1) die(_("the option '%s' requires '%s'"), "--unified", "--interactive/--patch"); - if (add_p_opt.interhunkcontext != -1) + if (interactive_opts.interhunkcontext != -1) die(_("the option '%s' requires '%s'"), "--inter-hunk-context", "--interactive/--patch"); } @@ -1743,8 +1743,8 @@ int cmd_commit(int argc, OPT_BOOL('i', "include", &also, N_("add specified files to index for commit")), OPT_BOOL(0, "interactive", &interactive, N_("interactively add files")), OPT_BOOL('p', "patch", &patch_interactive, N_("interactively add changes")), - OPT_DIFF_UNIFIED(&add_p_opt.context), - OPT_DIFF_INTERHUNK_CONTEXT(&add_p_opt.interhunkcontext), + OPT_DIFF_UNIFIED(&interactive_opts.context), + OPT_DIFF_INTERHUNK_CONTEXT(&interactive_opts.interhunkcontext), OPT_BOOL('o', "only", &only, N_("commit only specified files")), OPT_BOOL('n', "no-verify", &no_verify, N_("bypass pre-commit and commit-msg hooks")), OPT_BOOL(0, "dry-run", &dry_run, N_("show what would be committed")), diff --git a/builtin/reset.c b/builtin/reset.c index 88f95f9fc7..4a74a82c0a 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -346,7 +346,7 @@ int cmd_reset(int argc, struct object_id oid; struct pathspec pathspec; int intent_to_add = 0; - struct add_p_opt add_p_opt = ADD_P_OPT_INIT; + struct interactive_options interactive_opts = INTERACTIVE_OPTIONS_INIT; const struct option options[] = { OPT__QUIET(&quiet, N_("be quiet, only report errors")), OPT_BOOL(0, "no-refresh", &no_refresh, @@ -371,10 +371,10 @@ int cmd_reset(int argc, PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater), OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")), - OPT_BOOL(0, "auto-advance", &add_p_opt.auto_advance, + OPT_BOOL(0, "auto-advance", &interactive_opts.auto_advance, N_("auto advance to the next file when selecting hunks interactively")), - OPT_DIFF_UNIFIED(&add_p_opt.context), - OPT_DIFF_INTERHUNK_CONTEXT(&add_p_opt.interhunkcontext), + OPT_DIFF_UNIFIED(&interactive_opts.context), + OPT_DIFF_INTERHUNK_CONTEXT(&interactive_opts.interhunkcontext), OPT_BOOL('N', "intent-to-add", &intent_to_add, N_("record only the fact that removed paths will be added later")), OPT_PATHSPEC_FROM_FILE(&pathspec_from_file), @@ -425,9 +425,9 @@ int cmd_reset(int argc, oidcpy(&oid, &tree->object.oid); } - if (add_p_opt.context < -1) + if (interactive_opts.context < -1) die(_("'%s' cannot be negative"), "--unified"); - if (add_p_opt.interhunkcontext < -1) + if (interactive_opts.interhunkcontext < -1) die(_("'%s' cannot be negative"), "--inter-hunk-context"); prepare_repo_settings(the_repository); @@ -438,14 +438,14 @@ int cmd_reset(int argc, die(_("options '%s' and '%s' cannot be used together"), "--patch", "--{hard,mixed,soft}"); trace2_cmd_mode("patch-interactive"); update_ref_status = !!run_add_p(the_repository, ADD_P_RESET, - &add_p_opt, rev, &pathspec); + &interactive_opts, rev, &pathspec); goto cleanup; } else { - if (add_p_opt.context != -1) + if (interactive_opts.context != -1) die(_("the option '%s' requires '%s'"), "--unified", "--patch"); - if (add_p_opt.interhunkcontext != -1) + if (interactive_opts.interhunkcontext != -1) die(_("the option '%s' requires '%s'"), "--inter-hunk-context", "--patch"); - if (!add_p_opt.auto_advance) + if (!interactive_opts.auto_advance) die(_("the option '%s' requires '%s'"), "--no-auto-advance", "--patch"); } diff --git a/builtin/stash.c b/builtin/stash.c index e79d612e57..c467c02c7f 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -1306,7 +1306,7 @@ static int stash_staged(struct stash_info *info, struct strbuf *out_patch, static int stash_patch(struct stash_info *info, const struct pathspec *ps, struct strbuf *out_patch, int quiet, - struct add_p_opt *add_p_opt) + struct interactive_options *interactive_opts) { int ret = 0; struct child_process cp_read_tree = CHILD_PROCESS_INIT; @@ -1331,7 +1331,7 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps, old_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT)); setenv(INDEX_ENVIRONMENT, the_repository->index_file, 1); - ret = !!run_add_p(the_repository, ADD_P_STASH, add_p_opt, NULL, ps); + ret = !!run_add_p(the_repository, ADD_P_STASH, interactive_opts, NULL, ps); the_repository->index_file = old_repo_index_file; if (old_index_env && *old_index_env) @@ -1427,7 +1427,8 @@ static int stash_working_tree(struct stash_info *info, const struct pathspec *ps } static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_buf, - int include_untracked, int patch_mode, struct add_p_opt *add_p_opt, + int include_untracked, int patch_mode, + struct interactive_options *interactive_opts, int only_staged, struct stash_info *info, struct strbuf *patch, int quiet) { @@ -1509,7 +1510,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b untracked_commit_option = 1; } if (patch_mode) { - ret = stash_patch(info, ps, patch, quiet, add_p_opt); + ret = stash_patch(info, ps, patch, quiet, interactive_opts); if (ret < 0) { if (!quiet) fprintf_ln(stderr, _("Cannot save the current " @@ -1595,7 +1596,8 @@ static int create_stash(int argc, const char **argv, const char *prefix UNUSED, } static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int quiet, - int keep_index, int patch_mode, struct add_p_opt *add_p_opt, + int keep_index, int patch_mode, + struct interactive_options *interactive_opts, int include_untracked, int only_staged) { int ret = 0; @@ -1667,7 +1669,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q if (stash_msg) strbuf_addstr(&stash_msg_buf, stash_msg); if (do_create_stash(ps, &stash_msg_buf, include_untracked, patch_mode, - add_p_opt, only_staged, &info, &patch, quiet)) { + interactive_opts, only_staged, &info, &patch, quiet)) { ret = -1; goto done; } @@ -1841,7 +1843,7 @@ static int push_stash(int argc, const char **argv, const char *prefix, const char *stash_msg = NULL; char *pathspec_from_file = NULL; struct pathspec ps; - struct add_p_opt add_p_opt = ADD_P_OPT_INIT; + struct interactive_options interactive_opts = INTERACTIVE_OPTIONS_INIT; struct option options[] = { OPT_BOOL('k', "keep-index", &keep_index, N_("keep index")), @@ -1849,10 +1851,10 @@ static int push_stash(int argc, const char **argv, const char *prefix, N_("stash staged changes only")), OPT_BOOL('p', "patch", &patch_mode, N_("stash in patch mode")), - OPT_BOOL(0, "auto-advance", &add_p_opt.auto_advance, + OPT_BOOL(0, "auto-advance", &interactive_opts.auto_advance, N_("auto advance to the next file when selecting hunks interactively")), - OPT_DIFF_UNIFIED(&add_p_opt.context), - OPT_DIFF_INTERHUNK_CONTEXT(&add_p_opt.interhunkcontext), + OPT_DIFF_UNIFIED(&interactive_opts.context), + OPT_DIFF_INTERHUNK_CONTEXT(&interactive_opts.interhunkcontext), OPT__QUIET(&quiet, N_("quiet mode")), OPT_BOOL('u', "include-untracked", &include_untracked, N_("include untracked files in stash")), @@ -1909,21 +1911,21 @@ static int push_stash(int argc, const char **argv, const char *prefix, } if (!patch_mode) { - if (add_p_opt.context != -1) + if (interactive_opts.context != -1) die(_("the option '%s' requires '%s'"), "--unified", "--patch"); - if (add_p_opt.interhunkcontext != -1) + if (interactive_opts.interhunkcontext != -1) die(_("the option '%s' requires '%s'"), "--inter-hunk-context", "--patch"); - if (!add_p_opt.auto_advance) + if (!interactive_opts.auto_advance) die(_("the option '%s' requires '%s'"), "--no-auto-advance", "--patch"); } - if (add_p_opt.context < -1) + if (interactive_opts.context < -1) die(_("'%s' cannot be negative"), "--unified"); - if (add_p_opt.interhunkcontext < -1) + if (interactive_opts.interhunkcontext < -1) die(_("'%s' cannot be negative"), "--inter-hunk-context"); ret = do_push_stash(&ps, stash_msg, quiet, keep_index, patch_mode, - &add_p_opt, include_untracked, only_staged); + &interactive_opts, include_untracked, only_staged); clear_pathspec(&ps); free(pathspec_from_file); @@ -1948,7 +1950,7 @@ static int save_stash(int argc, const char **argv, const char *prefix, const char *stash_msg = NULL; struct pathspec ps; struct strbuf stash_msg_buf = STRBUF_INIT; - struct add_p_opt add_p_opt = ADD_P_OPT_INIT; + struct interactive_options interactive_opts = INTERACTIVE_OPTIONS_INIT; struct option options[] = { OPT_BOOL('k', "keep-index", &keep_index, N_("keep index")), @@ -1956,10 +1958,10 @@ static int save_stash(int argc, const char **argv, const char *prefix, N_("stash staged changes only")), OPT_BOOL('p', "patch", &patch_mode, N_("stash in patch mode")), - OPT_BOOL(0, "auto-advance", &add_p_opt.auto_advance, + OPT_BOOL(0, "auto-advance", &interactive_opts.auto_advance, N_("auto advance to the next file when selecting hunks interactively")), - OPT_DIFF_UNIFIED(&add_p_opt.context), - OPT_DIFF_INTERHUNK_CONTEXT(&add_p_opt.interhunkcontext), + OPT_DIFF_UNIFIED(&interactive_opts.context), + OPT_DIFF_INTERHUNK_CONTEXT(&interactive_opts.interhunkcontext), OPT__QUIET(&quiet, N_("quiet mode")), OPT_BOOL('u', "include-untracked", &include_untracked, N_("include untracked files in stash")), @@ -1979,22 +1981,22 @@ static int save_stash(int argc, const char **argv, const char *prefix, memset(&ps, 0, sizeof(ps)); - if (add_p_opt.context < -1) + if (interactive_opts.context < -1) die(_("'%s' cannot be negative"), "--unified"); - if (add_p_opt.interhunkcontext < -1) + if (interactive_opts.interhunkcontext < -1) die(_("'%s' cannot be negative"), "--inter-hunk-context"); if (!patch_mode) { - if (add_p_opt.context != -1) + if (interactive_opts.context != -1) die(_("the option '%s' requires '%s'"), "--unified", "--patch"); - if (add_p_opt.interhunkcontext != -1) + if (interactive_opts.interhunkcontext != -1) die(_("the option '%s' requires '%s'"), "--inter-hunk-context", "--patch"); - if (!add_p_opt.auto_advance) + if (!interactive_opts.auto_advance) die(_("the option '%s' requires '%s'"), "--no-auto-advance", "--patch"); } ret = do_push_stash(&ps, stash_msg, quiet, keep_index, - patch_mode, &add_p_opt, include_untracked, + patch_mode, &interactive_opts, include_untracked, only_staged); strbuf_release(&stash_msg_buf); diff --git a/commit.h b/commit.h index 1635de418b..a2337a5af0 100644 --- a/commit.h +++ b/commit.h @@ -277,7 +277,7 @@ int for_each_commit_graft(each_commit_graft_fn, void *); int interactive_add(struct repository *repo, const char **argv, const char *prefix, - int patch, struct add_p_opt *add_p_opt); + int patch, struct interactive_options *opts); struct commit_extra_header { struct commit_extra_header *next; -- 2.53.0.697.g625c4fb2da.dirty ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/8] add-patch: remove dependency on "add-interactive" subsystem 2026-03-02 12:13 [PATCH 0/8] history: introduce "split" subcommand Patrick Steinhardt 2026-03-02 12:13 ` [PATCH 1/8] add-patch: split out header from "add-interactive.h" Patrick Steinhardt 2026-03-02 12:13 ` [PATCH 2/8] add-patch: split out `struct interactive_options` Patrick Steinhardt @ 2026-03-02 12:13 ` Patrick Steinhardt 2026-03-02 12:13 ` [PATCH 4/8] add-patch: add support for in-memory index patching Patrick Steinhardt ` (6 subsequent siblings) 9 siblings, 0 replies; 16+ messages in thread From: Patrick Steinhardt @ 2026-03-02 12:13 UTC (permalink / raw) To: git; +Cc: Elijah Newren With the preceding commit we have split out interactive configuration that is used by both "git add -p" and "git add -i". But we still initialize that configuration in the "add -p" subsystem by calling `init_add_i_state()`, even though we only do so to initialize the interactive configuration as well as a repository pointer. Stop doing so and instead store and initialize the interactive configuration in `struct add_p_state` directly. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- add-patch.c | 88 ++++++++++++++++++++++++++++++++----------------------------- 1 file changed, 46 insertions(+), 42 deletions(-) diff --git a/add-patch.c b/add-patch.c index 756143eb84..4f089c82d0 100644 --- a/add-patch.c +++ b/add-patch.c @@ -2,7 +2,6 @@ #define DISABLE_SIGN_COMPARE_WARNINGS #include "git-compat-util.h" -#include "add-interactive.h" #include "add-patch.h" #include "advice.h" #include "config.h" @@ -263,7 +262,8 @@ struct hunk { }; struct add_p_state { - struct add_i_state s; + struct repository *r; + struct interactive_config cfg; struct strbuf answer, buf; /* parsed diff */ @@ -409,7 +409,7 @@ static void add_p_state_clear(struct add_p_state *s) for (i = 0; i < s->file_diff_nr; i++) free(s->file_diff[i].hunk); free(s->file_diff); - clear_add_i_state(&s->s); + interactive_config_clear(&s->cfg); } __attribute__((format (printf, 2, 3))) @@ -418,9 +418,9 @@ static void err(struct add_p_state *s, const char *fmt, ...) va_list args; va_start(args, fmt); - fputs(s->s.cfg.error_color, stdout); + fputs(s->cfg.error_color, stdout); vprintf(fmt, args); - puts(s->s.cfg.reset_color_interactive); + puts(s->cfg.reset_color_interactive); va_end(args); } @@ -438,7 +438,7 @@ static void setup_child_process(struct add_p_state *s, cp->git_cmd = 1; strvec_pushf(&cp->env, - INDEX_ENVIRONMENT "=%s", s->s.r->index_file); + INDEX_ENVIRONMENT "=%s", s->r->index_file); } static int parse_range(const char **p, @@ -543,12 +543,12 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) int res; strvec_pushv(&args, s->mode->diff_cmd); - if (s->s.cfg.context != -1) - strvec_pushf(&args, "--unified=%i", s->s.cfg.context); - if (s->s.cfg.interhunkcontext != -1) - strvec_pushf(&args, "--inter-hunk-context=%i", s->s.cfg.interhunkcontext); - if (s->s.cfg.interactive_diff_algorithm) - strvec_pushf(&args, "--diff-algorithm=%s", s->s.cfg.interactive_diff_algorithm); + if (s->cfg.context != -1) + strvec_pushf(&args, "--unified=%i", s->cfg.context); + if (s->cfg.interhunkcontext != -1) + strvec_pushf(&args, "--inter-hunk-context=%i", s->cfg.interhunkcontext); + if (s->cfg.interactive_diff_algorithm) + strvec_pushf(&args, "--diff-algorithm=%s", s->cfg.interactive_diff_algorithm); if (s->revision) { struct object_id oid; strvec_push(&args, @@ -577,9 +577,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) } strbuf_complete_line(plain); - if (want_color_fd(1, s->s.cfg.use_color_diff)) { + if (want_color_fd(1, s->cfg.use_color_diff)) { struct child_process colored_cp = CHILD_PROCESS_INIT; - const char *diff_filter = s->s.cfg.interactive_diff_filter; + const char *diff_filter = s->cfg.interactive_diff_filter; setup_child_process(s, &colored_cp, NULL); xsnprintf((char *)args.v[color_arg_index], 8, "--color"); @@ -812,7 +812,7 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk, hunk->colored_end - hunk->colored_start); return; } else { - strbuf_addstr(out, s->s.cfg.fraginfo_color); + strbuf_addstr(out, s->cfg.fraginfo_color); p = s->colored.buf + header->colored_extra_start; len = header->colored_extra_end - header->colored_extra_start; @@ -834,7 +834,7 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk, if (len) strbuf_add(out, p, len); else if (colored) - strbuf_addf(out, "%s\n", s->s.cfg.reset_color_diff); + strbuf_addf(out, "%s\n", s->cfg.reset_color_diff); else strbuf_addch(out, '\n'); } @@ -1223,12 +1223,12 @@ static void recolor_hunk(struct add_p_state *s, struct hunk *hunk) strbuf_addstr(&s->colored, plain[current] == '-' ? - s->s.cfg.file_old_color : + s->cfg.file_old_color : plain[current] == '+' ? - s->s.cfg.file_new_color : - s->s.cfg.context_color); + s->cfg.file_new_color : + s->cfg.context_color); strbuf_add(&s->colored, plain + current, eol - current); - strbuf_addstr(&s->colored, s->s.cfg.reset_color_diff); + strbuf_addstr(&s->colored, s->cfg.reset_color_diff); if (next > eol) strbuf_add(&s->colored, plain + eol, next - eol); current = next; @@ -1357,7 +1357,7 @@ static int run_apply_check(struct add_p_state *s, static int read_single_character(struct add_p_state *s) { - if (s->s.cfg.use_single_key) { + if (s->cfg.use_single_key) { int res = read_key_without_echo(&s->answer); printf("%s\n", res == EOF ? "" : s->answer.buf); return res; @@ -1371,7 +1371,7 @@ static int read_single_character(struct add_p_state *s) static int prompt_yesno(struct add_p_state *s, const char *prompt) { for (;;) { - color_fprintf(stdout, s->s.cfg.prompt_color, "%s", _(prompt)); + color_fprintf(stdout, s->cfg.prompt_color, "%s", _(prompt)); fflush(stdout); if (read_single_character(s) == EOF) return -1; @@ -1559,7 +1559,7 @@ static void apply_patch(struct add_p_state *s, struct file_diff *file_diff) strbuf_reset(&s->buf); reassemble_patch(s, file_diff, 0, &s->buf); - discard_index(s->s.r->index); + discard_index(s->r->index); if (s->mode->apply_for_checkout) apply_for_checkout(s, &s->buf, s->mode->is_reverse); @@ -1570,9 +1570,9 @@ static void apply_patch(struct add_p_state *s, struct file_diff *file_diff) NULL, 0, NULL, 0)) error(_("'git apply' failed")); } - if (repo_read_index(s->s.r) >= 0) - repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0, - 1, NULL, NULL, NULL); + if (repo_read_index(s->r) >= 0) + repo_refresh_and_write_index(s->r, REFRESH_QUIET, 0, + 1, NULL, NULL, NULL); } } @@ -1660,7 +1660,7 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) /* Everything decided? */ if (undecided_previous < 0 && undecided_next < 0 && hunk->use != UNDECIDED_HUNK) { - if (!s->s.cfg.auto_advance) + if (!s->cfg.auto_advance) all_decided = 1; else { patch_update_resp++; @@ -1714,11 +1714,11 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) permitted |= ALLOW_EDIT; strbuf_addstr(&s->buf, ",e"); } - if (!s->s.cfg.auto_advance && s->file_diff_nr > 1) { + if (!s->cfg.auto_advance && s->file_diff_nr > 1) { permitted |= ALLOW_GOTO_NEXT_FILE; strbuf_addstr(&s->buf, ",>"); } - if (!s->s.cfg.auto_advance && s->file_diff_nr > 1) { + if (!s->cfg.auto_advance && s->file_diff_nr > 1) { permitted |= ALLOW_GOTO_PREVIOUS_FILE; strbuf_addstr(&s->buf, ",<"); } @@ -1733,7 +1733,7 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) else prompt_mode_type = PROMPT_HUNK; - printf("%s(%"PRIuMAX"/%"PRIuMAX") ", s->s.cfg.prompt_color, + printf("%s(%"PRIuMAX"/%"PRIuMAX") ", s->cfg.prompt_color, (uintmax_t)hunk_index + 1, (uintmax_t)(file_diff->hunk_nr ? file_diff->hunk_nr @@ -1746,8 +1746,8 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) } printf(_(s->mode->prompt_mode[prompt_mode_type]), hunk_use_decision, s->buf.buf); - if (*s->s.cfg.reset_color_interactive) - fputs(s->s.cfg.reset_color_interactive, stdout); + if (*s->cfg.reset_color_interactive) + fputs(s->cfg.reset_color_interactive, stdout); fflush(stdout); if (read_single_character(s) == EOF) { patch_update_resp = s->file_diff_nr; @@ -1798,7 +1798,7 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) } else if (ch == 'q') { patch_update_resp = s->file_diff_nr; break; - } else if (!s->s.cfg.auto_advance && s->answer.buf[0] == '>') { + } else if (!s->cfg.auto_advance && s->answer.buf[0] == '>') { if (permitted & ALLOW_GOTO_NEXT_FILE) { if (patch_update_resp == s->file_diff_nr - 1) patch_update_resp = 0; @@ -1809,7 +1809,7 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) err(s, _("No next file")); continue; } - } else if (!s->s.cfg.auto_advance && s->answer.buf[0] == '<') { + } else if (!s->cfg.auto_advance && s->answer.buf[0] == '<') { if (permitted & ALLOW_GOTO_PREVIOUS_FILE) { if (patch_update_resp == 0) patch_update_resp = s->file_diff_nr - 1; @@ -1932,7 +1932,7 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) err(s, _("Sorry, cannot split this hunk")); } else if (!split_hunk(s, file_diff, hunk - file_diff->hunk)) { - color_fprintf_ln(stdout, s->s.cfg.header_color, + color_fprintf_ln(stdout, s->cfg.header_color, _("Split into %d hunks."), (int)splittable_into); rendered_hunk_index = -1; @@ -1950,7 +1950,7 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) } else if (s->answer.buf[0] == '?') { const char *p = _(help_patch_remainder), *eol = p; - color_fprintf(stdout, s->s.cfg.help_color, "%s", + color_fprintf(stdout, s->cfg.help_color, "%s", _(s->mode->help_patch_text)); /* @@ -1974,13 +1974,13 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) if (file_diff->hunk[i].use == SKIP_HUNK) skipped += 1; } - color_fprintf_ln(stdout, s->s.cfg.help_color, _(p), + color_fprintf_ln(stdout, s->cfg.help_color, _(p), total, used, skipped); } if (*p != '?' && !strchr(s->buf.buf, *p)) continue; - color_fprintf_ln(stdout, s->s.cfg.help_color, + color_fprintf_ln(stdout, s->cfg.help_color, "%.*s", (int)(eol - p), p); } } else { @@ -1989,7 +1989,7 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) } } - if (s->s.cfg.auto_advance) + if (s->cfg.auto_advance) apply_patch(s, file_diff); putchar('\n'); @@ -2001,11 +2001,15 @@ int run_add_p(struct repository *r, enum add_p_mode mode, const struct pathspec *ps) { struct add_p_state s = { - { r }, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT + .r = r, + .answer = STRBUF_INIT, + .buf = STRBUF_INIT, + .plain = STRBUF_INIT, + .colored = STRBUF_INIT, }; size_t i, binary_count = 0; - init_add_i_state(&s.s, r, opts); + interactive_config_init(&s.cfg, r, opts); if (mode == ADD_P_STASH) s.mode = &patch_mode_stash; @@ -2051,7 +2055,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode, if ((i = patch_update_file(&s, i)) == s.file_diff_nr) break; } - if (!s.s.cfg.auto_advance) + if (!s.cfg.auto_advance) for (i = 0; i < s.file_diff_nr; i++) apply_patch(&s, s.file_diff + i); -- 2.53.0.697.g625c4fb2da.dirty ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/8] add-patch: add support for in-memory index patching 2026-03-02 12:13 [PATCH 0/8] history: introduce "split" subcommand Patrick Steinhardt ` (2 preceding siblings ...) 2026-03-02 12:13 ` [PATCH 3/8] add-patch: remove dependency on "add-interactive" subsystem Patrick Steinhardt @ 2026-03-02 12:13 ` Patrick Steinhardt 2026-03-02 12:13 ` [PATCH 5/8] add-patch: allow disabling editing of hunks Patrick Steinhardt ` (5 subsequent siblings) 9 siblings, 0 replies; 16+ messages in thread From: Patrick Steinhardt @ 2026-03-02 12:13 UTC (permalink / raw) To: git; +Cc: Elijah Newren With `run_add_p()` callers have the ability to apply changes from a specific revision to a repository's index. This infra supports several different modes, like for example applying changes to the index, working tree or both. One feature that is missing though is the ability to apply changes to an in-memory index different from the repository's index. Add a new function `run_add_p_index()` to plug this gap. This new function will be used in a subsequent commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- add-patch.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++---------- add-patch.h | 8 ++++ 2 files changed, 132 insertions(+), 25 deletions(-) diff --git a/add-patch.c b/add-patch.c index 4f089c82d0..b4dc7d2293 100644 --- a/add-patch.c +++ b/add-patch.c @@ -4,11 +4,13 @@ #include "git-compat-util.h" #include "add-patch.h" #include "advice.h" +#include "commit.h" #include "config.h" #include "diff.h" #include "editor.h" #include "environment.h" #include "gettext.h" +#include "hex.h" #include "object-name.h" #include "pager.h" #include "read-cache-ll.h" @@ -263,6 +265,8 @@ struct hunk { struct add_p_state { struct repository *r; + struct index_state *index; + const char *index_file; struct interactive_config cfg; struct strbuf answer, buf; @@ -438,7 +442,7 @@ static void setup_child_process(struct add_p_state *s, cp->git_cmd = 1; strvec_pushf(&cp->env, - INDEX_ENVIRONMENT "=%s", s->r->index_file); + INDEX_ENVIRONMENT "=%s", s->index_file); } static int parse_range(const char **p, @@ -1559,7 +1563,7 @@ static void apply_patch(struct add_p_state *s, struct file_diff *file_diff) strbuf_reset(&s->buf); reassemble_patch(s, file_diff, 0, &s->buf); - discard_index(s->r->index); + discard_index(s->index); if (s->mode->apply_for_checkout) apply_for_checkout(s, &s->buf, s->mode->is_reverse); @@ -1570,9 +1574,11 @@ static void apply_patch(struct add_p_state *s, struct file_diff *file_diff) NULL, 0, NULL, 0)) error(_("'git apply' failed")); } - if (repo_read_index(s->r) >= 0) + if (read_index_from(s->index, s->index_file, s->r->gitdir) >= 0 && + s->index == s->r->index) { repo_refresh_and_write_index(s->r, REFRESH_QUIET, 0, 1, NULL, NULL, NULL); + } } } @@ -1996,18 +2002,51 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) return patch_update_resp; } +static int run_add_p_common(struct add_p_state *state, + const struct pathspec *ps) +{ + size_t binary_count = 0; + size_t i; + + if (parse_diff(state, ps) < 0) + return -1; + + for (i = 0; i < state->file_diff_nr;) { + if (state->file_diff[i].binary && !state->file_diff[i].hunk_nr) { + binary_count++; + i++; + continue; + } + if ((i = patch_update_file(state, i)) == state->file_diff_nr) + break; + } + + if (!state->cfg.auto_advance) + for (i = 0; i < state->file_diff_nr; i++) + apply_patch(state, state->file_diff + i); + + if (state->file_diff_nr == 0) + err(state, _("No changes.")); + else if (binary_count == state->file_diff_nr) + err(state, _("Only binary files changed.")); + + return 0; +} + int run_add_p(struct repository *r, enum add_p_mode mode, struct interactive_options *opts, const char *revision, const struct pathspec *ps) { struct add_p_state s = { .r = r, + .index = r->index, + .index_file = r->index_file, .answer = STRBUF_INIT, .buf = STRBUF_INIT, .plain = STRBUF_INIT, .colored = STRBUF_INIT, }; - size_t i, binary_count = 0; + int ret; interactive_config_init(&s.cfg, r, opts); @@ -2040,30 +2079,90 @@ int run_add_p(struct repository *r, enum add_p_mode mode, if (repo_read_index(r) < 0 || (!s.mode->index_only && repo_refresh_and_write_index(r, REFRESH_QUIET, 0, 1, - NULL, NULL, NULL) < 0) || - parse_diff(&s, ps) < 0) { - add_p_state_clear(&s); - return -1; + NULL, NULL, NULL) < 0)) { + ret = -1; + goto out; } - for (i = 0; i < s.file_diff_nr;) { - if (s.file_diff[i].binary && !s.file_diff[i].hunk_nr) { - binary_count++; - i++; - continue; - } - if ((i = patch_update_file(&s, i)) == s.file_diff_nr) - break; - } - if (!s.cfg.auto_advance) - for (i = 0; i < s.file_diff_nr; i++) - apply_patch(&s, s.file_diff + i); + ret = run_add_p_common(&s, ps); + if (ret < 0) + goto out; - if (s.file_diff_nr == 0) - err(&s, _("No changes.")); - else if (binary_count == s.file_diff_nr) - err(&s, _("Only binary files changed.")); + ret = 0; +out: add_p_state_clear(&s); - return 0; + return ret; +} + +int run_add_p_index(struct repository *r, + struct index_state *index, + const char *index_file, + struct interactive_options *opts, + const char *revision, + const struct pathspec *ps) +{ + struct patch_mode mode = { + .apply_args = { "--cached", NULL }, + .apply_check_args = { "--cached", NULL }, + .prompt_mode = { + N_("Stage mode change [y,n,q,a,d%s,?]? "), + N_("Stage deletion [y,n,q,a,d%s,?]? "), + N_("Stage addition [y,n,q,a,d%s,?]? "), + N_("Stage this hunk [y,n,q,a,d%s,?]? ") + }, + .edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk " + "will immediately be marked for staging."), + .help_patch_text = + N_("y - stage this hunk\n" + "n - do not stage this hunk\n" + "q - quit; do not stage this hunk or any of the remaining " + "ones\n" + "a - stage this hunk and all later hunks in the file\n" + "d - do not stage this hunk or any of the later hunks in " + "the file\n"), + .index_only = 1, + }; + struct add_p_state s = { + .r = r, + .index = index, + .index_file = index_file, + .answer = STRBUF_INIT, + .buf = STRBUF_INIT, + .plain = STRBUF_INIT, + .colored = STRBUF_INIT, + .mode = &mode, + .revision = revision, + }; + char parent_tree_oid[GIT_MAX_HEXSZ + 1]; + struct commit *commit; + int ret; + + interactive_config_init(&s.cfg, r, opts); + + commit = lookup_commit_reference_by_name(revision); + if (!commit) { + err(&s, _("Revision does not refer to a commit")); + ret = -1; + goto out; + } + + if (commit->parents) + oid_to_hex_r(parent_tree_oid, get_commit_tree_oid(commit->parents->item)); + else + oid_to_hex_r(parent_tree_oid, r->hash_algo->empty_tree); + + mode.diff_cmd[0] = "diff-tree"; + mode.diff_cmd[1] = "-r"; + mode.diff_cmd[2] = parent_tree_oid; + + ret = run_add_p_common(&s, ps); + if (ret < 0) + goto out; + + ret = 0; + +out: + add_p_state_clear(&s); + return ret; } diff --git a/add-patch.h b/add-patch.h index e6868c60a2..cf2a31a40f 100644 --- a/add-patch.h +++ b/add-patch.h @@ -3,6 +3,7 @@ #include "color.h" +struct index_state; struct pathspec; struct repository; @@ -56,4 +57,11 @@ int run_add_p(struct repository *r, enum add_p_mode mode, struct interactive_options *opts, const char *revision, const struct pathspec *ps); +int run_add_p_index(struct repository *r, + struct index_state *index, + const char *index_file, + struct interactive_options *opts, + const char *revision, + const struct pathspec *ps); + #endif -- 2.53.0.697.g625c4fb2da.dirty ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 5/8] add-patch: allow disabling editing of hunks 2026-03-02 12:13 [PATCH 0/8] history: introduce "split" subcommand Patrick Steinhardt ` (3 preceding siblings ...) 2026-03-02 12:13 ` [PATCH 4/8] add-patch: add support for in-memory index patching Patrick Steinhardt @ 2026-03-02 12:13 ` Patrick Steinhardt 2026-03-02 12:13 ` [PATCH 6/8] cache-tree: allow writing in-memory index as tree Patrick Steinhardt ` (4 subsequent siblings) 9 siblings, 0 replies; 16+ messages in thread From: Patrick Steinhardt @ 2026-03-02 12:13 UTC (permalink / raw) To: git; +Cc: Elijah Newren The "add-patch" mode allows the user to edit hunks to apply custom changes. This is incompatible with a new `git history split` command that we're about to introduce in a subsequent commit, so we need a way to disable this mode. Add a new flag to disable editing hunks. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- add-interactive.c | 2 +- add-patch.c | 22 ++++++++++++++-------- add-patch.h | 11 +++++++++-- builtin/add.c | 2 +- builtin/checkout.c | 2 +- builtin/reset.c | 2 +- builtin/stash.c | 2 +- 7 files changed, 28 insertions(+), 15 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 152e2a0297..3cf8a1dbf8 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -927,7 +927,7 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps, parse_pathspec(&ps_selected, PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL, PATHSPEC_LITERAL_PATH, "", args.v); - res = run_add_p(s->r, ADD_P_ADD, &opts, NULL, &ps_selected); + res = run_add_p(s->r, ADD_P_ADD, &opts, NULL, &ps_selected, 0); strvec_clear(&args); clear_pathspec(&ps_selected); } diff --git a/add-patch.c b/add-patch.c index b4dc7d2293..4e28e5c187 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1604,7 +1604,9 @@ static bool get_first_undecided(const struct file_diff *file_diff, size_t *idx) return false; } -static size_t patch_update_file(struct add_p_state *s, size_t idx) +static size_t patch_update_file(struct add_p_state *s, + size_t idx, + unsigned flags) { size_t hunk_index = 0; ssize_t i, undecided_previous, undecided_next, rendered_hunk_index = -1; @@ -1715,7 +1717,8 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) permitted |= ALLOW_SPLIT; strbuf_addstr(&s->buf, ",s"); } - if (hunk_index + 1 > file_diff->mode_change && + if (!(flags & ADD_P_DISALLOW_EDIT) && + hunk_index + 1 > file_diff->mode_change && !file_diff->deleted) { permitted |= ALLOW_EDIT; strbuf_addstr(&s->buf, ",e"); @@ -2003,7 +2006,8 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) } static int run_add_p_common(struct add_p_state *state, - const struct pathspec *ps) + const struct pathspec *ps, + unsigned flags) { size_t binary_count = 0; size_t i; @@ -2017,7 +2021,7 @@ static int run_add_p_common(struct add_p_state *state, i++; continue; } - if ((i = patch_update_file(state, i)) == state->file_diff_nr) + if ((i = patch_update_file(state, i, flags)) == state->file_diff_nr) break; } @@ -2035,7 +2039,8 @@ static int run_add_p_common(struct add_p_state *state, int run_add_p(struct repository *r, enum add_p_mode mode, struct interactive_options *opts, const char *revision, - const struct pathspec *ps) + const struct pathspec *ps, + unsigned flags) { struct add_p_state s = { .r = r, @@ -2084,7 +2089,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode, goto out; } - ret = run_add_p_common(&s, ps); + ret = run_add_p_common(&s, ps, flags); if (ret < 0) goto out; @@ -2100,7 +2105,8 @@ int run_add_p_index(struct repository *r, const char *index_file, struct interactive_options *opts, const char *revision, - const struct pathspec *ps) + const struct pathspec *ps, + unsigned flags) { struct patch_mode mode = { .apply_args = { "--cached", NULL }, @@ -2156,7 +2162,7 @@ int run_add_p_index(struct repository *r, mode.diff_cmd[1] = "-r"; mode.diff_cmd[2] = parent_tree_oid; - ret = run_add_p_common(&s, ps); + ret = run_add_p_common(&s, ps, flags); if (ret < 0) goto out; diff --git a/add-patch.h b/add-patch.h index cf2a31a40f..fb6d975b68 100644 --- a/add-patch.h +++ b/add-patch.h @@ -53,15 +53,22 @@ enum add_p_mode { ADD_P_WORKTREE, }; +enum add_p_flags { + /* Disallow "editing" hunks. */ + ADD_P_DISALLOW_EDIT = (1 << 0), +}; + int run_add_p(struct repository *r, enum add_p_mode mode, struct interactive_options *opts, const char *revision, - const struct pathspec *ps); + const struct pathspec *ps, + unsigned flags); int run_add_p_index(struct repository *r, struct index_state *index, const char *index_file, struct interactive_options *opts, const char *revision, - const struct pathspec *ps); + const struct pathspec *ps, + unsigned flags); #endif diff --git a/builtin/add.c b/builtin/add.c index 84f9bcb789..eeab779328 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -172,7 +172,7 @@ int interactive_add(struct repository *repo, prefix, argv); if (patch) - ret = !!run_add_p(repo, ADD_P_ADD, interactive_opts, NULL, &pathspec); + ret = !!run_add_p(repo, ADD_P_ADD, interactive_opts, NULL, &pathspec, 0); else ret = !!run_add_i(repo, &pathspec, interactive_opts); diff --git a/builtin/checkout.c b/builtin/checkout.c index bebe18c1d9..a8863277f2 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -563,7 +563,7 @@ static int checkout_paths(const struct checkout_opts *opts, BUG("either flag must have been set, worktree=%d, index=%d", opts->checkout_worktree, opts->checkout_index); return !!run_add_p(the_repository, patch_mode, &interactive_opts, - rev, &opts->pathspec); + rev, &opts->pathspec, 0); } repo_hold_locked_index(the_repository, &lock_file, LOCK_DIE_ON_ERROR); diff --git a/builtin/reset.c b/builtin/reset.c index 4a74a82c0a..3590be57a5 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -438,7 +438,7 @@ int cmd_reset(int argc, die(_("options '%s' and '%s' cannot be used together"), "--patch", "--{hard,mixed,soft}"); trace2_cmd_mode("patch-interactive"); update_ref_status = !!run_add_p(the_repository, ADD_P_RESET, - &interactive_opts, rev, &pathspec); + &interactive_opts, rev, &pathspec, 0); goto cleanup; } else { if (interactive_opts.context != -1) diff --git a/builtin/stash.c b/builtin/stash.c index c467c02c7f..7c68a1d7f9 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -1331,7 +1331,7 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps, old_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT)); setenv(INDEX_ENVIRONMENT, the_repository->index_file, 1); - ret = !!run_add_p(the_repository, ADD_P_STASH, interactive_opts, NULL, ps); + ret = !!run_add_p(the_repository, ADD_P_STASH, interactive_opts, NULL, ps, 0); the_repository->index_file = old_repo_index_file; if (old_index_env && *old_index_env) -- 2.53.0.697.g625c4fb2da.dirty ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 6/8] cache-tree: allow writing in-memory index as tree 2026-03-02 12:13 [PATCH 0/8] history: introduce "split" subcommand Patrick Steinhardt ` (4 preceding siblings ...) 2026-03-02 12:13 ` [PATCH 5/8] add-patch: allow disabling editing of hunks Patrick Steinhardt @ 2026-03-02 12:13 ` Patrick Steinhardt 2026-03-02 12:13 ` [PATCH 7/8] builtin/history: split out extended function to create commits Patrick Steinhardt ` (3 subsequent siblings) 9 siblings, 0 replies; 16+ messages in thread From: Patrick Steinhardt @ 2026-03-02 12:13 UTC (permalink / raw) To: git; +Cc: Elijah Newren The function `write_in_core_index_as_tree()` takes a repository and writes its index into a tree object. What this function cannot do though is to take an _arbitrary_ in-memory index. Introduce a new `struct index_state` parameter so that the caller can pass a different index than the one belonging to the repository. This will be used in a subsequent commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/checkout.c | 3 ++- cache-tree.c | 4 ++-- cache-tree.h | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index a8863277f2..f8b3a7b08c 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -891,7 +891,8 @@ static int merge_working_tree(const struct checkout_opts *opts, 0); init_ui_merge_options(&o, the_repository); o.verbosity = 0; - work = write_in_core_index_as_tree(the_repository); + work = write_in_core_index_as_tree(the_repository, + the_repository->index); ret = reset_tree(new_tree, opts, 1, diff --git a/cache-tree.c b/cache-tree.c index 16c3a36b48..60bcc07c3b 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -723,11 +723,11 @@ static int write_index_as_tree_internal(struct object_id *oid, return 0; } -struct tree* write_in_core_index_as_tree(struct repository *repo) { +struct tree *write_in_core_index_as_tree(struct repository *repo, + struct index_state *index_state) { struct object_id o; int was_valid, ret; - struct index_state *index_state = repo->index; was_valid = index_state->cache_tree && cache_tree_fully_valid(index_state->cache_tree); diff --git a/cache-tree.h b/cache-tree.h index b82c4963e7..f8bddae523 100644 --- a/cache-tree.h +++ b/cache-tree.h @@ -47,7 +47,8 @@ int cache_tree_verify(struct repository *, struct index_state *); #define WRITE_TREE_UNMERGED_INDEX (-2) #define WRITE_TREE_PREFIX_ERROR (-3) -struct tree* write_in_core_index_as_tree(struct repository *repo); +struct tree *write_in_core_index_as_tree(struct repository *repo, + struct index_state *index_state); int write_index_as_tree(struct object_id *oid, struct index_state *index_state, const char *index_path, int flags, const char *prefix); void prime_cache_tree(struct repository *, struct index_state *, struct tree *); -- 2.53.0.697.g625c4fb2da.dirty ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 7/8] builtin/history: split out extended function to create commits 2026-03-02 12:13 [PATCH 0/8] history: introduce "split" subcommand Patrick Steinhardt ` (5 preceding siblings ...) 2026-03-02 12:13 ` [PATCH 6/8] cache-tree: allow writing in-memory index as tree Patrick Steinhardt @ 2026-03-02 12:13 ` Patrick Steinhardt 2026-03-03 18:43 ` D. Ben Knoble 2026-03-02 12:13 ` [PATCH 8/8] builtin/history: implement "split" subcommand Patrick Steinhardt ` (2 subsequent siblings) 9 siblings, 1 reply; 16+ messages in thread From: Patrick Steinhardt @ 2026-03-02 12:13 UTC (permalink / raw) To: git; +Cc: Elijah Newren In the next commit we're about to introduce a new command that splits up a commit into two. Most of the logic will be shared with rewording commits, except that we also need to have control over the parents and the old/new trees. Extract a new function `commit_tree_with_edited_message_ext()` to prepare for this commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/history.c | 67 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/builtin/history.c b/builtin/history.c index 1cf6c668cf..80726ce14b 100644 --- a/builtin/history.c +++ b/builtin/history.c @@ -83,10 +83,13 @@ static int fill_commit_message(struct repository *repo, return 0; } -static int commit_tree_with_edited_message(struct repository *repo, - const char *action, - struct commit *original, - struct commit **out) +static int commit_tree_with_edited_message_ext(struct repository *repo, + const char *action, + struct commit *commit_with_message, + const struct commit_list *parents, + const struct object_id *old_tree, + const struct object_id *new_tree, + struct commit **out) { const char *exclude_gpgsig[] = { /* We reencode the message, so the encoding needs to be stripped. */ @@ -100,44 +103,27 @@ static int commit_tree_with_edited_message(struct repository *repo, struct commit_extra_header *original_extra_headers = NULL; struct strbuf commit_message = STRBUF_INIT; struct object_id rewritten_commit_oid; - struct object_id original_tree_oid; - struct object_id parent_tree_oid; char *original_author = NULL; - struct commit *parent; size_t len; int ret; - original_tree_oid = repo_get_commit_tree(repo, original)->object.oid; - - parent = original->parents ? original->parents->item : NULL; - if (parent) { - if (repo_parse_commit(repo, parent)) { - ret = error(_("unable to parse parent commit %s"), - oid_to_hex(&parent->object.oid)); - goto out; - } - - parent_tree_oid = repo_get_commit_tree(repo, parent)->object.oid; - } else { - oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree); - } - /* We retain authorship of the original commit. */ - original_message = repo_logmsg_reencode(repo, original, NULL, NULL); + original_message = repo_logmsg_reencode(repo, commit_with_message, NULL, NULL); ptr = find_commit_header(original_message, "author", &len); if (ptr) original_author = xmemdupz(ptr, len); find_commit_subject(original_message, &original_body); - ret = fill_commit_message(repo, &parent_tree_oid, &original_tree_oid, + ret = fill_commit_message(repo, old_tree, new_tree, original_body, action, &commit_message); if (ret < 0) goto out; - original_extra_headers = read_commit_extra_headers(original, exclude_gpgsig); + original_extra_headers = read_commit_extra_headers(commit_with_message, + exclude_gpgsig); - ret = commit_tree_extended(commit_message.buf, commit_message.len, &original_tree_oid, - original->parents, &rewritten_commit_oid, original_author, + ret = commit_tree_extended(commit_message.buf, commit_message.len, new_tree, + parents, &rewritten_commit_oid, original_author, NULL, NULL, original_extra_headers); if (ret < 0) goto out; @@ -151,6 +137,33 @@ static int commit_tree_with_edited_message(struct repository *repo, return ret; } +static int commit_tree_with_edited_message(struct repository *repo, + const char *action, + struct commit *original, + struct commit **out) +{ + struct object_id parent_tree_oid; + const struct object_id *tree_oid; + struct commit *parent; + + tree_oid = &repo_get_commit_tree(repo, original)->object.oid; + + parent = original->parents ? original->parents->item : NULL; + if (parent) { + if (repo_parse_commit(repo, parent)) { + return error(_("unable to parse parent commit %s"), + oid_to_hex(&parent->object.oid)); + } + + parent_tree_oid = repo_get_commit_tree(repo, parent)->object.oid; + } else { + oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree); + } + + return commit_tree_with_edited_message_ext(repo, action, original, original->parents, + &parent_tree_oid, tree_oid, out); +} + enum ref_action { REF_ACTION_DEFAULT, REF_ACTION_BRANCHES, -- 2.53.0.697.g625c4fb2da.dirty ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 7/8] builtin/history: split out extended function to create commits 2026-03-02 12:13 ` [PATCH 7/8] builtin/history: split out extended function to create commits Patrick Steinhardt @ 2026-03-03 18:43 ` D. Ben Knoble 2026-03-11 9:26 ` Patrick Steinhardt 0 siblings, 1 reply; 16+ messages in thread From: D. Ben Knoble @ 2026-03-03 18:43 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git, Elijah Newren On Mon, Mar 2, 2026 at 7:17 AM Patrick Steinhardt <ps@pks.im> wrote: > > In the next commit we're about to introduce a new command that splits up > a commit into two. Most of the logic will be shared with rewording > commits, except that we also need to have control over the parents and > the old/new trees. > > Extract a new function `commit_tree_with_edited_message_ext()` to > prepare for this commit. Curious—what's the "ext" suffix mean here. Extracted? External? (Maybe I'll get a better clue in the next patch.) > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > builtin/history.c | 67 +++++++++++++++++++++++++++++++++---------------------- > 1 file changed, 40 insertions(+), 27 deletions(-) > > diff --git a/builtin/history.c b/builtin/history.c > index 1cf6c668cf..80726ce14b 100644 > --- a/builtin/history.c > +++ b/builtin/history.c > @@ -83,10 +83,13 @@ static int fill_commit_message(struct repository *repo, > return 0; > } > > -static int commit_tree_with_edited_message(struct repository *repo, > - const char *action, > - struct commit *original, > - struct commit **out) > +static int commit_tree_with_edited_message_ext(struct repository *repo, > + const char *action, > + struct commit *commit_with_message, > + const struct commit_list *parents, > + const struct object_id *old_tree, > + const struct object_id *new_tree, > + struct commit **out) > { > const char *exclude_gpgsig[] = { > /* We reencode the message, so the encoding needs to be stripped. */ > @@ -100,44 +103,27 @@ static int commit_tree_with_edited_message(struct repository *repo, > struct commit_extra_header *original_extra_headers = NULL; > struct strbuf commit_message = STRBUF_INIT; > struct object_id rewritten_commit_oid; > - struct object_id original_tree_oid; > - struct object_id parent_tree_oid; > char *original_author = NULL; > - struct commit *parent; > size_t len; > int ret; > > - original_tree_oid = repo_get_commit_tree(repo, original)->object.oid; > - > - parent = original->parents ? original->parents->item : NULL; > - if (parent) { > - if (repo_parse_commit(repo, parent)) { > - ret = error(_("unable to parse parent commit %s"), > - oid_to_hex(&parent->object.oid)); > - goto out; > - } > - > - parent_tree_oid = repo_get_commit_tree(repo, parent)->object.oid; > - } else { > - oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree); > - } > - > /* We retain authorship of the original commit. */ > - original_message = repo_logmsg_reencode(repo, original, NULL, NULL); > + original_message = repo_logmsg_reencode(repo, commit_with_message, NULL, NULL); > ptr = find_commit_header(original_message, "author", &len); > if (ptr) > original_author = xmemdupz(ptr, len); > find_commit_subject(original_message, &original_body); > > - ret = fill_commit_message(repo, &parent_tree_oid, &original_tree_oid, > + ret = fill_commit_message(repo, old_tree, new_tree, > original_body, action, &commit_message); > if (ret < 0) > goto out; > > - original_extra_headers = read_commit_extra_headers(original, exclude_gpgsig); > + original_extra_headers = read_commit_extra_headers(commit_with_message, > + exclude_gpgsig); > > - ret = commit_tree_extended(commit_message.buf, commit_message.len, &original_tree_oid, > - original->parents, &rewritten_commit_oid, original_author, > + ret = commit_tree_extended(commit_message.buf, commit_message.len, new_tree, > + parents, &rewritten_commit_oid, original_author, > NULL, NULL, original_extra_headers); > if (ret < 0) > goto out; > @@ -151,6 +137,33 @@ static int commit_tree_with_edited_message(struct repository *repo, > return ret; > } > > +static int commit_tree_with_edited_message(struct repository *repo, > + const char *action, > + struct commit *original, > + struct commit **out) > +{ > + struct object_id parent_tree_oid; > + const struct object_id *tree_oid; > + struct commit *parent; > + > + tree_oid = &repo_get_commit_tree(repo, original)->object.oid; > + > + parent = original->parents ? original->parents->item : NULL; > + if (parent) { > + if (repo_parse_commit(repo, parent)) { > + return error(_("unable to parse parent commit %s"), > + oid_to_hex(&parent->object.oid)); > + } > + > + parent_tree_oid = repo_get_commit_tree(repo, parent)->object.oid; > + } else { > + oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree); > + } > + > + return commit_tree_with_edited_message_ext(repo, action, original, original->parents, > + &parent_tree_oid, tree_oid, out); > +} > + > enum ref_action { > REF_ACTION_DEFAULT, > REF_ACTION_BRANCHES, > > -- > 2.53.0.697.g625c4fb2da.dirty > > -- D. Ben Knoble ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 7/8] builtin/history: split out extended function to create commits 2026-03-03 18:43 ` D. Ben Knoble @ 2026-03-11 9:26 ` Patrick Steinhardt 0 siblings, 0 replies; 16+ messages in thread From: Patrick Steinhardt @ 2026-03-11 9:26 UTC (permalink / raw) To: D. Ben Knoble; +Cc: git, Elijah Newren On Tue, Mar 03, 2026 at 01:43:12PM -0500, D. Ben Knoble wrote: > On Mon, Mar 2, 2026 at 7:17 AM Patrick Steinhardt <ps@pks.im> wrote: > > > > In the next commit we're about to introduce a new command that splits up > > a commit into two. Most of the logic will be shared with rewording > > commits, except that we also need to have control over the parents and > > the old/new trees. > > > > Extract a new function `commit_tree_with_edited_message_ext()` to > > prepare for this commit. > > Curious—what's the "ext" suffix mean here. Extracted? External? (Maybe > I'll get a better clue in the next patch.) It stands for "extended". I thought that this was already common use in our code base: - `refs_for_each_ref_ext()` - `odb_write_object_ext()` - `peel_object_ext()` But Junio recently asked the same, so maybe I'm biased here (I am, two of these functions are my doing). Happy to take an alternative suffix. Patrick ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 8/8] builtin/history: implement "split" subcommand 2026-03-02 12:13 [PATCH 0/8] history: introduce "split" subcommand Patrick Steinhardt ` (6 preceding siblings ...) 2026-03-02 12:13 ` [PATCH 7/8] builtin/history: split out extended function to create commits Patrick Steinhardt @ 2026-03-02 12:13 ` Patrick Steinhardt 2026-03-03 18:47 ` D. Ben Knoble 2026-03-03 18:41 ` [PATCH 0/8] history: introduce " D. Ben Knoble 2026-03-13 22:35 ` Junio C Hamano 9 siblings, 1 reply; 16+ messages in thread From: Patrick Steinhardt @ 2026-03-02 12:13 UTC (permalink / raw) To: git; +Cc: Elijah Newren It is quite a common use case that one wants to split up one commit into multiple commits by moving parts of the changes of the original commit out into a separate commit. This is quite an involved operation though: 1. Identify the commit in question that is to be dropped. 2. Perform an interactive rebase on top of that commit's parent. 3. Modify the instruction sheet to "edit" the commit that is to be split up. 4. Drop the commit via "git reset HEAD~". 5. Stage changes that should go into the first commit and commit it. 6. Stage changes that should go into the second commit and commit it. 7. Finalize the rebase. This is quite complex, and overall I would claim that most people who are not experts in Git would struggle with this flow. Introduce a new "split" subcommand for git-history(1) to make this way easier. All the user needs to do is to say `git history split $COMMIT`. From hereon, Git asks the user which parts of the commit shall be moved out into a separate commit and, once done, asks the user for the commit message. Git then creates that split-out commit and applies the original commit on top of it. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- Documentation/git-history.adoc | 62 ++++ builtin/history.c | 250 ++++++++++++++ t/meson.build | 1 + t/t3452-history-split.sh | 757 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 1070 insertions(+) diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc index cc019de697..24dc907033 100644 --- a/Documentation/git-history.adoc +++ b/Documentation/git-history.adoc @@ -9,6 +9,7 @@ SYNOPSIS -------- [synopsis] git history reword <commit> [--dry-run] [--update-refs=(branches|head)] +git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...] DESCRIPTION ----------- @@ -57,6 +58,26 @@ The following commands are available to rewrite history in different ways: details of this commit remain unchanged. This command will spawn an editor with the current message of that commit. +`split <commit> [--] [<pathspec>...]`:: + Interactively split up <commit> into two commits by choosing + hunks introduced by it that will be moved into the new split-out + commit. These hunks will then be written into a new commit that + becomes the parent of the previous commit. The original commit + stays intact, except that its parent will be the newly split-out + commit. ++ +The commit messages of the split-up commits will be asked for by launching +the configured editor. Authorship of the commit will be the same as for the +original commit. ++ +If passed, _<pathspec>_ can be used to limit which changes shall be split out +of the original commit. Files not matching any of the pathspecs will remain +part of the original commit. For more details, see the 'pathspec' entry in +linkgit:gitglossary[7]. ++ +It is invalid to select either all or no hunks, as that would lead to +one of the commits becoming empty. + OPTIONS ------- @@ -72,6 +93,47 @@ OPTIONS descendants of the original commit will be rewritten. With `head`, only the current `HEAD` reference will be rewritten. Defaults to `branches`. +EXAMPLES +-------- + +Split a commit +~~~~~~~~~~~~~~ + +---------- +$ git log --stat --oneline +3f81232 (HEAD -> main) original + bar | 1 + + foo | 1 + + 2 files changed, 2 insertions(+) + +$ git history split HEAD +diff --git a/bar b/bar +new file mode 100644 +index 0000000..5716ca5 +--- /dev/null ++++ b/bar +@@ -0,0 +1 @@ ++bar +(1/1) Stage addition [y,n,q,a,d,p,?]? y + +diff --git a/foo b/foo +new file mode 100644 +index 0000000..257cc56 +--- /dev/null ++++ b/foo +@@ -0,0 +1 @@ ++foo +(1/1) Stage addition [y,n,q,a,d,p,?]? n + +$ git log --stat --oneline +7cebe64 (HEAD -> main) original + foo | 1 + + 1 file changed, 1 insertion(+) +d1582f3 split-out commit + bar | 1 + + 1 file changed, 1 insertion(+) +---------- + GIT --- Part of the linkgit:git[1] suite diff --git a/builtin/history.c b/builtin/history.c index 80726ce14b..dfb9d3f180 100644 --- a/builtin/history.c +++ b/builtin/history.c @@ -1,6 +1,7 @@ #define USE_THE_REPOSITORY_VARIABLE #include "builtin.h" +#include "cache-tree.h" #include "commit.h" #include "commit-reach.h" #include "config.h" @@ -8,17 +9,24 @@ #include "environment.h" #include "gettext.h" #include "hex.h" +#include "lockfile.h" +#include "oidmap.h" #include "parse-options.h" +#include "path.h" +#include "read-cache.h" #include "refs.h" #include "replay.h" #include "revision.h" #include "sequencer.h" #include "strvec.h" #include "tree.h" +#include "unpack-trees.h" #include "wt-status.h" #define GIT_HISTORY_REWORD_USAGE \ N_("git history reword <commit> [--dry-run] [--update-refs=(branches|head)]") +#define GIT_HISTORY_SPLIT_USAGE \ + N_("git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]") static void change_data_free(void *util, const char *str UNUSED) { @@ -484,6 +492,246 @@ static int cmd_history_reword(int argc, return ret; } +static int write_ondisk_index(struct repository *repo, + struct object_id *oid, + const char *path) +{ + struct unpack_trees_options opts = { 0 }; + struct lock_file lock = LOCK_INIT; + struct tree_desc tree_desc; + struct index_state index; + struct tree *tree; + int ret; + + index_state_init(&index, repo); + + opts.head_idx = -1; + opts.src_index = &index; + opts.dst_index = &index; + + tree = repo_parse_tree_indirect(repo, oid); + init_tree_desc(&tree_desc, &tree->object.oid, tree->buffer, tree->size); + + if (unpack_trees(1, &tree_desc, &opts)) { + ret = error(_("unable to populate index with tree")); + goto out; + } + + prime_cache_tree(repo, &index, tree); + + if (hold_lock_file_for_update(&lock, path, 0) < 0) { + ret = error_errno(_("unable to acquire index lock")); + goto out; + } + + if (write_locked_index(&index, &lock, COMMIT_LOCK)) { + ret = error(_("unable to write new index file")); + goto out; + } + + ret = 0; + +out: + rollback_lock_file(&lock); + release_index(&index); + return ret; +} + +static int split_commit(struct repository *repo, + struct commit *original, + struct pathspec *pathspec, + struct commit **out) +{ + struct interactive_options interactive_opts = INTERACTIVE_OPTIONS_INIT; + struct strbuf index_file = STRBUF_INIT; + struct index_state index = INDEX_STATE_INIT(repo); + const struct object_id *original_commit_tree_oid; + const struct object_id *old_tree_oid, *new_tree_oid; + struct object_id parent_tree_oid; + char original_commit_oid[GIT_MAX_HEXSZ + 1]; + struct commit *first_commit, *second_commit; + struct commit_list *parents = NULL; + struct tree *split_tree; + int ret; + + if (original->parents) { + if (repo_parse_commit(repo, original->parents->item)) { + ret = error(_("unable to parse parent commit %s"), + oid_to_hex(&original->parents->item->object.oid)); + goto out; + } + + parent_tree_oid = *get_commit_tree_oid(original->parents->item); + } else { + oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree); + } + original_commit_tree_oid = get_commit_tree_oid(original); + + /* + * Construct the first commit. This is done by taking the original + * commit parent's tree and selectively patching changes from the diff + * between that parent and its child. + */ + repo_git_path_replace(repo, &index_file, "%s", "history-split.index"); + + ret = write_ondisk_index(repo, &parent_tree_oid, index_file.buf); + if (ret < 0) + goto out; + + ret = read_index_from(&index, index_file.buf, repo->gitdir); + if (ret < 0) { + ret = error(_("failed reading temporary index")); + goto out; + } + + oid_to_hex_r(original_commit_oid, &original->object.oid); + ret = run_add_p_index(repo, &index, index_file.buf, &interactive_opts, + original_commit_oid, pathspec, ADD_P_DISALLOW_EDIT); + if (ret < 0) + goto out; + + split_tree = write_in_core_index_as_tree(repo, &index); + if (!split_tree) { + ret = error(_("failed split tree")); + goto out; + } + + unlink(index_file.buf); + strbuf_release(&index_file); + + /* + * We disallow the cases where either the split-out commit or the + * original commit would become empty. Consequently, if we see that the + * new tree ID matches either of those trees we abort. + */ + if (oideq(&split_tree->object.oid, &parent_tree_oid)) { + ret = error(_("split commit is empty")); + goto out; + } else if (oideq(&split_tree->object.oid, original_commit_tree_oid)) { + ret = error(_("split commit tree matches original commit")); + goto out; + } + + /* + * The first commit is constructed from the split-out tree. The base + * that shall be diffed against is the parent of the original commit. + */ + ret = commit_tree_with_edited_message_ext(repo, "split-out", original, + original->parents, &parent_tree_oid, + &split_tree->object.oid, &first_commit); + if (ret < 0) { + ret = error(_("failed writing first commit")); + goto out; + } + + /* + * The second commit is constructed from the original tree. The base to + * diff against and the parent in this case is the first split-out + * commit. + */ + commit_list_append(first_commit, &parents); + + old_tree_oid = &repo_get_commit_tree(repo, first_commit)->object.oid; + new_tree_oid = &repo_get_commit_tree(repo, original)->object.oid; + + ret = commit_tree_with_edited_message_ext(repo, "split-out", original, + parents, old_tree_oid, + new_tree_oid, &second_commit); + if (ret < 0) { + ret = error(_("failed writing second commit")); + goto out; + } + + *out = second_commit; + ret = 0; + +out: + if (index_file.len) + unlink(index_file.buf); + strbuf_release(&index_file); + free_commit_list(parents); + release_index(&index); + return ret; +} + +static int cmd_history_split(int argc, + const char **argv, + const char *prefix, + struct repository *repo) +{ + const char * const usage[] = { + GIT_HISTORY_SPLIT_USAGE, + NULL, + }; + enum ref_action action = REF_ACTION_DEFAULT; + int dry_run = 0; + struct option options[] = { + OPT_CALLBACK_F(0, "update-refs", &action, N_("<refs>"), + N_("control ref update behavior (branches|head|print)"), + PARSE_OPT_NONEG, parse_ref_action), + OPT_BOOL('n', "dry-run", &dry_run, + N_("perform a dry-run without updating any refs")), + OPT_END(), + }; + struct commit *original, *rewritten = NULL; + struct strbuf reflog_msg = STRBUF_INIT; + struct pathspec pathspec = { 0 }; + struct rev_info revs = { 0 }; + int ret; + + argc = parse_options(argc, argv, prefix, options, usage, 0); + if (argc < 1) { + ret = error(_("command expects a committish")); + goto out; + } + repo_config(repo, git_default_config, NULL); + + if (action == REF_ACTION_DEFAULT) + action = REF_ACTION_BRANCHES; + + parse_pathspec(&pathspec, 0, + PATHSPEC_PREFER_FULL | + PATHSPEC_SYMLINK_LEADING_PATH | + PATHSPEC_PREFIX_ORIGIN, + prefix, argv + 1); + + original = lookup_commit_reference_by_name(argv[0]); + if (!original) { + ret = error(_("commit cannot be found: %s"), argv[0]); + goto out; + } + + ret = setup_revwalk(repo, action, original, &revs); + if (ret < 0) + goto out; + + if (original->parents && original->parents->next) { + ret = error(_("cannot split up merge commit")); + goto out; + } + + ret = split_commit(repo, original, &pathspec, &rewritten); + if (ret < 0) + goto out; + + strbuf_addf(&reflog_msg, "split: updating %s", argv[0]); + + ret = handle_reference_updates(&revs, action, original, rewritten, + reflog_msg.buf, dry_run); + if (ret < 0) { + ret = error(_("failed replaying descendants")); + goto out; + } + + ret = 0; + +out: + strbuf_release(&reflog_msg); + clear_pathspec(&pathspec); + release_revisions(&revs); + return ret; +} + int cmd_history(int argc, const char **argv, const char *prefix, @@ -491,11 +739,13 @@ int cmd_history(int argc, { const char * const usage[] = { GIT_HISTORY_REWORD_USAGE, + GIT_HISTORY_SPLIT_USAGE, NULL, }; parse_opt_subcommand_fn *fn = NULL; struct option options[] = { OPT_SUBCOMMAND("reword", &fn, cmd_history_reword), + OPT_SUBCOMMAND("split", &fn, cmd_history_split), OPT_END(), }; diff --git a/t/meson.build b/t/meson.build index 6d91470ebc..2d578ef58b 100644 --- a/t/meson.build +++ b/t/meson.build @@ -392,6 +392,7 @@ integration_tests = [ 't3438-rebase-broken-files.sh', 't3450-history.sh', 't3451-history-reword.sh', + 't3452-history-split.sh', 't3500-cherry.sh', 't3501-revert-cherry-pick.sh', 't3502-cherry-pick-merge.sh', diff --git a/t/t3452-history-split.sh b/t/t3452-history-split.sh new file mode 100755 index 0000000000..8ed0cebb50 --- /dev/null +++ b/t/t3452-history-split.sh @@ -0,0 +1,757 @@ +#!/bin/sh + +test_description='tests for git-history split subcommand' + +. ./test-lib.sh +. "$TEST_DIRECTORY/lib-log-graph.sh" + +# The fake editor takes multiple arguments, each of which represents a commit +# message. Subsequent invocations of the editor will then yield those messages +# in order. +# +set_fake_editor () { + printf "%s\n" "$@" >fake-input && + write_script fake-editor.sh <<-\EOF && + head -n1 fake-input >"$1" + sed 1d fake-input >fake-input.trimmed && + mv fake-input.trimmed fake-input + EOF + test_set_editor "$(pwd)"/fake-editor.sh +} + +expect_graph () { + cat >expect && + lib_test_cmp_graph --graph --format=%s "$@" +} + +expect_log () { + git log --format="%s" >actual && + cat >expect && + test_cmp expect actual +} + +expect_tree_entries () { + git ls-tree --name-only "$1" >actual && + cat >expect && + test_cmp expect actual +} + +test_expect_success 'refuses to work with merge commits' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit base && + git branch branch && + test_commit ours && + git switch branch && + test_commit theirs && + git switch - && + git merge theirs && + test_must_fail git history split HEAD 2>err && + test_grep "cannot split up merge commit" err && + test_must_fail git history split HEAD~ 2>err && + test_grep "replaying merge commits is not supported yet" err + ) +' + +test_expect_success 'errors on missing commit argument' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit initial && + test_must_fail git history split 2>err && + test_grep "command expects a committish" err + ) +' + +test_expect_success 'errors on unknown revision' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit initial && + test_must_fail git history split does-not-exist 2>err && + test_grep "commit cannot be found" err + ) +' + +test_expect_success '--dry-run does not modify any refs' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit base && + touch bar foo && + git add . && + git commit -m split-me && + + git refs list --include-root-refs >before && + + set_fake_editor "first" "second" && + git history split --dry-run HEAD <<-EOF && + y + n + EOF + + git refs list --include-root-refs >after && + test_cmp before after + ) +' + +test_expect_success 'can split up tip commit' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit initial && + touch bar foo && + git add . && + git commit -m split-me && + + git symbolic-ref HEAD >expect && + set_fake_editor "first" "second" && + git history split HEAD <<-EOF && + y + n + EOF + git symbolic-ref HEAD >actual && + test_cmp expect actual && + + expect_log <<-EOF && + second + first + initial + EOF + + expect_tree_entries HEAD~ <<-EOF && + bar + initial.t + EOF + + expect_tree_entries HEAD <<-EOF && + bar + foo + initial.t + EOF + + git reflog >reflog && + test_grep "split: updating HEAD" reflog + ) +' + +test_expect_success 'can split up root commit' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + touch bar foo && + git add . && + git commit -m root && + test_commit tip && + + set_fake_editor "first" "second" && + git history split HEAD~ <<-EOF && + y + n + EOF + + expect_log <<-EOF && + tip + second + first + EOF + + expect_tree_entries HEAD~2 <<-EOF && + bar + EOF + + expect_tree_entries HEAD~ <<-EOF && + bar + foo + EOF + + expect_tree_entries HEAD <<-EOF + bar + foo + tip.t + EOF + ) +' + +test_expect_success 'can split up in-between commit' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit initial && + touch bar foo && + git add . && + git commit -m split-me && + test_commit tip && + + set_fake_editor "first" "second" && + git history split HEAD~ <<-EOF && + y + n + EOF + + expect_log <<-EOF && + tip + second + first + initial + EOF + + expect_tree_entries HEAD~2 <<-EOF && + bar + initial.t + EOF + + expect_tree_entries HEAD~ <<-EOF && + bar + foo + initial.t + EOF + + expect_tree_entries HEAD <<-EOF + bar + foo + initial.t + tip.t + EOF + ) +' + +test_expect_success 'can split HEAD only' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit base && + touch a b && + git add . && + git commit -m split-me && + git branch unrelated && + + set_fake_editor "ours-a" "ours-b" && + git history split --update-refs=head HEAD <<-EOF && + y + n + EOF + expect_graph --branches <<-EOF + * ours-b + * ours-a + | * split-me + |/ + * base + EOF + ) +' + +test_expect_success 'can split detached HEAD' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit initial && + touch bar foo && + git add . && + git commit -m split-me && + git checkout --detach HEAD && + + set_fake_editor "first" "second" && + git history split --update-refs=head HEAD <<-EOF && + y + n + EOF + + # HEAD should be detached and updated. + test_must_fail git symbolic-ref HEAD && + + expect_log <<-EOF + second + first + initial + EOF + ) +' + +test_expect_success 'can split commit in unrelated branch' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit base && + git branch ours && + git switch --create theirs && + touch theirs-a theirs-b && + git add . && + git commit -m theirs && + git switch ours && + test_commit ours && + + # With --update-refs=head it is not possible to split up a + # commit that is unrelated to HEAD. + test_must_fail git history split --update-refs=head theirs 2>err && + test_grep "rewritten commit must be an ancestor of HEAD" err && + + set_fake_editor "theirs-rewritten-a" "theirs-rewritten-b" && + git history split theirs <<-EOF && + y + n + EOF + expect_graph --branches <<-EOF && + * ours + | * theirs-rewritten-b + | * theirs-rewritten-a + |/ + * base + EOF + + expect_tree_entries theirs~ <<-EOF && + base.t + theirs-a + EOF + + expect_tree_entries theirs <<-EOF + base.t + theirs-a + theirs-b + EOF + ) +' + +test_expect_success 'updates multiple descendant branches' ' + test_when_finished "rm -rf repo" && + git init repo --initial-branch=main && + ( + cd repo && + test_commit base && + touch file-a file-b && + git add . && + git commit -m split-me && + git branch branch && + test_commit on-main && + git switch branch && + test_commit on-branch && + git switch main && + + set_fake_editor "split-a" "split-b" && + git history split HEAD~ <<-EOF && + y + n + EOF + + # Both branches should now descend from the split commits. + expect_graph --branches <<-EOF + * on-branch + | * on-main + |/ + * split-b + * split-a + * base + EOF + ) +' + +test_expect_success 'can pick multiple hunks' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + touch bar baz foo qux && + git add . && + git commit -m split-me && + + set_fake_editor "first" "second" && + git history split HEAD <<-EOF && + y + n + y + n + EOF + + expect_tree_entries HEAD~ <<-EOF && + bar + foo + EOF + + expect_tree_entries HEAD <<-EOF + bar + baz + foo + qux + EOF + ) +' + +test_expect_success 'can use only last hunk' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + touch bar foo && + git add . && + git commit -m split-me && + + set_fake_editor "first" "second" && + git history split HEAD <<-EOF && + n + y + EOF + + expect_log <<-EOF && + second + first + EOF + + expect_tree_entries HEAD~ <<-EOF && + foo + EOF + + expect_tree_entries HEAD <<-EOF + bar + foo + EOF + ) +' + +test_expect_success 'can split commit with file deletions' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + echo a >a && + echo b >b && + echo c >c && + git add . && + git commit -m base && + git rm a b && + git commit -m delete-both && + + set_fake_editor "delete-a" "delete-b" && + git history split HEAD <<-EOF && + y + n + EOF + + expect_log <<-EOF && + delete-b + delete-a + base + EOF + + expect_tree_entries HEAD~ <<-EOF && + b + c + EOF + + expect_tree_entries HEAD <<-EOF + c + EOF + ) +' + +test_expect_success 'preserves original authorship' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit initial && + touch bar foo && + git add . && + GIT_AUTHOR_NAME="Other Author" \ + GIT_AUTHOR_EMAIL="other@example.com" \ + git commit -m split-me && + + set_fake_editor "first" "second" && + git history split HEAD <<-EOF && + y + n + EOF + + git log -1 --format="%an <%ae>" HEAD~ >actual && + echo "Other Author <other@example.com>" >expect && + test_cmp expect actual && + + git log -1 --format="%an <%ae>" HEAD >actual && + test_cmp expect actual + ) +' + +test_expect_success 'aborts with empty commit message' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + touch bar foo && + git add . && + git commit -m split-me && + + set_fake_editor "" && + test_must_fail git history split HEAD <<-EOF 2>err && + y + n + EOF + test_grep "Aborting commit due to empty commit message." err + ) +' + +test_expect_success 'commit message editor sees split-out changes' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + touch bar foo && + git add . && + git commit -m split-me && + + write_script fake-editor.sh <<-\EOF && + cat "$1" >>MESSAGES && + echo "some commit message" >"$1" + EOF + test_set_editor "$(pwd)"/fake-editor.sh && + + git history split HEAD <<-EOF && + y + n + EOF + + # Note that we expect to see the messages twice, once for each + # of the commits. The committed files are different though. + cat >expect <<-EOF && + split-me + + # Please enter the commit message for the split-out changes. Lines starting + # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit. + # Changes to be committed: + # new file: bar + # + split-me + + # Please enter the commit message for the split-out changes. Lines starting + # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit. + # Changes to be committed: + # new file: foo + # + EOF + test_cmp expect MESSAGES && + + expect_log <<-EOF + some commit message + some commit message + EOF + ) +' + +test_expect_success 'can use pathspec to limit what gets split' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + touch bar foo && + git add . && + git commit -m split-me && + + set_fake_editor "first" "second" && + git history split HEAD -- foo <<-EOF && + y + EOF + + expect_tree_entries HEAD~ <<-EOF && + foo + EOF + + expect_tree_entries HEAD <<-EOF + bar + foo + EOF + ) +' + +test_expect_success 'pathspec matching no files produces empty split error' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit initial && + touch bar foo && + git add . && + git commit -m split-me && + + set_fake_editor "first" "second" && + test_must_fail git history split HEAD -- nonexistent 2>err && + test_grep "split commit is empty" err + ) +' + +test_expect_success 'split with multiple pathspecs' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit initial && + touch a b c d && + git add . && + git commit -m split-me && + + # Only a and c should be offered for splitting. + set_fake_editor "split-ac" "remainder" && + git history split HEAD -- a c <<-EOF && + y + y + EOF + + expect_tree_entries HEAD~ <<-EOF && + a + c + initial.t + EOF + + expect_tree_entries HEAD <<-EOF + a + b + c + d + initial.t + EOF + ) +' + +test_expect_success 'split with file mode change' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + echo content >script && + git add . && + git commit -m base && + test_chmod +x script && + echo change >script && + git commit -a -m "mode and content change" && + + set_fake_editor "mode-change" "content-change" && + git history split HEAD <<-EOF && + y + n + EOF + + expect_log <<-EOF + content-change + mode-change + base + EOF + ) +' + +test_expect_success 'refuses to create empty split-out commit' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit base && + touch bar foo && + git add . && + git commit -m split-me && + + test_must_fail git history split HEAD 2>err <<-EOF && + n + n + EOF + test_grep "split commit is empty" err + ) +' + +test_expect_success 'hooks are not executed for rewritten commits' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + touch bar foo && + git add . && + git commit -m split-me && + old_head=$(git rev-parse HEAD) && + + ORIG_PATH="$(pwd)" && + export ORIG_PATH && + for hook in prepare-commit-msg pre-commit post-commit post-rewrite commit-msg + do + write_script .git/hooks/$hook <<-\EOF || exit 1 + touch "$ORIG_PATH"/hooks.log + EOF + done && + + set_fake_editor "first" "second" && + git history split HEAD <<-EOF && + y + n + EOF + + expect_log <<-EOF && + second + first + EOF + + test_path_is_missing hooks.log + ) +' + +test_expect_success 'refuses to create empty original commit' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + touch bar foo && + git add . && + git commit -m split-me && + + test_must_fail git history split HEAD 2>err <<-EOF && + y + y + EOF + test_grep "split commit tree matches original commit" err + ) +' + +test_expect_success 'retains changes in the worktree and index' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + echo a >a && + echo b >b && + git add . && + git commit -m "initial commit" && + echo a-modified >a && + echo b-modified >b && + git add b && + set_fake_editor "a-only" "remainder" && + git history split HEAD <<-EOF && + y + n + EOF + + expect_tree_entries HEAD~ <<-EOF && + a + EOF + expect_tree_entries HEAD <<-EOF && + a + b + EOF + + cat >expect <<-\EOF && + M a + M b + ?? actual + ?? expect + ?? fake-editor.sh + ?? fake-input + EOF + git status --porcelain >actual && + test_cmp expect actual + ) +' + +test_done -- 2.53.0.697.g625c4fb2da.dirty ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 8/8] builtin/history: implement "split" subcommand 2026-03-02 12:13 ` [PATCH 8/8] builtin/history: implement "split" subcommand Patrick Steinhardt @ 2026-03-03 18:47 ` D. Ben Knoble 2026-03-11 9:26 ` Patrick Steinhardt 0 siblings, 1 reply; 16+ messages in thread From: D. Ben Knoble @ 2026-03-03 18:47 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git, Elijah Newren On Mon, Mar 2, 2026 at 7:13 AM Patrick Steinhardt <ps@pks.im> wrote: > > It is quite a common use case that one wants to split up one commit into > multiple commits by moving parts of the changes of the original commit > out into a separate commit. This is quite an involved operation though: > > 1. Identify the commit in question that is to be dropped. > > 2. Perform an interactive rebase on top of that commit's parent. > > 3. Modify the instruction sheet to "edit" the commit that is to be > split up. > > 4. Drop the commit via "git reset HEAD~". > > 5. Stage changes that should go into the first commit and commit it. > > 6. Stage changes that should go into the second commit and commit it. > > 7. Finalize the rebase. > > This is quite complex, and overall I would claim that most people who > are not experts in Git would struggle with this flow. > > Introduce a new "split" subcommand for git-history(1) to make this way > easier. All the user needs to do is to say `git history split $COMMIT`. > From hereon, Git asks the user which parts of the commit shall be moved > out into a separate commit and, once done, asks the user for the commit > message. Git then creates that split-out commit and applies the original > commit on top of it. > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > Documentation/git-history.adoc | 62 ++++ > builtin/history.c | 250 ++++++++++++++ > t/meson.build | 1 + > t/t3452-history-split.sh | 757 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 1070 insertions(+) > > diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc > index cc019de697..24dc907033 100644 > --- a/Documentation/git-history.adoc > +++ b/Documentation/git-history.adoc > @@ -9,6 +9,7 @@ SYNOPSIS > -------- > [synopsis] > git history reword <commit> [--dry-run] [--update-refs=(branches|head)] > +git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...] > > DESCRIPTION > ----------- > @@ -57,6 +58,26 @@ The following commands are available to rewrite history in different ways: > details of this commit remain unchanged. This command will spawn an > editor with the current message of that commit. > > +`split <commit> [--] [<pathspec>...]`:: > + Interactively split up <commit> into two commits by choosing > + hunks introduced by it that will be moved into the new split-out > + commit. These hunks will then be written into a new commit that > + becomes the parent of the previous commit. The original commit > + stays intact, except that its parent will be the newly split-out > + commit. > ++ > +The commit messages of the split-up commits will be asked for by launching > +the configured editor. Authorship of the commit will be the same as for the > +original commit. > ++ > +If passed, _<pathspec>_ can be used to limit which changes shall be split out > +of the original commit. Files not matching any of the pathspecs will remain > +part of the original commit. For more details, see the 'pathspec' entry in > +linkgit:gitglossary[7]. That is quite convenient when changes to 2 independent areas have become mixed. Nice. > +It is invalid to select either all or no hunks, as that would lead to > +one of the commits becoming empty. Is it easy to make this a no-op? It could be done later if that suggestion is contentions. But I figure rather than error we can silently do nothing, since we have performed the desired split. (Or even use this to split an "--allow-empty" commit, but… why that's desirable, I can't guess.) So yeah, probably for later. > + > OPTIONS > ------- > > @@ -72,6 +93,47 @@ OPTIONS > descendants of the original commit will be rewritten. With `head`, only > the current `HEAD` reference will be rewritten. Defaults to `branches`. > > +EXAMPLES > +-------- > + > +Split a commit > +~~~~~~~~~~~~~~ > + > +---------- > +$ git log --stat --oneline > +3f81232 (HEAD -> main) original > + bar | 1 + > + foo | 1 + > + 2 files changed, 2 insertions(+) > + > +$ git history split HEAD > +diff --git a/bar b/bar > +new file mode 100644 > +index 0000000..5716ca5 > +--- /dev/null > ++++ b/bar > +@@ -0,0 +1 @@ > ++bar > +(1/1) Stage addition [y,n,q,a,d,p,?]? y > + > +diff --git a/foo b/foo > +new file mode 100644 > +index 0000000..257cc56 > +--- /dev/null > ++++ b/foo > +@@ -0,0 +1 @@ > ++foo > +(1/1) Stage addition [y,n,q,a,d,p,?]? n > + > +$ git log --stat --oneline > +7cebe64 (HEAD -> main) original > + foo | 1 + > + 1 file changed, 1 insertion(+) > +d1582f3 split-out commit > + bar | 1 + > + 1 file changed, 1 insertion(+) > +---------- > + > GIT > --- > Part of the linkgit:git[1] suite > diff --git a/builtin/history.c b/builtin/history.c > index 80726ce14b..dfb9d3f180 100644 > --- a/builtin/history.c > +++ b/builtin/history.c > @@ -1,6 +1,7 @@ > #define USE_THE_REPOSITORY_VARIABLE > > #include "builtin.h" > +#include "cache-tree.h" > #include "commit.h" > #include "commit-reach.h" > #include "config.h" > @@ -8,17 +9,24 @@ > #include "environment.h" > #include "gettext.h" > #include "hex.h" > +#include "lockfile.h" > +#include "oidmap.h" > #include "parse-options.h" > +#include "path.h" > +#include "read-cache.h" > #include "refs.h" > #include "replay.h" > #include "revision.h" > #include "sequencer.h" > #include "strvec.h" > #include "tree.h" > +#include "unpack-trees.h" > #include "wt-status.h" > > #define GIT_HISTORY_REWORD_USAGE \ > N_("git history reword <commit> [--dry-run] [--update-refs=(branches|head)]") > +#define GIT_HISTORY_SPLIT_USAGE \ > + N_("git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]") > > static void change_data_free(void *util, const char *str UNUSED) > { > @@ -484,6 +492,246 @@ static int cmd_history_reword(int argc, > return ret; > } > > +static int write_ondisk_index(struct repository *repo, > + struct object_id *oid, > + const char *path) > +{ > + struct unpack_trees_options opts = { 0 }; > + struct lock_file lock = LOCK_INIT; > + struct tree_desc tree_desc; > + struct index_state index; > + struct tree *tree; > + int ret; > + > + index_state_init(&index, repo); > + > + opts.head_idx = -1; > + opts.src_index = &index; > + opts.dst_index = &index; > + > + tree = repo_parse_tree_indirect(repo, oid); > + init_tree_desc(&tree_desc, &tree->object.oid, tree->buffer, tree->size); > + > + if (unpack_trees(1, &tree_desc, &opts)) { > + ret = error(_("unable to populate index with tree")); > + goto out; > + } > + > + prime_cache_tree(repo, &index, tree); > + > + if (hold_lock_file_for_update(&lock, path, 0) < 0) { > + ret = error_errno(_("unable to acquire index lock")); > + goto out; > + } > + > + if (write_locked_index(&index, &lock, COMMIT_LOCK)) { > + ret = error(_("unable to write new index file")); > + goto out; > + } > + > + ret = 0; > + > +out: > + rollback_lock_file(&lock); > + release_index(&index); > + return ret; > +} > + > +static int split_commit(struct repository *repo, > + struct commit *original, > + struct pathspec *pathspec, > + struct commit **out) > +{ > + struct interactive_options interactive_opts = INTERACTIVE_OPTIONS_INIT; > + struct strbuf index_file = STRBUF_INIT; > + struct index_state index = INDEX_STATE_INIT(repo); > + const struct object_id *original_commit_tree_oid; > + const struct object_id *old_tree_oid, *new_tree_oid; > + struct object_id parent_tree_oid; > + char original_commit_oid[GIT_MAX_HEXSZ + 1]; > + struct commit *first_commit, *second_commit; > + struct commit_list *parents = NULL; > + struct tree *split_tree; > + int ret; > + > + if (original->parents) { > + if (repo_parse_commit(repo, original->parents->item)) { > + ret = error(_("unable to parse parent commit %s"), > + oid_to_hex(&original->parents->item->object.oid)); > + goto out; > + } > + > + parent_tree_oid = *get_commit_tree_oid(original->parents->item); > + } else { > + oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree); > + } > + original_commit_tree_oid = get_commit_tree_oid(original); > + > + /* > + * Construct the first commit. This is done by taking the original > + * commit parent's tree and selectively patching changes from the diff > + * between that parent and its child. > + */ > + repo_git_path_replace(repo, &index_file, "%s", "history-split.index"); > + > + ret = write_ondisk_index(repo, &parent_tree_oid, index_file.buf); > + if (ret < 0) > + goto out; > + > + ret = read_index_from(&index, index_file.buf, repo->gitdir); > + if (ret < 0) { > + ret = error(_("failed reading temporary index")); > + goto out; > + } > + > + oid_to_hex_r(original_commit_oid, &original->object.oid); > + ret = run_add_p_index(repo, &index, index_file.buf, &interactive_opts, > + original_commit_oid, pathspec, ADD_P_DISALLOW_EDIT); > + if (ret < 0) > + goto out; > + > + split_tree = write_in_core_index_as_tree(repo, &index); > + if (!split_tree) { > + ret = error(_("failed split tree")); > + goto out; > + } > + > + unlink(index_file.buf); > + strbuf_release(&index_file); > + > + /* > + * We disallow the cases where either the split-out commit or the > + * original commit would become empty. Consequently, if we see that the > + * new tree ID matches either of those trees we abort. > + */ > + if (oideq(&split_tree->object.oid, &parent_tree_oid)) { > + ret = error(_("split commit is empty")); > + goto out; > + } else if (oideq(&split_tree->object.oid, original_commit_tree_oid)) { > + ret = error(_("split commit tree matches original commit")); > + goto out; > + } > + > + /* > + * The first commit is constructed from the split-out tree. The base > + * that shall be diffed against is the parent of the original commit. > + */ > + ret = commit_tree_with_edited_message_ext(repo, "split-out", original, > + original->parents, &parent_tree_oid, > + &split_tree->object.oid, &first_commit); > + if (ret < 0) { > + ret = error(_("failed writing first commit")); > + goto out; > + } > + > + /* > + * The second commit is constructed from the original tree. The base to > + * diff against and the parent in this case is the first split-out > + * commit. > + */ > + commit_list_append(first_commit, &parents); > + > + old_tree_oid = &repo_get_commit_tree(repo, first_commit)->object.oid; > + new_tree_oid = &repo_get_commit_tree(repo, original)->object.oid; > + > + ret = commit_tree_with_edited_message_ext(repo, "split-out", original, > + parents, old_tree_oid, > + new_tree_oid, &second_commit); > + if (ret < 0) { > + ret = error(_("failed writing second commit")); > + goto out; > + } > + > + *out = second_commit; > + ret = 0; > + > +out: > + if (index_file.len) > + unlink(index_file.buf); > + strbuf_release(&index_file); > + free_commit_list(parents); > + release_index(&index); > + return ret; > +} > + > +static int cmd_history_split(int argc, > + const char **argv, > + const char *prefix, > + struct repository *repo) > +{ > + const char * const usage[] = { > + GIT_HISTORY_SPLIT_USAGE, > + NULL, > + }; > + enum ref_action action = REF_ACTION_DEFAULT; > + int dry_run = 0; > + struct option options[] = { > + OPT_CALLBACK_F(0, "update-refs", &action, N_("<refs>"), > + N_("control ref update behavior (branches|head|print)"), > + PARSE_OPT_NONEG, parse_ref_action), > + OPT_BOOL('n', "dry-run", &dry_run, > + N_("perform a dry-run without updating any refs")), > + OPT_END(), > + }; > + struct commit *original, *rewritten = NULL; > + struct strbuf reflog_msg = STRBUF_INIT; > + struct pathspec pathspec = { 0 }; > + struct rev_info revs = { 0 }; > + int ret; > + > + argc = parse_options(argc, argv, prefix, options, usage, 0); > + if (argc < 1) { > + ret = error(_("command expects a committish")); > + goto out; > + } > + repo_config(repo, git_default_config, NULL); > + > + if (action == REF_ACTION_DEFAULT) > + action = REF_ACTION_BRANCHES; > + > + parse_pathspec(&pathspec, 0, > + PATHSPEC_PREFER_FULL | > + PATHSPEC_SYMLINK_LEADING_PATH | > + PATHSPEC_PREFIX_ORIGIN, > + prefix, argv + 1); > + > + original = lookup_commit_reference_by_name(argv[0]); > + if (!original) { > + ret = error(_("commit cannot be found: %s"), argv[0]); > + goto out; > + } > + > + ret = setup_revwalk(repo, action, original, &revs); > + if (ret < 0) > + goto out; > + > + if (original->parents && original->parents->next) { > + ret = error(_("cannot split up merge commit")); > + goto out; > + } > + > + ret = split_commit(repo, original, &pathspec, &rewritten); > + if (ret < 0) > + goto out; > + > + strbuf_addf(&reflog_msg, "split: updating %s", argv[0]); > + > + ret = handle_reference_updates(&revs, action, original, rewritten, > + reflog_msg.buf, dry_run); > + if (ret < 0) { > + ret = error(_("failed replaying descendants")); > + goto out; > + } > + > + ret = 0; > + > +out: > + strbuf_release(&reflog_msg); > + clear_pathspec(&pathspec); > + release_revisions(&revs); > + return ret; > +} > + > int cmd_history(int argc, > const char **argv, > const char *prefix, > @@ -491,11 +739,13 @@ int cmd_history(int argc, > { > const char * const usage[] = { > GIT_HISTORY_REWORD_USAGE, > + GIT_HISTORY_SPLIT_USAGE, > NULL, > }; > parse_opt_subcommand_fn *fn = NULL; > struct option options[] = { > OPT_SUBCOMMAND("reword", &fn, cmd_history_reword), > + OPT_SUBCOMMAND("split", &fn, cmd_history_split), > OPT_END(), > }; > > diff --git a/t/meson.build b/t/meson.build > index 6d91470ebc..2d578ef58b 100644 > --- a/t/meson.build > +++ b/t/meson.build > @@ -392,6 +392,7 @@ integration_tests = [ > 't3438-rebase-broken-files.sh', > 't3450-history.sh', > 't3451-history-reword.sh', > + 't3452-history-split.sh', > 't3500-cherry.sh', > 't3501-revert-cherry-pick.sh', > 't3502-cherry-pick-merge.sh', > diff --git a/t/t3452-history-split.sh b/t/t3452-history-split.sh > new file mode 100755 > index 0000000000..8ed0cebb50 > --- /dev/null > +++ b/t/t3452-history-split.sh > @@ -0,0 +1,757 @@ > +#!/bin/sh > + > +test_description='tests for git-history split subcommand' > + > +. ./test-lib.sh > +. "$TEST_DIRECTORY/lib-log-graph.sh" > + > +# The fake editor takes multiple arguments, each of which represents a commit > +# message. Subsequent invocations of the editor will then yield those messages > +# in order. > +# > +set_fake_editor () { > + printf "%s\n" "$@" >fake-input && > + write_script fake-editor.sh <<-\EOF && > + head -n1 fake-input >"$1" > + sed 1d fake-input >fake-input.trimmed && > + mv fake-input.trimmed fake-input > + EOF > + test_set_editor "$(pwd)"/fake-editor.sh > +} > + > +expect_graph () { > + cat >expect && > + lib_test_cmp_graph --graph --format=%s "$@" > +} > + > +expect_log () { > + git log --format="%s" >actual && > + cat >expect && > + test_cmp expect actual > +} > + > +expect_tree_entries () { > + git ls-tree --name-only "$1" >actual && > + cat >expect && > + test_cmp expect actual > +} > + > +test_expect_success 'refuses to work with merge commits' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + test_commit base && > + git branch branch && > + test_commit ours && > + git switch branch && > + test_commit theirs && > + git switch - && > + git merge theirs && > + test_must_fail git history split HEAD 2>err && > + test_grep "cannot split up merge commit" err && > + test_must_fail git history split HEAD~ 2>err && > + test_grep "replaying merge commits is not supported yet" err > + ) > +' > + > +test_expect_success 'errors on missing commit argument' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + test_commit initial && > + test_must_fail git history split 2>err && > + test_grep "command expects a committish" err > + ) > +' > + > +test_expect_success 'errors on unknown revision' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + test_commit initial && > + test_must_fail git history split does-not-exist 2>err && > + test_grep "commit cannot be found" err > + ) > +' > + > +test_expect_success '--dry-run does not modify any refs' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + test_commit base && > + touch bar foo && > + git add . && > + git commit -m split-me && > + > + git refs list --include-root-refs >before && > + > + set_fake_editor "first" "second" && > + git history split --dry-run HEAD <<-EOF && > + y > + n > + EOF > + > + git refs list --include-root-refs >after && > + test_cmp before after > + ) > +' > + > +test_expect_success 'can split up tip commit' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + test_commit initial && > + touch bar foo && > + git add . && > + git commit -m split-me && > + > + git symbolic-ref HEAD >expect && > + set_fake_editor "first" "second" && > + git history split HEAD <<-EOF && > + y > + n > + EOF > + git symbolic-ref HEAD >actual && > + test_cmp expect actual && > + > + expect_log <<-EOF && > + second > + first > + initial > + EOF > + > + expect_tree_entries HEAD~ <<-EOF && > + bar > + initial.t > + EOF > + > + expect_tree_entries HEAD <<-EOF && > + bar > + foo > + initial.t > + EOF > + > + git reflog >reflog && > + test_grep "split: updating HEAD" reflog > + ) > +' > + > +test_expect_success 'can split up root commit' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + touch bar foo && > + git add . && > + git commit -m root && > + test_commit tip && > + > + set_fake_editor "first" "second" && > + git history split HEAD~ <<-EOF && > + y > + n > + EOF > + > + expect_log <<-EOF && > + tip > + second > + first > + EOF > + > + expect_tree_entries HEAD~2 <<-EOF && > + bar > + EOF > + > + expect_tree_entries HEAD~ <<-EOF && > + bar > + foo > + EOF > + > + expect_tree_entries HEAD <<-EOF > + bar > + foo > + tip.t > + EOF > + ) > +' > + > +test_expect_success 'can split up in-between commit' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + test_commit initial && > + touch bar foo && > + git add . && > + git commit -m split-me && > + test_commit tip && > + > + set_fake_editor "first" "second" && > + git history split HEAD~ <<-EOF && > + y > + n > + EOF > + > + expect_log <<-EOF && > + tip > + second > + first > + initial > + EOF > + > + expect_tree_entries HEAD~2 <<-EOF && > + bar > + initial.t > + EOF > + > + expect_tree_entries HEAD~ <<-EOF && > + bar > + foo > + initial.t > + EOF > + > + expect_tree_entries HEAD <<-EOF > + bar > + foo > + initial.t > + tip.t > + EOF > + ) > +' > + > +test_expect_success 'can split HEAD only' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + test_commit base && > + touch a b && > + git add . && > + git commit -m split-me && > + git branch unrelated && > + > + set_fake_editor "ours-a" "ours-b" && > + git history split --update-refs=head HEAD <<-EOF && > + y > + n > + EOF > + expect_graph --branches <<-EOF > + * ours-b > + * ours-a > + | * split-me > + |/ > + * base > + EOF > + ) > +' > + > +test_expect_success 'can split detached HEAD' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + test_commit initial && > + touch bar foo && > + git add . && > + git commit -m split-me && > + git checkout --detach HEAD && > + > + set_fake_editor "first" "second" && > + git history split --update-refs=head HEAD <<-EOF && > + y > + n > + EOF > + > + # HEAD should be detached and updated. > + test_must_fail git symbolic-ref HEAD && > + > + expect_log <<-EOF > + second > + first > + initial > + EOF > + ) > +' > + > +test_expect_success 'can split commit in unrelated branch' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + test_commit base && > + git branch ours && > + git switch --create theirs && > + touch theirs-a theirs-b && > + git add . && > + git commit -m theirs && > + git switch ours && > + test_commit ours && > + > + # With --update-refs=head it is not possible to split up a > + # commit that is unrelated to HEAD. > + test_must_fail git history split --update-refs=head theirs 2>err && > + test_grep "rewritten commit must be an ancestor of HEAD" err && > + > + set_fake_editor "theirs-rewritten-a" "theirs-rewritten-b" && > + git history split theirs <<-EOF && > + y > + n > + EOF > + expect_graph --branches <<-EOF && > + * ours > + | * theirs-rewritten-b > + | * theirs-rewritten-a > + |/ > + * base > + EOF > + > + expect_tree_entries theirs~ <<-EOF && > + base.t > + theirs-a > + EOF > + > + expect_tree_entries theirs <<-EOF > + base.t > + theirs-a > + theirs-b > + EOF > + ) > +' > + > +test_expect_success 'updates multiple descendant branches' ' > + test_when_finished "rm -rf repo" && > + git init repo --initial-branch=main && > + ( > + cd repo && > + test_commit base && > + touch file-a file-b && > + git add . && > + git commit -m split-me && > + git branch branch && > + test_commit on-main && > + git switch branch && > + test_commit on-branch && > + git switch main && > + > + set_fake_editor "split-a" "split-b" && > + git history split HEAD~ <<-EOF && > + y > + n > + EOF > + > + # Both branches should now descend from the split commits. > + expect_graph --branches <<-EOF > + * on-branch > + | * on-main > + |/ > + * split-b > + * split-a > + * base > + EOF > + ) > +' > + > +test_expect_success 'can pick multiple hunks' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + touch bar baz foo qux && > + git add . && > + git commit -m split-me && > + > + set_fake_editor "first" "second" && > + git history split HEAD <<-EOF && > + y > + n > + y > + n > + EOF > + > + expect_tree_entries HEAD~ <<-EOF && > + bar > + foo > + EOF > + > + expect_tree_entries HEAD <<-EOF > + bar > + baz > + foo > + qux > + EOF > + ) > +' > + > +test_expect_success 'can use only last hunk' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + touch bar foo && > + git add . && > + git commit -m split-me && > + > + set_fake_editor "first" "second" && > + git history split HEAD <<-EOF && > + n > + y > + EOF > + > + expect_log <<-EOF && > + second > + first > + EOF > + > + expect_tree_entries HEAD~ <<-EOF && > + foo > + EOF > + > + expect_tree_entries HEAD <<-EOF > + bar > + foo > + EOF > + ) > +' > + > +test_expect_success 'can split commit with file deletions' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + echo a >a && > + echo b >b && > + echo c >c && > + git add . && > + git commit -m base && > + git rm a b && > + git commit -m delete-both && > + > + set_fake_editor "delete-a" "delete-b" && > + git history split HEAD <<-EOF && > + y > + n > + EOF > + > + expect_log <<-EOF && > + delete-b > + delete-a > + base > + EOF > + > + expect_tree_entries HEAD~ <<-EOF && > + b > + c > + EOF > + > + expect_tree_entries HEAD <<-EOF > + c > + EOF > + ) > +' > + > +test_expect_success 'preserves original authorship' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + test_commit initial && > + touch bar foo && > + git add . && > + GIT_AUTHOR_NAME="Other Author" \ > + GIT_AUTHOR_EMAIL="other@example.com" \ > + git commit -m split-me && > + > + set_fake_editor "first" "second" && > + git history split HEAD <<-EOF && > + y > + n > + EOF > + > + git log -1 --format="%an <%ae>" HEAD~ >actual && > + echo "Other Author <other@example.com>" >expect && > + test_cmp expect actual && > + > + git log -1 --format="%an <%ae>" HEAD >actual && > + test_cmp expect actual > + ) > +' > + > +test_expect_success 'aborts with empty commit message' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + touch bar foo && > + git add . && > + git commit -m split-me && > + > + set_fake_editor "" && > + test_must_fail git history split HEAD <<-EOF 2>err && > + y > + n > + EOF > + test_grep "Aborting commit due to empty commit message." err > + ) > +' > + > +test_expect_success 'commit message editor sees split-out changes' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + touch bar foo && > + git add . && > + git commit -m split-me && > + > + write_script fake-editor.sh <<-\EOF && > + cat "$1" >>MESSAGES && > + echo "some commit message" >"$1" > + EOF > + test_set_editor "$(pwd)"/fake-editor.sh && > + > + git history split HEAD <<-EOF && > + y > + n > + EOF > + > + # Note that we expect to see the messages twice, once for each > + # of the commits. The committed files are different though. > + cat >expect <<-EOF && > + split-me > + > + # Please enter the commit message for the split-out changes. Lines starting > + # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit. > + # Changes to be committed: > + # new file: bar > + # > + split-me > + > + # Please enter the commit message for the split-out changes. Lines starting > + # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit. > + # Changes to be committed: > + # new file: foo > + # > + EOF > + test_cmp expect MESSAGES && > + > + expect_log <<-EOF > + some commit message > + some commit message > + EOF > + ) > +' > + > +test_expect_success 'can use pathspec to limit what gets split' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + touch bar foo && > + git add . && > + git commit -m split-me && > + > + set_fake_editor "first" "second" && > + git history split HEAD -- foo <<-EOF && > + y > + EOF > + > + expect_tree_entries HEAD~ <<-EOF && > + foo > + EOF > + > + expect_tree_entries HEAD <<-EOF > + bar > + foo > + EOF > + ) > +' > + > +test_expect_success 'pathspec matching no files produces empty split error' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + test_commit initial && > + touch bar foo && > + git add . && > + git commit -m split-me && > + > + set_fake_editor "first" "second" && > + test_must_fail git history split HEAD -- nonexistent 2>err && > + test_grep "split commit is empty" err > + ) > +' > + > +test_expect_success 'split with multiple pathspecs' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + test_commit initial && > + touch a b c d && > + git add . && > + git commit -m split-me && > + > + # Only a and c should be offered for splitting. > + set_fake_editor "split-ac" "remainder" && > + git history split HEAD -- a c <<-EOF && > + y > + y > + EOF > + > + expect_tree_entries HEAD~ <<-EOF && > + a > + c > + initial.t > + EOF > + > + expect_tree_entries HEAD <<-EOF > + a > + b > + c > + d > + initial.t > + EOF > + ) > +' > + > +test_expect_success 'split with file mode change' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + echo content >script && > + git add . && > + git commit -m base && > + test_chmod +x script && > + echo change >script && > + git commit -a -m "mode and content change" && > + > + set_fake_editor "mode-change" "content-change" && > + git history split HEAD <<-EOF && > + y > + n > + EOF > + > + expect_log <<-EOF > + content-change > + mode-change > + base > + EOF > + ) > +' > + > +test_expect_success 'refuses to create empty split-out commit' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + test_commit base && > + touch bar foo && > + git add . && > + git commit -m split-me && > + > + test_must_fail git history split HEAD 2>err <<-EOF && > + n > + n > + EOF > + test_grep "split commit is empty" err > + ) > +' > + > +test_expect_success 'hooks are not executed for rewritten commits' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + touch bar foo && > + git add . && > + git commit -m split-me && > + old_head=$(git rev-parse HEAD) && > + > + ORIG_PATH="$(pwd)" && > + export ORIG_PATH && > + for hook in prepare-commit-msg pre-commit post-commit post-rewrite commit-msg > + do > + write_script .git/hooks/$hook <<-\EOF || exit 1 > + touch "$ORIG_PATH"/hooks.log > + EOF > + done && > + > + set_fake_editor "first" "second" && > + git history split HEAD <<-EOF && > + y > + n > + EOF > + > + expect_log <<-EOF && > + second > + first > + EOF > + > + test_path_is_missing hooks.log > + ) > +' > + > +test_expect_success 'refuses to create empty original commit' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + touch bar foo && > + git add . && > + git commit -m split-me && > + > + test_must_fail git history split HEAD 2>err <<-EOF && > + y > + y > + EOF > + test_grep "split commit tree matches original commit" err > + ) > +' > + > +test_expect_success 'retains changes in the worktree and index' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + echo a >a && > + echo b >b && > + git add . && > + git commit -m "initial commit" && > + echo a-modified >a && > + echo b-modified >b && > + git add b && > + set_fake_editor "a-only" "remainder" && > + git history split HEAD <<-EOF && > + y > + n > + EOF > + > + expect_tree_entries HEAD~ <<-EOF && > + a > + EOF > + expect_tree_entries HEAD <<-EOF && > + a > + b > + EOF > + > + cat >expect <<-\EOF && > + M a > + M b > + ?? actual > + ?? expect > + ?? fake-editor.sh > + ?? fake-input > + EOF > + git status --porcelain >actual && > + test_cmp expect actual > + ) > +' > + > +test_done > > -- > 2.53.0.697.g625c4fb2da.dirty > > I didn't look particularly closely at the rest. I didn't really figure out what "ext" meant though ;) -- D. Ben Knoble ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 8/8] builtin/history: implement "split" subcommand 2026-03-03 18:47 ` D. Ben Knoble @ 2026-03-11 9:26 ` Patrick Steinhardt 0 siblings, 0 replies; 16+ messages in thread From: Patrick Steinhardt @ 2026-03-11 9:26 UTC (permalink / raw) To: D. Ben Knoble; +Cc: git, Elijah Newren On Tue, Mar 03, 2026 at 01:47:27PM -0500, D. Ben Knoble wrote: > On Mon, Mar 2, 2026 at 7:13 AM Patrick Steinhardt <ps@pks.im> wrote: > > diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc > > index cc019de697..24dc907033 100644 > > --- a/Documentation/git-history.adoc > > +++ b/Documentation/git-history.adoc > > @@ -57,6 +58,26 @@ The following commands are available to rewrite history in different ways: > > details of this commit remain unchanged. This command will spawn an > > editor with the current message of that commit. > > > > +`split <commit> [--] [<pathspec>...]`:: > > + Interactively split up <commit> into two commits by choosing > > + hunks introduced by it that will be moved into the new split-out > > + commit. These hunks will then be written into a new commit that > > + becomes the parent of the previous commit. The original commit > > + stays intact, except that its parent will be the newly split-out > > + commit. > > ++ > > +The commit messages of the split-up commits will be asked for by launching > > +the configured editor. Authorship of the commit will be the same as for the > > +original commit. > > ++ > > +If passed, _<pathspec>_ can be used to limit which changes shall be split out > > +of the original commit. Files not matching any of the pathspecs will remain > > +part of the original commit. For more details, see the 'pathspec' entry in > > +linkgit:gitglossary[7]. > > That is quite convenient when changes to 2 independent areas have > become mixed. Nice. > > > +It is invalid to select either all or no hunks, as that would lead to > > +one of the commits becoming empty. > > Is it easy to make this a no-op? It could be done later if that > suggestion is contentions. But I figure rather than error we can > silently do nothing, since we have performed the desired split. (Or > even use this to split an "--allow-empty" commit, but… why that's > desirable, I can't guess.) > > So yeah, probably for later. I mean we could make it a no-op, but wouldn't that make the interface even more confusing? You don't really split a commit in the case where you select everything or nothing, as you'd only end up with a single commit in that case. Making one of the commits completely empty would probably be an accident in almost all cases, I would claim. But yeah, if there actually are use cases for this I would say that we could then introduce "--allow-empty" for this command at a later point. Patrick ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/8] history: introduce "split" subcommand 2026-03-02 12:13 [PATCH 0/8] history: introduce "split" subcommand Patrick Steinhardt ` (7 preceding siblings ...) 2026-03-02 12:13 ` [PATCH 8/8] builtin/history: implement "split" subcommand Patrick Steinhardt @ 2026-03-03 18:41 ` D. Ben Knoble 2026-03-13 22:35 ` Junio C Hamano 9 siblings, 0 replies; 16+ messages in thread From: D. Ben Knoble @ 2026-03-03 18:41 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git, Elijah Newren On Mon, Mar 2, 2026 at 7:15 AM Patrick Steinhardt <ps@pks.im> wrote: > > Hi, > > this patch series introduces `git history split` as an easy way to split > up one commit into multiple commits. Exciting news, thanks! > This subcommand has already been > introduced in earlier versions of my git-history(1) patch series, but I > eventually decided to evict them from this series so that we can rather > focus more on basic decisions. > > In any case, the current version of this patch series matches (to the > best of my knowledge) the latest agreements on the mailing list around > its behaviour. Most importantly: > > - It will ask for commit messages for both commits, not only the first > commit, which has been a bit of a discussion point. > > - It is not possible to edit hunks. This results in a mode where > conflicts are not possible as the tree of the second commit will > always match the tree of the original commit. Conflict handling for > subsequent subcommands will be a bigger topic, as it probably > depends on support for first-class conflicts. > > - We also update dependent branches, same as with the latest iteration > of `git history reword`. Matches my memory. I think the dependent branches bit should probably depend on the same --update-refs option, so I'll expect to see that (not sure why we'd make any other choice ;). -- D. Ben Knoble ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/8] history: introduce "split" subcommand 2026-03-02 12:13 [PATCH 0/8] history: introduce "split" subcommand Patrick Steinhardt ` (8 preceding siblings ...) 2026-03-03 18:41 ` [PATCH 0/8] history: introduce " D. Ben Knoble @ 2026-03-13 22:35 ` Junio C Hamano 2026-03-16 7:13 ` Patrick Steinhardt 9 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2026-03-13 22:35 UTC (permalink / raw) To: D. Ben Knoble; +Cc: Patrick Steinhardt, git, Elijah Newren Patrick Steinhardt <ps@pks.im> writes: > this patch series introduces `git history split` as an easy way to split > up one commit into multiple commits. This subcommand has already been > introduced in earlier versions of my git-history(1) patch series, but I > eventually decided to evict them from this series so that we can rather > focus more on basic decisions. We saw a few exchanges and comments but didn't see an blocker. Everybody happy with the series? THanks. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/8] history: introduce "split" subcommand 2026-03-13 22:35 ` Junio C Hamano @ 2026-03-16 7:13 ` Patrick Steinhardt 0 siblings, 0 replies; 16+ messages in thread From: Patrick Steinhardt @ 2026-03-16 7:13 UTC (permalink / raw) To: Junio C Hamano; +Cc: D. Ben Knoble, git, Elijah Newren On Fri, Mar 13, 2026 at 03:35:32PM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > this patch series introduces `git history split` as an easy way to split > > up one commit into multiple commits. This subcommand has already been > > introduced in earlier versions of my git-history(1) patch series, but I > > eventually decided to evict them from this series so that we can rather > > focus more on basic decisions. > > We saw a few exchanges and comments but didn't see an blocker. > Everybody happy with the series? There wasn't a ton of discussion on this series, but a lot of it already happened back when it was still part of the initial git-history(1) introduction. So it has already evolved significantly, even if that's not quite visible here. From my perspective this is ready to go. Thanks! Patrick ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-03-16 7:13 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-02 12:13 [PATCH 0/8] history: introduce "split" subcommand Patrick Steinhardt 2026-03-02 12:13 ` [PATCH 1/8] add-patch: split out header from "add-interactive.h" Patrick Steinhardt 2026-03-02 12:13 ` [PATCH 2/8] add-patch: split out `struct interactive_options` Patrick Steinhardt 2026-03-02 12:13 ` [PATCH 3/8] add-patch: remove dependency on "add-interactive" subsystem Patrick Steinhardt 2026-03-02 12:13 ` [PATCH 4/8] add-patch: add support for in-memory index patching Patrick Steinhardt 2026-03-02 12:13 ` [PATCH 5/8] add-patch: allow disabling editing of hunks Patrick Steinhardt 2026-03-02 12:13 ` [PATCH 6/8] cache-tree: allow writing in-memory index as tree Patrick Steinhardt 2026-03-02 12:13 ` [PATCH 7/8] builtin/history: split out extended function to create commits Patrick Steinhardt 2026-03-03 18:43 ` D. Ben Knoble 2026-03-11 9:26 ` Patrick Steinhardt 2026-03-02 12:13 ` [PATCH 8/8] builtin/history: implement "split" subcommand Patrick Steinhardt 2026-03-03 18:47 ` D. Ben Knoble 2026-03-11 9:26 ` Patrick Steinhardt 2026-03-03 18:41 ` [PATCH 0/8] history: introduce " D. Ben Knoble 2026-03-13 22:35 ` Junio C Hamano 2026-03-16 7:13 ` Patrick Steinhardt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox