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

Calling Term::ANSIColor::color when coloured is disabled #112

Closed
mjegh opened this issue Apr 5, 2017 · 5 comments
Closed

Calling Term::ANSIColor::color when coloured is disabled #112

mjegh opened this issue Apr 5, 2017 · 5 comments

Comments

@mjegh
Copy link

mjegh commented Apr 5, 2017

I am getting an occasional use of uninitialized variable:

Use of uninitialized value in block entry at /home/feed/perl5/perlbrew/perls/perl-5.22.2/lib/5.22.2/Term/ANSIColor.pm line 368, <DATA> line 1.

followed by a core dump. I've tracked it down to calls from Data::Printer to color(). I have specifically disabled colored output in my .dataprinter. I have also set colored => 0 when I import Data::Printer. However, it seems Data::Printer calls color no matter in

sub _data_printer {
    my $wantarray = shift;

    croak 'When calling p() without prototypes, please pass arguments as references'
        unless ref $_[0];
    my ($item, %local_properties) = @_;
    local %ENV = %ENV;
    my $p = _merge(\%local_properties);

    unless ($p->{multiline}) {
        $p->{'_linebreak'} = ' ';
        $p->{'indent'}     = 0;
        $p->{'index'}      = 0;
    }

    # We disable colors if colored is set to false.
    # If set to "auto", we disable colors if the user
    # set ANSI_COLORS_DISABLED or if we're either
    # returning the value (instead of printing) or
    # being piped to another command.
    if ( !$p->{colored}
          or ($p->{colored} eq 'auto'
              and (exists $ENV{ANSI_COLORS_DISABLED}
                   or $wantarray
                   or not -t $p->{_output}
                  )
          )
    ) {
        $ENV{ANSI_COLORS_DISABLED} = 1;
    }
    else {
        delete $ENV{ANSI_COLORS_DISABLED};
    }

    my $out = color('reset');

    if ( $p->{caller_info} and $p->{_depth} == 0 ) {
        $out .= _get_info_message($p);
    }

    $out .= _p( $item, $p );
    return ($out, $p);
}

Data::Printer seems to be sprinkled with calls to color (e.g., _escape_chars) which get called even though colored => 0.

Could you please make colored => 0 actually do what it is documented as doing and not call color.

@garu
Copy link
Owner

garu commented Mar 19, 2018

Hi @mjegh! Thank you for the report.

colored => 0 is doing what it is documented as doing, which is not printing any coloured output at all. It never said what function is or isn't called. The decision to output colors (or not) comes from Term::ANSIColor itself, which should respect ANSI_COLORS_DISABLED as the code you highlighted sets it and not print any warnings. So maybe this is an issue with Term::ANSIColor and not with Data::Printer.

Could you please let us know what version of Term::ANSIColor you have installed? Currently, we require 3.0 or greater, but maybe there is an issue there that we are able to track down and fix. Anything else you can provide to help us track down and fix the issue would be greatly appreciated.

Thank you!

@mjegh
Copy link
Author

mjegh commented Mar 20, 2018

colored => 0 is doing what it is documented as doing, which is not printing any coloured output at all.
It never said what function is or isn't called.

It could just as easily be argued it isn't printing an colour because it is core dumping but I'll take your point as I believe you intended it.

perl -MTerm::ANSIColor -le 'print $Term::ANSIColor::VERSION;'
4.04

I reported this almost a year ago and I don't believe I can even reproduce it now. When we got nowhere with it I believe we hacked Term::ANSIColor to be one big NOOP.

I had forgotten about this issue and we eventually made it go away so you can probably close it now.

@garu
Copy link
Owner

garu commented Mar 21, 2018

Thank you for the quick reply! I'll look into it, and I'm also refactoring the code so it will require Term::ANSIColor on runtime and only if you actually requested a colored output.

@mjegh
Copy link
Author

mjegh commented Mar 23, 2018

A big ++ for using require.

@garu
Copy link
Owner

garu commented May 14, 2018

Hey @mjegh! The new version on github master (dev release 0.99_X on CPAN, soon to be 1.00) has dropped Term::ANSIColor entirely and now the entire coloring process - or lack thereof - should be much more robust.

Thanks again for the report and let me know if you bump into any issues.

Cheers!

@garu garu closed this as completed May 14, 2018
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

No branches or pull requests

2 participants