Add optional startIndex to String.replaceFirst

sdk/lib/_internal/lib/string_helper.dart

Index: sdk/lib/_internal/lib/string_helper.dart
diff --git a/sdk/lib/_internal/lib/string_helper.dart b/sdk/lib/_internal/lib/string_helper.dart
index e3fb93b40470279eb685584a98d8114f5d36849b..7a37d7c03d562cbea19256d96171ddf67143e66b 100644
--- a/sdk/lib/_internal/lib/string_helper.dart
+++ b/sdk/lib/_internal/lib/string_helper.dart
@@ -69,12 +69,19 @@ stringContainsUnchecked(receiver, other, startIndex) {
   }
 }
 
-stringReplaceJS(receiver, replacer, to) {
+stringReplaceJS(receiver, replacer, to, [startIndex = 0]) {

[Lasse Reichstein Nielsen, 2014/08/11 10:20:01]

No need to make startIndex optional here. It's an internal function so we always
control the number of arguments.

[srawlins, 2014/08/14 00:42:44]

Done.

   // The JavaScript String.replace method recognizes replacement
   // patterns in the replacement string. Dart does not have that
   // behavior.
   to = JS('String', r'#.replace(/\$/g, "$$$$")', to);
-  return JS('String', r'#.replace(#, #)', receiver, replacer, to);
+  if (startIndex == 0) {
+    return JS('String', r'#.replace(#, #)', receiver, replacer, to);
+  }
+
+  String prefix = JS('String', r'#.substring(0, #)', receiver, startIndex);
+  receiver = JS('String', r'#.substring(#)', receiver, startIndex);
+  String result = JS('String', r'#.replace(#, #)', receiver, replacer, to);
+  return JS('String', r'# + #', prefix, result);

[Lasse Reichstein Nielsen, 2014/08/11 10:20:01]

This also looks too complicated. It might be necessary, but I'm not convinced
yet :)

[Lasse Reichstein Nielsen, 2014/08/13 08:35:09]

It's also wrong due to the "^" problem.

Maybe not modify this function, so it only works with startIndex = 0, and use a
different function for the other case:

  replaceFirstJSRE(receiver, regexp, 
                   to, startIndex) {
    var match = regexp._execGlobal(receiver, startIndex);
    if (match == null) return receiver;
    var start = match.start;
    var end = match.end;
    return "${receiver.substring(0,start)}$to${receiver.substring(end)}";
  }

Maybe just inline this in the stringReplaceFirstUnchecked function's
regexp-case.

[srawlins, 2014/08/14 00:42:44]

Acknowledged.

[srawlins, 2014/08/14 00:42:44]

Thanks! I'll leave this as a separate function for now.

 }
 
 const String ESCAPE_REGEXP = r'[[\]{}()*+?.\\^$|]';
@@ -185,12 +192,12 @@ stringReplaceAllStringFuncUnchecked(receiver, pattern, onMatch, onNonMatch) {
 }
 
 
-stringReplaceFirstUnchecked(receiver, from, to) {
+stringReplaceFirstUnchecked(receiver, from, to, [int startIndex = 0]) {
   if (from is String) {
-    return stringReplaceJS(receiver, from, to);
+    return stringReplaceJS(receiver, from, to, startIndex);

[Lasse Reichstein Nielsen, 2014/08/11 10:20:01]

Have we tried timing this against:
  var index = receiver.indexOf(from, startIndex);
  if (index < 0) return receiver;
  return receiver.substring(0, index) + to + receiver.substring(index +
from.length);

[srawlins, 2014/08/14 00:42:45]

Your indexOf alternative must definitely be faster now that we're splitting
stringReplaceJS into a method with startIndex and one without: The second method
always fires up the regex engine, so I'd say this is faster.

   } else if (from is JSSyntaxRegExp) {
     var re = regExpGetNative(from);
-    return stringReplaceJS(receiver, re, to);
+    return stringReplaceJS(receiver, re, to, startIndex);

[Lasse Reichstein Nielsen, 2014/08/13 08:35:09]

Use this only if startIndex is zero, otherwise use the code above.

[srawlins, 2014/08/14 00:42:45]

Done.

   } else {
     checkNull(from);
     // TODO(floitsch): implement generic String.replace (with patterns).