From 3882bf9e9d69396b10af5082cad4583e0c17762e Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Mon, 11 Nov 2024 12:48:47 +0000 Subject: [PATCH 01/39] feat: create branch for surround implementation for x --- helix-term/src/commands.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index ee2949fa0f05..50385e687e0d 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5631,12 +5631,22 @@ fn surround_add(cx: &mut Context) { // surround_len is the number of new characters being added. let (open, close, surround_len) = match event.char() { Some(ch) => { - let (o, c) = match_brackets::get_pair(ch); let mut open = Tendril::new(); - open.push(o); let mut close = Tendril::new(); - close.push(c); - (open, close, 2) + let length = if ch == 'x' { + let (o, c) = match_brackets::get_pair(ch); + open.push(o); + close.push(c); + // Any character other than "x" will cause 2 chars to get added + 2 + } else { + let (o, c) = match_brackets::get_pair(ch); + open.push(o); + close.push(c); + // Any character other than "x" will cause 2 chars to get added + 2 + }; + (open, close, length) } None if event.code == KeyCode::Enter => ( doc.line_ending.as_str().into(), From c586e410d4dd884ae270b9864e07f4f2b05c3777 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Mon, 11 Nov 2024 13:30:12 +0000 Subject: [PATCH 02/39] refactor: add_surrounding_impl extract main logic into an fn --- helix-core/src/match_brackets.rs | 44 ++++++++++++ helix-term/src/commands.rs | 119 ++++++++++++++++++++++--------- 2 files changed, 131 insertions(+), 32 deletions(-) diff --git a/helix-core/src/match_brackets.rs b/helix-core/src/match_brackets.rs index 7520d3e4646a..03525087ede1 100644 --- a/helix-core/src/match_brackets.rs +++ b/helix-core/src/match_brackets.rs @@ -205,6 +205,50 @@ pub fn find_matching_bracket_plaintext(doc: RopeSlice, cursor_pos: usize) -> Opt None } +// fn create_rename_prompt( +// editor: &Editor, +// prefill: String, +// history_register: Option, +// language_server_id: Option, +// ) -> Box { +// let prompt = ui::Prompt::new( +// "rename-to:".into(), +// history_register, +// ui::completers::none, +// move |cx: &mut compositor::Context, input: &str, event: PromptEvent| { +// if event != PromptEvent::Validate { +// return; +// } +// let (view, doc) = current!(cx.editor); + +// let Some(language_server) = doc +// .language_servers_with_feature(LanguageServerFeature::RenameSymbol) +// .find(|ls| language_server_id.map_or(true, |id| id == ls.id())) +// else { +// cx.editor +// .set_error("No configured language server supports symbol renaming"); +// return; +// }; + +// let offset_encoding = language_server.offset_encoding(); +// let pos = doc.position(view.id, offset_encoding); +// let future = language_server +// .rename_symbol(doc.identifier(), pos, input.to_string()) +// .unwrap(); + +// match block_on(future) { +// Ok(edits) => { +// let _ = cx.editor.apply_workspace_edit(offset_encoding, &edits); +// } +// Err(err) => cx.editor.set_error(err.to_string()), +// } +// }, +// ) +// .with_line(prefill, editor); + +// Box::new(prompt) +// } + /// Returns the open and closing chars pair. If not found in /// [`BRACKETS`] returns (ch, ch). /// diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 50385e687e0d..9b84cae33add 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -36,7 +36,7 @@ use helix_core::{ textobject, unicode::width::UnicodeWidthChar, visual_offset_from_block, Deletion, LineEnding, Position, Range, Rope, RopeGraphemes, - RopeReader, RopeSlice, Selection, SmallVec, Syntax, Tendril, Transaction, + RopeReader, RopeSlice, Selection, SmallVec, SmartString, Syntax, Tendril, Transaction, }; use helix_view::{ document::{FormatterError, Mode, SCRATCH_BUFFER_NAME}, @@ -5616,6 +5616,7 @@ fn select_textobject(cx: &mut Context, objtype: textobject::TextObject) { ("a", "Argument/parameter (tree-sitter)"), ("c", "Comment (tree-sitter)"), ("T", "Test (tree-sitter)"), + ("x", "Tag (XML/HTML/JSX)"), ("e", "Data structure entry (tree-sitter)"), ("m", "Closest surrounding pair (tree-sitter)"), ("g", "Change"), @@ -5625,6 +5626,54 @@ fn select_textobject(cx: &mut Context, objtype: textobject::TextObject) { cx.editor.autoinfo = Some(Info::new(title, &help_text)); } +fn create_surround_prompt( + editor: &Editor, + prefill: String, + history_register: Option, +) -> Box { + let prompt = Prompt::new( + "tag name:".into(), + // TODO: change this to actually be history_register + Some('z'), + ui::completers::none, + move |cx: &mut compositor::Context, input: &str, event: PromptEvent| {}, + ) + .with_line("temp".into(), editor); + + Box::new(prompt) +} + +fn surround_add_impl( + doc: &mut Document, + // cx: &mut Context<'_>, + view: &mut View, + surround_len: usize, + open: Tendril, + close: Tendril, +) { + let selection = doc.selection(view.id); + let mut changes = Vec::with_capacity(selection.len() * 2); + let mut ranges = SmallVec::with_capacity(selection.len()); + let mut offs = 0; + + for range in selection.iter() { + changes.push((range.from(), range.from(), Some(open.clone()))); + changes.push((range.to(), range.to(), Some(close.clone()))); + + ranges.push( + Range::new(offs + range.from(), offs + range.to() + surround_len) + .with_direction(range.direction()), + ); + + offs += surround_len; + } + + let transaction = Transaction::change(doc.text(), changes.into_iter()) + .with_selection(Selection::new(ranges, selection.primary_index())); + doc.apply(&transaction, view.id); + // exit_select_mode(cx); +} + fn surround_add(cx: &mut Context) { cx.on_next_key(move |cx, event| { let (view, doc) = current!(cx.editor); @@ -5633,20 +5682,23 @@ fn surround_add(cx: &mut Context) { Some(ch) => { let mut open = Tendril::new(); let mut close = Tendril::new(); - let length = if ch == 'x' { - let (o, c) = match_brackets::get_pair(ch); - open.push(o); - close.push(c); - // Any character other than "x" will cause 2 chars to get added - 2 - } else { - let (o, c) = match_brackets::get_pair(ch); - open.push(o); - close.push(c); - // Any character other than "x" will cause 2 chars to get added - 2 - }; - (open, close, length) + // let length = if ch == 'x' { + // let prompt = create_surround_prompt(cx.editor, "none".into(), Some('z')); + // cx.push_layer(prompt); + + // let (o, c) = match_brackets::get_pair(ch); + // open.push(o); + // close.push(c); + // // Any character other than "x" will cause 2 chars to get added + // 2 + // } else { + let (o, c) = match_brackets::get_pair(ch); + open.push(o); + close.push(c); + // Any character other than "x" will cause 2 chars to get added + // 2 + // }; + (open, close, 2) } None if event.code == KeyCode::Enter => ( doc.line_ending.as_str().into(), @@ -5656,27 +5708,30 @@ fn surround_add(cx: &mut Context) { None => return, }; - let selection = doc.selection(view.id); - let mut changes = Vec::with_capacity(selection.len() * 2); - let mut ranges = SmallVec::with_capacity(selection.len()); - let mut offs = 0; + surround_add_impl(doc, view, surround_len, open, close); + exit_select_mode(cx); - for range in selection.iter() { - changes.push((range.from(), range.from(), Some(open.clone()))); - changes.push((range.to(), range.to(), Some(close.clone()))); + // let selection = doc.selection(view.id); + // let mut changes = Vec::with_capacity(selection.len() * 2); + // let mut ranges = SmallVec::with_capacity(selection.len()); + // let mut offs = 0; - ranges.push( - Range::new(offs + range.from(), offs + range.to() + surround_len) - .with_direction(range.direction()), - ); + // for range in selection.iter() { + // changes.push((range.from(), range.from(), Some(open.clone()))); + // changes.push((range.to(), range.to(), Some(close.clone()))); - offs += surround_len; - } + // ranges.push( + // Range::new(offs + range.from(), offs + range.to() + surround_len) + // .with_direction(range.direction()), + // ); - let transaction = Transaction::change(doc.text(), changes.into_iter()) - .with_selection(Selection::new(ranges, selection.primary_index())); - doc.apply(&transaction, view.id); - exit_select_mode(cx); + // offs += surround_len; + // } + + // let transaction = Transaction::change(doc.text(), changes.into_iter()) + // .with_selection(Selection::new(ranges, selection.primary_index())); + // doc.apply(&transaction, view.id); + // exit_select_mode(cx); }) } From d7397616361e0eef88db9e4a68106b246b98624e Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Mon, 11 Nov 2024 13:30:48 +0000 Subject: [PATCH 03/39] docs: add documentation --- helix-core/src/match_brackets.rs | 44 -------------------------------- helix-term/src/ui/prompt.rs | 1 + 2 files changed, 1 insertion(+), 44 deletions(-) diff --git a/helix-core/src/match_brackets.rs b/helix-core/src/match_brackets.rs index 03525087ede1..7520d3e4646a 100644 --- a/helix-core/src/match_brackets.rs +++ b/helix-core/src/match_brackets.rs @@ -205,50 +205,6 @@ pub fn find_matching_bracket_plaintext(doc: RopeSlice, cursor_pos: usize) -> Opt None } -// fn create_rename_prompt( -// editor: &Editor, -// prefill: String, -// history_register: Option, -// language_server_id: Option, -// ) -> Box { -// let prompt = ui::Prompt::new( -// "rename-to:".into(), -// history_register, -// ui::completers::none, -// move |cx: &mut compositor::Context, input: &str, event: PromptEvent| { -// if event != PromptEvent::Validate { -// return; -// } -// let (view, doc) = current!(cx.editor); - -// let Some(language_server) = doc -// .language_servers_with_feature(LanguageServerFeature::RenameSymbol) -// .find(|ls| language_server_id.map_or(true, |id| id == ls.id())) -// else { -// cx.editor -// .set_error("No configured language server supports symbol renaming"); -// return; -// }; - -// let offset_encoding = language_server.offset_encoding(); -// let pos = doc.position(view.id, offset_encoding); -// let future = language_server -// .rename_symbol(doc.identifier(), pos, input.to_string()) -// .unwrap(); - -// match block_on(future) { -// Ok(edits) => { -// let _ = cx.editor.apply_workspace_edit(offset_encoding, &edits); -// } -// Err(err) => cx.editor.set_error(err.to_string()), -// } -// }, -// ) -// .with_line(prefill, editor); - -// Box::new(prompt) -// } - /// Returns the open and closing chars pair. If not found in /// [`BRACKETS`] returns (ch, ch). /// diff --git a/helix-term/src/ui/prompt.rs b/helix-term/src/ui/prompt.rs index 6ba2fcb9e251..6e0f79f0e2d3 100644 --- a/helix-term/src/ui/prompt.rs +++ b/helix-term/src/ui/prompt.rs @@ -103,6 +103,7 @@ impl Prompt { self } + /// Sets the line of the prompt as if the user has typed that text manually pub fn set_line(&mut self, line: String, editor: &Editor) { let cursor = line.len(); self.line = line; From b126d9aeefa95e2a9794328b7d3f0140ac1983f8 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Mon, 11 Nov 2024 13:41:16 +0000 Subject: [PATCH 04/39] refactor: make surround_add use surround_add_impl in all of its brancehs --- helix-term/src/commands.rs | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 9b84cae33add..1ec2a2d21122 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5678,10 +5678,10 @@ fn surround_add(cx: &mut Context) { cx.on_next_key(move |cx, event| { let (view, doc) = current!(cx.editor); // surround_len is the number of new characters being added. - let (open, close, surround_len) = match event.char() { + match event.char() { Some(ch) => { - let mut open = Tendril::new(); - let mut close = Tendril::new(); + // let mut open = Tendril::new(); + // let mut close = Tendril::new(); // let length = if ch == 'x' { // let prompt = create_surround_prompt(cx.editor, "none".into(), Some('z')); // cx.push_layer(prompt); @@ -5692,25 +5692,30 @@ fn surround_add(cx: &mut Context) { // // Any character other than "x" will cause 2 chars to get added // 2 // } else { - let (o, c) = match_brackets::get_pair(ch); - open.push(o); - close.push(c); + let (open, close) = match_brackets::get_pair(ch); + // open.push(o); + // close.push(c); // Any character other than "x" will cause 2 chars to get added // 2 + // + let open = Tendril::from(open.to_string()); + let close = Tendril::from(close.to_string()); + + surround_add_impl(doc, view, 2, open, close); + exit_select_mode(cx); // }; - (open, close, 2) + // (open, close, 2) + } + None if event.code == KeyCode::Enter => { + let open = doc.line_ending.as_str().into(); + let close = doc.line_ending.as_str().into(); + let length = 2 * doc.line_ending.len_chars(); + surround_add_impl(doc, view, length, open, close); + exit_select_mode(cx); } - None if event.code == KeyCode::Enter => ( - doc.line_ending.as_str().into(), - doc.line_ending.as_str().into(), - 2 * doc.line_ending.len_chars(), - ), None => return, }; - surround_add_impl(doc, view, surround_len, open, close); - exit_select_mode(cx); - // let selection = doc.selection(view.id); // let mut changes = Vec::with_capacity(selection.len() * 2); // let mut ranges = SmallVec::with_capacity(selection.len()); From 089c2028fa06f5b1820ed7f4cfcc34f50492db8c Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Mon, 11 Nov 2024 13:51:39 +0000 Subject: [PATCH 05/39] refactor: remove unnecessary abstraction (create_surround_prompt) --- helix-term/src/commands.rs | 77 +++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 43 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 1ec2a2d21122..5153e3404116 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5626,26 +5626,26 @@ fn select_textobject(cx: &mut Context, objtype: textobject::TextObject) { cx.editor.autoinfo = Some(Info::new(title, &help_text)); } -fn create_surround_prompt( - editor: &Editor, - prefill: String, - history_register: Option, -) -> Box { - let prompt = Prompt::new( - "tag name:".into(), - // TODO: change this to actually be history_register - Some('z'), - ui::completers::none, - move |cx: &mut compositor::Context, input: &str, event: PromptEvent| {}, - ) - .with_line("temp".into(), editor); - - Box::new(prompt) -} +// fn create_surround_prompt( +// editor: &Editor, +// prefill: String, +// history_register: Option, +// callback_fn: impl FnMut(&mut compositor::Context<'_>, &str, PromptEvent) + 'static, +// ) -> Box { +// let prompt = Prompt::new( +// "tag name:".into(), +// // TODO: change this to actually be history_register +// Some('z'), +// ui::completers::none, +// callback_fn, +// ) +// .with_line("temp".into(), editor); + +// Box::new(prompt) +// } fn surround_add_impl( doc: &mut Document, - // cx: &mut Context<'_>, view: &mut View, surround_len: usize, open: Tendril, @@ -5671,7 +5671,6 @@ fn surround_add_impl( let transaction = Transaction::change(doc.text(), changes.into_iter()) .with_selection(Selection::new(ranges, selection.primary_index())); doc.apply(&transaction, view.id); - // exit_select_mode(cx); } fn surround_add(cx: &mut Context) { @@ -5680,31 +5679,23 @@ fn surround_add(cx: &mut Context) { // surround_len is the number of new characters being added. match event.char() { Some(ch) => { - // let mut open = Tendril::new(); - // let mut close = Tendril::new(); - // let length = if ch == 'x' { - // let prompt = create_surround_prompt(cx.editor, "none".into(), Some('z')); - // cx.push_layer(prompt); - - // let (o, c) = match_brackets::get_pair(ch); - // open.push(o); - // close.push(c); - // // Any character other than "x" will cause 2 chars to get added - // 2 - // } else { - let (open, close) = match_brackets::get_pair(ch); - // open.push(o); - // close.push(c); - // Any character other than "x" will cause 2 chars to get added - // 2 - // - let open = Tendril::from(open.to_string()); - let close = Tendril::from(close.to_string()); - - surround_add_impl(doc, view, 2, open, close); - exit_select_mode(cx); - // }; - // (open, close, 2) + if ch == 'x' { + let prompt = Prompt::new( + "tag name:".into(), + Some('z'), + ui::completers::none, + move |cx: &mut compositor::Context, input: &str, event: PromptEvent| {}, + ) + .with_line("temp".into(), cx.editor); + cx.push_layer(prompt); + } else { + let (open, close) = match_brackets::get_pair(ch); + let open = Tendril::from(open.to_string()); + let close = Tendril::from(close.to_string()); + + surround_add_impl(doc, view, 2, open, close); + exit_select_mode(cx); + }; } None if event.code == KeyCode::Enter => { let open = doc.line_ending.as_str().into(); From c620860bd63df019287fc07819ea614bca6331e9 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Mon, 11 Nov 2024 15:08:26 +0000 Subject: [PATCH 06/39] feat: complete implementation for adding surrounding tag --- helix-term/src/commands.rs | 47 ++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 5153e3404116..1aa1b14c3b0d 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -36,7 +36,7 @@ use helix_core::{ textobject, unicode::width::UnicodeWidthChar, visual_offset_from_block, Deletion, LineEnding, Position, Range, Rope, RopeGraphemes, - RopeReader, RopeSlice, Selection, SmallVec, SmartString, Syntax, Tendril, Transaction, + RopeReader, RopeSlice, Selection, SmallVec, Syntax, Tendril, Transaction, }; use helix_view::{ document::{FormatterError, Mode, SCRATCH_BUFFER_NAME}, @@ -5684,10 +5684,25 @@ fn surround_add(cx: &mut Context) { "tag name:".into(), Some('z'), ui::completers::none, - move |cx: &mut compositor::Context, input: &str, event: PromptEvent| {}, - ) - .with_line("temp".into(), cx.editor); - cx.push_layer(prompt); + move |context: &mut compositor::Context, + input: &str, + event: PromptEvent| { + let (view, doc) = current!(context.editor); + + if event != PromptEvent::Validate { + return; + } + + let open = Tendril::from(format!("<{}>", input.to_string())); + let close = Tendril::from(format!("", input.to_string())); + let length = open.len() + close.len(); + + surround_add_impl(doc, view, length, open, close); + context.editor.mode = Mode::Normal; + }, + ); + // .with_line(String::from("temp"), cx.editor); + cx.push_layer(Box::new(prompt)); } else { let (open, close) = match_brackets::get_pair(ch); let open = Tendril::from(open.to_string()); @@ -5706,28 +5721,6 @@ fn surround_add(cx: &mut Context) { } None => return, }; - - // let selection = doc.selection(view.id); - // let mut changes = Vec::with_capacity(selection.len() * 2); - // let mut ranges = SmallVec::with_capacity(selection.len()); - // let mut offs = 0; - - // for range in selection.iter() { - // changes.push((range.from(), range.from(), Some(open.clone()))); - // changes.push((range.to(), range.to(), Some(close.clone()))); - - // ranges.push( - // Range::new(offs + range.from(), offs + range.to() + surround_len) - // .with_direction(range.direction()), - // ); - - // offs += surround_len; - // } - - // let transaction = Transaction::change(doc.text(), changes.into_iter()) - // .with_selection(Selection::new(ranges, selection.primary_index())); - // doc.apply(&transaction, view.id); - // exit_select_mode(cx); }) } From 59df1fedf5b597b4d106dd0a5159af3de6b4d4ca Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Mon, 11 Nov 2024 15:10:01 +0000 Subject: [PATCH 07/39] fix: remove misleading textobject "x" mention --- helix-term/src/commands.rs | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 1aa1b14c3b0d..1c0011c60814 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5616,7 +5616,6 @@ fn select_textobject(cx: &mut Context, objtype: textobject::TextObject) { ("a", "Argument/parameter (tree-sitter)"), ("c", "Comment (tree-sitter)"), ("T", "Test (tree-sitter)"), - ("x", "Tag (XML/HTML/JSX)"), ("e", "Data structure entry (tree-sitter)"), ("m", "Closest surrounding pair (tree-sitter)"), ("g", "Change"), @@ -5626,24 +5625,6 @@ fn select_textobject(cx: &mut Context, objtype: textobject::TextObject) { cx.editor.autoinfo = Some(Info::new(title, &help_text)); } -// fn create_surround_prompt( -// editor: &Editor, -// prefill: String, -// history_register: Option, -// callback_fn: impl FnMut(&mut compositor::Context<'_>, &str, PromptEvent) + 'static, -// ) -> Box { -// let prompt = Prompt::new( -// "tag name:".into(), -// // TODO: change this to actually be history_register -// Some('z'), -// ui::completers::none, -// callback_fn, -// ) -// .with_line("temp".into(), editor); - -// Box::new(prompt) -// } - fn surround_add_impl( doc: &mut Document, view: &mut View, @@ -5701,7 +5682,6 @@ fn surround_add(cx: &mut Context) { context.editor.mode = Mode::Normal; }, ); - // .with_line(String::from("temp"), cx.editor); cx.push_layer(Box::new(prompt)); } else { let (open, close) = match_brackets::get_pair(ch); From f8cace7957224913e32271ad42cfaef1a1410701 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Mon, 11 Nov 2024 17:17:57 +0000 Subject: [PATCH 08/39] docs: add comments describing algorithm for replacing character --- helix-term/src/commands.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 1c0011c60814..de8efeea95c9 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5657,7 +5657,6 @@ fn surround_add_impl( fn surround_add(cx: &mut Context) { cx.on_next_key(move |cx, event| { let (view, doc) = current!(cx.editor); - // surround_len is the number of new characters being added. match event.char() { Some(ch) => { if ch == 'x' { @@ -5705,7 +5704,7 @@ fn surround_add(cx: &mut Context) { } fn surround_replace(cx: &mut Context) { - let count = cx.count(); + let layer = cx.count(); cx.on_next_key(move |cx, event| { let surround_ch = match event.char() { Some('m') => None, // m selects the closest surround pair @@ -5716,8 +5715,11 @@ fn surround_replace(cx: &mut Context) { let text = doc.text().slice(..); let selection = doc.selection(view.id); + // the first character represents the index of the first change + // the second character represents the index of the second change let change_pos = - match surround::get_surround_pos(doc.syntax(), text, selection, surround_ch, count) { + // also interested in changing this + match surround::get_surround_pos(doc.syntax(), text, selection, surround_ch, layer) { Ok(c) => c, Err(err) => { cx.editor.set_error(err.to_string()); @@ -5738,6 +5740,7 @@ fn surround_replace(cx: &mut Context) { Some(to) => to, None => return doc.set_selection(view.id, selection), }; + // we are interested in changing this specifically let (open, close) = match_brackets::get_pair(to); // the changeset has to be sorted to allow nested surrounds @@ -5753,6 +5756,7 @@ fn surround_replace(cx: &mut Context) { sorted_pos.iter().map(|&pos| { let mut t = Tendril::new(); t.push(pos.1); + log::error!("{:?}, {:?}", pos.0, Some(&t)); (pos.0, pos.0 + 1, Some(t)) }), ); From 3cb8519ffd30fcb76a5c5fe7550f63f554cf8b31 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Mon, 11 Nov 2024 17:18:16 +0000 Subject: [PATCH 09/39] docs: add comments describing algorithm --- helix-term/src/commands.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index de8efeea95c9..1f6168644919 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5754,6 +5754,8 @@ fn surround_replace(cx: &mut Context) { let transaction = Transaction::change( doc.text(), sorted_pos.iter().map(|&pos| { + // "pos" is the idx of the character we are replacing + // "t" is the replacement character let mut t = Tendril::new(); t.push(pos.1); log::error!("{:?}, {:?}", pos.0, Some(&t)); From abe32105f39f294f94cb344f638724c47a976278 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Tue, 12 Nov 2024 14:25:56 +0000 Subject: [PATCH 10/39] feat: add basic hard-coded implementation for replace tag --- helix-core/src/search.rs | 68 +++++++++++++++++++++++ helix-core/src/surround.rs | 110 ++++++++++++++++++++++++++++++++++++- helix-term/src/commands.rs | 91 ++++++++++++++++-------------- 3 files changed, 224 insertions(+), 45 deletions(-) diff --git a/helix-core/src/search.rs b/helix-core/src/search.rs index 81cb412939df..a59cf252f893 100644 --- a/helix-core/src/search.rs +++ b/helix-core/src/search.rs @@ -17,6 +17,40 @@ impl bool> CharMatcher for F { } } +pub fn find_nth_next_tag( + text: RopeSlice, + tag: &str, + mut pos: usize, + n: usize, +) -> Option> { + if pos >= text.len_chars() || n == 0 { + return None; + } + + let mut chars = text.chars_at(pos); + + let tag = format!(""); + let len = tag.len(); + + for _ in 0..n { + loop { + let c = chars.next()?; + let cloned_chars = chars.clone(); + let stri: String = cloned_chars.take(len).collect(); + + pos += 1; + + if stri == tag { + break; + } + } + } + + let range: Vec = (pos - 1..pos + len - 1).collect(); + + Some(range) +} + pub fn find_nth_next( text: RopeSlice, char_matcher: M, @@ -65,3 +99,37 @@ pub fn find_nth_prev(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Opt Some(pos) } + +pub fn find_nth_prev_tag( + text: RopeSlice, + tag: &str, + mut pos: usize, + n: usize, +) -> Option> { + if pos == 0 || n == 0 { + return None; + } + + let mut chars = text.chars_at(pos).reversed(); + + let tag = format!("<{tag}>"); + let len = tag.len(); + + for _ in 0..n { + loop { + let c = chars.next()?; + let cloned_chars = chars.clone(); + let stri: String = cloned_chars.take(len).collect(); + + pos -= 1; + + if stri == tag { + break; + } + } + } + + let range: Vec = (pos..pos + len).collect(); + + Some(range) +} diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index e45346c92957..e3d2d22531c9 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -308,11 +308,62 @@ pub fn get_surround_pos( return Err(Error::CursorOverlap); } // ensure the positions are always paired in the forward direction + // e.g. [41, 214] change_pos.extend_from_slice(&[open_pos.min(close_pos), close_pos.max(open_pos)]); } Ok(change_pos) } +fn find_nth_close_tag( + text: RopeSlice, + tag: &str, + mut pos: usize, + n: usize, +) -> Result<(Vec, Vec)> { + // if text.len_chars() < 2 { + // return Err(Error::PairNotFound); + // } + // if range.to() >= text.len_chars() { + // return Err(Error::RangeExceedsText); + // } + // + Ok((vec![4, 7], vec![10, 13])) +} + +/// like get_surround_pos, but for Tags +pub fn get_surround_pos_tag( + text: RopeSlice, + selection: &Selection, + tag: &str, + skip: usize, +) -> Result> { + let mut change_pos = Vec::new(); + + for &range in selection { + let (start1, end1, start2, end2) = { + let pos = range.cursor(text); + let range_raw = find_nth_close_tag(text, tag, pos, skip).unwrap(); + + let first = range_raw.0; + let second = range_raw.1; + let first_1 = first.first().unwrap(); + let first_2 = second.last().unwrap(); + let second_1 = first.first().unwrap(); + let second_2 = second.last().unwrap(); + let range_first = Range::new(*first_1, *first_2); + let range_last = Range::new(*second_1, *second_2); + ( + range_first.from(), + range_first.to(), + range_last.from(), + range_last.to(), + ) + }; + change_pos.extend_from_slice(&[start1, end1, start2, end2]); + } + Ok(change_pos) +} + #[cfg(test)] mod test { use super::*; @@ -336,6 +387,21 @@ mod test { ); } + #[test] + fn test_get_surround_pos_tag_simple() { + #[rustfmt::skip] + let (doc, selection, expectations) = + rope_with_selections_and_expectations_tags( + "hello lol", + " ___ ^ ___ _______ ^ _______ " + ); + + assert_eq!( + get_tag_surround_pos(None, doc.slice(..), &selection, 1).unwrap(), + expectations + ); + } + #[test] fn test_get_surround_pos_bail_different_surround_chars() { #[rustfmt::skip] @@ -445,9 +511,9 @@ mod test { ) } - // Create a Rope and a matching Selection using a specification language. - // ^ is a single-point selection. - // _ is an expected index. These are returned as a Vec for use in assertions. + /// Create a Rope and a matching Selection using a specification language. + /// ^ is a single-point selection. + /// _ is an expected index. These are returned as a Vec for use in assertions. fn rope_with_selections_and_expectations( text: &str, spec: &str, @@ -467,4 +533,42 @@ mod test { (rope, Selection::new(selections, 0), expectations) } + + fn rope_with_selections_and_expectations_tags( + text: &str, + spec: &str, + ) -> (Rope, Selection, Vec>) { + if text.len() != spec.len() { + panic!("specification must match text length -- are newlines aligned?"); + } + + let rope = Rope::from(text); + + let selections: SmallVec<[Range; 1]> = spec + .match_indices('^') + .map(|(i, _)| Range::point(i)) + .collect(); + + let expectations: Vec> = spec + .char_indices() + .filter(|&(_, c)| c == '_') + .map(|(i, _)| i) + .fold(Vec::new(), |mut groups, idx| { + if let Some(last_group) = groups.last_mut() { + if last_group + .last() + .map_or(false, |&last_idx| last_idx + 1 == idx) + { + last_group.push(idx); + } else { + groups.push(vec![idx]); + } + } else { + groups.push(vec![idx]); + } + groups + }); + + (rope, Selection::new(selections, 0), expectations) + } } diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 1f6168644919..b2e47bae2aca 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5715,57 +5715,64 @@ fn surround_replace(cx: &mut Context) { let text = doc.text().slice(..); let selection = doc.selection(view.id); - // the first character represents the index of the first change - // the second character represents the index of the second change - let change_pos = - // also interested in changing this - match surround::get_surround_pos(doc.syntax(), text, selection, surround_ch, layer) { + if false { + let change_pos = match surround::get_surround_pos_tag(text, selection, "lol", layer) { Ok(c) => c, Err(err) => { cx.editor.set_error(err.to_string()); return; } }; + // TODO: add back the logic for other surround changes + } else { + // TODO: obtain these values from a function + let change_pos: Vec<(usize, usize)> = vec![(4, 10), (13, 20), (24, 30), (33, 40)]; - let selection = selection.clone(); - let ranges: SmallVec<[Range; 1]> = change_pos.iter().map(|&p| Range::point(p)).collect(); - doc.set_selection( - view.id, - Selection::new(ranges, selection.primary_index() * 2), - ); + let selection = selection.clone(); + let ranges: SmallVec<[Range; 1]> = + change_pos.iter().map(|&p| Range::new(p.0, p.1)).collect(); - cx.on_next_key(move |cx, event| { - let (view, doc) = current!(cx.editor); - let to = match event.char() { - Some(to) => to, - None => return doc.set_selection(view.id, selection), - }; - // we are interested in changing this specifically - let (open, close) = match_brackets::get_pair(to); - - // the changeset has to be sorted to allow nested surrounds - let mut sorted_pos: Vec<(usize, char)> = Vec::new(); - for p in change_pos.chunks(2) { - sorted_pos.push((p[0], open)); - sorted_pos.push((p[1], close)); - } - sorted_pos.sort_unstable(); - - let transaction = Transaction::change( - doc.text(), - sorted_pos.iter().map(|&pos| { - // "pos" is the idx of the character we are replacing - // "t" is the replacement character - let mut t = Tendril::new(); - t.push(pos.1); - log::error!("{:?}, {:?}", pos.0, Some(&t)); - (pos.0, pos.0 + 1, Some(t)) - }), + doc.set_selection( + view.id, + Selection::new(ranges, selection.primary_index() * 2), ); - doc.set_selection(view.id, selection); - doc.apply(&transaction, view.id); - exit_select_mode(cx); - }); + + cx.on_next_key(move |cx, event| { + let (view, doc) = current!(cx.editor); + + // TODO: obtain this value from the input + let open = ""; + let close = ""; + + // the changeset has to be sorted to allow nested surrounds + let mut sorted_pos: Vec<((usize, usize), &str)> = Vec::new(); + + // taking chunks of two at once to replace with + for p in change_pos.chunks(2) { + let line_opening = p[0]; + let line_closing = p[1]; + sorted_pos.push((line_opening, open)); + sorted_pos.push((line_closing, close)); + } + sorted_pos.sort_unstable(); + + let transaction = Transaction::change( + doc.text(), + sorted_pos.iter().map(|&(pos, change_tag)| { + let mut tag = Tendril::new(); + tag.push_str(change_tag); + + // replace from pos.0 to pos.1 with t + (pos.0, pos.1, Some(tag)) + }), + ); + + // we don't need to touch this + doc.set_selection(view.id, selection); + doc.apply(&transaction, view.id); + exit_select_mode(cx); + }); + } }) } From 12d9fcee0e84657f3c0788dfc7bc1881249a4e95 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Tue, 12 Nov 2024 16:20:38 +0000 Subject: [PATCH 11/39] feat: add function to get next tag's position and test for it --- helix-core/src/surround.rs | 147 ++++++++++++++++++++++++------------- helix-term/src/commands.rs | 11 ++- 2 files changed, 104 insertions(+), 54 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index e3d2d22531c9..7c3394e7e00e 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -154,6 +154,29 @@ fn find_nth_closest_pairs_plain( Err(Error::PairNotFound) } +/// Find the position of surrounding tags around cursor +/// `n` will skip n - 1 tags (e.g. n=2 will discard only the first tag found) +/// and keep looking +// pub fn find_nth_tag_pos( +// text: RopeSlice, +// range: Range, +// n: usize, +// ) -> Result<((usize, usize), (usize, usize))> { +// // 7 is the minimum possible length for a tag, e.g. . +// if text.len_chars() < 7 { +// return Err(Error::PairNotFound); +// } + +// if range.to() >= text.len_chars() { +// return Err(Error::RangeExceedsText); +// } + +// let cursor_position = range.cursor(text); + +// let open = (); +// let close = (); +// } + /// Find the position of surround pairs of `ch` which can be either a closing /// or opening pair. `n` will skip n - 1 pairs (eg. n=2 will discard (only) /// the first pair found and keep looking) @@ -314,54 +337,82 @@ pub fn get_surround_pos( Ok(change_pos) } -fn find_nth_close_tag( - text: RopeSlice, - tag: &str, - mut pos: usize, - n: usize, -) -> Result<(Vec, Vec)> { - // if text.len_chars() < 2 { - // return Err(Error::PairNotFound); - // } - // if range.to() >= text.len_chars() { - // return Err(Error::RangeExceedsText); - // } - // - Ok((vec![4, 7], vec![10, 13])) -} - /// like get_surround_pos, but for Tags pub fn get_surround_pos_tag( text: RopeSlice, selection: &Selection, - tag: &str, skip: usize, -) -> Result> { - let mut change_pos = Vec::new(); +) -> Result> { + // // let change_pos: Vec<(usize, usize)> = vec![(4, 10), (13, 20), (24, 30), (33, 40)]; + // let mut change_pos = Vec::new(); + + // // for &range in selection { + // // let (start1, end1, start2, end2) = { + // // let cursor_pos = range.cursor(text); + // // // let range_raw = find_nth_close_tag(text, cursor_pos, skip).unwrap(); + + // // let first = range_raw.0; + // // let second = range_raw.1; + // // let first_1 = first.first().unwrap(); + // // let first_2 = second.last().unwrap(); + // // let second_1 = first.first().unwrap(); + // // let second_2 = second.last().unwrap(); + // // let range_first = Range::new(*first_1, *first_2); + // // let range_last = Range::new(*second_1, *second_2); + // // ( + // // range_first.from(), + // // range_first.to(), + // // range_last.from(), + // // range_last.to(), + // // ) + // // }; + // // change_pos.push((start1, end1)); + // // change_pos.push((start2, end2)); + // // } + Ok(vec![(14, 24)]) +} - for &range in selection { - let (start1, end1, start2, end2) = { - let pos = range.cursor(text); - let range_raw = find_nth_close_tag(text, tag, pos, skip).unwrap(); - - let first = range_raw.0; - let second = range_raw.1; - let first_1 = first.first().unwrap(); - let first_2 = second.last().unwrap(); - let second_1 = first.first().unwrap(); - let second_2 = second.last().unwrap(); - let range_first = Range::new(*first_1, *first_2); - let range_last = Range::new(*second_1, *second_2); - ( - range_first.from(), - range_first.to(), - range_last.from(), - range_last.to(), - ) - }; - change_pos.extend_from_slice(&[start1, end1, start2, end2]); +pub fn is_valid_tagname_char(ch: char) -> bool { + ch.is_alphanumeric() || ch == '_' || ch == '-' || ch == '.' +} + +pub fn find_next_tag(text: RopeSlice, mut pos: usize, n: usize) -> Option<(usize, usize)> { + if pos >= text.len_chars() || n == 0 { + return None; } - Ok(change_pos) + + let mut chars = text.chars_at(pos); + let mut possible_tag = String::new(); + + 'outer: loop { + // look forward, try to find something that looks like a closing tag e.g. + // extract the name so e.g. "html". + // set current_tag_name to this "html" string, then break. + let next_char = chars.next()?; + pos += 1; + if next_char == '<' { + let char_after_that = chars.next()?; + pos += 1; + if char_after_that == '/' { + let mut possible_tag_name = String::new(); + loop { + let current_char = chars.next()?; + pos += 1; + if is_valid_tagname_char(current_char) { + possible_tag_name.push(current_char); + } else if current_char == '>' && possible_tag_name.len() != 0 { + // victory! + possible_tag.push_str(&possible_tag_name[..]); + break 'outer; + } else { + break; + } + } + } + } + } + + return Some((pos - possible_tag.len() - 1, pos - 2)); } #[cfg(test)] @@ -388,18 +439,10 @@ mod test { } #[test] - fn test_get_surround_pos_tag_simple() { - #[rustfmt::skip] - let (doc, selection, expectations) = - rope_with_selections_and_expectations_tags( - "hello lol", - " ___ ^ ___ _______ ^ _______ " - ); + fn test_find_next_tag() { + let doc = Rope::from("hello"); - assert_eq!( - get_tag_surround_pos(None, doc.slice(..), &selection, 1).unwrap(), - expectations - ); + assert_eq!(find_next_tag(doc.slice(..), 7, 1).unwrap(), (14, 24)); } #[test] diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index b2e47bae2aca..7fcdfaeeeb1e 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5716,7 +5716,7 @@ fn surround_replace(cx: &mut Context) { let selection = doc.selection(view.id); if false { - let change_pos = match surround::get_surround_pos_tag(text, selection, "lol", layer) { + let change_pos = match surround::get_surround_pos_tag(text, selection, layer) { Ok(c) => c, Err(err) => { cx.editor.set_error(err.to_string()); @@ -5726,7 +5726,14 @@ fn surround_replace(cx: &mut Context) { // TODO: add back the logic for other surround changes } else { // TODO: obtain these values from a function - let change_pos: Vec<(usize, usize)> = vec![(4, 10), (13, 20), (24, 30), (33, 40)]; + // let change_pos: Vec<(usize, usize)> = vec![(4, 10), (13, 20), (24, 30), (33, 40)]; + let change_pos = match surround::get_surround_pos_tag(text, selection, layer) { + Ok(c) => c, + Err(err) => { + cx.editor.set_error(err.to_string()); + return; + } + }; let selection = selection.clone(); let ranges: SmallVec<[Range; 1]> = From c45f230f2d13dfe6a05705840b9ec8b96d27941a Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Tue, 12 Nov 2024 16:45:16 +0000 Subject: [PATCH 12/39] test: add some extra tests --- helix-core/src/search.rs | 34 -------------------- helix-core/src/surround.rs | 64 ++++++++++++++++++++++++++++++++------ 2 files changed, 54 insertions(+), 44 deletions(-) diff --git a/helix-core/src/search.rs b/helix-core/src/search.rs index a59cf252f893..18bbbc01a918 100644 --- a/helix-core/src/search.rs +++ b/helix-core/src/search.rs @@ -99,37 +99,3 @@ pub fn find_nth_prev(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Opt Some(pos) } - -pub fn find_nth_prev_tag( - text: RopeSlice, - tag: &str, - mut pos: usize, - n: usize, -) -> Option> { - if pos == 0 || n == 0 { - return None; - } - - let mut chars = text.chars_at(pos).reversed(); - - let tag = format!("<{tag}>"); - let len = tag.len(); - - for _ in 0..n { - loop { - let c = chars.next()?; - let cloned_chars = chars.clone(); - let stri: String = cloned_chars.take(len).collect(); - - pos -= 1; - - if stri == tag { - break; - } - } - } - - let range: Vec = (pos..pos + len).collect(); - - Some(range) -} diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index 7c3394e7e00e..b042b16ea578 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -376,7 +376,17 @@ pub fn is_valid_tagname_char(ch: char) -> bool { ch.is_alphanumeric() || ch == '_' || ch == '-' || ch == '.' } -pub fn find_next_tag(text: RopeSlice, mut pos: usize, n: usize) -> Option<(usize, usize)> { +// pub fn find_surrounding_tag +// Look in the forward direction and store the tag that we find e.g. "div" +// Look in the backward direction and store the tag that we find e.g. "span" +// If they are the same, success. If they're different, continue looking forward until we find the same +// Alternate between looking forward and backward + +pub fn find_next_tag( + text: RopeSlice, + mut pos: usize, + n: usize, +) -> Option<((usize, usize), String)> { if pos >= text.len_chars() || n == 0 { return None; } @@ -401,7 +411,6 @@ pub fn find_next_tag(text: RopeSlice, mut pos: usize, n: usize) -> Option<(usize if is_valid_tagname_char(current_char) { possible_tag_name.push(current_char); } else if current_char == '>' && possible_tag_name.len() != 0 { - // victory! possible_tag.push_str(&possible_tag_name[..]); break 'outer; } else { @@ -412,7 +421,7 @@ pub fn find_next_tag(text: RopeSlice, mut pos: usize, n: usize) -> Option<(usize } } - return Some((pos - possible_tag.len() - 1, pos - 2)); + return Some(((pos - possible_tag.len() - 1, pos - 2), possible_tag)); } #[cfg(test)] @@ -442,7 +451,40 @@ mod test { fn test_find_next_tag() { let doc = Rope::from("hello"); - assert_eq!(find_next_tag(doc.slice(..), 7, 1).unwrap(), (14, 24)); + assert_eq!( + find_next_tag(doc.slice(..), 7, 1).unwrap(), + ((12, 14), String::from("tag")) + ); + } + + #[test] + fn test_find_next_tag_broken() { + let doc = Rope::from("hello"); + + assert_eq!( + find_next_tag(doc.slice(..), 7, 1).unwrap(), + ((18, 28), String::from("Hello.World")) + ); + } + + #[test] + fn test_find_prev_tag() { + let doc = Rope::from("hello"); + + assert_eq!( + find_next_tag(doc.slice(..), 7, 1).unwrap(), + ((1, 3), String::from("tag")) + ); + } + + #[test] + fn test_find_prev_tag_broken() { + let doc = Rope::from(" "); + + assert_eq!( + find_next_tag(doc.slice(..), 32, 1).unwrap(), + ((1, 11), String::from("Hello.World")) + ); } #[test] @@ -580,17 +622,19 @@ mod test { fn rope_with_selections_and_expectations_tags( text: &str, spec: &str, - ) -> (Rope, Selection, Vec>) { + ) -> (Rope, usize, Vec>) { if text.len() != spec.len() { panic!("specification must match text length -- are newlines aligned?"); } let rope = Rope::from(text); - let selections: SmallVec<[Range; 1]> = spec - .match_indices('^') - .map(|(i, _)| Range::point(i)) - .collect(); + // let selections: SmallVec<[Range; 1]> = spec + // .match_indices('^') + // .map(|(i, _)| Range::point(i)) + // .collect(); + + let cursor_idx = spec.find("^").unwrap(); let expectations: Vec> = spec .char_indices() @@ -612,6 +656,6 @@ mod test { groups }); - (rope, Selection::new(selections, 0), expectations) + (rope, cursor_idx, expectations) } } From 5f4c4623c2dfda90de98ba295512be80519a3d46 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Tue, 12 Nov 2024 17:37:25 +0000 Subject: [PATCH 13/39] feat: prev_tag finder passes tests --- helix-core/src/surround.rs | 43 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index b042b16ea578..d52766d46158 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -382,6 +382,45 @@ pub fn is_valid_tagname_char(ch: char) -> bool { // If they are the same, success. If they're different, continue looking forward until we find the same // Alternate between looking forward and backward +pub fn find_prev_tag( + text: RopeSlice, + mut pos: usize, + n: usize, +) -> Option<((usize, usize), String)> { + if pos == 0 || n == 0 { + return None; + } + + let mut chars = text.chars_at(pos); + let mut possible_tag = String::new(); + + 'outer: loop { + let prev_char = chars.prev()?; + pos -= 1; + + if prev_char == '>' { + let mut possible_tag_name = String::new(); + loop { + let current_char = chars.prev()?; + pos -= 1; + if current_char == '<' { + possible_tag.push_str( + &possible_tag_name + .chars() + .rev() + .take_while(|&ch| is_valid_tagname_char(ch)) + .collect::()[..], + ); + break 'outer; + } + possible_tag_name.push(current_char); + } + } + } + + Some(((pos + 1, pos + possible_tag.len()), possible_tag)) +} + pub fn find_next_tag( text: RopeSlice, mut pos: usize, @@ -472,7 +511,7 @@ mod test { let doc = Rope::from("hello"); assert_eq!( - find_next_tag(doc.slice(..), 7, 1).unwrap(), + find_prev_tag(doc.slice(..), 7, 1).unwrap(), ((1, 3), String::from("tag")) ); } @@ -482,7 +521,7 @@ mod test { let doc = Rope::from(" "); assert_eq!( - find_next_tag(doc.slice(..), 32, 1).unwrap(), + find_prev_tag(doc.slice(..), 32, 1).unwrap(), ((1, 11), String::from("Hello.World")) ); } From 93fc1af2309ad0b9debe8080fe2f5726e1c08789 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Tue, 12 Nov 2024 17:51:27 +0000 Subject: [PATCH 14/39] feat: create skeleton for the replacement implementation --- helix-core/src/surround.rs | 44 +++++++++++--------------------------- 1 file changed, 12 insertions(+), 32 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index d52766d46158..982e6cb5c30a 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -343,44 +343,24 @@ pub fn get_surround_pos_tag( selection: &Selection, skip: usize, ) -> Result> { - // // let change_pos: Vec<(usize, usize)> = vec![(4, 10), (13, 20), (24, 30), (33, 40)]; - // let mut change_pos = Vec::new(); - - // // for &range in selection { - // // let (start1, end1, start2, end2) = { - // // let cursor_pos = range.cursor(text); - // // // let range_raw = find_nth_close_tag(text, cursor_pos, skip).unwrap(); - - // // let first = range_raw.0; - // // let second = range_raw.1; - // // let first_1 = first.first().unwrap(); - // // let first_2 = second.last().unwrap(); - // // let second_1 = first.first().unwrap(); - // // let second_2 = second.last().unwrap(); - // // let range_first = Range::new(*first_1, *first_2); - // // let range_last = Range::new(*second_1, *second_2); - // // ( - // // range_first.from(), - // // range_first.to(), - // // range_last.from(), - // // range_last.to(), - // // ) - // // }; - // // change_pos.push((start1, end1)); - // // change_pos.push((start2, end2)); - // // } - Ok(vec![(14, 24)]) + let mut change_pos = Vec::new(); + + for &range in selection { + let cursor_pos = range.cursor(text); + let (next_tag, prev_tag) = find_nearest_tag(text, cursor_pos, 1)?; + change_pos.push((prev_tag.from(), prev_tag.to())); + change_pos.push((next_tag.from(), next_tag.to())); + } + + Ok(change_pos) } pub fn is_valid_tagname_char(ch: char) -> bool { ch.is_alphanumeric() || ch == '_' || ch == '-' || ch == '.' } -// pub fn find_surrounding_tag -// Look in the forward direction and store the tag that we find e.g. "div" -// Look in the backward direction and store the tag that we find e.g. "span" -// If they are the same, success. If they're different, continue looking forward until we find the same -// Alternate between looking forward and backward +pub fn find_nearest_tag(text: RopeSlice, cursor_pos: usize, skip: usize) -> Result<(Range, Range)> { +} pub fn find_prev_tag( text: RopeSlice, From cfc49f7657ce5bd5908116b8191dcb06453a0b1c Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Tue, 12 Nov 2024 18:00:48 +0000 Subject: [PATCH 15/39] docs: add doc comments to new functions --- helix-core/src/surround.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index 982e6cb5c30a..af6e4cf6a5a8 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -337,7 +337,9 @@ pub fn get_surround_pos( Ok(change_pos) } -/// like get_surround_pos, but for Tags +/// 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( text: RopeSlice, selection: &Selection, @@ -355,6 +357,10 @@ pub fn get_surround_pos_tag( Ok(change_pos) } +/// 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 "_" pub fn is_valid_tagname_char(ch: char) -> bool { ch.is_alphanumeric() || ch == '_' || ch == '-' || ch == '.' } @@ -362,6 +368,9 @@ pub fn is_valid_tagname_char(ch: char) -> bool { pub fn find_nearest_tag(text: RopeSlice, cursor_pos: usize, skip: usize) -> Result<(Range, Range)> { } +/// Find the opening starting from "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 pub fn find_prev_tag( text: RopeSlice, mut pos: usize, @@ -401,6 +410,9 @@ pub fn find_prev_tag( Some(((pos + 1, pos + possible_tag.len()), possible_tag)) } +/// 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 pub fn find_next_tag( text: RopeSlice, mut pos: usize, From 757ad239e9fe314367aa089db63756ed73e5673f Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Tue, 12 Nov 2024 18:09:47 +0000 Subject: [PATCH 16/39] refactor: remove loop labels and return early instead --- helix-core/src/surround.rs | 79 +++++++++++++++++++------------------- 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index af6e4cf6a5a8..e52ca39f8aa9 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -343,7 +343,7 @@ pub fn get_surround_pos( pub fn get_surround_pos_tag( text: RopeSlice, selection: &Selection, - skip: usize, + _skip: usize, ) -> Result> { let mut change_pos = Vec::new(); @@ -365,7 +365,12 @@ pub fn is_valid_tagname_char(ch: char) -> bool { ch.is_alphanumeric() || ch == '_' || ch == '-' || ch == '.' } -pub fn find_nearest_tag(text: RopeSlice, cursor_pos: usize, skip: usize) -> Result<(Range, Range)> { +pub fn find_nearest_tag( + _text: RopeSlice, + _cursor_pos: usize, + _skip: usize, +) -> Result<(Range, Range)> { + Ok((Range::point(1), Range::point(1))) } /// Find the opening starting from "pos" and iterating until the beginning of the text. @@ -373,41 +378,38 @@ pub fn find_nearest_tag(text: RopeSlice, cursor_pos: usize, skip: usize) -> Resu /// As well as the actual name of the tag pub fn find_prev_tag( text: RopeSlice, - mut pos: usize, - n: usize, -) -> Option<((usize, usize), String)> { - if pos == 0 || n == 0 { + mut cursor_pos: usize, + skip: usize, +) -> Option<(Range, String)> { + if cursor_pos == 0 || skip == 0 { return None; } - let mut chars = text.chars_at(pos); - let mut possible_tag = String::new(); + let mut chars = text.chars_at(cursor_pos); - 'outer: loop { + loop { let prev_char = chars.prev()?; - pos -= 1; + cursor_pos -= 1; if prev_char == '>' { let mut possible_tag_name = String::new(); loop { let current_char = chars.prev()?; - pos -= 1; + cursor_pos -= 1; if current_char == '<' { - possible_tag.push_str( - &possible_tag_name - .chars() - .rev() - .take_while(|&ch| is_valid_tagname_char(ch)) - .collect::()[..], - ); - break 'outer; + let tag_name = possible_tag_name + .chars() + .rev() + .take_while(|&ch| is_valid_tagname_char(ch)) + .collect::(); + + let range = Range::new(cursor_pos + 1, cursor_pos + tag_name.len()); + return Some((range, tag_name)); } possible_tag_name.push(current_char); } } } - - Some(((pos + 1, pos + possible_tag.len()), possible_tag)) } /// Find the closing starting from "pos" and iterating the end of the text. @@ -415,35 +417,36 @@ pub fn find_prev_tag( /// As well as the actual name of the tag pub fn find_next_tag( text: RopeSlice, - mut pos: usize, - n: usize, -) -> Option<((usize, usize), String)> { - if pos >= text.len_chars() || n == 0 { + mut cursor_pos: usize, + skip: usize, +) -> Option<(Range, String)> { + if cursor_pos >= text.len_chars() || skip == 0 { return None; } - let mut chars = text.chars_at(pos); - let mut possible_tag = String::new(); + let mut chars = text.chars_at(cursor_pos); - 'outer: loop { + loop { // look forward, try to find something that looks like a closing tag e.g. // extract the name so e.g. "html". // set current_tag_name to this "html" string, then break. let next_char = chars.next()?; - pos += 1; + cursor_pos += 1; if next_char == '<' { let char_after_that = chars.next()?; - pos += 1; + cursor_pos += 1; if char_after_that == '/' { let mut possible_tag_name = String::new(); loop { let current_char = chars.next()?; - pos += 1; + 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 { - possible_tag.push_str(&possible_tag_name[..]); - break 'outer; + let range = + Range::new(cursor_pos - possible_tag_name.len() - 1, cursor_pos - 2); + + return Some((range, possible_tag_name)); } else { break; } @@ -451,8 +454,6 @@ pub fn find_next_tag( } } } - - return Some(((pos - possible_tag.len() - 1, pos - 2), possible_tag)); } #[cfg(test)] @@ -484,7 +485,7 @@ mod test { assert_eq!( find_next_tag(doc.slice(..), 7, 1).unwrap(), - ((12, 14), String::from("tag")) + (Range::new(12, 14), String::from("tag")) ); } @@ -494,7 +495,7 @@ mod test { assert_eq!( find_next_tag(doc.slice(..), 7, 1).unwrap(), - ((18, 28), String::from("Hello.World")) + (Range::new(18, 28), String::from("Hello.World")) ); } @@ -504,7 +505,7 @@ mod test { assert_eq!( find_prev_tag(doc.slice(..), 7, 1).unwrap(), - ((1, 3), String::from("tag")) + (Range::new(1, 3), String::from("tag")) ); } @@ -514,7 +515,7 @@ mod test { assert_eq!( find_prev_tag(doc.slice(..), 32, 1).unwrap(), - ((1, 11), String::from("Hello.World")) + (Range::new(1, 11), String::from("Hello.World")) ); } From 49084538bc8c97f5a6fd0047a0034ff7fa5f4511 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Tue, 12 Nov 2024 18:10:32 +0000 Subject: [PATCH 17/39] chore: remove unused function --- helix-core/src/search.rs | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/helix-core/src/search.rs b/helix-core/src/search.rs index 18bbbc01a918..81cb412939df 100644 --- a/helix-core/src/search.rs +++ b/helix-core/src/search.rs @@ -17,40 +17,6 @@ impl bool> CharMatcher for F { } } -pub fn find_nth_next_tag( - text: RopeSlice, - tag: &str, - mut pos: usize, - n: usize, -) -> Option> { - if pos >= text.len_chars() || n == 0 { - return None; - } - - let mut chars = text.chars_at(pos); - - let tag = format!(""); - let len = tag.len(); - - for _ in 0..n { - loop { - let c = chars.next()?; - let cloned_chars = chars.clone(); - let stri: String = cloned_chars.take(len).collect(); - - pos += 1; - - if stri == tag { - break; - } - } - } - - let range: Vec = (pos - 1..pos + len - 1).collect(); - - Some(range) -} - pub fn find_nth_next( text: RopeSlice, char_matcher: M, From 5a61331822c34543fdd33c695daef29fe7844fc4 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Tue, 12 Nov 2024 18:11:13 +0000 Subject: [PATCH 18/39] chore: remove unused comment --- helix-term/src/ui/prompt.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/helix-term/src/ui/prompt.rs b/helix-term/src/ui/prompt.rs index 6e0f79f0e2d3..6ba2fcb9e251 100644 --- a/helix-term/src/ui/prompt.rs +++ b/helix-term/src/ui/prompt.rs @@ -103,7 +103,6 @@ impl Prompt { self } - /// Sets the line of the prompt as if the user has typed that text manually pub fn set_line(&mut self, line: String, editor: &Editor) { let cursor = line.len(); self.line = line; From 6d671e4a92078c6caafaf115ad333d7faee8494a Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Tue, 12 Nov 2024 18:11:59 +0000 Subject: [PATCH 19/39] feat: remove unused comments --- empty.toml | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 empty.toml diff --git a/empty.toml b/empty.toml new file mode 100644 index 000000000000..e69de29bb2d1 From b77e09d1bf2b49fac5933b6967a7031e29e8834e Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Tue, 12 Nov 2024 18:12:31 +0000 Subject: [PATCH 20/39] chore: remove empty file --- empty.toml | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 empty.toml diff --git a/empty.toml b/empty.toml deleted file mode 100644 index e69de29bb2d1..000000000000 From f52b500fb443bce90c4880570c756cf36f858a44 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Tue, 12 Nov 2024 18:12:38 +0000 Subject: [PATCH 21/39] feat: remove unused comments --- helix-core/src/surround.rs | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index e52ca39f8aa9..b8e274d58658 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -154,29 +154,6 @@ fn find_nth_closest_pairs_plain( Err(Error::PairNotFound) } -/// Find the position of surrounding tags around cursor -/// `n` will skip n - 1 tags (e.g. n=2 will discard only the first tag found) -/// and keep looking -// pub fn find_nth_tag_pos( -// text: RopeSlice, -// range: Range, -// n: usize, -// ) -> Result<((usize, usize), (usize, usize))> { -// // 7 is the minimum possible length for a tag, e.g. . -// if text.len_chars() < 7 { -// return Err(Error::PairNotFound); -// } - -// if range.to() >= text.len_chars() { -// return Err(Error::RangeExceedsText); -// } - -// let cursor_position = range.cursor(text); - -// let open = (); -// let close = (); -// } - /// Find the position of surround pairs of `ch` which can be either a closing /// or opening pair. `n` will skip n - 1 pairs (eg. n=2 will discard (only) /// the first pair found and keep looking) From 12aa00680301166162b07b64bfb69281fb5dd273 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Tue, 12 Nov 2024 20:40:41 +0000 Subject: [PATCH 22/39] feat: complete implementation for getting surrounding tag --- helix-core/src/surround.rs | 146 ++++++++++++++++++++++++------------- 1 file changed, 95 insertions(+), 51 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index b8e274d58658..ccb0f77aa185 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -308,7 +308,6 @@ pub fn get_surround_pos( return Err(Error::CursorOverlap); } // ensure the positions are always paired in the forward direction - // e.g. [41, 214] change_pos.extend_from_slice(&[open_pos.min(close_pos), close_pos.max(open_pos)]); } Ok(change_pos) @@ -320,13 +319,13 @@ pub fn get_surround_pos( pub fn get_surround_pos_tag( text: RopeSlice, selection: &Selection, - _skip: usize, + skip: usize, ) -> Result> { let mut change_pos = Vec::new(); for &range in selection { let cursor_pos = range.cursor(text); - let (next_tag, prev_tag) = find_nearest_tag(text, cursor_pos, 1)?; + let ((prev_tag, next_tag), _) = find_nearest_tag(text, cursor_pos, skip)?; change_pos.push((prev_tag.from(), prev_tag.to())); change_pos.push((next_tag.from(), next_tag.to())); } @@ -335,43 +334,66 @@ 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 "_" +/// 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 `_` pub fn is_valid_tagname_char(ch: char) -> bool { ch.is_alphanumeric() || ch == '_' || ch == '-' || ch == '.' } +/// Get the two `Range`s corresponding to matching tags surrounding the cursor, as well as the name of the tags. pub fn find_nearest_tag( - _text: RopeSlice, - _cursor_pos: usize, - _skip: usize, -) -> Result<(Range, Range)> { - Ok((Range::point(1), Range::point(1))) + text: RopeSlice, + cursor_pos: usize, + skip: usize, +) -> Result<((Range, Range), String)> { + let mut next_tag_counter = 0; + + let forward_cursor_pos = cursor_pos.clone(); + let forward_text = text.clone(); + + loop { + let (next_tag_range, next_tag) = find_next_tag(forward_text, forward_cursor_pos, skip)?; + next_tag_counter += 1; + if next_tag_counter == skip { + loop { + let (prev_tag_range, prev_tag) = find_prev_tag(text, cursor_pos, skip)?; + if prev_tag == next_tag { + return Ok(((prev_tag_range, next_tag_range), prev_tag)); + } + } + } + } } -/// Find the opening starting from "pos" and iterating until the beginning of the text. -/// Returns the Range of the tag's name (excluding the "<" and ">" characters.) +/// 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 pub fn find_prev_tag( text: RopeSlice, mut cursor_pos: usize, skip: usize, -) -> Option<(Range, String)> { +) -> Result<(Range, String)> { if cursor_pos == 0 || skip == 0 { - return None; + return Err(Error::RangeExceedsText); } let mut chars = text.chars_at(cursor_pos); loop { - let prev_char = chars.prev()?; + let prev_char = match chars.prev() { + Some(ch) => ch, + None => return Err(Error::RangeExceedsText), + }; cursor_pos -= 1; if prev_char == '>' { let mut possible_tag_name = String::new(); loop { - let current_char = chars.prev()?; + let current_char = match chars.prev() { + Some(ch) => ch, + None => return Err(Error::RangeExceedsText), + }; cursor_pos -= 1; if current_char == '<' { let tag_name = possible_tag_name @@ -381,7 +403,7 @@ pub fn find_prev_tag( .collect::(); let range = Range::new(cursor_pos + 1, cursor_pos + tag_name.len()); - return Some((range, tag_name)); + return Ok((range, tag_name)); } possible_tag_name.push(current_char); } @@ -389,33 +411,40 @@ pub fn find_prev_tag( } } -/// Find the closing starting from "pos" and iterating the end of the text. -/// Returns the Range of the tag's name (excluding the "" characters.) +/// 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 pub fn find_next_tag( text: RopeSlice, mut cursor_pos: usize, skip: usize, -) -> Option<(Range, String)> { +) -> Result<(Range, String)> { if cursor_pos >= text.len_chars() || skip == 0 { - return None; + 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 { - // look forward, try to find something that looks like a closing tag e.g. - // extract the name so e.g. "html". - // set current_tag_name to this "html" string, then break. - let next_char = chars.next()?; + let next_char = match chars.next() { + Some(ch) => ch, + None => return Err(Error::RangeExceedsText), + }; cursor_pos += 1; if next_char == '<' { - let char_after_that = chars.next()?; + let char_after_that = match chars.next() { + Some(ch) => ch, + None => return Err(Error::RangeExceedsText), + }; cursor_pos += 1; if char_after_that == '/' { let mut possible_tag_name = String::new(); loop { - let current_char = chars.next()?; + let current_char = match chars.next() { + Some(ch) => ch, + None => return Err(Error::RangeExceedsText), + }; cursor_pos += 1; if is_valid_tagname_char(current_char) { possible_tag_name.push(current_char); @@ -423,7 +452,7 @@ pub fn find_next_tag( let range = Range::new(cursor_pos - possible_tag_name.len() - 1, cursor_pos - 2); - return Some((range, possible_tag_name)); + return Ok((range, possible_tag_name)); } else { break; } @@ -496,6 +525,21 @@ mod test { ); } + #[test] + fn test_find_surrounding_tag() { + #[rustfmt::skip] + let (doc, selection, expectations) = + rope_with_selections_and_expectations_tags( + " simple example ", + " ____ ^ ____ " + ); + + assert_eq!( + get_surround_pos_tag(doc.slice(..), &selection, 1), + Ok(expectations) + ); + } + #[test] fn test_get_surround_pos_bail_different_surround_chars() { #[rustfmt::skip] @@ -631,40 +675,40 @@ mod test { fn rope_with_selections_and_expectations_tags( text: &str, spec: &str, - ) -> (Rope, usize, Vec>) { + ) -> (Rope, Selection, Vec<(usize, usize)>) { if text.len() != spec.len() { panic!("specification must match text length -- are newlines aligned?"); } let rope = Rope::from(text); - // let selections: SmallVec<[Range; 1]> = spec - // .match_indices('^') - // .map(|(i, _)| Range::point(i)) - // .collect(); - - let cursor_idx = spec.find("^").unwrap(); + let selections: SmallVec<[Range; 1]> = spec + .match_indices('^') + .map(|(i, _)| Range::point(i)) + .collect(); - let expectations: Vec> = spec + let expectations: Vec<(usize, usize)> = spec .char_indices() - .filter(|&(_, c)| c == '_') - .map(|(i, _)| i) - .fold(Vec::new(), |mut groups, idx| { - if let Some(last_group) = groups.last_mut() { - if last_group - .last() - .map_or(false, |&last_idx| last_idx + 1 == idx) - { - last_group.push(idx); - } else { - groups.push(vec![idx]); + .chain(std::iter::once((spec.len(), ' '))) // Add sentinel to capture trailing groups + .fold(Vec::new(), |mut groups, (i, c)| { + match (groups.last_mut(), c) { + // Current character is an underscore, and the previous index is one lower than the current index, so extend the current group. + (Some((_start, end)), '_') if *end + 1 == i => { + *end = i; + } + // There is a gap of more than 1 between the current underscore's index and the previous underscore's index + (Some((_start, end)), '_') if *end < i => { + groups.push((i, i)); } - } else { - groups.push(vec![idx]); + // There hasn't been a group yet, so we are going to start it. + (None, '_') => { + groups.push((i, i)); + } + _non_underscore => {} } groups }); - (rope, cursor_idx, expectations) + (rope, Selection::new(selections, 0), expectations) } } From db3202b1cd1821b9198c1d8b30d3a5f6dbe3ffd9 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Tue, 12 Nov 2024 21:04:25 +0000 Subject: [PATCH 23/39] fix: infinite loop --- helix-core/src/surround.rs | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index ccb0f77aa185..e67ace4a5ea0 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -356,8 +356,16 @@ pub fn find_nearest_tag( let (next_tag_range, next_tag) = find_next_tag(forward_text, forward_cursor_pos, skip)?; next_tag_counter += 1; if next_tag_counter == skip { + let mut backward_search_idx = cursor_pos; + loop { - let (prev_tag_range, prev_tag) = find_prev_tag(text, cursor_pos, skip)?; + let (prev_tag_range, prev_tag, last_search_idx) = + find_prev_tag(text, backward_search_idx, skip)?; + + backward_search_idx = last_search_idx; + + dbg!(&prev_tag_range, &prev_tag); + if prev_tag == next_tag { return Ok(((prev_tag_range, next_tag_range), prev_tag)); } @@ -369,11 +377,12 @@ pub fn find_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. pub fn find_prev_tag( text: RopeSlice, mut cursor_pos: usize, skip: usize, -) -> Result<(Range, String)> { +) -> Result<(Range, String, usize)> { if cursor_pos == 0 || skip == 0 { return Err(Error::RangeExceedsText); } @@ -403,7 +412,7 @@ pub fn find_prev_tag( .collect::(); let range = Range::new(cursor_pos + 1, cursor_pos + tag_name.len()); - return Ok((range, tag_name)); + return Ok((range, tag_name, cursor_pos)); } possible_tag_name.push(current_char); } @@ -511,7 +520,7 @@ mod test { assert_eq!( find_prev_tag(doc.slice(..), 7, 1).unwrap(), - (Range::new(1, 3), String::from("tag")) + (Range::new(1, 3), String::from("tag"), 0) ); } @@ -521,12 +530,12 @@ mod test { assert_eq!( find_prev_tag(doc.slice(..), 32, 1).unwrap(), - (Range::new(1, 11), String::from("Hello.World")) + (Range::new(1, 11), String::from("Hello.World"), 0) ); } #[test] - fn test_find_surrounding_tag() { + fn test_find_surrounding_tag_simple() { #[rustfmt::skip] let (doc, selection, expectations) = rope_with_selections_and_expectations_tags( @@ -540,6 +549,21 @@ mod test { ); } + #[test] + fn test_find_surrounding_tag_with_imposter() { + #[rustfmt::skip] + let (doc, selection, expectations) = + rope_with_selections_and_expectations_tags( + "
simple example
", + " ___ ^ ___ " + ); + + assert_eq!( + get_surround_pos_tag(doc.slice(..), &selection, 1), + Ok(expectations) + ); + } + #[test] fn test_get_surround_pos_bail_different_surround_chars() { #[rustfmt::skip] From a6ab0fe1e4d7392ee331c14a69eb27b51fbe84e4 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Wed, 13 Nov 2024 04:01:18 +0000 Subject: [PATCH 24/39] feat: complete feature for find_nth_nearest_tag --- helix-core/src/surround.rs | 138 +++++++++++++++++++++++++++++-------- 1 file changed, 111 insertions(+), 27 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index e67ace4a5ea0..af01522f2e3f 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -325,7 +325,9 @@ pub fn get_surround_pos_tag( for &range in selection { let cursor_pos = range.cursor(text); - let ((prev_tag, next_tag), _) = find_nearest_tag(text, cursor_pos, skip)?; + + let ((prev_tag, next_tag), tag_name) = find_nth_nearest_tag(text, cursor_pos, skip)?; + change_pos.push((prev_tag.from(), prev_tag.to())); change_pos.push((next_tag.from(), next_tag.to())); } @@ -341,36 +343,81 @@ pub fn is_valid_tagname_char(ch: char) -> bool { ch.is_alphanumeric() || ch == '_' || ch == '-' || ch == '.' } -/// Get the two `Range`s corresponding to matching tags surrounding the cursor, as well as the name of the tags. -pub fn find_nearest_tag( - text: RopeSlice, +/// Get the two sorted `Range`s corresponding to nth matching tags surrounding the cursor, as well as the name of the tags. +pub fn find_nth_nearest_tag( + forward_text: RopeSlice, cursor_pos: usize, skip: usize, ) -> Result<((Range, Range), String)> { - let mut next_tag_counter = 0; + let backward_text = forward_text.clone(); - let forward_cursor_pos = cursor_pos.clone(); - let forward_text = text.clone(); + let mut forward_tags = vec![]; + let mut previous_forward_pos = cursor_pos; - loop { - let (next_tag_range, next_tag) = find_next_tag(forward_text, forward_cursor_pos, skip)?; - next_tag_counter += 1; - if next_tag_counter == skip { - let mut backward_search_idx = cursor_pos; + /// the maximum length of chars we will search forward and backward to find the tags, provided we don't hit the end or the beginning of the document + const SEARCH_CHARS: usize = 2000; - loop { - let (prev_tag_range, prev_tag, last_search_idx) = - find_prev_tag(text, backward_search_idx, skip)?; + while (previous_forward_pos - cursor_pos) < SEARCH_CHARS + && previous_forward_pos < forward_text.len_chars() + { + let (forward_tag_range, forward_tag_name, forward_search_idx) = + find_next_tag(forward_text, previous_forward_pos, skip)?; + + forward_tags.push((forward_tag_range, forward_tag_name)); + previous_forward_pos = forward_search_idx; + } - backward_search_idx = last_search_idx; + let mut backward_tags = vec![]; + let mut previous_backward_pos = cursor_pos; - dbg!(&prev_tag_range, &prev_tag); + while (cursor_pos - previous_backward_pos) < SEARCH_CHARS && previous_backward_pos > 0 { + let (backward_tag_range, backward_tag_name, backward_search_idx) = + find_prev_tag(backward_text, previous_backward_pos, skip)?; - if prev_tag == next_tag { - return Ok(((prev_tag_range, next_tag_range), prev_tag)); - } - } - } + backward_tags.push((backward_tag_range, backward_tag_name)); + previous_backward_pos = backward_search_idx; + } + + // only consider the tags which are common in both vectors + let backward_tags: Vec<(Range, String)> = backward_tags + .into_iter() + .filter(|(_, backward_tag_name)| { + forward_tags + .iter() + .any(|(_, forward_tag_name)| backward_tag_name == forward_tag_name) + }) + .collect(); + let forward_tags: Vec<(Range, String)> = forward_tags + .into_iter() + .filter(|(_, forward_tag_name)| { + backward_tags + .iter() + .any(|(_, backward_tag_name)| forward_tag_name == backward_tag_name) + }) + .collect(); + + // improperly ordered tags such as
are ignored completely + let matching_tags: Vec<((Range, String), (Range, String))> = forward_tags + .into_iter() + .zip(backward_tags) + .filter(|((_, forward_tag_name), (_, backward_tag_name))| { + forward_tag_name == backward_tag_name + }) + .collect(); + + // If the count overflows past the highest available outer tag, e.g. user types 100 but we can only select up to 4 nestings of tags -- simply select the last one available + let access_index = if skip - 1 <= matching_tags.len() { + skip - 1 + } else { + matching_tags.len() - 1 + }; + + if let Some(nth_matching_tags) = matching_tags.into_iter().nth(access_index) { + let ((range_forward, tag_name), (range_backward, _tag_name)) = nth_matching_tags; + + Ok(((range_backward, range_forward), tag_name)) + } else { + Err(Error::PairNotFound) } } @@ -422,12 +469,12 @@ pub fn find_prev_tag( /// 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 +/// As well as the actual name of the tag and where it last stopped searching. pub fn find_next_tag( text: RopeSlice, mut cursor_pos: usize, skip: usize, -) -> Result<(Range, String)> { +) -> Result<(Range, String, usize)> { if cursor_pos >= text.len_chars() || skip == 0 { return Err(Error::RangeExceedsText); } @@ -461,7 +508,7 @@ pub fn find_next_tag( let range = Range::new(cursor_pos - possible_tag_name.len() - 1, cursor_pos - 2); - return Ok((range, possible_tag_name)); + return Ok((range, possible_tag_name, cursor_pos)); } else { break; } @@ -500,7 +547,11 @@ mod test { assert_eq!( find_next_tag(doc.slice(..), 7, 1).unwrap(), - (Range::new(12, 14), String::from("tag")) + ( + Range::new(12, 14), + String::from("tag"), + doc.to_string().len() + ) ); } @@ -510,7 +561,11 @@ mod test { assert_eq!( find_next_tag(doc.slice(..), 7, 1).unwrap(), - (Range::new(18, 28), String::from("Hello.World")) + ( + Range::new(18, 28), + String::from("Hello.World"), + doc.to_string().len() + ) ); } @@ -564,6 +619,35 @@ mod test { ); } + #[test] + fn test_find_surrounding_tag_with_many_tags() { + #[rustfmt::skip] + let (doc, selection, expectations) = + rope_with_selections_and_expectations_tags( + "
simple example
", + " ___ ^ ___ " + ); + + assert_eq!( + get_surround_pos_tag(doc.slice(..), &selection, 1), + Ok(expectations) + ); + } + #[test] + fn test_find_surrounding_tag_with_many_many_tags() { + #[rustfmt::skip] + let (doc, selection, expectations) = + rope_with_selections_and_expectations_tags( + "
simple example
", + " ____ ^ ____ " + ); + + assert_eq!( + get_surround_pos_tag(doc.slice(..), &selection, 1), + Ok(expectations) + ); + } + #[test] fn test_get_surround_pos_bail_different_surround_chars() { #[rustfmt::skip] From b78a777e2bd5bdf4639ebb40e705a8624083f706 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Wed, 13 Nov 2024 04:30:24 +0000 Subject: [PATCH 25/39] feat: correct handling of nested tags --- helix-core/src/surround.rs | 132 +++++++++++++++++++++++-------------- helix-term/src/commands.rs | 8 ++- 2 files changed, 88 insertions(+), 52 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index af01522f2e3f..da54fe61af16 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -320,16 +320,16 @@ pub fn get_surround_pos_tag( text: RopeSlice, selection: &Selection, skip: usize, -) -> Result> { - let mut change_pos = Vec::new(); +) -> Result> { + let mut change_pos = vec![]; for &range in selection { let cursor_pos = range.cursor(text); let ((prev_tag, next_tag), tag_name) = find_nth_nearest_tag(text, cursor_pos, skip)?; - change_pos.push((prev_tag.from(), prev_tag.to())); - change_pos.push((next_tag.from(), next_tag.to())); + let tag_change = ((prev_tag, next_tag), tag_name); + change_pos.push(tag_change); } Ok(change_pos) @@ -592,7 +592,7 @@ mod test { #[test] fn test_find_surrounding_tag_simple() { #[rustfmt::skip] - let (doc, selection, expectations) = + let (doc, selection, _expectations) = rope_with_selections_and_expectations_tags( " simple example ", " ____ ^ ____ " @@ -600,53 +600,87 @@ mod test { assert_eq!( get_surround_pos_tag(doc.slice(..), &selection, 1), - Ok(expectations) + Ok(vec![( + (Range::new(1, 4), Range::new(24, 27)), + String::from("html") + )]) ); } - #[test] - fn test_find_surrounding_tag_with_imposter() { - #[rustfmt::skip] - let (doc, selection, expectations) = - rope_with_selections_and_expectations_tags( - "
simple example
", - " ___ ^ ___ " - ); - - assert_eq!( - get_surround_pos_tag(doc.slice(..), &selection, 1), - Ok(expectations) - ); - } - - #[test] - fn test_find_surrounding_tag_with_many_tags() { - #[rustfmt::skip] - let (doc, selection, expectations) = - rope_with_selections_and_expectations_tags( - "
simple example
", - " ___ ^ ___ " - ); - - assert_eq!( - get_surround_pos_tag(doc.slice(..), &selection, 1), - Ok(expectations) - ); - } - #[test] - fn test_find_surrounding_tag_with_many_many_tags() { - #[rustfmt::skip] - let (doc, selection, expectations) = - rope_with_selections_and_expectations_tags( - "
simple example
", - " ____ ^ ____ " - ); - - assert_eq!( - get_surround_pos_tag(doc.slice(..), &selection, 1), - Ok(expectations) - ); - } + // #[test] + // fn test_find_surrounding_tag_with_imposter() { + // #[rustfmt::skip] + // let (doc, selection, expectations) = + // rope_with_selections_and_expectations_tags( + // "
simple example
", + // " ___ ^ ___ " + // ); + + // assert_eq!( + // get_surround_pos_tag(doc.slice(..), &selection, 1), + // Ok(expectations) + // ); + // } + + // #[test] + // fn test_find_surrounding_tag_with_many_tags() { + // #[rustfmt::skip] + // let (doc, selection, _expectations) = + // rope_with_selections_and_expectations_tags( + // "
simple example
", + // " ___ ^ ____ " + // ); + + // assert_eq!( + // get_surround_pos_tag(doc.slice(..), &selection, 1), + // Err(Error::PairNotFound) + // ); + // } + + // #[test] + // fn test_find_surrounding_tag_with_many_many_tags() { + // #[rustfmt::skip] + // let (doc, selection, expectations) = + // rope_with_selections_and_expectations_tags( + // "
simple example
", + // " ____ ^ ____ " + // ); + + // assert_eq!( + // get_surround_pos_tag(doc.slice(..), &selection, 1), + // Ok(expectations) + // ); + // } + + // #[test] + // fn test_find_surrounding_tag_with_nth_tag() { + // #[rustfmt::skip] + // let (doc, selection, expectations) = + // rope_with_selections_and_expectations_tags( + // "
", + // " ____ ^ ____ " + // ); + + // assert_eq!( + // get_surround_pos_tag(doc.slice(..), &selection, 2), + // Ok(expectations) + // ); + // } + + // #[test] + // fn test_find_surrounding_tag_multiple_cursor() { + // #[rustfmt::skip] + // let (doc, selection, expectations) = + // rope_with_selections_and_expectations_tags( + // "
\n\n ", + // " ____ ^ ____ \n\n _ ^^^ _ " + // ); + + // assert_eq!( + // get_surround_pos_tag(doc.slice(..), &selection, 2), + // Ok(expectations) + // ); + // } #[test] fn test_get_surround_pos_bail_different_surround_chars() { diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 7fcdfaeeeb1e..5822d227d576 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5737,7 +5737,7 @@ fn surround_replace(cx: &mut Context) { let selection = selection.clone(); let ranges: SmallVec<[Range; 1]> = - change_pos.iter().map(|&p| Range::new(p.0, p.1)).collect(); + change_pos.iter().map(|p| Range::new(14, 14)).collect(); doc.set_selection( view.id, @@ -5756,8 +5756,10 @@ fn surround_replace(cx: &mut Context) { // taking chunks of two at once to replace with for p in change_pos.chunks(2) { - let line_opening = p[0]; - let line_closing = p[1]; + // let line_opening = p[0]; + // let line_closing = p[1]; + let line_opening = (1, 1); + let line_closing = (1, 1); sorted_pos.push((line_opening, open)); sorted_pos.push((line_closing, close)); } From 89fe2ffa3ec84266636fe8d29d1aa57bcce56552 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Wed, 13 Nov 2024 18:49:35 +0000 Subject: [PATCH 26/39] test: use new util function to test ranges of html tags and their names --- helix-core/src/surround.rs | 299 ++++++++++++++++--------------------- 1 file changed, 130 insertions(+), 169 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index da54fe61af16..2d4242b39790 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -541,147 +541,6 @@ mod test { ); } - #[test] - fn test_find_next_tag() { - let doc = Rope::from("hello"); - - assert_eq!( - find_next_tag(doc.slice(..), 7, 1).unwrap(), - ( - Range::new(12, 14), - String::from("tag"), - doc.to_string().len() - ) - ); - } - - #[test] - fn test_find_next_tag_broken() { - let doc = Rope::from("hello"); - - assert_eq!( - find_next_tag(doc.slice(..), 7, 1).unwrap(), - ( - Range::new(18, 28), - String::from("Hello.World"), - doc.to_string().len() - ) - ); - } - - #[test] - fn test_find_prev_tag() { - let doc = Rope::from("hello"); - - assert_eq!( - find_prev_tag(doc.slice(..), 7, 1).unwrap(), - (Range::new(1, 3), String::from("tag"), 0) - ); - } - - #[test] - fn test_find_prev_tag_broken() { - let doc = Rope::from(" "); - - assert_eq!( - find_prev_tag(doc.slice(..), 32, 1).unwrap(), - (Range::new(1, 11), String::from("Hello.World"), 0) - ); - } - - #[test] - fn test_find_surrounding_tag_simple() { - #[rustfmt::skip] - let (doc, selection, _expectations) = - rope_with_selections_and_expectations_tags( - " simple example ", - " ____ ^ ____ " - ); - - assert_eq!( - get_surround_pos_tag(doc.slice(..), &selection, 1), - Ok(vec![( - (Range::new(1, 4), Range::new(24, 27)), - String::from("html") - )]) - ); - } - - // #[test] - // fn test_find_surrounding_tag_with_imposter() { - // #[rustfmt::skip] - // let (doc, selection, expectations) = - // rope_with_selections_and_expectations_tags( - // "
simple example
", - // " ___ ^ ___ " - // ); - - // assert_eq!( - // get_surround_pos_tag(doc.slice(..), &selection, 1), - // Ok(expectations) - // ); - // } - - // #[test] - // fn test_find_surrounding_tag_with_many_tags() { - // #[rustfmt::skip] - // let (doc, selection, _expectations) = - // rope_with_selections_and_expectations_tags( - // "
simple example
", - // " ___ ^ ____ " - // ); - - // assert_eq!( - // get_surround_pos_tag(doc.slice(..), &selection, 1), - // Err(Error::PairNotFound) - // ); - // } - - // #[test] - // fn test_find_surrounding_tag_with_many_many_tags() { - // #[rustfmt::skip] - // let (doc, selection, expectations) = - // rope_with_selections_and_expectations_tags( - // "
simple example
", - // " ____ ^ ____ " - // ); - - // assert_eq!( - // get_surround_pos_tag(doc.slice(..), &selection, 1), - // Ok(expectations) - // ); - // } - - // #[test] - // fn test_find_surrounding_tag_with_nth_tag() { - // #[rustfmt::skip] - // let (doc, selection, expectations) = - // rope_with_selections_and_expectations_tags( - // "
", - // " ____ ^ ____ " - // ); - - // assert_eq!( - // get_surround_pos_tag(doc.slice(..), &selection, 2), - // Ok(expectations) - // ); - // } - - // #[test] - // fn test_find_surrounding_tag_multiple_cursor() { - // #[rustfmt::skip] - // let (doc, selection, expectations) = - // rope_with_selections_and_expectations_tags( - // "
\n\n ", - // " ____ ^ ____ \n\n _ ^^^ _ " - // ); - - // assert_eq!( - // get_surround_pos_tag(doc.slice(..), &selection, 2), - // Ok(expectations) - // ); - // } - #[test] fn test_get_surround_pos_bail_different_surround_chars() { #[rustfmt::skip] @@ -791,65 +650,167 @@ mod test { ) } - /// Create a Rope and a matching Selection using a specification language. - /// ^ is a single-point selection. - /// _ is an expected index. These are returned as a Vec for use in assertions. - fn rope_with_selections_and_expectations( - text: &str, - spec: &str, - ) -> (Rope, Selection, Vec) { - if text.len() != spec.len() { - panic!("specification must match text length -- are newlines aligned?"); - } + #[test] + fn test_find_surrounding_tag_simple() { + let (doc, selection, expectations) = rope_with_selections_and_expectations_tags( + " test ", + " ____ ^ ____ ", + vec!["html"], + ); - let rope = Rope::from(text); + assert_eq!( + get_surround_pos_tag(doc.slice(..), &selection, 1), + expectations + ); + } - let selections: SmallVec<[Range; 1]> = spec - .match_indices('^') - .map(|(i, _)| Range::point(i)) - .collect(); + #[test] + fn test_find_surrounding_tag_with_imposter() { + let (doc, selection, expectations) = rope_with_selections_and_expectations_tags( + "
test
", + " ___ ^ ___ ", + vec!["div"], + ); - let expectations: Vec = spec.match_indices('_').map(|(i, _)| i).collect(); + assert_eq!( + get_surround_pos_tag(doc.slice(..), &selection, 1), + expectations + ); + } - (rope, Selection::new(selections, 0), expectations) + #[test] + fn test_find_surrounding_tag_with_many_tags() { + let (doc, selection, _) = rope_with_selections_and_expectations_tags( + "
simple example
", + " ^ ", + vec!["span"], + ); + + assert_eq!( + get_surround_pos_tag(doc.slice(..), &selection, 1), + Err(Error::PairNotFound) + ); + } + + #[test] + fn test_find_surrounding_tag_with_many_many_tags() { + let (doc, selection, expectations) = rope_with_selections_and_expectations_tags( + "
simple example
", + " ____ ^ ____ ", + vec!["span"], + ); + + assert_eq!( + get_surround_pos_tag(doc.slice(..), &selection, 1), + expectations + ); + } + + #[test] + fn test_find_surrounding_tag_with_nth_tag() { + let (doc, selection, expectations) = rope_with_selections_and_expectations_tags( + "
test
", + " ____ ^ ____ ", + vec!["span"], + ); + + assert_eq!( + get_surround_pos_tag(doc.slice(..), &selection, 2), + expectations + ); + } + + #[test] + fn test_find_surrounding_tag_multiple_cursor() { + let (doc, selection, expectations) = rope_with_selections_and_expectations_tags( + "
\n\n ", + " ____ ^ ____ \n\n _ ^ _ ", + vec!["span", "b"], + ); + + assert_eq!( + get_surround_pos_tag(doc.slice(..), &selection, 2), + expectations + ); } + /// Create a Rope and a matching Selection using a specification language. + /// ^ is a cursor position. + /// Continuous _ denote start and end of ranges. These are returned as (Range, Range) + /// for use within assertions. fn rope_with_selections_and_expectations_tags( text: &str, spec: &str, - ) -> (Rope, Selection, Vec<(usize, usize)>) { + tag_names: Vec<&str>, + ) -> (Rope, Selection, Result>) { if text.len() != spec.len() { panic!("specification must match text length -- are newlines aligned?"); } - - let rope = Rope::from(text); - let selections: SmallVec<[Range; 1]> = spec .match_indices('^') .map(|(i, _)| Range::point(i)) .collect(); - let expectations: Vec<(usize, usize)> = spec + let mut tag_names = tag_names.into_iter(); + + let mut raw_ranges = spec .char_indices() - .chain(std::iter::once((spec.len(), ' '))) // Add sentinel to capture trailing groups + .chain(std::iter::once((spec.len(), ' '))) .fold(Vec::new(), |mut groups, (i, c)| { match (groups.last_mut(), c) { - // Current character is an underscore, and the previous index is one lower than the current index, so extend the current group. (Some((_start, end)), '_') if *end + 1 == i => { + // Extend current group *end = i; } - // There is a gap of more than 1 between the current underscore's index and the previous underscore's index (Some((_start, end)), '_') if *end < i => { + // Start a new group after a gap groups.push((i, i)); } - // There hasn't been a group yet, so we are going to start it. (None, '_') => { + // Start the first group groups.push((i, i)); } - _non_underscore => {} + _ => {} // Ignore non-underscore characters } groups - }); + }) + .into_iter(); + + let range_and_tags = std::iter::from_fn(|| Some((raw_ranges.next()?, raw_ranges.next()?))) + .map(|((anchor1, head1), (anchor2, head2))| { + let range1 = Range::new(anchor1, head1); + let range2 = Range::new(anchor2, head2); + let next_tag_name = tag_names.next().unwrap(); + ((range1, range2), String::from(next_tag_name)) + }) + .collect(); + + ( + Rope::from(text), + Selection::new(selections, 0), + Ok(range_and_tags), + ) + } + + /// Create a Rope and a matching Selection using a specification language. + /// ^ is a single-point selection. + /// _ is an expected index. These are returned as a Vec for use in assertions. + fn rope_with_selections_and_expectations( + text: &str, + spec: &str, + ) -> (Rope, Selection, Vec) { + if text.len() != spec.len() { + panic!("specification must match text length -- are newlines aligned?"); + } + + let rope = Rope::from(text); + + let selections: SmallVec<[Range; 1]> = spec + .match_indices('^') + .map(|(i, _)| Range::point(i)) + .collect(); + + let expectations: Vec = spec.match_indices('_').map(|(i, _)| i).collect(); (rope, Selection::new(selections, 0), expectations) } From f16aa7b18d1ccf3ac1c9dd7245f3ec96ed9424aa Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Wed, 13 Nov 2024 19:13:25 +0000 Subject: [PATCH 27/39] test: add several more tests --- helix-core/src/surround.rs | 108 ++++++++++++++++++++++++++++++++----- 1 file changed, 96 insertions(+), 12 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index 2d4242b39790..fada7c5c6f16 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -439,7 +439,7 @@ pub fn find_prev_tag( loop { let prev_char = match chars.prev() { Some(ch) => ch, - None => return Err(Error::RangeExceedsText), + None => return Err(Error::PairNotFound), }; cursor_pos -= 1; @@ -448,7 +448,7 @@ pub fn find_prev_tag( loop { let current_char = match chars.prev() { Some(ch) => ch, - None => return Err(Error::RangeExceedsText), + None => return Err(Error::PairNotFound), }; cursor_pos -= 1; if current_char == '<' { @@ -485,13 +485,13 @@ pub fn find_next_tag( loop { let next_char = match chars.next() { Some(ch) => ch, - None => return Err(Error::RangeExceedsText), + 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::RangeExceedsText), + None => return Err(Error::PairNotFound), }; cursor_pos += 1; if char_after_that == '/' { @@ -499,7 +499,7 @@ pub fn find_next_tag( loop { let current_char = match chars.next() { Some(ch) => ch, - None => return Err(Error::RangeExceedsText), + None => return Err(Error::PairNotFound), }; cursor_pos += 1; if is_valid_tagname_char(current_char) { @@ -665,7 +665,7 @@ mod test { } #[test] - fn test_find_surrounding_tag_with_imposter() { + fn test_find_surrounding_tag_with_extra_closing_tag() { let (doc, selection, expectations) = rope_with_selections_and_expectations_tags( "
test
", " ___ ^ ___ ", @@ -679,11 +679,11 @@ mod test { } #[test] - fn test_find_surrounding_tag_with_many_tags() { + fn test_find_surrounding_tag_with_broken_tags() { let (doc, selection, _) = rope_with_selections_and_expectations_tags( "
simple example
", " ^ ", - vec!["span"], + vec![], ); assert_eq!( @@ -693,7 +693,7 @@ mod test { } #[test] - fn test_find_surrounding_tag_with_many_many_tags() { + fn test_find_surrounding_tag_with_many_tags() { let (doc, selection, expectations) = rope_with_selections_and_expectations_tags( "
simple example
", " ____ ^ ____ ", @@ -707,10 +707,10 @@ mod test { } #[test] - fn test_find_surrounding_tag_with_nth_tag() { + fn test_find_surrounding_tag_with_nth_tag_newline() { let (doc, selection, expectations) = rope_with_selections_and_expectations_tags( - "
test
", - " ____ ^ ____ ", + "
test\n\n
", + " ____ ^ \n\n ____ ", vec!["span"], ); @@ -734,6 +734,90 @@ mod test { ); } + #[test] + fn test_find_surrounding_tag_empty_document() { + let (doc, selection, _) = rope_with_selections_and_expectations_tags( + " hello world, wonderful world! ", + " ^ ", + vec![], + ); + + assert_eq!( + get_surround_pos_tag(doc.slice(..), &selection, 1), + Err(Error::PairNotFound) + ); + } + + #[test] + fn test_find_surrounding_tag_unclosed_tag() { + let (doc, selection, _) = rope_with_selections_and_expectations_tags( + "this is an
Unclosed tag", + " ^ ", + vec![], + ); + + assert_eq!( + get_surround_pos_tag(doc.slice(..), &selection, 1), + Err(Error::PairNotFound) + ); + } + + #[test] + fn test_find_surrounding_tag_nested_with_partial_overlap() { + let (doc, selection, expectations) = rope_with_selections_and_expectations_tags( + "

Text

", + " ____ ^ ____ ", + vec!["span"], + ); + + assert_eq!( + get_surround_pos_tag(doc.slice(..), &selection, 1), + expectations + ); + } + + #[test] + fn test_find_surrounding_tag_nested_same_tag_multiple_levels() { + let (doc, selection, _) = rope_with_selections_and_expectations_tags( + "
Nested
", + " ___ ^ ___ ", + vec!["div"], + ); + + assert_eq!( + get_surround_pos_tag(doc.slice(..), &selection, 2), + Err(Error::PairNotFound) + ); + } + + #[test] + fn test_find_surrounding_tag_self_closing_tags_ignored() { + let (doc, selection, expectations) = rope_with_selections_and_expectations_tags( + "
Text
", + " ___ ^ ___ ", + vec!["div"], + ); + + assert_eq!( + get_surround_pos_tag(doc.slice(..), &selection, 1), + expectations + ); + } + + #[test] + fn test_find_surrounding_tag_adjacent_tags() { + let (doc, selection, expectations) = rope_with_selections_and_expectations_tags( + "
", + " ___ ^ ___ ", + vec!["div"], + ); + + assert_eq!( + get_surround_pos_tag(doc.slice(..), &selection, 1), + expectations + ); + } + /// Create a Rope and a matching Selection using a specification language. /// ^ is a cursor position. /// Continuous _ denote start and end of ranges. These are returned as (Range, Range) From dfd1252f5f48a410c33fe58c235a6850a04b7ea2 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Wed, 13 Nov 2024 19:14:29 +0000 Subject: [PATCH 28/39] chore: remove "pub" keyword from functions which do not need export --- helix-core/src/surround.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index fada7c5c6f16..50f77c6dc7de 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -339,12 +339,12 @@ pub fn get_surround_pos_tag( /// 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 `_` -pub fn is_valid_tagname_char(ch: char) -> bool { +fn is_valid_tagname_char(ch: char) -> bool { ch.is_alphanumeric() || ch == '_' || ch == '-' || ch == '.' } /// Get the two sorted `Range`s corresponding to nth matching tags surrounding the cursor, as well as the name of the tags. -pub fn find_nth_nearest_tag( +fn find_nth_nearest_tag( forward_text: RopeSlice, cursor_pos: usize, skip: usize, @@ -425,7 +425,7 @@ pub fn find_nth_nearest_tag( /// 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. -pub fn find_prev_tag( +fn find_prev_tag( text: RopeSlice, mut cursor_pos: usize, skip: usize, @@ -470,7 +470,7 @@ pub fn find_prev_tag( /// 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. -pub fn find_next_tag( +fn find_next_tag( text: RopeSlice, mut cursor_pos: usize, skip: usize, From 74b8ca9c72108ef71424064c90c795186de33f4e Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Wed, 13 Nov 2024 19:29:37 +0000 Subject: [PATCH 29/39] feat: add basic core functionality to replace Tag with another --- helix-term/src/commands.rs | 42 +++++++++++++++----------------------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 5822d227d576..40fbd30510a2 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5725,8 +5725,6 @@ fn surround_replace(cx: &mut Context) { }; // TODO: add back the logic for other surround changes } else { - // TODO: obtain these values from a function - // let change_pos: Vec<(usize, usize)> = vec![(4, 10), (13, 20), (24, 30), (33, 40)]; let change_pos = match surround::get_surround_pos_tag(text, selection, layer) { Ok(c) => c, Err(err) => { @@ -5736,43 +5734,37 @@ fn surround_replace(cx: &mut Context) { }; let selection = selection.clone(); - let ranges: SmallVec<[Range; 1]> = - change_pos.iter().map(|p| Range::new(14, 14)).collect(); + let ranges: SmallVec<[Range; 1]> = change_pos + .iter() + .flat_map(|&((opening_tag_range, closing_tag_range), _)| { + vec![opening_tag_range, closing_tag_range] + }) + .collect(); doc.set_selection( view.id, Selection::new(ranges, selection.primary_index() * 2), ); - cx.on_next_key(move |cx, event| { + cx.on_next_key(move |cx, _event| { let (view, doc) = current!(cx.editor); // TODO: obtain this value from the input - let open = ""; - let close = ""; - - // the changeset has to be sorted to allow nested surrounds - let mut sorted_pos: Vec<((usize, usize), &str)> = Vec::new(); - - // taking chunks of two at once to replace with - for p in change_pos.chunks(2) { - // let line_opening = p[0]; - // let line_closing = p[1]; - let line_opening = (1, 1); - let line_closing = (1, 1); - sorted_pos.push((line_opening, open)); - sorted_pos.push((line_closing, close)); - } - sorted_pos.sort_unstable(); + let user_input = "help"; let transaction = Transaction::change( doc.text(), - sorted_pos.iter().map(|&(pos, change_tag)| { + change_pos.iter().flat_map(|(pos, _)| { let mut tag = Tendril::new(); - tag.push_str(change_tag); + tag.push_str(user_input); + + let opening_range = pos.0; + let closing_range = pos.1; - // replace from pos.0 to pos.1 with t - (pos.0, pos.1, Some(tag)) + vec![ + (opening_range.from(), opening_range.to(), Some(tag.clone())), + (closing_range.from(), closing_range.to(), Some(tag.clone())), + ] }), ); From b2f1bb8de9f65929f31ee4a459aa0ec01240fd0b Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Wed, 13 Nov 2024 19:59:43 +0000 Subject: [PATCH 30/39] fix: handle pair not found error gracefully --- helix-core/src/surround.rs | 21 +++++++++++++++++---- helix-term/src/commands.rs | 14 +++++++------- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index 50f77c6dc7de..1ccf6c2eae50 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -360,11 +360,23 @@ fn find_nth_nearest_tag( while (previous_forward_pos - cursor_pos) < SEARCH_CHARS && previous_forward_pos < forward_text.len_chars() { - let (forward_tag_range, forward_tag_name, forward_search_idx) = - find_next_tag(forward_text, previous_forward_pos, skip)?; + let next_tag_maybe = find_next_tag(forward_text, previous_forward_pos, skip); - forward_tags.push((forward_tag_range, forward_tag_name)); - previous_forward_pos = forward_search_idx; + match next_tag_maybe { + Ok((forward_tag_range, forward_tag_name, forward_search_idx)) => { + forward_tags.push((forward_tag_range, forward_tag_name)); + previous_forward_pos = forward_search_idx; + } + Err(err) => match err { + Error::PairNotFound => { + break; + } + other_error => { + // Handle other errors + return Err(other_error); + } + }, + } } let mut backward_tags = vec![]; @@ -510,6 +522,7 @@ fn find_next_tag( return Ok((range, possible_tag_name, cursor_pos)); } else { + log::error!("BREAKING!"); break; } } diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 40fbd30510a2..0e2a85110236 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5716,13 +5716,13 @@ fn surround_replace(cx: &mut Context) { let selection = doc.selection(view.id); if false { - let change_pos = match surround::get_surround_pos_tag(text, selection, layer) { - Ok(c) => c, - Err(err) => { - cx.editor.set_error(err.to_string()); - return; - } - }; + // let change_pos = match surround::get_surround_pos_tag(text, selection, layer) { + // Ok(c) => c, + // Err(err) => { + // cx.editor.set_error(err.to_string()); + // return; + // } + // }; // TODO: add back the logic for other surround changes } else { let change_pos = match surround::get_surround_pos_tag(text, selection, layer) { From 6f6ceda1ba438e1a8d302000057573a33415eea1 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Wed, 13 Nov 2024 20:40:46 +0000 Subject: [PATCH 31/39] fix: add + 1 to head for every range --- helix-core/src/surround.rs | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index 1ccf6c2eae50..30bd004b8bb5 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -360,9 +360,7 @@ fn find_nth_nearest_tag( while (previous_forward_pos - cursor_pos) < SEARCH_CHARS && previous_forward_pos < forward_text.len_chars() { - let next_tag_maybe = find_next_tag(forward_text, previous_forward_pos, skip); - - match next_tag_maybe { + match find_next_tag(forward_text, previous_forward_pos, skip) { Ok((forward_tag_range, forward_tag_name, forward_search_idx)) => { forward_tags.push((forward_tag_range, forward_tag_name)); previous_forward_pos = forward_search_idx; @@ -372,7 +370,6 @@ fn find_nth_nearest_tag( break; } other_error => { - // Handle other errors return Err(other_error); } }, @@ -383,11 +380,20 @@ fn find_nth_nearest_tag( let mut previous_backward_pos = cursor_pos; while (cursor_pos - previous_backward_pos) < SEARCH_CHARS && previous_backward_pos > 0 { - let (backward_tag_range, backward_tag_name, backward_search_idx) = - find_prev_tag(backward_text, previous_backward_pos, skip)?; - - backward_tags.push((backward_tag_range, backward_tag_name)); - previous_backward_pos = backward_search_idx; + match find_prev_tag(backward_text, previous_backward_pos, skip) { + Ok((backward_tag_range, backward_tag_name, backward_search_idx)) => { + backward_tags.push((backward_tag_range, backward_tag_name)); + previous_backward_pos = backward_search_idx; + } + Err(err) => match err { + Error::PairNotFound => { + break; + } + other_error => { + return Err(other_error); + } + }, + } } // only consider the tags which are common in both vectors @@ -470,7 +476,7 @@ fn find_prev_tag( .take_while(|&ch| is_valid_tagname_char(ch)) .collect::(); - let range = Range::new(cursor_pos + 1, cursor_pos + tag_name.len()); + 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); @@ -518,7 +524,7 @@ fn find_next_tag( 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 - 2); + Range::new(cursor_pos - possible_tag_name.len() - 1, cursor_pos - 1); return Ok((range, possible_tag_name, cursor_pos)); } else { @@ -875,8 +881,8 @@ mod test { let range_and_tags = std::iter::from_fn(|| Some((raw_ranges.next()?, raw_ranges.next()?))) .map(|((anchor1, head1), (anchor2, head2))| { - let range1 = Range::new(anchor1, head1); - let range2 = Range::new(anchor2, head2); + let range1 = Range::new(anchor1, head1 + 1); + let range2 = Range::new(anchor2, head2 + 1); let next_tag_name = tag_names.next().unwrap(); ((range1, range2), String::from(next_tag_name)) }) From 6e4d022e98568698d00d35d8e0622d06e2200872 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Wed, 13 Nov 2024 21:23:03 +0000 Subject: [PATCH 32/39] feat: complete implementation for replace surrounding tag --- helix-term/src/commands.rs | 125 +++++++++++++++++++++++++++---------- 1 file changed, 91 insertions(+), 34 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 0e2a85110236..b2742761fafe 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5704,7 +5704,7 @@ fn surround_add(cx: &mut Context) { } fn surround_replace(cx: &mut Context) { - let layer = cx.count(); + let count = cx.count(); cx.on_next_key(move |cx, event| { let surround_ch = match event.char() { Some('m') => None, // m selects the closest surround pair @@ -5715,17 +5715,8 @@ fn surround_replace(cx: &mut Context) { let text = doc.text().slice(..); let selection = doc.selection(view.id); - if false { - // let change_pos = match surround::get_surround_pos_tag(text, selection, layer) { - // Ok(c) => c, - // Err(err) => { - // cx.editor.set_error(err.to_string()); - // return; - // } - // }; - // TODO: add back the logic for other surround changes - } else { - let change_pos = match surround::get_surround_pos_tag(text, selection, layer) { + if surround_ch.is_some_and(|ch| ch == 'x') { + let change_pos = match surround::get_surround_pos_tag(text, &selection, count) { Ok(c) => c, Err(err) => { cx.editor.set_error(err.to_string()); @@ -5733,42 +5724,108 @@ fn surround_replace(cx: &mut Context) { } }; - let selection = selection.clone(); - let ranges: SmallVec<[Range; 1]> = change_pos + // change_pos is guaranteed to have at least one value, because if not, then we would have returned earlier + let tag_name = change_pos.first().unwrap().1.clone(); + let all_same = change_pos .iter() - .flat_map(|&((opening_tag_range, closing_tag_range), _)| { - vec![opening_tag_range, closing_tag_range] - }) - .collect(); + .all(|(_, tag_name_other)| tag_name_other == &tag_name); + + let prompt = Prompt::new( + "tag name:".into(), + Some('z'), + ui::completers::none, + move |context: &mut compositor::Context, input: &str, event: PromptEvent| { + let (view, document) = current!(context.editor); + let copy_selection = document.selection(view.id); + + if event != PromptEvent::Validate { + return; + } + + let ranges: SmallVec<[Range; 1]> = change_pos + .iter() + .flat_map(|&((opening_tag_range, closing_tag_range), _)| { + vec![opening_tag_range, closing_tag_range] + }) + .collect(); + document.set_selection( + view.id, + Selection::new(ranges, copy_selection.primary_index() * 2), + ); + let tag = Tendril::from(input); + + 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())), + ] + }), + ); + + // we don't need to touch this + // document.set_selection(view.id, *copy_selection); + document.apply(&transaction, view.id); + context.editor.mode = Mode::Normal; + }, + ); + + let prompt = if all_same { + // prefill with tag name because all tags have the same name + prompt.with_line(tag_name, &cx.editor) + } else { + prompt + }; + + cx.push_layer(Box::new(prompt)); + } else { + let change_pos = + match surround::get_surround_pos(doc.syntax(), text, selection, surround_ch, count) + { + Ok(c) => c, + Err(err) => { + cx.editor.set_error(err.to_string()); + return; + } + }; + + let selection = selection.clone(); + let ranges: SmallVec<[Range; 1]> = + change_pos.iter().map(|&p| Range::point(p)).collect(); doc.set_selection( view.id, Selection::new(ranges, selection.primary_index() * 2), ); - cx.on_next_key(move |cx, _event| { + cx.on_next_key(move |cx, event| { let (view, doc) = current!(cx.editor); + let to = match event.char() { + Some(to) => to, + None => return doc.set_selection(view.id, selection), + }; + let (open, close) = match_brackets::get_pair(to); - // TODO: obtain this value from the input - let user_input = "help"; + // the changeset has to be sorted to allow nested surrounds + let mut sorted_pos: Vec<(usize, char)> = Vec::new(); + for p in change_pos.chunks(2) { + sorted_pos.push((p[0], open)); + sorted_pos.push((p[1], close)); + } + sorted_pos.sort_unstable(); let transaction = Transaction::change( doc.text(), - change_pos.iter().flat_map(|(pos, _)| { - let mut tag = Tendril::new(); - tag.push_str(user_input); - - 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())), - ] + sorted_pos.iter().map(|&pos| { + let mut t = Tendril::new(); + t.push(pos.1); + (pos.0, pos.0 + 1, Some(t)) }), ); - - // we don't need to touch this doc.set_selection(view.id, selection); doc.apply(&transaction, view.id); exit_select_mode(cx); From 2a61b113de2e052a4d95c3d3cba14d03b316d7c4 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Wed, 13 Nov 2024 21:28:00 +0000 Subject: [PATCH 33/39] feat: add delete surrounding tag functionality --- helix-term/src/commands.rs | 42 ++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index b2742761fafe..4312bdc2b475 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5768,8 +5768,6 @@ fn surround_replace(cx: &mut Context) { }), ); - // we don't need to touch this - // document.set_selection(view.id, *copy_selection); document.apply(&transaction, view.id); context.editor.mode = Mode::Normal; }, @@ -5846,19 +5844,45 @@ fn surround_delete(cx: &mut Context) { let text = doc.text().slice(..); let selection = doc.selection(view.id); - let mut change_pos = - match surround::get_surround_pos(doc.syntax(), text, selection, surround_ch, count) { + if surround_ch.is_some_and(|ch| ch == 'x') { + let change_pos = match surround::get_surround_pos_tag(text, &selection, count) { Ok(c) => c, Err(err) => { cx.editor.set_error(err.to_string()); return; } }; - change_pos.sort_unstable(); // the changeset has to be sorted to allow nested surrounds - let transaction = - Transaction::change(doc.text(), change_pos.into_iter().map(|p| (p, p + 1, None))); - doc.apply(&transaction, view.id); - exit_select_mode(cx); + let transaction = Transaction::change( + doc.text(), + change_pos.iter().flat_map(|(pos, _)| { + let opening_range = pos.0; + let closing_range = pos.1; + + vec![ + // add extra numbers to account for "<", ">" and "/" characters + (opening_range.from() - 1, opening_range.to() + 1, None), + (closing_range.from() - 2, closing_range.to() + 1, None), + ] + }), + ); + doc.apply(&transaction, view.id); + exit_select_mode(cx); + } else { + let mut change_pos = + match surround::get_surround_pos(doc.syntax(), text, selection, surround_ch, count) + { + Ok(c) => c, + Err(err) => { + cx.editor.set_error(err.to_string()); + return; + } + }; + change_pos.sort_unstable(); // the changeset has to be sorted to allow nested surrounds + let transaction = + Transaction::change(doc.text(), change_pos.into_iter().map(|p| (p, p + 1, None))); + doc.apply(&transaction, view.id); + exit_select_mode(cx); + } }) } From 3633c8cd0f0b45a41c2a86c5841f8e6835214324 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Wed, 13 Nov 2024 22:47:39 +0000 Subject: [PATCH 34/39] fix: panic which occured if cursors overlapped --- helix-core/src/transaction.rs | 1 + helix-term/src/commands.rs | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/helix-core/src/transaction.rs b/helix-core/src/transaction.rs index c5c94b750529..ad08d0c65666 100644 --- a/helix-core/src/transaction.rs +++ b/helix-core/src/transaction.rs @@ -615,6 +615,7 @@ impl Transaction { let mut last = 0; for (from, to, tendril) in changes { // Verify ranges are ordered and not overlapping + dbg!(last, from); debug_assert!(last <= from); // Verify ranges are correct debug_assert!( diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 4312bdc2b475..69a68d9869eb 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5742,13 +5742,26 @@ fn surround_replace(cx: &mut Context) { return; } - let ranges: SmallVec<[Range; 1]> = change_pos + 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; + } + document.set_selection( view.id, Selection::new(ranges, copy_selection.primary_index() * 2), From 7f9e7611b6ec8da99c9e1fbfef744db487cb72bc Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Wed, 13 Nov 2024 22:49:52 +0000 Subject: [PATCH 35/39] fix: panic which occured if ranges overlapped --- helix-core/src/transaction.rs | 1 - helix-term/src/commands.rs | 18 ++++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/helix-core/src/transaction.rs b/helix-core/src/transaction.rs index ad08d0c65666..c5c94b750529 100644 --- a/helix-core/src/transaction.rs +++ b/helix-core/src/transaction.rs @@ -615,7 +615,6 @@ impl Transaction { let mut last = 0; for (from, to, tendril) in changes { // Verify ranges are ordered and not overlapping - dbg!(last, from); debug_assert!(last <= from); // Verify ranges are correct debug_assert!( diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 69a68d9869eb..550b4c1c9973 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5865,6 +5865,24 @@ 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, _)| { 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 36/39] 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 30bd004b8bb5..aa50eda42909 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 550b4c1c9973..8d45474f3c0e 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), ] From a88ffcd413454e38fd783154af1f883779f66f02 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:41:57 +0000 Subject: [PATCH 37/39] test: refactored selections and expectations util to return only 1 range --- helix-core/src/surround.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index aa50eda42909..0fa9935ad402 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -855,18 +855,21 @@ mod test { text: &str, spec: &str, tag_names: Vec<&str>, - ) -> (Rope, Selection, Result>) { + ) -> (Rope, Selection, Result>) { if text.len() != spec.len() { panic!("specification must match text length -- are newlines aligned?"); } + let selections: SmallVec<[Range; 1]> = spec .match_indices('^') .map(|(i, _)| Range::point(i)) .collect(); - let mut tag_names = tag_names.into_iter(); + let mut tag_names = tag_names + .into_iter() + .flat_map(|tag_name| vec![tag_name, tag_name]); - let mut raw_ranges = spec + let raw_ranges = spec .char_indices() .chain(std::iter::once((spec.len(), ' '))) .fold(Vec::new(), |mut groups, (i, c)| { @@ -889,12 +892,12 @@ mod test { }) .into_iter(); - let range_and_tags = std::iter::from_fn(|| Some((raw_ranges.next()?, raw_ranges.next()?))) - .map(|((anchor1, head1), (anchor2, head2))| { - let range1 = Range::new(anchor1, head1 + 1); - let range2 = Range::new(anchor2, head2 + 1); - let next_tag_name = tag_names.next().unwrap(); - ((range1, range2), String::from(next_tag_name)) + let range_and_tags = raw_ranges + .map(|(anchor, head)| { + ( + Range::new(anchor, head + 1), + String::from(tag_names.next().unwrap()), + ) }) .collect(); From c3a3e4803c68075266f4fd276e8a81034b031f44 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:53:20 +0000 Subject: [PATCH 38/39] refactor: extract while loop logic into separate variable --- helix-core/src/surround.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index 0fa9935ad402..c62c9a693c28 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -432,9 +432,12 @@ fn find_nth_nearest_tag( /// the maximum length of chars we will search forward and backward to find the tags, provided we don't hit the end or the beginning of the document const SEARCH_CHARS: usize = 2000; - while (previous_forward_pos - cursor_pos) < SEARCH_CHARS - && previous_forward_pos < forward_text.len_chars() - { + while { + let is_within_offset = previous_forward_pos - cursor_pos < SEARCH_CHARS; + let is_within_bounds = previous_forward_pos < forward_text.len_chars(); + + is_within_offset && is_within_bounds + } { match find_next_tag(forward_text, previous_forward_pos, skip) { Ok((forward_tag_range, forward_tag_name, forward_search_idx)) => { forward_tags.push((forward_tag_range, forward_tag_name)); @@ -454,7 +457,12 @@ fn find_nth_nearest_tag( let mut backward_tags = vec![]; let mut previous_backward_pos = cursor_pos; - while (cursor_pos - previous_backward_pos) < SEARCH_CHARS && previous_backward_pos > 0 { + while { + let is_within_offset = cursor_pos - previous_backward_pos < SEARCH_CHARS; + let is_within_bounds = previous_backward_pos > 0; + + is_within_offset && is_within_bounds + } { match find_prev_tag(backward_text, previous_backward_pos, skip) { Ok((backward_tag_range, backward_tag_name, backward_search_idx)) => { backward_tags.push((backward_tag_range, backward_tag_name)); @@ -480,6 +488,7 @@ fn find_nth_nearest_tag( .any(|(_, forward_tag_name)| backward_tag_name == forward_tag_name) }) .collect(); + let forward_tags: Vec<(Range, String)> = forward_tags .into_iter() .filter(|(_, forward_tag_name)| { @@ -516,7 +525,7 @@ fn find_nth_nearest_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. +/// Use get_surround_pos_tag().chunks(2) to get matching pairs of surround positions. pub fn get_surround_pos_tag( text: RopeSlice, selection: &Selection, @@ -533,9 +542,10 @@ pub fn get_surround_pos_tag( change_pos.push((next_tag, tag_name)); } - // sort all ranges by the first key + // sort all ranges by their beginning change_pos.sort_by(|&(a, _), (b, _)| a.from().cmp(&b.from())); + // if the end of any range exceeds beginning of the next range, there is an overlap let has_overlaps = change_pos .windows(2) .any(|window| window[0].0.to() > window[1].0.from()); From dd3251da466439deef54a8cb5241709f9b9c3b75 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+nikitarevenco@users.noreply.github.com> Date: Thu, 14 Nov 2024 16:05:29 +0000 Subject: [PATCH 39/39] perf: use HashSet to filter out tags which are not in both collections --- helix-core/src/surround.rs | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index c62c9a693c28..813e42b37c48 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -1,4 +1,4 @@ -use std::fmt::Display; +use std::{collections::HashSet, fmt::Display}; use crate::{ graphemes::next_grapheme_boundary, @@ -479,23 +479,22 @@ fn find_nth_nearest_tag( } } - // only consider the tags which are common in both vectors - let backward_tags: Vec<(Range, String)> = backward_tags + // only consider the tags which are in both collections. + let backward_tag_names: HashSet<_> = backward_tags.iter().map(|(_, tag)| tag.clone()).collect(); + let forward_tag_names: HashSet<_> = forward_tags.iter().map(|(_, tag)| tag.clone()).collect(); + + let common_tags: HashSet<_> = backward_tag_names + .intersection(&forward_tag_names) + .collect(); + + let backward_tags: Vec<_> = backward_tags .into_iter() - .filter(|(_, backward_tag_name)| { - forward_tags - .iter() - .any(|(_, forward_tag_name)| backward_tag_name == forward_tag_name) - }) + .filter(|(_, tag)| common_tags.contains(tag)) .collect(); - let forward_tags: Vec<(Range, String)> = forward_tags + let forward_tags: Vec<_> = forward_tags .into_iter() - .filter(|(_, forward_tag_name)| { - backward_tags - .iter() - .any(|(_, backward_tag_name)| forward_tag_name == backward_tag_name) - }) + .filter(|(_, tag)| common_tags.contains(tag)) .collect(); // improperly ordered tags such as
are ignored completely