-
Notifications
You must be signed in to change notification settings - Fork 378
Move lines with Alt+Up/Down #230
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
Move lines with Alt+Up/Down #230
Conversation
@microsoft-github-policy-service agree |
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.
Damn, that's a thorough PR. 😅 This may take me a bit to come back to, as I focus on bug fixes first.
There's also #166 which is a competing solution for this. I believe yours is likely to be more correct though. I think I may just merge the other one first and then yours, given that the other PR is very small.
for sure. No problem. And no pressure btw, I know this project got a pretty big hug of death from the community and you got a lot on your plate rn so thanks for even looking at this. I might look for some other smaller things to tackle. GL! |
124d978
to
d37c46f
Compare
- Respects vscode functionality: - 1. if no selection is made, move the line at the current cursor up/down - 2. if a selection is made, move all lines spanning the selection up/down
d37c46f
to
15d9953
Compare
This solution is definitely better than mine is currently. I'm probably going to rewrite mine soon-ish to be closer to this. Not trying to make this some sort of competition though xD |
After a bit of thinking, I think my solution should be retired. This is like... 10x better. |
…election_lines_with_alt_up_down
|
||
self.cursor_move_to_logical(Point { x: 0, y: paste }); | ||
self.edit_begin(HistoryType::Write, self.cursor); | ||
if line.is_empty() || self.cursor.logical_pos.y != paste { |
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.
|| self.cursor.logical_pos.y != paste
what common occurrence does this handle? The cursor couldn't be moved to the paste line, so it's not on the paste line..?
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.
Oh, I wanted to document this code before submitting it and forgot. Let me fix that.
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.
Done.
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.
OK I think I get it
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.
It's beautiful. Thanks both of you for working on it
Closes #27
In this pr
[feat]: move cursor or selection lines with alt+up/down
[dev][feat]: add tests for
move_selected_lines
commandImportant
I couldnt figure out how to get the TextBuffer's scratch arena to work with parallel tests so they need to be run with
--test-threads=1
Additional Notes
Atm undoing an operationctrl+z
sets the selection to the entire range of the source text & swap location text.This is due to how i use a selection to make sure im writing to the correct location withTextBuffer.write()
This is wrong and the restored selection should be the original selection before the move.Any suggestions on correcting this?scratch_arena
?Result
return instead of a boolean for the failure cases.images
