Merge branch 'jk/color-variable-fixes'

Some places in the code confused a variable that is *not* a boolean
to enable color but is an enum that records what the user requested
to do about color.  A couple of bugs of this sort have been fixed,
while the code has been cleaned up to prevent similar bugs in the
future.

* jk/color-variable-fixes:
  config: store want_color() result in a separate bool
  add-interactive: retain colorbool values longer
  color: return bool from want_color()
  color: use git_colorbool enum type to store colorbools
  pretty: use format_commit_context.auto_color as colorbool
  diff: stop passing ecbdata->use_color as boolean
  diff: pass o->use_color directly to fill_metainfo()
  diff: don't use diff_options.use_color as a strict bool
  diff: simplify color_moved check when flushing
  grep: don't treat grep_opt.color as a strict bool
  color: return enum from git_config_colorbool()
  color: use GIT_COLOR_* instead of numeric constants
This commit is contained in:
Junio C Hamano
2025-09-29 11:40:35 -07:00
30 changed files with 114 additions and 109 deletions

44
diff.c
View File

@@ -57,7 +57,7 @@ static int diff_detect_rename_default;
static int diff_indent_heuristic = 1;
static int diff_rename_limit_default = 1000;
static int diff_suppress_blank_empty;
static int diff_use_color_default = -1;
static enum git_colorbool diff_use_color_default = GIT_COLOR_UNKNOWN;
static int diff_color_moved_default;
static int diff_color_moved_ws_default;
static int diff_context_default = 3;
@@ -1672,7 +1672,7 @@ static void emit_hunk_header(struct emit_callback *ecbdata,
const char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);
const char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);
const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
const char *reverse = ecbdata->color_diff ? GIT_COLOR_REVERSE : "";
const char *reverse = want_color(ecbdata->color_diff) ? GIT_COLOR_REVERSE : "";
static const char atat[2] = { '@', '@' };
const char *cp, *ep;
struct strbuf msgbuf = STRBUF_INIT;
@@ -1826,7 +1826,7 @@ static void emit_rewrite_diff(const char *name_a,
size_two = fill_textconv(o->repo, textconv_two, two, &data_two);
memset(&ecbdata, 0, sizeof(ecbdata));
ecbdata.color_diff = want_color(o->use_color);
ecbdata.color_diff = o->use_color;
ecbdata.ws_rule = whitespace_rule(o->repo->index, name_b);
ecbdata.opt = o;
if (ecbdata.ws_rule & WS_BLANK_AT_EOF) {
@@ -2303,7 +2303,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata)
}
}
const char *diff_get_color(int diff_use_color, enum color_diff ix)
const char *diff_get_color(enum git_colorbool diff_use_color, enum color_diff ix)
{
if (want_color(diff_use_color))
return diff_colors[ix];
@@ -3732,7 +3732,7 @@ static void builtin_diff(const char *name_a,
if (o->flags.suppress_diff_headers)
lbl[0] = NULL;
ecbdata.label_path = lbl;
ecbdata.color_diff = want_color(o->use_color);
ecbdata.color_diff = o->use_color;
ecbdata.ws_rule = whitespace_rule(o->repo->index, name_b);
if (ecbdata.ws_rule & WS_BLANK_AT_EOF)
check_blank_at_eof(&mf1, &mf2, &ecbdata);
@@ -4497,7 +4497,7 @@ static void fill_metainfo(struct strbuf *msg,
struct diff_options *o,
struct diff_filepair *p,
int *must_show_header,
int use_color)
enum git_colorbool use_color)
{
const char *set = diff_get_color(use_color, DIFF_METAINFO);
const char *reset = diff_get_color(use_color, DIFF_RESET);
@@ -4596,7 +4596,7 @@ static void run_diff_cmd(const struct external_diff *pgm,
*/
fill_metainfo(msg, name, other, one, two, o, p,
&must_show_header,
want_color(o->use_color) && !pgm);
pgm ? GIT_COLOR_NEVER : o->use_color);
xfrm_msg = msg->len ? msg->buf : NULL;
}
@@ -4995,8 +4995,7 @@ void diff_setup_done(struct diff_options *options)
if (options->flags.follow_renames)
diff_check_follow_pathspec(&options->pathspec, 1);
if (!options->use_color ||
(options->flags.allow_external && external_diff()))
if (options->flags.allow_external && external_diff())
options->color_moved = 0;
if (options->filter_not) {
@@ -5278,7 +5277,7 @@ static int diff_opt_color_words(const struct option *opt,
struct diff_options *options = opt->value;
BUG_ON_OPT_NEG(unset);
options->use_color = 1;
options->use_color = GIT_COLOR_ALWAYS;
options->word_diff = DIFF_WORDS_COLOR;
options->word_regex = arg;
return 0;
@@ -5600,7 +5599,7 @@ static int diff_opt_word_diff(const struct option *opt,
if (!strcmp(arg, "plain"))
options->word_diff = DIFF_WORDS_PLAIN;
else if (!strcmp(arg, "color")) {
options->use_color = 1;
options->use_color = GIT_COLOR_ALWAYS;
options->word_diff = DIFF_WORDS_COLOR;
}
else if (!strcmp(arg, "porcelain"))
@@ -6733,7 +6732,7 @@ static void diff_flush_patch_all_file_pairs(struct diff_options *o)
if (WSEH_NEW & WS_RULE_MASK)
BUG("WS rules bit mask overlaps with diff symbol flags");
if (o->color_moved)
if (o->color_moved && want_color(o->use_color))
o->emitted_symbols = &esm;
if (o->additional_path_headers)
@@ -6746,20 +6745,17 @@ static void diff_flush_patch_all_file_pairs(struct diff_options *o)
}
if (o->emitted_symbols) {
if (o->color_moved) {
struct mem_pool entry_pool;
struct moved_entry_list *entry_list;
struct mem_pool entry_pool;
struct moved_entry_list *entry_list;
mem_pool_init(&entry_pool, 1024 * 1024);
entry_list = add_lines_to_move_detection(o,
&entry_pool);
mark_color_as_moved(o, entry_list);
if (o->color_moved == COLOR_MOVED_ZEBRA_DIM)
dim_moved_lines(o);
mem_pool_init(&entry_pool, 1024 * 1024);
entry_list = add_lines_to_move_detection(o, &entry_pool);
mark_color_as_moved(o, entry_list);
if (o->color_moved == COLOR_MOVED_ZEBRA_DIM)
dim_moved_lines(o);
mem_pool_discard(&entry_pool, 0);
free(entry_list);
}
mem_pool_discard(&entry_pool, 0);
free(entry_list);
for (i = 0; i < esm.nr; i++)
emit_diff_symbol_from_struct(o, &esm.buf[i]);