Propose strategy for dealing with color flags

[wswoodruff]

trafficstars

Not sure how I feel about what I came up with here:

• Looking up the command definitions and looking for [*] to determine if it's a meta flag or not. In my brain I'm saying meta flags are flags that can be applied to any command, and I'm removing them from the equation for the rest of the logic so no code changes are needed in specific command paths besides referencing argsSansMetaFlags instead of extraArgs.
  • I didn't want to have 2 places where these special flags are defined so that's why I decided to lookup by the description. Lemme know what you think!

After approach is decided, I can write some tests to bring it back up to 100% coverage.

[coveralls]

Coverage remained the same at 100.0% when pulling 7d8080b5fa7f5e821fc1e91b1d0991463d62e08a on wswoodruff:color-and-no-color-flags into dae9481c7adc13cc6ce90c7241e82efea41cb497 on hapipal:master.

[devinivy]

Hey, thanks for this! I appreciate you moving the ball forward on this long-standing issue.

Here's the basic acceptance criteria:

• [x] Providing --no-color or --color as an argument should always be allowed.
• [x] The run command should still receive these arguments.
• [x] All other commands should move ahead as though those arguments have already been processed.

I think you nailed all these just by adding --color and --no-color to the bossy config. I believe the logic to exclude those flags from extraArgs is already taken care of after that. So you may be able to cut this concept of meta flags and a handful of lines of code. I also wouldn't worry about handling the case that both flags are specified at the same time— I'm fine saying that it's the supports-color module's job to deal with that. Even providing an error message will happen in color or not, so there's really no way you can win, ha! Curious what you think of all this. Thanks again for moving it forward :)

[wswoodruff]

So any bossy flags will be stripped out if u grab args from args._ — but here we're grabbing options.argv for run commands — https://github.com/hapipal/hpal/blob/master/lib/index.js#L23

Should we grab args._ instead? Looks like in the run command we're piecing through the args, should we change that part?

[devinivy]

The way it currently works is intentional: run commands should receive all arguments, and all other commands should only receive arguments that weren't processed by e.g. bossy.

[wswoodruff]

Left off attempting to write tests for the --color and --no-color flags by trying to see how we can prove color is or isn't there. So I was looking at how chalk does it and that's as far as I got https://github.com/chalk/chalk/blob/main/test/chalk.js

[devinivy]

@wswoodruff one way to do it could be to use StripAnsi(value) === value. If it is true the value with and without ansi is the same, so it must not have contained any ansi :) I am pretty sure the strip-ansi package is already used in the test suite.

[devinivy]

By the way, if you add some tests around that I think you'll want to add it to the bin section of the tests, since I believe that is where supports-color is actually used: https://github.com/hapipal/hpal/blob/master/test/index.js#L1626