Re: [PATCH RFC 3/6] send-email: Handle "GIT:" rather than "GIT: " during --compose

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 11, 2009, 19:22 UTC
Message-ID: <7vprfjf11h.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1239139522-24118-3-git-send-email-mfwitten@gmail.com>

Michael Witten <mfwitten@gmail.com> writes:

> This should make things a little more robust in terms of user input;
> before, even the program got it wrong by outputting a line with only
> "GIT:", which was left in place as a header, because there would be
> no following space character.

An alternative could be to add an extra space after the "GIT:" on the lines the compose template generated by this program, but people can set their editors to strip trailing whitespaces, so I think yours is a better approach. I suspect this patch comes from your own experience of getting bitten by this once, perhaps?

> Also, I cleaned up get_patch_subject().

Which is a bit iffy. It does not belong to the primary topic of the patch to begin with, so it shouldn't be in here even if it weren't iffy.

Show 27 quoted lines

> diff --git a/git-send-email.perl b/git-send-email.perl
> index 63d6063..098c620 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -505,15 +505,16 @@ if (@files) {
>  }
>  
>  sub get_patch_subject($) {
> -	my $fn = shift;
> -	open (my $fh, '<', $fn);
> -	while (my $line = <$fh>) {
> -		next unless ($line =~ /^Subject: (.*)$/);
> -		close $fh;
> -		return "GIT: $1\n";
> +
> +	my $patch = shift;
> +	open (my $fh, '<', $patch);
> +
> +	while (<$fh>) {
> +		next unless (/^Subject: (.*)$/);
> +		return $1;
>  	}
> -	close $fh;
> -	die "No subject line in $fn ?";
> +
> +	die "'Subject:' line expected in '$patch'";
>  }

Because "while (<>)" does not localize $_, you are clobbering it in the caller's context. I do not know if any of the the existing callers cares, but it is a change in behaviour.

$ cat >/var/tmp/j.perl <<\EOF
#!/usr/bin/perl -w
use strict;
sub foo($) {
	my $name = shift;
	open my $fh, "<$name";
	while (my $line = <$fh>) {
		chomp $line;
		close $fh;
		return $line;
	}
	close $fh;
	return undef;
}
sub bar($) {
	my $name = shift;
	open my $fh, "<$name";
	while (<$fh>) {
		chomp;
		close $fh;
		return $_;
	}
	close $fh;
	return undef;
}
$_ = 'original';
foo($0);
print "after running foo: $_\n";

$_ = 'original'; bar($0); print "after running bar: $_\n"; EOF $ perl /var/tmp/j.perl after running foo: original after running bar: #!/usr/bin/perl -w $ exit