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

Detecting ANSI compatibility #553

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

monte-monte
Copy link
Contributor

Added check for ANSI escape support. This should resolve possible edge cases.
For windows I am setting console mode to enable ANSI and if it fails, fall back to simple terminal.
https://learn.microsoft.com/en-us/windows/console/getconsolemode
Tested on win11 and win7. Would appreciate if you test it on win10 or 8 also.

@monte-monte
Copy link
Contributor Author

  • Need to find linux terminal that doesn't support ANSI escape chars to test.
  • Maybe this should be moved to a function in terminalhelp.h?

@cnlohr
Copy link
Owner

cnlohr commented Mar 10, 2025

You know, you can call system("") to put cmd or powershell into ansi mode, right?

@monte-monte
Copy link
Contributor Author

No, I don't know that. What is that?

@cnlohr
Copy link
Owner

cnlohr commented Mar 11, 2025

It's how you get ANSI control codes in cmd and powershell. I don't know why it's not better optimized. Just call that at program start. There are no downsides that I am aware of.

@monte-monte
Copy link
Contributor Author

weird, I didn't see any mention of it while I was looking for a way to enable ANSI in cmd on windows. Also that won't work on <win10? By using SetConsoleMode you can see if it works or no, and act accordingly.

@cnlohr
Copy link
Owner

cnlohr commented Mar 11, 2025

That is true, it is Win10+. But it will work in all situations/combinations (power shell or cmd) on Win10+, so no need to check

@monte-monte
Copy link
Contributor Author

monte-monte commented Mar 12, 2025

So, you're ok with making it incompatible with anything lower than win10? What's wrong with the proposed solution, that should cover all edge cases?
Also why does empty system(") call produce this result? Is it documented anywhere, or you've found it experimentally?

@cnlohr
Copy link
Owner

cnlohr commented Mar 13, 2025

system("") and I don't know. It was told to me by someone who works at Microsoft.

If you want you could also set with the set console properties! I definitely don't want to deliberately leave out Win7 and lower.

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.

2 participants