fix(handler): do not call supports_color::on when color is forced

[Boshen]

This reduces a sys call when color is forced.

I'm not sure whether the logic after the change is correct or not, but the documentation seems to support it?

https://docs.rs/miette/latest/miette/struct.MietteHandlerOpts.html#method.color

If true, colors will be used during graphical rendering, regardless of whether or not the terminal supports them.

If false, colors will never be used.

If unspecified, colors will be used only if the terminal supports them.

[olivia-fl]

The existing code needs to call supports_color::on even when color = Some(true) to check for truecolor support (this is the RgbColors::Preferred branch). The current change in this PR would switch the behavior when the terminal supports truecolor, color is forced, but RGB is set to Preferred from truecolor to 16-color ANSI. It looks like there isn't a unit test for this case, unfortunately.

In theory, you could skip the supports_color::on call when color == Some(true) && rgb_colors != RgbColors::Preferred. What's the reasoning behind wanting to eliminate the syscall? Is the perf impact significant?

[Boshen]

It's not significant but observable when many threads try to initialize the hook

miette/src/eyreish/mod.rs

Line 65 in b074446

static HOOK: OnceCell<ErrorHook> = OnceCell::new();

It led me to this discovery when I tried to initialize the hook in main and discovered it still calls supports_color when I tried to force the color.

[olivia-fl]

Hmm... I'm not sure I understand what your use-case looks like. set_hook shouldn't be calling support_color::on by itself. In typical usage, the hook will be called for each diagnostic and will construct a fresh handler each time. This would generate a separate supports_color::on for each diagnostic. You're saying that the supports_color::on call noticeably reduces the rate that your application can format a large number of errors?

If handler construction is a bottleneck, I might go for something like this:

fn main() {
    let handler = GraphicalReportHandler::new(); // set options here
    set_hook(Box::new(move |_| {
        // could probably avoid the clone if we had something like
        //   impl<T: Deref<Target: ReportHandler>> ReportHandler for T
        // but I would be *very* surprised if the memcpy overhead is significant...
        Box::new(handler.clone()))
    }));
}

It led me to this discovery when I tried to initialize the hook in main and discovered it still calls supports_color when I tried to force the color.

It sounds like this is similar what you're doing?

---

For reducing the cost of creating a large number of graphical handlers, we could skip the supports_color::on call when color == Some(true) && rgb_colors != RgbColors::Preferred, which is pretty close to what this PR is already doing.

We could also experiment with caching the return value in a static OnceCell (or maybe a Mutex<Option<_>> if racing the first batch of calls turns out to be bad). If there are cases where color support changes over the lifetime of a program this could run into problems, but I really hope not.