fix: check if it is a Real terminal, not just if it is a terminal

This will stop printing stuff to dumb terminals that they don't support.

I've overall audited usage of isatty and replaced the ones with intent
to mean "is a Real terminal" with checking for that. I've also caught a
case of carelessly assuming "is a tty" means "should be colour" in
nix-env.

Change-Id: I6d83725d9a2d932ac94ff2294f92c0a1100d23c9
This commit is contained in:
Jade Lovelace 2024-08-10 16:02:42 -07:00
parent 3775b6ac88
commit 292567e0b0
7 changed files with 50 additions and 15 deletions

View file

@ -5,9 +5,9 @@
#include "signals.hh"
#include "loggers.hh"
#include "current-process.hh"
#include "terminal.hh"
#include <algorithm>
#include <cctype>
#include <exception>
#include <iostream>
@ -347,7 +347,7 @@ int handleExceptions(const std::string & programName, std::function<void()> fun)
RunPager::RunPager()
{
if (!isatty(STDOUT_FILENO)) return;
if (!isOutputARealTerminal(StandardOutputStream::Stdout)) return;
char * pager = getenv("NIX_PAGER");
if (!pager) pager = getenv("PAGER");
if (pager && ((std::string) pager == "" || (std::string) pager == "cat")) return;

View file

@ -37,7 +37,15 @@ void Logger::warn(const std::string & msg)
void Logger::writeToStdout(std::string_view s)
{
writeFull(STDOUT_FILENO, filterANSIEscapes(s, !shouldANSI(), std::numeric_limits<unsigned int>::max(), false));
writeFull(
STDOUT_FILENO,
filterANSIEscapes(
s,
!shouldANSI(StandardOutputStream::Stdout),
std::numeric_limits<unsigned int>::max(),
false
)
);
writeFull(STDOUT_FILENO, "\n");
}

View file

@ -7,7 +7,12 @@
namespace nix {
bool shouldANSI()
bool isOutputARealTerminal(StandardOutputStream fileno)
{
return isatty(int(fileno)) && getEnv("TERM").value_or("dumb") != "dumb";
}
bool shouldANSI(StandardOutputStream fileno)
{
// Implements the behaviour described by https://bixense.com/clicolors/
// As well as https://force-color.org/ for compatibility, since it fits in the same shape.
@ -16,14 +21,14 @@ bool shouldANSI()
// unset x set Yes
// unset x unset If attached to a terminal
// [we choose the "modern" approach of colour-by-default]
auto compute = []() -> bool {
auto compute = [](StandardOutputStream fileno) -> bool {
bool mustNotColour = getEnv("NO_COLOR").has_value() || getEnv("NOCOLOR").has_value();
bool shouldForce = getEnv("CLICOLOR_FORCE").has_value() || getEnv("FORCE_COLOR").has_value();
bool isTerminal = isatty(STDERR_FILENO) && getEnv("TERM").value_or("dumb") != "dumb";
bool isTerminal = isOutputARealTerminal(fileno);
return !mustNotColour && (shouldForce || isTerminal);
};
static bool cached = compute();
return cached;
static bool cached[2] = {compute(StandardOutputStream::Stdout), compute(StandardOutputStream::Stderr)};
return cached[int(fileno) - 1];
}
// FIXME(jade): replace with TerminalCodeEater. wowie this is evil code.

View file

@ -6,6 +6,26 @@
namespace nix {
enum class StandardOutputStream {
Stdout = 1,
Stderr = 2,
};
/**
* Determine whether the output is a real terminal (i.e. not dumb, not a pipe).
*
* This is probably not what you want, you may want shouldANSI() or something
* more specific. Think about how the output should work with a pager or
* entirely non-interactive scripting use.
*
* The user may be redirecting the Lix output to a pager, but have stderr
* connected to a terminal. Think about where you are outputting the text when
* deciding whether to use STDERR_FILENO or STDOUT_FILENO.
*
* \param fileno file descriptor number to check if it is a tty
*/
bool isOutputARealTerminal(StandardOutputStream fileno);
/**
* Determine whether ANSI escape sequences are appropriate for the
* present output.
@ -18,8 +38,10 @@ namespace nix {
* - CLICOLOR_FORCE or FORCE_COLOR set -> enable colour
* - The output is a tty; TERM != "dumb" -> enable colour
* - Otherwise -> disable colour
*
* \param fileno which file descriptor number to consider. Use the one you are outputting to
*/
bool shouldANSI();
bool shouldANSI(StandardOutputStream fileno = StandardOutputStream::Stderr);
/**
* Truncate a string to 'width' printable characters. If 'filterAll'

View file

@ -1,6 +1,7 @@
#include "attr-path.hh"
#include "common-eval-args.hh"
#include "derivations.hh"
#include "terminal.hh"
#include "eval.hh"
#include "get-drvs.hh"
#include "globals.hh"
@ -17,7 +18,6 @@
#include "legacy.hh"
#include "eval-settings.hh" // for defexpr
#include <cerrno>
#include <ctime>
#include <algorithm>
#include <iostream>
@ -1092,7 +1092,6 @@ static void opQuery(Globals & globals, Strings opFlags, Strings opArgs)
return;
}
bool tty = isatty(STDOUT_FILENO);
RunPager pager;
Table table;
@ -1171,7 +1170,7 @@ static void opQuery(Globals & globals, Strings opFlags, Strings opArgs)
}
} else {
auto column = (std::string) "" + ch + " " + version;
if (diff == cvGreater && tty)
if (diff == cvGreater && shouldANSI(StandardOutputStream::Stdout))
column = ANSI_RED + column + ANSI_NORMAL;
columns.push_back(column);
}

View file

@ -362,6 +362,7 @@ void mainWrapped(int argc, char * * argv)
setLogFormat(LogFormat::bar);
Finally f([] { logger->pause(); });
settings.verboseBuild = false;
// FIXME: stop messing about with log verbosity depending on if it is interactive use
if (isatty(STDERR_FILENO)) {
verbosity = lvlNotice;
} else {

View file

@ -4,11 +4,11 @@
#include "shared.hh"
#include "store-api.hh"
#include "filetransfer.hh"
#include "finally.hh"
#include "tarfile.hh"
#include "attr-path.hh"
#include "eval-inline.hh"
#include "eval-inline.hh" // IWYU pragma: keep
#include "legacy.hh"
#include "terminal.hh"
#include <nlohmann/json.hpp>
@ -180,7 +180,7 @@ static int main_nix_prefetch_url(int argc, char * * argv)
if (args.size() > 2)
throw UsageError("too many arguments");
if (isatty(STDERR_FILENO))
if (isOutputARealTerminal(StandardOutputStream::Stderr))
setLogFormat(LogFormat::bar);
auto store = openStore();