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

Terminal colors for output #636

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

9999years
Copy link
Contributor

@9999years 9999years commented Dec 12, 2024

This adds terminal colors to cabal2nix's output. This checks if stderr isatty so that output will not be colored if (for example) cabal2nix's stderr is being piped to a file. It also respects the $NO_COLOR and $FORCE_COLOR environment variables for toggling the output coloring (see no-color.org).

A couple small changes while I was preparing this:

  1. I added nix-prefetch-scripts to the shell.nix so that you can run cabal2nix successfully while hacking on it.
  2. I added a line to print the fetcher command (like nix-prefetch-git ...) before we run it. This makes it a lot clearer where the errors come from, and which errors are being ignored (because we're trying another fetcher).

Example output:
image

If stderr isatty (i.e. the output is not being pumped to a file) and
`$NO_COLOR` is unset or empty, or if `$FORCE_COLOR` is set and
non-empty, output is colored.

This helps distinguish `cabal2nix`'s output from the output of the
fetchers it runs.
@sternenseemann
Copy link
Member

Good idea, I think this is a good migitation for the error message problem the current fetcher logic has!

Not quite sure what to think about colored output, i.e. if the extra code is worth it…

@9999years
Copy link
Contributor Author

@sternenseemann Happy to remove the colored output if you'd prefer (or you can update my branch -- just drop the last commit), let me know

@sternenseemann sternenseemann changed the title Print commands before we run them + terminal colors for output Terminal colors for output Dec 16, 2024
@sternenseemann
Copy link
Member

I've merged those changes manually, we can leave this PR for the color changes. I'm going to keep thinking about it. Maybe @maralorn and @cdepillabout want to comment?

@cdepillabout
Copy link
Member

cdepillabout commented Dec 17, 2024

I think I'm generally in favor of (optional) colors.

For CLI programs that use color, I think I'd also expect a --color / --no-color CLI flag to exist, but I don't think not having it should necessarily block this PR.

@9999years could you post a screenshot of what the colors look like in the output of cabal2nix?

@9999years
Copy link
Contributor Author

@cdepillabout Added a screenshot to the PR description!

@cdepillabout
Copy link
Member

I think the colors seems nice, and it will likely make visually parsing the output of cabal2nix easier. I'm for merging this.

@maralorn
Copy link
Member

Unsurprisingly I am all in favor of colorful output. Code complexity seems manageable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants