From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 01 Jul 2013 18:50:30 GMT
Subject: Re: [PATCH/RFC 2/4] contrib: contacts: add support for multiple patches
Message-ID: <7v7ghaglmx.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1372590512-21341-3-git-send-email-sunshine@sunshineco.com>

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, $_);
	}	        

>  import_commits(\%commits);
>  
>  my %count_per_person;