-
Notifications
You must be signed in to change notification settings - Fork 1.9k
implement assist invert_if #2343
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
Conversation
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 put the assist cursor range to if of the if expression,
I think that's fine!
Is there any way to replace them without cover the cursor position?
There's set_cursor method to explicitelly set the cursor position after the assist has been applied
| let expr = ctx.find_node_at_offset::<ast::IfExpr>()?; | ||
| let expr_range = expr.syntax().text_range(); | ||
| let if_range = TextRange::offset_len(expr_range.start(), TextUnit::from_usize(2)); | ||
| let cursor_in_range = ctx.frange.range.is_subrange(&if_range); |
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.
If we are looking specifically for if token, it might be better to start with ctx.find_token_at_offset(T![if]), and cast it's parent to IfExpr
| let then_node = expr.then_branch()?.syntax().clone(); | ||
|
|
||
| if let ast::ElseBranch::Block(else_block) = expr.else_branch()? { | ||
| let flip_cond = undo_negation(cond.clone())?; |
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.
This might be a good opportunity for refactoring: undo_negation(node: SyntaxNode) -> SyntaxNode -> invert_boolean_expression(expr: ast::Expr) -> ast::Expr
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.
How could I create a new Expr in ra_assist crate? For example, 1 != 2 flip to 1 == 2 need a new Expr. I don't know how to create a new SyntaxNode. Maybe use a SyntaxTreeBuilder from ra_syntax crate?
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.
See edit and make modules in ra_syntax, and various replace and insert functions in the algo module. Note that some editing functions work on an untyped SyntaxNode tree, you can get back to typed ast::FooDef by casting
| edit.target(if_range); | ||
| edit.replace(cond_range, flip_cond); | ||
| edit.replace(else_range, then_node.text()); | ||
| edit.replace(then_range, else_node.text()); |
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.
Another option would be to use edit::replace_descendants method, though I am not sure it'll work here in the current form...
| edit.replace(then_range, else_node.text()); | ||
| }) | ||
| } else { | ||
| None |
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.
Let's do an early return instead of a hanging None
| // | ||
| // ``` | ||
| // fn main() { | ||
| // if<|> !y {A} else {B} |
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.
| // if<|> !y {A} else {B} | |
| // if<|> !y { A } else { B } |
|
LGTM! bors r+ |
2343: implement assist invert_if r=matklad a=bravomikekilo fix [issue 2219 invert if condition](#2219) I put the assist cursor range to `if` of the if expression, because both condition and body will be replaced. Is there any way to replace them without cover the cursor position? @matklad Co-authored-by: bravomikekilo <[email protected]>
Build succeeded
|
fix(docs): add newlines between prefix/suffix chapters
fix issue 2219 invert if condition
I put the assist cursor range to
ifof the if expression, because both condition and body will be replaced. Is there any way to replace them without cover the cursor position?@matklad