From 27a86cb7df5e1764c85f5eeb8bb85885072f782e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20Ochagav=C3=ADa?= Date: Wed, 10 Oct 2018 12:37:06 +0200 Subject: Collapse comments upon join --- crates/ra_editor/src/typing.rs | 79 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 64 insertions(+), 15 deletions(-) (limited to 'crates') diff --git a/crates/ra_editor/src/typing.rs b/crates/ra_editor/src/typing.rs index 3384389d1..6c1a91ffb 100644 --- a/crates/ra_editor/src/typing.rs +++ b/crates/ra_editor/src/typing.rs @@ -30,6 +30,7 @@ pub fn join_lines(file: &File, range: TextRange) -> LocalEdit { } else { range }; + let node = find_covering_node(file.syntax(), range); let mut edit = EditBuilder::new(); for node in node.descendants() { @@ -140,30 +141,71 @@ fn remove_newline( offset: TextUnit, ) { if node.kind() == WHITESPACE && node_text.bytes().filter(|&b| b == b'\n').count() == 1 { + // Special case that turns something like: + // + // ``` + // my_function({<|> + // + // }) + // ``` + // + // into `my_function()` if join_single_expr_block(edit, node).is_some() { return } - match (node.prev_sibling(), node.next_sibling()) { - (Some(prev), Some(next)) => { - let range = TextRange::from_to(prev.range().start(), node.range().end()); - if is_trailing_comma(prev.kind(), next.kind()) { - edit.delete(range); - } else if no_space_required(prev.kind(), next.kind()) { - edit.delete(node.range()); - } else if prev.kind() == COMMA && next.kind() == R_CURLY { - edit.replace(range, " ".to_string()); + + if let (Some(prev), Some(next)) = (node.prev_sibling(), node.next_sibling()) { + let range = TextRange::from_to(prev.range().start(), node.range().end()); + if is_trailing_comma(prev.kind(), next.kind()) { + // Removes: trailing comma, newline (incl. surrounding whitespace) + edit.delete(range); + } else if no_space_required(prev.kind(), next.kind()) { + // Removes: newline (incl. surrounding whitespace) + edit.delete(node.range()); + } else if prev.kind() == COMMA && next.kind() == R_CURLY { + // Removes: comma, newline (incl. surrounding whitespace) + // Adds: a single whitespace + edit.replace(range, " ".to_string()); + } else if prev.kind() == COMMENT && next.kind() == COMMENT { + // Removes: newline (incl. surrounding whitespace), start of the next comment + + // FIXME: I guess it is safe to unwrap here? A comment always has text, right? + let comment_text = next.leaf_text().unwrap().as_str(); + let comment_start_length = comment_start_length(comment_text); + + if let Some(newline_pos) = comment_text.find('\n') { + // Special case for multi-line c-like comments: join the comment content but + // keep the leading `/*` + + let newline_offset = next.range().start() + + TextUnit::from(newline_pos as u32) + + TextUnit::of_char('\n'); + + edit.insert(newline_offset, "/*".to_string()); + edit.delete(TextRange::from_to( + node.range().start(), + next.range().start() + comment_start_length + )); } else { - edit.replace( - node.range(), - compute_ws(prev, next).to_string(), - ); + // Single-line comments + edit.delete(TextRange::from_to( + node.range().start(), + next.range().start() + comment_start_length + )); } - return; + } else { + // Remove newline but add a computed amount of whitespace characters + edit.replace( + node.range(), + compute_ws(prev, next).to_string(), + ); } - _ => (), + + return; } } + // FIXME: do we ever reach this point? What does it mean to be here? I think we should document it let suff = &node_text[TextRange::from_to( offset - node.range().start() + TextUnit::of_char('\n'), TextUnit::of_str(node_text), @@ -176,6 +218,13 @@ fn remove_newline( ); } +// Return the start length of the comment (e.g. 2 for `//` and 3 for `//!`) +fn comment_start_length(_text: &str) -> TextUnit { + // TODO: use the parser here instead of reimplementing comment parsing? + // Otherwise, reimplement comment parsing :) + return TextUnit::from(2); +} + fn is_trailing_comma(left: SyntaxKind, right: SyntaxKind) -> bool { match (left, right) { (COMMA, R_PAREN) | (COMMA, R_BRACK) => true, -- cgit v1.2.3 From f88e13f5393c75b02c3619ec432675c3316ee6e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20Ochagav=C3=ADa?= Date: Thu, 11 Oct 2018 16:25:35 +0200 Subject: Use Comment wrapper --- crates/ra_editor/src/typing.rs | 52 +++++++++++------------------------ crates/ra_syntax/src/ast/generated.rs | 18 ++++++++++++ crates/ra_syntax/src/ast/mod.rs | 43 +++++++++++++++++++++++++++++ crates/ra_syntax/src/grammar.ron | 1 + 4 files changed, 78 insertions(+), 36 deletions(-) (limited to 'crates') diff --git a/crates/ra_editor/src/typing.rs b/crates/ra_editor/src/typing.rs index 6c1a91ffb..ae82ff89b 100644 --- a/crates/ra_editor/src/typing.rs +++ b/crates/ra_editor/src/typing.rs @@ -58,14 +58,19 @@ pub fn join_lines(file: &File, range: TextRange) -> LocalEdit { } pub fn on_enter(file: &File, offset: TextUnit) -> Option { - let comment = find_leaf_at_offset(file.syntax(), offset).left_biased().filter(|it| it.kind() == COMMENT)?; - let prefix = comment_preffix(comment)?; - if offset < comment.range().start() + TextUnit::of_str(prefix) { + let comment = find_leaf_at_offset(file.syntax(), offset).left_biased().and_then(|it| ast::Comment::cast(it))?; + + if let ast::CommentFlavor::Multiline = comment.flavor() { + return None; + } + + let prefix = comment.prefix(); + if offset < comment.syntax().range().start() + TextUnit::of_str(prefix) + TextUnit::from(1) { return None; } - let indent = node_indent(file, comment)?; - let inserted = format!("\n{}{}", indent, prefix); + let indent = node_indent(file, comment.syntax())?; + let inserted = format!("\n{}{} ", indent, prefix); let cursor_position = offset + TextUnit::of_str(&inserted); let mut edit = EditBuilder::new(); edit.insert(offset, inserted); @@ -75,20 +80,6 @@ pub fn on_enter(file: &File, offset: TextUnit) -> Option { }) } -fn comment_preffix(comment: SyntaxNodeRef) -> Option<&'static str> { - let text = comment.leaf_text().unwrap(); - let res = if text.starts_with("///") { - "/// " - } else if text.starts_with("//!") { - "//! " - } else if text.starts_with("//") { - "// " - } else { - return None; - }; - Some(res) -} - fn node_indent<'a>(file: &'a File, node: SyntaxNodeRef) -> Option<&'a str> { let ws = match find_leaf_at_offset(file.syntax(), node.range().start()) { LeafAtOffset::Between(l, r) => { @@ -166,31 +157,27 @@ fn remove_newline( // Removes: comma, newline (incl. surrounding whitespace) // Adds: a single whitespace edit.replace(range, " ".to_string()); - } else if prev.kind() == COMMENT && next.kind() == COMMENT { + } else if let (Some(_), Some(next)) = (ast::Comment::cast(prev), ast::Comment::cast(next)) { // Removes: newline (incl. surrounding whitespace), start of the next comment - - // FIXME: I guess it is safe to unwrap here? A comment always has text, right? - let comment_text = next.leaf_text().unwrap().as_str(); - let comment_start_length = comment_start_length(comment_text); - + let comment_text = next.text(); if let Some(newline_pos) = comment_text.find('\n') { // Special case for multi-line c-like comments: join the comment content but // keep the leading `/*` - let newline_offset = next.range().start() + let newline_offset = next.syntax().range().start() + TextUnit::from(newline_pos as u32) + TextUnit::of_char('\n'); edit.insert(newline_offset, "/*".to_string()); edit.delete(TextRange::from_to( node.range().start(), - next.range().start() + comment_start_length + next.syntax().range().start() + TextUnit::of_str(next.prefix()) )); } else { // Single-line comments edit.delete(TextRange::from_to( node.range().start(), - next.range().start() + comment_start_length + next.syntax().range().start() + TextUnit::of_str(next.prefix()) )); } } else { @@ -205,7 +192,7 @@ fn remove_newline( } } - // FIXME: do we ever reach this point? What does it mean to be here? I think we should document it + // The node is either the first or the last in the file let suff = &node_text[TextRange::from_to( offset - node.range().start() + TextUnit::of_char('\n'), TextUnit::of_str(node_text), @@ -218,13 +205,6 @@ fn remove_newline( ); } -// Return the start length of the comment (e.g. 2 for `//` and 3 for `//!`) -fn comment_start_length(_text: &str) -> TextUnit { - // TODO: use the parser here instead of reimplementing comment parsing? - // Otherwise, reimplement comment parsing :) - return TextUnit::from(2); -} - fn is_trailing_comma(left: SyntaxKind, right: SyntaxKind) -> bool { match (left, right) { (COMMA, R_PAREN) | (COMMA, R_BRACK) => true, diff --git a/crates/ra_syntax/src/ast/generated.rs b/crates/ra_syntax/src/ast/generated.rs index 2db6dff1b..b0b855eb4 100644 --- a/crates/ra_syntax/src/ast/generated.rs +++ b/crates/ra_syntax/src/ast/generated.rs @@ -227,6 +227,24 @@ impl<'a> AstNode<'a> for CastExpr<'a> { impl<'a> CastExpr<'a> {} +// Comment +#[derive(Debug, Clone, Copy)] +pub struct Comment<'a> { + syntax: SyntaxNodeRef<'a>, +} + +impl<'a> AstNode<'a> for Comment<'a> { + fn cast(syntax: SyntaxNodeRef<'a>) -> Option { + match syntax.kind() { + COMMENT => Some(Comment { syntax }), + _ => None, + } + } + fn syntax(self) -> SyntaxNodeRef<'a> { self.syntax } +} + +impl<'a> Comment<'a> {} + // Condition #[derive(Debug, Clone, Copy)] pub struct Condition<'a> { diff --git a/crates/ra_syntax/src/ast/mod.rs b/crates/ra_syntax/src/ast/mod.rs index c1570b868..10dac72e5 100644 --- a/crates/ra_syntax/src/ast/mod.rs +++ b/crates/ra_syntax/src/ast/mod.rs @@ -99,6 +99,49 @@ impl<'a> Lifetime<'a> { } } +impl<'a> Comment<'a> { + pub fn text(&self) -> SmolStr { + self.syntax().leaf_text().unwrap().clone() + } + + pub fn flavor(&self) -> CommentFlavor { + let text = self.text(); + if text.starts_with("///") { + CommentFlavor::Doc + } else if text.starts_with("//!") { + CommentFlavor::ModuleDoc + } else if text.starts_with("//") { + CommentFlavor::Line + } else { + CommentFlavor::Multiline + } + } + + pub fn prefix(&self) -> &'static str { + self.flavor().prefix() + } +} + +#[derive(Debug)] +pub enum CommentFlavor { + Line, + Doc, + ModuleDoc, + Multiline +} + +impl CommentFlavor { + pub fn prefix(&self) -> &'static str { + use self::CommentFlavor::*; + match *self { + Line => "//", + Doc => "///", + ModuleDoc => "//!", + Multiline => "/*" + } + } +} + impl<'a> Name<'a> { pub fn text(&self) -> SmolStr { let ident = self.syntax().first_child() diff --git a/crates/ra_syntax/src/grammar.ron b/crates/ra_syntax/src/grammar.ron index 4b990fd8d..9da0c2c13 100644 --- a/crates/ra_syntax/src/grammar.ron +++ b/crates/ra_syntax/src/grammar.ron @@ -537,5 +537,6 @@ Grammar( "PathSegment": ( options: [ "NameRef" ] ), + "Comment": (), }, ) -- cgit v1.2.3 From 5508c91b3e430689e00903d6fe0ad26eb1f00a9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20Ochagav=C3=ADa?= Date: Thu, 11 Oct 2018 16:45:52 +0200 Subject: Remove nesting --- crates/ra_editor/src/typing.rs | 143 ++++++++++++++++++++--------------------- 1 file changed, 68 insertions(+), 75 deletions(-) (limited to 'crates') diff --git a/crates/ra_editor/src/typing.rs b/crates/ra_editor/src/typing.rs index ae82ff89b..3ebdf153e 100644 --- a/crates/ra_editor/src/typing.rs +++ b/crates/ra_editor/src/typing.rs @@ -131,78 +131,77 @@ fn remove_newline( node_text: &str, offset: TextUnit, ) { - if node.kind() == WHITESPACE && node_text.bytes().filter(|&b| b == b'\n').count() == 1 { - // Special case that turns something like: - // - // ``` - // my_function({<|> - // - // }) - // ``` - // - // into `my_function()` - if join_single_expr_block(edit, node).is_some() { - return - } + if node.kind() != WHITESPACE || node_text.bytes().filter(|&b| b == b'\n').count() != 1 { + // The node is either the first or the last in the file + let suff = &node_text[TextRange::from_to( + offset - node.range().start() + TextUnit::of_char('\n'), + TextUnit::of_str(node_text), + )]; + let spaces = suff.bytes().take_while(|&b| b == b' ').count(); + + edit.replace( + TextRange::offset_len(offset, ((spaces + 1) as u32).into()), + " ".to_string(), + ); + return; + } - if let (Some(prev), Some(next)) = (node.prev_sibling(), node.next_sibling()) { - let range = TextRange::from_to(prev.range().start(), node.range().end()); - if is_trailing_comma(prev.kind(), next.kind()) { - // Removes: trailing comma, newline (incl. surrounding whitespace) - edit.delete(range); - } else if no_space_required(prev.kind(), next.kind()) { - // Removes: newline (incl. surrounding whitespace) - edit.delete(node.range()); - } else if prev.kind() == COMMA && next.kind() == R_CURLY { - // Removes: comma, newline (incl. surrounding whitespace) - // Adds: a single whitespace - edit.replace(range, " ".to_string()); - } else if let (Some(_), Some(next)) = (ast::Comment::cast(prev), ast::Comment::cast(next)) { - // Removes: newline (incl. surrounding whitespace), start of the next comment - let comment_text = next.text(); - if let Some(newline_pos) = comment_text.find('\n') { - // Special case for multi-line c-like comments: join the comment content but - // keep the leading `/*` - - let newline_offset = next.syntax().range().start() - + TextUnit::from(newline_pos as u32) - + TextUnit::of_char('\n'); - - edit.insert(newline_offset, "/*".to_string()); - edit.delete(TextRange::from_to( - node.range().start(), - next.syntax().range().start() + TextUnit::of_str(next.prefix()) - )); - } else { - // Single-line comments - edit.delete(TextRange::from_to( - node.range().start(), - next.syntax().range().start() + TextUnit::of_str(next.prefix()) - )); - } - } else { - // Remove newline but add a computed amount of whitespace characters - edit.replace( - node.range(), - compute_ws(prev, next).to_string(), - ); - } + // Special case that turns something like: + // + // ``` + // my_function({<|> + // + // }) + // ``` + // + // into `my_function()` + if join_single_expr_block(edit, node).is_some() { + return + } - return; + // The node is between two other nodes + let prev = node.prev_sibling().unwrap(); + let next = node.next_sibling().unwrap(); + if is_trailing_comma(prev.kind(), next.kind()) { + // Removes: trailing comma, newline (incl. surrounding whitespace) + edit.delete(TextRange::from_to(prev.range().start(), node.range().end())); + } else if prev.kind() == COMMA && next.kind() == R_CURLY { + // Removes: comma, newline (incl. surrounding whitespace) + // Adds: a single whitespace + edit.replace( + TextRange::from_to(prev.range().start(), node.range().end()), + " ".to_string() + ); + } else if let (Some(_), Some(next)) = (ast::Comment::cast(prev), ast::Comment::cast(next)) { + // Removes: newline (incl. surrounding whitespace), start of the next comment + let comment_text = next.text(); + if let Some(newline_pos) = comment_text.find('\n') { + // Special case for multiline comments: join the comment content but + // keep the leading `/*` + + let newline_offset = next.syntax().range().start() + + TextUnit::from(newline_pos as u32) + + TextUnit::of_char('\n'); + + edit.insert(newline_offset, "/*".to_string()); + edit.delete(TextRange::from_to( + node.range().start(), + next.syntax().range().start() + TextUnit::of_str(next.prefix()) + )); + } else { + // Single-line comments + edit.delete(TextRange::from_to( + node.range().start(), + next.syntax().range().start() + TextUnit::of_str(next.prefix()) + )); } + } else { + // Remove newline but add a computed amount of whitespace characters + edit.replace( + node.range(), + compute_ws(prev, next).to_string(), + ); } - - // The node is either the first or the last in the file - let suff = &node_text[TextRange::from_to( - offset - node.range().start() + TextUnit::of_char('\n'), - TextUnit::of_str(node_text), - )]; - let spaces = suff.bytes().take_while(|&b| b == b' ').count(); - - edit.replace( - TextRange::offset_len(offset, ((spaces + 1) as u32).into()), - " ".to_string(), - ); } fn is_trailing_comma(left: SyntaxKind, right: SyntaxKind) -> bool { @@ -212,13 +211,6 @@ fn is_trailing_comma(left: SyntaxKind, right: SyntaxKind) -> bool { } } -fn no_space_required(left: SyntaxKind, right: SyntaxKind) -> bool { - match (left, right) { - (_, DOT) => true, - _ => false - } -} - fn join_single_expr_block( edit: &mut EditBuilder, node: SyntaxNodeRef, @@ -260,6 +252,7 @@ fn compute_ws(left: SyntaxNodeRef, right: SyntaxNodeRef) -> &'static str { } match right.kind() { R_PAREN | R_BRACK => return "", + DOT => return "", _ => (), } " " -- cgit v1.2.3 From 92f5ca64ae09ab996e83acb4017e355437eddc86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20Ochagav=C3=ADa?= Date: Thu, 11 Oct 2018 17:11:00 +0200 Subject: Add tests --- crates/ra_editor/src/typing.rs | 56 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) (limited to 'crates') diff --git a/crates/ra_editor/src/typing.rs b/crates/ra_editor/src/typing.rs index 3ebdf153e..97ff01e40 100644 --- a/crates/ra_editor/src/typing.rs +++ b/crates/ra_editor/src/typing.rs @@ -313,6 +313,62 @@ fn foo() { }"); } + #[test] + fn test_join_lines_normal_comments() { + check_join_lines(r" +fn foo() { + // Hello<|> + // world! +} +", r" +fn foo() { + // Hello<|> world! +} +"); + } + + #[test] + fn test_join_lines_doc_comments() { + check_join_lines(r" +fn foo() { + /// Hello<|> + /// world! +} +", r" +fn foo() { + /// Hello<|> world! +} +"); + } + + #[test] + fn test_join_lines_mod_comments() { + check_join_lines(r" +fn foo() { + //! Hello<|> + //! world! +} +", r" +fn foo() { + //! Hello<|> world! +} +"); + } + + #[test] + fn test_join_lines_multiline_comments() { + check_join_lines(r" +fn foo() { + // Hello<|> + /* world! */ +} +", r" +fn foo() { + // Hello<|> world! */ +} +"); + } + fn check_join_lines_sel(before: &str, after: &str) { let (sel, before) = extract_range(before); let file = File::parse(&before); -- cgit v1.2.3 From 6fe77db41307da8ead8a0b0355488221b61c0349 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20Ochagav=C3=ADa?= Date: Thu, 11 Oct 2018 17:16:12 +0200 Subject: Remove smart multiline comment join --- crates/ra_editor/src/typing.rs | 45 +++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 22 deletions(-) (limited to 'crates') diff --git a/crates/ra_editor/src/typing.rs b/crates/ra_editor/src/typing.rs index 97ff01e40..1dc658f9b 100644 --- a/crates/ra_editor/src/typing.rs +++ b/crates/ra_editor/src/typing.rs @@ -174,27 +174,10 @@ fn remove_newline( ); } else if let (Some(_), Some(next)) = (ast::Comment::cast(prev), ast::Comment::cast(next)) { // Removes: newline (incl. surrounding whitespace), start of the next comment - let comment_text = next.text(); - if let Some(newline_pos) = comment_text.find('\n') { - // Special case for multiline comments: join the comment content but - // keep the leading `/*` - - let newline_offset = next.syntax().range().start() - + TextUnit::from(newline_pos as u32) - + TextUnit::of_char('\n'); - - edit.insert(newline_offset, "/*".to_string()); - edit.delete(TextRange::from_to( - node.range().start(), - next.syntax().range().start() + TextUnit::of_str(next.prefix()) - )); - } else { - // Single-line comments - edit.delete(TextRange::from_to( - node.range().start(), - next.syntax().range().start() + TextUnit::of_str(next.prefix()) - )); - } + edit.delete(TextRange::from_to( + node.range().start(), + next.syntax().range().start() + TextUnit::of_str(next.prefix()) + )); } else { // Remove newline but add a computed amount of whitespace characters edit.replace( @@ -356,7 +339,7 @@ fn foo() { } #[test] - fn test_join_lines_multiline_comments() { + fn test_join_lines_multiline_comments_1() { check_join_lines(r" fn foo() { // Hello<|> @@ -369,6 +352,24 @@ fn foo() { "); } + #[test] + fn test_join_lines_multiline_comments_2() { + check_join_lines(r" +fn foo() { + // The<|> + /* quick + brown + fox! */ +} +", r" +fn foo() { + // The<|> quick + brown + fox! */ +} +"); + } + fn check_join_lines_sel(before: &str, after: &str) { let (sel, before) = extract_range(before); let file = File::parse(&before); -- cgit v1.2.3