Skip to content
New issue

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

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

Already on GitHub? # to your account

Add --color option #1033

Merged
merged 1 commit into from
Jan 6, 2023
Merged

Add --color option #1033

merged 1 commit into from
Jan 6, 2023

Conversation

matthew-healy
Copy link
Contributor

This flag has three possible values: auto, always and never. Its default is 'auto'.

With 'auto' selected, we allow the codespan_reporting and rustyline libs to determine whether to show colors based on the current terminal settings.

With 'always' selected, we force-enable colors in both error reporting and the repl.

With 'never' selected, we disable all colors in output.

@github-actions github-actions bot temporarily deployed to pull request January 5, 2023 13:21 Inactive
This flag has three possible values: auto, always and never. Its default
is 'auto'.

With 'auto' selected, we allow the codespan_reporting and rustyline libs
to determine whether to show colors based on the current terminal
settings.

With 'always' selected, we force-enable colors in both error reporting
and the repl.

With 'never' selected, we disable all colors in output.
@github-actions github-actions bot temporarily deployed to pull request January 5, 2023 13:42 Inactive
@matthew-healy matthew-healy marked this pull request as ready for review January 5, 2023 16:04
Copy link
Contributor

@vkleen vkleen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This look reasonable. I think newer versions of clap have a way of automatically deriving the command line parsing for Enums; that would also expose the possible values in the help message. But that would require updating to a new clap version and using the derive feature there instead of structopt, I gues.
Maybe we should do that at some point? For now, this PR looks good enough to me.

@matthew-healy
Copy link
Contributor Author

@vkleen I agree that replacing structopt with clap is overall a good goal.

I think that we might need to rethink the architecture of the binary's main file, the Program struct and the repl function first though. Currently I think it'd be impossible to make that work without either leaking clap into the Nickel lib, or leaking rustyline and codespan_reporting's colour types into the binary. I guess the latter isn't the end of the world, but I feel like there's probably a cleaner separation we can achieve if we move things around a little bit.

@matthew-healy matthew-healy merged commit 1b65525 into master Jan 6, 2023
@matthew-healy matthew-healy deleted the cli/color-opt branch January 6, 2023 12:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants