Subject: Re: [PATCH v2] add: check color.ui for interactive add

From: Derrick Stolee <[email-masked]>
Date: Wed, 7 Jun 2023 09:33:58 -0400

On 6/7/2023 7:09 AM, Phillip Wood wrote:
> On 06/06/2023 15:20, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <[email-masked]>
>>
>>       -+    *** 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> status        - show paths with changes
>>       -+    update        - add working tree state to the staged set of changes
>>       -+    revert        - revert staged set of changes back to the HEAD version
>>       -+    patch         - pick hunks and update selectively
>>       -+    diff          - view diff between HEAD and index
>>       -+    add untracked - add contents of untracked files to the staged set of changes
>>       -+    *** 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_cmp actual.raw actual
>>        +'
> 
> I know Junio suggested this change, but I'm in two minds as to whether
> it is a good idea. The reason is that we're no-longer testing that we
> add "[]" around the text that would have been highlighted if color was
> enabled. That is with --color we print "1: status" with the "s" highlighted
> rather than "1: [s]tatus". So while the revised patch tests there is no
> color in the output, it does not test that we print the output correctly
> in that case.

This is a good point, and something that I somewhat expected to find as
an example check in the rest of the test script. I think the color would
be disabled if we didn't do force_color, even before this fix.

However, I'll double-check that by adding a separate test for just that
bit of the UI, keeping this color.ui=false test focused on the color side
of things.

Thanks,
-Stolee