threads / patch / 25102

patchsend-email: use catfile() to concatenate files

Subject: [PATCH] send-email: use catfile() to concatenate files

## tl;dr

5 messages between Sep 14, 2010 and Sep 14, 2010. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Ævar Arnfjörð Bjarmason· Sep 14, 2010, 19:02 UTC · lore

Change send-email to use Perl's catfile() function instead of "$dir/$file". If send-email is given a $dir that ends with a / we'll end up printing a double slashed path like "dir//mtfnpy.patch".

This doesn't cause any problems since Perl's IO layer will handle it, but it looks ugly.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 git-send-email.perl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

Show changes to git-send-email.perl +2 −1

diff --git a/git-send-email.perl b/git-send-email.perl
index 6dab3bf..7f702e3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -24,6 +24,7 @@ use Text::ParseWords;
 use Data::Dumper;
 use Term::ANSIColor;
 use File::Temp qw/ tempdir tempfile /;
+use File::Spec::Functions qw(catfile);
 use Error qw(:try);
 use Git;
 
@@ -511,7 +512,7 @@ while (defined(my $f = shift @ARGV)) {
 		opendir(DH,$f)
 			or die "Failed to opendir $f: $!";
 
-		push @files, grep { -f $_ } map { +$f . "/" . $_ }
+		push @files, grep { -f $_ } map { catfile($f, $_) }
 				sort readdir(DH);
 		closedir(DH);
 	} elsif ((-f $f or -p $f) and !check_file_rev_conflict($f)) {

-- 
1.7.3.rc1.234.g8dc15

Junio C Hamano· Sep 14, 2010, 19:26 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] send-email: use catfile() to concatenate files

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 8 quoted lines

> Change send-email to use Perl's catfile() function instead of
> "$dir/$file". If send-email is given a $dir that ends with a / we'll
> end up printing a double slashed path like "dir//mtfnpy.patch".
>
> This doesn't cause any problems since Perl's IO layer will handle it,
> but it looks ugly.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

If you mentioned in the proposed log message that File::Spec::Functions have been with us since Perl 5.6.1, it would have saved me (and others) some time worrying about the portability issues.

Show 26 quoted lines

>  git-send-email.perl |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 6dab3bf..7f702e3 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -24,6 +24,7 @@ use Text::ParseWords;
>  use Data::Dumper;
>  use Term::ANSIColor;
>  use File::Temp qw/ tempdir tempfile /;
> +use File::Spec::Functions qw(catfile);
>  use Error qw(:try);
>  use Git;
>  
> @@ -511,7 +512,7 @@ while (defined(my $f = shift @ARGV)) {
>  		opendir(DH,$f)
>  			or die "Failed to opendir $f: $!";
>  
> -		push @files, grep { -f $_ } map { +$f . "/" . $_ }
> +		push @files, grep { -f $_ } map { catfile($f, $_) }
>  				sort readdir(DH);
>  		closedir(DH);
>  	} elsif ((-f $f or -p $f) and !check_file_rev_conflict($f)) {
> -- 
> 1.7.3.rc1.234.g8dc15

Ævar Arnfjörð Bjarmason· Sep 14, 2010, 19:55 UTC · re: Junio C Hamano · lore

Re: [PATCH] send-email: use catfile() to concatenate files

On Tue, Sep 14, 2010 at 19:26, Junio C Hamano <gitster@pobox.com> wrote:

Show 14 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> Change send-email to use Perl's catfile() function instead of
>> "$dir/$file". If send-email is given a $dir that ends with a / we'll
>> end up printing a double slashed path like "dir//mtfnpy.patch".
>>
>> This doesn't cause any problems since Perl's IO layer will handle it,
>> but it looks ugly.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>
> If you mentioned in the proposed log message that File::Spec::Functions
> have been with us since Perl 5.6.1, it would have saved me (and others)
> some time worrying about the portability issues.

I thought you might trust me to write portable code by default :)

Anyway, I forgot to mention it. But one can use the corelist(1) program to see when what modules appeared in perl core:

    $ corelist File::Spec::Functions
    File::Spec::Functions was first released with perl 5.00504

Jakub Narebski· Sep 14, 2010, 20:35 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] send-email: use catfile() to concatenate files

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 23 quoted lines

> On Tue, Sep 14, 2010 at 19:26, Junio C Hamano <gitster@pobox.com> wrote:
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>
>>> Change send-email to use Perl's catfile() function instead of
>>> "$dir/$file". If send-email is given a $dir that ends with a / we'll
>>> end up printing a double slashed path like "dir//mtfnpy.patch".
>>>
>>> This doesn't cause any problems since Perl's IO layer will handle it,
>>> but it looks ugly.
>>>
>>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>>
>> If you mentioned in the proposed log message that File::Spec::Functions
>> have been with us since Perl 5.6.1, it would have saved me (and others)
>> some time worrying about the portability issues.
> 
> I thought you might trust me to write portable code by default :)
> 
> Anyway, I forgot to mention it. But one can use the corelist(1)
> program to see when what modules appeared in perl core:
> 
>     $ corelist File::Spec::Functions
>     File::Spec::Functions was first released with perl 5.00504

Also using File::Spec / File::Spec::Functions makes code more portable (with respect to things such as different directory separators, and volume portion of pathname) than using "$dir/$file".

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Ævar Arnfjörð Bjarmason· Sep 14, 2010, 20:42 UTC · re: Jakub Narebski · lore

Re: [PATCH] send-email: use catfile() to concatenate files

On Tue, Sep 14, 2010 at 20:35, Jakub Narebski <jnareb@gmail.com> wrote:

Show 28 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>> On Tue, Sep 14, 2010 at 19:26, Junio C Hamano <gitster@pobox.com> wrote:
>>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>>
>>>> Change send-email to use Perl's catfile() function instead of
>>>> "$dir/$file". If send-email is given a $dir that ends with a / we'll
>>>> end up printing a double slashed path like "dir//mtfnpy.patch".
>>>>
>>>> This doesn't cause any problems since Perl's IO layer will handle it,
>>>> but it looks ugly.
>>>>
>>>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>>>
>>> If you mentioned in the proposed log message that File::Spec::Functions
>>> have been with us since Perl 5.6.1, it would have saved me (and others)
>>> some time worrying about the portability issues.
>>
>> I thought you might trust me to write portable code by default :)
>>
>> Anyway, I forgot to mention it. But one can use the corelist(1)
>> program to see when what modules appeared in perl core:
>>
>>     $ corelist File::Spec::Functions
>>     File::Spec::Functions was first released with perl 5.00504
>
> Also using File::Spec / File::Spec::Functions makes code more portable
> (with respect to things such as different directory separators, and
> volume portion of pathname) than using "$dir/$file".

Right, but perl mitigates that to some extent by converting Unix paths on-the-fly to whatever the native platform uses. But of course that falls down in cases like Win32 volume labels.