All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Michael Witten <mfwitten@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH RFC 3/6] send-email: Handle "GIT:" rather than "GIT: " during --compose
Date: Sat, 11 Apr 2009 12:22:34 -0700	[thread overview]
Message-ID: <7vprfjf11h.fsf@gitster.siamese.dyndns.org> (raw)
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.

> 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

  parent reply	other threads:[~2009-04-11 19:24 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-07 21:25 [PATCH RFC 1/6] send-email: Add --delay for separating emails Michael Witten
2009-04-07 21:25 ` [PATCH RFC 2/6] send-email: --smtp-server-port should take an integer Michael Witten
2009-04-07 21:25   ` [PATCH RFC 3/6] send-email: Handle "GIT:" rather than "GIT: " during --compose Michael Witten
2009-04-07 21:25     ` [PATCH RFC 4/6] send-email: --compose takes optional argument to existing file Michael Witten
2009-04-07 21:25       ` [PATCH RFC 5/6] send-email: Cleanup the usage text a bit Michael Witten
2009-04-07 21:25         ` [PATCH RFC 6/6] send-email: Remove horrible mix of tabs and spaces Michael Witten
2009-04-07 21:35           ` demerphq
2009-04-07 21:42             ` Michael Witten
2009-04-07 21:44               ` demerphq
2009-04-07 21:57                 ` demerphq
2009-04-07 22:00               ` Jeff King
2009-04-07 22:10                 ` Andreas Ericsson
2009-04-07 23:33                   ` Tomas Carnecky
2009-04-08  2:02                   ` Jeff King
2009-04-11 19:22         ` [PATCH RFC 5/6] send-email: Cleanup the usage text a bit Junio C Hamano
2009-04-11 19:22       ` [PATCH RFC 4/6] send-email: --compose takes optional argument to existing file Junio C Hamano
2009-04-11 19:22     ` Junio C Hamano [this message]
2009-04-11 20:45       ` [PATCH RFC 3/6] send-email: Handle "GIT:" rather than "GIT: " during --compose Michael Witten
2009-04-12  0:59         ` Junio C Hamano
2009-04-12  2:36           ` Michael Witten
2009-04-07 23:20   ` [PATCH RFC 2/6] send-email: --smtp-server-port should take an integer Junio C Hamano
2009-04-11 19:22   ` Junio C Hamano
2009-04-11 21:01     ` Wesley J. Landaker
2009-04-11 21:07       ` Michael Witten
2009-04-07 21:51 ` [PATCH RFC 1/6] send-email: Add --delay for separating emails Jeff King
2009-04-07 22:08   ` [PATCH RFC 1/6] " Nicolas Sebrecht
2009-04-07 22:17     ` Andreas Ericsson
2009-04-08  6:05       ` Jeff King
2009-04-08  6:03     ` Jeff King
2009-04-07 23:17 ` [PATCH RFC 1/6] " Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vprfjf11h.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=mfwitten@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.