Combine more expressions #1621
Combine more expressions #1621
Conversation
There was a problem hiding this comment.
The code looks good, and I think the implementation will make discussion on rust-lang/style-team#61 easier. But given that we're still deciding there, could you put the control flow exprs behind an option please?
|
Thank you for your comment! I have a question, after #1520 (#525) rustfmt allows |
I would keep the current behaviour for now |
|
Added an option for combining control expressions and refactoring/bug fixes to avoid regressions. |
|
Thank you for you feedback! |
If the method chain goes multi line before the last element, disallow combining the method chain.
|
Added commits to disallow combining method chain if it uses vertical layout and rebased to resolve conficts. |
src/types.rs
Outdated
| Shape::legacy(budget, shape.indent + used_width)) | ||
| }) | ||
| .collect::<Option<Vec<_>>>() | ||
| ) |
There was a problem hiding this comment.
this seems unfortunate, especially as the very similar example below looks better
There was a problem hiding this comment.
Thanks for pointing out, this was caused by rustfmt not allowing the chain item to stay on the same line with the closing parens when context.use_block_indent() returns false.
In the similar example, .join(", ") was kept on the same line because the chain was inside the function call which was formatted in block style and hence context.use_block_indent() returned true.
|
I think we should perhaps default combine_control_expr to false (and add it as true to rfc-rustfmt.toml) |
|
I added commits to reflect the feedbacks. |
|
Thanks for all the changes! |
This PR allows more expressions to be combined with function calls, but not with match arms because I am not sure how combining interacts with
wrap_match_armsoption.If it is prefered to wait before implementing, I am happy to close this without merging.