Skip to content

Commit

Permalink
Consistently replace line-endings in paste/replace commands
Browse files Browse the repository at this point in the history
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<String>` 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 helix-editor#12329
  • Loading branch information
the-mikedavis authored and GladkihEgor committed Jan 4, 2025
1 parent f0dc797 commit e36a9c9
Showing 1 changed file with 35 additions and 26 deletions.
61 changes: 35 additions & 26 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4401,6 +4401,8 @@ enum Paste {
Cursor,
}

static LINE_ENDING_REGEX: Lazy<Regex> = Lazy::new(|| Regex::new(r"\r\n|\r|\n").unwrap());

fn paste_impl(
values: &[String],
doc: &mut Document,
Expand All @@ -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<Regex> = 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);
Expand Down Expand Up @@ -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<str>| {
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| {
Expand All @@ -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);
Expand Down

0 comments on commit e36a9c9

Please sign in to comment.