From 0ee20611022b5a7bec727d2159ec7c6b36e956b6 Mon Sep 17 00:00:00 2001 From: Matthias Deiml Date: Thu, 4 Aug 2022 07:44:43 +0200 Subject: [PATCH] Avoid copying fragments (#3136) * Avoid copying fragments * Add slice / slices method * Better documentation for fragment and slice methods --- helix-core/src/selection.rs | 20 +++++++++++++++++++- helix-term/src/commands.rs | 32 +++++++++++++++++++------------- helix-term/src/commands/typed.rs | 4 ++-- helix-term/src/ui/editor.rs | 4 ++-- 4 files changed, 42 insertions(+), 18 deletions(-) diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index 83bab5e30..59bd736e6 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -222,9 +222,23 @@ impl Range { // groupAt + /// Returns the text inside this range given the text of the whole buffer. + /// + /// The returned `Cow` is a reference if the range of text is inside a single + /// chunk of the rope. Otherwise a copy of the text is returned. Consider + /// using `slice` instead if you do not need a `Cow` or `String` to avoid copying. #[inline] pub fn fragment<'a, 'b: 'a>(&'a self, text: RopeSlice<'b>) -> Cow<'b, str> { - text.slice(self.from()..self.to()).into() + self.slice(text).into() + } + + /// Returns the text inside this range given the text of the whole buffer. + /// + /// The returned value is a reference to the passed slice. This method never + /// copies any contents. + #[inline] + pub fn slice<'a, 'b: 'a>(&'a self, text: RopeSlice<'b>) -> RopeSlice<'b> { + text.slice(self.from()..self.to()) } //-------------------------------- @@ -548,6 +562,10 @@ impl Selection { self.ranges.iter().map(move |range| range.fragment(text)) } + pub fn slices<'a>(&'a self, text: RopeSlice<'a>) -> impl Iterator + 'a { + self.ranges.iter().map(move |range| range.slice(text)) + } + #[inline(always)] pub fn iter(&self) -> std::slice::Iter<'_, Range> { self.ranges.iter() diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 7e757ce13..cd79cfb84 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -766,7 +766,7 @@ fn trim_selections(cx: &mut Context) { .selection(view.id) .iter() .filter_map(|range| { - if range.is_empty() || range.fragment(text).chars().all(|ch| ch.is_whitespace()) { + if range.is_empty() || range.slice(text).chars().all(|ch| ch.is_whitespace()) { return None; } let mut start = range.from(); @@ -1289,12 +1289,12 @@ fn replace(cx: &mut Context) { fn switch_case_impl(cx: &mut Context, change_fn: F) where - F: Fn(Cow) -> Tendril, + F: Fn(RopeSlice) -> Tendril, { let (view, doc) = current!(cx.editor); let selection = doc.selection(view.id); let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { - let text: Tendril = change_fn(range.fragment(doc.text().slice(..))); + let text: Tendril = change_fn(range.slice(doc.text().slice(..))); (range.from(), range.to(), Some(text)) }); @@ -1320,11 +1320,15 @@ fn switch_case(cx: &mut Context) { } fn switch_to_uppercase(cx: &mut Context) { - switch_case_impl(cx, |string| string.to_uppercase().into()); + switch_case_impl(cx, |string| { + string.chunks().map(|chunk| chunk.to_uppercase()).collect() + }); } fn switch_to_lowercase(cx: &mut Context) { - switch_case_impl(cx, |string| string.to_lowercase().into()); + switch_case_impl(cx, |string| { + string.chunks().map(|chunk| chunk.to_lowercase()).collect() + }); } pub fn scroll(cx: &mut Context, offset: usize, direction: Direction) { @@ -3903,8 +3907,8 @@ fn rotate_selection_contents(cx: &mut Context, direction: Direction) { let selection = doc.selection(view.id); let mut fragments: Vec<_> = selection - .fragments(text) - .map(|fragment| Tendril::from(fragment.as_ref())) + .slices(text) + .map(|fragment| fragment.chunks().collect()) .collect(); let group = count @@ -4510,8 +4514,8 @@ fn shell_keep_pipe(cx: &mut Context) { let text = doc.text().slice(..); for (i, range) in selection.ranges().iter().enumerate() { - let fragment = range.fragment(text); - let (_output, success) = match shell_impl(shell, input, Some(fragment.as_bytes())) { + let fragment = range.slice(text); + let (_output, success) = match shell_impl(shell, input, Some(fragment)) { Ok(result) => result, Err(err) => { cx.editor.set_error(err.to_string()); @@ -4542,7 +4546,7 @@ fn shell_keep_pipe(cx: &mut Context) { fn shell_impl( shell: &[String], cmd: &str, - input: Option<&[u8]>, + input: Option, ) -> anyhow::Result<(Tendril, bool)> { use std::io::Write; use std::process::{Command, Stdio}; @@ -4564,7 +4568,9 @@ fn shell_impl( }; if let Some(input) = input { let mut stdin = process.stdin.take().unwrap(); - stdin.write_all(input)?; + for chunk in input.chunks() { + stdin.write_all(chunk.as_bytes())?; + } } let output = process.wait_with_output()?; @@ -4593,8 +4599,8 @@ fn shell(cx: &mut compositor::Context, cmd: &str, behavior: &ShellBehavior) { let text = doc.text().slice(..); for range in selection.ranges() { - let fragment = range.fragment(text); - let (output, success) = match shell_impl(shell, cmd, pipe.then(|| fragment.as_bytes())) { + let fragment = range.slice(text); + let (output, success) = match shell_impl(shell, cmd, pipe.then(|| fragment)) { Ok(result) => result, Err(err) => { cx.editor.set_error(err.to_string()); diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index f6eedea95..dbf7cb243 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -1291,8 +1291,8 @@ fn sort_impl( let selection = doc.selection(view.id); let mut fragments: Vec<_> = selection - .fragments(text) - .map(|fragment| Tendril::from(fragment.as_ref())) + .slices(text) + .map(|fragment| fragment.chunks().collect()) .collect(); fragments.sort_by(match reverse { diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index f5598095a..d99f1a4e6 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -1029,8 +1029,8 @@ impl EditorView { if doc .selection(view.id) .primary() - .fragment(doc.text().slice(..)) - .width() + .slice(doc.text().slice(..)) + .len_chars() <= 1 { return EventResult::Ignored(None);