Skip to content

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