diff options
author | Michael Davis | 2023-07-25 18:15:36 +0000 |
---|---|---|
committer | Blaž Hrastnik | 2023-07-27 02:50:19 +0000 |
commit | 98ef05d768d287fef2eb790e0a8a6e9a72832e00 (patch) | |
tree | 856fa79ea0924834f786a22a76ddfbe188301330 | |
parent | f0b877e2588306882f71eaad45a3f3e604885a34 (diff) |
Prefer RopeSlice to &Rope in helix_core::syntax
Pascal and I discussed this and we think it's generally better to
take a 'RopeSlice' rather than a '&Rope'. The code block rendering
function in the markdown component module is a good example for how
this can be useful: we can remove an allocation of a rope and instead
directly turn a '&str' into a 'RopeSlice' which is very cheap.
A change to prefer 'RopeSlice' to '&Rope' whenever the rope isn't
modified would be nice, but it would be a very large diff (around 500+
500-). Starting off with just the syntax functions seems like a nice
middle-ground, and we can remove a Rope allocation because of it.
Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>
-rw-r--r-- | helix-core/src/history.rs | 4 | ||||
-rw-r--r-- | helix-core/src/syntax.rs | 29 | ||||
-rw-r--r-- | helix-core/src/transaction.rs | 13 | ||||
-rw-r--r-- | helix-core/tests/indent.rs | 4 | ||||
-rw-r--r-- | helix-term/src/ui/markdown.rs | 8 | ||||
-rw-r--r-- | helix-term/src/ui/picker.rs | 6 | ||||
-rw-r--r-- | helix-view/src/document.rs | 18 |
7 files changed, 45 insertions, 37 deletions
diff --git a/helix-core/src/history.rs b/helix-core/src/history.rs index 1aac38d9..28d6dd6e 100644 --- a/helix-core/src/history.rs +++ b/helix-core/src/history.rs @@ -72,8 +72,8 @@ impl Default for History { revisions: vec![Revision { parent: 0, last_child: None, - transaction: Transaction::from(ChangeSet::new(&Rope::new())), - inversion: Transaction::from(ChangeSet::new(&Rope::new())), + transaction: Transaction::from(ChangeSet::new("".into())), + inversion: Transaction::from(ChangeSet::new("".into())), timestamp: Instant::now(), }], current: 0, diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index 73d3f738..79fb52a7 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -4,7 +4,7 @@ use crate::{ diagnostic::Severity, regex::Regex, transaction::{ChangeSet, Operation}, - Rope, RopeSlice, Tendril, + RopeSlice, Tendril, }; use ahash::RandomState; @@ -818,7 +818,10 @@ impl Loader { // TODO: content_regex handling conflict resolution } - pub fn language_config_for_shebang(&self, source: &Rope) -> Option<Arc<LanguageConfiguration>> { + pub fn language_config_for_shebang( + &self, + source: RopeSlice, + ) -> Option<Arc<LanguageConfiguration>> { let line = Cow::from(source.line(0)); static SHEBANG_REGEX: Lazy<Regex> = Lazy::new(|| Regex::new(&["^", SHEBANG].concat()).unwrap()); @@ -928,7 +931,7 @@ fn byte_range_to_str(range: std::ops::Range<usize>, source: RopeSlice) -> Cow<st impl Syntax { pub fn new( - source: &Rope, + source: RopeSlice, config: Arc<HighlightConfiguration>, loader: Arc<Loader>, ) -> Option<Self> { @@ -967,8 +970,8 @@ impl Syntax { pub fn update( &mut self, - old_source: &Rope, - source: &Rope, + old_source: RopeSlice, + source: RopeSlice, changeset: &ChangeSet, ) -> Result<(), Error> { let mut queue = VecDeque::new(); @@ -1387,7 +1390,7 @@ impl LanguageLayer { self.tree.as_ref().unwrap() } - fn parse(&mut self, parser: &mut Parser, source: &Rope) -> Result<(), Error> { + fn parse(&mut self, parser: &mut Parser, source: RopeSlice) -> Result<(), Error> { parser .set_included_ranges(&self.ranges) .map_err(|_| Error::InvalidRanges)?; @@ -1418,7 +1421,7 @@ impl LanguageLayer { } pub(crate) fn generate_edits( - old_text: &Rope, + old_text: RopeSlice, changeset: &ChangeSet, ) -> Vec<tree_sitter::InputEdit> { use Operation::*; @@ -1434,7 +1437,7 @@ pub(crate) fn generate_edits( // TODO; this is a lot easier with Change instead of Operation. - fn point_at_pos(text: &Rope, pos: usize) -> (usize, Point) { + fn point_at_pos(text: RopeSlice, pos: usize) -> (usize, Point) { let byte = text.char_to_byte(pos); // <- attempted to index past end let line = text.char_to_line(pos); let line_start_byte = text.line_to_byte(line); @@ -2529,7 +2532,7 @@ mod test { let mut cursor = QueryCursor::new(); let config = HighlightConfiguration::new(language, "", "", "").unwrap(); - let syntax = Syntax::new(&source, Arc::new(config), Arc::new(loader)).unwrap(); + let syntax = Syntax::new(source.slice(..), Arc::new(config), Arc::new(loader)).unwrap(); let root = syntax.tree().root_node(); let mut test = |capture, range| { @@ -2603,7 +2606,7 @@ mod test { fn main() {} ", ); - let syntax = Syntax::new(&source, Arc::new(config), Arc::new(loader)).unwrap(); + let syntax = Syntax::new(source.slice(..), Arc::new(config), Arc::new(loader)).unwrap(); let tree = syntax.tree(); let root = tree.root_node(); assert_eq!(root.kind(), "source_file"); @@ -2630,7 +2633,7 @@ mod test { &doc, vec![(6, 11, Some("test".into())), (12, 17, None)].into_iter(), ); - let edits = generate_edits(&doc, transaction.changes()); + let edits = generate_edits(doc.slice(..), transaction.changes()); // transaction.apply(&mut state); assert_eq!( @@ -2659,7 +2662,7 @@ mod test { let mut doc = Rope::from("fn test() {}"); let transaction = Transaction::change(&doc, vec![(8, 8, Some("a: u32".into()))].into_iter()); - let edits = generate_edits(&doc, transaction.changes()); + let edits = generate_edits(doc.slice(..), transaction.changes()); transaction.apply(&mut doc); assert_eq!(doc, "fn test(a: u32) {}"); @@ -2693,7 +2696,7 @@ mod test { let language = get_language(language_name).unwrap(); let config = HighlightConfiguration::new(language, "", "", "").unwrap(); - let syntax = Syntax::new(&source, Arc::new(config), Arc::new(loader)).unwrap(); + let syntax = Syntax::new(source.slice(..), Arc::new(config), Arc::new(loader)).unwrap(); let root = syntax .tree() diff --git a/helix-core/src/transaction.rs b/helix-core/src/transaction.rs index 9d2a3e5c..fec62578 100644 --- a/helix-core/src/transaction.rs +++ b/helix-core/src/transaction.rs @@ -1,3 +1,4 @@ +use ropey::RopeSlice; use smallvec::SmallVec; use crate::{Range, Rope, Selection, Tendril}; @@ -42,7 +43,7 @@ impl ChangeSet { } #[must_use] - pub fn new(doc: &Rope) -> Self { + pub fn new(doc: RopeSlice) -> Self { let len = doc.len_chars(); Self { changes: Vec::new(), @@ -485,7 +486,7 @@ impl Transaction { /// Create a new, empty transaction. pub fn new(doc: &Rope) -> Self { Self { - changes: ChangeSet::new(doc), + changes: ChangeSet::new(doc.slice(..)), selection: None, } } @@ -946,9 +947,9 @@ mod test { #[test] fn combine_with_empty() { let empty = Rope::from(""); - let a = ChangeSet::new(&empty); + let a = ChangeSet::new(empty.slice(..)); - let mut b = ChangeSet::new(&empty); + let mut b = ChangeSet::new(empty.slice(..)); b.insert("a".into()); let changes = a.compose(b); @@ -962,9 +963,9 @@ mod test { const TEST_CASE: &str = "Hello, これはヘリックスエディターです!"; let empty = Rope::from(""); - let a = ChangeSet::new(&empty); + let a = ChangeSet::new(empty.slice(..)); - let mut b = ChangeSet::new(&empty); + let mut b = ChangeSet::new(empty.slice(..)); b.insert(TEST_CASE.into()); let changes = a.compose(b); diff --git a/helix-core/tests/indent.rs b/helix-core/tests/indent.rs index 409706bb..1dec9989 100644 --- a/helix-core/tests/indent.rs +++ b/helix-core/tests/indent.rs @@ -72,9 +72,9 @@ fn test_treesitter_indent(file_name: &str, lang_scope: &str) { let language_config = loader.language_config_for_scope(lang_scope).unwrap(); let highlight_config = language_config.highlight_config(&[]).unwrap(); - let syntax = Syntax::new(&doc, highlight_config, std::sync::Arc::new(loader)).unwrap(); - let indent_query = language_config.indent_query().unwrap(); let text = doc.slice(..); + let syntax = Syntax::new(text, highlight_config, std::sync::Arc::new(loader)).unwrap(); + let indent_query = language_config.indent_query().unwrap(); for i in 0..doc.len_lines() { let line = text.line(i); diff --git a/helix-term/src/ui/markdown.rs b/helix-term/src/ui/markdown.rs index def64434..cb2abf68 100644 --- a/helix-term/src/ui/markdown.rs +++ b/helix-term/src/ui/markdown.rs @@ -10,7 +10,7 @@ use pulldown_cmark::{CodeBlockKind, Event, HeadingLevel, Options, Parser, Tag}; use helix_core::{ syntax::{self, HighlightEvent, InjectionLanguageMarker, Syntax}, - Rope, + RopeSlice, }; use helix_view::{ graphics::{Margin, Rect, Style}, @@ -45,13 +45,13 @@ pub fn highlighted_code_block<'a>( None => return styled_multiline_text(text, code_style), }; - let rope = Rope::from(text.as_ref()); + let ropeslice = RopeSlice::from(text); let syntax = config_loader .language_configuration_for_injection_string(&InjectionLanguageMarker::Name( language.into(), )) .and_then(|config| config.highlight_config(theme.scopes())) - .and_then(|config| Syntax::new(&rope, config, Arc::clone(&config_loader))); + .and_then(|config| Syntax::new(ropeslice, config, Arc::clone(&config_loader))); let syntax = match syntax { Some(s) => s, @@ -59,7 +59,7 @@ pub fn highlighted_code_block<'a>( }; let highlight_iter = syntax - .highlight_iter(rope.slice(..), None, None) + .highlight_iter(ropeslice, None, None) .map(|e| e.unwrap()); let highlight_iter: Box<dyn Iterator<Item = HighlightEvent>> = if let Some(spans) = additional_highlight_spans { diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index 13746cfc..4605e2f1 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -453,9 +453,9 @@ impl<T: Item + 'static> Picker<T> { let text = doc.text().clone(); let loader = cx.editor.syn_loader.clone(); let job = tokio::task::spawn_blocking(move || { - let syntax = language_config - .highlight_config(&loader.scopes()) - .and_then(|highlight_config| Syntax::new(&text, highlight_config, loader)); + let syntax = language_config.highlight_config(&loader.scopes()).and_then( + |highlight_config| Syntax::new(text.slice(..), highlight_config, loader), + ); let callback = move |editor: &mut Editor, compositor: &mut Compositor| { let Some(syntax) = syntax else { log::info!("highlighting picker item failed"); diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index b08370f9..af7e3a7e 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -642,7 +642,7 @@ impl Document { ) -> Self { let (encoding, has_bom) = encoding_with_bom_info.unwrap_or((encoding::UTF_8, false)); let line_ending = config.load().default_line_ending.into(); - let changes = ChangeSet::new(&text); + let changes = ChangeSet::new(text.slice(..)); let old_state = None; Self { @@ -938,7 +938,7 @@ impl Document { ) -> Option<Arc<helix_core::syntax::LanguageConfiguration>> { config_loader .language_config_for_file_name(self.path.as_ref()?) - .or_else(|| config_loader.language_config_for_shebang(self.text())) + .or_else(|| config_loader.language_config_for_shebang(self.text().slice(..))) } /// Detect the indentation used in the file, or otherwise defaults to the language indentation @@ -1030,7 +1030,7 @@ impl Document { ) { if let (Some(language_config), Some(loader)) = (language_config, loader) { if let Some(highlight_config) = language_config.highlight_config(&loader.scopes()) { - self.syntax = Syntax::new(&self.text, highlight_config, loader); + self.syntax = Syntax::new(self.text.slice(..), highlight_config, loader); } self.language = Some(language_config); @@ -1165,7 +1165,11 @@ impl Document { // update tree-sitter syntax tree if let Some(syntax) = &mut self.syntax { // TODO: no unwrap - let res = syntax.update(&old_doc, &self.text, transaction.changes()); + let res = syntax.update( + old_doc.slice(..), + self.text.slice(..), + transaction.changes(), + ); if res.is_err() { log::error!("TS parser failed, disabeling TS for the current buffer: {res:?}"); self.syntax = None; @@ -1288,7 +1292,7 @@ impl Document { if success { // reset changeset to fix len - self.changes = ChangeSet::new(self.text()); + self.changes = ChangeSet::new(self.text().slice(..)); // Sync with changes with the jumplist selections. view.sync_changes(self); } @@ -1371,7 +1375,7 @@ impl Document { } if success { // reset changeset to fix len - self.changes = ChangeSet::new(self.text()); + self.changes = ChangeSet::new(self.text().slice(..)); // Sync with changes with the jumplist selections. view.sync_changes(self); } @@ -1394,7 +1398,7 @@ impl Document { return; } - let new_changeset = ChangeSet::new(self.text()); + let new_changeset = ChangeSet::new(self.text().slice(..)); let changes = std::mem::replace(&mut self.changes, new_changeset); // Instead of doing this messy merge we could always commit, and based on transaction // annotations either add a new layer or compose into the previous one. |