[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





------- Comment #16 from ioseb.dzmanashvili at gmail.com  2009-06-24 01:37 PDT -------
(In reply to comment #14)

> You need to have a change log entry here, with comments explaining what you
> did, and quoting the URL and title of the bug. Look at other ChangeLog entries
> to see examples of how to do it. Ideallyou you have a separate comment for each
> function.

Done

> If this is used only within the file it should be marked "static" to give it
> internal linkage.

Done

> I don't understand what's going on with this function. It has the name
> "isWhiteSpace" which doesn't seem specific enough. Is there any difference
> between this and the isStrWhiteSpace function in JSGlobalObjectFunctions.h? Can
> you just use that function instead of adding your own?

As I mentioned in my previous comment "isStrWhiteSpace" function in
JSGlobalObjectFunction checks passed character against very limited list of
whitespace chars... that was the reason to implementing my own function. I've
nothing against existing functions but what they do is not really enough. 

> > +    if(trimMode == TrimRight || trimMode == TrimBothLeftAndRight) {
> 
> Need a space here as I mentioned before.

Done

> > +    return jsSubstring(exec, s, static_cast<unsigned>(start), static_cast<unsigned>(end - start));
> 
> Please remove these unneeded static_cast.
> 
> review- because of the isWhiteSpace issue and lack of ChangeLog contents
> 


-- 
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.