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

Eric Sunshine [off-list ref] writes:

quoted

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

On Thu, Jun 6, 2013 at 10:30 PM, Junio C Hamano [off-list ref] wrote:

Eric Sunshine [off-list ref] writes:

quoted

quoted

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