-
Notifications
You must be signed in to change notification settings - Fork 504
Implemented Line shifting using Alt+Up/Down #166
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
@microsoft-github-policy-service agree |
I'm too sleepy to do a full review today still1, but one thing I noticed at a cursory glance is that this code can be abstracted away, can't it? Moving a line up and down should roughly be similar logic in both directions so I think you could move this into a single function with a bool parameter. This would help keep the binary size low. Footnotes
|
Does not work in iTerm2, may also not work under other ones? CleanShot.2025-05-21.at.03.21.17.mp4 |
Yeah I wrote that while I was dead tired as well. Only reason I was up this late was because Google I/O went on for so long. I'll try to make both solutions closer to one another. I remember that I tried to make them essentially the same yesterday but that was causing some weird behavior. I'll see if I can get that weird behavior state for a recording when I try to adjust my current implementation. Honestly, I haven't worked with rust that much before, so this is good practice. Once I get moving down working with basically the same code as moving lines up, I'll abstract it. Edit: I managed to get back to that "weird behavior" by doing what I initially did yesterday, i.e. copying the "Shifting up" code and trying to adjust it for shifting down. I'll figure this out after school. 2025-05-21.08-06-20.mp4 |
That is really weird. I only added a condition to vk::Up and vk::Down for when Alt is being held... My only guess is that it's an issue with input handling on macOS. Can you or anyone else with a mac test if this also applies to the regular terminal or if it's only an iTerm2 thing? I don't have the money to buy a mac right now so I'm unable to test this myself. I'll test it under WSL to see if it's an issue on Linux, if so then it's likely something in input.rs by my guesses. I'll definitely get back to you about the Linux test after school, gotta head out in a few minutes so I don't miss the bus xD. |
CleanShot.2025-05-21.at.11.04.06.mp4It behaves differently than iTerm2, but it still does dos not work. It just moves the cursor. |
I'm going to make the assumption that the ALT modifier is not triggered by its mac equivelant key (Option if we want to mimic VS Code's shortcuts). If anyone wants to make Option trigger the Alt modifier, it would be greatly appreciated. The relevant code seems to be in input.rs starting at line 206. That said, I have no idea if my assumption is correct. Still no idea why iTerm2 would react differently to be honest. Maybe something with the shortcuts configuration in iTerm2? |
Well, I have a default keymap, and with default keymap iterm behaves like this. You 100% can change it through key remapping, but how many people are going to remap half of the keys if the terminal works in other use cases? |
Maybe we can use this? @JanluOfficial fn textarea_shift_line(&mut self, tb: &mut TextBuffer, direction: i32) {
let line_count = tb.visual_line_count() as i32;
let current_line = tb.cursor_visual_pos().y;
let target_line = current_line + direction;
if target_line < 0 || target_line >= line_count {
return;
}
let original_cursor = tb.cursor_visual_pos();
let first_line = current_line.min(target_line);
let second_line = current_line.max(target_line);
tb.cursor_move_to_logical(Point { x: 0, y: second_line });
let content = tb.extract_selection(true);
tb.cursor_move_to_logical(Point { x: 0, y: first_line });
tb.write(&content, true);
tb.cursor_move_to_logical(Point { x: original_cursor.x, y: target_line });
} |
Let me check if this works the exact same way as my current abstracted function. |
Okay so there is a bug, and I confirmed it happens with both your function and my lesser optimized function. Essentially, trying to shift up the last line in the file (or I guess technically the buffer) just adds it to the front of the line above instead of moving the line above down. I'll see what I can do here. Other than that, the new (way more optimized) function works perfectly. |
fixes the bug: fn textarea_shift_line(&mut self, tb: &mut TextBuffer, direction: i32) {
let line_count = tb.visual_line_count() as i32;
let current_line = tb.cursor_visual_pos().y;
let target_line = current_line + direction;
if target_line < 0 || target_line >= line_count {
return;
}
let original_cursor = tb.cursor_visual_pos();
let first_line = current_line.min(target_line);
let second_line = current_line.max(target_line);
tb.cursor_move_to_logical(Point { x: 0, y: first_line });
let mut content = tb.extract_selection(true);
tb.cursor_move_to_logical(Point { x: 0, y: second_line });
if second_line == line_count - 1 {
tb.write(b"\n", true);
tb.cursor_move_to_logical(Point { x: 0, y: second_line });
content.pop();
}
tb.write(&content, true);
tb.cursor_move_to_logical(Point { x: original_cursor.x, y: target_line });
} |
Thank you @diabloproject for the help. I greatly appreciate it. As for the macOS stuff, I can't really fix that since I don't own a mac, which I'd need for testing. Hopefully someone else can take care of that. |
No problem. I think the keyboard shortcuts for mac could be made in a separate PR that will introduce mappings, later, since it is probably out-of-scope for this task. |
Then I'd consider this PR mergeable. I'll let someone else take the mac thing because I don't own a mac unfortunately. |
I'll close this in favor of #230, as it's more thorough. Counter-intuitively though, its approach is not actually as much better as you may expect. It will require some simplifications. 🙂 |
About this PR
This PR adds the requested feature from #27, that being the ability for lines to be moved up and down using
Alt + Up/Down
Demo Video
2025-05-21.01-14-14.mp4