Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support CLICOLOR_FORCE #10619

Open
willbush opened this issue Apr 29, 2024 · 0 comments
Open

Support CLICOLOR_FORCE #10619

willbush opened this issue Apr 29, 2024 · 0 comments
Labels
feature Feature request or proposal

Comments

@willbush
Copy link
Member

willbush commented Apr 29, 2024

Is your feature request related to a problem? Please describe.

I've been looking into ways to force nix to use ANSI color output in Github Actions for nixpkgs-check-by-name (which runs on every nixpkgs PR) because the majority of errors that people run into are basic nix evaluation errors. here is an example

Describe the solution you'd like

Perhaps simply returning true in this function when CLICCOLOR_FORCE is set.

and/or follow this standard: https://bixense.com/clicolors/

and/or progress on #7650

Describe alternatives you've considered

The way to force nix to output ANSI colors to get above mentioned function to return true.

Because Github actions don't run as a TTY, at first I thought we could get around it by doing:

script --return \
  --quiet \
  --log-out /dev/null \
  --command "unset NO_COLOR && TERM=xterm nix eval --expr 'not a valid expression'"

Or using faketty. The above works if calling nix directly:

However, we are calling nix from a process in Rust similar to:

let mut command = Command::new("nix");
let output = command
    // Capture stderr so that it can be printed later in case of failure
    .stderr(process::Stdio::piped())
    .args(["eval"])
    .args(["--expr", "not a valid nix expression"])
    .output()
    .expect("failed to execute process");

And the TTY check isatty(STDERR_FILENO) evaluates false.

We can get around that by using a crate like the following to spawn new processes attached a pty:

https://crates.io/crates/pty-process

However, it would be nice if we didn't have to resort to that.

Additional context

Seems IsTty function used to be called shouldANSI. I was going to just make a PR to add a check to see if CLICOLOR_FORCE is set, but I wanted to check here first.

I'm willing to help out if needed. Just need some guidance.

Priorities

Add 👍 to issues you find important.

@willbush willbush added the feature Feature request or proposal label Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature request or proposal
Projects
None yet
Development

No branches or pull requests

1 participant