Skip to content

Commit e0d2568

Browse files
gitstergitster
committed
Merge branch 'js/add-i-color-fix'
"git add -i" failed to honor custom colors configured to show patches, which has been corrected. * js/add-i-color-fix: add -i: verify in the tests that colors can be overridden add -p: prefer color.diff.context over color.diff.plain add -i (Perl version): color header to match the C version add -i (built-in): use the same indentation as the Perl version add -p (built-in): do not color the progress indicator separately add -i (built-in): use correct names to load color.diff.* config add -i (built-in): prevent the `reset` "color" from being configured add -i: use `reset_color` consistently add -p (built-in): imitate `xdl_format_hunk_hdr()` generating hunk headers add -i (built-in): send error messages to stderr add -i (built-in): do show an error message for incorrect inputs
2 parents 2aeafbc + 96386fa commit e0d2568

File tree

4 files changed

+127
-32
lines changed

4 files changed

+127
-32
lines changed

‎add-interactive.c

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@
1212
#include "prompt.h"
1313

1414
static void init_color(struct repository *r, struct add_i_state *s,
15-
const char *slot_name, char *dst,
15+
const char *section_and_slot, char *dst,
1616
const char *default_color)
1717
{
18-
char *key = xstrfmt("color.interactive.%s", slot_name);
18+
char *key = xstrfmt("color.%s", section_and_slot);
1919
const char *value;
2020

2121
if (!s->use_color)
@@ -40,20 +40,27 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)
4040
git_config_colorbool("color.interactive", value);
4141
s->use_color = want_color(s->use_color);
4242

43-
init_color(r, s, "header", s->header_color, GIT_COLOR_BOLD);
44-
init_color(r, s, "help", s->help_color, GIT_COLOR_BOLD_RED);
45-
init_color(r, s, "prompt", s->prompt_color, GIT_COLOR_BOLD_BLUE);
46-
init_color(r, s, "error", s->error_color, GIT_COLOR_BOLD_RED);
47-
init_color(r, s, "reset", s->reset_color, GIT_COLOR_RESET);
48-
init_color(r, s, "fraginfo", s->fraginfo_color,
43+
init_color(r, s, "interactive.header", s->header_color, GIT_COLOR_BOLD);
44+
init_color(r, s, "interactive.help", s->help_color, GIT_COLOR_BOLD_RED);
45+
init_color(r, s, "interactive.prompt", s->prompt_color,
46+
GIT_COLOR_BOLD_BLUE);
47+
init_color(r, s, "interactive.error", s->error_color,
48+
GIT_COLOR_BOLD_RED);
49+
50+
init_color(r, s, "diff.frag", s->fraginfo_color,
4951
diff_get_color(s->use_color, DIFF_FRAGINFO));
50-
init_color(r, s, "context", s->context_color,
51-
diff_get_color(s->use_color, DIFF_CONTEXT));
52-
init_color(r, s, "old", s->file_old_color,
52+
init_color(r, s, "diff.context", s->context_color, "fall back");
53+
if (!strcmp(s->context_color, "fall back"))
54+
init_color(r, s, "diff.plain", s->context_color,
55+
diff_get_color(s->use_color, DIFF_CONTEXT));
56+
init_color(r, s, "diff.old", s->file_old_color,
5357
diff_get_color(s->use_color, DIFF_FILE_OLD));
54-
init_color(r, s, "new", s->file_new_color,
58+
init_color(r, s, "diff.new", s->file_new_color,
5559
diff_get_color(s->use_color, DIFF_FILE_NEW));
5660

61+
strlcpy(s->reset_color,
62+
s->use_color ? GIT_COLOR_RESET : "", COLOR_MAXLEN);
63+
5764
FREE_AND_NULL(s->interactive_diff_filter);
5865
git_config_get_string("interactive.difffilter",
5966
&s->interactive_diff_filter);
@@ -194,7 +201,8 @@ static ssize_t find_unique(const char *string, struct prefix_item_list *list)
194201
else if (index + 1 < list->sorted.nr &&
195202
starts_with(list->sorted.items[index + 1].string, string))
196203
return -1;
197-
else if (index < list->sorted.nr)
204+
else if (index < list->sorted.nr &&
205+
starts_with(list->sorted.items[index].string, string))
198206
item = list->sorted.items[index].util;
199207
else
200208
return -1;
@@ -364,7 +372,7 @@ static ssize_t list_and_choose(struct add_i_state *s,
364372

365373
if (from < 0 || from >= items->items.nr ||
366374
(singleton && from + 1 != to)) {
367-
color_fprintf_ln(stdout, s->error_color,
375+
color_fprintf_ln(stderr, s->error_color,
368376
_("Huh (%s)?"), p);
369377
break;
370378
} else if (singleton) {
@@ -1131,7 +1139,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps)
11311139
print_file_item_data.color = data.color;
11321140
print_file_item_data.reset = data.reset;
11331141

1134-
strbuf_addstr(&header, " ");
1142+
strbuf_addstr(&header, " ");
11351143
strbuf_addf(&header, print_file_item_data.modified_fmt,
11361144
_("staged"), _("unstaged"), _("path"));
11371145
opts.list_opts.header = header.buf;

‎add-patch.c

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -661,13 +661,18 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk,
661661
else
662662
new_offset += delta;
663663

664-
strbuf_addf(out, "@@ -%lu,%lu +%lu,%lu @@",
665-
old_offset, header->old_count,
666-
new_offset, header->new_count);
664+
strbuf_addf(out, "@@ -%lu", old_offset);
665+
if (header->old_count != 1)
666+
strbuf_addf(out, ",%lu", header->old_count);
667+
strbuf_addf(out, " +%lu", new_offset);
668+
if (header->new_count != 1)
669+
strbuf_addf(out, ",%lu", header->new_count);
670+
strbuf_addstr(out, " @@");
671+
667672
if (len)
668673
strbuf_add(out, p, len);
669674
else if (colored)
670-
strbuf_addf(out, "%s\n", GIT_COLOR_RESET);
675+
strbuf_addf(out, "%s\n", s->s.reset_color);
671676
else
672677
strbuf_addch(out, '\n');
673678
}
@@ -1060,7 +1065,7 @@ static void recolor_hunk(struct add_p_state *s, struct hunk *hunk)
10601065
s->s.file_new_color :
10611066
s->s.context_color);
10621067
strbuf_add(&s->colored, plain + current, eol - current);
1063-
strbuf_addstr(&s->colored, GIT_COLOR_RESET);
1068+
strbuf_addstr(&s->colored, s->s.reset_color);
10641069
if (next > eol)
10651070
strbuf_add(&s->colored, plain + eol, next - eol);
10661071
current = next;
@@ -1456,15 +1461,15 @@ static int patch_update_file(struct add_p_state *s,
14561461
else
14571462
prompt_mode_type = PROMPT_HUNK;
14581463

1459-
color_fprintf(stdout, s->s.prompt_color,
1460-
"(%"PRIuMAX"/%"PRIuMAX") ",
1464+
printf("%s(%"PRIuMAX"/%"PRIuMAX") ", s->s.prompt_color,
14611465
(uintmax_t)hunk_index + 1,
14621466
(uintmax_t)(file_diff->hunk_nr
14631467
? file_diff->hunk_nr
14641468
: 1));
1465-
color_fprintf(stdout, s->s.prompt_color,
1466-
_(s->mode->prompt_mode[prompt_mode_type]),
1467-
s->buf.buf);
1469+
printf(_(s->mode->prompt_mode[prompt_mode_type]),
1470+
s->buf.buf);
1471+
if (*s->s.reset_color)
1472+
fputs(s->s.reset_color, stdout);
14681473
fflush(stdout);
14691474
if (read_single_character(s) == EOF)
14701475
break;

‎git-add--interactive.perl

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@
3030
$diff_use_color ? (
3131
$repo->get_color('color.diff.frag', 'cyan'),
3232
) : ();
33-
my ($diff_plain_color) =
33+
my ($diff_context_color) =
3434
$diff_use_color ? (
35-
$repo->get_color('color.diff.plain', ''),
35+
$repo->get_color($repo->config('color.diff.context') ? 'color.diff.context' : 'color.diff.plain', ''),
3636
) : ();
3737
my ($diff_old_color) =
3838
$diff_use_color ? (
@@ -483,10 +483,8 @@ sub list_and_choose {
483483
my $last_lf = 0;
484484

485485
if ($opts->{HEADER}) {
486-
if (!$opts->{LIST_FLAT}) {
487-
print " ";
488-
}
489-
print colored $header_color, "$opts->{HEADER}\n";
486+
my $indent = $opts->{LIST_FLAT} ? "" : " ";
487+
print colored $header_color, "$indent$opts->{HEADER}\n";
490488
}
491489
for ($i = 0; $i < @stuff; $i++) {
492490
my $chosen = $chosen[$i] ? '*' : ' ';
@@ -1048,7 +1046,7 @@ sub color_diff {
10481046
colored((/^@/ ? $fraginfo_color :
10491047
/^\+/ ? $diff_new_color :
10501048
/^-/ ? $diff_old_color :
1051-
$diff_plain_color),
1049+
$diff_context_color),
10521050
$_);
10531051
} @_;
10541052
}

‎t/t3701-add-interactive.sh

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,90 @@ test_expect_success 'diffs can be colorized' '
589589
grep "$(printf "\\033")" output
590590
'
591591

592+
test_expect_success 'colors can be overridden' '
593+
git reset --hard &&
594+
test_when_finished "git rm -f color-test" &&
595+
test_write_lines context old more-context >color-test &&
596+
git add color-test &&
597+
test_write_lines context new more-context another-one >color-test &&
598+
599+
echo trigger an error message >input &&
600+
force_color git \
601+
-c color.interactive.error=blue \
602+
add -i 2>err.raw <input &&
603+
test_decode_color <err.raw >err &&
604+
grep "<BLUE>Huh (trigger)?<RESET>" err &&
605+
606+
test_write_lines help quit >input &&
607+
force_color git \
608+
-c color.interactive.header=red \
609+
-c color.interactive.help=green \
610+
-c color.interactive.prompt=yellow \
611+
add -i >actual.raw <input &&
612+
test_decode_color <actual.raw >actual &&
613+
cat >expect <<-\EOF &&
614+
<RED> staged unstaged path<RESET>
615+
1: +3/-0 +2/-1 color-test
616+
617+
<RED>*** Commands ***<RESET>
618+
1: <YELLOW>s<RESET>tatus 2: <YELLOW>u<RESET>pdate 3: <YELLOW>r<RESET>evert 4: <YELLOW>a<RESET>dd untracked
619+
5: <YELLOW>p<RESET>atch 6: <YELLOW>d<RESET>iff 7: <YELLOW>q<RESET>uit 8: <YELLOW>h<RESET>elp
620+
<YELLOW>What now<RESET>> <GREEN>status - show paths with changes<RESET>
621+
<GREEN>update - add working tree state to the staged set of changes<RESET>
622+
<GREEN>revert - revert staged set of changes back to the HEAD version<RESET>
623+
<GREEN>patch - pick hunks and update selectively<RESET>
624+
<GREEN>diff - view diff between HEAD and index<RESET>
625+
<GREEN>add untracked - add contents of untracked files to the staged set of changes<RESET>
626+
<RED>*** Commands ***<RESET>
627+
1: <YELLOW>s<RESET>tatus 2: <YELLOW>u<RESET>pdate 3: <YELLOW>r<RESET>evert 4: <YELLOW>a<RESET>dd untracked
628+
5: <YELLOW>p<RESET>atch 6: <YELLOW>d<RESET>iff 7: <YELLOW>q<RESET>uit 8: <YELLOW>h<RESET>elp
629+
<YELLOW>What now<RESET>> Bye.
630+
EOF
631+
test_cmp expect actual &&
632+
633+
: exercise recolor_hunk by editing and then look at the hunk again &&
634+
test_write_lines s e K q >input &&
635+
force_color git \
636+
-c color.interactive.prompt=yellow \
637+
-c color.diff.meta=italic \
638+
-c color.diff.frag=magenta \
639+
-c color.diff.context=cyan \
640+
-c color.diff.old=bold \
641+
-c color.diff.new=blue \
642+
-c core.editor=touch \
643+
add -p >actual.raw <input &&
644+
test_decode_color <actual.raw >actual.decoded &&
645+
sed "s/index [0-9a-f]*\\.\\.[0-9a-f]* 100644/<INDEX-LINE>/" <actual.decoded >actual &&
646+
cat >expect <<-\EOF &&
647+
<ITALIC>diff --git a/color-test b/color-test<RESET>
648+
<ITALIC><INDEX-LINE><RESET>
649+
<ITALIC>--- a/color-test<RESET>
650+
<ITALIC>+++ b/color-test<RESET>
651+
<MAGENTA>@@ -1,3 +1,4 @@<RESET>
652+
<CYAN> context<RESET>
653+
<BOLD>-old<RESET>
654+
<BLUE>+<RESET><BLUE>new<RESET>
655+
<CYAN> more-context<RESET>
656+
<BLUE>+<RESET><BLUE>another-one<RESET>
657+
<YELLOW>(1/1) Stage this hunk [y,n,q,a,d,s,e,?]? <RESET><BOLD>Split into 2 hunks.<RESET>
658+
<MAGENTA>@@ -1,3 +1,3 @@<RESET>
659+
<CYAN> context<RESET>
660+
<BOLD>-old<RESET>
661+
<BLUE>+<RESET><BLUE>new<RESET>
662+
<CYAN> more-context<RESET>
663+
<YELLOW>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]? <RESET><MAGENTA>@@ -3 +3,2 @@<RESET>
664+
<CYAN> more-context<RESET>
665+
<BLUE>+<RESET><BLUE>another-one<RESET>
666+
<YELLOW>(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,?]? <RESET><MAGENTA>@@ -1,3 +1,3 @@<RESET>
667+
<CYAN> context<RESET>
668+
<BOLD>-old<RESET>
669+
<BLUE>+new<RESET>
670+
<CYAN> more-context<RESET>
671+
<YELLOW>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]? <RESET>
672+
EOF
673+
test_cmp expect actual
674+
'
675+
592676
test_expect_success 'colorized diffs respect diff.wsErrorHighlight' '
593677
git reset --hard &&
594678

0 commit comments

Comments
 (0)