From c262fe41ab1aedfa50c2443fbc2b2946aaf8121d Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Wed, 25 Dec 2024 11:21:19 -0500 Subject: [PATCH] Consistently replace line-endings in paste/replace commands Previously we replaced line-endings in pasted text to the document line-ending for some values in paste commands. We missed the `repeat` values in paste though and didn't do any replacement in the replace command. Along with this change I've refactored the replace command to avoid intermediary collections. We previously eagerly collected the values from the input register as a `Vec` but we can avoid both of those conversions and only allocate for the conversion to a `Tendril`. We can also switch from `str::repeat` to a manual implementation to avoid the intermediary conversion to a String - this avoids an extra allocation in the common case (i.e. no count). Fixes #12329 --- helix-term/src/commands.rs | 61 ++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 04e39e5e..a93fa445 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -4401,6 +4401,8 @@ enum Paste { Cursor, } +static LINE_ENDING_REGEX: Lazy = Lazy::new(|| Regex::new(r"\r\n|\r|\n").unwrap()); + fn paste_impl( values: &[String], doc: &mut Document, @@ -4417,26 +4419,26 @@ fn paste_impl( doc.append_changes_to_history(view); } - let repeat = std::iter::repeat( - // `values` is asserted to have at least one entry above. - values - .last() - .map(|value| Tendril::from(value.repeat(count))) - .unwrap(), - ); - // if any of values ends with a line ending, it's linewise paste let linewise = values .iter() .any(|value| get_line_ending_of_str(value).is_some()); - // Only compiled once. - static REGEX: Lazy = Lazy::new(|| Regex::new(r"\r\n|\r|\n").unwrap()); - let mut values = values - .iter() - .map(|value| REGEX.replace_all(value, doc.line_ending.as_str())) - .map(|value| Tendril::from(value.as_ref().repeat(count))) - .chain(repeat); + let map_value = |value| { + let value = LINE_ENDING_REGEX.replace_all(value, doc.line_ending.as_str()); + let mut out = Tendril::from(value.as_ref()); + for _ in 1..count { + out.push_str(&value); + } + out + }; + + let repeat = std::iter::repeat( + // `values` is asserted to have at least one entry above. + map_value(values.last().unwrap()), + ); + + let mut values = values.iter().map(|value| map_value(value)).chain(repeat); let text = doc.text(); let selection = doc.selection(view.id); @@ -4533,19 +4535,24 @@ fn replace_with_yanked_impl(editor: &mut Editor, register: char, count: usize) { else { return; }; - let values: Vec<_> = values.map(|value| value.to_string()).collect(); let scrolloff = editor.config().scrolloff; + let (view, doc) = current_ref!(editor); - let (view, doc) = current!(editor); - let repeat = std::iter::repeat( - values - .last() - .map(|value| Tendril::from(&value.repeat(count))) - .unwrap(), - ); - let mut values = values - .iter() - .map(|value| Tendril::from(&value.repeat(count))) + let map_value = |value: &Cow| { + let value = LINE_ENDING_REGEX.replace_all(value, doc.line_ending.as_str()); + let mut out = Tendril::from(value.as_ref()); + for _ in 1..count { + out.push_str(&value); + } + out + }; + let mut values_rev = values.rev().peekable(); + // `values` is asserted to have at least one entry above. + let last = values_rev.peek().unwrap(); + let repeat = std::iter::repeat(map_value(last)); + let mut values = values_rev + .rev() + .map(|value| map_value(&value)) .chain(repeat); let selection = doc.selection(view.id); let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { @@ -4555,7 +4562,9 @@ fn replace_with_yanked_impl(editor: &mut Editor, register: char, count: usize) { (range.from(), range.to(), None) } }); + drop(values); + let (view, doc) = current!(editor); doc.apply(&transaction, view.id); doc.append_changes_to_history(view); view.ensure_cursor_in_view(doc, scrolloff);