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

Preserve attributes order for element node #10958

Merged
merged 5 commits into from Jun 4, 2021
Merged

Conversation

dcyriller
Copy link
Collaborator

@dcyriller dcyriller commented May 27, 2021

Description

Closes #10203

Checklist

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
  • (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

@dcyriller dcyriller added lang:handlebars Issues affecting Handlebars (Glimmer) type:bug Issues identifying ugly output, or a defect in the program labels May 27, 2021
@@ -418,24 +418,50 @@ function print(path, options, print) {

/* ElementNode print helpers */

function sortByLoc(a, b) {
if (a.loc.start.line < b.loc.start.line) {
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

return (a.loc.start.line - b.loc.start.line) || (a.loc.start.column - b.loc.start.column)

Copy link
Sponsor Member

@fisker fisker May 28, 2021

Choose a reason for hiding this comment

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

We also have .offset added in #9626, so a.loc.start.offset - b.loc.start.offset or locStart(a) - locStart(b) like js printer

isNonEmptyArray(node[property])
);
const attributes = types
.reduce((acc, type) => [...acc, ...node[type]], [])
Copy link
Sponsor Member

@fisker fisker May 27, 2021

Choose a reason for hiding this comment

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

Suggested change
.reduce((acc, type) => [...acc, ...node[type]], [])
.flatMap((type) => node[type])

{{! this is a comment for arg 5}}
@arg5="hello"
...arguments
/>
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Can we make this example a little shorter?

Copy link
Member

Choose a reason for hiding this comment

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

Oops. Sorry, missed this comment, but IMHO it's good as is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is odd, I thought I applied the change. If you think it's worth correcting, I'll open a PR

Copy link
Member

Choose a reason for hiding this comment

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

It'll be a minor release without a blog post, so it's okay, don't bother

@dcyriller

This comment has been minimized.

@thorn0 thorn0 merged commit 3cd28ca into prettier:main Jun 4, 2021
@dcyriller dcyriller deleted the 10203 branch June 4, 2021 11:03
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lang:handlebars Issues affecting Handlebars (Glimmer) type:bug Issues identifying ugly output, or a defect in the program
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Glimmer parser destructively re-orders component modifiers and comments
3 participants