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

[Eric Sunshine <sunshine@sunshineco.com>]

On Wed, Feb 3, 2021 at 5:51 PM Junio C Hamano <gitster@pobox.com> wrote:
> "Lance Ward via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >  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.

> > +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.

> > +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`.

> > +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.