Git fork

Merge branch 'jk/add-i-color' into maint-2.51

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

+148 -40
+53 -31
add-interactive.c
··· 20 #include "prompt.h" 21 #include "tree.h" 22 23 - static void init_color(struct repository *r, struct add_i_state *s, 24 const char *section_and_slot, char *dst, 25 const char *default_color) 26 { 27 char *key = xstrfmt("color.%s", section_and_slot); 28 const char *value; 29 30 - if (!s->use_color) 31 dst[0] = '\0'; 32 else if (repo_config_get_value(r, key, &value) || 33 color_parse(value, dst)) ··· 36 free(key); 37 } 38 39 - void init_add_i_state(struct add_i_state *s, struct repository *r, 40 - struct add_p_opt *add_p_opt) 41 { 42 const char *value; 43 44 s->r = r; 45 s->context = -1; 46 s->interhunkcontext = -1; 47 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); 54 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); 61 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"); 65 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); 75 76 FREE_AND_NULL(s->interactive_diff_filter); 77 repo_config_get_string(r, "interactive.difffilter", ··· 109 FREE_AND_NULL(s->interactive_diff_filter); 110 FREE_AND_NULL(s->interactive_diff_algorithm); 111 memset(s, 0, sizeof(*s)); 112 - s->use_color = -1; 113 } 114 115 /* ··· 1188 * When color was asked for, use the prompt color for 1189 * highlighting, otherwise use square brackets. 1190 */ 1191 - if (s.use_color) { 1192 data.color = s.prompt_color; 1193 - data.reset = s.reset_color; 1194 } 1195 print_file_item_data.color = data.color; 1196 print_file_item_data.reset = data.reset;
··· 20 #include "prompt.h" 21 #include "tree.h" 22 23 + static void init_color(struct repository *r, int use_color, 24 const char *section_and_slot, char *dst, 25 const char *default_color) 26 { 27 char *key = xstrfmt("color.%s", section_and_slot); 28 const char *value; 29 30 + if (!use_color) 31 dst[0] = '\0'; 32 else if (repo_config_get_value(r, key, &value) || 33 color_parse(value, dst)) ··· 36 free(key); 37 } 38 39 + static int check_color_config(struct repository *r, const char *var) 40 { 41 const char *value; 42 + int ret; 43 44 + if (repo_config_get_value(r, var, &value)) 45 + ret = -1; 46 + else 47 + ret = git_config_colorbool(var, value); 48 + 49 + /* 50 + * Do not rely on want_color() to fall back to color.ui for us. It uses 51 + * the value parsed by git_color_config(), which may not have been 52 + * called by the main command. 53 + */ 54 + if (ret < 0 && !repo_config_get_value(r, "color.ui", &value)) 55 + ret = git_config_colorbool("color.ui", value); 56 + 57 + return want_color(ret); 58 + } 59 + 60 + void init_add_i_state(struct add_i_state *s, struct repository *r, 61 + struct add_p_opt *add_p_opt) 62 + { 63 s->r = r; 64 s->context = -1; 65 s->interhunkcontext = -1; 66 67 + s->use_color_interactive = check_color_config(r, "color.interactive"); 68 69 + init_color(r, s->use_color_interactive, "interactive.header", 70 + s->header_color, GIT_COLOR_BOLD); 71 + init_color(r, s->use_color_interactive, "interactive.help", 72 + s->help_color, GIT_COLOR_BOLD_RED); 73 + init_color(r, s->use_color_interactive, "interactive.prompt", 74 + s->prompt_color, GIT_COLOR_BOLD_BLUE); 75 + init_color(r, s->use_color_interactive, "interactive.error", 76 + s->error_color, GIT_COLOR_BOLD_RED); 77 + strlcpy(s->reset_color_interactive, 78 + s->use_color_interactive ? GIT_COLOR_RESET : "", COLOR_MAXLEN); 79 80 + s->use_color_diff = check_color_config(r, "color.diff"); 81 + 82 + init_color(r, s->use_color_diff, "diff.frag", s->fraginfo_color, 83 + diff_get_color(s->use_color_diff, DIFF_FRAGINFO)); 84 + init_color(r, s->use_color_diff, "diff.context", s->context_color, 85 + "fall back"); 86 if (!strcmp(s->context_color, "fall back")) 87 + init_color(r, s->use_color_diff, "diff.plain", 88 + s->context_color, 89 + diff_get_color(s->use_color_diff, DIFF_CONTEXT)); 90 + init_color(r, s->use_color_diff, "diff.old", s->file_old_color, 91 + diff_get_color(s->use_color_diff, DIFF_FILE_OLD)); 92 + init_color(r, s->use_color_diff, "diff.new", s->file_new_color, 93 + diff_get_color(s->use_color_diff, DIFF_FILE_NEW)); 94 + strlcpy(s->reset_color_diff, 95 + s->use_color_diff ? GIT_COLOR_RESET : "", COLOR_MAXLEN); 96 97 FREE_AND_NULL(s->interactive_diff_filter); 98 repo_config_get_string(r, "interactive.difffilter", ··· 130 FREE_AND_NULL(s->interactive_diff_filter); 131 FREE_AND_NULL(s->interactive_diff_algorithm); 132 memset(s, 0, sizeof(*s)); 133 + s->use_color_interactive = -1; 134 + s->use_color_diff = -1; 135 } 136 137 /* ··· 1210 * When color was asked for, use the prompt color for 1211 * highlighting, otherwise use square brackets. 1212 */ 1213 + if (s.use_color_interactive) { 1214 data.color = s.prompt_color; 1215 + data.reset = s.reset_color_interactive; 1216 } 1217 print_file_item_data.color = data.color; 1218 print_file_item_data.reset = data.reset;
+5 -2
add-interactive.h
··· 12 13 struct add_i_state { 14 struct repository *r; 15 - int use_color; 16 char header_color[COLOR_MAXLEN]; 17 char help_color[COLOR_MAXLEN]; 18 char prompt_color[COLOR_MAXLEN]; 19 char error_color[COLOR_MAXLEN]; 20 - char reset_color[COLOR_MAXLEN]; 21 char fraginfo_color[COLOR_MAXLEN]; 22 char context_color[COLOR_MAXLEN]; 23 char file_old_color[COLOR_MAXLEN]; 24 char file_new_color[COLOR_MAXLEN]; 25 26 int use_single_key; 27 char *interactive_diff_filter, *interactive_diff_algorithm;
··· 12 13 struct add_i_state { 14 struct repository *r; 15 + int use_color_interactive; 16 + int use_color_diff; 17 char header_color[COLOR_MAXLEN]; 18 char help_color[COLOR_MAXLEN]; 19 char prompt_color[COLOR_MAXLEN]; 20 char error_color[COLOR_MAXLEN]; 21 + char reset_color_interactive[COLOR_MAXLEN]; 22 + 23 char fraginfo_color[COLOR_MAXLEN]; 24 char context_color[COLOR_MAXLEN]; 25 char file_old_color[COLOR_MAXLEN]; 26 char file_new_color[COLOR_MAXLEN]; 27 + char reset_color_diff[COLOR_MAXLEN]; 28 29 int use_single_key; 30 char *interactive_diff_filter, *interactive_diff_algorithm;
+6 -6
add-patch.c
··· 300 va_start(args, fmt); 301 fputs(s->s.error_color, stdout); 302 vprintf(fmt, args); 303 - puts(s->s.reset_color); 304 va_end(args); 305 } 306 ··· 457 } 458 strbuf_complete_line(plain); 459 460 - if (want_color_fd(1, -1)) { 461 struct child_process colored_cp = CHILD_PROCESS_INIT; 462 const char *diff_filter = s->s.interactive_diff_filter; 463 ··· 714 if (len) 715 strbuf_add(out, p, len); 716 else if (colored) 717 - strbuf_addf(out, "%s\n", s->s.reset_color); 718 else 719 strbuf_addch(out, '\n'); 720 } ··· 1107 s->s.file_new_color : 1108 s->s.context_color); 1109 strbuf_add(&s->colored, plain + current, eol - current); 1110 - strbuf_addstr(&s->colored, s->s.reset_color); 1111 if (next > eol) 1112 strbuf_add(&s->colored, plain + eol, next - eol); 1113 current = next; ··· 1528 : 1)); 1529 printf(_(s->mode->prompt_mode[prompt_mode_type]), 1530 s->buf.buf); 1531 - if (*s->s.reset_color) 1532 - fputs(s->s.reset_color, stdout); 1533 fflush(stdout); 1534 if (read_single_character(s) == EOF) 1535 break;
··· 300 va_start(args, fmt); 301 fputs(s->s.error_color, stdout); 302 vprintf(fmt, args); 303 + puts(s->s.reset_color_interactive); 304 va_end(args); 305 } 306 ··· 457 } 458 strbuf_complete_line(plain); 459 460 + if (want_color_fd(1, s->s.use_color_diff)) { 461 struct child_process colored_cp = CHILD_PROCESS_INIT; 462 const char *diff_filter = s->s.interactive_diff_filter; 463 ··· 714 if (len) 715 strbuf_add(out, p, len); 716 else if (colored) 717 + strbuf_addf(out, "%s\n", s->s.reset_color_diff); 718 else 719 strbuf_addch(out, '\n'); 720 } ··· 1107 s->s.file_new_color : 1108 s->s.context_color); 1109 strbuf_add(&s->colored, plain + current, eol - current); 1110 + strbuf_addstr(&s->colored, s->s.reset_color_diff); 1111 if (next > eol) 1112 strbuf_add(&s->colored, plain + eol, next - eol); 1113 current = next; ··· 1528 : 1)); 1529 printf(_(s->mode->prompt_mode[prompt_mode_type]), 1530 s->buf.buf); 1531 + if (*s->s.reset_color_interactive) 1532 + fputs(s->s.reset_color_interactive, stdout); 1533 fflush(stdout); 1534 if (read_single_character(s) == EOF) 1535 break;
+4 -1
builtin/stash.c
··· 377 * however it should be done together with apply_cached. 378 */ 379 cp.git_cmd = 1; 380 - strvec_pushl(&cp.args, "diff-tree", "--binary", NULL); 381 strvec_pushf(&cp.args, "%s^2^..%s^2", w_commit_hex, w_commit_hex); 382 383 return pipe_command(&cp, NULL, 0, out, 0, NULL, 0); ··· 1283 1284 cp_diff_tree.git_cmd = 1; 1285 strvec_pushl(&cp_diff_tree.args, "diff-tree", "-p", "--binary", 1286 "-U1", "HEAD", oid_to_hex(&info->w_tree), "--", NULL); 1287 if (pipe_command(&cp_diff_tree, NULL, 0, out_patch, 0, NULL, 0)) { 1288 ret = -1; ··· 1345 1346 cp_diff_tree.git_cmd = 1; 1347 strvec_pushl(&cp_diff_tree.args, "diff-tree", "-p", "-U1", "HEAD", 1348 oid_to_hex(&info->w_tree), "--", NULL); 1349 if (pipe_command(&cp_diff_tree, NULL, 0, out_patch, 0, NULL, 0)) { 1350 ret = -1; ··· 1719 1720 cp_diff.git_cmd = 1; 1721 strvec_pushl(&cp_diff.args, "diff-index", "-p", 1722 "--cached", "--binary", "HEAD", "--", 1723 NULL); 1724 add_pathspecs(&cp_diff.args, ps);
··· 377 * however it should be done together with apply_cached. 378 */ 379 cp.git_cmd = 1; 380 + strvec_pushl(&cp.args, "diff-tree", "--binary", "--no-color", NULL); 381 strvec_pushf(&cp.args, "%s^2^..%s^2", w_commit_hex, w_commit_hex); 382 383 return pipe_command(&cp, NULL, 0, out, 0, NULL, 0); ··· 1283 1284 cp_diff_tree.git_cmd = 1; 1285 strvec_pushl(&cp_diff_tree.args, "diff-tree", "-p", "--binary", 1286 + "--no-color", 1287 "-U1", "HEAD", oid_to_hex(&info->w_tree), "--", NULL); 1288 if (pipe_command(&cp_diff_tree, NULL, 0, out_patch, 0, NULL, 0)) { 1289 ret = -1; ··· 1346 1347 cp_diff_tree.git_cmd = 1; 1348 strvec_pushl(&cp_diff_tree.args, "diff-tree", "-p", "-U1", "HEAD", 1349 + "--no-color", 1350 oid_to_hex(&info->w_tree), "--", NULL); 1351 if (pipe_command(&cp_diff_tree, NULL, 0, out_patch, 0, NULL, 0)) { 1352 ret = -1; ··· 1721 1722 cp_diff.git_cmd = 1; 1723 strvec_pushl(&cp_diff.args, "diff-index", "-p", 1724 + "--no-color", 1725 "--cached", "--binary", "HEAD", "--", 1726 NULL); 1727 add_pathspecs(&cp_diff.args, ps);
+8
contrib/diff-highlight/README
··· 58 diff = diff-highlight | less 59 --------------------------------------------- 60 61 62 Color Config 63 ------------
··· 58 diff = diff-highlight | less 59 --------------------------------------------- 60 61 + If you use the interactive patch mode of `git add -p`, `git checkout 62 + -p`, etc, you may also want to configure it to be used there: 63 + 64 + --------------------------------------------- 65 + [interactive] 66 + diffFilter = diff-highlight 67 + --------------------------------------------- 68 + 69 70 Color Config 71 ------------
+53
t/t3701-add-interactive.sh
··· 866 test_grep "old<" output 867 ' 868 869 test_expect_success 'diffFilter filters diff' ' 870 git reset --hard && 871 ··· 1283 test_grep "@@ -2,20 +2,20 @@" actual 1284 ' 1285 1286 for cmd in add checkout commit reset restore "stash save" "stash push" 1287 do 1288 test_expect_success "$cmd rejects invalid context options" ' ··· 1298 1299 test_must_fail git $cmd --inter-hunk-context 2 2>actual && 1300 test_grep -E ".--inter-hunk-context. requires .(--interactive/)?--patch." actual 1301 ' 1302 done 1303
··· 866 test_grep "old<" output 867 ' 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 + 907 test_expect_success 'diffFilter filters diff' ' 908 git reset --hard && 909 ··· 1321 test_grep "@@ -2,20 +2,20 @@" actual 1322 ' 1323 1324 + test_expect_success 'set up base for -p color tests' ' 1325 + echo commit >file && 1326 + git commit -am "commit state" && 1327 + git tag patch-base 1328 + ' 1329 + 1330 for cmd in add checkout commit reset restore "stash save" "stash push" 1331 do 1332 test_expect_success "$cmd rejects invalid context options" ' ··· 1342 1343 test_must_fail git $cmd --inter-hunk-context 2 2>actual && 1344 test_grep -E ".--inter-hunk-context. requires .(--interactive/)?--patch." actual 1345 + ' 1346 + 1347 + test_expect_success "$cmd falls back to color.ui" ' 1348 + git reset --hard patch-base && 1349 + echo working-tree >file && 1350 + test_write_lines y | 1351 + force_color git -c color.ui=false $cmd -p >output.raw 2>&1 && 1352 + test_decode_color <output.raw >output && 1353 + test_cmp output.raw output 1354 ' 1355 done 1356
+19
t/t3904-stash-patch.sh
··· 107 ! grep "added line 2" test 108 ' 109 110 test_done
··· 107 ! grep "added line 2" test 108 ' 109 110 + test_expect_success 'stash -p not confused by GIT_PAGER_IN_USE' ' 111 + echo to-stash >test && 112 + # Set both GIT_PAGER_IN_USE and TERM. Our goal is to entice any 113 + # diff subprocesses into thinking that they could output 114 + # color, even though their stdout is not going into a tty. 115 + echo y | 116 + GIT_PAGER_IN_USE=1 TERM=vt100 git stash -p && 117 + git diff --exit-code 118 + ' 119 + 120 + test_expect_success 'index push not confused by GIT_PAGER_IN_USE' ' 121 + echo index >test && 122 + git add test && 123 + echo working-tree >test && 124 + # As above, we try to entice the child diff into using color. 125 + GIT_PAGER_IN_USE=1 TERM=vt100 git stash push test && 126 + git diff --exit-code 127 + ' 128 + 129 test_done