From mboxrd@z Thu Jan 1 00:00:00 1970 From: Junio C Hamano Subject: Re: [PATCH RFC 3/6] send-email: Handle "GIT:" rather than "GIT: " during --compose Date: Sat, 11 Apr 2009 12:22:34 -0700 Message-ID: <7vprfjf11h.fsf@gitster.siamese.dyndns.org> References: <1239139522-24118-1-git-send-email-mfwitten@gmail.com> <1239139522-24118-2-git-send-email-mfwitten@gmail.com> <1239139522-24118-3-git-send-email-mfwitten@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: git@vger.kernel.org To: Michael Witten X-From: git-owner@vger.kernel.org Sat Apr 11 21:24:16 2009 Return-path: Envelope-to: gcvg-git-2@gmane.org Received: from vger.kernel.org ([209.132.176.167]) by lo.gmane.org with esmtp (Exim 4.50) id 1LsioG-0004cm-OS for gcvg-git-2@gmane.org; Sat, 11 Apr 2009 21:24:13 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758833AbZDKTWl (ORCPT ); Sat, 11 Apr 2009 15:22:41 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1758655AbZDKTWl (ORCPT ); Sat, 11 Apr 2009 15:22:41 -0400 Received: from a-sasl-fastnet.sasl.smtp.pobox.com ([207.106.133.19]:50151 "EHLO sasl.smtp.pobox.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758740AbZDKTWk (ORCPT ); Sat, 11 Apr 2009 15:22:40 -0400 Received: from localhost.localdomain (unknown [127.0.0.1]) by a-sasl-fastnet.sasl.smtp.pobox.com (Postfix) with ESMTP id AE317A9E5A; Sat, 11 Apr 2009 15:22:39 -0400 (EDT) Received: from pobox.com (unknown [68.225.240.211]) (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by a-sasl-fastnet.sasl.smtp.pobox.com (Postfix) with ESMTPSA id 82D7BA9E59; Sat, 11 Apr 2009 15:22:36 -0400 (EDT) User-Agent: Gnus/5.110006 (No Gnus v0.6) Emacs/21.4 (gnu/linux) X-Pobox-Relay-ID: 1F3D1E38-26CE-11DE-AB86-C121C5FC92D5-77302942!a-sasl-fastnet.pobox.com Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Archived-At: Michael Witten 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. > 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