Skip to content
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

[fix]: keep space in <pre> or preserveWhitespace: true #6990

Merged
merged 3 commits into from Feb 2, 2022

Conversation

ota-meshi
Copy link
Member

@ota-meshi ota-meshi commented Dec 6, 2021

close #6437
close #4731

This PR fixes to keep the space inside the <pre> tag. Also, if the user uses the preserveWhitespace: true option, it will keep the space in the same way with or without the <pre> tag.

The previous compiler removes the spaces after the start tag and before the end tag. It works fine in most cases, but it doesn't output as intended inside the <pre> tag.

However, even after this fix, the space remains removed inside the mustache tag. This is intentional because I think that outputting a lot of space with {#each} etc. may be included in the breaking change.

There's already a PR #4737 about fixing preserveWhitespace: true, but it didn't seem to be active.

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with [feat], [fix], [chore], or [docs].
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with npm test and lint the project with npm run lint

@ota-meshi ota-meshi changed the title fix: keep space in <pre> or preserveWhitespace: true [fix]: keep space in <pre> or preserveWhitespace: true Dec 6, 2021
@ota-meshi
Copy link
Member Author

I fixed tests that was failing due to crlf on windows. Would you please run the tests again?

@ota-meshi
Copy link
Member Author

I don't know how to fix the CI timeout. Can you let me know how to solve it?

@dummdidumm
Copy link
Member

It's unrelated, don't worry about it

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

Code change looks great 👍 Could you replace the test in test/js with a runtime test inside test/runtime? You could check the rendered output via target.querySelector('something something').innerHTML. The advantage would be that these tests are more robust to compiler implementation changes and need less adjustments later on.

@ota-meshi
Copy link
Member Author

Thank you for checking this PR! I will make that change.

@dummdidumm dummdidumm merged commit 7463d51 into sveltejs:master Feb 2, 2022
@ota-meshi ota-meshi deleted the pre branch February 2, 2022 10:07
himanshiLt pushed a commit to himanshiLt/svelte that referenced this pull request Mar 3, 2022
…tejs#6990)

Fixes sveltejs#6437
Fixes sveltejs#4731
Closes sveltejs#4737

Whitespace is now untouched inside <pre> tag and other tags if preserveWhitespace is true
nevilm-lt pushed a commit to nevilm-lt/svelte that referenced this pull request Mar 14, 2022
…tejs#6990)

Fixes sveltejs#6437
Fixes sveltejs#4731
Closes sveltejs#4737

Whitespace is now untouched inside <pre> tag and other tags if preserveWhitespace is true
nevilm-lt pushed a commit to nevilm-lt/svelte that referenced this pull request Apr 21, 2022
…tejs#6990)

Fixes sveltejs#6437
Fixes sveltejs#4731
Closes sveltejs#4737

Whitespace is now untouched inside <pre> tag and other tags if preserveWhitespace is true
nevilm-lt pushed a commit to nevilm-lt/svelte that referenced this pull request Apr 21, 2022
…tejs#6990)

Fixes sveltejs#6437
Fixes sveltejs#4731
Closes sveltejs#4737

Whitespace is now untouched inside <pre> tag and other tags if preserveWhitespace is true
nevilm-lt pushed a commit to nevilm-lt/svelte that referenced this pull request Apr 22, 2022
…tejs#6990)

Fixes sveltejs#6437
Fixes sveltejs#4731
Closes sveltejs#4737

Whitespace is now untouched inside <pre> tag and other tags if preserveWhitespace is true
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Svelte trims content for <pre> preserveWhitespace strips prefix space
3 participants