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): params_first_multi split if more than one param #8762

Merged
merged 1 commit into from
Aug 28, 2024

Conversation

grandizzy
Copy link
Collaborator

@grandizzy grandizzy commented Aug 28, 2024

Motivation

Closes #8761 - introduced breaking change for params_first

As suggested in #8761 (comment)
params_first Writes function parameters multiline first regardless number of params
params_first_multiline new option added which only splits parameters onto their own line when there is more than one param

Solution

@grandizzy grandizzy marked this pull request as ready for review August 28, 2024 05:43
@grandizzy grandizzy enabled auto-merge (squash) August 28, 2024 06:45
@grandizzy grandizzy merged commit e0aeef9 into foundry-rs:master Aug 28, 2024
20 checks passed
@mds1
Copy link
Collaborator

mds1 commented Aug 28, 2024

@grandizzy this is great overall, thank you! It seems there might be one remaining issue though: we use multiline_func_header='all', and when running this new version, the diff is now significantly smaller, but there are still a few diffs that look odd and seem unexpected, as shown in the screenshots, all around if/else statements
image
image
image

@grandizzy
Copy link
Collaborator Author

@grandizzy this is great overall, thank you! It seems there might be one remaining issue though

yeah, was able to reproduce, will make a fix for

kcsongor added a commit to wormhole-foundation/native-token-transfers that referenced this pull request Aug 29, 2024
This partially undoes #501, which itself updated formatting after an
upstream foundry change. That upstream change has been reverted
foundry-rs/foundry#8762, so we do the same here.
nik-suri pushed a commit to wormhole-foundation/native-token-transfers that referenced this pull request Aug 29, 2024
This partially undoes #501, which itself updated formatting after an
upstream foundry change. That upstream change has been reverted
foundry-rs/foundry#8762, so we do the same here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(fmt): regression introduced in PR 8637
3 participants