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

[GP CLI] Allow explicity and automated disabling of output colorization #10638

Merged
merged 1 commit into from
Jun 15, 2022

Conversation

andreafalzetti
Copy link
Contributor

@andreafalzetti andreafalzetti commented Jun 13, 2022

Description

This PR updates the commands of gitpod-cli which use output colorization, making it explicitly disabled when requested by the user (via --no-color) or automatically (when stdin/stdout detected or for other community/standard conditions) [1].

Related Issue(s)

Fixes #10633
Related #9930

How to test

  1. Open the prev env
  2. Run watch gp ports list and watch gp tasks list notice how the output does not display any invalid character and the output is readable (and not colorized).

Release Notes

NONE

Documentation

We should document this flag/feature: https://github.com/gitpod-io/website/issues/2235

@andreafalzetti andreafalzetti marked this pull request as ready for review June 13, 2022 23:52
@andreafalzetti andreafalzetti requested a review from a team June 13, 2022 23:52
@andreafalzetti andreafalzetti mentioned this pull request Jun 13, 2022
@andreafalzetti andreafalzetti changed the title feat(gp-cli): allow disabling output colorization [GP CLI] Allow explicity and automated disabling of output colorization Jun 13, 2022
Copy link
Member

@filiptronicek filiptronicek left a comment

Choose a reason for hiding this comment

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

Thanks a lot for taking this, @andreafalzetti!

I tested it and it's working just as advertised, but I have one question: could we abstract the logic for all of the gp commands so that for future commands we could just wrap the CLI in some function that would add the no-color options? This would help with many commands failing if you provide them with --no-color, like gp ports, which I think it would be better if it just ignored (if there was no color to display in the first place).

Holding the PR if you have any thoughts on this, feel free to unhold :)

/hold

@andreafalzetti
Copy link
Contributor Author

andreafalzetti commented Jun 15, 2022

Thanks a lot for taking this, @andreafalzetti!

I tested it and it's working just as advertised, but I have one question: could we abstract the logic for all of the gp commands so that for future commands we could just wrap the CLI in some function that would add the no-color options? This would help with many commands failing if you provide them with --no-color, like gp ports, which I think it would be better if it just ignored (if there was no color to display in the first place).

Holding the PR if you have any thoughts on this, feel free to unhold :)

/hold

Thanks, @filiptronicek for the review 🙏

We now have the util.ColorsEnabled which wraps all the logic to disable colors, the noColor flag variable has global scope, so it's accessible from all commands - if they need to have it. I thought about having a global --no-color flag for all commands, but for those which don't use colors, it would be confusing why it's there, so I've opted for having the 1-line copy/paste flag on each command that uses colors.

At the moment that's the best I can think of!

If anyone has any idea for further improvements, I am all ears :)

/unhold

@roboquat roboquat merged commit 8dad2f1 into main Jun 15, 2022
@roboquat roboquat deleted the andreafalzetti/gp-cli-automatically-10633 branch June 15, 2022 17:24
@roboquat roboquat added deployed: IDE IDE change is running in production deployed Change is completely running in production labels Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: gp cli deployed: IDE IDE change is running in production deployed Change is completely running in production release-note-none size/S team: IDE
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[GP CLI] Automatically disable output colorization when not required
3 participants