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

feat(rome_js_formatter): Binary parentheses #3002

Merged
merged 3 commits into from
Aug 4, 2022

Conversation

MichaReiser
Copy link
Contributor

@MichaReiser MichaReiser commented Aug 4, 2022

This PR refines the rules for setting parentheses around binary expression to more closely match Prettier's behaviour.

It does so by re-using OperatorPrecedence enum from the parser crate (which is why it is moved to rome_js_syntax).

Tests

Reviewed the updates to the prettier snapshots.

File Based Average Prettier Similarity: 78.58% -> 78.85%
Line Based Average Prettier Similarity: 74.00% -> 74.25%

@MichaReiser MichaReiser force-pushed the feat/binary-parentheses branch from c6f51eb to 7c5de7e Compare August 4, 2022 08:55
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Aug 4, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5ab7e6d
Status:⚡️  Build in progress...

View logs

@MichaReiser MichaReiser marked this pull request as ready for review August 4, 2022 09:02
@MichaReiser MichaReiser requested a review from a team August 4, 2022 09:02
@MichaReiser MichaReiser force-pushed the feat/binary-parentheses branch from 7c5de7e to fc56855 Compare August 4, 2022 09:05
@MichaReiser MichaReiser temporarily deployed to aws August 4, 2022 09:05 Inactive
@github-actions
Copy link

github-actions bot commented Aug 4, 2022

@github-actions
Copy link

github-actions bot commented Aug 4, 2022

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45878 45878 0
Passed 44938 44938 0
Failed 940 940 0
Panics 0 0 0
Coverage 97.95% 97.95% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 39 39 0
Passed 36 36 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.31% 92.31% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 5946 5946 0
Passed 395 395 0
Failed 5551 5551 0
Panics 0 0 0
Coverage 6.64% 6.64% 0.00%

ts/babel

Test result main count This PR count Difference
Total 588 588 0
Passed 519 519 0
Failed 69 69 0
Panics 0 0 0
Coverage 88.27% 88.27% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 16257 16257 0
Passed 12397 12397 0
Failed 3860 3860 0
Panics 0 0 0
Coverage 76.26% 76.26% 0.00%

This PR refines the rules for setting parentheses around binary expression to more closely match Prettier's behaviour.

It does so by re-using `OperatorPrecedence` enum from the parser crate (which is why it is moved to `rome_js_syntax`).

Reviewed the updates to the prettier snapshots.

**File Based Average Prettier Similarity**: 78.38%
**Line Based Average Prettier Similarity**: 73.45%
@MichaReiser MichaReiser force-pushed the feat/binary-parentheses branch from fc56855 to 4ee2925 Compare August 4, 2022 09:35
@MichaReiser MichaReiser temporarily deployed to aws August 4, 2022 09:35 Inactive
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.

Great progresses! 🎆

let current_precedence = current_operator.precedence();
let parent_precedence = parent_operator.precedence();

#[allow(clippy::if_same_then_else)]
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose you added this because it helps for readability?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I find it easier to reason if every if handles a specific case rather than grouping them all together because the main point here isn't about the body (which is a simple bool) but about the condition.

@ematipico ematipico added this to the 0.9.0 milestone Aug 4, 2022
@ematipico ematipico added the A-Formatter Area: formatter label Aug 4, 2022
@MichaReiser MichaReiser temporarily deployed to aws August 4, 2022 15:35 Inactive
@MichaReiser MichaReiser temporarily deployed to aws August 4, 2022 15:44 Inactive
@MichaReiser MichaReiser merged commit ff4153b into main Aug 4, 2022
@MichaReiser MichaReiser deleted the feat/binary-parentheses branch August 4, 2022 15:49
IWANABETHATGUY pushed a commit to IWANABETHATGUY/tools that referenced this pull request Aug 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Formatter Area: formatter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载