Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_js_formatter): assignment expressions with layout #2728

Merged
merged 5 commits into from
Jun 20, 2022

Conversation

ematipico
Copy link
Contributor

@ematipico ematipico commented Jun 17, 2022

Summary

This PR closes #2416

This PR adds Chain and ChainTail to the list of layouts and implements them. I left out on purpose ChainTailArrowFunction to not add too code, mostly because this PR moves code around and it might cause some confusion.

I moved the implementation of write_assignment_like inside the format function of JsAnyAssignmentLike for two important reasons:

  • I needed access to JsAnyAssignmentLike variants in order to understand whether a group should be added or not;
  • I needed access to the parent of JsAnyAssignmentLike, which was needed for the implementation of the new layout;

I also moved few functions inside the struct itself, because they needed to be exposed. The only loose function is used else where, so I kept there.

Test Plan

I created two new tests for the new layouts, one case where the groups breaks and on where it doesn't.

PR

File Based Average Prettier Similarity: 74.79%
Line Based Average Prettier Similarity: 69.89%

main
File Based Average Prettier Similarity: 74.65%
Line Based Average Prettier Similarity: 69.75%

@github-actions
Copy link

github-actions bot commented Jun 17, 2022

@cloudflare-pages
Copy link

cloudflare-pages bot commented Jun 17, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0730743
Status: ✅  Deploy successful!
Preview URL: https://b8bf4bc2.tools-8rn.pages.dev
Branch Preview URL: https://feature-assignment-chain.tools-8rn.pages.dev

View logs

@ematipico ematipico marked this pull request as ready for review June 17, 2022 15:42
@ematipico ematipico temporarily deployed to aws June 17, 2022 15:42 Inactive
@ematipico ematipico temporarily deployed to aws June 17, 2022 16:27 Inactive
@ematipico ematipico temporarily deployed to aws June 17, 2022 17:14 Inactive
@ematipico ematipico temporarily deployed to aws June 17, 2022 17:31 Inactive
Copy link
Contributor

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Looking good. Can you add our metrics to the test plan?

@ematipico ematipico temporarily deployed to aws June 20, 2022 12:26 Inactive
@ematipico
Copy link
Contributor Author

Looking good. Can you add our metrics to the test plan?

Done!

@ematipico ematipico merged commit f7e5970 into main Jun 20, 2022
@ematipico ematipico deleted the feature/assignment-chain branch June 20, 2022 12:36
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.

Nested assignments
3 participants