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

Handlebars: fix formatting of attributes #10145

Merged
merged 7 commits into from
Jan 28, 2021
Merged

Conversation

thorn0
Copy link
Member

@thorn0 thorn0 commented Jan 26, 2021

Description

This PR

  • fixes escaping of {{ in attributes and text
  • fixes the choice between ' and " for attributes with interpolations
  • fixes the bug with [object Object] printed in class
  • implements simple formatting for class, like in HTML in the current stable version (before Smarter formatting of HTML class attribute #7865)

Closes #8648
Closes #8667

cc @dcyriller @kangax

Checklist

  • I’ve added tests to confirm my change works.
  • (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
  • I’ve read the contributing guidelines.

Try the playground for this PR

Copy link
Collaborator

@dcyriller dcyriller left a comment

Choose a reason for hiding this comment

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

Left one small comment about n.chars that might be transformed into text. Sounds like a great improvement overall.

Do you think we should use src/language-html/syntax-attribute.js functions to format handlebars attributes in the future?

@@ -204,8 +254,6 @@ function print(path, options, print) {
const [lead] = n.chars.match(leadingWhitespacesRE);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const [lead] = n.chars.match(leadingWhitespacesRE);
const [lead] = text.match(leadingWhitespacesRE);

Copy link
Collaborator

Choose a reason for hiding this comment

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

also on lines 295:299 (can't suggest there)

@@ -204,8 +254,6 @@ function print(path, options, print) {
const [lead] = n.chars.match(leadingWhitespacesRE);
const [tail] = n.chars.match(trailingWhitespacesRE);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const [tail] = n.chars.match(trailingWhitespacesRE);
const [tail] = text.match(trailingWhitespacesRE);

return [quote, stringLiteral.replace(regex, `\\${quote}`), quote];
}

function chooseEnclosingQuote(options, stringLiteral) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@@ -0,0 +1,25 @@
#### Fix formatting of attributes (#10145 by @thorn0)

- fix escaping of `{{` in attributes and text
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice catch, I had this refactor / fix on a branch too

@thorn0
Copy link
Member Author

thorn0 commented Jan 27, 2021

@dcyriller

Do you think we should use src/language-html/syntax-attribute.js functions to format handlebars attributes in the future?

What can be reused should definitely be reused, but we need to figure out a way to share that code between the two printers. Currently language-html and language-handlebars are built as separate bundles.

@thorn0 thorn0 merged commit 8846448 into prettier:main Jan 28, 2021
@thorn0 thorn0 deleted the handlebars-attrs branch January 28, 2021 21:45
@thorn0
Copy link
Member Author

thorn0 commented Feb 6, 2021

@dcyriller Just realized that I was wrong here:

Currently language-html and language-handlebars are built as separate bundles

Only the parsers are extracted to separate bundles. The printers all go in one bundle, so it's okay to require things directly from language-html in the printer.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handlebars: newline in an attribute incorrectly rendered as [object Object]
2 participants