-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
[refactor] refactor trim utils and write tests #6909
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
It looks good to me. Should we add a test that contains spaces between content like |
Would it be better if I wrote the tests this way? const value = trim_start(' lorem ipsum ');
assert.equal(value, 'lorem ipsum ');
const value = trim_end(' lorem ipsum ');
assert.equal(value, ' lorem ipsum'); |
Yes. like that! NOTE:
|
Hi, i'm pretty sure you have a good reason, but i'm just curious as to why you don't use Does it have limitations i'm not familiar with, in that case i'd like to know ;) |
'trimStart' is available since Node v10. Also, older browsers do not support it. @drunkwinter |
35346d6
to
5a0185a
Compare
Tests updated. @baseballyama Thanks for your feedback Trim methods pass the tests, but I don't understand what it is that doesn't pass the test. |
Thank you for the fix!
|
Thanks for explaining! There is a polyfill listed on MDN with a shorter regex |
|
Before submitting the PR, please make sure you do the following
[feat]
,[fix]
,[chore]
, or[docs]
.Tests
npm test
and lint the project withnpm run lint
I made the
trim
utils more readable and performant. I've also included their tests to make sure they're working correctly.