freshlybakedca.ke / git
Git fork
fork atom

add-interactive: respect color.diff for diff coloring

The old perl git-add--interactive.perl script used the color.diff config
option to decide whether to color diffs (and if not set, it fell back to
the value of color.ui via git-config's --get-colorbool option). When we
switched to the builtin version, this was lost: we respect only
color.ui. So for example:

git -c color.diff=false add -p

would color the diff, even when it should not.

The culprit is this line in add-interactive.c's parse_diff():

if (want_color_fd(1, -1))

That "-1" means "no config has been set", which causes it to fall back
to the color.ui setting. We should instead be passing the value of
color.diff. But the problem is that we never even parse that config
option!

Instead the builtin interactive code parses only the value of
color.interactive, which is used for prompts and other messages. One
could perhaps argue that this should cover interactive diff coloring,
too, but historically it did not. The perl script treated
color.interactive and color.diff separately. So we should grab the
values for both, keeping separate fields in our add_i_state variable,
rather than a single use_color field.

We also load individual color slots (e.g., color.interactive.prompt),
leaving them as the empty string when color is disabled. This happens
via the init_color() helper in add-interactive, which checks that
use_color field. Now that there are two such fields, we need to pass the
appropriate one for each color.

The colors are mostly easy to divide up; color.interactive.* follows
color.interactive, and color.diff.* follows color.diff. But the "reset"
color is tricky. It is used for both types of coloring, but the two can
be configured independently. So we introduce two separate reset colors,
and use each in the appropriate spot.

There are two new tests. The first enables interactive prompt colors but
disables color.diff. We should see a colored prompt but not a colored
diff, showing that we are now respecting color.diff (and not
color.interactive or color.ui).

The second does the opposite. We disable color.interactive but turn on
color.diff with a custom fragment color. When we split a hunk, the
interactive code has to re-color the hunk header, which lets us check
that we correctly loaded the color.diff.frag config based on color.diff,
not color.interactive.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

authored by

Jeff King and committed by
Junio C Hamano 8c78b5c8 89b4183e

+93 -39
unified split
+44 -31
add-interactive.c
··· 20 20 #include "prompt.h" 21 21 #include "tree.h" 22 22 23 - static void init_color(struct repository *r, struct add_i_state *s, 23 + static void init_color(struct repository *r, int use_color, 24 24 const char *section_and_slot, char *dst, 25 25 const char *default_color) 26 26 { 27 27 char *key = xstrfmt("color.%s", section_and_slot); 28 28 const char *value; 29 29 30 - if (!s->use_color) 30 + if (!use_color) 31 31 dst[0] = '\0'; 32 32 else if (repo_config_get_value(r, key, &value) || 33 33 color_parse(value, dst)) ··· 36 36 free(key); 37 37 } 38 38 39 - void init_add_i_state(struct add_i_state *s, struct repository *r, 40 - struct add_p_opt *add_p_opt) 39 + static int check_color_config(struct repository *r, const char *var) 41 40 { 42 41 const char *value; 42 + int ret; 43 43 44 + if (repo_config_get_value(r, var, &value)) 45 + ret = -1; 46 + else 47 + ret = git_config_colorbool(var, value); 48 + return want_color(ret); 49 + } 50 + 51 + void init_add_i_state(struct add_i_state *s, struct repository *r, 52 + struct add_p_opt *add_p_opt) 53 + { 44 54 s->r = r; 45 55 s->context = -1; 46 56 s->interhunkcontext = -1; 47 57 48 - if (repo_config_get_value(r, "color.interactive", &value)) 49 - s->use_color = -1; 50 - else 51 - s->use_color = 52 - git_config_colorbool("color.interactive", value); 53 - s->use_color = want_color(s->use_color); 58 + s->use_color_interactive = check_color_config(r, "color.interactive"); 59 + 60 + init_color(r, s->use_color_interactive, "interactive.header", 61 + s->header_color, GIT_COLOR_BOLD); 62 + init_color(r, s->use_color_interactive, "interactive.help", 63 + s->help_color, GIT_COLOR_BOLD_RED); 64 + init_color(r, s->use_color_interactive, "interactive.prompt", 65 + s->prompt_color, GIT_COLOR_BOLD_BLUE); 66 + init_color(r, s->use_color_interactive, "interactive.error", 67 + s->error_color, GIT_COLOR_BOLD_RED); 68 + strlcpy(s->reset_color_interactive, 69 + s->use_color_interactive ? GIT_COLOR_RESET : "", COLOR_MAXLEN); 54 70 55 - init_color(r, s, "interactive.header", s->header_color, GIT_COLOR_BOLD); 56 - init_color(r, s, "interactive.help", s->help_color, GIT_COLOR_BOLD_RED); 57 - init_color(r, s, "interactive.prompt", s->prompt_color, 58 - GIT_COLOR_BOLD_BLUE); 59 - init_color(r, s, "interactive.error", s->error_color, 60 - GIT_COLOR_BOLD_RED); 71 + s->use_color_diff = check_color_config(r, "color.diff"); 61 72 62 - init_color(r, s, "diff.frag", s->fraginfo_color, 63 - diff_get_color(s->use_color, DIFF_FRAGINFO)); 64 - init_color(r, s, "diff.context", s->context_color, "fall back"); 73 + init_color(r, s->use_color_diff, "diff.frag", s->fraginfo_color, 74 + diff_get_color(s->use_color_diff, DIFF_FRAGINFO)); 75 + init_color(r, s->use_color_diff, "diff.context", s->context_color, 76 + "fall back"); 65 77 if (!strcmp(s->context_color, "fall back")) 66 - init_color(r, s, "diff.plain", s->context_color, 67 - diff_get_color(s->use_color, DIFF_CONTEXT)); 68 - init_color(r, s, "diff.old", s->file_old_color, 69 - diff_get_color(s->use_color, DIFF_FILE_OLD)); 70 - init_color(r, s, "diff.new", s->file_new_color, 71 - diff_get_color(s->use_color, DIFF_FILE_NEW)); 72 - 73 - strlcpy(s->reset_color, 74 - s->use_color ? GIT_COLOR_RESET : "", COLOR_MAXLEN); 78 + init_color(r, s->use_color_diff, "diff.plain", 79 + s->context_color, 80 + diff_get_color(s->use_color_diff, DIFF_CONTEXT)); 81 + init_color(r, s->use_color_diff, "diff.old", s->file_old_color, 82 + diff_get_color(s->use_color_diff, DIFF_FILE_OLD)); 83 + init_color(r, s->use_color_diff, "diff.new", s->file_new_color, 84 + diff_get_color(s->use_color_diff, DIFF_FILE_NEW)); 85 + strlcpy(s->reset_color_diff, 86 + s->use_color_diff ? GIT_COLOR_RESET : "", COLOR_MAXLEN); 75 87 76 88 FREE_AND_NULL(s->interactive_diff_filter); 77 89 repo_config_get_string(r, "interactive.difffilter", ··· 109 121 FREE_AND_NULL(s->interactive_diff_filter); 110 122 FREE_AND_NULL(s->interactive_diff_algorithm); 111 123 memset(s, 0, sizeof(*s)); 112 - s->use_color = -1; 124 + s->use_color_interactive = -1; 125 + s->use_color_diff = -1; 113 126 } 114 127 115 128 /* ··· 1188 1201 * When color was asked for, use the prompt color for 1189 1202 * highlighting, otherwise use square brackets. 1190 1203 */ 1191 - if (s.use_color) { 1204 + if (s.use_color_interactive) { 1192 1205 data.color = s.prompt_color; 1193 - data.reset = s.reset_color; 1206 + data.reset = s.reset_color_interactive; 1194 1207 } 1195 1208 print_file_item_data.color = data.color; 1196 1209 print_file_item_data.reset = data.reset;
+5 -2
add-interactive.h
··· 12 12 13 13 struct add_i_state { 14 14 struct repository *r; 15 - int use_color; 15 + int use_color_interactive; 16 + int use_color_diff; 16 17 char header_color[COLOR_MAXLEN]; 17 18 char help_color[COLOR_MAXLEN]; 18 19 char prompt_color[COLOR_MAXLEN]; 19 20 char error_color[COLOR_MAXLEN]; 20 - char reset_color[COLOR_MAXLEN]; 21 + char reset_color_interactive[COLOR_MAXLEN]; 22 + 21 23 char fraginfo_color[COLOR_MAXLEN]; 22 24 char context_color[COLOR_MAXLEN]; 23 25 char file_old_color[COLOR_MAXLEN]; 24 26 char file_new_color[COLOR_MAXLEN]; 27 + char reset_color_diff[COLOR_MAXLEN]; 25 28 26 29 int use_single_key; 27 30 char *interactive_diff_filter, *interactive_diff_algorithm;
+6 -6
add-patch.c
··· 300 300 va_start(args, fmt); 301 301 fputs(s->s.error_color, stdout); 302 302 vprintf(fmt, args); 303 - puts(s->s.reset_color); 303 + puts(s->s.reset_color_interactive); 304 304 va_end(args); 305 305 } 306 306 ··· 457 457 } 458 458 strbuf_complete_line(plain); 459 459 460 - if (want_color_fd(1, -1)) { 460 + if (want_color_fd(1, s->s.use_color_diff)) { 461 461 struct child_process colored_cp = CHILD_PROCESS_INIT; 462 462 const char *diff_filter = s->s.interactive_diff_filter; 463 463 ··· 714 714 if (len) 715 715 strbuf_add(out, p, len); 716 716 else if (colored) 717 - strbuf_addf(out, "%s\n", s->s.reset_color); 717 + strbuf_addf(out, "%s\n", s->s.reset_color_diff); 718 718 else 719 719 strbuf_addch(out, '\n'); 720 720 } ··· 1107 1107 s->s.file_new_color : 1108 1108 s->s.context_color); 1109 1109 strbuf_add(&s->colored, plain + current, eol - current); 1110 - strbuf_addstr(&s->colored, s->s.reset_color); 1110 + strbuf_addstr(&s->colored, s->s.reset_color_diff); 1111 1111 if (next > eol) 1112 1112 strbuf_add(&s->colored, plain + eol, next - eol); 1113 1113 current = next; ··· 1528 1528 : 1)); 1529 1529 printf(_(s->mode->prompt_mode[prompt_mode_type]), 1530 1530 s->buf.buf); 1531 - if (*s->s.reset_color) 1532 - fputs(s->s.reset_color, stdout); 1531 + if (*s->s.reset_color_interactive) 1532 + fputs(s->s.reset_color_interactive, stdout); 1533 1533 fflush(stdout); 1534 1534 if (read_single_character(s) == EOF) 1535 1535 break;
+38
t/t3701-add-interactive.sh
··· 866 866 test_grep "old<" output 867 867 ' 868 868 869 + test_expect_success 'diff color respects color.diff' ' 870 + git reset --hard && 871 + 872 + echo old >test && 873 + git add test && 874 + echo new >test && 875 + 876 + printf n >n && 877 + force_color git \ 878 + -c color.interactive=auto \ 879 + -c color.interactive.prompt=blue \ 880 + -c color.diff=false \ 881 + -c color.diff.old=red \ 882 + add -p >output.raw 2>&1 <n && 883 + test_decode_color <output.raw >output && 884 + test_grep "BLUE.*Stage this hunk" output && 885 + test_grep ! "RED" output 886 + ' 887 + 888 + test_expect_success 're-coloring diff without color.interactive' ' 889 + git reset --hard && 890 + 891 + test_write_lines 1 2 3 >test && 892 + git add test && 893 + test_write_lines one 2 three >test && 894 + 895 + test_write_lines s n n | 896 + force_color git \ 897 + -c color.interactive=false \ 898 + -c color.interactive.prompt=blue \ 899 + -c color.diff=true \ 900 + -c color.diff.frag="bold magenta" \ 901 + add -p >output.raw 2>&1 && 902 + test_decode_color <output.raw >output && 903 + test_grep "<BOLD;MAGENTA>@@" output && 904 + test_grep ! "BLUE" output 905 + ' 906 + 869 907 test_expect_success 'diffFilter filters diff' ' 870 908 git reset --hard && 871 909