bendns' repos / helix
Unnamed repository; edit this file 'description' to name the repository.
Fix align_selections command (#15342)
* Add integration test for bug #15340 This test case handles aligning rows with varying numbers of selections. A bug was found where things would become misaligned in this case. * Fix align_selections command The align selections command wouldn't line up properly when lines had different numbers of selections. The existing code was hard for me to understand with nested vecs and sorting. So I rewrote a large portion of the function body. Now there's two vecs without nested structure, so it should be more efficient and easier to reason about. It also fixes the bug, and rows with varying numbers of selections align.
Browse Source
Andrew Davis 3 months ago
parent 4acbcd8 · commit d3c238f
Diffstat
-rw-r--r--helix-term/src/commands.rs80
-rw-r--r--helix-term/tests/test/commands.rs22
2 files changed, 69 insertions, 33 deletions
diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs
index c2dd13af..5c791303 100644
--- a/helix-term/src/commands.rs
+++ b/helix-term/src/commands.rs
@@ -1068,9 +1068,13 @@ fn align_selections(cx: &mut Context) {
let selection = doc.selection(view.id);
let tab_width = doc.tab_width();
- let mut column_widths: Vec<Vec<_>> = Vec::new();
- let mut last_line = text.len_lines() + 1;
- let mut col = 0;
+
+ let mut column_widths: Vec<usize> = Vec::new();
+ let mut coordinates = Vec::with_capacity(selection.len());
+
+ let mut previous_line = usize::MAX;
+ let mut col_idx = 0;
+ let mut running_offset = 0;
for range in selection {
let coords = visual_coords_at_pos(text, range.head, tab_width);
@@ -1081,48 +1085,58 @@ fn align_selections(cx: &mut Context) {
.set_error("align cannot work with multi line selections");
return;
}
+ if coords.row != previous_line {
+ col_idx = 0;
+ running_offset = 0;
+ previous_line = coords.row;
+ }
- col = if coords.row == last_line { col + 1 } else { 0 };
+ let width = coords.col - running_offset;
- if col >= column_widths.len() {
- column_widths.push(Vec::new());
+ match column_widths.get_mut(col_idx) {
+ Some(n) => *n = (*n).max(width),
+ None => column_widths.push(width),
}
- column_widths[col].push((range.from(), coords.col));
-
- last_line = coords.row;
- }
- let mut changes = Vec::with_capacity(selection.len());
+ coordinates.push(coords);
- // Account for changes on each row
- let len = column_widths.first().map(|cols| cols.len()).unwrap_or(0);
- let mut offs = vec![0; len];
+ running_offset += width;
+ col_idx += 1;
+ }
- for col in column_widths {
- let max_col = col
- .iter()
- .enumerate()
- .map(|(row, (_, cursor))| *cursor + offs[row])
- .max()
- .unwrap_or(0);
+ let column_positions: Vec<_> = column_widths
+ .into_iter()
+ .scan(0, |sum, n| {
+ *sum += n;
+ Some(*sum)
+ })
+ .collect();
- for (row, (insert_pos, last_col)) in col.into_iter().enumerate() {
- let ins_count = max_col - (last_col + offs[row]);
+ previous_line = usize::MAX;
- if ins_count == 0 {
- continue;
+ let changes = coordinates
+ .into_iter()
+ .zip(selection)
+ .map(|(coords, range)| {
+ if coords.row != previous_line {
+ col_idx = 0;
+ running_offset = 0;
+ previous_line = coords.row;
}
+ let current_inserts = column_positions[col_idx] - coords.col - running_offset;
+ let insert_pos = range.from();
- offs[row] += ins_count;
-
- changes.push((insert_pos, insert_pos, Some(" ".repeat(ins_count).into())));
- }
- }
+ col_idx += 1;
+ running_offset += current_inserts;
- // The changeset has to be sorted
- changes.sort_unstable_by_key(|(from, _, _)| *from);
+ (
+ insert_pos,
+ insert_pos,
+ Some(" ".repeat(current_inserts).into()),
+ )
+ });
- let transaction = Transaction::change(doc.text(), changes.into_iter());
+ let transaction = Transaction::change(doc.text(), changes);
doc.apply(&transaction, view.id);
exit_select_mode(cx);
}
diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs
index 9e87cead..97653c7e 100644
--- a/helix-term/tests/test/commands.rs
+++ b/helix-term/tests/test/commands.rs
@@ -868,3 +868,25 @@ async fn global_search_with_multibyte_chars() -> anyhow::Result<()> {
Ok(())
}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn align_selections_with_varying_columns() -> anyhow::Result<()> {
+ test((
+ indoc! {r"
+ #[|]#I I II I
+ IIIIIIIII
+ IIIII
+ IIIIIIIII
+ "},
+ r"%sI<ret>&gg",
+ indoc! {r"
+ #[I|]# I II I
+ I I II IIIII
+ I I II I
+ I I II IIIII
+ "},
+ ))
+ .await?;
+
+ Ok(())
+}