opal · Mar 24, 2014 · Mar 24, 2014
Showing with 102 additions and 55 deletions.

+100 −45 opal/corelib/string.rb

+0 −9 spec/opal/filters/bugs/string.rb

+2 −1 spec/opal/filters/unsupported/encoding.rb
diff --git a/opal/corelib/string.rb b/opal/corelib/string.rb
@@ -599,71 +599,126 @@ def scan(pattern, &block)
 
   def split(pattern = $; || ' ', limit = undefined)
     %x{
+      if (pattern === nil || pattern === undefined) {
+        pattern = #{$;};
+      }
+
+      var result = [];
+      if (limit !== undefined) {
+        limit = #{Opal.coerce_to!(limit, Integer, :to_int)};
+      }
+
+      if (self.length === 0) {
+        return [];
+      }
+
+      if (limit === 1) {
+        return [self];
+      }
+
       if (pattern && pattern._isRegexp) {
-          return self.split(pattern, limit);
-        } else {
-          result = [],
-          splitted = start = lim = 0,
-          splat = #{Opal.try_convert(pattern, String, :to_str).to_s};
+        var pattern_str = pattern.toString();
 
-          if (undefined !== limit) {
-            lim = #{Opal.try_convert(limit, Integer, :to_int)};
-          }
+        /* Opal and JS's repr of an empty RE. */
+        var blank_pattern = (pattern_str.substr(0, 3) == '/^/') ||
+                  (pattern_str.substr(0, 6) == '/(?:)/');
 
-          if (pattern === nil) {
-            if (#{$;} === undefined || #{$;} === nil) {
-              splat = ' ';
-            }
-            else {
-              splat = #{$;};
-            }
+        /* This is our fast path */
+        if (limit === undefined || limit === 0) {
+          result = self.split(blank_pattern ? /(?:)/ : pattern);
+        }
+        else {
+          /* RegExp.exec only has sane behavior with global flag */
+          if (! pattern.global) {
+            pattern = eval(pattern_str + 'g');
           }
 
-          if (lim == 1) {
-            if (self.length == 0) {
-              return [];
-            }
-            else {
-              return [self];
-            }
-          }
+          var match_data;
+          var prev_index = 0;
+          pattern.lastIndex = 0;
+
+          while ((match_data = pattern.exec(self)) !== null) {
+            var segment = self.slice(prev_index, match_data.index);
+            result.push(segment);
+
+            prev_index = pattern.lastIndex;
 
-          string = (splat == ' ') ? self.replace(/[\r\n\t\v]\s+/g, ' ')
-                                  : self;
+            if (match_data[0].length === 0) {
+              if (blank_pattern) {
+                /* explicitly split on JS's empty RE form.*/
+                pattern = /(?:)/;
+              }
+
+              result = self.split(pattern);
+              /* with "unlimited", ruby leaves a trail on blanks. */
+              if (limit !== undefined && limit < 0 && blank_pattern) {
+                result.push('');
+              }
 
-          while ((cursor = string.indexOf(splat, start)) > -1 && cursor < string.length) {
-            if (splitted + 1 == lim) {
+              prev_index = undefined;
               break;
             }
 
-            if (splat == ' ' && cursor == start) {
-              start = cursor + 1;
-              continue;
+            if (limit !== undefined && limit > 1 && result.length + 1 == limit) {
+              break;
             }
+          }
 
-            result.push(string.substr(start, splat.length ? cursor - start : 1));
-            splitted++;
+          if (prev_index !== undefined) {
+            result.push(self.slice(prev_index, self.length));
+          }
+        }
+      }
+      else {
+        var splitted = 0, start = 0, lim = 0;
 
-            start = cursor + (splat.length ? splat.length : 1);
+        if (pattern === nil || pattern === undefined) {
+          pattern = ' '
+        } else {
+          pattern = #{Opal.try_convert(pattern, String, :to_str).to_s};
+        }
+
+        var string = (pattern == ' ') ? self.replace(/[\r\n\t\v]\s+/g, ' ')
+                                      : self;
+        var cursor = -1;
+        while ((cursor = string.indexOf(pattern, start)) > -1 && cursor < string.length) {
+          if (splitted + 1 === limit) {
+            break;
           }
 
-          if (string.length > 0 && (limit || lim < 0 || string.length > start)) {
-            if (string.length == start) {
-              result.push('');
-            }
-            else {
-              result.push(string.substr(start, string.length));
-            }
+          if (pattern == ' ' && cursor == start) {
+            start = cursor + 1;
+            continue;
           }
 
-          if (limit === undefined || lim == 0) {
-            while (result.length > 0 && result[result.length - 1].length == 0) {
-              result.pop();
-            }
+          result.push(string.substr(start, pattern.length ? cursor - start : 1));
+          splitted++;
+
+          start = cursor + (pattern.length ? pattern.length : 1);
+        }
+
+        if (string.length > 0 && (limit < 0 || string.length > start)) {
+          if (string.length == start) {
+            result.push('');
           }
+          else {
+            result.push(string.substr(start, string.length));
+          }
+        }
+      }
 
-          return result;
+      if (limit === undefined || limit === 0) {
+        while (result[result.length-1] === '') {
+          result.length = result.length - 1;
         }
+      }
+
+      if (limit > 0) {
+        var tail = result.slice(limit - 1).join('');
+        result.splice(limit - 1, result.length - 1, tail);
+      }
+
+      return result;
     }
   end
 

diff --git a/spec/opal/filters/bugs/string.rb b/spec/opal/filters/bugs/string.rb
@@ -163,18 +163,9 @@
   fails "String#slice with Regexp, group"
 
   fails "String#split with String returns subclass instances based on self"
-  fails "String#split with Regexp divides self on regexp matches"
-  fails "String#split with Regexp treats negative limits as no limit"
-  fails "String#split with Regexp suppresses trailing empty fields when limit isn't given or 0"
-  fails "String#split with Regexp returns an array with one entry if limit is 1: the original string"
-  fails "String#split with Regexp returns at most limit fields when limit > 1"
-  fails "String#split with Regexp defaults to $; when regexp isn't given or nil"
-  fails "String#split with Regexp splits between characters when regexp matches a zero-length string"
   fails "String#split with Regexp respects $KCODE when splitting between characters"
   fails "String#split with Regexp includes all captures in the result array"
   fails "String#split with Regexp does not include non-matching captures in the result array"
-  fails "String#split with Regexp tries converting limit to an integer via to_int"
-  fails "String#split with Regexp returns a type error if limit can't be converted to an integer"
   fails "String#split with Regexp returns subclass instances based on self"
   fails "String#split with Regexp does not call constructor on created subclass instances"
   fails "String#split with String does not call constructor on created subclass instances"

diff --git a/spec/opal/filters/unsupported/encoding.rb b/spec/opal/filters/unsupported/encoding.rb
@@ -36,7 +36,8 @@
 
   fails "String#split with Regexp retains the encoding of the source string"
   fails "String#split with Regexp returns an ArgumentError if an invalid UTF-8 string is supplied"
-
+  fails "String#split with Regexp respects the encoding of the regexp when splitting between characters"
+
   fails "String#upcase is locale insensitive (only replaces a-z)"
 
   # language/magic_comment_spec