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

feat(rome_js_formatter): variable declarator formatting #2762

Merged
merged 2 commits into from
Jun 22, 2022

Conversation

denbezrukov
Copy link
Contributor

@denbezrukov denbezrukov commented Jun 22, 2022

Summary

This PR partially implements #2423

Prev PR with discussion: #2737

current:
File Based Average Prettier Similarity: 76.28%
Line Based Average Prettier Similarity: 70.99%

main:
File Based Average Prettier Similarity: 75.24%
Line Based Average Prettier Similarity: 70.33%

This PR adds

  • OnlyLeft layout to the list of layouts. This layout is hit, usually, when variable declarator doesn't have initializer.
  • New conditional branch for should_break_left_hand_side when variable declaration has_complex_type_annotation. Prettier hasComplexTypeAnnotation.
  • New conditional branch for should_not_indent_if_parent_indents. Don't indent if great_parent is JS_VARIABLE_DECLARATOR.

Test Plan

Add new tests specs

Copy link
Contributor

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

That's great! Could you add the compatibility numbers?

You'd need to run

REPORT_PRETTIER=1 cargo test

And copy the lines at the bottom of the file report.md.

It will be something like

**File Based Average Prettier Similarity**: 75.24%  
**Line Based Average Prettier Similarity**: 70.33%  

On this PR and main (both updated) so we can see how much we got closer!

crates/rome_js_formatter/src/utils/assignment_like.rs Outdated Show resolved Hide resolved
crates/rome_js_formatter/src/utils/assignment_like.rs Outdated Show resolved Hide resolved
@denbezrukov
Copy link
Contributor Author

current:
File Based Average Prettier Similarity: 76.28%
Line Based Average Prettier Similarity: 70.99%

main:
File Based Average Prettier Similarity: 75.24%
Line Based Average Prettier Similarity: 70.33%

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.

3 participants