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

fix(fmt): write files on disk only if they're not perfect match #8775

Merged
merged 4 commits into from
Aug 30, 2024

Conversation

grandizzy
Copy link
Collaborator

@grandizzy grandizzy commented Aug 29, 2024

Motivation

Closes #6033
Closes #8769
Fixes issue reported in #8762 (comment)

Solution

  • do not write files on disk if no change (ratio = 1.0)
  • return false if statement empty and block doesn't fit on a single line

@zerosnacks
Copy link
Member

zerosnacks commented Aug 29, 2024

Fix works! Haven't been able to replicate the issue of #8769 with this PR, even with multiple modified files

@grandizzy grandizzy marked this pull request as ready for review August 29, 2024 13:22
@grandizzy grandizzy enabled auto-merge (squash) August 29, 2024 13:23
@mds1
Copy link
Collaborator

mds1 commented Aug 29, 2024

Does this also fix #8762 (comment)? Wondering if I should test this PR to verify

@grandizzy
Copy link
Collaborator Author

Does this also fix #8762 (comment)? Wondering if I should test this PR to verify

No, it doesn't, I'll work on it next and tag you when ready.

@mds1
Copy link
Collaborator

mds1 commented Aug 29, 2024

Got it, thanks :)

@grandizzy
Copy link
Collaborator Author

grandizzy commented Aug 30, 2024

Got it, thanks :)

hey @mds1 I included a fix for in this PR pls see 00d1a31 The issue was related to if/else with empty statements: when prev branch didn't have any statement then the current branch was moved on a new line, pls give it a try, thank you!

Copy link
Member

@zerosnacks zerosnacks left a comment

Choose a reason for hiding this comment

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

Thanks!

@grandizzy grandizzy merged commit 818eeb9 into foundry-rs:master Aug 30, 2024
20 checks passed
@grandizzy grandizzy deleted the issue-6033 branch August 30, 2024 11:45
@mds1
Copy link
Collaborator

mds1 commented Aug 30, 2024

@grandizzy Confirmed that forge fmt using this PR results in no more diff, so that issue is fixed—thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants