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

Colorized output support for Windows. #99

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

gwankyun
Copy link

@gwankyun gwankyun commented Nov 25, 2020

  1. Use SetConsoleTextAttribute to support Colorized output on Windows.
  2. Must set NOMINMAX when use std::min on Windows.
  3. __cplusplus is not enough to check MSVC standard.

@sharkdp
Copy link
Owner

sharkdp commented Dec 1, 2020

Thank you very much for your contribution!

Happy to take a look when the builds are green.

@sharkdp
Copy link
Owner

sharkdp commented Dec 31, 2020

I have updated our CI system to GitHub Actions, see #104. It would be great if you could rebase your branch to run the new set of tests.

@gwankyun
Copy link
Author

The checks have passed, Please review it.^_^

@gwankyun
Copy link
Author

gwankyun commented Feb 3, 2021

I learning to solve #97 .

@sharkdp
Copy link
Owner

sharkdp commented Feb 13, 2021

Thank you for your contribution. A few things are included here. It would be great if we could split them into separate PRs. As for part 2 (Must set NOMINMAX when use std::min on Windows.), have you seen #100?

As for the main change here (colorized support for Windows): I would prefer if we could keep the complexity somewhat limited and only support ANSI-escape sequence based terminals. This should work just fine on Windows 10, if the support is enabled (https://docs.microsoft.com/de-de/windows/console/console-virtual-terminal-sequences).

@gwankyun
Copy link
Author

I defined a DBG_ANSI_TYPE marco in dbg.h, for optional support Classic Console API on Windows 7 in example.cpp.

@bl-ue
Copy link

bl-ue commented Apr 7, 2021

cc @sharkdp

@sharkdp
Copy link
Owner

sharkdp commented Apr 13, 2021

cc @sharkdp

?

@bl-ue
Copy link

bl-ue commented Apr 14, 2021

Sorry, that wasn't very clear. 😄

I was using my own macro on a project, but I recently came upon this library, which I immediately preferred and attempted to use, but I found that it doesn't support the platform I'm developing for, Windows. I can easily checkout this PR, but I'd love to see it merged into master (which maybe should be renamed to main?)

@sharkdp
Copy link
Owner

sharkdp commented Aug 8, 2021

To be honest, I have lost track of what this PR does. There are a lot of changes, especially to the CMake config. As I said above: I'm happy to integrate this, but only for terminals that support ANSI escape sequences. I don't want to add (and maintain) a lot of code for legacy Windows terminals.

@niXman
Copy link

niXman commented Jan 16, 2023

Must set NOMINMAX when use std::min on Windows.

just wrap it in parenthesis =)

(std::min)(...)

@gwankyun
Copy link
Author

Must set NOMINMAX when use std::min on Windows.

just wrap it in parenthesis =)

(std::min)(...)

Thank you but >_<

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.

4 participants