opal · May 17, 2015 · May 17, 2015 · May 17, 2015 · May 18, 2015 · May 18, 2015
Showing with 71 additions and 104 deletions.

+51 −95 opal/corelib/string.rb

+18 −0 opal/corelib/string/inheritance.rb

+2 −9 spec/filters/bugs/string.rb
diff --git a/opal/corelib/string.rb b/opal/corelib/string.rb
@@ -952,127 +952,83 @@ def scan(pattern, &block)
   alias slice []
   alias slice! <<
 
-  def split(pattern = $; || ' ', limit = undefined)
+  def split(pattern = undefined, 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 (limit === undefined) {
+        limit = 0;
+      } else {
+        limit = #{Opal.coerce_to!(limit, Integer, :to_int)};
+        if (limit === 1) {
+          return [self];
+        }
       }
 
-      if (pattern && pattern.$$is_regexp) {
-        var pattern_str = pattern.toString();
+      if (pattern === undefined || pattern === nil) {
+        pattern = #{$; || ' '};
+      }
 
-        /* Opal and JS's repr of an empty RE. */
-        var blank_pattern = (pattern_str.substr(0, 3) == '/^/') ||
-                  (pattern_str.substr(0, 6) == '/(?:)/');
+      var result = [],
+          string = self.toString(),
+          index = 0,
+          match,
+          i;
 
-        /* This is our fast path */
-        if (limit === undefined || limit === 0) {
-          result = self.split(blank_pattern ? /(?:)/ : pattern);
+      if (pattern.$$is_regexp) {
+        pattern = new RegExp(pattern.source, 'gm' + (pattern.ignoreCase ? 'i' : ''));
+      } else {
+        pattern = #{Opal.coerce_to(pattern, String, :to_str).to_s};
+        if (pattern === ' ') {
+          pattern = /\s+/gm;
+          string = string.replace(/^\s+/, '');
+        } else {
+          pattern = new RegExp(pattern.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'gm');
         }
-        else {
-          /* RegExp.exec only has sane behavior with global flag */
-          if (! pattern.global) {
-            pattern = eval(pattern_str + 'g');
-          }
-
-          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;
-
-            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('');
-              }
-
-              prev_index = undefined;
-              break;
-            }
+      }
 
-            if (limit !== undefined && limit > 1 && result.length + 1 == limit) {
-              break;
-            }
-          }
+      result = string.split(pattern);
 
-          if (prev_index !== undefined) {
-            result.push(self.slice(prev_index, self.length));
-          }
-        }
+      while ((i = result.indexOf(undefined)) !== -1) {
+        result.splice(i, 1);
       }
-      else {
-        var splitted = 0, start = 0, lim = 0;
 
-        if (pattern === nil || pattern === undefined) {
-          pattern = ' '
-        } else {
-          pattern = #{Opal.try_convert(pattern, String, :to_str).to_s};
+      if (limit === 0) {
+        while (result[result.length - 1] === '') {
+          result.length -= 1;
         }
+        return result;
+      }
 
-        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 (pattern == ' ' && cursor == start) {
-            start = cursor + 1;
-            continue;
-          }
-
-          result.push(string.substr(start, pattern.length ? cursor - start : 1));
-          splitted++;
-
-          start = cursor + (pattern.length ? pattern.length : 1);
-        }
+      match = pattern.exec(string);
 
-        if (string.length > 0 && (limit < 0 || string.length > start)) {
-          if (string.length == start) {
+      if (limit < 0) {
+        if (match !== null && match[0] === '' && pattern.source.indexOf('(?=') === -1) {
+          for (i = 0; i < match.length; i++) {
             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 (match !== null && match[0] === '') {
+        result.splice(limit - 1, result.length - 1, result.slice(limit - 1).join(''));
+        return result;
       }
 
-      if (limit > 0) {
-        var tail = result.slice(limit - 1).join('');
-        result.splice(limit - 1, result.length - 1, tail);
+      i = 0;
+      while (match !== null) {
+        i++;
+        index = pattern.lastIndex;
+        if (i + 1 === limit) {
+          break;
+        }
+        match = pattern.exec(string);
       }
 
+      result.splice(limit - 1, result.length - 1, string.slice(index));
       return result;
     }
   end

diff --git a/opal/corelib/string/inheritance.rb b/opal/corelib/string/inheritance.rb
@@ -92,4 +92,22 @@ def *(other)
       }
     }
   end
+
+  def split(pattern = undefined, limit = undefined)
+    @literal.split(pattern, limit).map{|str| self.class.allocate(str)}
+  end
+
+  def replace(string)
+    @literal = string
+  end
+
+  def each_line(separator = $/)
+    return enum_for :each_line, separator unless block_given?
+    @literal.each_line(separator){|str| yield self.class.allocate(str)}
+  end
+
+  def lines(separator = $/, &block)
+    e = each_line(separator, &block)
+    block ? self : e.to_a
+  end
 end
diff --git a/spec/filters/bugs/string.rb b/spec/filters/bugs/string.rb
@@ -9,16 +9,9 @@
 
   fails "String#dup does not copy constants defined in the singleton class"
 
-  fails "String#each_line yields subclass instances for subclasses"
-
-  fails "String#lines yields subclass instances for subclasses"
-
-  fails "String#split with String returns subclass instances based on self"
+  #The following failing only on phantomjs v1.9.8
+  #OK to remove after we upgrade to phantomjs 2.0
   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 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"
 
   fails "String#byteslice returns the character code of the character at the given index"
   fails "String#byteslice returns nil if index is outside of self"