Re: [PATCH 2/5] git send-email: interpret unknown files as revision lists

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 4, 2008, 23:54 UTC
Message-ID: <7v1vxroxn1.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1225815858-30617-3-git-send-email-madcoder@debian.org>

Pierre Habouzit <madcoder@debian.org> writes:

Show 9 quoted lines

> diff --git a/git-send-email.perl b/git-send-email.perl
> index aaace02..c29868a 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -22,8 +22,11 @@ use Term::ReadLine;
>  use Getopt::Long;
>  use Data::Dumper;
>  use Term::ANSIColor;
> +use File::Temp qw/ tempdir /;

We seem to use File::Temp::tempdir already elsewhere, but they are in archimport, cvsexportcommit and cvsserver, all of which are rather rarely used ones. I think this is Perl 5.6.1 addition. Is everybody Ok with this dependency? Just double checking.

Show 8 quoted lines

> @@ -363,10 +366,22 @@ if (@alias_files and $aliasfiletype and defined $parse_alias{$aliasfiletype}) {
>  
>  ($sender) = expand_aliases($sender) if defined $sender;
>  
> +sub check_file_rev_conflict($) {
> +	my $f = shift;
> +	if ($repo->command('rev-parse', '--verify', '--quiet', $f)) {
> +		die("revision/filename conflict on `$f'");

Perhaps wording this a bit more to the point? This is triggered when '$f' can be both a filename or a revision, so...

	File '$f' exists but it could also be the range of commits
        to produce patches for.  Please disambiguate by...

	* Saying "./$f" if you mean a file; or
        * Giving -F option if you mean a range.

Earlier I suggested that "origin^0" is a way for the user to disambiguate favouring a rev, but such a filename can exist, so we cannot blindly suggest to say "$f^0" here. I think adding -F (or --format-patch) option to send-email to explicitly disable file/directory interpretation would be a cleaner solution for this (and it would allow you to drive this from a script without worrying about what garbage files you happen to have in the working tree).