aaron.ballman updated this revision to Diff 529299.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.
Update based on review feedback so that we check for an empty environment
variable value.
This was a fun rabbit hole to fall into: lit is not a PTY, so our fallback of
"automatically enable colors" means `env NO_COLOR=` still disables colors as
far as lit tests are concerned. See
https://github.com/llvm/llvm-project/blob/4418434c6de7a861e241ba2448ea4a12080cf08f/clang/test/Driver/color-diagnostics.c#L25
for another test running into this. What's more, Windows does not have a way
to set an empty environment variable on the command line from what I can find,
so testing this manually also doesn't work (at least for me). However, because
this piggy-backs on the functionality for `-fdiagnostics-color=auto`, I think
the test coverage I added is sufficient.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152285/new/
https://reviews.llvm.org/D152285
Files:
clang/docs/ReleaseNotes.rst
clang/docs/UsersManual.rst
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/Driver/no-color.c
Index: clang/test/Driver/no-color.c
===================================================================
--- /dev/null
+++ clang/test/Driver/no-color.c
@@ -0,0 +1,17 @@
+// RUN: env NO_COLOR=1 %clang -### %s 2>&1 | FileCheck --check-prefix=NO-COLOR
%s
+// RUN: env NO_COLOR=1 %clang -fcolor-diagnostics -### %s 2>&1 | FileCheck
--check-prefix=COLOR %s
+// RUN: env NO_COLOR=1 %clang -fdiagnostics-color=auto -### %s 2>&1 |
FileCheck --check-prefix=NO-COLOR %s
+// RUN: env NO_COLOR=1 %clang -fdiagnostics-color=always -### %s 2>&1 |
FileCheck --check-prefix=COLOR %s
+// RUN: env NO_COLOR=1 %clang -fdiagnostics-color=never -### %s 2>&1 |
FileCheck --check-prefix=NO-COLOR %s
+
+// Note, the value of the environment variable does not matter, only that it
is defined and not empty.
+// RUN: env NO_COLOR=0 %clang -### %s 2>&1 | FileCheck --check-prefix=NO-COLOR
%s
+// Note, an empty value means we automatically decide whether to enable colors
or not, and lit tests
+// are not run in a PTY, so colors are disabled by default. There is no easy
way for us to test this
+// configuration where auto enables colors.
+// RUN: env NO_COLOR= %clang -### %s 2>&1 | FileCheck --check-prefix=NO-COLOR
%s
+
+int main(void) {}
+
+// COLOR: -fcolor-diagnostics
+// NO-COLOR-NOT: -fcolor-diagnostics
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2341,10 +2341,20 @@
unsigned MissingArgIndex, MissingArgCount;
InputArgList Args = getDriverOptTable().ParseArgs(
Argv.slice(1), MissingArgIndex, MissingArgCount);
+
+ bool ShowColors = true;
+ if (std::optional<std::string> NoColor =
+ llvm::sys::Process::GetEnv("NO_COLOR");
+ NoColor && !NoColor->empty() && NoColor->at(0) != '\0') {
+ // If the user set the NO_COLOR environment variable, we'll honor that
+ // unless the command line overrides it.
+ ShowColors = false;
+ }
+
// We ignore MissingArgCount and the return value of ParseDiagnosticArgs.
// Any errors that would be diagnosed here will also be diagnosed later,
// when the DiagnosticsEngine actually exists.
- (void)ParseDiagnosticArgs(*DiagOpts, Args);
+ (void)ParseDiagnosticArgs(*DiagOpts, Args, /*Diags=*/nullptr, ShowColors);
return DiagOpts;
}
Index: clang/docs/UsersManual.rst
===================================================================
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -251,6 +251,11 @@
^
//
+ If the ``NO_COLOR`` environment variable is defined and not empty
+ (regardless of value), color diagnostics are disabled. If ``NO_COLOR`` is
+ defined and ``-fcolor-diagnostics`` is passed on the command line, Clang
+ will honor the command line argument.
+
.. option:: -fansi-escape-codes
Controls whether ANSI escape codes are used instead of the Windows Console
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -222,6 +222,8 @@
- Clang now support ``__builtin_FUNCSIG()`` which returns the same information
as the ``__FUNCSIG__`` macro (available only with ``-fms-extensions`` flag).
This fixes (`#58951 <https://github.com/llvm/llvm-project/issues/58951>`_).
+- Clang now supports the `NO_COLOR <https://no-color.org/>`_ environment
+ variable as a way to disable color diagnostics.
New Compiler Flags
------------------
Index: clang/test/Driver/no-color.c
===================================================================
--- /dev/null
+++ clang/test/Driver/no-color.c
@@ -0,0 +1,17 @@
+// RUN: env NO_COLOR=1 %clang -### %s 2>&1 | FileCheck --check-prefix=NO-COLOR %s
+// RUN: env NO_COLOR=1 %clang -fcolor-diagnostics -### %s 2>&1 | FileCheck --check-prefix=COLOR %s
+// RUN: env NO_COLOR=1 %clang -fdiagnostics-color=auto -### %s 2>&1 | FileCheck --check-prefix=NO-COLOR %s
+// RUN: env NO_COLOR=1 %clang -fdiagnostics-color=always -### %s 2>&1 | FileCheck --check-prefix=COLOR %s
+// RUN: env NO_COLOR=1 %clang -fdiagnostics-color=never -### %s 2>&1 | FileCheck --check-prefix=NO-COLOR %s
+
+// Note, the value of the environment variable does not matter, only that it is defined and not empty.
+// RUN: env NO_COLOR=0 %clang -### %s 2>&1 | FileCheck --check-prefix=NO-COLOR %s
+// Note, an empty value means we automatically decide whether to enable colors or not, and lit tests
+// are not run in a PTY, so colors are disabled by default. There is no easy way for us to test this
+// configuration where auto enables colors.
+// RUN: env NO_COLOR= %clang -### %s 2>&1 | FileCheck --check-prefix=NO-COLOR %s
+
+int main(void) {}
+
+// COLOR: -fcolor-diagnostics
+// NO-COLOR-NOT: -fcolor-diagnostics
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2341,10 +2341,20 @@
unsigned MissingArgIndex, MissingArgCount;
InputArgList Args = getDriverOptTable().ParseArgs(
Argv.slice(1), MissingArgIndex, MissingArgCount);
+
+ bool ShowColors = true;
+ if (std::optional<std::string> NoColor =
+ llvm::sys::Process::GetEnv("NO_COLOR");
+ NoColor && !NoColor->empty() && NoColor->at(0) != '\0') {
+ // If the user set the NO_COLOR environment variable, we'll honor that
+ // unless the command line overrides it.
+ ShowColors = false;
+ }
+
// We ignore MissingArgCount and the return value of ParseDiagnosticArgs.
// Any errors that would be diagnosed here will also be diagnosed later,
// when the DiagnosticsEngine actually exists.
- (void)ParseDiagnosticArgs(*DiagOpts, Args);
+ (void)ParseDiagnosticArgs(*DiagOpts, Args, /*Diags=*/nullptr, ShowColors);
return DiagOpts;
}
Index: clang/docs/UsersManual.rst
===================================================================
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -251,6 +251,11 @@
^
//
+ If the ``NO_COLOR`` environment variable is defined and not empty
+ (regardless of value), color diagnostics are disabled. If ``NO_COLOR`` is
+ defined and ``-fcolor-diagnostics`` is passed on the command line, Clang
+ will honor the command line argument.
+
.. option:: -fansi-escape-codes
Controls whether ANSI escape codes are used instead of the Windows Console
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -222,6 +222,8 @@
- Clang now support ``__builtin_FUNCSIG()`` which returns the same information
as the ``__FUNCSIG__`` macro (available only with ``-fms-extensions`` flag).
This fixes (`#58951 <https://github.com/llvm/llvm-project/issues/58951>`_).
+- Clang now supports the `NO_COLOR <https://no-color.org/>`_ environment
+ variable as a way to disable color diagnostics.
New Compiler Flags
------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
