Merge branch 'jk/add-i-color'

Some among "git add -p" and friends ignored color.diff and/or
color.ui configuration variables, which is an old regression, which
has been corrected.

* jk/add-i-color:
  contrib/diff-highlight: mention interactive.diffFilter
  add-interactive: manually fall back color config to color.ui
  add-interactive: respect color.diff for diff coloring
  stash: pass --no-color to diff plumbing child processes
main
Junio C Hamano 2025-09-23 11:53:40 -07:00
commit 2e8d7569ea
7 changed files with 148 additions and 40 deletions

View File

@ -20,14 +20,14 @@
#include "prompt.h"
#include "tree.h"

static void init_color(struct repository *r, struct add_i_state *s,
static void init_color(struct repository *r, int 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 (!s->use_color)
if (!use_color)
dst[0] = '\0';
else if (repo_config_get_value(r, key, &value) ||
color_parse(value, dst))
@ -36,42 +36,63 @@ static void init_color(struct repository *r, struct add_i_state *s,
free(key);
}

static int check_color_config(struct repository *r, const char *var)
{
const char *value;
int ret;

if (repo_config_get_value(r, var, &value))
ret = -1;
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 < 0 && !repo_config_get_value(r, "color.ui", &value))
ret = git_config_colorbool("color.ui", value);

return want_color(ret);
}

void init_add_i_state(struct add_i_state *s, struct repository *r,
struct add_p_opt *add_p_opt)
{
const char *value;

s->r = r;
s->context = -1;
s->interhunkcontext = -1;

if (repo_config_get_value(r, "color.interactive", &value))
s->use_color = -1;
else
s->use_color =
git_config_colorbool("color.interactive", value);
s->use_color = want_color(s->use_color);
s->use_color_interactive = check_color_config(r, "color.interactive");

init_color(r, s, "interactive.header", s->header_color, GIT_COLOR_BOLD);
init_color(r, s, "interactive.help", s->help_color, GIT_COLOR_BOLD_RED);
init_color(r, s, "interactive.prompt", s->prompt_color,
GIT_COLOR_BOLD_BLUE);
init_color(r, s, "interactive.error", s->error_color,
GIT_COLOR_BOLD_RED);
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,
s->use_color_interactive ? GIT_COLOR_RESET : "", COLOR_MAXLEN);

init_color(r, s, "diff.frag", s->fraginfo_color,
diff_get_color(s->use_color, DIFF_FRAGINFO));
init_color(r, s, "diff.context", s->context_color, "fall back");
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, "diff.plain", s->context_color,
diff_get_color(s->use_color, DIFF_CONTEXT));
init_color(r, s, "diff.old", s->file_old_color,
diff_get_color(s->use_color, DIFF_FILE_OLD));
init_color(r, s, "diff.new", s->file_new_color,
diff_get_color(s->use_color, DIFF_FILE_NEW));

strlcpy(s->reset_color,
s->use_color ? GIT_COLOR_RESET : "", COLOR_MAXLEN);
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,
s->use_color_diff ? GIT_COLOR_RESET : "", COLOR_MAXLEN);

FREE_AND_NULL(s->interactive_diff_filter);
repo_config_get_string(r, "interactive.difffilter",
@ -109,7 +130,8 @@ void clear_add_i_state(struct add_i_state *s)
FREE_AND_NULL(s->interactive_diff_filter);
FREE_AND_NULL(s->interactive_diff_algorithm);
memset(s, 0, sizeof(*s));
s->use_color = -1;
s->use_color_interactive = -1;
s->use_color_diff = -1;
}

/*
@ -1188,9 +1210,9 @@ int run_add_i(struct repository *r, const struct pathspec *ps,
* When color was asked for, use the prompt color for
* highlighting, otherwise use square brackets.
*/
if (s.use_color) {
if (s.use_color_interactive) {
data.color = s.prompt_color;
data.reset = s.reset_color;
data.reset = s.reset_color_interactive;
}
print_file_item_data.color = data.color;
print_file_item_data.reset = data.reset;

View File

@ -12,16 +12,19 @@ struct add_p_opt {

struct add_i_state {
struct repository *r;
int use_color;
int use_color_interactive;
int 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[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;

View File

@ -300,7 +300,7 @@ static void err(struct add_p_state *s, const char *fmt, ...)
va_start(args, fmt);
fputs(s->s.error_color, stdout);
vprintf(fmt, args);
puts(s->s.reset_color);
puts(s->s.reset_color_interactive);
va_end(args);
}

@ -457,7 +457,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
}
strbuf_complete_line(plain);

if (want_color_fd(1, -1)) {
if (want_color_fd(1, s->s.use_color_diff)) {
struct child_process colored_cp = CHILD_PROCESS_INIT;
const char *diff_filter = s->s.interactive_diff_filter;

@ -714,7 +714,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);
strbuf_addf(out, "%s\n", s->s.reset_color_diff);
else
strbuf_addch(out, '\n');
}
@ -1107,7 +1107,7 @@ static void recolor_hunk(struct add_p_state *s, struct hunk *hunk)
s->s.file_new_color :
s->s.context_color);
strbuf_add(&s->colored, plain + current, eol - current);
strbuf_addstr(&s->colored, s->s.reset_color);
strbuf_addstr(&s->colored, s->s.reset_color_diff);
if (next > eol)
strbuf_add(&s->colored, plain + eol, next - eol);
current = next;
@ -1528,8 +1528,8 @@ static int patch_update_file(struct add_p_state *s,
: 1));
printf(_(s->mode->prompt_mode[prompt_mode_type]),
s->buf.buf);
if (*s->s.reset_color)
fputs(s->s.reset_color, stdout);
if (*s->s.reset_color_interactive)
fputs(s->s.reset_color_interactive, stdout);
fflush(stdout);
if (read_single_character(s) == EOF)
break;

View File

@ -377,7 +377,7 @@ static int diff_tree_binary(struct strbuf *out, struct object_id *w_commit)
* however it should be done together with apply_cached.
*/
cp.git_cmd = 1;
strvec_pushl(&cp.args, "diff-tree", "--binary", NULL);
strvec_pushl(&cp.args, "diff-tree", "--binary", "--no-color", NULL);
strvec_pushf(&cp.args, "%s^2^..%s^2", w_commit_hex, w_commit_hex);

return pipe_command(&cp, NULL, 0, out, 0, NULL, 0);
@ -1284,6 +1284,7 @@ static int stash_staged(struct stash_info *info, struct strbuf *out_patch,

cp_diff_tree.git_cmd = 1;
strvec_pushl(&cp_diff_tree.args, "diff-tree", "-p", "--binary",
"--no-color",
"-U1", "HEAD", oid_to_hex(&info->w_tree), "--", NULL);
if (pipe_command(&cp_diff_tree, NULL, 0, out_patch, 0, NULL, 0)) {
ret = -1;
@ -1346,6 +1347,7 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,

cp_diff_tree.git_cmd = 1;
strvec_pushl(&cp_diff_tree.args, "diff-tree", "-p", "-U1", "HEAD",
"--no-color",
oid_to_hex(&info->w_tree), "--", NULL);
if (pipe_command(&cp_diff_tree, NULL, 0, out_patch, 0, NULL, 0)) {
ret = -1;
@ -1720,6 +1722,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q

cp_diff.git_cmd = 1;
strvec_pushl(&cp_diff.args, "diff-index", "-p",
"--no-color",
"--cached", "--binary", "HEAD", "--",
NULL);
add_pathspecs(&cp_diff.args, ps);

View File

@ -58,6 +58,14 @@ following in your git configuration:
diff = diff-highlight | less
---------------------------------------------

If you use the interactive patch mode of `git add -p`, `git checkout
-p`, etc, you may also want to configure it to be used there:

---------------------------------------------
[interactive]
diffFilter = diff-highlight
---------------------------------------------


Color Config
------------

View File

@ -866,6 +866,44 @@ test_expect_success 'colorized diffs respect diff.wsErrorHighlight' '
test_grep "old<" output
'

test_expect_success 'diff color respects color.diff' '
git reset --hard &&

echo old >test &&
git add test &&
echo new >test &&

printf n >n &&
force_color git \
-c color.interactive=auto \
-c color.interactive.prompt=blue \
-c color.diff=false \
-c color.diff.old=red \
add -p >output.raw 2>&1 <n &&
test_decode_color <output.raw >output &&
test_grep "BLUE.*Stage this hunk" output &&
test_grep ! "RED" output
'

test_expect_success 're-coloring diff without color.interactive' '
git reset --hard &&

test_write_lines 1 2 3 >test &&
git add test &&
test_write_lines one 2 three >test &&

test_write_lines s n n |
force_color git \
-c color.interactive=false \
-c color.interactive.prompt=blue \
-c color.diff=true \
-c color.diff.frag="bold magenta" \
add -p >output.raw 2>&1 &&
test_decode_color <output.raw >output &&
test_grep "<BOLD;MAGENTA>@@" output &&
test_grep ! "BLUE" output
'

test_expect_success 'diffFilter filters diff' '
git reset --hard &&

@ -1283,6 +1321,12 @@ test_expect_success 'stash accepts -U and --inter-hunk-context' '
test_grep "@@ -2,20 +2,20 @@" actual
'

test_expect_success 'set up base for -p color tests' '
echo commit >file &&
git commit -am "commit state" &&
git tag patch-base
'

for cmd in add checkout commit reset restore "stash save" "stash push"
do
test_expect_success "$cmd rejects invalid context options" '
@ -1299,6 +1343,15 @@ do
test_must_fail git $cmd --inter-hunk-context 2 2>actual &&
test_grep -E ".--inter-hunk-context. requires .(--interactive/)?--patch." actual
'

test_expect_success "$cmd falls back to color.ui" '
git reset --hard patch-base &&
echo working-tree >file &&
test_write_lines y |
force_color git -c color.ui=false $cmd -p >output.raw 2>&1 &&
test_decode_color <output.raw >output &&
test_cmp output.raw output
'
done

test_done

View File

@ -107,4 +107,23 @@ test_expect_success 'stash -p with split hunk' '
! grep "added line 2" test
'

test_expect_success 'stash -p not confused by GIT_PAGER_IN_USE' '
echo to-stash >test &&
# Set both GIT_PAGER_IN_USE and TERM. Our goal is to entice any
# diff subprocesses into thinking that they could output
# color, even though their stdout is not going into a tty.
echo y |
GIT_PAGER_IN_USE=1 TERM=vt100 git stash -p &&
git diff --exit-code
'

test_expect_success 'index push not confused by GIT_PAGER_IN_USE' '
echo index >test &&
git add test &&
echo working-tree >test &&
# As above, we try to entice the child diff into using color.
GIT_PAGER_IN_USE=1 TERM=vt100 git stash push test &&
git diff --exit-code
'

test_done