Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid copying fragments #3136

Merged
merged 3 commits into from
Aug 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion helix-core/src/selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

//--------------------------------
Expand Down Expand Up @@ -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<Item = RopeSlice> + 'a {
self.ranges.iter().map(move |range| range.slice(text))
}

#[inline(always)]
pub fn iter(&self) -> std::slice::Iter<'_, Range> {
self.ranges.iter()
Expand Down
32 changes: 19 additions & 13 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,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();
Expand Down Expand Up @@ -1287,12 +1287,12 @@ fn replace(cx: &mut Context) {

fn switch_case_impl<F>(cx: &mut Context, change_fn: F)
where
F: Fn(Cow<str>) -> 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))
});
Expand All @@ -1318,11 +1318,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) {
Expand Down Expand Up @@ -3788,8 +3792,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
Expand Down Expand Up @@ -4397,8 +4401,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());
Expand Down Expand Up @@ -4429,7 +4433,7 @@ fn shell_keep_pipe(cx: &mut Context) {
fn shell_impl(
shell: &[String],
cmd: &str,
input: Option<&[u8]>,
input: Option<RopeSlice>,
) -> anyhow::Result<(Tendril, bool)> {
use std::io::Write;
use std::process::{Command, Stdio};
Expand All @@ -4451,7 +4455,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()?;

Expand Down Expand Up @@ -4480,8 +4486,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());
Expand Down
4 changes: 2 additions & 2 deletions helix-term/src/commands/typed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions helix-term/src/ui/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down