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

implement trimLeft and trimRight #321

Merged
merged 1 commit into from Aug 2, 2015

Conversation

Projects
None yet
3 participants
@jvoigtlaender
Contributor

jvoigtlaender commented Aug 1, 2015

... since they are not standard, thus not supported in all browsers.

Addresses https://github.com/elm-lang/core/issues/319 and thus issues like elm/package.elm-lang.org#40.

implement trimLeft and trimRight
... since they are not standard, thus not supported in all browsers
@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Aug 2, 2015

Member

Ah, nice find! Why do you use \s in your regex but mozilla uses a more complex sequence in their polyfill for trim? I know they are trimming both ends, but they are detecting more than just \s.

Also, does trim work?

Member

evancz commented Aug 2, 2015

Ah, nice find! Why do you use \s in your regex but mozilla uses a more complex sequence in their polyfill for trim? I know they are trimming both ends, but they are detecting more than just \s.

Also, does trim work?

@texastoland

This comment has been minimized.

Show comment
Hide comment
@texastoland

texastoland Aug 2, 2015

It looks like the EcmaScript specification defines a couple extended Unicode points as whitespace:

WhiteSpace ::
<TAB>
<VT>
<FF>
<SP>
<NBSP>
<ZWNBSP>
<USP>

For practical purposes they're the same.

texastoland commented Aug 2, 2015

It looks like the EcmaScript specification defines a couple extended Unicode points as whitespace:

WhiteSpace ::
<TAB>
<VT>
<FF>
<SP>
<NBSP>
<ZWNBSP>
<USP>

For practical purposes they're the same.

@texastoland

This comment has been minimized.

Show comment
Hide comment
@texastoland

texastoland Aug 2, 2015

Also the MDN link reports trim compatibility starting with IE9.

texastoland commented Aug 2, 2015

Also the MDN link reports trim compatibility starting with IE9.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Aug 2, 2015

Member

Okay, I guess let's go with this. If folks find it odd that trim and trimLeft work on slightly different kinds of whitespace, let's fix that in another PR.

Thanks @jvoigtlaender and @dnalot!

Member

evancz commented Aug 2, 2015

Okay, I guess let's go with this. If folks find it odd that trim and trimLeft work on slightly different kinds of whitespace, let's fix that in another PR.

Thanks @jvoigtlaender and @dnalot!

evancz pushed a commit that referenced this pull request Aug 2, 2015

@evancz evancz merged commit b928046 into elm:master Aug 2, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Aug 2, 2015

Contributor

I had modelled it on both stuff I "found on the internet" as well as your implementation of words:

    function words(str)
    {
        return List.fromArray(str.trim().split(/\s+/g));
    }

Thought it made sense to do trimming by eliminating the same kind of whitespace that is used for finding word boundaries for splitting in that function.

Contributor

jvoigtlaender commented Aug 2, 2015

I had modelled it on both stuff I "found on the internet" as well as your implementation of words:

    function words(str)
    {
        return List.fromArray(str.trim().split(/\s+/g));
    }

Thought it made sense to do trimming by eliminating the same kind of whitespace that is used for finding word boundaries for splitting in that function.

@jvoigtlaender jvoigtlaender deleted the jvoigtlaender:trimLeft-trimRight branch Aug 2, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment