Skip to content

Enable output color and effects on Windows #7463

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

Merged
merged 8 commits into from
Sep 8, 2019
Merged

Enable output color and effects on Windows #7463

merged 8 commits into from
Sep 8, 2019

Conversation

emmatyping
Copy link
Member

@emmatyping emmatyping commented Sep 4, 2019

image

It turns out it is relatively simple to do this on Windows 10 without dependencies. Note that we can get color output on Windows 7/8 but not really underlining or other VT100 features. This should be supported on all versions of Windows 10 people are running. I'm not sure Windows 7/8 support is worth it here. Windows 7 is EOL at the end of the year, and Windows 8.1 has only 5% market share.

Fixes #7437

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, it is great to have good support across all platforms! I just have couple small suggestions.

Have you checked that get_terminal_width() works as expected and there are no problems with the daemon?

@emmatyping
Copy link
Member Author

emmatyping commented Sep 5, 2019

Here is the daemon in action. I also tested and the terminal width information does indeed work.

image
(ignore the commands being corrupted, that is just a bug in the console itself)

I've gone ahead and followed your suggestions, hopefully the background I gave is enough (I also added a check for major version). Let me know if you have further feedback/suggestions.

@emmatyping emmatyping closed this Sep 5, 2019
@emmatyping emmatyping reopened this Sep 5, 2019
@emmatyping
Copy link
Member Author

Huh, that's concerning. I'm not sure why the daemon timeouts happened. I triggered a new build so hopefully that was a fluke (but I will think about why that could happen).

@ilevkivskyi
Copy link
Member

The problem may be because if initialize_unix_colors() exits early the color attributes are not initialized actually, so it will crash right after in __init__(). I think both these methods should return bool, and then the called should set dummy_terminal to the result (and return soon if False).

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updates!

I have couple more comments (feel free to land after you consider them).

@emmatyping emmatyping merged commit 52c286f into master Sep 8, 2019
@emmatyping emmatyping deleted the wincolor branch September 8, 2019 19:17
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.

Support colorized output on Windows
2 participants