98ef05d · helix · git.iko.soy

git.iko.soy/helix

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>

commit98ef05d768d287fef2eb790e0a8a6e9a72832e00 authorMichael Davis <mcarsondavis@gmail.com> dateTue 25 Jul 2023, 13:15 (3 years ago) parentf0b877e

7 files changed+45−37

compare with master

unifiedsplit

helix-core/src/history.rs+2−2 helix-core/src/syntax.rs+16−13 helix-core/src/transaction.rs+7−6 helix-core/tests/indent.rs+2−2 helix-term/src/ui/markdown.rs+4−4 helix-term/src/ui/picker.rs+3−3 helix-view/src/document.rs+11−7

helix-core/src/history.rs+2−2

@@ -72,8 +72,8 @@ impl Default for History {

7272 revisions: vec![Revision {

7373 parent: 0,

7474 last_child: None,

75− transaction: Transaction::from(ChangeSet::new(&Rope::new())),

76− inversion: Transaction::from(ChangeSet::new(&Rope::new())),

75+ transaction: Transaction::from(ChangeSet::new("".into())),

76+ inversion: Transaction::from(ChangeSet::new("".into())),

7777 timestamp: Instant::now(),

7878 }],

7979 current: 0,

helix-core/src/syntax.rs+16−13

@@ -4,7 +4,7 @@ use crate::{

44 diagnostic::Severity,

55 regex::Regex,

66 transaction::{ChangeSet, Operation},

7− Rope, RopeSlice, Tendril,

7+ RopeSlice, Tendril,

88};

1010use ahash::RandomState;

@@ -818,7 +818,10 @@ impl Loader {

818818 // TODO: content_regex handling conflict resolution

819819 }

820820

821− pub fn language_config_for_shebang(&self, source: &Rope) -> Option<Arc<LanguageConfiguration>> {

821+ pub fn language_config_for_shebang(

822+ &self,

823+ source: RopeSlice,

824+ ) -> Option<Arc<LanguageConfiguration>> {

822825 let line = Cow::from(source.line(0));

823826 static SHEBANG_REGEX: Lazy<Regex> =

824827 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

928931

929932impl Syntax {

930933 pub fn new(

931− source: &Rope,

934+ source: RopeSlice,

932935 config: Arc<HighlightConfiguration>,

933936 loader: Arc<Loader>,

934937 ) -> Option<Self> {

@@ -967,8 +970,8 @@ impl Syntax {

967970

968971 pub fn update(

969972 &mut self,

970− old_source: &Rope,

971− source: &Rope,

973+ old_source: RopeSlice,

974+ source: RopeSlice,

972975 changeset: &ChangeSet,

973976 ) -> Result<(), Error> {

974977 let mut queue = VecDeque::new();

@@ -1387,7 +1390,7 @@ impl LanguageLayer {

13871390 self.tree.as_ref().unwrap()

13881391 }

13891392

1390− fn parse(&mut self, parser: &mut Parser, source: &Rope) -> Result<(), Error> {

1393+ fn parse(&mut self, parser: &mut Parser, source: RopeSlice) -> Result<(), Error> {

13911394 parser

13921395 .set_included_ranges(&self.ranges)

13931396 .map_err(|_| Error::InvalidRanges)?;

@@ -1418,7 +1421,7 @@ impl LanguageLayer {

14181421}

14191422

14201423pub(crate) fn generate_edits(

1421− old_text: &Rope,

1424+ old_text: RopeSlice,

14221425 changeset: &ChangeSet,

14231426) -> Vec<tree_sitter::InputEdit> {

14241427 use Operation::*;

@@ -1434,7 +1437,7 @@ pub(crate) fn generate_edits(

14341437

14351438 // TODO; this is a lot easier with Change instead of Operation.

14361439

1437− fn point_at_pos(text: &Rope, pos: usize) -> (usize, Point) {

1440+ fn point_at_pos(text: RopeSlice, pos: usize) -> (usize, Point) {

14381441 let byte = text.char_to_byte(pos); // <- attempted to index past end

14391442 let line = text.char_to_line(pos);

14401443 let line_start_byte = text.line_to_byte(line);

@@ -2529,7 +2532,7 @@ mod test {

25292532 let mut cursor = QueryCursor::new();

25302533

25312534 let config = HighlightConfiguration::new(language, "", "", "").unwrap();

2532− let syntax = Syntax::new(&source, Arc::new(config), Arc::new(loader)).unwrap();

2535+ let syntax = Syntax::new(source.slice(..), Arc::new(config), Arc::new(loader)).unwrap();

25332536

25342537 let root = syntax.tree().root_node();

25352538 let mut test = |capture, range| {

@@ -2603,7 +2606,7 @@ mod test {

26032606 fn main() {}

26042607 ",

26052608 );

2606− let syntax = Syntax::new(&source, Arc::new(config), Arc::new(loader)).unwrap();

2609+ let syntax = Syntax::new(source.slice(..), Arc::new(config), Arc::new(loader)).unwrap();

26072610 let tree = syntax.tree();

26082611 let root = tree.root_node();

26092612 assert_eq!(root.kind(), "source_file");

@@ -2630,7 +2633,7 @@ mod test {

26302633 &doc,

26312634 vec![(6, 11, Some("test".into())), (12, 17, None)].into_iter(),

26322635 );

2633− let edits = generate_edits(&doc, transaction.changes());

2636+ let edits = generate_edits(doc.slice(..), transaction.changes());

26342637 // transaction.apply(&mut state);

26352638

26362639 assert_eq!(

@@ -2659,7 +2662,7 @@ mod test {

26592662 let mut doc = Rope::from("fn test() {}");

26602663 let transaction =

26612664 Transaction::change(&doc, vec![(8, 8, Some("a: u32".into()))].into_iter());

2662− let edits = generate_edits(&doc, transaction.changes());

2665+ let edits = generate_edits(doc.slice(..), transaction.changes());

26632666 transaction.apply(&mut doc);

26642667

26652668 assert_eq!(doc, "fn test(a: u32) {}");

@@ -2693,7 +2696,7 @@ mod test {

26932696 let language = get_language(language_name).unwrap();

26942697

26952698 let config = HighlightConfiguration::new(language, "", "", "").unwrap();

2696− let syntax = Syntax::new(&source, Arc::new(config), Arc::new(loader)).unwrap();

2699+ let syntax = Syntax::new(source.slice(..), Arc::new(config), Arc::new(loader)).unwrap();

26972700

26982701 let root = syntax

26992702 .tree()

helix-core/src/transaction.rs+7−6

@@ -1,3 +1,4 @@

1+use ropey::RopeSlice;

12use smallvec::SmallVec;

34use crate::{Range, Rope, Selection, Tendril};

@@ -42,7 +43,7 @@ impl ChangeSet {

4243 }

4344

4445 #[must_use]

45− pub fn new(doc: &Rope) -> Self {

46+ pub fn new(doc: RopeSlice) -> Self {

4647 let len = doc.len_chars();

4748 Self {

4849 changes: Vec::new(),

@@ -485,7 +486,7 @@ impl Transaction {

485486 /// Create a new, empty transaction.

486487 pub fn new(doc: &Rope) -> Self {

487488 Self {

488− changes: ChangeSet::new(doc),

489+ changes: ChangeSet::new(doc.slice(..)),

489490 selection: None,

490491 }

491492 }

@@ -946,9 +947,9 @@ mod test {

946947 #[test]

947948 fn combine_with_empty() {

948949 let empty = Rope::from("");

949− let a = ChangeSet::new(&empty);

950+ let a = ChangeSet::new(empty.slice(..));

950951

951− let mut b = ChangeSet::new(&empty);

952+ let mut b = ChangeSet::new(empty.slice(..));

952953 b.insert("a".into());

953954

954955 let changes = a.compose(b);

@@ -962,9 +963,9 @@ mod test {

962963 const TEST_CASE: &str = "Hello, これはヘリックスエディターです！";

963964

964965 let empty = Rope::from("");

965− let a = ChangeSet::new(&empty);

966+ let a = ChangeSet::new(empty.slice(..));

966967

967− let mut b = ChangeSet::new(&empty);

968+ let mut b = ChangeSet::new(empty.slice(..));

968969 b.insert(TEST_CASE.into());

969970

970971 let changes = a.compose(b);

helix-core/tests/indent.rs+2−2

@@ -72,9 +72,9 @@ fn test_treesitter_indent(file_name: &str, lang_scope: &str) {

7272

7373 let language_config = loader.language_config_for_scope(lang_scope).unwrap();

7474 let highlight_config = language_config.highlight_config(&[]).unwrap();

75− let syntax = Syntax::new(&doc, highlight_config, std::sync::Arc::new(loader)).unwrap();

76− let indent_query = language_config.indent_query().unwrap();

7775 let text = doc.slice(..);

76+ let syntax = Syntax::new(text, highlight_config, std::sync::Arc::new(loader)).unwrap();

77+ let indent_query = language_config.indent_query().unwrap();

7878

7979 for i in 0..doc.len_lines() {

8080 let line = text.line(i);

helix-term/src/ui/markdown.rs+4−4

@@ -10,7 +10,7 @@ use pulldown_cmark::{CodeBlockKind, Event, HeadingLevel, Options, Parser, Tag};

1010

1111use helix_core::{

1212 syntax::{self, HighlightEvent, InjectionLanguageMarker, Syntax},

13− Rope,

13+ RopeSlice,

1414};

1515use helix_view::{

1616 graphics::{Margin, Rect, Style},

@@ -45,13 +45,13 @@ pub fn highlighted_code_block<'a>(

4545 None => return styled_multiline_text(text, code_style),

4646 };

4747

48− let rope = Rope::from(text.as_ref());

48+ let ropeslice = RopeSlice::from(text);

4949 let syntax = config_loader

5050 .language_configuration_for_injection_string(&InjectionLanguageMarker::Name(

5151 language.into(),

5252 ))

5353 .and_then(|config| config.highlight_config(theme.scopes()))

54− .and_then(|config| Syntax::new(&rope, config, Arc::clone(&config_loader)));

54+ .and_then(|config| Syntax::new(ropeslice, config, Arc::clone(&config_loader)));

5555

5656 let syntax = match syntax {

5757 Some(s) => s,

@@ -59,7 +59,7 @@ pub fn highlighted_code_block<'a>(

5959 };

6060

6161 let highlight_iter = syntax

62− .highlight_iter(rope.slice(..), None, None)

62+ .highlight_iter(ropeslice, None, None)

6363 .map(|e| e.unwrap());

6464 let highlight_iter: Box<dyn Iterator<Item = HighlightEvent>> =

6565 if let Some(spans) = additional_highlight_spans {

helix-term/src/ui/picker.rs+3−3

@@ -453,9 +453,9 @@ impl<T: Item + 'static> Picker<T> {

453453 let text = doc.text().clone();

454454 let loader = cx.editor.syn_loader.clone();

455455 let job = tokio::task::spawn_blocking(move || {

456− let syntax = language_config

457− .highlight_config(&loader.scopes())

458− .and_then(|highlight_config| Syntax::new(&text, highlight_config, loader));

456+ let syntax = language_config.highlight_config(&loader.scopes()).and_then(

457+ |highlight_config| Syntax::new(text.slice(..), highlight_config, loader),

458+ );

459459 let callback = move |editor: &mut Editor, compositor: &mut Compositor| {

460460 let Some(syntax) = syntax else {

461461 log::info!("highlighting picker item failed");

helix-view/src/document.rs+11−7

@@ -642,7 +642,7 @@ impl Document {

642642 ) -> Self {

643643 let (encoding, has_bom) = encoding_with_bom_info.unwrap_or((encoding::UTF_8, false));

644644 let line_ending = config.load().default_line_ending.into();

645− let changes = ChangeSet::new(&text);

645+ let changes = ChangeSet::new(text.slice(..));

646646 let old_state = None;

647647

648648 Self {

@@ -938,7 +938,7 @@ impl Document {

938938 ) -> Option<Arc<helix_core::syntax::LanguageConfiguration>> {

939939 config_loader

940940 .language_config_for_file_name(self.path.as_ref()?)

941− .or_else(|| config_loader.language_config_for_shebang(self.text()))

941+ .or_else(|| config_loader.language_config_for_shebang(self.text().slice(..)))

942942 }

943943

944944 /// Detect the indentation used in the file, or otherwise defaults to the language indentation

@@ -1030,7 +1030,7 @@ impl Document {

10301030 ) {

10311031 if let (Some(language_config), Some(loader)) = (language_config, loader) {

10321032 if let Some(highlight_config) = language_config.highlight_config(&loader.scopes()) {

1033− self.syntax = Syntax::new(&self.text, highlight_config, loader);

1033+ self.syntax = Syntax::new(self.text.slice(..), highlight_config, loader);

10341034 }

10351035

10361036 self.language = Some(language_config);

@@ -1165,7 +1165,11 @@ impl Document {

11651165 // update tree-sitter syntax tree

11661166 if let Some(syntax) = &mut self.syntax {

11671167 // TODO: no unwrap

1168− let res = syntax.update(&old_doc, &self.text, transaction.changes());

1168+ let res = syntax.update(

1169+ old_doc.slice(..),

1170+ self.text.slice(..),

1171+ transaction.changes(),

1172+ );

11691173 if res.is_err() {

11701174 log::error!("TS parser failed, disabeling TS for the current buffer: {res:?}");

11711175 self.syntax = None;

@@ -1288,7 +1292,7 @@ impl Document {

12881292

12891293 if success {

12901294 // reset changeset to fix len

1291− self.changes = ChangeSet::new(self.text());

1295+ self.changes = ChangeSet::new(self.text().slice(..));

12921296 // Sync with changes with the jumplist selections.

12931297 view.sync_changes(self);

12941298 }

@@ -1371,7 +1375,7 @@ impl Document {

13711375 }

13721376 if success {

13731377 // reset changeset to fix len

1374− self.changes = ChangeSet::new(self.text());

1378+ self.changes = ChangeSet::new(self.text().slice(..));

13751379 // Sync with changes with the jumplist selections.

13761380 view.sync_changes(self);

13771381 }

@@ -1394,7 +1398,7 @@ impl Document {

13941398 return;

13951399 }

13961400

1397− let new_changeset = ChangeSet::new(self.text());

1401+ let new_changeset = ChangeSet::new(self.text().slice(..));

13981402 let changes = std::mem::replace(&mut self.changes, new_changeset);

13991403 // Instead of doing this messy merge we could always commit, and based on transaction

14001404 // annotations either add a new layer or compose into the previous one.