From a50fbc9b5c8aab7324c40d8bd63809e715ae5fde Mon Sep 17 00:00:00 2001 From: Smit Barmase Date: Wed, 23 Apr 2025 01:20:25 +0530 Subject: [PATCH] language: Fix `language_scope_at` for markdown code comments (#29230) Closes #29176 This PR fix an issue where uncommenting a code block in Markdown would add Markdown comments instead of removing the language-specific comments. Why? `language_scope_at` for comments in a code block in Markdown would result in the language being detected as Markdown. This happens because the smallest range, such as `//` or `#` on the Markdown layer, is preferred over `// whole comment line` for any other language. This results in language detection as Markdown for that point. To fix this, we also use a depth factor and try to prefer the layer with greater depth over one with lesser depth. In this case, the code block's language depth would be preferred over Markdown. The smallest range is now used as a tiebreaker. Added test for this case. Release Notes: - Fixed issue where uncommenting a code block in Markdown would add Markdown comments instead of removing the language comments. --- crates/language/src/buffer.rs | 22 ++++++++---- crates/language/src/buffer_tests.rs | 53 +++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 7 deletions(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index a95799845f14ff05f0e6d7103dc383d5e8256682..5097a0bab0f11730a0dd354aacf3c165df8a0dee 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -3167,7 +3167,7 @@ impl BufferSnapshot { pub fn language_scope_at(&self, position: D) -> Option { let offset = position.to_offset(self); let mut scope = None; - let mut smallest_range: Option> = None; + let mut smallest_range_and_depth: Option<(Range, usize)> = None; // Use the layer that has the smallest node intersecting the given point. for layer in self @@ -3179,7 +3179,7 @@ impl BufferSnapshot { let mut range = None; loop { let child_range = cursor.node().byte_range(); - if !child_range.to_inclusive().contains(&offset) { + if !child_range.contains(&offset) { break; } @@ -3190,11 +3190,19 @@ impl BufferSnapshot { } if let Some(range) = range { - if smallest_range - .as_ref() - .map_or(true, |smallest_range| range.len() < smallest_range.len()) - { - smallest_range = Some(range); + if smallest_range_and_depth.as_ref().map_or( + true, + |(smallest_range, smallest_range_depth)| { + if layer.depth > *smallest_range_depth { + true + } else if layer.depth == *smallest_range_depth { + range.len() < smallest_range.len() + } else { + false + } + }, + ) { + smallest_range_and_depth = Some((range, layer.depth)); scope = Some(LanguageScope { language: layer.language.clone(), override_id: layer.override_id(offset, &self.text), diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs index dbe54b5c6d41765588108bad05025eeb06bd5fd6..86e8da4c3ccc5ef2861d3e31d7438886db81fc27 100644 --- a/crates/language/src/buffer_tests.rs +++ b/crates/language/src/buffer_tests.rs @@ -2507,6 +2507,59 @@ fn test_language_at_with_hidden_languages(cx: &mut App) { }); } +#[gpui::test] +fn test_language_at_for_markdown_code_block(cx: &mut App) { + init_settings(cx, |_| {}); + + cx.new(|cx| { + let text = r#" + ```rs + let a = 2; + // let b = 3; + ``` + "# + .unindent(); + + let language_registry = Arc::new(LanguageRegistry::test(cx.background_executor().clone())); + language_registry.add(Arc::new(markdown_lang())); + language_registry.add(Arc::new(markdown_inline_lang())); + language_registry.add(Arc::new(rust_lang())); + + let mut buffer = Buffer::local(text, cx); + buffer.set_language_registry(language_registry.clone()); + buffer.set_language( + language_registry + .language_for_name("Markdown") + .now_or_never() + .unwrap() + .ok(), + cx, + ); + + let snapshot = buffer.snapshot(); + + // Test points in the code line + for point in [Point::new(1, 4), Point::new(1, 6)] { + let config = snapshot.language_scope_at(point).unwrap(); + assert_eq!(config.language_name(), "Rust".into()); + + let language = snapshot.language_at(point).unwrap(); + assert_eq!(language.name().as_ref(), "Rust"); + } + + // Test points in the comment line to verify it's still detected as Rust + for point in [Point::new(2, 4), Point::new(2, 6)] { + let config = snapshot.language_scope_at(point).unwrap(); + assert_eq!(config.language_name(), "Rust".into()); + + let language = snapshot.language_at(point).unwrap(); + assert_eq!(language.name().as_ref(), "Rust"); + } + + buffer + }); +} + #[gpui::test] fn test_serialization(cx: &mut gpui::App) { let mut now = Instant::now();