Change pygmentize to not strip ansi escapes on windows

[rmccampbell]

Fixes #2491. This maintains the colorama conversion behavior when needed (only for the legacy Windows console) but does not strip them for non-console output, e.g. to pipes.

[jeanas]

This is a backwards-incompatible change that I doubt Pygments, as a mature and widely used library, can afford. I'd rather see the non-stripping behavior turned on by an opt-in CLI option.

[rmccampbell]

Hmm but this seems like an unexpected behavior considering that stripping escapes defeats the entire purpose of using pygmentize, and the behavior is also inconsistent between Windows and Unix. I guess some people might rely on it for functions/scripts that only generate color when not piped/redirected, but I think it would be much preferable to have an option to enable stripping escapes on both Windows and Unix consistently.

[jeanas]

If we started Pygments from scratch today, it would indeed be preferable to make the behavior consistent between UNIX and Windows by default, but when a library is downloaded millions of times a day, you can't change its existing behavior (on a somewhat fundamental point) without a lot of thought.

Maybe @birkenfeld and @Anteru will disagree with me here, but I would put this incompatibility on the "too much breakage" side of the line.

[Anteru]

A new option is fine, but I agree with @jeanas, changing the behavior isn't. This is something to reconsider for a Pygments 3.0 release but for now an option is fine (and I'm starting to gather a list of changes for a Pygments 3.0 release ...)

[birkenfeld]

I'm not sure actually. On the one hand, yes breaking compatibility is not desirable, but aren't we just "breaking" buggy behavior here really? This is (a) only relevant for the terminal formatters, and (b) @rmccampbell is correct that currently the output of pygmentize is useless in those cases, so any use cases that are "breaking" are questionable at best...

[Anteru]

We can always try to change the default and provide an option to turn back, and see how big the fallout is. Maybe it is indeed one of those breaking cases which only break broken behavior, and there is precedence for doing this. Let's try that? My main objection here is to not have an option to start with.

[jeanas]

Well, my understanding is that this will also introduce escape sequences when redirecting command output to a file. If a tool like pytest gives an error log with syntax-highlighted output and I redirect the log to a file because it's large, it's helpful to strip escape sequences because my text editor probably will probably show them in a way that makes the log hard to read.

[birkenfeld]

Well, my understanding is that this will also introduce escape sequences when redirecting command output to a file. If a tool like pytest gives an error log with syntax-highlighted output and I redirect the log to a file because it's large, it's helpful to strip escape sequences because my text editor probably will probably show them in a way that makes the log hard to read.

That behavior would be inconsistent between platforms then though, which is IMO also a bug. Because we have no magic on non-Windows that strips escapes when output goes to a non-tty.

(It would be an interesting option to add, actually!)

[rmccampbell]

@jeanas I think this is a potential feature but was likely not intentional (or only intentional under the assumption that windows platforms couldn't do anything useful with ANSI escapes, which is not true). It would be better as an opt-in flag that also works on Linux/Mac.

Also AFAIK it isn't possible to distinguish pipes from regular files, and I pipe pygmentize output on Linux all the time (usually to less) so I definitely don't think that should change.

[Anteru]

Is that true? I thought you could stat the standard input/output handles and figure out if it's going to a file from there, i.e. using https://docs.python.org/3/library/stat.html#module-stat and S_ISREG?

[birkenfeld]

Hmm, you could, at least on Posix, but AFAIK nobody else does that. Everyone checks for isatty.

[Anteru]

So how do we proceed here? I'm ok with hiding this behind an option and defaulting it to on or off (either way is fine for me, given the behaviour right now is definitely not great.) I'd be going as far as saying we could introduce an environment variable to control this, even though we never used environment variables before in Pygments for configuration purposes ... but we need some kind of solution. @jeanas , @birkenfeld any objections?

[birkenfeld]

My opinion: Add an option to strip escapes (off/auto/on), make it work on all platforms, and default it to off everywhere.
(Where "auto" means the usual isatty check)

[rmccampbell]

Ditto @birkenfeld, I think the behavior on Linux/Unix is much more likely to be relied on currently then the Windows behavior which likely has a much smaller user base anyway. Since this is a bit more involved than the current PR it can probably be closed and followed up in #2491.

[jeanas]

My opinion: Add an option to strip escapes (off/auto/on), make it work on all platforms, and default it to off everywhere. (Where "auto" means the usual isatty check)

OK, fine with me.

[jeanas]

However, "strip escapes = on" would just be a cat equivalent. From the UI perspective, let's have an option --only-terminal (off by default) that skips highlighting if the output isn't a tty?

[birkenfeld]

I'm neutral on that. Just thought to mirror the usual options for CLI tools. But it doesn't have to be that way since for us, color output isn't just a nicety.

[Anteru]

So where are we on this? I do want to make a release soon, and this should be in. From reading this thread, I'm leaning towards what @birkenfeld wrote: --strip-escapes, with off being the default, on meaning nothing gets escaped, and auto trying to chose a sensible default (i.e. use isatty and refine further as needed.)

[jeanas]

OK, so let's merge this PR and implement the flag in another PR?

[Anteru]

I'll drop this PR but implement it directly as a new flag.