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-patch.c | 187 +++++++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 153 insertions(+), 34 deletions(-) (limited to 'add-patch.c') 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 @@ soft_increment: } 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 @@ soft_increment: 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 @@ soft_increment: 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 @@ soft_increment: } 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 @@ soft_increment: 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 @@ soft_increment: } } - if (s->s.auto_advance) + if (s->s.cfg.auto_advance) apply_patch(s, file_diff); putchar('\n'); @@ -1878,7 +1997,7 @@ soft_increment: } 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); -- cgit v1.3