-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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
remove extra "
#28218
remove extra "
#28218
Conversation
Preview URLs (comment last updated: 2023-07-27 15:42:53) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good @yin1999! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix. Running prettier on HTML is usually not a good idea - it does a horrible job. Was this something you chose to do or the toolchain forced you to do?
Added it accidentally. The fix itself is good but the HTML prettier stuff is not.
We still run prettier format check in translated-content. After I fix the broken html (in mdn/translated-content#14568), the format check just failed. So I run |
OK. Thanks. @queengooborg Thoughts on this? @yin1999 is "doing the right thing", but the resulting prettier changes to HTML are rubbish. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prettier's formatting for HTML is not great, I think we all agree on this. We don't have any control over it, though. Plus, we already format all our HTML with Prettier.
Really? I thought we formatted only the JS. I hate this. I would have suggested nolint. |
Hi @yin1999 Just FYI, following some advice from @queengooborg I improved the HTML in #28230 . What I hadn't realized is that prettier does different things based on your starting point. So by putting some of the text on different lines it did a better job of formatting. Thanks for your fix! |
Thanks for the additional work on this @hamishwillee 👍 |
Description
remove extra
"