From 18d39e3f812463ecd6f9e4649a82bb4f547eccce Mon Sep 17 00:00:00 2001 From: Smit Barmase Date: Fri, 16 May 2025 12:42:11 +0530 Subject: [PATCH] editor: Improve JSDoc extend comment on newline to follow convention (#30800) Follow up for https://github.com/zed-industries/zed/pull/30768 This PR makes JSDoc auto comment on new line lot better by: - Inserting delimiters regardless of whether previous delimiters have trailing spaces or not - When on start tag, auto-indenting both prefix and end tag upon new line This makes it correct as per convention out of the box. No need to manually adjust spaces on every new line. https://github.com/user-attachments/assets/81b8e05a-fe8a-4459-9e90-c8a3d70a51a2 Release Notes: - Improved JSDoc auto-commenting on newline which now correctly indents as per convention. --- crates/editor/src/editor.rs | 257 ++++++++++++-------- crates/editor/src/editor_tests.rs | 96 +++++++- crates/language/src/language.rs | 39 +-- crates/languages/src/javascript/config.toml | 3 +- crates/languages/src/tsx/config.toml | 3 +- crates/languages/src/typescript/config.toml | 3 +- 6 files changed, 263 insertions(+), 138 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index da7389f4aa6c8d680203e70bb47cc63b58e14872..3f926fe8975e6d83b9d13dc81d44e0e37e80b696 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -107,9 +107,9 @@ pub use items::MAX_TAB_TITLE_LEN; use itertools::Itertools; use language::{ AutoindentMode, BracketMatch, BracketPair, Buffer, Capability, CharKind, CodeLabel, - CursorShape, DiagnosticEntry, DiffOptions, EditPredictionsMode, EditPreview, HighlightedText, - IndentKind, IndentSize, Language, OffsetRangeExt, Point, Selection, SelectionGoal, TextObject, - TransactionId, TreeSitterOptions, WordsQuery, + CursorShape, DiagnosticEntry, DiffOptions, DocumentationConfig, EditPredictionsMode, + EditPreview, HighlightedText, IndentKind, IndentSize, Language, OffsetRangeExt, Point, + Selection, SelectionGoal, TextObject, TransactionId, TreeSitterOptions, WordsQuery, language_settings::{ self, InlayHintSettings, LspInsertMode, RewrapBehavior, WordsCompletionMode, all_language_settings, language_settings, @@ -3912,7 +3912,7 @@ impl Editor { pub fn newline(&mut self, _: &Newline, window: &mut Window, cx: &mut Context) { self.hide_mouse_cursor(&HideMouseCursorOrigin::TypingAction); self.transact(window, cx, |this, window, cx| { - let (edits, selection_fixup_info): (Vec<_>, Vec<_>) = { + let (edits_with_flags, selection_info): (Vec<_>, Vec<_>) = { let selections = this.selections.all::(cx); let multi_buffer = this.buffer.read(cx); let buffer = multi_buffer.snapshot(cx); @@ -3920,22 +3920,26 @@ impl Editor { .iter() .map(|selection| { let start_point = selection.start.to_point(&buffer); - let mut indent = + let mut existing_indent = buffer.indent_size_for_line(MultiBufferRow(start_point.row)); - indent.len = cmp::min(indent.len, start_point.column); + existing_indent.len = cmp::min(existing_indent.len, start_point.column); let start = selection.start; let end = selection.end; let selection_is_empty = start == end; let language_scope = buffer.language_scope_at(start); - let (comment_delimiter, insert_extra_newline) = if let Some(language) = - &language_scope - { + let ( + comment_delimiter, + doc_delimiter, + insert_extra_newline, + indent_on_newline, + indent_on_extra_newline, + ) = if let Some(language) = &language_scope { let mut insert_extra_newline = 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 mut comment_delimiter = maybe!({ + let comment_delimiter = maybe!({ if !selection_is_empty { return None; } @@ -3975,128 +3979,181 @@ impl Editor { } }); - if comment_delimiter.is_none() { - comment_delimiter = maybe!({ - if !selection_is_empty { - return None; - } + let mut indent_on_newline = IndentSize::spaces(0); + let mut indent_on_extra_newline = IndentSize::spaces(0); - if !multi_buffer.language_settings(cx).extend_comment_on_newline - { - return None; + let doc_delimiter = maybe!({ + if !selection_is_empty { + return None; + } + + if !multi_buffer.language_settings(cx).extend_comment_on_newline { + return None; + } + + let DocumentationConfig { + start: start_tag, + end: end_tag, + prefix: delimiter, + tab_size: len, + } = language.documentation()?; + + let (snapshot, range) = + buffer.buffer_line_for_row(MultiBufferRow(start_point.row))?; + + let num_of_whitespaces = snapshot + .chars_for_range(range.clone()) + .take_while(|c| c.is_whitespace()) + .count(); + + let cursor_is_after_start_tag = { + let start_tag_len = start_tag.len(); + let start_tag_line = snapshot + .chars_for_range(range.clone()) + .skip(num_of_whitespaces) + .take(start_tag_len) + .collect::(); + if start_tag_line.starts_with(start_tag.as_ref()) { + num_of_whitespaces + start_tag_len + <= start_point.column as usize + } else { + false } + }; - let doc_block = language.documentation_block(); - let doc_block_prefix = doc_block.first()?; - let doc_block_suffix = doc_block.last()?; - - let doc_comment_prefix = - language.documentation_comment_prefix()?; - - let (snapshot, range) = buffer - .buffer_line_for_row(MultiBufferRow(start_point.row))?; - - let cursor_is_after_prefix = { - let doc_block_prefix_len = doc_block_prefix.len(); - let max_len_of_delimiter = std::cmp::max( - doc_comment_prefix.len(), - doc_block_prefix_len, - ); - let index_of_first_non_whitespace = snapshot - .chars_for_range(range.clone()) - .take_while(|c| c.is_whitespace()) - .count(); - let doc_line_candidate = snapshot - .chars_for_range(range.clone()) - .skip(index_of_first_non_whitespace) - .take(max_len_of_delimiter) - .collect::(); - if doc_line_candidate.starts_with(doc_block_prefix.as_ref()) - { - index_of_first_non_whitespace + doc_block_prefix_len - <= start_point.column as usize - } else if doc_line_candidate - .starts_with(doc_comment_prefix.as_ref()) - { - index_of_first_non_whitespace + doc_comment_prefix.len() - <= start_point.column as usize - } else { - false - } - }; + let cursor_is_after_delimiter = { + let delimiter_trim = delimiter.trim_end(); + let delimiter_line = snapshot + .chars_for_range(range.clone()) + .skip(num_of_whitespaces) + .take(delimiter_trim.len()) + .collect::(); + if delimiter_line.starts_with(delimiter_trim) { + num_of_whitespaces + delimiter_trim.len() + <= start_point.column as usize + } else { + false + } + }; - let cursor_is_before_suffix_if_exits = { - let whitespace_char_from_last = snapshot - .reversed_chars_for_range(range.clone()) - .take_while(|c| c.is_whitespace()) - .count(); - let mut line_rev_iter = snapshot - .reversed_chars_for_range(range) - .skip(whitespace_char_from_last); - let suffix_exists = doc_block_suffix - .chars() - .rev() - .all(|char| line_rev_iter.next() == Some(char)); - if suffix_exists { - let max_point = - snapshot.line_len(start_point.row) as usize; - let cursor_is_before_suffix = whitespace_char_from_last - + doc_block_suffix.len() - + start_point.column as usize - <= max_point; - if cursor_is_before_suffix { + let cursor_is_before_end_tag_if_exists = { + let num_of_whitespaces_rev = snapshot + .reversed_chars_for_range(range.clone()) + .take_while(|c| c.is_whitespace()) + .count(); + let mut line_iter = snapshot + .reversed_chars_for_range(range) + .skip(num_of_whitespaces_rev); + let end_tag_exists = end_tag + .chars() + .rev() + .all(|char| line_iter.next() == Some(char)); + if end_tag_exists { + let max_point = snapshot.line_len(start_point.row) as usize; + let ordering = (num_of_whitespaces_rev + + end_tag.len() + + start_point.column as usize) + .cmp(&max_point); + let cursor_is_before_end_tag = + ordering != Ordering::Greater; + if cursor_is_after_start_tag { + if cursor_is_before_end_tag { insert_extra_newline = true; } - cursor_is_before_suffix - } else { - true + let cursor_is_at_start_of_end_tag = + ordering == Ordering::Equal; + if cursor_is_at_start_of_end_tag { + indent_on_extra_newline.len = (*len).into(); + } } - }; - - if cursor_is_after_prefix && cursor_is_before_suffix_if_exits { - Some(doc_comment_prefix.clone()) + cursor_is_before_end_tag } else { - None + true } - }); - } + }; - (comment_delimiter, insert_extra_newline) + if (cursor_is_after_start_tag || cursor_is_after_delimiter) + && cursor_is_before_end_tag_if_exists + { + if cursor_is_after_start_tag { + indent_on_newline.len = (*len).into(); + } + Some(delimiter.clone()) + } else { + None + } + }); + + ( + comment_delimiter, + doc_delimiter, + insert_extra_newline, + indent_on_newline, + indent_on_extra_newline, + ) } else { - (None, false) + ( + None, + None, + false, + IndentSize::default(), + IndentSize::default(), + ) }; - let capacity_for_delimiter = comment_delimiter - .as_deref() - .map(str::len) - .unwrap_or_default(); - let mut new_text = - String::with_capacity(1 + capacity_for_delimiter + indent.len as usize); + let prevent_auto_indent = doc_delimiter.is_some(); + let delimiter = comment_delimiter.or(doc_delimiter); + + let capacity_for_delimiter = + delimiter.as_deref().map(str::len).unwrap_or_default(); + let mut new_text = String::with_capacity( + 1 + capacity_for_delimiter + + existing_indent.len as usize + + indent_on_newline.len as usize + + indent_on_extra_newline.len as usize, + ); new_text.push('\n'); - new_text.extend(indent.chars()); + new_text.extend(existing_indent.chars()); + new_text.extend(indent_on_newline.chars()); - if let Some(delimiter) = &comment_delimiter { + if let Some(delimiter) = &delimiter { new_text.push_str(delimiter); } if insert_extra_newline { new_text.push('\n'); - new_text.extend(indent.chars()); + new_text.extend(existing_indent.chars()); + new_text.extend(indent_on_extra_newline.chars()); } let anchor = buffer.anchor_after(end); let new_selection = selection.map(|_| anchor); ( - (start..end, new_text), + ((start..end, new_text), prevent_auto_indent), (insert_extra_newline, new_selection), ) }) .unzip() }; - this.edit_with_autoindent(edits, cx); + let mut auto_indent_edits = Vec::new(); + let mut edits = Vec::new(); + for (edit, prevent_auto_indent) in edits_with_flags { + if prevent_auto_indent { + edits.push(edit); + } else { + auto_indent_edits.push(edit); + } + } + if !edits.is_empty() { + this.edit(edits, cx); + } + if !auto_indent_edits.is_empty() { + this.edit_with_autoindent(auto_indent_edits, cx); + } + let buffer = this.buffer.read(cx).snapshot(cx); - let new_selections = selection_fixup_info + let new_selections = selection_info .into_iter() .map(|(extra_newline_inserted, new_selection)| { let mut cursor = new_selection.end.to_point(&buffer); diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 61381c974c149a5b10d1962e20c5a4acb8401741..d169c2febef9ba93a702845163ac48d68f81e5fe 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -2805,8 +2805,12 @@ async fn test_newline_documentation_comments(cx: &mut TestAppContext) { let language = Arc::new(Language::new( LanguageConfig { - documentation_block: Some(vec!["/**".into(), "*/".into()]), - documentation_comment_prefix: Some("*".into()), + documentation: Some(language::DocumentationConfig { + start: "/**".into(), + end: "*/".into(), + prefix: "* ".into(), + tab_size: NonZeroU32::new(1).unwrap(), + }), ..LanguageConfig::default() }, None, @@ -2821,9 +2825,10 @@ async fn test_newline_documentation_comments(cx: &mut TestAppContext) { cx.update_editor(|e, window, cx| e.newline(&Newline, window, cx)); cx.assert_editor_state(indoc! {" /** - *ˇ + * ˇ "}); - // Ensure that if cursor is before the comment start, we do not actually insert a comment prefix. + // Ensure that if cursor is before the comment start, + // we do not actually insert a comment prefix. cx.set_state(indoc! {" ˇ/** "}); @@ -2848,8 +2853,71 @@ async fn test_newline_documentation_comments(cx: &mut TestAppContext) { cx.update_editor(|e, window, cx| e.newline(&Newline, window, cx)); cx.assert_editor_state(indoc! {" /** - *ˇ - */ + * ˇ + */ + "}); + // Ensure that if suffix exists on same line after cursor with space it adds new line. + cx.set_state(indoc! {" + /**ˇ */ + "}); + cx.update_editor(|e, window, cx| e.newline(&Newline, window, cx)); + cx.assert_editor_state(indoc! {" + /** + * ˇ + */ + "}); + // Ensure that if suffix exists on same line after cursor with space it adds new line. + cx.set_state(indoc! {" + /** ˇ*/ + "}); + cx.update_editor(|e, window, cx| e.newline(&Newline, window, cx)); + cx.assert_editor_state( + indoc! {" + /**s + * ˇ + */ + "} + .replace("s", " ") // s is used as space placeholder to prevent format on save + .as_str(), + ); + // Ensure that delimiter space is preserved when newline on already + // spaced delimiter. + cx.update_editor(|e, window, cx| e.newline(&Newline, window, cx)); + cx.assert_editor_state( + indoc! {" + /**s + *s + * ˇ + */ + "} + .replace("s", " ") // s is used as space placeholder to prevent format on save + .as_str(), + ); + // Ensure that delimiter space is preserved when space is not + // on existing delimiter. + cx.set_state(indoc! {" + /** + *ˇ + */ + "}); + cx.update_editor(|e, window, cx| e.newline(&Newline, window, cx)); + cx.assert_editor_state(indoc! {" + /** + * + * ˇ + */ + "}); + // Ensure that if suffix exists on same line after cursor it + // doesn't add extra new line if prefix is not on same line. + cx.set_state(indoc! {" + /** + ˇ*/ + "}); + cx.update_editor(|e, window, cx| e.newline(&Newline, window, cx)); + cx.assert_editor_state(indoc! {" + /** + + ˇ*/ "}); // Ensure that it detects suffix after existing prefix. cx.set_state(indoc! {" @@ -2860,7 +2928,8 @@ async fn test_newline_documentation_comments(cx: &mut TestAppContext) { /** ˇ/ "}); - // Ensure that if suffix exists on same line before cursor it does not add comment prefix. + // Ensure that if suffix exists on same line before + // cursor it does not add comment prefix. cx.set_state(indoc! {" /** */ˇ "}); @@ -2869,18 +2938,19 @@ async fn test_newline_documentation_comments(cx: &mut TestAppContext) { /** */ ˇ "}); - // Ensure that if suffix exists on same line before cursor it does not add comment prefix. + // Ensure that if suffix exists on same line before + // cursor it does not add comment prefix. cx.set_state(indoc! {" /** - * - */ˇ + * + */ˇ "}); cx.update_editor(|e, window, cx| e.newline(&Newline, window, cx)); cx.assert_editor_state(indoc! {" /** - * - */ - ˇ + * + */ + ˇ "}); } // Ensure that comment continuations can be disabled. diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs index 1cb82be1ef426c290b22c4e6bc9cc9634c9393ca..e0c476fa58b68692b6b26c184e3454ddd432e541 100644 --- a/crates/language/src/language.rs +++ b/crates/language/src/language.rs @@ -755,12 +755,10 @@ pub struct LanguageConfig { /// A list of preferred debuggers for this language. #[serde(default)] pub debuggers: IndexSet, - /// A character to add as a prefix when a new line is added to a documentation block. - #[serde(default)] - pub documentation_comment_prefix: Option>, - /// Returns string documentation block of this language should start with. + /// Whether to treat documentation comment of this language differently by + /// auto adding prefix on new line, adjusting the indenting , etc. #[serde(default)] - pub documentation_block: Option>>, + pub documentation: Option, } #[derive(Clone, Debug, Serialize, Deserialize, Default, JsonSchema)] @@ -811,6 +809,19 @@ pub struct JsxTagAutoCloseConfig { pub erroneous_close_tag_name_node_name: Option, } +/// The configuration for documentation block for this language. +#[derive(Clone, Deserialize, JsonSchema)] +pub struct DocumentationConfig { + /// A start tag of documentation block. + pub start: Arc, + /// A end tag of documentation block. + pub end: Arc, + /// A character to add as a prefix when a new line is added to a documentation block. + pub prefix: Arc, + /// A indent to add for prefix and end line upon new line. + pub tab_size: NonZeroU32, +} + /// Represents a language for the given range. Some languages (e.g. HTML) /// interleave several languages together, thus a single buffer might actually contain /// several nested scopes. @@ -889,8 +900,7 @@ impl Default for LanguageConfig { completion_query_characters: Default::default(), debuggers: Default::default(), significant_indentation: Default::default(), - documentation_comment_prefix: None, - documentation_block: None, + documentation: None, } } } @@ -1810,21 +1820,12 @@ impl LanguageScope { .unwrap_or(false) } - /// A character to add as a prefix when a new line is added to a documentation block. + /// Returns config to documentation block for this language. /// /// Used for documentation styles that require a leading character on each line, /// such as the asterisk in JSDoc, Javadoc, etc. - pub fn documentation_comment_prefix(&self) -> Option<&Arc> { - self.language.config.documentation_comment_prefix.as_ref() - } - - /// Returns prefix and suffix for documentation block of this language. - pub fn documentation_block(&self) -> &[Arc] { - self.language - .config - .documentation_block - .as_ref() - .map_or([].as_slice(), |e| e.as_slice()) + pub fn documentation(&self) -> Option<&DocumentationConfig> { + self.language.config.documentation.as_ref() } /// Returns a list of bracket pairs for a given language with an additional diff --git a/crates/languages/src/javascript/config.toml b/crates/languages/src/javascript/config.toml index 1559a2b29541fa07c6b3e8d5f44063ee549be362..db5641e7c6cd6ab244cd16f3a3405e85f75d4c3b 100644 --- a/crates/languages/src/javascript/config.toml +++ b/crates/languages/src/javascript/config.toml @@ -20,8 +20,7 @@ tab_size = 2 scope_opt_in_language_servers = ["tailwindcss-language-server", "emmet-language-server"] prettier_parser_name = "babel" debuggers = ["JavaScript"] -documentation_comment_prefix = "*" -documentation_block = ["/**", "*/"] +documentation = { start = "/**", end = "*/", prefix = "* ", tab_size = 1 } [jsx_tag_auto_close] open_tag_node_name = "jsx_opening_element" diff --git a/crates/languages/src/tsx/config.toml b/crates/languages/src/tsx/config.toml index 2a2a3000b1c708c6465133fbc8107bbfea72a135..c581a9c1c84c3db481fb7dfe9c789d54eb261ad3 100644 --- a/crates/languages/src/tsx/config.toml +++ b/crates/languages/src/tsx/config.toml @@ -18,8 +18,7 @@ scope_opt_in_language_servers = ["tailwindcss-language-server", "emmet-language- prettier_parser_name = "typescript" tab_size = 2 debuggers = ["JavaScript"] -documentation_comment_prefix = "*" -documentation_block = ["/**", "*/"] +documentation = { start = "/**", end = "*/", prefix = "* ", tab_size = 1 } [jsx_tag_auto_close] open_tag_node_name = "jsx_opening_element" diff --git a/crates/languages/src/typescript/config.toml b/crates/languages/src/typescript/config.toml index 5e76789a580d188276087e46cee681eaacc17c4e..6d8598c69d51ec8b02546966206a35c37066ec6e 100644 --- a/crates/languages/src/typescript/config.toml +++ b/crates/languages/src/typescript/config.toml @@ -18,8 +18,7 @@ word_characters = ["#", "$"] prettier_parser_name = "typescript" tab_size = 2 debuggers = ["JavaScript"] -documentation_comment_prefix = "*" -documentation_block = ["/**", "*/"] +documentation = { start = "/**", end = "*/", prefix = "* ", tab_size = 1 } [overrides.string] completion_query_characters = ["."]