Skip to content

Handle singleQuote option more precisely#312

Merged
Shinigami92 merged 9 commits into
prettier:mainfrom
shadowgate15:fix/quotes
Nov 26, 2021
Merged

Handle singleQuote option more precisely#312
Shinigami92 merged 9 commits into
prettier:mainfrom
shadowgate15:fix/quotes

Conversation

@shadowgate15
Copy link
Copy Markdown
Collaborator

This should now have code interpolation be using the pugSingleQuote unless it is inside of quotes.

@shadowgate15
Copy link
Copy Markdown
Collaborator Author

shadowgate15 commented Nov 24, 2021

closes #45

@shadowgate15 shadowgate15 marked this pull request as ready for review November 24, 2021 04:25
Copy link
Copy Markdown
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

I will look into other changes later, I'm currently just on mobile.

Comment thread docs/guide/standard-prettier-overrides.md
Comment thread docs/guide/standard-prettier-overrides.md Outdated
Copy link
Copy Markdown
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

Please revert pnpm-lock.yaml. It's unrelated to the PR

Copy link
Copy Markdown
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

I think you should hugely enhance / extends the tests/options/singleQuote/single-quote.test.ts test cases. IMO the expected and code are not sufficient anymore now.

Comment thread src/printer.ts
Comment thread src/printer.ts Outdated
@shadowgate15
Copy link
Copy Markdown
Collaborator Author

shadowgate15 commented Nov 24, 2021

this is probably a pretty good start on this. there are still some issues that are related to pug-lexer and the way certain tokens are handled but I would say outside of the scope of this PR. Eventually I will try to circle back around to fix those up.

Ideally if I say singleQuote: true then all top level quotes should be a single quote unless it is inside of double quotes.

Comment thread src/printer.ts Outdated
@Shinigami92
Copy link
Copy Markdown
Member

I would really like that you extend the code fragments in tests/options/singleQuote/single-quote.test.ts so that still every combination of singleQuote + pugSingleQuote is covered

@Shinigami92
Copy link
Copy Markdown
Member

Will look into it tomorrow
Thanks for your work on this ❤️

@Shinigami92 Shinigami92 linked an issue Nov 25, 2021 that may be closed by this pull request
@Shinigami92 Shinigami92 changed the title Fix/quotes Handle singleQuote option more precisely Nov 25, 2021
Copy link
Copy Markdown
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

I only have two more requests 🙂 Then I will merge it

  1. You are allowed to remove tests/options/singleQuote/single-quote.test.ts due to it is covered by your new tests 🎉
  2. Please add in each of these tests/options/singleQuote/**/*.pug files a normal attribute to a pug token.
    So e.g. div(:class='{ "pt-2": !dense }') becomes div(:class='{ "pt-2": !dense }', test='test')
    Best would be that in the unformatted file always wrong quote/format version is present, so it does explicitly get formatted by the test case
    This will help that no regression will happen there in the future

@Shinigami92 Shinigami92 merged commit 38d787b into prettier:main Nov 26, 2021
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.

Bug: Doesn't take the singleQuote option

2 participants