Re: [PATCH v3] send-email: recognize absolute path on Windows

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Apr 22, 2014 at 6:50 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Erik Faye-Lund <kusmabite@xxxxxxxxx> writes:
>
>>>> 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?
>
> Also somebody earlier mentioned that we would be redefining, which
> has a different kind of ugliness, so I'd agree with the code structure
> of what you sent out (which has been queued on 'pu').
>
> My earlier question "don't we want to make sure 'C:' is at the
> betginning of the string?" still stands, though.  I do not think I
> futzed with your regexp in the version I queued on 'pu'.

Ah, yes of course. Thanks for spotting that. I also like the other
clean-ups you did to the regex (above).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]