From 7cf3b49f47f02ed1cab5b1cd03a5e27acaa13c99 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 6 Jun 2023 14:20:18 +0000 Subject: [PATCH 1/2] add: check color.ui for interactive add When 'git add -i' and 'git add -p' were converted to a builtin, they introduced a color bug: the 'color.ui' config setting is ignored. The included test demonstrates an example that is similar to the previous test, which focuses on customizing colors. Here, we are demonstrating that colors are not being used at all by comparing the raw output and the color-decoded version of that output. The fix is simple, to use git_color_default_config() as the fallback for git_add_config(). A more robust change would instead encapsulate the git_use_color_default global in methods that would check the config setting if it has not been initialized yet. Some ideas are being discussed on this front [1], but nothing has been finalized. [1] https://lore.kernel.org/git/pull.1539.git.1685716420.gitgitgadget@gmail.com/ This test case naturally bisects down to 0527ccb1b55 (add -i: default to the built-in implementation, 2021-11-30), but the fix makes it clear that this would be broken even if we added the config to use the builtin earlier than this. Reported-by: Greg Alexander Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/add.c | 2 +- t/t3701-add-interactive.sh | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/builtin/add.c b/builtin/add.c index 61dd386d109b37..3e13983c463d7e 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -362,7 +362,7 @@ static int add_config(const char *var, const char *value, void *cb) return 0; } - return git_default_config(var, value, cb); + return git_color_default_config(var, value, cb); } static const char embedded_advice[] = N_( diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 3a99837d9b15f7..99192d76a3d185 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -738,6 +738,21 @@ test_expect_success 'colors can be overridden' ' test_cmp expect actual ' +test_expect_success 'colors can be skipped with color.ui=false' ' + git reset --hard && + test_when_finished "git rm -f color-test" && + test_write_lines context old more-context >color-test && + git add color-test && + test_write_lines context new more-context another-one >color-test && + + test_write_lines help quit >input && + force_color git \ + -c color.ui=false \ + add -i >actual.raw actual && + test_cmp actual.raw actual +' + test_expect_success 'colorized diffs respect diff.wsErrorHighlight' ' git reset --hard && From 6f74648cea1ada4dd0add46b2ae038606bb47395 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Wed, 7 Jun 2023 09:44:48 -0400 Subject: [PATCH 2/2] add: test use of brackets when color is disabled From 02156b81bbb2cafb19d702c55d45714fcf224048 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Wed, 7 Jun 2023 09:39:01 -0400 Subject: [PATCH v2 2/2] add: test use of brackets when color is disabled The interactive add command, 'git add -i', displays a menu of options using full words. When color is enabled, the first letter of each word is changed to a highlight color to signal that the first letter could be used as a command. Without color, brackets ("[]") are used around these first letters. This behavior was not previously tested directly in t3701, so add a test for it now. Since we use 'git add -i >actual Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- t/t3701-add-interactive.sh | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 99192d76a3d185..f22e74e6e4829e 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -738,6 +738,29 @@ test_expect_success 'colors can be overridden' ' test_cmp expect actual ' +test_expect_success 'brackets appear without color' ' + git reset --hard && + test_when_finished "git rm -f bracket-test" && + test_write_lines context old more-context >bracket-test && + git add bracket-test && + test_write_lines context new more-context another-one >bracket-test && + + test_write_lines quit >input && + git add -i >actual expect <<-\EOF && + | staged unstaged path + | 1: +3/-0 +2/-1 bracket-test + | + |*** Commands *** + | 1: [s]tatus 2: [u]pdate 3: [r]evert 4: [a]dd untracked + | 5: [p]atch 6: [d]iff 7: [q]uit 8: [h]elp + |What now> Bye. + EOF + + test_cmp expect actual +' + test_expect_success 'colors can be skipped with color.ui=false' ' git reset --hard && test_when_finished "git rm -f color-test" &&