[Webkit-unassigned] [Bug 26590] Support for String.trim(), String.trimLeft() and String.trimRight() methods

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26590


oliver at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #31624|review?                     |review-
               Flag|                            |




------- Comment #2 from oliver at apple.com  2009-06-21 16:06 PDT -------
(From update of attachment 31624)
> +static inline int isWhiteSpace(int c) 
> +{
> +	return (c >= 0x09 && c <= 0x0d) || c == 0x20;
> +}
There is already an isWhitespace style function  
> +static inline JSValue trimString(ExecState* exec, JSValue thisValue, bool left, bool right) 
> +{		
> +	JSString* sVal = thisValue.toThisJSString(exec);
toThisJSString may throw, so there should be an exception check here.

> +	const UString& s = sVal->value();
> +		
> +	int sSize = s.size();
> +	if (!sSize)
> +		return sVal;
> +		
> +	const UChar* sData = s.data();
> +		
> +	int start = 0;
> +	int end = sSize - 1;
...             
> +	return jsSubstring(exec, s, static_cast<unsigned>(start), static_cast<unsigned>((end - start) + 1));
> +}
Rather than casting here just define start and end as unsigned in the first
place
> Index: LayoutTests/fast/js/resources/string-trim.js
> ===================================================================
> --- LayoutTests/fast/js/resources/string-trim.js	(revision 0)
> +++ LayoutTests/fast/js/resources/string-trim.js	(revision 0)
> @@ -0,0 +1,12 @@
> +description(
> +"This test checks trim(), leftTrim() and rightTrim() functions."
> +);
> +
> +shouldBe("'					   '.trim()",          "''");
> +shouldBe("'					   '.trimLeft()",      "''");
> +shouldBe("'					   '.trimRight()",     "''");
> +shouldBe("'		foo bar			   '.trim()",      "'foo bar'");
> +shouldBe("'		foo bar			   '.trimLeft()",  "'foo bar			   '");
> +shouldBe("'		foo bar			   '.trimRight()", "'		foo bar'");

You should test non-space whitespace as well, \n, \t, \r, etc, also you should
test trim.apply({}), etc


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.