Love this idea. Will take a while to review.

I was looking into whether this is going to to work on Windows. I saw that you can enable Virtual Terminal processing, by doing:

// Set output mode to handle virtual terminal sequences
HANDLE hOut = GetStdHandle(STD_OUTPUT_HANDLE);
DWORD dwMode = 0;
GetConsoleMode(hOut, &dwMode)
dwMode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
SetConsoleMode(hOut, dwMode)

I have not added the code above right now, and rely on detecting color support through the TERM environment variable. I don't know if the code I have for detecting color support is adequate on Windows right now. Maybe @slomp can test this? I wanna be sure this doesn't produce gibberish on Windows.

I was looking into whether this is going to work on Windows.

On Posix-y systems at least, I think you need to check isatty somewhere, in case stdout or stderr are redirected to a file or pipe. On such systems, a function like this should work:

bool supports_ansi(std::ostream &os) {
    if (&os == &std::cout) {
        return isatty(fileno(stdout));
    } else if (&os == &std::cerr) {
        return isatty(fileno(stderr));
    }
    return false;
}

This should be more robust than checking TERM. In this case, you can also check on Windows (using appropriate #ifdefs):

bool supports_ansi(std::ostream &os) {
    HANDLE h;
    if (&os == &std::cout) {
        h = GetStdHandle(STD_OUTPUT_HANDLE);
    } else if (&os == &std::cerr) {
        h = GetStdHandle(STD_ERROR_HANDLE);
    } else {
        return false;
    }

    DWORD mode;
    return GetConsoleMode(h, &mode) &&
           SetConsoleMode(h, mode | ENABLE_VIRTUAL_TERMINAL_PROCESSING);
}

Should be something like that.

I also wonder whether the other ad-hoc if (ansi) checks can be replaced with more dedicated helpers? You could instantiate IRPrinter with one of two tables for the ANSI codes: one has all empty strings and the other has the real ones.

I also wonder whether the other ad-hoc if (ansi) checks can be replaced with more dedicated helpers? You could instantiate IRPrinter with one of two tables for the ANSI codes: one has all empty strings and the other has the real ones.

I thought about alternatives, but decided for this, as this will be 100% correctly branch predicted.

I thought about alternatives, but decided for this, as this will be 100% correctly branch predicted.

Sure, but performance doesn't really matter for the IRPrinter (it's all IO bound anyway) and I'm thinking about saving future headaches if we need to add another case.

You could also template-ize the whole thing and have no branches at all. 🤷

Implemented your suggestion for the tabular constants, and added the Windows-specific code. Thanks for those!

A few things to keep in mind:
If, say, stdout is redirected, and another stream (say, myout) is attached to where the stdout pipe was originally connected to the terminal, and myout is passed to supports_ansi(myout) then the if (&os == &std::cout) will fail, even though isatty(fileno(myout)) is true.

That said, this is catastrophizing to the next level! (but I have seen super weird stream rebindings in production over the years...)

Implemented your suggestion for the tabular constants, and added the Windows-specific code. Thanks for those!

That looks great!

A few things to keep in mind: If, say, stdout is redirected, and another stream (say, myout) is attached to where the stdout pipe was originally connected to the terminal, and myout is passed to supports_ansi(myout) then the if (&os == &std::cout) will fail, even though isatty(fileno(myout)) is true.

That said, this is catastrophizing to the next level! (but I have seen super weird stream rebindings in production over the years...)

In this case, the failure mode would be to not emit ANSI codes, which is always safe. I suppose HL_NO_COLORS could be replaced with a ternary HL_COLORS policy variable where:

• (not defined) means "auto-detect"
• 0 means "do not emit ANSI codes", and
• 1 means "always emit ANSI codes"

Of course, the real failure is of the C++ standards committee to make the std::ostream interface so anemic. 🙄

Thanks @alexreinking and @slomp for the feedback. I implemented this:

In this case, the failure mode would be to not emit ANSI codes, which is always safe. I suppose HL_NO_COLORS could be replaced with a ternary HL_COLORS policy variable where:

• (not defined) means "auto-detect"
• 0 means "do not emit ANSI codes", and
• 1 means "always emit ANSI codes"

I also validated that piping to a file does not produce colors. Overriding with HL_COLORS works.

the if (&os == &std::cout) will fail, even though isatty(fileno(myout)) is true.

There seems to be no way to get the file descriptor from an std::ostream. I guess because it's not guaranteed to go to a file (such as stringstream).

the if (&os == &std::cout) will fail, even though isatty(fileno(myout)) is true.

There seems to be no way to get the file descriptor from an std::ostream. I guess because it's not guaranteed to go to a file (such as stringstream).

Yup, no standard way of doing so, and as you pointed, a stream object does not need to be backed by a file stream or (file-descriptor). It's one of the reasons I tend to steer away from C++ io abstractions.
Anyway, not much we can do about it, I just wanted to point this very rare edge-case, in case you experience that some day.

Dang I switched to another branch to continue work there, and I'm missing this already badly 😝

Does anybody want to add something to this, or can we merge this? @steven-johnson @abadams ?

I think it's safe to merge this. We can always revert if something goes wrong and both @slomp and I have taken a look.

Btw, if you liked this, you might also wanna be aware of the neovim/vim syntax highlighting file I contributed to the Wiki: https://github.com/halide/Halide/wiki/Syntax-highlighting-for-Stmt-files