Re: [PATCH 02/18] Change style of some regular expressions to make them clearer

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

 



Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:

> On Thu, Jun 6, 2013 at 10:30 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
>>
>>>> -                       if (my ($child, $parents) = $line =~ m/^-?([a-f0-9]+) ([a-f0-9 ]+)/) {
>>>> -                               foreach my $parent (split(' ', $parents)) {
>>>> +                       if (my ($child, $parents) = $line =~ /^-?([a-f0-9]+) ([a-f0-9 ]+)/) {
>>>> +                               foreach my $parent (split(/ /, $parents)) {
>>>
>>> This is a behavior-altering change. split(' ',...) is handled as a
>>> special case[*1*] which strips leading whitespace and then splits on
>>> /\s+/ (run of whitespace). Changing it to split(/ /,...) makes it
>>> match only a single space (rather than a run of whitespace).
>>
>> I initially had the same reaction, but this is reading the output of
>> the "rev-list --parents" command, whose fields are separated by one
>> SP each, so there is indeed no behaviour change.
>
> True. This potentially subtle point may deserve mention in the commit
> message (or this particular change can be dropped).

I agree that the log message should mention it; but the resulting
code is more correct than the original, so I do not see a need to
drop it, or add any in-code comment to it.
--
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]