Added check to ensure init() will only have any effect in Windows.

[coredumperror]

Much like #304, I am also experiencing issues because of the not-quite-true nature of that line from the docs that carltongibson quoted.

On other platforms, calling init() has no effect

This is untrue when running in Linux within a Docker container, because even if you don't pass strip=True to init(), it'll still act like you did because in Docker, sys.stderr isn't a tty. It's a file descriptor that prints to a special log file which you can follow to get output that looks like a tty, but isatty() returns False. This forces strip=True, do to the if strip is None check in the AnsiToWin32 constructor, which activates the AnsiToWin32 wrapper even though the code isn't running on Windows. This completely breaks the colored output of all logs coming out of the Docker container.

This pull request makes it 100% certain that Colorama won't screw around with sys.stderr or sys.stdout unless the program is actually running on Windows. Libraries like StructLog already wrap their call to colorama.init() in such a check, so I figure that Colorama itself should be doing this.

[tartley]

Massive thanks for this. It does sound like a sensible idea. However, because we have so many dependencies, I'm currently terrified of breaking compatibility with the old behavior. For example, someone somewhere might be relying on the effects of calling init(autoreset=True) on Linux.

For context, the reason I say this, the last time I made a release, I built a wheel which didn't pick up the README file for reasons unknown (no doubt my fault somehow), and a Python package without a README is invalid, and won't install, but I didn't notice, and uploaded to PyPI, and within seconds I received a tsunami of github issues, emails and tweets, because I'd broken a million people's "pip install" commands of packages which depend on Colorama.. So... we've got a new release process sketched out, which uploads to test-PyPI and tries installing it from there, but I'm keen to tread very carefully at this point.

I haven't merged anything to Colorama, nor made a release, for years. But I'm aiming to do one this week. I might hold off on merging this until I've thought about it more, so it might not make this release. Thoughts welcome.

[coredumperror]

Maybe make this part of a major release, then? Compatibility-breaking changes are OK in major releases. Finally bring Colorama to 1.0, since the project is essentially finished anyway, and people will accept that they have to make a conscious decision to upgrade.

[tartley]

Yeah, that's fair. OK then, I'll aim for that...

[tartley]

Thanks once again. This change sounds sensible but needs some thought.

1. The tests don't pass with this change in place.
2. The change needs it's own automated tests. Omitting them is punting work onto future developers, who have no way of knowing that their changes have broken this MP's code.
3. Not your fault, but the codebase seems to detect Windows with a mixture of os.name in some places, and sys.platform in others. We should normalize on one or the other. That isn't the responsibility of this PR, but in order to write sane test for (2) above, one might want to sort it out.

[tartley]

Thanks heaps for this. FYI, an alternative approach has been mentioned. Instead of modifying the behavior of the existing 'init()' call, upon which some people may be relying, the idea has been floated to add a new method alongside 'init', which does something like what you (and others) require.

For details, please see the final final comment my njsmith here ("I would like to sneak one more feature in") , just before I merged.

[tartley]

On reflection, to be explicit, I'm going to close this PR in favor of the approach suggested by njsmith, with the intention of getting a relatively trivial PR merged which can do this, and then making a release.