[llvm] [llvm] Do not use Console API if the output isn't a console device (PR #90230)

Kacper Michajłow via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 17 14:59:03 PDT 2024


kasper93 wrote:

> If we're outputting directly to a terminal, we set colors using the console APIs - this works since a long time

Correct.

> If we're invoked via ninja, the output is buffered, the output isn't a terminal, and we currently don't output any colors. But this patch changes that.

Correct.

> This patch makes us output ANSI codes instead of using console APIs for color. This has traditionally not worked with older versions of windows terminal, but is supported by recent versions. This is also supported by virtually all unix-like terminals.

To clarify. This patch makes us output ANSI codes instead of using console APIs for color, **only** if we are not outputting to Windows Console. So the current default behavior of using Console API is preserved. This patch changes the default behavior to output ANSI codes, if forced by `-fcolor-diagnostics`, just like on any other platform, when not outputting to Console.

Note that this overwrite ``-fno-ansi-escape-codes` when not outputting to Console, but IMHO this is expected behavior. Else `-fcolor-diagnostics` just doesn't work.

> I'm not entirely following @alvinhochun's comment here - I presume this is about when we're outputting directly to a terminal, whether we should prefer ANSI codes or console APIs for setting color? And that it's possible to query this with ENABLE_VIRTUAL_TERMINAL_PROCESSING?

Yes, this is another topic, that we briefly discussed on IRC. Generally speaking `-f[no-]ansi-escape-codes` is dummy option. In practice there is no need to manually control this.

I've made this patch the most basic to make it convenient from the user point of view.

Possible extension of this changes would be:
* Make ANSI codes default as recommended by Microsoft nowadays
* Fallback to Console API if outputting to Console and `ENABLE_VIRTUAL_TERMINAL_PROCESSING` is disabled.
* Deprecate `-f[no-]ansi-escape-codes` option, as it is not that useful.

I realize the proposed patch is only small change and we may want to more thorough changes, but I wanted to start off discussion about it. I think this patch alone is fine and even further changes can be made in another PR.

https://github.com/llvm/llvm-project/pull/90230


More information about the llvm-commits mailing list