fix and simplify MSWin32 colorization

[rivy]

• rationale: using Win32::Console::ANSI simplifies the code and improves Win32 outputs

---

- simplified usage ~ Win32::Console::ANSI is fire-and-forget - once loaded, use Term::ANSIColor without regard for platform - fixes incorrect color 'reset' behavior - Win32::Color::ANSI resets correctly to the prior default color attribute set - bug due to incorrect assumption that LIGHTGRAY on BLACK are always the default color attributes - fixes [GH#26](https://github.com//issues/26) - Win32::Console::ANSI works for all output (no tie to a specific output handle) - [PR#34](https://github.com//pull/34) is not needed

---

## Notes

Here's an example of output reset breakage ...

• baseline

- with PR

Notably, CPAN also uses Win32::Console::ANSI and the color output is correct (no aberrant color resets) (see CPAN/Shell.pm#L1483).

[rivy]

This should close both TAP::Harness/GH/PR #32 and TAP::Harness/GH/PR #34.

[karenetheridge]

Win32::Console::ANSI doesn't seem to be in core, so we would need to change that before this change can be made.

[rivy]

What's the process there?

FWIW, it's in all of the windows distributions that I know of ... Strawberry & ActiveState both include it. Strawberry has had it since about 5.16 (Win::Console was added in about 5.20 or so).

Would it be any better if it's loaded only when present on the system, defaulting back to Win::Console otherwise? I can refactor the PR to work that way if you wish. Or can the code not mention non-CORE modules?

Let me know if I can help move it along. It's a much better way to go, and is a big step toward more parity between platforms for the toolset.

[Leont]

FWIW, it's in all of the windows distributions that I know of ... Strawberry & ActiveState both include it. Strawberry has had it since about 5.16 (Win::Console was added in about 5.20 or so).

This was exactly the information I was going to ask you for. In that case it looks good to me.

[wchristian]

As long as things keep working without colors when its missing, i'm all for it. The exception just needs to stay in place for people who compile their own Perl.

[haarg]

Win32::Console is already non-core, so this doesn't really put us in any worse of a position on that front. Especially because the existing win32 dists include Win32::Console::ANSI. So +1 from me.

[andygrundman]

Andy from ActiveState here. This looks pretty good to me. One very minor thing I noticed: If Term::ANSIColor is not installed but Win32::Console::ANSI is, $NO_COLOR will still be set.

Term::ANSIColor doesn't appear to be required by Win32::C::A so in theory this situation could occur. ActivePerl ships both modules together, as does Strawberry, so I wouldn't worry too much about it.

[Leont]

Thanks for the feedback guys! :-)

[haarg]

Term::ANSIColor is core since 5.6, it would make sense to remove the checks for it.

[rivy]

@andygrundman , Term::ANSIColor (via the color() function) is required as the interface to set output color (not by Win32::Console::ANSI, but by the TAP::Formatter::Color object).

$output->( color($color) ) is the code that outputs the color codes. It's a bit obscure to me, as it's not immediately clear that the color() function is imported from Term::ANSIColor and not defined in the class.

So, I'd recommend changing the line to $output->( Term::ANSIColor::color($color) ). If not that, I think that the line eval 'use Term::ANSIColor'; might be changed to eval 'use Term::ANSIColor qw / color /';.

To me, either would more prominently show it's source (and the need for Term::ANSIColor for any type of color output).

[haarg]

Term::ANSIColor is a core module on every version of perl that Test::Harness is compatible with. The eval and $@ check for Term::ANSIColor can be eliminated entirely.

[rivy]

I'm happy to change the loading of Term::ANSIColor to a simple use, if that's the general wish.

But, does the comment by @wchristian cause any concern? As I don't compile my own Perl, I don't know about the availability of Term::ANSIColor for those situations. And as I've come to understand recently, being in CORE doesn't actually guarantee being present in the installation. Ultimately, I don't really understand all of the possible edge cases.

To me, colorization is very useful, but it is non-essential. I'd hate to break usage of Test::Harness by making Term::ANSIColors a requirement instead of an optional dependency. The eval seems to be a small price to pay if it maintains wider compatibility. OTOH, if it's useless, I'll chop it.

[bulk88]

Why load Term::ANSIColor on Windows if it won't be used by the harness proc?

[haarg]

Because it will be used by the harness. Win32::Console::ANSI just means that the escape codes generated by Term::ANSIColor will automatically be translated to Win32 API calls to control the console colors.

[rivy]

I amended the commit to use eval 'require Term::ANSIColor' (using stringy-eval in the manner of the original code). As far as conversion to an eval BLOCK and whether to remove the eval completely, I'm happy to take direction ... @Leont ?

I see that @haarg votes for removal of the eval completely. But does that introduce a requirement that might needlessly break testing for self-compiled perls (? @wchristian) or perl installations which don't contain the full CORE?

[rivy]

@Leont , any progress on this?

[Leont]

Haven't worked on a new Test::Harness release in ages, but one is coming soon due to other somewhat urgent matters.

[rivy]

@Leont , I think the implementation, as I have it now, is good as is... But, given the comments, if you'd like some change, just let me know.

[rivy]

@Leont

Any progress here?

[Leont]

This has been merged into 3.40_01 (a dev-release) and unless problems are encountered will become part of 3.41