Fixes #5690: Add trimStart and trimEnd #5693
Conversation
| { | ||
| name: "trimLeft is same function as trimStart", | ||
| body: function () { | ||
| // NOTE: See comments in test/UnitTestFramework/UnitTestFramework.js for more info about what assertions you can use |
There was a problem hiding this comment.
nitpick: please remove these comments that came from the UnitTestFramework template
| ]; | ||
|
|
||
| // The test runner will pass "-args summary -endargs" to ch, so that "summary" appears as argument [0[] to the script, | ||
| // and the following line will then ensure that the test will only display summary output (i.e. "PASS"). |
There was a problem hiding this comment.
nitpick: please remove these comments that came from the UnitTestFramework template
test/Strings/trimStart_trimEnd.js
Outdated
| assert.areEqual(String.prototype.trimRight, String.prototype.trimEnd, "Both trimRight and trimEnd should point to the same function"); | ||
| } | ||
| } | ||
| ]; |
There was a problem hiding this comment.
Please add the following test cases to match other browsers' behavior:
assert.areEqual(String.prototype.trimStart.name, 'trimStart')
assert.areEqual(String.prototype.trimEnd.name, 'trimEnd')
assert.areEqual(String.prototype.trimLeft.name, 'trimStart')
assert.areEqual(String.prototype.trimRight.name, 'trimEnd')| @@ -3994,9 +4000,15 @@ namespace Js | |||
| /* No inlining String_EndsWith */ library->AddFunctionToLibraryObject(stringPrototype, PropertyIds::endsWith, &JavascriptString::EntryInfo::EndsWith, 1); | |||
| /* No inlining String_Includes */ library->AddFunctionToLibraryObject(stringPrototype, PropertyIds::includes, &JavascriptString::EntryInfo::Includes, 1); | |||
| builtinFuncs[BuiltinFunction::JavascriptString_TrimLeft] = library->AddFunctionToLibraryObject(stringPrototype, PropertyIds::trimLeft, &JavascriptString::EntryInfo::TrimLeft, 0); | |||
| //builtinFuncs[BuiltinFunction::JavascriptString_TrimStart] = library->AddFunctionToLibraryObject(stringPrototype, PropertyIds::trimStart, &JavascriptString::EntryInfo::TrimStart, 0); | |||
There was a problem hiding this comment.
please remove this commented-out code (I think you already have this change pending)
| } | ||
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Please remove these extra blank lines (I think you already have this change pending)
sharmasuraj0123
force-pushed
the
trimStart_trimEnd
branch
from
78343f2 to
8fa8ece
Compare
sharmasuraj0123
added
the
Bytecode-Update
| @@ -244,7 +244,9 @@ namespace Js | |||
| static FunctionInfo ToUpperCase; | |||
| static FunctionInfo Trim; | |||
| static FunctionInfo TrimLeft; | |||
| static FunctionInfo TrimStart; | |||
| static FunctionInfo TrimRight; | |||
There was a problem hiding this comment.
Any reason why we are keeping a lot of the old TrimLeft/TrimRight references around? I feel like we should be able to effectively change the names entirely and then arbitrarily tack on the start/end entrypoints as trimStart/trimEnd in InitializeStringPrototype.
There was a problem hiding this comment.
trimLeft/trimRight were never included in a spec, but every browser implements them (and pass all these test cases), so we have to keep them. See also the test cases (with eshost output) in #5690
There was a problem hiding this comment.
As for possibly redundant objects -- we will follow up with a clean-up commit
sharmasuraj0123
force-pushed
the
trimStart_trimEnd
branch
from
8fa8ece to
ae77032
Compare
sharmasuraj0123
changed the title
Fixes #5690