505890 - Implement String.trim

Status: RESOLVED FIXED

(Rhino Graveyard :: Core, enhancement)

Description

[rspeyer]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_5_7; en-gb) AppleWebKit/530.18 (KHTML, like Gecko) Version/4.0.1 Safari/530.18
Build Identifier: Rhino 1.7 release 2 2009 03 22

As per 15.5.4.20 of the spec

Reproducible: Always

Comment 1

[rspeyer]

Attachment: Implementation of String.trim

Comment 2

[rspeyer]

Attachment: make String.trim remove additional whitespace and line terminator chars

Update to String.trim, making it remove <NBSP>, <BOM>, <LS> and <PS> as well as conventional whitespace chars

Comment 3

[Norris Boyd]

Comment on attachment 390121 [details] [diff] [review]
make String.trim remove additional whitespace and line terminator chars

>diff --git a/src/org/mozilla/javascript/NativeString.java b/src/org/mozilla/javascript/NativeString.java
>index 7997275..be9aab3 100644
>--- a/src/org/mozilla/javascript/NativeString.java
>+++ b/src/org/mozilla/javascript/NativeString.java
>@@ -152,7 +152,7 @@ final class NativeString extends IdScriptableObject
>         addIdFunctionProperty(ctor, STRING_TAG,
>                 ConstructorId_toLocaleLowerCase, "toLocaleLowerCase", 1);
>         addIdFunctionProperty(ctor, STRING_TAG,
>-                ConstructorId_fromCharCode, "fromCharCode", 1);
>+                ConstructorId_trim, "trim", 0);

Good you spotted that fromCharCode was in here twice. 

I don't see in the spec that "trim" should be a property of the constructor as well as the prototype. Do you see that?

>         super.fillConstructorProperties(ctor);
>     }
> 
>@@ -198,6 +198,7 @@ final class NativeString extends IdScriptableObject
>           case Id_localeCompare:     arity=1; s="localeCompare";     break;
>           case Id_toLocaleLowerCase: arity=0; s="toLocaleLowerCase"; break;
>           case Id_toLocaleUpperCase: arity=0; s="toLocaleUpperCase"; break;
>+          case Id_trim:              arity=0; s="trim";              break;
>           default: throw new IllegalArgumentException(String.valueOf(id));
>         }
>         initPrototypeMethod(STRING_TAG, id, s, arity);
>@@ -230,7 +231,8 @@ final class NativeString extends IdScriptableObject
>               case ConstructorId_search:
>               case ConstructorId_replace:
>               case ConstructorId_localeCompare:
>-              case ConstructorId_toLocaleLowerCase: {
>+              case ConstructorId_toLocaleLowerCase: 
>+              case ConstructorId_trim: {
>                 if (args.length > 0) {
>                     thisObj = ScriptRuntime.toObject(scope,
>                             ScriptRuntime.toString(args[0]));
>@@ -412,11 +414,32 @@ final class NativeString extends IdScriptableObject
>                     return ScriptRuntime.toString(thisObj)
>                             .toUpperCase(cx.getLocale());
>                 }
>+              case Id_trim:
>+                {
>+                    String str = ScriptRuntime.toString(thisObj).trim();

Why use the Java trim() here if you have to scan the characters anyway?

>+                    char[] chars = str.toCharArray();
>+
>+                    int start = 0;
>+                    while (start < chars.length && isWhitespaceOrLineTerminator(chars[start])) {
>+                      start += 1;

Use ++

>+                    }
>+                    int end = chars.length;
>+                    while (end > start && isWhitespaceOrLineTerminator(chars[end-1])) {
>+                      end--;
>+                    }
>+
>+                    return str.substring(start, end);
>+                }
>             }
>             throw new IllegalArgumentException(String.valueOf(id));
>         }
>     }
> 
>+    private boolean isWhitespaceOrLineTerminator(char c) {
>+      return Character.isWhitespace(c) || 
>+        c == '\u2028' || c == '\u2029' || c == '\uFEFF' || c == '\u00A0';
>+    }
>+
>     private static NativeString realThis(Scriptable thisObj, IdFunctionObject f)
>     {
>         if (!(thisObj instanceof NativeString))
>@@ -652,7 +675,7 @@ final class NativeString extends IdScriptableObject
>     protected int findPrototypeId(String s)
>     {
>         int id;
>-// #generated# Last update: 2007-05-01 22:11:49 EDT
>+// #generated# Last update: 2009-07-23 07:32:39 EST
>         L0: { id = 0; String X = null; int c;
>             L: switch (s.length()) {
>             case 3: c=s.charAt(2);
>@@ -663,6 +686,7 @@ final class NativeString extends IdScriptableObject
>             case 4: c=s.charAt(0);
>                 if (c=='b') { X="bold";id=Id_bold; }
>                 else if (c=='l') { X="link";id=Id_link; }
>+                else if (c=='t') { X="trim";id=Id_trim; }
>                 break L;
>             case 5: switch (s.charAt(4)) {
>                 case 'd': X="fixed";id=Id_fixed; break L;
>@@ -756,7 +780,8 @@ final class NativeString extends IdScriptableObject
>         Id_localeCompare             = 34,
>         Id_toLocaleLowerCase         = 35,
>         Id_toLocaleUpperCase         = 36,
>-        MAX_PROTOTYPE_ID             = 36;
>+        Id_trim                      = 37,
>+        MAX_PROTOTYPE_ID             = Id_trim;
> 
> // #/string_id_map#
> 
>@@ -777,7 +802,8 @@ final class NativeString extends IdScriptableObject
>         ConstructorId_search         = -Id_search,
>         ConstructorId_replace        = -Id_replace,
>         ConstructorId_localeCompare  = -Id_localeCompare,
>-        ConstructorId_toLocaleLowerCase = -Id_toLocaleLowerCase;
>+        ConstructorId_toLocaleLowerCase = -Id_toLocaleLowerCase,
>+        ConstructorId_trim           = -Id_trim;
> 
>     private String string;
> }
>diff --git a/testsrc/doctests/string.trim.doctest b/testsrc/doctests/string.trim.doctest
>new file mode 100644
>index 0000000..3730495
>--- /dev/null
>+++ b/testsrc/doctests/string.trim.doctest
>@@ -0,0 +1,17 @@
>+js> load('testsrc/doctests/util.js');
>+
>+js> "".trim
>+function trim() { [native code for String.trim, arity=0] }
>+
>+js> String.trim
>+function trim() { [native code for String.trim, arity=0] }
>+
>+js> String.trim.call({toString: function() { return "a" }})
>+a
>+
>+js> "  hello ".trim().length
>+5
>+
>+js> var [t, n, r] = [String.fromCharCode(9), String.fromCharCode(10), String.fromCharCode(13)]
>+js> (t + n + r + " " + "abc").trim().length
>+3

Comment 4

[rspeyer]

Attachment: updated trim implementation

Removed trim from String constructor, and made scanning of whitespace and line terminator characters explicit.

Comment 5

[Norris Boyd]

Committed patch