Support forcing color output #411
Conversation
Add support for the FORCE_COLOR environment variable. It is possible to run in an environment that does not look like either a TTY or a named pipe and still want ANSI escape codes emitted. The NO_COLOR environment variable takes precedence. - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
plemarquand
requested review from
stmontgomery,
grynspan,
dennisweissmann,
briancroom,
SeanROlszewski and
suzannaratcliff
as code owners
|
Please update the markup for |
grynspan
added
enhancement
| if result.useANSIEscapeCodes { | ||
|
|
||
| // Respect the FORCE_COLOR environment variable. SEE: https://www.force-color.org | ||
| if result.useANSIEscapeCodes || Environment.variable(named: "FORCE_COLOR")?.isEmpty == false { |
There was a problem hiding this comment.
What is the intended interaction between FORCE_COLOR and NO_COLOR if both are specified?
There was a problem hiding this comment.
The NO_COLOR environment variable takes precedence.
There was a problem hiding this comment.
I appreciate the intent here, but NO_COLOR is supported specifically for the benefit of SwiftPM which uses pipes rather than TTYs to pass data along to/from swift test. FORCE_COLOR is not widely supported and the interactions with NO_COLOR don't seem to be defined.
Rather than adding another environment variable check, this seems like something we could pass as a command-line argument instead, e.g. --color or --enable-color or some such.
|
My thoughts are supporting only Adding another CLI flag would mean plumbing it through SwiftPM to the swift-testing binary. An environment variable lets anyone opt in all the way down the chain. There is some debate around whether to use |
|
Neither |
On [FreeBSD](https://man.freebsd.org/cgi/man.cgi?query=posix_spawn_file_actions_adddup2), [OpenBSD](https://github.com/openbsd/src/blob/master/lib/libc/gen/posix_spawn.c#L155), [Android](https://android.googlesource.com/platform/bionic/+/master/libc/bionic/spawn.cpp#103), and [Glibc ≥ 2.29](https://sourceware.org/bugzilla/show_bug.cgi?id=23640), `posix_spawn_file_actions_adddup2()` automatically clears `FD_CLOEXEC` if the file descriptors passed to it are equal. Relying on this behaviour eliminates a race condition when spawning child processes. This functionality is standardized in [POSIX.1-2024](https://pubs.opengroup.org/onlinepubs/9799919799/) thanks to [Austin Group Defect #411](https://www.austingroupbugs.net/view.php?id=411). Some older Linuxes (Amazon Linux 2 in particular) don't have this functionality, so we do a runtime check of the Glibc version. This PR is a follow-up to #1145. Resolves #1140. ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
Add support for the
FORCE_COLORenvironment variable. It is possible to run in an environment that does not look like either a TTY or a named pipe and still want ANSI escape codes emitted. TheNO_COLORenvironment variable takes precedence.Checklist: