Skip to content

Conversation

@FFY00
Copy link
Member

@FFY00 FFY00 commented May 21, 2021

It seems Windows has settings to enable/disable color support by default
and we'd have to use low level Windows APIs to force enable, so I am
just gonna remove colors altogether. Shouldn't really be a big deal as
they are only used for the ERROR and WARNING prefixes.

Signed-off-by: Filipe Laíns lains@riseup.net

@FFY00 FFY00 requested review from gaborbernat and layday as code owners May 21, 2021 21:17
@uranusjr
Copy link
Member

How about a ready-made solution like colorama? Colours on Windows is a solved problem, build is having issue only because the current implementation is inventing wheels from scratch.

@henryiii
Copy link
Contributor

henryiii commented May 21, 2021

I need a tears emoji reaction. I love colors... (GitHub only has 8 reactions)

@henryiii
Copy link
Contributor

henryiii commented May 21, 2021

PS: I know that @FFY00 is against adding dependencies, but this would be a Windows-only dependency, which might be a lot less invasive? Edit: Okay, just two words colorized might be a valid loss. Still. 😢

@FFY00 FFY00 force-pushed the no-color-windows branch from e33a325 to c90d846 Compare May 22, 2021 00:17
@FFY00
Copy link
Member Author

FFY00 commented May 22, 2021

I added it as a hard dependency on windows, but not actually a runtime dependency, people wanting to use the project to bootstrap environments may skip the dependency.

I am not 100% sure of the decision, so if it causes issues for anyone, we will reconsider.

@henryiii
Copy link
Contributor

I like the idea quite a bit, I think.

Note that adding comments in setup.cfg is not supported when using setup-cfg-fmt (which I love using), and the changelog entry is still unchanged, I think. (From my phone it seems to be)

@FFY00
Copy link
Member Author

FFY00 commented May 22, 2021

Oops, I forgot the changelog.

I tried putting a comment in a line itself and it got removed, but it seems appending it to a meaningful line works fine.

It seems Windows has settings to enable/disable color support by default
and we'd have to use low level Windows APIs to force enable.

Signed-off-by: Filipe Laíns <lains@riseup.net>
@FFY00 FFY00 force-pushed the no-color-windows branch from c90d846 to 48b5f28 Compare May 22, 2021 00:27
@FFY00
Copy link
Member Author

FFY00 commented May 23, 2021

I am gonna merge as-is, please open a PR moving colorama.init() to a different place if you care about that.

@FFY00 FFY00 merged commit 6a86979 into pypa:main May 23, 2021
@FFY00 FFY00 deleted the no-color-windows branch May 23, 2021 22:11
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.

5 participants