Re: [PATCH] status: fix verbose status coloring inconsistency

On Wed, Feb 3, 2021 at 5:51 PM Junio C Hamano [off-list ref] wrote:

"Lance Ward via GitGitGadget" [off-list ref] writes:

quoted

 t/t7527-status-color-pipe.sh | 55 ++++++++++++++++++++++++++++++++++++

Don't we already have test that checks "status" output including its
coloring already? I'd rather see us adding to existing test script,
than allocating a new number for a small subset of features being
tested for a command.  After all, test numbers are limited resource.

t7508 seems like a good place for these tests.

quoted

+test_expect_success 'git status' '
+     git status >raw &&
+     test_decode_color <raw >out &&
+     grep "original$" out
+'

Not "new file: *original$" or something less false-positive prone?

The "new file:" message is localized, so this `grep` will need to
become `test_i18ngrep` again (as it was in the original submission) if
this approach is adopted, which is fine.

quoted

+test_expect_success 'git -c color.status=never status' '
+     git -c color.status=never status >raw &&
+     test_decode_color <raw >out &&
+     grep "original$" out
+'

Would it make sense to have tests for color.status=true, I wonder.
It requires tty to actually "see" the colors output but sending
the output to tty is the normal use case, so we should care...

I wondered if `color.status=true` might already be tested by t7508 but
apparently it only checks `auto`.

quoted

+test_expect_success 'git -c color.status=always status -v' '
+     git -c color.status=always status -v >raw &&
+     test_decode_color <raw >out &&
+     grep "<CYAN>@@ -0,0 +1 @@<RESET>" out &&
+     grep "GREEN>+<RESET><GREEN>1<RESET>" out
+'

Are we forcing the standard palette?

As this is a stand-alone test script with a well-controlled initial
configuration, I expect it would be using the default palette. t7508
does assign a custom palette for `color.status` but not, apparently,
for `color.diff`, so it presumably should be okay there, as well.