-
Notifications
You must be signed in to change notification settings - Fork 974
support closure block indent_style #3867
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
support closure block indent_style #3867
Conversation
|
CI failures are due to issues with latest nightly, see #3864 |
topecongiro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR!
78ba6e7 to
e41f32a
Compare
src/closures.rs
Outdated
| let param_shape = nested_shape.offset_left(1)?.visual_indent(0); | ||
| let param_shape = match indent_style { | ||
| IndentStyle::Block => { | ||
| if version == Version::Two { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should drop the version gating from here since this would be a v2.x change (not going into v1.x) correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will remove the version anyway so it would be helpful if you can remove it in this PR :)
topecongiro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! Just a bikeshedding on the style.
src/closures.rs
Outdated
| IndentStyle::Block => list_str.contains('\n') || list_str.len() > one_line_budget, | ||
| _ => false, | ||
| }; | ||
| let (param_str, put_params_in_block) = if multi_line_params && !item_vec.is_empty() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: would you mind simplifying this if block into something like the following
let put_params_in_block = multi_line_params && !item_vec.is_empty();
let param_str = if !put_params_in_block {
list_str
} else {
// ...
};There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: would you mind simplifying this
ifblock into something like the following
Good spot, that's a small change but definitely helps improve readability 👍 I will update it accordingly
2fa6f79 to
22fe6a5
Compare
Closes #3865