From e996c321085dfa465c109d9254d2d8796e1411f9 Mon Sep 17 00:00:00 2001 From: Amaan Qureshi Date: Wed, 7 Feb 2024 02:02:32 -0500 Subject: [PATCH] refactor!: remove the apply-all-captures flag, make last-wins precedence the default --- cli/loader/src/lib.rs | 5 +- cli/src/highlight.rs | 4 +- cli/src/main.rs | 13 ++--- cli/src/test_highlight.rs | 16 ++----- cli/src/tests/helpers/fixtures.rs | 1 - cli/src/tests/highlight_test.rs | 6 +-- highlight/include/tree_sitter/highlight.h | 1 - highlight/src/c_lib.rs | 2 - highlight/src/lib.rs | 58 ++++++++--------------- 9 files changed, 31 insertions(+), 75 deletions(-) diff --git a/cli/loader/src/lib.rs b/cli/loader/src/lib.rs index 48e95262..f040e603 100644 --- a/cli/loader/src/lib.rs +++ b/cli/loader/src/lib.rs @@ -781,7 +781,6 @@ impl Loader { pub fn highlight_config_for_injection_string<'a>( &'a self, string: &str, - apply_all_captures: bool, ) -> Option<&'a HighlightConfiguration> { match self.language_configuration_for_injection_string(string) { Err(e) => { @@ -790,7 +789,7 @@ impl Loader { } Ok(None) => None, Ok(Some((language, configuration))) => { - match configuration.highlight_config(language, apply_all_captures, None) { + match configuration.highlight_config(language, None) { Err(e) => { eprintln!( "Failed to load property sheet for injection string '{string}': {e}", @@ -1050,7 +1049,6 @@ impl<'a> LanguageConfiguration<'a> { pub fn highlight_config( &self, language: Language, - apply_all_captures: bool, paths: Option<&[String]>, ) -> Result> { let (highlights_filenames, injections_filenames, locals_filenames) = match paths { @@ -1115,7 +1113,6 @@ impl<'a> LanguageConfiguration<'a> { &highlights_query, &injections_query, &locals_query, - apply_all_captures, ) .map_err(|error| match error.kind { QueryErrorKind::Language => Error::from(error), diff --git a/cli/src/highlight.rs b/cli/src/highlight.rs index 073b1754..28aad3b6 100644 --- a/cli/src/highlight.rs +++ b/cli/src/highlight.rs @@ -342,7 +342,7 @@ pub fn ansi( let mut highlighter = Highlighter::new(); let events = highlighter.highlight(config, source, cancellation_flag, |string| { - loader.highlight_config_for_injection_string(string, config.apply_all_captures) + loader.highlight_config_for_injection_string(string) })?; let mut style_stack = vec![theme.default_style().ansi]; @@ -388,7 +388,7 @@ pub fn html( let mut highlighter = Highlighter::new(); let events = highlighter.highlight(config, source, cancellation_flag, |string| { - loader.highlight_config_for_injection_string(string, config.apply_all_captures) + loader.highlight_config_for_injection_string(string) })?; let mut renderer = HtmlRenderer::new(); diff --git a/cli/src/main.rs b/cli/src/main.rs index 085391ed..4c87fb8a 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -194,8 +194,6 @@ struct Test { help = "Compile parsers to wasm instead of native dynamic libraries" )] pub wasm: bool, - #[arg(long, help = "Apply all captures to highlights")] - pub apply_all_captures: bool, #[arg( long, help = "Open `log.html` in the default browser, if `--debug-graph` is supplied" @@ -267,8 +265,6 @@ struct Highlight { pub paths_file: Option, #[arg(num_args = 1.., help = "The source file(s) to use")] pub paths: Option>, - #[arg(long, help = "Apply all captures to highlights")] - pub apply_all_captures: bool, } #[derive(Args)] @@ -583,7 +579,6 @@ fn run() -> Result<()> { &config.get()?, &mut highlighter, &test_highlight_dir, - test_options.apply_all_captures, )?; parser = highlighter.parser; } @@ -674,11 +669,9 @@ fn run() -> Result<()> { } }; - if let Some(highlight_config) = language_config.highlight_config( - language, - highlight_options.apply_all_captures, - highlight_options.query_paths.as_deref(), - )? { + if let Some(highlight_config) = language_config + .highlight_config(language, highlight_options.query_paths.as_deref())? + { if highlight_options.check { let names = if let Some(path) = highlight_options.captures_path.as_deref() { let path = Path::new(path); diff --git a/cli/src/test_highlight.rs b/cli/src/test_highlight.rs index 63f80897..919d7864 100644 --- a/cli/src/test_highlight.rs +++ b/cli/src/test_highlight.rs @@ -48,17 +48,9 @@ pub fn test_highlights( loader_config: &Config, highlighter: &mut Highlighter, directory: &Path, - apply_all_captures: bool, ) -> Result<()> { println!("syntax highlighting:"); - test_highlights_indented( - loader, - loader_config, - highlighter, - directory, - apply_all_captures, - 2, - ) + test_highlights_indented(loader, loader_config, highlighter, directory, 2) } fn test_highlights_indented( @@ -66,7 +58,6 @@ fn test_highlights_indented( loader_config: &Config, highlighter: &mut Highlighter, directory: &Path, - apply_all_captures: bool, indent_level: usize, ) -> Result<()> { let mut failed = false; @@ -87,7 +78,6 @@ fn test_highlights_indented( loader_config, highlighter, &test_file_path, - apply_all_captures, indent_level + 1, ) .is_err() @@ -104,7 +94,7 @@ fn test_highlights_indented( ) })?; let highlight_config = language_config - .highlight_config(language, apply_all_captures, None)? + .highlight_config(language, None)? .ok_or_else(|| anyhow!("No highlighting config found for {test_file_path:?}"))?; match test_highlight( loader, @@ -235,7 +225,7 @@ pub fn get_highlight_positions( let source = String::from_utf8_lossy(source); let mut char_indices = source.char_indices(); for event in highlighter.highlight(highlight_config, source.as_bytes(), None, |string| { - loader.highlight_config_for_injection_string(string, highlight_config.apply_all_captures) + loader.highlight_config_for_injection_string(string) })? { match event? { HighlightEvent::HighlightStart(h) => highlight_stack.push(h), diff --git a/cli/src/tests/helpers/fixtures.rs b/cli/src/tests/helpers/fixtures.rs index 2e4bb213..69eb7c8a 100644 --- a/cli/src/tests/helpers/fixtures.rs +++ b/cli/src/tests/helpers/fixtures.rs @@ -63,7 +63,6 @@ pub fn get_highlight_config( &highlights_query, &injections_query, &locals_query, - false, ) .unwrap(); result.configure(highlight_names); diff --git a/cli/src/tests/highlight_test.rs b/cli/src/tests/highlight_test.rs index 44e52c27..77a95d7d 100644 --- a/cli/src/tests/highlight_test.rs +++ b/cli/src/tests/highlight_test.rs @@ -310,7 +310,7 @@ fn test_highlighting_empty_lines() { .join("\n"); assert_eq!( - &to_html(&source, &JS_HIGHLIGHT,).unwrap(), + &to_html(&source, &JS_HIGHLIGHT).unwrap(), &[ "class A {\n".to_string(), "\n".to_string(), @@ -529,7 +529,6 @@ fn test_highlighting_via_c_api() { highlights_query.len() as u32, injections_query.len() as u32, locals_query.len() as u32, - false, ); } @@ -553,7 +552,6 @@ fn test_highlighting_via_c_api() { highlights_query.len() as u32, injections_query.len() as u32, 0, - false, ); } @@ -622,7 +620,7 @@ fn test_highlighting_with_all_captures_applied() { [ \"{\" \"}\" \"(\" \")\" ] @punctuation.bracket "}; let mut rust_highlight_reverse = - HighlightConfiguration::new(language, "rust", highlights_query, "", "", true).unwrap(); + HighlightConfiguration::new(language, "rust", highlights_query, "", "").unwrap(); rust_highlight_reverse.configure(&HIGHLIGHT_NAMES); assert_eq!( diff --git a/highlight/include/tree_sitter/highlight.h b/highlight/include/tree_sitter/highlight.h index 325cf413..5db458c1 100644 --- a/highlight/include/tree_sitter/highlight.h +++ b/highlight/include/tree_sitter/highlight.h @@ -49,7 +49,6 @@ TSHighlightError ts_highlighter_add_language( uint32_t highlight_query_len, uint32_t injection_query_len, uint32_t locals_query_len, - bool apply_all_captures ); // Compute syntax highlighting for a given document. You must first diff --git a/highlight/src/c_lib.rs b/highlight/src/c_lib.rs index a06674db..6b4d1cf8 100644 --- a/highlight/src/c_lib.rs +++ b/highlight/src/c_lib.rs @@ -87,7 +87,6 @@ pub unsafe extern "C" fn ts_highlighter_add_language( highlight_query_len: u32, injection_query_len: u32, locals_query_len: u32, - apply_all_captures: bool, ) -> ErrorCode { let f = move || { let this = unwrap_mut_ptr(this); @@ -134,7 +133,6 @@ pub unsafe extern "C" fn ts_highlighter_add_language( highlight_query, injection_query, locals_query, - apply_all_captures, ) .or(Err(ErrorCode::InvalidQuery))?; config.configure(this.highlight_names.as_slice()); diff --git a/highlight/src/lib.rs b/highlight/src/lib.rs index f1c55026..22c0bc86 100644 --- a/highlight/src/lib.rs +++ b/highlight/src/lib.rs @@ -106,7 +106,6 @@ pub struct HighlightConfiguration { pub language: Language, pub language_name: String, pub query: Query, - pub apply_all_captures: bool, combined_injections_query: Option, locals_pattern_index: usize, highlights_pattern_index: usize, @@ -165,7 +164,6 @@ where iter_count: usize, next_event: Option, last_highlight_range: Option<(usize, usize, usize)>, - apply_all_captures: bool, } struct HighlightIterLayer<'a> { @@ -233,7 +231,6 @@ impl Highlighter { layers, next_event: None, last_highlight_range: None, - apply_all_captures: config.apply_all_captures, }; result.sort_layers(); Ok(result) @@ -261,7 +258,6 @@ impl HighlightConfiguration { highlights_query: &str, injection_query: &str, locals_query: &str, - apply_all_captures: bool, ) -> Result { // Concatenate the query strings, keeping track of the start offset of each section. let mut query_source = String::new(); @@ -343,7 +339,6 @@ impl HighlightConfiguration { language, language_name: name.into(), query, - apply_all_captures, combined_injections_query, locals_pattern_index, highlights_pattern_index, @@ -777,12 +772,13 @@ where } // If there are no more captures, then emit any remaining highlight end events. // And if there are none of those, then just advance to the end of the document. - else if let Some(end_byte) = layer.highlight_end_stack.last().copied() { - layer.highlight_end_stack.pop(); - return self.emit_event(end_byte, Some(HighlightEvent::HighlightEnd)); - } else { + else { + if let Some(end_byte) = layer.highlight_end_stack.last().copied() { + layer.highlight_end_stack.pop(); + return self.emit_event(end_byte, Some(HighlightEvent::HighlightEnd)); + } return self.emit_event(self.source.len(), None); - }; + } let (mut match_, capture_index) = layer.captures.next().unwrap(); let mut capture = match_.captures[capture_index]; @@ -936,40 +932,26 @@ where } } - // If the current node was found to be a local variable, then skip over any - // highlighting patterns that are disabled for local variables. - if definition_highlight.is_some() || reference_highlight.is_some() { - while layer.config.non_local_variable_patterns[match_.pattern_index] { - match_.remove(); - if let Some((next_match, next_capture_index)) = layer.captures.peek() { - let next_capture = next_match.captures[*next_capture_index]; - if next_capture.node == capture.node { - capture = next_capture; - match_ = layer.captures.next().unwrap().0; - continue; - } - } - - self.sort_layers(); - continue 'main; - } - } - - // Once a highlighting pattern is found for the current node, skip over - // any later highlighting patterns that also match this node. Captures - // for a given node are ordered by pattern index, so these subsequent + // Once a highlighting pattern is found for the current node, keep iterating over + // any later highlighting patterns that also match this node and set the match to it. + // Captures for a given node are ordered by pattern index, so these subsequent // captures are guaranteed to be for highlighting, not injections or // local variables. while let Some((next_match, next_capture_index)) = layer.captures.peek() { let next_capture = next_match.captures[*next_capture_index]; if next_capture.node == capture.node { - if self.apply_all_captures { - match_.remove(); - capture = next_capture; - match_ = layer.captures.next().unwrap().0; - } else { - layer.captures.next(); + let following_match = layer.captures.next().unwrap().0; + // If the current node was found to be a local variable, then ignore + // the following match if it's a highlighting pattern that is disabled + // for local variables. + if (definition_highlight.is_some() || reference_highlight.is_some()) + && layer.config.non_local_variable_patterns[following_match.pattern_index] + { + continue; } + match_.remove(); + capture = next_capture; + match_ = following_match; } else { break; }