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

add disables colorized output option #718

Merged
merged 2 commits into from
Jul 22, 2024
Merged

Conversation

ch1aki
Copy link
Contributor

@ch1aki ch1aki commented Jul 19, 2024

I want to easily disable terminal formatting sequences when automatically commenting on a PR with the results of ecspresso diff, using tools like suzuki-shunsuke/github-comment."

colorized

colorized

no color

noncolor

cli.go Outdated
@@ -16,6 +16,7 @@ type CLIOptions struct {
AssumeRoleARN string `help:"the ARN of the role to assume" default:"" env:"ECSPRESSO_ASSUME_ROLE_ARN"`
Timeout *time.Duration `help:"timeout. Override in a configuration file." env:"ECSPRESSO_TIMEOUT"`
FilterCommand string `help:"filter command" env:"ECSPRESSO_FILTER_COMMAND"`
NoColor bool `help:"disables colorized output" default:"false"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the negative flag that has a default false value is confusing a little.

alecthomas/kong has the negatable attribute to make --no-xxx flag from the positive flag --xxx automatically.

Suggested change
NoColor bool `help:"disables colorized output" default:"false"`
Color bool `help:"enable colorized output" env:"ECSPRESSO_COLOR" default:"true" negatable:""`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in aee93f2 .

Thanks for the suggestion!

cliv2.go Outdated
Comment on lines 40 to 42
if opts.NoColor {
color.NoColor = true
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if opts.NoColor {
color.NoColor = true
}
color.NoColor = !opts.Color

@fujiwara
Copy link
Contributor

@ch1aki Thank you! I've commented.

@fujiwara fujiwara added the v2.4 label Jul 19, 2024
@fujiwara fujiwara merged commit 5fba421 into kayac:v2 Jul 22, 2024
2 checks passed
@fujiwara
Copy link
Contributor

@ch1aki Thank you! merged. This will be released in v2.4.

fujiwara added a commit that referenced this pull request Jul 22, 2024
@ch1aki ch1aki deleted the no-color-opt branch July 22, 2024 02:46
@fujiwara fujiwara mentioned this pull request Jul 22, 2024
fujiwara added a commit that referenced this pull request Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants