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

Indentation on multiline single generic bound #4730

Conversation

davidBar-On
Copy link
Contributor

Suggested fix for issue #4689. The change is that join_bounds_inner is calling itself with force_newline set, not only when the items.len() > 1, but also when the single item is Trait which contains segments array with more then one item, or AngleBracketed args array with more then one item.

I don't know rust well enough to understand whether the test cases fully cover these cases.

Comment on lines 1043 to 1081
let retry_with_force_newline =
if force_newline || (!result.0.contains('\n') && result.0.len() <= shape.width) {
false
} else {
if items.len() > 1 {
true
} else {
match items[0] {
ast::GenericBound::Trait(ref poly_trait_ref, ..) => {
let segments = &poly_trait_ref.trait_ref.path.segments;
if segments.len() > 1 {
true
} else {
if let Some(args_in) = &segments[0].args {
match &**args_in {
ast::AngleBracketed(args) => {
if args.args.len() > 1 {
true
} else {
false
}
}
_ => false,
}
} else {
false
}
}
}
_ => false,
}
}
};
Copy link
Member

Choose a reason for hiding this comment

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

👀 this is a pretty gnarly looking conditional.

before thinking about the approach, i'd really like to see if this can be refactored (e.g. with different control flows) to reduce the excessive nesting or otherwise extract it out entirely as a separate function to be able to reduce the levels and rightward drift

Copy link
Contributor Author

Choose a reason for hiding this comment

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

... or otherwise extract it out entirely as a separate function to be able to reduce the levels and rightward drift

Refactored by add is_segment_with_multi_items_array function which took out part of the conditions nesting. Also re-based, so old version is not available.

@davidBar-On davidBar-On force-pushed the issue-4689-indentation-on-multiline-single-generic-bound branch from 32e3ee9 to 879b592 Compare March 30, 2021 08:26
@calebcartwright calebcartwright changed the base branch from master to rustfmt-2.0.0-rc.2 April 30, 2021 00:25
@calebcartwright
Copy link
Member

I think this probably could be simplified a bit more (e.g. guards on the match arms) but really just stylistic nits so going to go ahead and merge, thank you!

@calebcartwright calebcartwright merged commit cc5521d into rust-lang:rustfmt-2.0.0-rc.2 Oct 23, 2021
@karyon karyon added backport-triage 1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release and removed backport-triage labels Oct 27, 2021
davidBar-On added a commit to davidBar-On/rustfmt that referenced this pull request Jun 18, 2022
@davidBar-On davidBar-On mentioned this pull request Jun 18, 2022
davidBar-On added a commit to davidBar-On/rustfmt that referenced this pull request Aug 9, 2022
ytmimi pushed a commit that referenced this pull request Aug 9, 2022
* Backport PR #4730 that fix issue #4689

* Test files for each Verion One and Two

* Simplify per review comment - use defer and matches!

* Changes per reviewer comments for reducing indentations
@ytmimi ytmimi added 1x-backport:completed and removed 1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release labels Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants