From e3d4d7787cc3b2f0281e808042ceaa08e05c281b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 2 Mar 2026 13:13:06 +0100 Subject: add-patch: split out `struct interactive_options` 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 Signed-off-by: Junio C Hamano --- add-interactive.c | 177 ++++++++++++------------------------------------------ 1 file changed, 37 insertions(+), 140 deletions(-) (limited to 'add-interactive.c') 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; -- cgit v1.3-6-g1900 From 48f6d9232834be661f0d1dc4f187b324124ccbe0 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 2 Mar 2026 13:13:09 +0100 Subject: add-patch: allow disabling editing of hunks 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 Signed-off-by: Junio C Hamano --- 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(-) (limited to 'add-interactive.c') 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 @@ soft_increment: } 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) -- cgit v1.3-6-g1900