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

Support forcing colors #17229

Open
grant0417 opened this issue Dec 30, 2022 · 9 comments
Open

Support forcing colors #17229

grant0417 opened this issue Dec 30, 2022 · 9 comments
Labels
suggestion suggestions for new features (yet to be agreed)

Comments

@grant0417
Copy link

If deno is not running in a tty there is currently no way to override it so that colors are shown

colors.setNoColor(runtimeOptions.noColor || !runtimeOptions.isTty);

Some options are:

  • FORCE_COLOR env var - this would ignore the other values and never set noColor
  • --color flag - similar to ls and other CLI utils
    • auto -> current behavior
    • never -> same behavior as NO_COLOR
    • always -> force color on
@grant0417
Copy link
Author

Also open to making the actual change if this is something wanted, just don't know which behavior is preferred.

@bartlomieju
Copy link
Member

If we were to support it, it should be via env var. Is there a prior art for FORCE_COLOR env variable?

@bartlomieju bartlomieju added the suggestion suggestions for new features (yet to be agreed) label Dec 31, 2022
@grant0417
Copy link
Author

grant0417 commented Jan 2, 2023

Prior art for FORCE_COLOR

More approaches:

I do not have any strong preference on the approach here

@bartlomieju
Copy link
Member

@dsherret thoughts on this one?

@dsherret
Copy link
Member

dsherret commented Jan 3, 2023

We should probably do exactly what Node does, so go with FORCE_COLOR. It defaults to 2 when non-empty and not 0-3:

https://github.com/nodejs/node/blob/9eb363a3e00dbba572756c7ed314273f17ea8e2e/lib/internal/tty.js#L123

Environment variable sounds much better than a flag.

@grant0417
Copy link
Author

Seems like the lib termcolor only supports forcing color, no color or auto, but has no concept of color depth support: https://docs.rs/termcolor/latest/termcolor/enum.ColorChoice.html

Would replicating the logic of what is "enabled" for the flags be enough or should the it support the concept of color depth as well?

@dsherret
Copy link
Member

dsherret commented Jan 3, 2023

I think for compatibility with Node we should get it to work the same way, so maybe that means using a different crate or implementing this ourselves. As a stepping stone, we could maybe implement this broken for now with the same API as Node then fix it up later.

@jcbhmr
Copy link
Contributor

jcbhmr commented Oct 24, 2023

this would be beneficial to allow forcing color to be enabled in ci like github actions which arent tty but still can show colors

related: pypa/pip#10909 Textualize/rich#2425 wntrblm/nox#502 python-poetry/cleo#341 astral-sh/ruff#5499 dotnet/command-line-api#1710 pre-commit/pre-commit#2918 pytest-dev/pytest#7443 python/mypy#13806

@mdekstrand
Copy link

Also the extremely useful case of piping to less.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests

5 participants