On Mon, Jul 1, 2013 at 2:50 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
>
>> Accept multiple patch files rather than only one. For example:
>>
>> % git contacts feature/*.patch
>>
>> Signed-off-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
>
>> @@ -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();
> }
> }
Good point. It did not occur to me that a single file might contain
multiple patches. Felipe's script handled this case correctly.
>> @@ -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, $_);
> }
Emitting a diagnostic about missing input makes sense.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html