8000
Skip to content

Init colorama in ESPHome main - #5111

Merged
jesserockz merged 3 commits into
esphome:devfrom
libretiny-eu:feature/windows-colorama
Jul 22, 2023
Merged

Init colorama in ESPHome main#5111
jesserockz merged 3 commits into
esphome:devfrom
libretiny-eu:feature/windows-colorama

Conversation

@kuba2k2
@kuba2k2 kuba2k2 commented Jul 17, 2023
Copy link
Copy Markdown
Member

What does this implement/fix?

Call colorama.init() in ESPHome CLI tools, to fix terminal color support on Windows (especially in editors such as VSCode, having their own terminal support).

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

@probot-esphome probot-esphome Bot added core small-pr PR < 30 lines labels Jul 17, 2023
@jesserockz
jesserockz commented Jul 19, 2023
Copy link
Copy Markdown
Member

Which part exactly is this fixing?
Do you have a before and after screenshot?

@kuba2k2
kuba2k2 commented Jul 19, 2023
Copy link
Copy Markdown
Member Author

This fixes colored text output, where otherwise ANSI color codes are shown instead.

Take a look (before/after):
obraz

(the 2nd one errors out, because it doesn't have the Popen() PR checked out).

Nevertheless, it's always good to init Colorama whenever working with colored text output.

@jesserockz
jesserockz commented Jul 20, 2023
Copy link
Copy Markdown
Member

Weird, it was colouring for me when I was testing the popen PR just fine on windows.

Since it is going to be init'd here, can you remove the init from the log file?

@kuba2k2
kuba2k2 commented Jul 20, 2023
Copy link
Copy Markdown
Member Author

Sure, I didn't really notice it was in log.py already. It seems that the whole issue was that init() was placed after the logging calls - placing it before them fixes coloring.

Anyway, I removed it from log.py, and moved to run_esphome() since it seems more appropriate.

@jesserockz
jesserockz commented Jul 21, 2023
Copy link
Copy Markdown
Member

If moving it before the logging calls fixes the issue on windows, I would rather keep in inside the log.py file in that case...
(sorry to go around in circles)

@kuba2k2
kuba2k2 commented Jul 21, 2023
Copy link
Copy Markdown
Member Author

Done.

@jesserockz
jesserockz merged commit 80154b2 into esphome:dev Jul 22, 2023
@jesserockz
jesserockz commented Jul 22, 2023
Copy link
Copy Markdown
Member

Thanks!

@github-actions github-actions Bot locked and limited conversation to collaborators Jul 24, 2023
@kuba2k2
kuba2k2 deleted the feature/windows-colorama branch August 21, 2023 16:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jesserockz jesserockz jesserockz approved these changes

Assignees

No one assigned

Labels

core small-pr PR < 30 lines

46F8 Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kuba2k2 @jesserockz
0