public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
* [PATCH] send-email: use catfile() to concatenate files
@ 2010-09-14 19:02 Ævar Arnfjörð Bjarmason
  2010-09-14 19:26 ` Junio C Hamano
  0 siblings, 1 reply; 5+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-09-14 19:02 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

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(-)

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] send-email: use catfile() to concatenate files
  2010-09-14 19:02 [PATCH] send-email: use catfile() to concatenate files Ævar Arnfjörð Bjarmason
@ 2010-09-14 19:26 ` Junio C Hamano
  2010-09-14 19:55   ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2010-09-14 19:26 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git

Æ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.

>  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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] send-email: use catfile() to concatenate files
  2010-09-14 19:26 ` Junio C Hamano
@ 2010-09-14 19:55   ` Ævar Arnfjörð Bjarmason
  2010-09-14 20:35     ` Jakub Narebski
  0 siblings, 1 reply; 5+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-09-14 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] send-email: use catfile() to concatenate files
  2010-09-14 19:55   ` Ævar Arnfjörð Bjarmason
@ 2010-09-14 20:35     ` Jakub Narebski
  2010-09-14 20:42       ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 5+ messages in thread
From: Jakub Narebski @ 2010-09-14 20:35 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Junio C Hamano, git

Æ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".

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] send-email: use catfile() to concatenate files
  2010-09-14 20:35     ` Jakub Narebski
@ 2010-09-14 20:42       ` Ævar Arnfjörð Bjarmason
  0 siblings, 0 replies; 5+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-09-14 20:42 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Junio C Hamano, git

On Tue, Sep 14, 2010 at 20:35, Jakub Narebski <jnareb@gmail•com> wrote:
> Æ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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-09-14 20:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-14 19:02 [PATCH] send-email: use catfile() to concatenate files Ævar Arnfjörð Bjarmason
2010-09-14 19:26 ` Junio C Hamano
2010-09-14 19:55   ` Ævar Arnfjörð Bjarmason
2010-09-14 20:35     ` Jakub Narebski
2010-09-14 20:42       ` Ævar Arnfjörð Bjarmason

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox