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

add color for the MSWin32 platform #5

Merged
merged 1 commit into from
Sep 10, 2016

Conversation

rivy
Copy link
Contributor

@rivy rivy commented Sep 8, 2016

.# Discussion

Term::ANSIColor doesn't include MSWin32 support by default. And, unfortunately, it seems
that perl-porters don't want to include it (see https://rt.cpan.org/Ticket/Display.html?id=44807
@@ http://archive.is/gqB1N). So, to support the platform, Win32::Console::ANSI must be
used whenever Term::ANSIColor is loaded. It's tiresome but has a simple fix.

Adding eval { require Win32::Console::ANSI } after any require/use of Term::ANSIColor is
all that is needed for MSWin32 support. Since Win32::Console::ANSI can only be installed
on MSWin32 platforms, no $^O eq 'MSWin32' test is needed.

@rivy
Copy link
Contributor Author

rivy commented Sep 8, 2016

The additional Perl::Critic directive can be dispensed with by using ...

() = eval { require Win32::Console::ANSI }

if you prefer that implementation.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.656% when pulling 4db68a8 on rivy:fix/MSWin32.color into 0158bb9 on kentnl:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.656% when pulling 6ca7a9d on rivy:fix/MSWin32.color into 0158bb9 on kentnl:master.

@rivy rivy force-pushed the fix/MSWin32.color branch 2 times, most recently from d22bbe5 to 578cda5 Compare September 10, 2016 04:37
@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.656% when pulling 578cda5 on rivy:fix/MSWin32.color into 0158bb9 on kentnl:master.

.# Discussion

Term::ANSIColor doesn't include MSWin32 support by default. And, unfortunately, it seems
that perl-porters don't want to include it (see https://rt.cpan.org/Ticket/Display.html?id=44807
@@ http://archive.is/gqB1N). So, to support the platform, Win32::Console::ANSI must be
used whenever Term::ANSIColor is loaded. It's tiresome but has a simple fix.

Adding `eval { require Win32::Console::ANSI }` after any require/use of Term::ANSIColor is
all that is needed for MSWin32 support. Since Win32::Console::ANSI can only be installed
on MSWin32 platforms, no `$^O eq 'MSWin32'` test is needed. But, since Win32::Console::ANSI
is an optional dependency, keeping the platform test avoids exposing non-'MSWin32' platforms
to the ". in @inc" threat vector.

ref: https://dzone.com/articles/security-separation-of-concerns-and-cve-2016-1238 @@ https://archive.is/sKbcs
@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.656% when pulling 578cda5 on rivy:fix/MSWin32.color into 0158bb9 on kentnl:master.

@rivy
Copy link
Contributor Author

rivy commented Sep 10, 2016

I amended the PR to include the OS check (as well as the changing to a critic-compatible form).

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.656% when pulling f1a6928 on rivy:fix/MSWin32.color into 0158bb9 on kentnl:master.

@kentfredric kentfredric merged commit f1a6928 into kentnl:master Sep 10, 2016
kentfredric added a commit that referenced this pull request Sep 10, 2016
kentfredric added a commit that referenced this pull request Sep 10, 2016
 [Dependencies::Stats]
 - Dependencies changed since 1.000007, see misc/*.deps* for details
 - configure: (suggests: +1)
 - develop: +5 ↑1 -1 (suggests: ↑2)

 [Features / Win32 ANSI Colors]
 - ANSI Escape code colours should render correctly on Win32 if users have Win32::Console::ANSI installed.
 - Patched (with thanks) by Roy Ivy III ( #5 )
@rivy rivy deleted the fix/MSWin32.color branch September 10, 2016 18:49
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.

None yet

3 participants