sd

sd copied to clipboard

Currently our support for terminal coloring is pretty spotty. Ideally we would

• [ ] Migrate away from ansi_term since it's been unmaintained for quite a while
• [ ] Respect NO_COLOR and FORCE_COLOR (most coloring libraries support this out-of-the-box these days)
• [ ] Add a --colo[u]r <when> flag where <when> can be always|auto|never
• [ ] Properly handle either stdout or stderr independently being a tty
  • A lot of programs naively assume that if stdout is a tty then so is stderr and vice-versa. We should appropriately handle either one being a tty (*cough cough* use something like anstream *cough*)

I was planning on looking into anstream and owo-colors, but anything that can handle all of the above should work just fine

Oh this is a rather comprehensive listing. I think I can take this with such detail.

Feel free to take up the work if ya want! I'd be happy to review a PR for it

There's a kind of chicken and eggs problem: if we want a --color argument, it obviously goes through clap, but clap itself doesn't use that. If it is to be supported for clap, we are gonna dig deep into clap internals.

we are gonna dig deep into clap internals.

Fortunately doesn't look like we need to dig very deep

https://docs.rs/clap/latest/clap/enum.ColorChoice.html

There just may need to be some extra logic to determine that choice, but passing it to clap is straightforward

Not completely, there's at least an ClapError::unknown_argument that goes before we can Command::color(). It's constructed and shown during (or right after) the parsing phase, before you can get the value of --color.

I think it's reasonable enough to ignore --color if there's a CLI parsing failure

Alright, it's just mildly annoying to be partially inconsistent.

Besides, IMO --color takes precedence over NO_COLOR & co., because the latter cover more area.

I agree it would be normal for --color to have higher precedence

I don't want to do any "permissive parsing" of the CLI args though. If there was a failure parsing any of the args then we discard all of that information

Sure, current usage is just that, clap would handle parsing failure itself and exit before we get the results.