-
Notifications
You must be signed in to change notification settings - Fork 10
Update ansi.luau #160
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
base: primary
Are you sure you want to change the base?
Update ansi.luau #160
Conversation
batteries/colorful.luau
Outdated
local function colorDisabled(): boolean | ||
-- is it really okay to index env every style call? | ||
return process.env.NO_COLOR ~= nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest looking at how picocolors handles NO_COLOR. it also returns a generator function where you can explicitly specify if you want colors or not (but default returns the inferred preference) https://github.com/alexeyraspopov/picocolors/blob/main/picocolors.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think it'd be reasonable to return a function here, e.g.
function(colorsEnabled: boolean?)
local colorsEnabled = colorsEnabled or (process.env.NO_COLOR ~= nil)
return ...
end
Co-authored-by: ariel <[email protected]>
combine(...)
, looks likecombine(colorful.red, colorful.bold)("This is red bold text!")