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.

Sign up for GitHub

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

Jump to bottom

Show surround delete and replace errors in editor #1709

Merged
merged 2 commits into from
Feb 28, 2022
Merged

Show surround delete and replace errors in editor #1709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ba105b8
Refactor surround commands to use early returns
sudormrfbin Feb 23, 2022
047f820
Show surround delete and replace errors in editor
sudormrfbin Feb 24, 2022
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
111 changes: 73 additions & 38 deletions helix-core/src/surround.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::fmt::Display;

use crate::{search, Range, Selection};
use ropey::RopeSlice;

Expand All @@ -11,6 +13,27 @@ pub const PAIRS: &[(char, char)] = &[
('（', '）'),
];

#[derive(Debug, PartialEq)]
pub enum Error {
PairNotFound,
CursorOverlap,
RangeExceedsText,
CursorOnAmbiguousPair,
}

impl Display for Error {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(match *self {
Error::PairNotFound => "Surround pair not found around all cursors",
Error::CursorOverlap => "Cursors overlap for a single surround pair range",
Error::RangeExceedsText => "Cursor range exceeds text length",
Error::CursorOnAmbiguousPair => "Cursor on ambiguous surround pair",
})
}
}

type Result<T> = std::result::Result<T, Error>;

/// Given any char in [PAIRS], return the open and closing chars. If not found in
/// [PAIRS] return (ch, ch).
///
Expand All @@ -37,31 +60,36 @@ pub fn find_nth_pairs_pos(
ch: char,
range: Range,
n: usize,
) -> Option<(usize, usize)> {
if text.len_chars() < 2 || range.to() >= text.len_chars() {
return None;
) -> Result<(usize, usize)> {
if text.len_chars() < 2 {
return Err(Error::PairNotFound);
}
if range.to() >= text.len_chars() {
return Err(Error::RangeExceedsText);
}

let (open, close) = get_pair(ch);
let pos = range.cursor(text);

if open == close {
let (open, close) = if open == close {
if Some(open) == text.get_char(pos) {
// Cursor is directly on match char. We return no match
// because there's no way to know which side of the char
// we should be searching on.
return None;
return Err(Error::CursorOnAmbiguousPair);
}
Some((
search::find_nth_prev(text, open, pos, n)?,
search::find_nth_next(text, close, pos, n)?,
))
(
search::find_nth_prev(text, open, pos, n),
search::find_nth_next(text, close, pos, n),
)
} else {
Some((
find_nth_open_pair(text, open, close, pos, n)?,
find_nth_close_pair(text, open, close, pos, n)?,
))
}
(
find_nth_open_pair(text, open, close, pos, n),
find_nth_close_pair(text, open, close, pos, n),
)
};

Option::zip(open, close).ok_or(Error::PairNotFound)
}

fn find_nth_open_pair(
Expand Down Expand Up @@ -151,17 +179,17 @@ pub fn get_surround_pos(
selection: &Selection,
ch: char,
skip: usize,
) -> Option<Vec<usize>> {
) -> Result<Vec<usize>> {
let mut change_pos = Vec::new();

for &range in selection {
let (open_pos, close_pos) = find_nth_pairs_pos(text, ch, range, skip)?;
if change_pos.contains(&open_pos) || change_pos.contains(&close_pos) {
return None;
return Err(Error::CursorOverlap);
}
change_pos.extend_from_slice(&[open_pos, close_pos]);
}
Some(change_pos)
Ok(change_pos)
}

#[cfg(test)]
Expand All @@ -175,7 +203,7 @@ mod test {
#[allow(clippy::type_complexity)]
fn check_find_nth_pair_pos(
text: &str,
cases: Vec<(usize, char, usize, Option<(usize, usize)>)>,
cases: Vec<(usize, char, usize, Result<(usize, usize)>)>,
) {
let doc = Rope::from(text);
let slice = doc.slice(..);
Expand All @@ -196,13 +224,13 @@ mod test {
"some (text) here",
vec![
// cursor on [t]ext
(6, '(', 1, Some((5, 10))),
(6, ')', 1, Some((5, 10))),
(6, '(', 1, Ok((5, 10))),
(6, ')', 1, Ok((5, 10))),
// cursor on so[m]e
(2, '(', 1, None),
(2, '(', 1, Err(Error::PairNotFound)),
// cursor on bracket itself
(5, '(', 1, Some((5, 10))),
(10, '(', 1, Some((5, 10))),
(5, '(', 1, Ok((5, 10))),
(10, '(', 1, Ok((5, 10))),
],
);
}
Expand All @@ -213,9 +241,9 @@ mod test {
"(so (many (good) text) here)",
vec![
// cursor on go[o]d
(13, '(', 1, Some((10, 15))),
(13, '(', 2, Some((4, 21))),
(13, '(', 3, Some((0, 27))),
(13, '(', 1, Ok((10, 15))),
(13, '(', 2, Ok((4, 21))),
(13, '(', 3, Ok((0, 27))),
],
);
}
Expand All @@ -226,11 +254,11 @@ mod test {
"'so 'many 'good' text' here'",
vec![
// cursor on go[o]d
(13, '\'', 1, Some((10, 15))),
(13, '\'', 2, Some((4, 21))),
(13, '\'', 3, Some((0, 27))),
(13, '\'', 1, Ok((10, 15))),
(13, '\'', 2, Ok((4, 21))),
(13, '\'', 3, Ok((0, 27))),
// cursor on the quotes
(10, '\'', 1, None),
(10, '\'', 1, Err(Error::CursorOnAmbiguousPair)),
],
)
}
Expand All @@ -241,8 +269,8 @@ mod test {
"((so)((many) good (text))(here))",
vec![
// cursor on go[o]d
(15, '(', 1, Some((5, 24))),
(15, '(', 2, Some((0, 31))),
(15, '(', 1, Ok((5, 24))),
(15, '(', 2, Ok((0, 31))),
],
)
}
Expand All @@ -253,9 +281,9 @@ mod test {
"(so [many {good} text] here)",
vec![
// cursor on go[o]d
(13, '{', 1, Some((10, 15))),
(13, '[', 1, Some((4, 21))),
(13, '(', 1, Some((0, 27))),
(13, '{', 1, Ok((10, 15))),
(13, '[', 1, Ok((4, 21))),
(13, '(', 1, Ok((0, 27))),
],
)
}
Expand Down Expand Up @@ -285,11 +313,10 @@ mod test {

let selection =
Selection::new(SmallVec::from_slice(&[Range::point(2), Range::point(9)]), 0);

// cursor on s[o]me, c[h]ars
assert_eq!(
get_surround_pos(slice, &selection, '(', 1),
None // different surround chars
Err(Error::PairNotFound) // different surround chars
);

let selection = Selection::new(
Expand All @@ -299,7 +326,15 @@ mod test {
// cursor on [x]x, newli[n]e
assert_eq!(
get_surround_pos(slice, &selection, '(', 1),
None // overlapping surround chars
Err(Error::PairNotFound) // overlapping surround chars
);

let selection =
Selection::new(SmallVec::from_slice(&[Range::point(2), Range::point(3)]), 0);
// cursor on s[o][m]e
assert_eq!(
get_surround_pos(slice, &selection, '[', 1),
Err(Error::CursorOverlap)
);
}
}
122 changes: 68 additions & 54 deletions helix-term/src/commands.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5407,76 +5407,90 @@ fn select_textobject(cx: &mut Context, objtype: textobject::TextObject) {

fn surround_add(cx: &mut Context) {
cx.on_next_key(move |cx, event| {
if let Some(ch) = event.char() {
let (view, doc) = current!(cx.editor);
let selection = doc.selection(view.id);
let (open, close) = surround::get_pair(ch);

let mut changes = Vec::with_capacity(selection.len() * 2);
for range in selection.iter() {
let mut o = Tendril::new();
o.push(open);
let mut c = Tendril::new();
c.push(close);
changes.push((range.from(), range.from(), Some(o)));
changes.push((range.to(), range.to(), Some(c)));
}

let transaction = Transaction::change(doc.text(), changes.into_iter());
doc.apply(&transaction, view.id);
let ch = match event.char() {
Some(ch) => ch,
None => return,
};
let (view, doc) = current!(cx.editor);
let selection = doc.selection(view.id);
let (open, close) = surround::get_pair(ch);

let mut changes = Vec::with_capacity(selection.len() * 2);
for range in selection.iter() {
let mut o = Tendril::new();
o.push(open);
let mut c = Tendril::new();
c.push(close);
changes.push((range.from(), range.from(), Some(o)));
changes.push((range.to(), range.to(), Some(c)));
}

let transaction = Transaction::change(doc.text(), changes.into_iter());
doc.apply(&transaction, view.id);
})
}

fn surround_replace(cx: &mut Context) {
let count = cx.count();
cx.on_next_key(move |cx, event| {
if let Some(from) = event.char() {
cx.on_next_key(move |cx, event| {
if let Some(to) = event.char() {
let (view, doc) = current!(cx.editor);
let text = doc.text().slice(..);
let selection = doc.selection(view.id);

let change_pos = match surround::get_surround_pos(text, selection, from, count)
{
Some(c) => c,
None => return,
};
let from = match event.char() {
Some(from) => from,
None => return,
};
let (view, doc) = current!(cx.editor);
let text = doc.text().slice(..);
let selection = doc.selection(view.id);

let (open, close) = surround::get_pair(to);
let transaction = Transaction::change(
doc.text(),
change_pos.iter().enumerate().map(|(i, &pos)| {
let mut t = Tendril::new();
t.push(if i % 2 == 0 { open } else { close });
(pos, pos + 1, Some(t))
}),
);
doc.apply(&transaction, view.id);
}
});
}
let change_pos = match surround::get_surround_pos(text, selection, from, count) {
Ok(c) => c,
Err(err) => {
cx.editor.set_error(err.to_string());
return;
}
};

cx.on_next_key(move |cx, event| {
let (view, doc) = current!(cx.editor);
let to = match event.char() {
Some(to) => to,
None => return,
};
let (open, close) = surround::get_pair(to);
let transaction = Transaction::change(
doc.text(),
change_pos.iter().enumerate().map(|(i, &pos)| {
let mut t = Tendril::new();
t.push(if i % 2 == 0 { open } else { close });
(pos, pos + 1, Some(t))
}),
);
doc.apply(&transaction, view.id);
});
})
}

fn surround_delete(cx: &mut Context) {
let count = cx.count();
cx.on_next_key(move |cx, event| {
if let Some(ch) = event.char() {
let (view, doc) = current!(cx.editor);
let text = doc.text().slice(..);
let selection = doc.selection(view.id);
let ch = match event.char() {
Some(ch) => ch,
None => return,
};
let (view, doc) = current!(cx.editor);
let text = doc.text().slice(..);
let selection = doc.selection(view.id);

let change_pos = match surround::get_surround_pos(text, selection, ch, count) {
Some(c) => c,
None => return,
};
let change_pos = match surround::get_surround_pos(text, selection, ch, count) {
Ok(c) => c,
Err(err) => {
cx.editor.set_error(err.to_string());
return;
}
};

let transaction =
Transaction::change(doc.text(), change_pos.into_iter().map(|p| (p, p + 1, None)));
doc.apply(&transaction, view.id);
}
let transaction =
Transaction::change(doc.text(), change_pos.into_iter().map(|p| (p, p + 1, None)));
doc.apply(&transaction, view.id);
})
}

Expand Down