Re^4: Please critique this script -- Read emails and write URLs into bookmarks

I light of my brainfart yesterday I felt compelled to show a revised script including proper error handling:

I blame it on the meds!

#!/usr/bin/perl

# read emails I sent myself containing just a link
# and make a HTML-redirect based bookmark out of them
#
# SPEC
# read given files as internet messages
# use subject as title and filename, escape as needed
# use one URL in body as href (if there a two they are both the same)
# and output one HTML file for each bookmark

use strict;
use warnings;
use Carp qw(cluck confess);
use Encode;

use Email::Simple;
use HTML::Entities;
use Path::Tiny;
use URI::Find;

use version; our $VERSION = qv('0.0.1');

if ( @ARGV == 0 ) { confess 'Invalid number of arguments' }

foreach my $in_filepath (@ARGV) {

    if ( not path($in_filepath)->exists ) {
        cluck "$in_filepath: Input file '$in_filepath' does not exist"
+;
        next;
    }

    my $in_file = Path::Tiny->new($in_filepath)->slurp
      or confess "$in_filepath: Fatal error in module";
    my $message = Email::Simple->new($in_file)
      or confess "$in_filepath: Fatal error in module";

    if ( not length $message->as_string
        or $message->as_string =~ /\A\s*\z/ )
    {
        cluck "$in_filepath: Invalid message";
        next;
    }
    if ( not length $message->header('Subject') ) {
        cluck "$in_filepath: Invalid subject";
        next;
    }

    my $subject = decode( 'MIME-Header', $message->header('Subject') )
      or confess "$in_filepath: Fatal error in module";
    my $title = HTML::Entities::encode($subject)
      or confess "$in_filepath: Fatal error in module";
    my $out_filepath = $subject =~ s/[%\/\?<>\\:\*\|":]/_/gr . '.URL.H
+TML';

    # TODO IMPR URL::Search might be nicer than URI::Find for some cas
+es
    my @uris;
    my $finder = URI::Find->new( sub { push @uris, $_[0] } );
    $finder->find( \$message->body );

    if ( @uris == 0 ) { cluck "$in_filepath: Invalid number of URIs"; 
+next; }
    if ( path($out_filepath)->exists ) {
        cluck "$in_filepath: Output file '$out_filepath' exists";
        next;
    }

    path($out_filepath)->spew(
"<!DOCTYPE html><title>$title</title><meta http-equiv=\"refresh\" cont
+ent=\"0; url=@uris\">",
    ) or confess "$in_filepath: Fatal error in module";

}
[download]

Comment on Re^4: Please critique this script -- Read emails and write URLs into bookmarks Download Code

Replies are listed 'Best First'.
Re^5: Please critique this script -- Read emails and write URLs into bookmarks by talexb (Chancellor) on Oct 07, 2016 at 20:47 UTC
Rather than `if ( not length $message->as_string or $message->as_string =~ /\A\s\z/ ) { cluck "$in_filepath: Invalid message"; next; }` [download] I would prefer something more along the lines of this: `my @errors; length $message->as_string or push ( @errors, 'Message empty' ); $message->as_string =~ /\A\s\z/ or push ( @errors, 'Invalid message' ); if ( @errors ) { foreach my $e ( @errors ) { cluck "in_filepath: $e"; } next; }` [download] Also, I would recommend `\|\|` to or two conditionals together, rather than `or`. Alex / talexb / Toronto Thanks PJ. We owe you so much. Groklaw -- RIP -- 2003 to 2013.	[reply] [d/l] [select]

Replies are listed 'Best First'.

Re^5: Please critique this script -- Read emails and write URLs into bookmarks
by talexb (Chancellor) on Oct 07, 2016 at 20:47 UTC

Rather than

if ( not length $message->as_string
  or $message->as_string =~ /\A\s*\z/ )
{
    cluck "$in_filepath: Invalid message";
    next;
}
[download]

my @errors;

length $message->as_string or
  push ( @errors, 'Message empty' );
$message->as_string =~ /\A\s*\z/ or
  push ( @errors, 'Invalid message' );

if ( @errors ) {

  foreach my $e ( @errors ) { cluck "in_filepath: $e"; }
  next;
}
[download]

||

or

Alex / talexb / Toronto

Thanks PJ. We owe you so much. Groklaw -- RIP -- 2003 to 2013.

[reply]
[d/l]
[select]