MaskRay added a comment. Thanks for the detailed description. I am generally conservative when it comes to new use cases for environment variables. However, `NO_COLOR` seems to become a standard and a large number of tools respect it, I think it is the right call to support it.
If we don't intend to support both standards, we can close https://github.com/llvm/llvm-project/issues/23983 (CLICOLOR) as a wontfix :) ================ Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2346 + bool ShowColors = true; + if (std::optional<std::string> NoColor = + llvm::sys::Process::GetEnv("NO_COLOR")) { ---------------- We should inspect that `NO_COLOR` is not empty. > Command-line software which adds ANSI color to its output by default should > check for a NO_COLOR environment variable that, when **present and not an > empty string** (regardless of its value), prevents the addition of ANSI color. ================ Comment at: clang/test/Driver/no-color.c:7 + +// Note, the value of the environment variable does not matter, only that it is defined. +// RUN: env NO_COLOR=0 %clang -### %s 2>&1 | FileCheck --check-prefix=NO-COLOR %s ---------------- when present and not an empty string, the value does not matter. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152285/new/ https://reviews.llvm.org/D152285 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits