Repository navigation
Conversation
|
Would love native support for this, I currently have this roughly sketched out with macros but it would be nice for it not to destroy my selections while doing so: keys.normal.A-j = "@Xdp"
keys.normal.A-k = "@XdkP"
keys.select.A-j = "@<esc>Xdp"
keys.select.A-k = "@<esc>XdkP" |
7154c0e to
970f5ae
Compare
970f5ae to
f72aad7
Compare
|
trying this pr out and am enjoying it more than I expected to... it's really smooth 😋 |
|
Maybe we should consider making the default keybinds for that as in the There are already linewise mappings in Helix done the same way: So, what about making the |
|
that makes the pr a harder sell to the maintainers, I'd love for this to get merged first and then possibly add mappings :3 |
|
the pr already has mappings @Axlefublr, |
|
dear github, |
|
@the-mikedavis I'd like to request a review, if you don't mind. I know you all tend to leave PRs you don't want open so others can pull from them, but I would be interested to hear what you think in response to the points in the description here. I think this is a good fit for a core function of the editor and subsequently a bad fit for bindings / a future plugin. |
|
In terms of the feature itself I wasn't a fan of it in the past but I understand it's a popular ask. I haven't looked at the code yet but if it can be done in a way that's fairly clean and it's not buggy then I'm not opposed to it |
| let slice = text.slice(..); | ||
| let mut last_step_changes: Vec<ExtendedChange> = vec![]; | ||
| let mut at_doc_edge = false; | ||
| let all_changes = selection.into_iter().map(|range| { |
There was a problem hiding this comment.
Rather than iterating over all ranges here, Selection::line_ranges might be simpler. It's meant for line-wise stuff like this: it gives an iterator which merges overlapping and adjacent ranges. See its use in :reset-diff-change for an example
There was a problem hiding this comment.
For one test case let's have at least two selections on the same line. I haven't looked through the command's impl fully yet but it's something that might break some assumptions and maybe lead to a panic
Add support for moving selected lines up and down don't trim trailing whitespace in tests lol
Add support for moving selected lines up and down don't trim trailing whitespace in tests lol
Add support for moving selected lines up and down don't trim trailing whitespace in tests lol
Add support for moving selected lines up and down don't trim trailing whitespace in tests lol Co-authored-by: sireliah <sajuukk@gmail.com>
|
Haven't seen this posted yet; to @the-mikedavis's point about testing multiple cursors on one line, it looks like doing that does indeed mess up the buffer pretty bad. In my vid you can see I have a few cursors on one line, then move the line up a few times, try undoing all the way, and Maybe @the-mikedavis's suggestion about move_lines_up-multi-cursor-bug.mp4 |
I think my change works with cursors all over the place, if you want to get some inspiration: #14656 |
Add support for moving selected lines up and down don't trim trailing whitespace in tests lol Co-authored-by: sireliah <sajuukk@gmail.com>
Add support for moving selected lines up and down don't trim trailing whitespace in tests lol Co-authored-by: sireliah <sajuukk@gmail.com>
Add support for moving selected lines up and down don't trim trailing whitespace in tests lol Co-authored-by: sireliah <sajuukk@gmail.com>
Add support for moving selected lines up and down don't trim trailing whitespace in tests lol Co-authored-by: sireliah <sajuukk@gmail.com>
Add support for moving selected lines up and down don't trim trailing whitespace in tests lol Co-authored-by: sireliah <sajuukk@gmail.com>
Hi! I'd like to reopen discussion on #2245.
This patch adds support for moving the current line (or some arbitrary selected lines) up and down.
It is a rebased, bugfixed version of #4545. It's bugfixed, stable, behaves predictably with multiple cursors, doesn't have issues with Unicode or with moving before/after a file start/end, etc. I've been running this as a patch on my personal fork for a couple of years now and rely on it heavily.
However, Helix has recently been merging a lot of PRs that I was running the fork for -- so I'm trying to move away from using a fork. I thought I would reopen discussion on upstreaming this because I think it may be useful to others.
There are two lines of argumentation that have been made in the past for keeping this out of Helix core.
This is a matter of perspective. Some operations -- like this one -- are inherently line-based. If an operation is inherently line-based, it doesn't operate on the full selection. This still fits into the select-than-act paradigm, in my mind -- your initial selection is the current line, which can be expanded to multiple lines across continuous or discontinuous selections.
It's the same thing as
insert_at_line_start, imo.It can't. It can be roughly implemented in terms of existing commands -- but only roughly.
Per discussion on #2245 (which has been ongoing -- a testament to this patch's utility, imo) the closest someone's gotten is
["extend_to_line_bounds", "delete_selection", "paste_after", "select_mode", "goto_line_start", "normal_mode"]. But even that doesn't preserve the cursor position after moving lines: which is really important!Line movement is a core enough utility with enough edge cases that I think it is worth being built in.