Fix short-if formatting #2778
Fix short-if formatting #2778
Conversation
|
@nrc how can I auto-update the target files formatting when making some changes in rustfmt codebase? I used to update them with |
| @@ -708,7 +708,7 @@ impl Rewrite for ast::Stmt { | |||
| }; | |||
|
|
|||
| let shape = shape.sub_width(suffix.len())?; | |||
| format_expr(ex, ExprType::Statement, context, shape).map(|s| s + suffix) | |||
There was a problem hiding this comment.
I think that this is incorrect. We should only put the if expression in a single line if it is the last statement of the block.
tests/target/expr.rs
Outdated
| { | ||
| for _ in 0..10 {} | ||
| } | ||
| { for _ in 0..10 {} } |
There was a problem hiding this comment.
This should not happen, we never put a block statement in a single line.
tests/target/expr.rs
Outdated
| } | ||
| } | ||
| } | ||
| { { { {} } } } |
tests/target/fn-single-line.rs
Outdated
| { | ||
| "inner-block" | ||
| } | ||
| { "inner-block" } |
There's no good way to do this, sorry. If all the tests have the same config, then you can just create a rustfmt.toml and put it in some surrounding directory, then run rustfmt. Otherwise you have to do things manually. |
csmoe
force-pushed
the
if
branch
4 times, most recently
from
20aec94 to
c766393
Compare
|
@topecongiro ping for review. |
There was a problem hiding this comment.
Thank you for the update! Though the current implementation is not correct yet: we only want to put an if statement on a single line only if it is the last statement.
So for example,
fn f() {
if x { foo() } else { bar() }
if x { foo() } else { bar() }
}should become
fn f() {
if x {
foo()
} else {
bar()
}
if x { foo() } else { bar() }
}
src/expr.rs
Outdated
| @@ -710,7 +710,13 @@ impl Rewrite for ast::Stmt { | |||
| }; | |||
|
|
|||
| let shape = shape.sub_width(suffix.len())?; | |||
| format_expr(ex, ExprType::Statement, context, shape).map(|s| s + suffix) | |||
| let expr_type = | |||
| if stmt_is_expr(&self) && context.snippet(self.span).starts_with("if ") { | |||
There was a problem hiding this comment.
This is a bit hacky - you can inspect the ast::ExprKind to see whether the given expression is if or not.
|
@topecongiro @csmoe |
|
@csmoe @petrochenkov My apologies for the late reply. I missed the notification of new commits being pushed to this PR. Since this could not get merged before 1.0 release, the formatting change needs to be version gated. Also we need to fix the merge conflicts. I think in the current form getting these done are a bit tough, so I would rather fix the issue in a separate PR. @csmoe Again, I am very sorry that I could not review the PR in a timely manner. I really appreciate your contribution to rustfmt. |
Cloese #2777