Perl script to print multi line log entries which has a match for given pattern

Question

I am a Java programmer and very new to Perl. I decided to work on a simple script that could solve a problem I encounter in my day to day work. This script will search the log file for a pattern and print the complete potentially multi line log statement that has a match for this pattern. Additionally, the matched pattern should be displayed in color for highlighting purpose.

I've used very basic features of Perl and as you can see, the code very verbose (like Java some might say ;) ) and there are two main areas I'm aiming to improve:

• I want to see Perl's idiomatic ways to make the program less verbose without sacrificing too much readability.
• Any poor practice is that I have used. I purpose fully left out "strict" and "warnings" as I want to use Perl for loose and less verbose features.

I'd be really grateful for some feedback to make this more elegant. I'll only be using Perl 5.8 as that is what available on most systems at my place.

#!/usr/bin/env perl

use Term::ANSIColor;

my $pattern = qr"[.a-zA-Z0-9]*Exception";
my $hilight = 'black on_yellow';
my $timestamp = qr"\d\d:\d\d"; #To identify beginning of new log entry.

my $begin = qr/^$timestamp/;

open(my $fn, '<', $ARGV[0]) or die "Could not open file '$ARGV[0]' $!";
while (<$fn>) {
    if ( $_ =~ m/($begin)/ ) {
        if($found) {
            foreach(@fh) {
                if ( $_ =~ m/($pattern)/ ) {
                    $matched = $1;
                    @parts = split /($pattern)/, $_;
                    foreach(@parts) {
                        if ($_ eq $matched) {
                            print colored($matched, $hilight);
                        } else {
                            print $_;
                        }
                    }
                } else {
                    print $_
                }
            }
        }

        if(defined @fh) {
            undef @fh;
        }
        $found = 0;
    }
    if ( $_ =~ m/($pattern)/ ) {
        $found = 1;
    }
    push (@fh, $_);
}

close $fn;

Answer 1

The code definitely isn't bad. I'd still use strict and warnings, though. Minor remarks:

• Why do you use " for qr? / or {} are canonical.
• $_ is good for short loops, and its advantage is you don't have to type it. So, I'd name the main loop's variable, as its scope is rather wide.
• Avoid deep indentation. Use subroutines.
• defined @fh is deprecated.

My version:

#!/usr/bin/env perl
use warnings;
use strict;

use Term::ANSIColor;

my $pattern = qr/[.a-zA-Z0-9]*Exception/;


sub output {
    my @buffer = @_;
    my $hilight = 'black on_yellow';
    for (@buffer) {
        if (my ($matched) = /($pattern)/) {
            for (split /($pattern)/) {
                print $_ eq $matched ? colored($matched, $hilight) : $_;
            }
        } else {
            print;
        }
    }
}


my $begin   = qr/^\d\d:\d\d/;

my ($found, @buffer);
open my $fh, '<', $ARGV[0] or die "Could not open file '$ARGV[0]' $!";
while (my $line = <$fh>) {
    if ($line =~ /($begin)/) {
        output(@buffer) if @buffer && $found;
        @buffer = ();
        undef $found;
    }
    push @buffer, $line;
    $found = 1 if $line =~ /$pattern/;
}

# Output the last match if it ends with an EOF instead of a timestamp.
output(@buffer) if @buffer && $found;