On Sat, Jan 25, 2014 at 01:46:50PM +0400, Brilliantov Kirill Vladimirovich 
wrote:

> +     if (!defined $smtp_server) {
> +             my $mailrc = File::HomeDir->my_home . "/.mailrc";

The new module dependency has been discussed elsewhere in the thread.

> +             if (-e $mailrc) {
> +                     open FILE, $mailrc or die "Failed open $mailrc: $!";

Please use the three-argument of 'open', and use a regular scalar
instead of a glob. Both are safer, and we assume a modern enough perl to
support both. I.e.:

  open(my $file, '<', $mailrc)
          or die "failed to open $mailrc: $!";

> +                     while (<FILE>) {
> +                         chomp;
> +                         if (/set sendmail=.*/) {
> +                             my @data = split '"';
> +                             $smtp_server = $data[1];
> +                             last;
> +                         }

Your split is a rather unusual way to do the parsing, and it took me a
minute to figure it out. It might be more obvious as:

  if (/set sendmail="(.*)"/) {
          $smtp_server = $1;
          last;
  }

I do not know anything about the mailrc format, nor does it seem to be
well documented. Are the double-quotes required? If not, then the above
regex can easily make them optional. I also wonder if any whitespace is
allowed. E.g., this might be more forgiving:

  /set sendmail\s*=\s*"?(.*?)"?/

but I am just guessing at what the format allows.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to