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

Configure whitespace-sensitive tags #28

Closed
awmottaz opened this issue May 22, 2019 · 10 comments
Closed

Configure whitespace-sensitive tags #28

awmottaz opened this issue May 22, 2019 · 10 comments
Labels
bug Something isn't working

Comments

@awmottaz
Copy link

Related to #24, whitespace-sensitive tags should not be modified on format. For example, this

    <pre>foo
bar baz</pre>

becomes this

    <pre>foo bar baz</pre>

I would also like to be able to configure custom tags to follow the same rules as the <pre> tag. For example, if I configure the CodeSnippet tag to be whitespace-sensitive, this should not change after formatting:

    <CodeSnippet lang="js">const foo = () => {
  return 42
}</CodeSnippet>
@jamesbirtles jamesbirtles added the bug Something isn't working label May 23, 2019
@jarrodldavis
Copy link
Contributor

Hey @awmottaz, I've begun work on fixing this issue in #41. Unfortunately, the ability to define custom component tags to be treated as preformatted looks to be a bit difficult due to how Prettier validates options (see prettier/prettier#6151).

However, any vanilla HTML tag can be configured as preformatted (as you can see in my Pull Request).

@ourway
Copy link

ourway commented Sep 13, 2019

It is a major issue for me write now, I am not able to format the code without breaking typography and other elements.

dummdidumm pushed a commit that referenced this issue Aug 28, 2020
- Maintain whitespace and newlines as-in inside `<pre>` tags
- Also consider attribute values (with one exception: the class attribute) to be pre-formatted. Do not add or remove line breaks there.
#28
@ehrencrona
Copy link
Contributor

ehrencrona commented Aug 31, 2020

There are really two issues mentioned here:

Whitespace should not be changed inside <pre>

This is now fixed

It should be possible to configure which tags are whitespace-sensitive

I personally don't believe this is a good idea: if you define custom tags that are whitespace sensitive that is something that relates to your project code and I don't believe the formatting options should be that intimately tied to the project code. You'd need to touch the configuration as you add new tags.

Furthermore, prettier has a different solution using "magic comments" in the code to hint about whitespace-sensitivity for that specific block, e.g. <!-- display: block -->. I'd prefer adding support for this solution if required.

Could others please comment on whether they need this kind of configurability beyond the recent fixes to respect whitespace in inline tags and pre blocks?

@dummdidumm
Copy link
Member

+1 for not adding a list of tags that are whitespace-sensitive. You could also a css-styling to tags to preserve whitespaces - what then? Things like <!-- prettier-ignore --> are exactly for cases like these.

@ehrencrona
Copy link
Contributor

@dummdidumm I just tested applying <!-- prettier-ignore --> to a whole <style> or <script> tag. That will fail because of how the Svelte parser works. The parser essentially cuts out those blocks, meaning that from within the formatter there is no way to tell that the comment belonged together with the tag. The comment will instead apply to the next tag.

That's a bit problematic. We could invent some other way of signaling "do not format", e.g. <style prettier:ignore>, but it feels quite unintuitive. I don't see any other obvious way out, though.

@dummdidumm
Copy link
Member

dummdidumm commented Aug 31, 2020

I think it's possible but we would have to jump through some hoops. Inside embed, we can access the parent through path.getParentNode(). Than can be used to find out if the previous sibling node contains the prettier-ignore comment (I hope).

@awmottaz
Copy link
Author

I would be happy with a magic comment to disable Prettier's whitespace formatting. I like the "disable"/"enable" semantics from ESLint for things like this, e.g.

    <!-- prettier-disable whitespace -->
    <CodeSnippet lang="js">const foo = () => {
  return 42
}</CodeSnippet>
    <!-- prettier-enable whitespace -->

@dummdidumm
Copy link
Member

I'm against adding anything that is not adhering to the prettier standards. There is a prettier-disable-start / prettier-disable-end for top level though.

@ehrencrona
Copy link
Contributor

ehrencrona commented Sep 1, 2020

Inside embed, we can access the parent through path.getParentNode(). Than can be used to find out if the previous sibling node contains the prettier-ignore comment (I hope).

@dummdidumm Works! I have to look at the start attribute to figure out which one is the sibling. It's fiddly but feels like an ok solution. Added support for this to #130

@dummdidumm
Copy link
Member

dummdidumm commented Feb 8, 2021

The pre-issue was fixed a while ago. Since prettier-plugin-svelte version 2.0.0, only known block elements such as div or p are formatted as whitespace-insensitive. Since 2.1.0, the htmlWhitespaceSensitivity option is supported, which means you can chose yourself if you want known block elements, all or none treated as whitespace sensitive. If you want to break out of the behavior for certain tags or want to leave the contents untouched (for example in that CodeSnippet case), use <!-- prettier-ignore -->. These options are enough to tweak the setup as desired, therefore I'm going to close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants