From: Jay Soffian <jaysoffian+git@gmail.com>
Date: Fri, 18 Jan 2008 17:39:45 GMT
Subject: Re: [PATCH 2/3] send-email: validate patches before sending anything
Message-ID: <76718490801180939v12112b5btd71dfb1fb5be5897@mail.gmail.com>
In-Reply-To: <20080118141948.GB19783@coredump.intra.peff.net>

On 1/18/08, Jeff King <peff@peff.net> wrote:

> +foreach my $f (@files) {
> +       my $error = validate_patch($f);
> +       $error and die "fatal: $f: $error\nwarning: no patches were sent\n";
> +}
> +
>  if (@files) {
>         unless ($quiet) {
>                 print $_,"\n" for (@files);
> @@ -837,3 +842,15 @@ sub unique_email_list(@) {
>         }
>         return @emails;
>  }
> +
> +sub validate_patch {
> +       my $fn = shift;
> +       open(my $fh, '<', $fn)
> +               or die "unable to open $fn: $!\n";
> +       while (my $line = <$fh>) {
> +               if (length($line) > 998) {
> +                       return "patch contains line longer than 998 characters";
> +               }
> +       }
> +       return undef;
> +}

How about offering the line number. e.g.:

return "patch line number $. is longer than 998 characters";

> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index 659f9c7..1c41810 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -78,4 +78,24 @@ test_expect_success 'Show all headers' '
>         diff -u expected-show-all-headers actual-show-all-headers
>  '
>
> +test_expect_success 'no patch was sent' '

Shouldn't that be "no patches were sent" to match the perl output?

j.