From 1f2471fe0f969b72f306eb81d19789c8d7d6c6ee Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Tue, 25 Jul 2023 14:20:33 -0500 Subject: [PATCH 1/4] Syntax-highlight regex prompts We can use tree-sitter-regex highlighting in prompts for entering regexes, like `search` or `global_search`. The `highlighted_code_block` function from the markdown component makes this a very small change. This could be improved in the future by leaving the parsed syntax tree on the prompt, allowing incremental updates. Prompt lines are usually so short though and tree-sitter-regex is rather small and uncomplicated, so that improvement probably wouldn't make a big difference. --- helix-term/src/ui/mod.rs | 3 ++- helix-term/src/ui/prompt.rs | 45 +++++++++++++++++++++---------------- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index 3359155d5399..8ca1b131fe8b 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -151,7 +151,8 @@ pub fn regex_prompt( } } }, - ); + ) + .with_language("regex", std::sync::Arc::clone(&cx.editor.syn_loader)); // Calculate initial completion prompt.recalculate_completion(cx.editor); // prompt diff --git a/helix-term/src/ui/prompt.rs b/helix-term/src/ui/prompt.rs index 4f383bf56e61..f376b9c9cb3d 100644 --- a/helix-term/src/ui/prompt.rs +++ b/helix-term/src/ui/prompt.rs @@ -1,7 +1,9 @@ use crate::compositor::{Component, Compositor, Context, Event, EventResult}; use crate::{alt, ctrl, key, shift, ui}; +use helix_core::syntax; use helix_view::input::KeyEvent; use helix_view::keyboard::KeyCode; +use std::sync::Arc; use std::{borrow::Cow, ops::RangeFrom}; use tui::buffer::Buffer as Surface; use tui::widgets::{Block, Borders, Widget}; @@ -32,6 +34,7 @@ pub struct Prompt { callback_fn: CallbackFn, pub doc_fn: DocFn, next_char_handler: Option, + language: Option<(&'static str, Arc)>, } #[derive(Clone, Copy, PartialEq, Eq)] @@ -83,6 +86,7 @@ impl Prompt { callback_fn: Box::new(callback_fn), doc_fn: Box::new(|_| None), next_char_handler: None, + language: None, } } @@ -94,6 +98,11 @@ impl Prompt { self } + pub fn with_language(mut self, language: &'static str, loader: Arc) -> Self { + self.language = Some((language, loader)); + self + } + pub fn line(&self) -> &String { &self.line } @@ -456,30 +465,28 @@ impl Prompt { // render buffer text surface.set_string(area.x, area.y + line, &self.prompt, prompt_color); - let (input, is_suggestion): (Cow, bool) = if self.line.is_empty() { - // latest value in the register list - match self + let line_area = area.clip_left(self.prompt.len() as u16).clip_top(line); + if self.line.is_empty() { + // Show the most recently entered value as a suggestion. + if let Some(suggestion) = self .history_register .and_then(|reg| cx.editor.registers.last(reg)) - .map(|entry| entry.into()) { - Some(value) => (value, true), - None => (Cow::from(""), false), + surface.set_string(line_area.x, line_area.y, suggestion, suggestion_color); } + } else if let Some((language, loader)) = self.language.as_ref() { + let mut text: ui::text::Text = crate::ui::markdown::highlighted_code_block( + self.line.clone(), + language, + Some(&cx.editor.theme), + loader.clone(), + None, + ) + .into(); + text.render(line_area, surface, cx); } else { - (self.line.as_str().into(), false) - }; - - surface.set_string( - area.x + self.prompt.len() as u16, - area.y + line, - &input, - if is_suggestion { - suggestion_color - } else { - prompt_color - }, - ); + surface.set_string(line_area.x, line_area.y, self.line.clone(), prompt_color); + } } } From 869a17d7057ca57dd946f0c89d7f33e588fe48f5 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Tue, 25 Jul 2023 16:11:11 -0500 Subject: [PATCH 2/4] Tune regex highlights for usage in prompts Since regex is almost always injected into other languages, `pattern_character`s will inherit the highlight for the structure that injects them (for example `/foo/` in JavaScript or `~r/foo/` in Elixir). This removes the string highlight when used in the prompt. We also add `ERROR` node highlighting so that errors in regex syntax appear in the prompt. This resolves a TODO in the `regex_prompt` function about highlighting errors in the regex. --- helix-term/src/ui/mod.rs | 3 --- runtime/queries/regex/highlights.scm | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index 8ca1b131fe8b..215794e3c7ef 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -142,9 +142,6 @@ pub fn regex_prompt( }; cx.jobs.callback(callback); - } else { - // Update - // TODO: mark command line as error } } } diff --git a/runtime/queries/regex/highlights.scm b/runtime/queries/regex/highlights.scm index cad08f4092a3..302cc6b3132e 100644 --- a/runtime/queries/regex/highlights.scm +++ b/runtime/queries/regex/highlights.scm @@ -50,4 +50,4 @@ ]) (class_character) @constant.character -(pattern_character) @string +(ERROR) @error From b533be9522efc3c33b2eb011cc91063644f510aa Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Tue, 25 Jul 2023 13:15:36 -0500 Subject: [PATCH 3/4] 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 --- helix-core/src/history.rs | 4 ++-- helix-core/src/syntax.rs | 29 ++++++++++++++++------------- helix-core/src/transaction.rs | 13 +++++++------ helix-core/tests/indent.rs | 4 ++-- helix-term/src/ui/markdown.rs | 8 ++++---- helix-term/src/ui/picker.rs | 6 +++--- 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 1aac38d934c7..28d6dd6ec1a0 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 73d3f73872b9..79fb52a7966c 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> { + pub fn language_config_for_shebang( + &self, + source: RopeSlice, + ) -> Option> { let line = Cow::from(source.line(0)); static SHEBANG_REGEX: Lazy = Lazy::new(|| Regex::new(&["^", SHEBANG].concat()).unwrap()); @@ -928,7 +931,7 @@ fn byte_range_to_str(range: std::ops::Range, source: RopeSlice) -> Cow, loader: Arc, ) -> Option { @@ -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 { 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 9d2a3e5c43a5..fec62578b5a7 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 409706bb99bf..1dec99894738 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 def64434a3f1..cb2abf68c03d 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> = if let Some(spans) = additional_highlight_spans { diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index 13746cfc81f9..4605e2f18c78 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -453,9 +453,9 @@ impl Picker { 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 b08370f9f371..af7e3a7e80d1 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> { 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. From ef47075febbadc6127d05f00968d0fdbb87bcafe Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Wed, 26 Jul 2023 10:26:56 -0500 Subject: [PATCH 4/4] highlighted_code_block: Take input text as &str This removes a handful of allocations for functions calling into the function, which is nice because the prompt may call this function on every keypress. --- helix-term/src/ui/lsp.rs | 4 ++-- helix-term/src/ui/markdown.rs | 6 +++--- helix-term/src/ui/prompt.rs | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/helix-term/src/ui/lsp.rs b/helix-term/src/ui/lsp.rs index 44050aa12969..880df6d8ea3b 100644 --- a/helix-term/src/ui/lsp.rs +++ b/helix-term/src/ui/lsp.rs @@ -62,7 +62,7 @@ impl Component for SignatureHelp { }); let sig_text = crate::ui::markdown::highlighted_code_block( - self.signature.clone(), + &self.signature, &self.language, Some(&cx.editor.theme), Arc::clone(&self.config_loader), @@ -109,7 +109,7 @@ impl Component for SignatureHelp { let max_text_width = (viewport.0 - PADDING).min(120); let signature_text = crate::ui::markdown::highlighted_code_block( - self.signature.clone(), + &self.signature, &self.language, None, Arc::clone(&self.config_loader), diff --git a/helix-term/src/ui/markdown.rs b/helix-term/src/ui/markdown.rs index cb2abf68c03d..1433381d59bc 100644 --- a/helix-term/src/ui/markdown.rs +++ b/helix-term/src/ui/markdown.rs @@ -17,7 +17,7 @@ use helix_view::{ Theme, }; -fn styled_multiline_text<'a>(text: String, style: Style) -> Text<'a> { +fn styled_multiline_text<'a>(text: &str, style: Style) -> Text<'a> { let spans: Vec<_> = text .lines() .map(|line| Span::styled(line.to_string(), style)) @@ -27,7 +27,7 @@ fn styled_multiline_text<'a>(text: String, style: Style) -> Text<'a> { } pub fn highlighted_code_block<'a>( - text: String, + text: &str, language: &str, theme: Option<&Theme>, config_loader: Arc, @@ -267,7 +267,7 @@ impl Markdown { CodeBlockKind::Indented => "", }; let tui_text = highlighted_code_block( - text.to_string(), + &text, language, theme, Arc::clone(&self.config_loader), diff --git a/helix-term/src/ui/prompt.rs b/helix-term/src/ui/prompt.rs index f376b9c9cb3d..1352f4937753 100644 --- a/helix-term/src/ui/prompt.rs +++ b/helix-term/src/ui/prompt.rs @@ -476,7 +476,7 @@ impl Prompt { } } else if let Some((language, loader)) = self.language.as_ref() { let mut text: ui::text::Text = crate::ui::markdown::highlighted_code_block( - self.line.clone(), + &self.line, language, Some(&cx.editor.theme), loader.clone(),