From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Tue, 22 Apr 2014 12:15:59 GMT
Subject: Re: [PATCH v3] send-email: recognize absolute path on Windows
Message-ID: <CABPQNSbcWjg3nLPD9U114zSk5rBNupOGLr901u4ptCkdiiKvCA@mail.gmail.com>
In-Reply-To: <xmqqbnw1i43p.fsf@gitster.dls.corp.google.com>

On Wed, Apr 16, 2014 at 7:19 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Erik Faye-Lund <kusmabite@gmail.com> writes:
>>
>>> So let's manually check for these in that case, and fall back to
>>> the File::Spec-helper on other platforms (e.g Win32 with native
>>> Perl)
>>> ...
>>> +sub file_name_is_absolute {
>>> +    my ($path) = @_;
>>> +
>>> +    # msys does not grok DOS drive-prefixes
>>> +    if ($^O eq 'msys') {
>>> +            return ($path =~ m#^/# || $path =~ m#[a-zA-Z]\:#)
>>
>> Shouldn't the latter also be anchored at the beginning of the string
>> with a leading "^"?
>>
>>> +    }
>>> +
>>> +    require File::Spec::Functions;
>>> +    return File::Spec::Functions::file_name_is_absolute($path);
>>
>> We already "use File::Spec qw(something else)" at the beginning, no?
>> Why not throw file_name_is_absolute into that qw() instead?
>
> Ahh, OK, if you did so, you won't have any place to hook the "only
> on msys do this" trick into.
>
> It somehow feels somewhat confusing that we define a sub with the
> same name as the system one, while not overriding it entirely but
> delegate back to the system one.  I am debating myself if it is more
> obvious if it is done this way:
>
>         use File::Spec::Functions qw(file_name_is_absolute);
>         if ($^O eq 'msys') {
>                 sub file_name_is_absolute {
>                         return $_[0] =~ /^\// || $_[0] =~ /^[A-Z]:/i;
>                 }
>         }
>

In this case, we end up requiring that module even when we end up
using it, no? Not that I have very strong objections for doing just
that, after all, it appears to be built-in. (As you might understand
from this message, my perl-fu is really lacking :-P)

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.