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): multiline single param only if func definition is multiline for all_params #9187

Merged
merged 3 commits into from
Oct 24, 2024

Conversation

ChiTimesChi
Copy link
Contributor

Motivation

Follow-up for #9176. Spec is defined in #9176 (comment)

Solution

  • When all_params is selected, multiline the single parameter only when the full function definition takes more than a single line (i.e. multilined).

@ChiTimesChi
Copy link
Contributor Author

@grandizzy please let me know if styling needs to be adjusted here. Seems to do the job functionality-wise.

@grandizzy
Copy link
Collaborator

grandizzy commented Oct 24, 2024

@grandizzy please let me know if styling needs to be adjusted here. Seems to do the job functionality-wise.

hey @ChiTimesChi please check https://github.com/foundry-rs/foundry/blob/master/CONTRIBUTING.md#resolving-an-issue for formatting, also to run only fmt tests run cargo test from crates/fmt

@ChiTimesChi
Copy link
Contributor Author

@grandizzy cargo test is all green for me. cargo +nightly fmt -- --check also passes, as well other commands from here:

foundry/CONTRIBUTING.md

Lines 85 to 90 in b74e467

```sh
cargo check --all
cargo test --all --all-features
cargo +nightly fmt -- --check
cargo +nightly clippy --all --all-targets --all-features -- -D warnings
```

I rebased the branch on top of the latest master, so this should be good for the review. Please let me know if I missed anything.

@grandizzy grandizzy merged commit 216b60a into foundry-rs:master Oct 24, 2024
21 checks passed
@ChiTimesChi ChiTimesChi deleted the issue-9173-2 branch October 30, 2024 14:21
rplusq pushed a commit to rplusq/foundry that referenced this pull request Nov 29, 2024
… for `all_params` (foundry-rs#9187)

* test: adjust single param multiline expected behavior

* fix: `AllParams` single param multiline condition

* refactor: try simplifying the condition logic
@grandizzy grandizzy added T-bug Type: bug C-forge Command: forge labels Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-forge Command: forge T-bug Type: bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants