Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Can't move item command in some circumstances #8236

Closed
redactedscribe opened this issue Mar 29, 2021 · 4 comments · Fixed by #8256
Closed

Can't move item command in some circumstances #8236

redactedscribe opened this issue Mar 29, 2021 · 4 comments · Fixed by #8256
Labels
S-actionable Someone could pick this issue up and work on it right now

Comments

@redactedscribe
Copy link

Move item command was recently added to rust-analyzer.

|#[derive(Debug)]
enum FooBar {
    Foo,
    Bar,
}

If the cursor is on the left side of the # symbol, the move item action cannot be performed. Same goes for the keywords fn or impl (or an enum without an attribute above it).

|const FOO: &str = "Bar";

With the cursor on the left side of a const however, the move action works without needing to "be inside" it (i.e. cursor to the right side of c).

fn foobar() {
    // ...
}|

Interestingly, if the cursor is on the right side of the ending curly brace of a function (what could be considered "not inside" the function), the move action still succeeds.

I don't know if this all expected behavior, but I thought I'd mention it as it seems inconsistent.

Thanks.

rust-analyzer nightly 01dc53a (2021-03-29)
@Veykril Veykril added the S-actionable Someone could pick this issue up and work on it right now label Mar 29, 2021
@ivan770
Copy link
Contributor

ivan770 commented Mar 29, 2021

Movability of some nodes (like attributes) can be added by adding its node to this list https://github.com/rust-analyzer/rust-analyzer/blob/5dd6b931388dac00d272a41a139c4f0cc3c449dc/crates/ide/src/move_item.rs#L44-L70
About putting cursor at the start or end of some nodes - yeah, that seems to be a problem. Current algorithm relies simply on getting first token/node that is under cursor, checking if it's eligible for moving, and if it can be moved, we move it. If item under cursor is not movable - we expand selection for next node ancestor, and so on, until we find a movable node.

@ivan770
Copy link
Contributor

ivan770 commented Mar 29, 2021

I can work on that, though it would be awesome if someone collected a list of some common movability inconsistencies.

@Veykril
Copy link
Member

Veykril commented Mar 29, 2021

I think you can check if the selection range is empty and in that case take a biased(biased towards non whitespace) token, via file.token_at_offset(position.offset), like hover does for example https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/ide/src/hover.rs#L93. That should fix most problems with no selection, I think?

@ivan770
Copy link
Contributor

ivan770 commented Mar 29, 2021

I think you can check if the selection range is empty and in that case take a biased(biased towards non whitespace) token, via file.token_at_offset(position.offset), like hover does for example https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/ide/src/hover.rs#L93. That should fix most problems with no selection, I think?

I'll take a look at it, thanks!

@ivan770 ivan770 mentioned this issue Mar 30, 2021
Merged
@bors bors bot closed this as completed in 0b68e03 Mar 30, 2021
@bors bors bot closed this as completed in #8256 Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Item movers improvements ivan770/rust-analyzer
3 participants
@Veykril @ivan770 @redactedscribe