Mirrors
/
helix
mirror of https://github.com/helix-editor/helix
1
1
Fork 1
Code Issues Packages Projects Releases Wiki Activity

Prefer RopeSlice to &Rope in helix_core::syntax

Browse Source 
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>
pull/7764/head
Michael Davis 1 year ago committed by Blaž Hrastnik
parent f0b877e258
commit 98ef05d768
7 changed files with 45 additions and 37 deletions
Show Stats Download Patch File Download Diff File

4
helix-core/src/history.rs
Unescape Escape View File

@ -72,8 +72,8 @@ impl Default for History {
revisions: vec![Revision { revisions: vec![Revision {
parent: 0, parent: 0,
last_child: None, last_child: None,
transaction: Transaction::from(ChangeSet::new(&Rope::new())), transaction: Transaction::from(ChangeSet::new("".into())),
inversion: Transaction::from(ChangeSet::new(&Rope::new())), inversion: Transaction::from(ChangeSet::new("".into())),
timestamp: Instant::now(), timestamp: Instant::now(),
}], }],
current: 0, current: 0,

29
helix-core/src/syntax.rs
Unescape Escape View File

@ -4,7 +4,7 @@ use crate::{
diagnostic::Severity, diagnostic::Severity,
regex::Regex, regex::Regex,
transaction::{ChangeSet, Operation}, transaction::{ChangeSet, Operation},
Rope, RopeSlice, Tendril, RopeSlice, Tendril,
}; };
use ahash::RandomState; use ahash::RandomState;
@ -818,7 +818,10 @@ impl Loader {
// TODO: content_regex handling conflict resolution // 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)); let line = Cow::from(source.line(0));
static SHEBANG_REGEX: Lazy<Regex> = static SHEBANG_REGEX: Lazy<Regex> =
Lazy::new(|| Regex::new(&["^", SHEBANG].concat()).unwrap()); 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 { impl Syntax {
pub fn new( pub fn new(
source: &Rope, source: RopeSlice,
config: Arc<HighlightConfiguration>, config: Arc<HighlightConfiguration>,
loader: Arc<Loader>, loader: Arc<Loader>,
) -> Option<Self> { ) -> Option<Self> {
@ -967,8 +970,8 @@ impl Syntax {
pub fn update( pub fn update(
&mut self, &mut self,
old_source: &Rope, old_source: RopeSlice,
source: &Rope, source: RopeSlice,
changeset: &ChangeSet, changeset: &ChangeSet,
) -> Result<(), Error> { ) -> Result<(), Error> {
let mut queue = VecDeque::new(); let mut queue = VecDeque::new();
@ -1387,7 +1390,7 @@ impl LanguageLayer {
self.tree.as_ref().unwrap() 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 parser
.set_included_ranges(&self.ranges) .set_included_ranges(&self.ranges)
.map_err(|_| Error::InvalidRanges)?; .map_err(|_| Error::InvalidRanges)?;
@ -1418,7 +1421,7 @@ impl LanguageLayer {
} }
pub(crate) fn generate_edits( pub(crate) fn generate_edits(
old_text: &Rope, old_text: RopeSlice,
changeset: &ChangeSet, changeset: &ChangeSet,
) -> Vec<tree_sitter::InputEdit> { ) -> Vec<tree_sitter::InputEdit> {
use Operation::*; use Operation::*;
@ -1434,7 +1437,7 @@ pub(crate) fn generate_edits(
// TODO; this is a lot easier with Change instead of Operation. // 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 byte = text.char_to_byte(pos); // <- attempted to index past end
let line = text.char_to_line(pos); let line = text.char_to_line(pos);
let line_start_byte = text.line_to_byte(line); let line_start_byte = text.line_to_byte(line);
@ -2529,7 +2532,7 @@ mod test {
let mut cursor = QueryCursor::new(); let mut cursor = QueryCursor::new();
let config = HighlightConfiguration::new(language, "", "", "").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().root_node(); let root = syntax.tree().root_node();
let mut test = |capture, range| { let mut test = |capture, range| {
@ -2603,7 +2606,7 @@ mod test {
fn main() {} 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 tree = syntax.tree();
let root = tree.root_node(); let root = tree.root_node();
assert_eq!(root.kind(), "source_file"); assert_eq!(root.kind(), "source_file");
@ -2630,7 +2633,7 @@ mod test {
&doc, &doc,
vec![(6, 11, Some("test".into())), (12, 17, None)].into_iter(), 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); // transaction.apply(&mut state);
assert_eq!( assert_eq!(
@ -2659,7 +2662,7 @@ mod test {
let mut doc = Rope::from("fn test() {}"); let mut doc = Rope::from("fn test() {}");
let transaction = let transaction =
Transaction::change(&doc, vec![(8, 8, Some("a: u32".into()))].into_iter()); 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); transaction.apply(&mut doc);
assert_eq!(doc, "fn test(a: u32) {}"); assert_eq!(doc, "fn test(a: u32) {}");
@ -2693,7 +2696,7 @@ mod test {
let language = get_language(language_name).unwrap(); let language = get_language(language_name).unwrap();
let config = HighlightConfiguration::new(language, "", "", "").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 let root = syntax
.tree() .tree()

13
helix-core/src/transaction.rs
Unescape Escape View File

@ -1,3 +1,4 @@
use ropey::RopeSlice;
use smallvec::SmallVec; use smallvec::SmallVec;
use crate::{Range, Rope, Selection, Tendril}; use crate::{Range, Rope, Selection, Tendril};
@ -42,7 +43,7 @@ impl ChangeSet {
} }
#[must_use] #[must_use]
pub fn new(doc: &Rope) -> Self { pub fn new(doc: RopeSlice) -> Self {
let len = doc.len_chars(); let len = doc.len_chars();
Self { Self {
changes: Vec::new(), changes: Vec::new(),
@ -485,7 +486,7 @@ impl Transaction {
/// Create a new, empty transaction. /// Create a new, empty transaction.
pub fn new(doc: &Rope) -> Self { pub fn new(doc: &Rope) -> Self {
Self { Self {
changes: ChangeSet::new(doc), changes: ChangeSet::new(doc.slice(..)),
selection: None, selection: None,
} }
} }
@ -946,9 +947,9 @@ mod test {
#[test] #[test]
fn combine_with_empty() { fn combine_with_empty() {
let empty = Rope::from(""); 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()); b.insert("a".into());
let changes = a.compose(b); let changes = a.compose(b);
@ -962,9 +963,9 @@ mod test {
const TEST_CASE: &str = "Hello, これはヘリックスエディターです！"; const TEST_CASE: &str = "Hello, これはヘリックスエディターです！";
let empty = Rope::from(""); 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()); b.insert(TEST_CASE.into());
let changes = a.compose(b); let changes = a.compose(b);

4
helix-core/tests/indent.rs
Unescape Escape View File

@ -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 language_config = loader.language_config_for_scope(lang_scope).unwrap();
let highlight_config = language_config.highlight_config(&[]).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 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() { for i in 0..doc.len_lines() {
let line = text.line(i); let line = text.line(i);

8
helix-term/src/ui/markdown.rs
Unescape Escape View File

@ -10,7 +10,7 @@ use pulldown_cmark::{CodeBlockKind, Event, HeadingLevel, Options, Parser, Tag};
use helix_core::{ use helix_core::{
syntax::{self, HighlightEvent, InjectionLanguageMarker, Syntax}, syntax::{self, HighlightEvent, InjectionLanguageMarker, Syntax},
Rope, RopeSlice,
}; };
use helix_view::{ use helix_view::{
graphics::{Margin, Rect, Style}, graphics::{Margin, Rect, Style},
@ -45,13 +45,13 @@ pub fn highlighted_code_block<'a>(
None => return styled_multiline_text(text, code_style), None => return styled_multiline_text(text, code_style),
}; };
let rope = Rope::from(text.as_ref()); let ropeslice = RopeSlice::from(text);
let syntax = config_loader let syntax = config_loader
.language_configuration_for_injection_string(&InjectionLanguageMarker::Name( .language_configuration_for_injection_string(&InjectionLanguageMarker::Name(
language.into(), language.into(),
)) ))
.and_then(|config| config.highlight_config(theme.scopes())) .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 { let syntax = match syntax {
Some(s) => s, Some(s) => s,
@ -59,7 +59,7 @@ pub fn highlighted_code_block<'a>(
}; };
let highlight_iter = syntax let highlight_iter = syntax
.highlight_iter(rope.slice(..), None, None) .highlight_iter(ropeslice, None, None)
.map(|e| e.unwrap()); .map(|e| e.unwrap());
let highlight_iter: Box<dyn Iterator<Item = HighlightEvent>> = let highlight_iter: Box<dyn Iterator<Item = HighlightEvent>> =
if let Some(spans) = additional_highlight_spans { if let Some(spans) = additional_highlight_spans {

6
helix-term/src/ui/picker.rs
Unescape Escape View File

@ -453,9 +453,9 @@ impl<T: Item + 'static> Picker<T> {
let text = doc.text().clone(); let text = doc.text().clone();
let loader = cx.editor.syn_loader.clone(); let loader = cx.editor.syn_loader.clone();
let job = tokio::task::spawn_blocking(move || { let job = tokio::task::spawn_blocking(move || {
let syntax = language_config let syntax = language_config.highlight_config(&loader.scopes()).and_then(
.highlight_config(&loader.scopes()) |highlight_config| Syntax::new(text.slice(..), highlight_config, loader),
.and_then(|highlight_config| Syntax::new(&text, highlight_config, loader)); );
let callback = move |editor: &mut Editor, compositor: &mut Compositor| { let callback = move |editor: &mut Editor, compositor: &mut Compositor| {
let Some(syntax) = syntax else { let Some(syntax) = syntax else {
log::info!("highlighting picker item failed"); log::info!("highlighting picker item failed");

18
helix-view/src/document.rs
Unescape Escape View File

@ -642,7 +642,7 @@ impl Document {
) -> Self { ) -> Self {
let (encoding, has_bom) = encoding_with_bom_info.unwrap_or((encoding::UTF_8, false)); let (encoding, has_bom) = encoding_with_bom_info.unwrap_or((encoding::UTF_8, false));
let line_ending = config.load().default_line_ending.into(); let line_ending = config.load().default_line_ending.into();
let changes = ChangeSet::new(&text); let changes = ChangeSet::new(text.slice(..));
let old_state = None; let old_state = None;
Self { Self {
@ -938,7 +938,7 @@ impl Document {
) -> Option<Arc<helix_core::syntax::LanguageConfiguration>> { ) -> Option<Arc<helix_core::syntax::LanguageConfiguration>> {
config_loader config_loader
.language_config_for_file_name(self.path.as_ref()?) .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 /// 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(language_config), Some(loader)) = (language_config, loader) {
if let Some(highlight_config) = language_config.highlight_config(&loader.scopes()) { 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); self.language = Some(language_config);
@ -1165,7 +1165,11 @@ impl Document {
// update tree-sitter syntax tree // update tree-sitter syntax tree
if let Some(syntax) = &mut self.syntax { if let Some(syntax) = &mut self.syntax {
// TODO: no unwrap // 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() { if res.is_err() {
log::error!("TS parser failed, disabeling TS for the current buffer: {res:?}"); log::error!("TS parser failed, disabeling TS for the current buffer: {res:?}");
self.syntax = None; self.syntax = None;
@ -1288,7 +1292,7 @@ impl Document {
if success { if success {
// reset changeset to fix len // 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. // Sync with changes with the jumplist selections.
view.sync_changes(self); view.sync_changes(self);
} }
@ -1371,7 +1375,7 @@ impl Document {
} }
if success { if success {
// reset changeset to fix len // 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. // Sync with changes with the jumplist selections.
view.sync_changes(self); view.sync_changes(self);
} }
@ -1394,7 +1398,7 @@ impl Document {
return; 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); let changes = std::mem::replace(&mut self.changes, new_changeset);
// Instead of doing this messy merge we could always commit, and based on transaction // 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. // annotations either add a new layer or compose into the previous one.

Write Preview
Loading…
Cancel
Save