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;

• choroba: I'd put strict and warnings back for anything longer than 3 lines. In case of this script, you only need to decide the scope of $found and @fh.

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;

• sadiq.ali: Good. I did not knew how to pass array to sub routines and script kind of grew to deep nesting. I think you forgot to print $_. The first print statement should be print $_ eq $matched ? colored($matched, $hilight) : $_;
• choroba: @sadiq.ali: print without parameters prints $_. I tested my script before posting.
• sadiq.ali: I meant the print statement where you are using ternary operator to print colored text. In case of false, we have to print uncolored output, but you are giving empty set (). I ran in local system and for lines with matching pattern, your program is only printing matched pattern and ignoring other parts of the line.
• choroba: @sadiq.ali: Ah, you're right.
• simbabque: I'd also rename $fn to $fh because that's more canonical. In the original code I got confused because of @fh and $fn.