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

[unicode] Fix trailing nbsp #5165

Merged
merged 1 commit into from Sep 29, 2018

Conversation

Projects
None yet
3 participants
@vjeux
Collaborator

vjeux commented Sep 29, 2018

I don't know why I added all the unicode whitespaces instead of just space and tabs. It's not only unnecessary but also wrong.

It is preserved in the output
image

Fixes #5077

@vjeux vjeux requested a review from lydell Sep 29, 2018

@lydell

This comment has been minimized.

Collaborator

lydell commented Sep 29, 2018

Could you add the test case from the issue as well? Available here: master...lydell:jsx-nbsp-test

[unicode] Fix trailing nbsp
I don't know why I added all the unicode whitespaces instead of just space and tabs, but it's not only unecessary but also wrong.

Fixes #5077
@vjeux

This comment has been minimized.

Collaborator

vjeux commented Sep 29, 2018

Done

@lydell

lydell approved these changes Sep 29, 2018

Nice!

I checked out your branch locally and verified that the test snapshots contain actual non-breaking spaces.

@vjeux vjeux merged commit 9910536 into prettier:master Sep 29, 2018

10 checks passed

ci/circleci: build_prod Your tests passed on CircleCI!
Details
ci/circleci: checkout_code Your tests passed on CircleCI!
Details
ci/circleci: test_prod_node4 Your tests passed on CircleCI!
Details
ci/circleci: test_prod_node9 Your tests passed on CircleCI!
Details
ci/circleci: test_prod_standalone Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing 80c22d8...18faf56
Details
codecov/project 96.31% remains the same compared to 80c22d8
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@ikatyang ikatyang added this to the 1.15 milestone Oct 25, 2018

@ikatyang ikatyang referenced this pull request Nov 3, 2018

Merged

fix(less): remove CRs from inline comments #5334

1 of 1 task complete

ikatyang added a commit that referenced this pull request Nov 4, 2018

fix(less): remove CRs from inline comments (#5334)
The issue here is that less parser somehow included CRs in `comment.raws.content`, but it was hidden by the wrong trailing space elimination previously, which was fixed by #5165.

We already have such tests but it's not reproducible in AppVeyor since they use LF ([`core.autocrlf=input`](https://stackoverflow.com/a/20653073)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment