Skip to content

Allow control of output colorization#572

Closed
zapo wants to merge 1 commit intocli:trunkfrom
zapo:feature/colorize-output-control
Closed

Allow control of output colorization#572
zapo wants to merge 1 commit intocli:trunkfrom
zapo:feature/colorize-output-control

Conversation

@zapo
Copy link

@zapo zapo commented Mar 3, 2020

Follow up PR based on #447 conversation.

This PR adds two new persistent flags to control colorization:

  • --color=WHEN, with WHEN one of:

    • "auto": current auto-detection of environment behavior, default if omitted
    • "always": force colorization
    • "never": force no-colorization
  • --no-color, disable colorization, equivalent to --color="never" and has precedence over --color

- --color=WHEN, with WHEN one of:
  - "auto": current behavior, default if omitted
  - "always": force colorization
  - "never": force no-colorization
- --no-color, disable colorization, equivalent to --color="never"
@zapo zapo closed this Mar 3, 2020
@zapo
Copy link
Author

zapo commented Mar 3, 2020

I realize this doesn't work with latest changes on utils/table_printer.go when output is detected as non-tty and we use instead a tsv table renderer. This doesn't fix my initial problem when using watch.

Is this still the direction you think we should go @mislav ? Should --color=always be a strong enough signal that we are using a tty or should we allow override tty detection with --tty ?
It seems like tty detection is now doing more than coloration only (truncation as well).

@zapo zapo reopened this Mar 3, 2020
@muesli
Copy link
Contributor

muesli commented Mar 4, 2020

Just a quick note: since cli uses glamour for rendering markdown content, we could also enforce/disable its colorization. Let me know if you need some help!

@mislav mislav changed the base branch from master to trunk May 27, 2020 11:41
@probablycorey
Copy link
Contributor

Thanks for looking into this @zapo. Adding the --no-color flag is definitely something the app needs. But because this PR is a few months old it is mostly out of date, so I'm going to close it. If you'd like to continue working on the problem I'd look at modifying the colorableOut function to check for the --no-color flag. This would make the surface area of the change pretty minimal and would ensure it works with future commands.

If you're interested in following progress on the colorization problem you can follow this issue #364

@mislav
Copy link
Contributor

mislav commented Jun 9, 2020

@zapo We still definitely want this feature, but we first making some changes to how we structure Cobra commands. After that, it will be easier to reconsider the global --color setting, but this PR will not apply there anyore. So thanks @probablycorey for closing this. 🙇

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants