Skip to content

[unicode] Fix trailing nbsp#5165

Merged
vjeux merged 1 commit intoprettier:masterfrom
vjeux:trailing_nbsp
Sep 29, 2018
Merged

[unicode] Fix trailing nbsp#5165
vjeux merged 1 commit intoprettier:masterfrom
vjeux:trailing_nbsp

Conversation

@vjeux
Copy link
Copy Markdown
Contributor

@vjeux 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 September 29, 2018 21:16
@lydell
Copy link
Copy Markdown
Member

lydell commented Sep 29, 2018

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

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 prettier#5077
@vjeux
Copy link
Copy Markdown
Contributor Author

vjeux commented Sep 29, 2018

Done

Copy link
Copy Markdown
Member

@lydell lydell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
@ikatyang ikatyang added this to the 1.15 milestone Oct 25, 2018
ikatyang added a commit that referenced this pull request Nov 4, 2018
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)).
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jan 23, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jan 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants