Re: [PATCH/RFC 2/4] contrib: contacts: add support for multiple patches

> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> Accept multiple patch files rather than only one. For example:
>>
>>   % git contacts feature/*.patch
>>
>> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
>
>> @@ -93,6 +96,7 @@ sub commits_from_patch {
>>       while (<$f>) {
>>               if (/^From ($id_rx) /o) {
>>                       $id = $1;
>> +                     $seen{$id} = 1;
>>                       last;
>>               }
>>       }
>
> This looks less useful than it could be.
>
>         $ git format-patch --stdout -4 >P.mbox
>         $ git contacts P.mbox
>
> would have the same number of patches but in a single file.
>
> Wouldn't it be more useful to do something like
>
>         $id = undef;
>         while (<$f>) {
>                 if (/^From ([0-9a-f]{40}) Mon Sep 17 00:00:00 2001$/) {
>                         # beginning of a patch
>                         $id = $1;
>                 }
>                 next if (!defined $id);
>                 # inline the body of scan_hunks here...
>                 if (m{^--- (a/.*|/dev/null)$}) {
>                         $source = ...
>                 } elsif (/^@@ -(\d+)...) {
>                         get_blame();
>                 }
>         }

>> @@ -100,10 +104,8 @@ sub commits_from_patch {
>>       close $f;
>>  }
>>
>> -exit 1 unless @ARGV == 1;
>> -
>>  my %commits;
>> -commits_from_patch(\%commits, $ARGV[0]);
>> +commits_from_patch(\%commits, $_) for (@ARGV);
>
> This change does not seem to account for an invocation without any
> argument.  Perhaps write it like so to make it more readable?
>
>         if (!@ARGV) {
>                 die "No input file?\n";
>         }
>
>         for (@ARGV) {
>                 commits_from_patch(\%commits, $_);
>         }