Skip to content

fix: fix cursor position after item move command #8967

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

Merged
merged 1 commit into from
May 24, 2021

Conversation

dbofmmbt
Copy link
Contributor

@dbofmmbt dbofmmbt commented May 24, 2021

Closes #8492

@dbofmmbt
Copy link
Contributor Author

dbofmmbt commented May 24, 2021

I started by trying to add tests for the cases I was referring to in #8492. It seems that the TextEdit generated by move_item is correct for the cases I added, but the cursor position was not in the right position when I tested on VS Code.

I'll continue looking to see if I can find the cause of the undesired offset.

@dbofmmbt
Copy link
Contributor Author

Thanks for the links! The problem is certainly on VS side. There's a off-by-one error in something involving startLine on snippets.ts:42. Maybe with lineDelta...

@dbofmmbt
Copy link
Contributor Author

dbofmmbt commented May 24, 2021

@matklad I think I found the issue. See the following line updating lineDelta:

https://github.com/eduardocanellas/rust-analyzer/blob/3bc69f362d8c173ce4748e7e853c0da5183e557a/editors/code/src/snippets.ts#L55

I think that it should increment lineDelta instead of overwritting its value. When I changed = to += the item movers seemed to work perfectly in the case I was checking. Looks reasonable?

It really feels good to have the item movers not moving the cursor around =D

@dbofmmbt dbofmmbt marked this pull request as ready for review May 24, 2021 16:11
@dbofmmbt dbofmmbt force-pushed the eduardocanellas/issue8492 branch from 1ac9934 to 3dce8a3 Compare May 24, 2021 16:17
@matklad
Copy link
Member

matklad commented May 24, 2021

bors r+

Thanks @eduardocanellas

@bors
Copy link
Contributor

bors bot commented May 24, 2021

@bors bors bot merged commit a59d41c into rust-lang:master May 24, 2021
@dbofmmbt dbofmmbt deleted the eduardocanellas/issue8492 branch May 24, 2021 16:45
@lnicola lnicola changed the title fix cursor position after item move command fix: fix cursor position after item move command May 25, 2021
@lnicola
Copy link
Member

lnicola commented May 27, 2021

changelog fix (first contribution) fix cursor position after move item command

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Item movers need some fixes
3 participants