On 6/7/2023 7:09 AM, Phillip Wood wrote: > On 06/06/2023 15:20, Derrick Stolee via GitGitGadget wrote: >> From: Derrick Stolee <derrickstolee@xxxxxxxxxx> >> >> -+ *** 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