fix: remove parenthesis should ensure space #15857
Conversation
rustbot
added
the
S-waiting-on-review
| // we should use `find_node_at_offset` at `SourceFile` level to get expectant `Between` | ||
| let token_at_offset = ctx | ||
| .find_node_at_offset::<ast::SourceFile>()? | ||
| .syntax() | ||
| .token_at_offset(parens.syntax().text_range().start()); | ||
| let need_to_add_ws = match token_at_offset { | ||
| syntax::TokenAtOffset::Between(before, _after) => { | ||
| // anyother `SyntaxKind` we missing here? | ||
| let tokens = vec![T![&], T![!], T!['('], T!['['], T!['{']]; | ||
| before.kind() != SyntaxKind::WHITESPACE && !tokens.contains(&before.kind()) | ||
| } | ||
| _ => false, | ||
| }; | ||
| let expr = if need_to_add_ws { format!(" {}", expr) } else { expr.to_string() }; |
There was a problem hiding this comment.
Two things, first this "analysis" should go into the |builder| closure so its only executed if the user makes use of the assist (for perf reasons).
Second, we can do this simpler, we can just take the preceding token of the expression via expr.syntax().first_token().and_then(|it| it.prev_token()) and check that for whitespace, if its not a whitespace token we should insert one.
As usual, ideally we'd have a formatter that just generally formats our outputs but thats a far future thing
There was a problem hiding this comment.
Ah seeing the commented out tests I just realized I was tunnel visioning on the match(1+1) thing, my approach doesnt generally work for other stuff I suppose hmm ... Oh or did you forget to uncomment the tests again? Since you d ospecial case things in way that should work there ithink
There was a problem hiding this comment.
Sorry, I forgot to uncomment them after debug. Your approach works well over all the tests 👍
|
@Veykril could you please review it again when you have time? |
|
@bors r+ |
|
☀️ Test successful - checks-actions |
close #15844