From 1429363218f61049001b3429ea77976ec0242559 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 20 Feb 2025 01:09:19 -0500 Subject: [PATCH] html: Open extra newline between opening and closing HTML tags (#25130) Closes #12064 It feels a bit strange to use `brackets` for this but it seems to work without unintended consequences from my testing so far. Release Notes: - N/A --------- Co-authored-by: Marshall Bowers --- crates/editor/src/editor.rs | 104 ++++++++++++++------ crates/editor/src/editor_tests.rs | 54 ++++++++++ crates/language/src/buffer.rs | 56 +++++++---- crates/language/src/buffer_tests.rs | 5 +- crates/language/src/language.rs | 25 ++++- crates/multi_buffer/src/multi_buffer.rs | 16 +-- extensions/html/languages/html/brackets.scm | 1 + 7 files changed, 199 insertions(+), 62 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index c0f30fb4b0..074e4611aa 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -101,10 +101,10 @@ use language::{ language_settings::{ self, all_language_settings, language_settings, InlayHintSettings, RewrapBehavior, }, - point_from_lsp, text_diff_with_options, AutoindentMode, BracketPair, Buffer, Capability, - CharKind, CodeLabel, CursorShape, Diagnostic, DiffOptions, DiskState, EditPredictionsMode, - EditPreview, HighlightedText, IndentKind, IndentSize, Language, OffsetRangeExt, Point, - Selection, SelectionGoal, TextObject, TransactionId, TreeSitterOptions, + point_from_lsp, text_diff_with_options, AutoindentMode, BracketMatch, BracketPair, Buffer, + Capability, CharKind, CodeLabel, CursorShape, Diagnostic, DiffOptions, DiskState, + EditPredictionsMode, EditPreview, HighlightedText, IndentKind, IndentSize, Language, + OffsetRangeExt, Point, Selection, SelectionGoal, TextObject, TransactionId, TreeSitterOptions, }; use language::{point_to_lsp, BufferRow, CharClassifier, Runnable, RunnableRange}; use linked_editing_ranges::refresh_linked_ranges; @@ -3153,35 +3153,9 @@ impl Editor { let (comment_delimiter, insert_extra_newline) = if let Some(language) = &language_scope { - let leading_whitespace_len = buffer - .reversed_chars_at(start) - .take_while(|c| c.is_whitespace() && *c != '\n') - .map(|c| c.len_utf8()) - .sum::(); - - let trailing_whitespace_len = buffer - .chars_at(end) - .take_while(|c| c.is_whitespace() && *c != '\n') - .map(|c| c.len_utf8()) - .sum::(); - let insert_extra_newline = - language.brackets().any(|(pair, enabled)| { - let pair_start = pair.start.trim_end(); - let pair_end = pair.end.trim_start(); - - enabled - && pair.newline - && buffer.contains_str_at( - end + trailing_whitespace_len, - pair_end, - ) - && buffer.contains_str_at( - (start - leading_whitespace_len) - .saturating_sub(pair_start.len()), - pair_start, - ) - }); + insert_extra_newline_brackets(&buffer, start..end, language) + || insert_extra_newline_tree_sitter(&buffer, start..end); // Comment extension on newline is allowed only for cursor selections let comment_delimiter = maybe!({ @@ -15088,6 +15062,72 @@ impl Editor { } } +fn insert_extra_newline_brackets( + buffer: &MultiBufferSnapshot, + range: Range, + language: &language::LanguageScope, +) -> bool { + let leading_whitespace_len = buffer + .reversed_chars_at(range.start) + .take_while(|c| c.is_whitespace() && *c != '\n') + .map(|c| c.len_utf8()) + .sum::(); + let trailing_whitespace_len = buffer + .chars_at(range.end) + .take_while(|c| c.is_whitespace() && *c != '\n') + .map(|c| c.len_utf8()) + .sum::(); + let range = range.start - leading_whitespace_len..range.end + trailing_whitespace_len; + + language.brackets().any(|(pair, enabled)| { + let pair_start = pair.start.trim_end(); + let pair_end = pair.end.trim_start(); + + enabled + && pair.newline + && buffer.contains_str_at(range.end, pair_end) + && buffer.contains_str_at(range.start.saturating_sub(pair_start.len()), pair_start) + }) +} + +fn insert_extra_newline_tree_sitter(buffer: &MultiBufferSnapshot, range: Range) -> bool { + let (buffer, range) = match buffer.range_to_buffer_ranges(range).as_slice() { + [(buffer, range, _)] => (*buffer, range.clone()), + _ => return false, + }; + let pair = { + let mut result: Option = None; + + for pair in buffer + .all_bracket_ranges(range.clone()) + .filter(move |pair| { + pair.open_range.start <= range.start && pair.close_range.end >= range.end + }) + { + let len = pair.close_range.end - pair.open_range.start; + + if let Some(existing) = &result { + let existing_len = existing.close_range.end - existing.open_range.start; + if len > existing_len { + continue; + } + } + + result = Some(pair); + } + + result + }; + let Some(pair) = pair else { + return false; + }; + pair.newline_only + && buffer + .chars_for_range(pair.open_range.end..range.start) + .chain(buffer.chars_for_range(range.end..pair.close_range.start)) + .all(|c| c.is_whitespace() && c != '\n') +} + fn get_uncommitted_diff_for_buffer( project: &Entity, buffers: impl IntoIterator>, diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 0b6e80e271..a3d502f5ed 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -15934,6 +15934,60 @@ async fn test_rename_without_prepare(cx: &mut gpui::TestAppContext) { "}); } +#[gpui::test] +async fn test_tree_sitter_brackets_newline_insertion(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + let mut cx = EditorTestContext::new(cx).await; + + let language = Arc::new( + Language::new( + LanguageConfig::default(), + Some(tree_sitter_html::LANGUAGE.into()), + ) + .with_brackets_query( + r#" + ("<" @open "/>" @close) + ("" @close) + ("<" @open ">" @close) + ("\"" @open "\"" @close) + ((element (start_tag) @open (end_tag) @close) (#set! newline.only)) + "#, + ) + .unwrap(), + ); + cx.update_buffer(|buffer, cx| buffer.set_language(Some(language), cx)); + + cx.set_state(indoc! {" + ˇ + "}); + cx.update_editor(|e, window, cx| e.newline(&Newline, window, cx)); + cx.assert_editor_state(indoc! {" + + ˇ + + "}); + + cx.set_state(indoc! {" + ˇ + "}); + cx.update_editor(|e, window, cx| e.newline(&Newline, window, cx)); + cx.assert_editor_state(indoc! {" + + ˇ + "}); + + cx.set_state(indoc! {" + ˇ + + "}); + cx.update_editor(|e, window, cx| e.newline(&Newline, window, cx)); + cx.assert_editor_state(indoc! {" + + ˇ + + "}); +} + fn empty_range(row: usize, column: usize) -> Range { let point = DisplayPoint::new(DisplayRow(row as u32), column as u32); point..point diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 192519367b..0202a96089 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -782,6 +782,13 @@ impl EditPreview { } } +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct BracketMatch { + pub open_range: Range, + pub close_range: Range, + pub newline_only: bool, +} + impl Buffer { /// Create a new buffer with the given base text. pub fn local>(base_text: T, cx: &Context) -> Self { @@ -3556,15 +3563,10 @@ impl BufferSnapshot { self.syntax.matches(range, self, query) } - /// Returns bracket range pairs overlapping or adjacent to `range` - pub fn bracket_ranges( + pub fn all_bracket_ranges( &self, - range: Range, - ) -> impl Iterator, Range)> + '_ { - // Find bracket pairs that *inclusively* contain the given range. - let range = range.start.to_offset(self).saturating_sub(1) - ..self.len().min(range.end.to_offset(self) + 1); - + range: Range, + ) -> impl Iterator + '_ { let mut matches = self.syntax.matches(range.clone(), &self.text, |grammar| { grammar.brackets_config.as_ref().map(|c| &c.query) }); @@ -3579,6 +3581,7 @@ impl BufferSnapshot { let mut open = None; let mut close = None; let config = &configs[mat.grammar_index]; + let pattern = &config.patterns[mat.pattern_index]; for capture in mat.captures { if capture.index == config.open_capture_ix { open = Some(capture.node.byte_range()); @@ -3589,21 +3592,37 @@ impl BufferSnapshot { matches.advance(); - let Some((open, close)) = open.zip(close) else { + let Some((open_range, close_range)) = open.zip(close) else { continue; }; - let bracket_range = open.start..=close.end; + let bracket_range = open_range.start..=close_range.end; if !bracket_range.overlaps(&range) { continue; } - return Some((open, close)); + return Some(BracketMatch { + open_range, + close_range, + newline_only: pattern.newline_only, + }); } None }) } + /// Returns bracket range pairs overlapping or adjacent to `range` + pub fn bracket_ranges( + &self, + range: Range, + ) -> impl Iterator + '_ { + // Find bracket pairs that *inclusively* contain the given range. + let range = range.start.to_offset(self).saturating_sub(1) + ..self.len().min(range.end.to_offset(self) + 1); + self.all_bracket_ranges(range) + .filter(|pair| !pair.newline_only) + } + pub fn text_object_ranges( &self, range: Range, @@ -3674,11 +3693,12 @@ impl BufferSnapshot { pub fn enclosing_bracket_ranges( &self, range: Range, - ) -> impl Iterator, Range)> + '_ { + ) -> impl Iterator + '_ { let range = range.start.to_offset(self)..range.end.to_offset(self); - self.bracket_ranges(range.clone()) - .filter(move |(open, close)| open.start <= range.start && close.end >= range.end) + self.bracket_ranges(range.clone()).filter(move |pair| { + pair.open_range.start <= range.start && pair.close_range.end >= range.end + }) } /// Returns the smallest enclosing bracket ranges containing the given range or None if no brackets contain range @@ -3694,14 +3714,14 @@ impl BufferSnapshot { // Get the ranges of the innermost pair of brackets. let mut result: Option<(Range, Range)> = None; - for (open, close) in self.enclosing_bracket_ranges(range.clone()) { + for pair in self.enclosing_bracket_ranges(range.clone()) { if let Some(range_filter) = range_filter { - if !range_filter(open.clone(), close.clone()) { + if !range_filter(pair.open_range.clone(), pair.close_range.clone()) { continue; } } - let len = close.end - open.start; + let len = pair.close_range.end - pair.open_range.start; if let Some((existing_open, existing_close)) = &result { let existing_len = existing_close.end - existing_open.start; @@ -3710,7 +3730,7 @@ impl BufferSnapshot { } } - result = Some((open, close)); + result = Some((pair.open_range, pair.close_range)); } result diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs index 1b3b39dcc4..94aef99fc4 100644 --- a/crates/language/src/buffer_tests.rs +++ b/crates/language/src/buffer_tests.rs @@ -3401,7 +3401,10 @@ fn assert_bracket_pairs( .collect::>(); assert_set_eq!( - buffer.bracket_ranges(selection_range).collect::>(), + buffer + .bracket_ranges(selection_range) + .map(|pair| (pair.open_range, pair.close_range)) + .collect::>(), bracket_pairs ); } diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs index 89efb27ec4..7bb545dc5a 100644 --- a/crates/language/src/language.rs +++ b/crates/language/src/language.rs @@ -918,7 +918,7 @@ pub struct Grammar { pub ts_language: tree_sitter::Language, pub(crate) error_query: Option, pub(crate) highlights_query: Option, - pub(crate) brackets_config: Option, + pub(crate) brackets_config: Option, pub(crate) redactions_config: Option, pub(crate) runnable_config: Option, pub(crate) indents_config: Option, @@ -1039,10 +1039,16 @@ struct InjectionPatternConfig { combined: bool, } -struct BracketConfig { +struct BracketsConfig { query: Query, open_capture_ix: u32, close_capture_ix: u32, + patterns: Vec, +} + +#[derive(Clone, Debug, Default)] +struct BracketsPatternConfig { + newline_only: bool, } impl Language { @@ -1284,11 +1290,24 @@ impl Language { ("close", &mut close_capture_ix), ], ); + let patterns = (0..query.pattern_count()) + .map(|ix| { + let mut config = BracketsPatternConfig::default(); + for setting in query.property_settings(ix) { + match setting.key.as_ref() { + "newline.only" => config.newline_only = true, + _ => {} + } + } + config + }) + .collect(); if let Some((open_capture_ix, close_capture_ix)) = open_capture_ix.zip(close_capture_ix) { - grammar.brackets_config = Some(BracketConfig { + grammar.brackets_config = Some(BracketsConfig { query, open_capture_ix, close_capture_ix, + patterns, }); } Ok(self) diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 56988abb4e..e009328efd 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -5156,11 +5156,11 @@ impl MultiBufferSnapshot { excerpt .buffer() .enclosing_bracket_ranges(excerpt.map_range_to_buffer(range)) - .filter_map(move |(open, close)| { - if excerpt.contains_buffer_range(open.start..close.end) { + .filter_map(move |pair| { + if excerpt.contains_buffer_range(pair.open_range.start..pair.close_range.end) { Some(( - excerpt.map_range_from_buffer(open), - excerpt.map_range_from_buffer(close), + excerpt.map_range_from_buffer(pair.open_range), + excerpt.map_range_from_buffer(pair.close_range), )) } else { None @@ -5207,12 +5207,12 @@ impl MultiBufferSnapshot { excerpt .buffer() .bracket_ranges(excerpt.map_range_to_buffer(range)) - .filter_map(move |(start_bracket_range, close_bracket_range)| { - let buffer_range = start_bracket_range.start..close_bracket_range.end; + .filter_map(move |pair| { + let buffer_range = pair.open_range.start..pair.close_range.end; if excerpt.contains_buffer_range(buffer_range) { Some(( - excerpt.map_range_from_buffer(start_bracket_range), - excerpt.map_range_from_buffer(close_bracket_range), + excerpt.map_range_from_buffer(pair.open_range), + excerpt.map_range_from_buffer(pair.close_range), )) } else { None diff --git a/extensions/html/languages/html/brackets.scm b/extensions/html/languages/html/brackets.scm index e865561f77..f9be89a263 100644 --- a/extensions/html/languages/html/brackets.scm +++ b/extensions/html/languages/html/brackets.scm @@ -2,3 +2,4 @@ ("" @close) ("<" @open ">" @close) ("\"" @open "\"" @close) +((element (start_tag) @open (end_tag) @close) (#set! newline.only))