From d24677da626caa32ce0d699d278272a15df9a229 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:33:15 +0000 Subject: [PATCH] refactor: move overlap check into find_nth_tag fn --- helix-core/src/surround.rs | 222 +++++++++++++++++++------------------ helix-term/src/commands.rs | 58 ++-------- 2 files changed, 125 insertions(+), 155 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index 30bd004b8..aa50eda42 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -313,34 +313,109 @@ pub fn get_surround_pos( Ok(change_pos) } -/// Find position of surrounding s around every cursor. Returns None -/// if any positions overlap. Note that the positions are in a flat Vec. -/// Use get_surround_pos().chunks(2) to get matching pairs of surround positions. -pub fn get_surround_pos_tag( +/// Test whether a character would be considered a valid character if it was used for either JSX, HTML or XML tags +/// JSX tags may have `.` in them for scoping +/// HTML tags may have `-` in them if it's a custom element +/// Both JSX and HTML tags may have `_` +fn is_valid_tagname_char(ch: char) -> bool { + ch.is_alphanumeric() || ch == '_' || ch == '-' || ch == '.' +} + +/// Find the opening `` starting from `cursor_pos` and iterating until the beginning of the text. +/// Returns the Range of the tag's name (excluding the `<` and `>` characters.) +/// As well as the actual name of the tag +/// Additionally, it returns the last position where it stopped searching. +fn find_prev_tag( text: RopeSlice, - selection: &Selection, + mut cursor_pos: usize, skip: usize, -) -> Result> { - let mut change_pos = vec![]; +) -> Result<(Range, String, usize)> { + if cursor_pos == 0 || skip == 0 { + return Err(Error::RangeExceedsText); + } - for &range in selection { - let cursor_pos = range.cursor(text); + let mut chars = text.chars_at(cursor_pos); - let ((prev_tag, next_tag), tag_name) = find_nth_nearest_tag(text, cursor_pos, skip)?; + loop { + let prev_char = match chars.prev() { + Some(ch) => ch, + None => return Err(Error::PairNotFound), + }; + cursor_pos -= 1; - let tag_change = ((prev_tag, next_tag), tag_name); - change_pos.push(tag_change); - } + if prev_char == '>' { + let mut possible_tag_name = String::new(); + loop { + let current_char = match chars.prev() { + Some(ch) => ch, + None => return Err(Error::PairNotFound), + }; + cursor_pos -= 1; + if current_char == '<' { + let tag_name = possible_tag_name + .chars() + .rev() + .take_while(|&ch| is_valid_tagname_char(ch)) + .collect::(); - Ok(change_pos) + let range = Range::new(cursor_pos + 1, cursor_pos + tag_name.len() + 1); + return Ok((range, tag_name, cursor_pos)); + } + possible_tag_name.push(current_char); + } + } + } } -/// Test whether a character would be considered a valid character if it was used for either JSX, HTML or XML tags -/// JSX tags may have `.` in them for scoping -/// HTML tags may have `-` in them if it's a custom element -/// Both JSX and HTML tags may have `_` -fn is_valid_tagname_char(ch: char) -> bool { - ch.is_alphanumeric() || ch == '_' || ch == '-' || ch == '.' +/// Find the closing `` starting from `pos` and iterating the end of the text. +/// Returns the Range of the tag's name (excluding the `` characters.) +/// As well as the actual name of the tag and where it last stopped searching. +fn find_next_tag( + text: RopeSlice, + mut cursor_pos: usize, + skip: usize, +) -> Result<(Range, String, usize)> { + if cursor_pos >= text.len_chars() || skip == 0 { + return Err(Error::RangeExceedsText); + } + + let mut chars = text.chars_at(cursor_pos); + + // look forward and find something that looks like a closing tag, e.g. and extract it's name so we get "html" + loop { + let next_char = match chars.next() { + Some(ch) => ch, + None => return Err(Error::PairNotFound), + }; + cursor_pos += 1; + if next_char == '<' { + let char_after_that = match chars.next() { + Some(ch) => ch, + None => return Err(Error::PairNotFound), + }; + cursor_pos += 1; + if char_after_that == '/' { + let mut possible_tag_name = String::new(); + loop { + let current_char = match chars.next() { + Some(ch) => ch, + None => return Err(Error::PairNotFound), + }; + cursor_pos += 1; + if is_valid_tagname_char(current_char) { + possible_tag_name.push(current_char); + } else if current_char == '>' && possible_tag_name.len() != 0 { + let range = + Range::new(cursor_pos - possible_tag_name.len() - 1, cursor_pos - 1); + + return Ok((range, possible_tag_name, cursor_pos)); + } else { + break; + } + } + } + } + } } /// Get the two sorted `Range`s corresponding to nth matching tags surrounding the cursor, as well as the name of the tags. @@ -439,101 +514,36 @@ fn find_nth_nearest_tag( } } -/// Find the opening `` starting from `cursor_pos` and iterating until the beginning of the text. -/// Returns the Range of the tag's name (excluding the `<` and `>` characters.) -/// As well as the actual name of the tag -/// Additionally, it returns the last position where it stopped searching. -fn find_prev_tag( +/// Find position of surrounding s around every cursor as well as the tag's names. +/// Returns Err if any positions overlap. Note that the positions are in a flat Vec. +/// Use get_surround_pos().chunks(2) to get matching pairs of surround positions. +pub fn get_surround_pos_tag( text: RopeSlice, - mut cursor_pos: usize, + selection: &Selection, skip: usize, -) -> Result<(Range, String, usize)> { - if cursor_pos == 0 || skip == 0 { - return Err(Error::RangeExceedsText); - } - - let mut chars = text.chars_at(cursor_pos); - - loop { - let prev_char = match chars.prev() { - Some(ch) => ch, - None => return Err(Error::PairNotFound), - }; - cursor_pos -= 1; +) -> Result> { + let mut change_pos = vec![]; - if prev_char == '>' { - let mut possible_tag_name = String::new(); - loop { - let current_char = match chars.prev() { - Some(ch) => ch, - None => return Err(Error::PairNotFound), - }; - cursor_pos -= 1; - if current_char == '<' { - let tag_name = possible_tag_name - .chars() - .rev() - .take_while(|&ch| is_valid_tagname_char(ch)) - .collect::(); + for &range in selection { + let cursor_pos = range.cursor(text); - let range = Range::new(cursor_pos + 1, cursor_pos + tag_name.len() + 1); - return Ok((range, tag_name, cursor_pos)); - } - possible_tag_name.push(current_char); - } - } - } -} + let ((prev_tag, next_tag), tag_name) = find_nth_nearest_tag(text, cursor_pos, skip)?; -/// Find the closing `` starting from `pos` and iterating the end of the text. -/// Returns the Range of the tag's name (excluding the `` characters.) -/// As well as the actual name of the tag and where it last stopped searching. -fn find_next_tag( - text: RopeSlice, - mut cursor_pos: usize, - skip: usize, -) -> Result<(Range, String, usize)> { - if cursor_pos >= text.len_chars() || skip == 0 { - return Err(Error::RangeExceedsText); + change_pos.push((prev_tag, tag_name.clone())); + change_pos.push((next_tag, tag_name)); } - let mut chars = text.chars_at(cursor_pos); + // sort all ranges by the first key + change_pos.sort_by(|&(a, _), (b, _)| a.from().cmp(&b.from())); - // look forward and find something that looks like a closing tag, e.g. and extract it's name so we get "html" - loop { - let next_char = match chars.next() { - Some(ch) => ch, - None => return Err(Error::PairNotFound), - }; - cursor_pos += 1; - if next_char == '<' { - let char_after_that = match chars.next() { - Some(ch) => ch, - None => return Err(Error::PairNotFound), - }; - cursor_pos += 1; - if char_after_that == '/' { - let mut possible_tag_name = String::new(); - loop { - let current_char = match chars.next() { - Some(ch) => ch, - None => return Err(Error::PairNotFound), - }; - cursor_pos += 1; - if is_valid_tagname_char(current_char) { - possible_tag_name.push(current_char); - } else if current_char == '>' && possible_tag_name.len() != 0 { - let range = - Range::new(cursor_pos - possible_tag_name.len() - 1, cursor_pos - 1); + let has_overlaps = change_pos + .windows(2) + .any(|window| window[0].0.to() > window[1].0.from()); - return Ok((range, possible_tag_name, cursor_pos)); - } else { - log::error!("BREAKING!"); - break; - } - } - } - } + if has_overlaps { + Err(Error::CursorOverlap) + } else { + Ok(change_pos) } } diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 550b4c1c9..8d45474f3 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5742,25 +5742,8 @@ fn surround_replace(cx: &mut Context) { return; } - let mut ranges: SmallVec<[Range; 1]> = change_pos - .iter() - .flat_map(|&((opening_tag_range, closing_tag_range), _)| { - vec![opening_tag_range, closing_tag_range] - }) - .collect(); - - ranges.sort_by(|a, b| a.from().cmp(&b.from())); - - let has_overlaps = ranges - .windows(2) - .any(|window| window[0].to() > window[1].from()); - - if has_overlaps { - context - .editor - .set_error("Cursors overlap for a single surround pair range"); - return; - } + let ranges: SmallVec<[Range; 1]> = + change_pos.iter().map(|&(range, _)| range).collect(); document.set_selection( view.id, @@ -5770,15 +5753,9 @@ fn surround_replace(cx: &mut Context) { let transaction = Transaction::change( document.text(), - change_pos.iter().flat_map(|(pos, _)| { - let opening_range = pos.0; - let closing_range = pos.1; - - vec![ - (opening_range.from(), opening_range.to(), Some(tag.clone())), - (closing_range.from(), closing_range.to(), Some(tag.clone())), - ] - }), + change_pos + .iter() + .map(|(range, _)| (range.from(), range.to(), Some(tag.clone()))), ); document.apply(&transaction, view.id); @@ -5865,32 +5842,15 @@ fn surround_delete(cx: &mut Context) { return; } }; - let mut ranges: SmallVec<[Range; 1]> = change_pos - .iter() - .flat_map(|&((opening_tag_range, closing_tag_range), _)| { - vec![opening_tag_range, closing_tag_range] - }) - .collect(); - ranges.sort_by(|a, b| a.from().cmp(&b.from())); - - let has_overlaps = ranges - .windows(2) - .any(|window| window[0].to() > window[1].from()); - - if has_overlaps { - cx.editor - .set_error("Cursors overlap for a single surround pair range"); - return; - } let transaction = Transaction::change( doc.text(), - change_pos.iter().flat_map(|(pos, _)| { - let opening_range = pos.0; - let closing_range = pos.1; + change_pos.chunks_exact(2).flat_map(|chunk| { + let opening_range = chunk[0].0; + let closing_range = chunk[1].0; vec![ - // add extra numbers to account for "<", ">" and "/" characters + // account for "<", ">" and "/" characters (opening_range.from() - 1, opening_range.to() + 1, None), (closing_range.from() - 2, closing_range.to() + 1, None), ]