From ee0a6bf0535a5a6c7e536d2cffa11959c3ee2ae3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20Ochagav=C3=ADa?= Date: Fri, 12 Oct 2018 08:59:12 +0200 Subject: Fold multiline comments --- crates/ra_editor/src/folding_ranges.rs | 38 +++++++++++++++++----------------- editors/code/src/extension.ts | 4 ++-- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/crates/ra_editor/src/folding_ranges.rs b/crates/ra_editor/src/folding_ranges.rs index 3aabd54ae..1d89ac853 100644 --- a/crates/ra_editor/src/folding_ranges.rs +++ b/crates/ra_editor/src/folding_ranges.rs @@ -1,6 +1,8 @@ use rustc_hash::FxHashSet; use ra_syntax::{ + ast, + AstNode, File, TextRange, SyntaxNodeRef, SyntaxKind, Direction, @@ -27,27 +29,25 @@ pub fn folding_ranges(file: &File) -> Vec { continue; } - let range_and_kind = match node.kind() { - SyntaxKind::COMMENT => ( - contiguous_range_for(SyntaxKind::COMMENT, node, &mut visited), - Some(FoldKind::Comment), - ), - SyntaxKind::USE_ITEM => ( - contiguous_range_for(SyntaxKind::USE_ITEM, node, &mut visited), - Some(FoldKind::Imports), - ), - _ => (None, None), - }; + if let Some(comment) = ast::Comment::cast(node) { + // Multiline comments (`/* ... */`) can only be folded if they span multiple lines + let range = if let ast::CommentFlavor::Multiline = comment.flavor() { + if comment.text().contains('\n') { + Some(comment.syntax().range()) + } else { + None + } + } else { + contiguous_range_for(SyntaxKind::COMMENT, node, &mut visited) + }; - match range_and_kind { - (Some(range), Some(kind)) => { - res.push(Fold { - range: range, - kind: kind - }); - } - _ => {} + range.map(|range| res.push(Fold { range, kind: FoldKind::Comment })); } + + if let SyntaxKind::USE_ITEM = node.kind() { + contiguous_range_for(SyntaxKind::USE_ITEM, node, &mut visited) + .map(|range| res.push(Fold { range, kind: FoldKind::Imports})); + }; } res diff --git a/editors/code/src/extension.ts b/editors/code/src/extension.ts index ff8f23c7a..d1c525f68 100644 --- a/editors/code/src/extension.ts +++ b/editors/code/src/extension.ts @@ -20,8 +20,8 @@ export function activate(context: vscode.ExtensionContext) { f: (...args: any[]) => Promise ) { const defaultCmd = `default:${name}`; - const original = async (...args: any[]) => - await vscode.commands.executeCommand(defaultCmd, ...args); + const original = (...args: any[]) => + vscode.commands.executeCommand(defaultCmd, ...args); registerCommand(name, async (...args: any[]) => { const editor = vscode.window.activeTextEditor; -- cgit v1.2.3 From 4b3737510b97faa7d2fad3c98aa16eed46334703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20Ochagav=C3=ADa?= Date: Fri, 12 Oct 2018 19:20:58 +0200 Subject: Cleanup fold code and split logic to fold single elements --- crates/ra_editor/src/folding_ranges.rs | 107 +++++++++++++++++++++------------ crates/ra_syntax/src/ast/generated.rs | 18 ++++++ crates/ra_syntax/src/ast/mod.rs | 26 +++++++- crates/ra_syntax/src/grammar.ron | 1 + 4 files changed, 112 insertions(+), 40 deletions(-) diff --git a/crates/ra_editor/src/folding_ranges.rs b/crates/ra_editor/src/folding_ranges.rs index 1d89ac853..d45210ec7 100644 --- a/crates/ra_editor/src/folding_ranges.rs +++ b/crates/ra_editor/src/folding_ranges.rs @@ -22,65 +22,96 @@ pub struct Fold { pub fn folding_ranges(file: &File) -> Vec { let mut res = vec![]; - let mut visited = FxHashSet::default(); + let mut group_members = FxHashSet::default(); for node in file.syntax().descendants() { - if visited.contains(&node) { - continue; + // Fold items that span multiple lines + if let Some(kind) = fold_kind(node.kind()) { + if has_newline(node) { + res.push(Fold { range: node.range(), kind }); + } } - if let Some(comment) = ast::Comment::cast(node) { - // Multiline comments (`/* ... */`) can only be folded if they span multiple lines - let range = if let ast::CommentFlavor::Multiline = comment.flavor() { - if comment.text().contains('\n') { - Some(comment.syntax().range()) - } else { - None - } - } else { - contiguous_range_for(SyntaxKind::COMMENT, node, &mut visited) - }; - - range.map(|range| res.push(Fold { range, kind: FoldKind::Comment })); + // Also fold item *groups* that span multiple lines + + // Note: we need to skip elements of the group that we have already visited, + // otherwise there will be folds for the whole group and for its sub groups + if group_members.contains(&node) { + continue; } - if let SyntaxKind::USE_ITEM = node.kind() { - contiguous_range_for(SyntaxKind::USE_ITEM, node, &mut visited) - .map(|range| res.push(Fold { range, kind: FoldKind::Imports})); - }; + if let Some(kind) = fold_kind(node.kind()) { + contiguous_range_for_group(node.kind(), node, &mut group_members) + .map(|range| res.push(Fold { range, kind })); + } } res } -fn contiguous_range_for<'a>( - kind: SyntaxKind, - node: SyntaxNodeRef<'a>, +fn fold_kind(kind: SyntaxKind) -> Option { + match kind { + SyntaxKind::COMMENT => Some(FoldKind::Comment), + SyntaxKind::USE_ITEM => Some(FoldKind::Imports), + _ => None + } +} + +fn has_newline( + node: SyntaxNodeRef, +) -> bool { + for descendant in node.descendants() { + if let Some(ws) = ast::Whitespace::cast(descendant) { + if ws.has_newlines() { + return true; + } + } else if let Some(comment) = ast::Comment::cast(descendant) { + if comment.has_newlines() { + return true; + } + } + } + + false +} + +fn contiguous_range_for_group<'a>( + group_kind: SyntaxKind, + first: SyntaxNodeRef<'a>, visited: &mut FxHashSet>, ) -> Option { - visited.insert(node); + visited.insert(first); + + let mut last = first; - let left = node; - let mut right = node; - for node in node.siblings(Direction::Next) { + for node in first.siblings(Direction::Next) { visited.insert(node); - match node.kind() { - SyntaxKind::WHITESPACE if !node.leaf_text().unwrap().as_str().contains("\n\n") => (), - k => { - if k == kind { - right = node - } else { - break; - } + if let Some(ws) = ast::Whitespace::cast(node) { + // There is a blank line, which means the group ends here + if ws.count_newlines_lazy().take(2).count() == 2 { + break; } + + // Ignore whitespace without blank lines + continue; + } + + // The group ends when an element of a different kind is reached + if node.kind() != group_kind { + break; } + + // Keep track of the last node in the group + last = node; } - if left != right { + + if first != last { Some(TextRange::from_to( - left.range().start(), - right.range().end(), + first.range().start(), + last.range().end(), )) } else { + // The group consists of only one element, therefore it cannot be folded None } } diff --git a/crates/ra_syntax/src/ast/generated.rs b/crates/ra_syntax/src/ast/generated.rs index ef7b5b1a1..85aa5e0dd 100644 --- a/crates/ra_syntax/src/ast/generated.rs +++ b/crates/ra_syntax/src/ast/generated.rs @@ -2193,3 +2193,21 @@ impl<'a> WhileExpr<'a> { } } +// Whitespace +#[derive(Debug, Clone, Copy)] +pub struct Whitespace<'a> { + syntax: SyntaxNodeRef<'a>, +} + +impl<'a> AstNode<'a> for Whitespace<'a> { + fn cast(syntax: SyntaxNodeRef<'a>) -> Option { + match syntax.kind() { + WHITESPACE => Some(Whitespace { syntax }), + _ => None, + } + } + fn syntax(self) -> SyntaxNodeRef<'a> { self.syntax } +} + +impl<'a> Whitespace<'a> {} + diff --git a/crates/ra_syntax/src/ast/mod.rs b/crates/ra_syntax/src/ast/mod.rs index 10dac72e5..00c852274 100644 --- a/crates/ra_syntax/src/ast/mod.rs +++ b/crates/ra_syntax/src/ast/mod.rs @@ -100,8 +100,8 @@ impl<'a> Lifetime<'a> { } impl<'a> Comment<'a> { - pub fn text(&self) -> SmolStr { - self.syntax().leaf_text().unwrap().clone() + pub fn text(&self) -> &SmolStr { + self.syntax().leaf_text().unwrap() } pub fn flavor(&self) -> CommentFlavor { @@ -120,6 +120,14 @@ impl<'a> Comment<'a> { pub fn prefix(&self) -> &'static str { self.flavor().prefix() } + + pub fn count_newlines_lazy(&self) -> impl Iterator { + self.text().chars().filter(|&c| c == '\n').map(|_| &()) + } + + pub fn has_newlines(&self) -> bool { + self.count_newlines_lazy().count() > 0 + } } #[derive(Debug)] @@ -142,6 +150,20 @@ impl CommentFlavor { } } +impl<'a> Whitespace<'a> { + pub fn text(&self) -> &SmolStr { + &self.syntax().leaf_text().unwrap() + } + + pub fn count_newlines_lazy(&self) -> impl Iterator { + self.text().chars().filter(|&c| c == '\n').map(|_| &()) + } + + pub fn has_newlines(&self) -> bool { + self.count_newlines_lazy().count() > 0 + } +} + 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 9da0c2c13..d538739de 100644 --- a/crates/ra_syntax/src/grammar.ron +++ b/crates/ra_syntax/src/grammar.ron @@ -538,5 +538,6 @@ Grammar( options: [ "NameRef" ] ), "Comment": (), + "Whitespace": (), }, ) -- cgit v1.2.3 From c5069eeef5ba0260f2daed513a28b43ae45445bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20Ochagav=C3=ADa?= Date: Fri, 12 Oct 2018 19:49:08 +0200 Subject: Only fold groups of similar comments --- crates/ra_editor/src/folding_ranges.rs | 57 ++++++++++++++++++---------------- crates/ra_syntax/src/ast/mod.rs | 2 +- 2 files changed, 31 insertions(+), 28 deletions(-) diff --git a/crates/ra_editor/src/folding_ranges.rs b/crates/ra_editor/src/folding_ranges.rs index d45210ec7..8aabb5e0b 100644 --- a/crates/ra_editor/src/folding_ranges.rs +++ b/crates/ra_editor/src/folding_ranges.rs @@ -4,7 +4,7 @@ use ra_syntax::{ ast, AstNode, File, TextRange, SyntaxNodeRef, - SyntaxKind, + SyntaxKind::{self, *}, Direction, }; @@ -22,7 +22,7 @@ pub struct Fold { pub fn folding_ranges(file: &File) -> Vec { let mut res = vec![]; - let mut group_members = FxHashSet::default(); + let mut visited_comments = FxHashSet::default(); for node in file.syntax().descendants() { // Fold items that span multiple lines @@ -32,17 +32,13 @@ pub fn folding_ranges(file: &File) -> Vec { } } - // Also fold item *groups* that span multiple lines - - // Note: we need to skip elements of the group that we have already visited, - // otherwise there will be folds for the whole group and for its sub groups - if group_members.contains(&node) { + // Also fold groups of comments + if visited_comments.contains(&node) { continue; } - - if let Some(kind) = fold_kind(node.kind()) { - contiguous_range_for_group(node.kind(), node, &mut group_members) - .map(|range| res.push(Fold { range, kind })); + if node.kind() == COMMENT { + contiguous_range_for_comment(node, &mut visited_comments) + .map(|range| res.push(Fold { range, kind: FoldKind::Comment })); } } @@ -51,8 +47,8 @@ pub fn folding_ranges(file: &File) -> Vec { fn fold_kind(kind: SyntaxKind) -> Option { match kind { - SyntaxKind::COMMENT => Some(FoldKind::Comment), - SyntaxKind::USE_ITEM => Some(FoldKind::Imports), + COMMENT => Some(FoldKind::Comment), + USE_ITEM => Some(FoldKind::Imports), _ => None } } @@ -75,17 +71,17 @@ fn has_newline( false } -fn contiguous_range_for_group<'a>( - group_kind: SyntaxKind, +fn contiguous_range_for_comment<'a>( first: SyntaxNodeRef<'a>, visited: &mut FxHashSet>, ) -> Option { visited.insert(first); - let mut last = first; + // Only fold comments of the same flavor + let group_flavor = ast::Comment::cast(first)?.flavor(); + let mut last = first; for node in first.siblings(Direction::Next) { - visited.insert(node); if let Some(ws) = ast::Whitespace::cast(node) { // There is a blank line, which means the group ends here if ws.count_newlines_lazy().take(2).count() == 2 { @@ -96,13 +92,18 @@ fn contiguous_range_for_group<'a>( continue; } - // The group ends when an element of a different kind is reached - if node.kind() != group_kind { - break; + match ast::Comment::cast(node) { + Some(next_comment) if next_comment.flavor() == group_flavor => { + visited.insert(node); + last = node; + } + // The comment group ends because either: + // * An element of a different kind was reached + // * A comment of a different flavor was reached + _ => { + break + } } - - // Keep track of the last node in the group - last = node; } if first != last { @@ -152,9 +153,11 @@ fn main() { #[test] fn test_fold_imports() { let text = r#" -use std::str; -use std::vec; -use std::io as iop; +use std::{ + str, + vec, + io as iop +}; fn main() { }"#; @@ -163,7 +166,7 @@ fn main() { let folds = folding_ranges(&file); assert_eq!(folds.len(), 1); assert_eq!(folds[0].range.start(), 1.into()); - assert_eq!(folds[0].range.end(), 48.into()); + assert_eq!(folds[0].range.end(), 46.into()); assert_eq!(folds[0].kind, FoldKind::Imports); } diff --git a/crates/ra_syntax/src/ast/mod.rs b/crates/ra_syntax/src/ast/mod.rs index 00c852274..12ddc0210 100644 --- a/crates/ra_syntax/src/ast/mod.rs +++ b/crates/ra_syntax/src/ast/mod.rs @@ -130,7 +130,7 @@ impl<'a> Comment<'a> { } } -#[derive(Debug)] +#[derive(Debug, PartialEq, Eq)] pub enum CommentFlavor { Line, Doc, -- cgit v1.2.3 From 2bc9e9f32711047b06940c335eb5327281f8c555 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20Ochagav=C3=ADa?= Date: Sat, 13 Oct 2018 21:33:15 +0200 Subject: Improve tests --- crates/ra_editor/src/folding_ranges.rs | 55 ++++++++++++++++++++-------------- crates/test_utils/src/lib.rs | 38 ++++++++++++++++++----- 2 files changed, 63 insertions(+), 30 deletions(-) diff --git a/crates/ra_editor/src/folding_ranges.rs b/crates/ra_editor/src/folding_ranges.rs index 8aabb5e0b..a1699d449 100644 --- a/crates/ra_editor/src/folding_ranges.rs +++ b/crates/ra_editor/src/folding_ranges.rs @@ -120,54 +120,65 @@ fn contiguous_range_for_comment<'a>( #[cfg(test)] mod tests { use super::*; + use test_utils::extract_ranges; + + fn do_check(text: &str, fold_kinds: &[FoldKind]) { + let (ranges, text) = extract_ranges(text); + let file = File::parse(&text); + let folds = folding_ranges(&file); + + assert_eq!(folds.len(), ranges.len()); + for ((fold, range), fold_kind) in folds.into_iter().zip(ranges.into_iter()).zip(fold_kinds.into_iter()) { + assert_eq!(fold.range.start(), range.start()); + assert_eq!(fold.range.end(), range.end()); + assert_eq!(&fold.kind, fold_kind); + } + } #[test] fn test_fold_comments() { let text = r#" -// Hello +<|>// Hello // this is a multiline // comment -// +//<|> // But this is not fn main() { - // We should + <|>// We should // also // fold - // this one. + // this one.<|> + <|>//! But this one is different + //! because it has another flavor<|> + <|>/* As does this + multiline comment */<|> }"#; - let file = File::parse(&text); - let folds = folding_ranges(&file); - assert_eq!(folds.len(), 2); - assert_eq!(folds[0].range.start(), 1.into()); - assert_eq!(folds[0].range.end(), 46.into()); - assert_eq!(folds[0].kind, FoldKind::Comment); - - assert_eq!(folds[1].range.start(), 84.into()); - assert_eq!(folds[1].range.end(), 137.into()); - assert_eq!(folds[1].kind, FoldKind::Comment); + let fold_kinds = &[ + FoldKind::Comment, + FoldKind::Comment, + FoldKind::Comment, + FoldKind::Comment, + ]; + do_check(text, fold_kinds); } #[test] fn test_fold_imports() { let text = r#" -use std::{ +<|>use std::{ str, vec, io as iop -}; +};<|> fn main() { }"#; - let file = File::parse(&text); - let folds = folding_ranges(&file); - assert_eq!(folds.len(), 1); - assert_eq!(folds[0].range.start(), 1.into()); - assert_eq!(folds[0].range.end(), 46.into()); - assert_eq!(folds[0].kind, FoldKind::Imports); + let folds = &[FoldKind::Imports]; + do_check(text, folds); } diff --git a/crates/test_utils/src/lib.rs b/crates/test_utils/src/lib.rs index 068eb80ce..ee73153f0 100644 --- a/crates/test_utils/src/lib.rs +++ b/crates/test_utils/src/lib.rs @@ -38,22 +38,44 @@ pub fn assert_eq_dbg(expected: &str, actual: &impl fmt::Debug) { } pub fn extract_offset(text: &str) -> (TextUnit, String) { - let cursor = "<|>"; - let cursor_pos = match text.find(cursor) { + match try_extract_offset(text) { None => panic!("text should contain cursor marker"), - Some(pos) => pos, - }; + Some(result) => result, + } +} + +pub fn try_extract_offset(text: &str) -> Option<(TextUnit, String)> { + let cursor = "<|>"; + let cursor_pos = text.find(cursor)?; let mut new_text = String::with_capacity(text.len() - cursor.len()); new_text.push_str(&text[..cursor_pos]); new_text.push_str(&text[cursor_pos + cursor.len()..]); let cursor_pos = TextUnit::from(cursor_pos as u32); - (cursor_pos, new_text) + Some((cursor_pos, new_text)) } pub fn extract_range(text: &str) -> (TextRange, String) { - let (start, text) = extract_offset(text); - let (end, text) = extract_offset(&text); - (TextRange::from_to(start, end), text) + match try_extract_range(text) { + None => panic!("text should contain cursor marker"), + Some(result) => result, + } +} + +pub fn try_extract_range(text: &str) -> Option<(TextRange, String)> { + let (start, text) = try_extract_offset(text)?; + let (end, text) = try_extract_offset(&text)?; + Some((TextRange::from_to(start, end), text)) +} + +pub fn extract_ranges(text: &str) -> (Vec, String) { + let mut ranges = Vec::new(); + let mut text = String::from(text); + while let Some((range, new_text)) = try_extract_range(&text) { + text = new_text; + ranges.push(range); + } + + (ranges, text) } pub fn add_cursor(text: &str, offset: TextUnit) -> String { -- cgit v1.2.3