aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbors[bot] <bors[bot]@users.noreply.github.com>2018-10-15 17:48:17 +0100
committerbors[bot] <bors[bot]@users.noreply.github.com>2018-10-15 17:48:17 +0100
commita230b438e025c5878b8d17e11d3928cbad95bb28 (patch)
tree231872915ede46874b6e7d2b3d1240d1defa5231
parente031b65f93f73164a5729cf81ff60299708bc931 (diff)
parent2bc9e9f32711047b06940c335eb5327281f8c555 (diff)
Merge #127
127: Improve folding r=matklad a=aochagavia I was messing around with adding support for multiline comments in folding and ended up changing a bunch of other things. First of all, I am not convinced of folding groups of successive items. For instance, I don't see why it is worthwhile to be able to fold something like the following: ```rust use foo; use bar; ``` Furthermore, this causes problems if you want to fold a multiline import: ```rust use foo::{ quux }; use bar; ``` The problem is that now there are two possible folds at the same position: we could fold the first use or we could fold the import group. IMO, the only place where folding groups makes sense is when folding comments. Therefore I have **removed folding import groups in favor of folding multiline imports**. Regarding folding comments, I made it a bit more robust by requiring that comments can only be folded if they have the same flavor. So if you have a bunch of `//` comments followed by `//!` comments, you will get two separate fold groups instead of a single one. Finally, I rewrote the API in such a way that it should be trivial to add new folds. You only need to: * Create a new FoldKind * Add it to the `fold_kind` function that converts from `SyntaxKind` to `FoldKind` Fixes #113 Co-authored-by: Adolfo OchagavĂ­a <[email protected]>
-rw-r--r--crates/ra_editor/src/folding_ranges.rs177
-rw-r--r--crates/ra_syntax/src/ast/generated.rs18
-rw-r--r--crates/ra_syntax/src/ast/mod.rs28
-rw-r--r--crates/ra_syntax/src/grammar.ron1
-rw-r--r--crates/test_utils/src/lib.rs38
-rw-r--r--editors/code/src/extension.ts4
6 files changed, 187 insertions, 79 deletions
diff --git a/crates/ra_editor/src/folding_ranges.rs b/crates/ra_editor/src/folding_ranges.rs
index 3aabd54ae..a1699d449 100644
--- a/crates/ra_editor/src/folding_ranges.rs
+++ b/crates/ra_editor/src/folding_ranges.rs
@@ -1,8 +1,10 @@
1use rustc_hash::FxHashSet; 1use rustc_hash::FxHashSet;
2 2
3use ra_syntax::{ 3use ra_syntax::{
4 ast,
5 AstNode,
4 File, TextRange, SyntaxNodeRef, 6 File, TextRange, SyntaxNodeRef,
5 SyntaxKind, 7 SyntaxKind::{self, *},
6 Direction, 8 Direction,
7}; 9};
8 10
@@ -20,67 +22,97 @@ pub struct Fold {
20 22
21pub fn folding_ranges(file: &File) -> Vec<Fold> { 23pub fn folding_ranges(file: &File) -> Vec<Fold> {
22 let mut res = vec![]; 24 let mut res = vec![];
23 let mut visited = FxHashSet::default(); 25 let mut visited_comments = FxHashSet::default();
24 26
25 for node in file.syntax().descendants() { 27 for node in file.syntax().descendants() {
26 if visited.contains(&node) { 28 // Fold items that span multiple lines
29 if let Some(kind) = fold_kind(node.kind()) {
30 if has_newline(node) {
31 res.push(Fold { range: node.range(), kind });
32 }
33 }
34
35 // Also fold groups of comments
36 if visited_comments.contains(&node) {
27 continue; 37 continue;
28 } 38 }
39 if node.kind() == COMMENT {
40 contiguous_range_for_comment(node, &mut visited_comments)
41 .map(|range| res.push(Fold { range, kind: FoldKind::Comment }));
42 }
43 }
44
45 res
46}
29 47
30 let range_and_kind = match node.kind() { 48fn fold_kind(kind: SyntaxKind) -> Option<FoldKind> {
31 SyntaxKind::COMMENT => ( 49 match kind {
32 contiguous_range_for(SyntaxKind::COMMENT, node, &mut visited), 50 COMMENT => Some(FoldKind::Comment),
33 Some(FoldKind::Comment), 51 USE_ITEM => Some(FoldKind::Imports),
34 ), 52 _ => None
35 SyntaxKind::USE_ITEM => ( 53 }
36 contiguous_range_for(SyntaxKind::USE_ITEM, node, &mut visited), 54}
37 Some(FoldKind::Imports), 55
38 ), 56fn has_newline(
39 _ => (None, None), 57 node: SyntaxNodeRef,
40 }; 58) -> bool {
41 59 for descendant in node.descendants() {
42 match range_and_kind { 60 if let Some(ws) = ast::Whitespace::cast(descendant) {
43 (Some(range), Some(kind)) => { 61 if ws.has_newlines() {
44 res.push(Fold { 62 return true;
45 range: range, 63 }
46 kind: kind 64 } else if let Some(comment) = ast::Comment::cast(descendant) {
47 }); 65 if comment.has_newlines() {
66 return true;
48 } 67 }
49 _ => {}
50 } 68 }
51 } 69 }
52 70
53 res 71 false
54} 72}
55 73
56fn contiguous_range_for<'a>( 74fn contiguous_range_for_comment<'a>(
57 kind: SyntaxKind, 75 first: SyntaxNodeRef<'a>,
58 node: SyntaxNodeRef<'a>,
59 visited: &mut FxHashSet<SyntaxNodeRef<'a>>, 76 visited: &mut FxHashSet<SyntaxNodeRef<'a>>,
60) -> Option<TextRange> { 77) -> Option<TextRange> {
61 visited.insert(node); 78 visited.insert(first);
62 79
63 let left = node; 80 // Only fold comments of the same flavor
64 let mut right = node; 81 let group_flavor = ast::Comment::cast(first)?.flavor();
65 for node in node.siblings(Direction::Next) { 82
66 visited.insert(node); 83 let mut last = first;
67 match node.kind() { 84 for node in first.siblings(Direction::Next) {
68 SyntaxKind::WHITESPACE if !node.leaf_text().unwrap().as_str().contains("\n\n") => (), 85 if let Some(ws) = ast::Whitespace::cast(node) {
69 k => { 86 // There is a blank line, which means the group ends here
70 if k == kind { 87 if ws.count_newlines_lazy().take(2).count() == 2 {
71 right = node 88 break;
72 } else { 89 }
73 break; 90
74 } 91 // Ignore whitespace without blank lines
92 continue;
93 }
94
95 match ast::Comment::cast(node) {
96 Some(next_comment) if next_comment.flavor() == group_flavor => {
97 visited.insert(node);
98 last = node;
99 }
100 // The comment group ends because either:
101 // * An element of a different kind was reached
102 // * A comment of a different flavor was reached
103 _ => {
104 break
75 } 105 }
76 } 106 }
77 } 107 }
78 if left != right { 108
109 if first != last {
79 Some(TextRange::from_to( 110 Some(TextRange::from_to(
80 left.range().start(), 111 first.range().start(),
81 right.range().end(), 112 last.range().end(),
82 )) 113 ))
83 } else { 114 } else {
115 // The group consists of only one element, therefore it cannot be folded
84 None 116 None
85 } 117 }
86} 118}
@@ -88,52 +120,65 @@ fn contiguous_range_for<'a>(
88#[cfg(test)] 120#[cfg(test)]
89mod tests { 121mod tests {
90 use super::*; 122 use super::*;
123 use test_utils::extract_ranges;
124
125 fn do_check(text: &str, fold_kinds: &[FoldKind]) {
126 let (ranges, text) = extract_ranges(text);
127 let file = File::parse(&text);
128 let folds = folding_ranges(&file);
129
130 assert_eq!(folds.len(), ranges.len());
131 for ((fold, range), fold_kind) in folds.into_iter().zip(ranges.into_iter()).zip(fold_kinds.into_iter()) {
132 assert_eq!(fold.range.start(), range.start());
133 assert_eq!(fold.range.end(), range.end());
134 assert_eq!(&fold.kind, fold_kind);
135 }
136 }
91 137
92 #[test] 138 #[test]
93 fn test_fold_comments() { 139 fn test_fold_comments() {
94 let text = r#" 140 let text = r#"
95// Hello 141<|>// Hello
96// this is a multiline 142// this is a multiline
97// comment 143// comment
98// 144//<|>
99 145
100// But this is not 146// But this is not
101 147
102fn main() { 148fn main() {
103 // We should 149 <|>// We should
104 // also 150 // also
105 // fold 151 // fold
106 // this one. 152 // this one.<|>
153 <|>//! But this one is different
154 //! because it has another flavor<|>
155 <|>/* As does this
156 multiline comment */<|>
107}"#; 157}"#;
108 158
109 let file = File::parse(&text); 159 let fold_kinds = &[
110 let folds = folding_ranges(&file); 160 FoldKind::Comment,
111 assert_eq!(folds.len(), 2); 161 FoldKind::Comment,
112 assert_eq!(folds[0].range.start(), 1.into()); 162 FoldKind::Comment,
113 assert_eq!(folds[0].range.end(), 46.into()); 163 FoldKind::Comment,
114 assert_eq!(folds[0].kind, FoldKind::Comment); 164 ];
115 165 do_check(text, fold_kinds);
116 assert_eq!(folds[1].range.start(), 84.into());
117 assert_eq!(folds[1].range.end(), 137.into());
118 assert_eq!(folds[1].kind, FoldKind::Comment);
119 } 166 }
120 167
121 #[test] 168 #[test]
122 fn test_fold_imports() { 169 fn test_fold_imports() {
123 let text = r#" 170 let text = r#"
124use std::str; 171<|>use std::{
125use std::vec; 172 str,
126use std::io as iop; 173 vec,
174 io as iop
175};<|>
127 176
128fn main() { 177fn main() {
129}"#; 178}"#;
130 179
131 let file = File::parse(&text); 180 let folds = &[FoldKind::Imports];
132 let folds = folding_ranges(&file); 181 do_check(text, folds);
133 assert_eq!(folds.len(), 1);
134 assert_eq!(folds[0].range.start(), 1.into());
135 assert_eq!(folds[0].range.end(), 48.into());
136 assert_eq!(folds[0].kind, FoldKind::Imports);
137 } 182 }
138 183
139 184
diff --git a/crates/ra_syntax/src/ast/generated.rs b/crates/ra_syntax/src/ast/generated.rs
index 48c9038dc..4db1bcbf9 100644
--- a/crates/ra_syntax/src/ast/generated.rs
+++ b/crates/ra_syntax/src/ast/generated.rs
@@ -2197,3 +2197,21 @@ impl<'a> WhileExpr<'a> {
2197 } 2197 }
2198} 2198}
2199 2199
2200// Whitespace
2201#[derive(Debug, Clone, Copy)]
2202pub struct Whitespace<'a> {
2203 syntax: SyntaxNodeRef<'a>,
2204}
2205
2206impl<'a> AstNode<'a> for Whitespace<'a> {
2207 fn cast(syntax: SyntaxNodeRef<'a>) -> Option<Self> {
2208 match syntax.kind() {
2209 WHITESPACE => Some(Whitespace { syntax }),
2210 _ => None,
2211 }
2212 }
2213 fn syntax(self) -> SyntaxNodeRef<'a> { self.syntax }
2214}
2215
2216impl<'a> Whitespace<'a> {}
2217
diff --git a/crates/ra_syntax/src/ast/mod.rs b/crates/ra_syntax/src/ast/mod.rs
index 10dac72e5..12ddc0210 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> {
100} 100}
101 101
102impl<'a> Comment<'a> { 102impl<'a> Comment<'a> {
103 pub fn text(&self) -> SmolStr { 103 pub fn text(&self) -> &SmolStr {
104 self.syntax().leaf_text().unwrap().clone() 104 self.syntax().leaf_text().unwrap()
105 } 105 }
106 106
107 pub fn flavor(&self) -> CommentFlavor { 107 pub fn flavor(&self) -> CommentFlavor {
@@ -120,9 +120,17 @@ impl<'a> Comment<'a> {
120 pub fn prefix(&self) -> &'static str { 120 pub fn prefix(&self) -> &'static str {
121 self.flavor().prefix() 121 self.flavor().prefix()
122 } 122 }
123
124 pub fn count_newlines_lazy(&self) -> impl Iterator<Item = &()> {
125 self.text().chars().filter(|&c| c == '\n').map(|_| &())
126 }
127
128 pub fn has_newlines(&self) -> bool {
129 self.count_newlines_lazy().count() > 0
130 }
123} 131}
124 132
125#[derive(Debug)] 133#[derive(Debug, PartialEq, Eq)]
126pub enum CommentFlavor { 134pub enum CommentFlavor {
127 Line, 135 Line,
128 Doc, 136 Doc,
@@ -142,6 +150,20 @@ impl CommentFlavor {
142 } 150 }
143} 151}
144 152
153impl<'a> Whitespace<'a> {
154 pub fn text(&self) -> &SmolStr {
155 &self.syntax().leaf_text().unwrap()
156 }
157
158 pub fn count_newlines_lazy(&self) -> impl Iterator<Item = &()> {
159 self.text().chars().filter(|&c| c == '\n').map(|_| &())
160 }
161
162 pub fn has_newlines(&self) -> bool {
163 self.count_newlines_lazy().count() > 0
164 }
165}
166
145impl<'a> Name<'a> { 167impl<'a> Name<'a> {
146 pub fn text(&self) -> SmolStr { 168 pub fn text(&self) -> SmolStr {
147 let ident = self.syntax().first_child() 169 let ident = self.syntax().first_child()
diff --git a/crates/ra_syntax/src/grammar.ron b/crates/ra_syntax/src/grammar.ron
index a904f7505..ea8063d3b 100644
--- a/crates/ra_syntax/src/grammar.ron
+++ b/crates/ra_syntax/src/grammar.ron
@@ -538,5 +538,6 @@ Grammar(
538 options: [ "NameRef" ] 538 options: [ "NameRef" ]
539 ), 539 ),
540 "Comment": (), 540 "Comment": (),
541 "Whitespace": (),
541 }, 542 },
542) 543)
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) {
38} 38}
39 39
40pub fn extract_offset(text: &str) -> (TextUnit, String) { 40pub fn extract_offset(text: &str) -> (TextUnit, String) {
41 let cursor = "<|>"; 41 match try_extract_offset(text) {
42 let cursor_pos = match text.find(cursor) {
43 None => panic!("text should contain cursor marker"), 42 None => panic!("text should contain cursor marker"),
44 Some(pos) => pos, 43 Some(result) => result,
45 }; 44 }
45}
46
47pub fn try_extract_offset(text: &str) -> Option<(TextUnit, String)> {
48 let cursor = "<|>";
49 let cursor_pos = text.find(cursor)?;
46 let mut new_text = String::with_capacity(text.len() - cursor.len()); 50 let mut new_text = String::with_capacity(text.len() - cursor.len());
47 new_text.push_str(&text[..cursor_pos]); 51 new_text.push_str(&text[..cursor_pos]);
48 new_text.push_str(&text[cursor_pos + cursor.len()..]); 52 new_text.push_str(&text[cursor_pos + cursor.len()..]);
49 let cursor_pos = TextUnit::from(cursor_pos as u32); 53 let cursor_pos = TextUnit::from(cursor_pos as u32);
50 (cursor_pos, new_text) 54 Some((cursor_pos, new_text))
51} 55}
52 56
53pub fn extract_range(text: &str) -> (TextRange, String) { 57pub fn extract_range(text: &str) -> (TextRange, String) {
54 let (start, text) = extract_offset(text); 58 match try_extract_range(text) {
55 let (end, text) = extract_offset(&text); 59 None => panic!("text should contain cursor marker"),
56 (TextRange::from_to(start, end), text) 60 Some(result) => result,
61 }
62}
63
64pub fn try_extract_range(text: &str) -> Option<(TextRange, String)> {
65 let (start, text) = try_extract_offset(text)?;
66 let (end, text) = try_extract_offset(&text)?;
67 Some((TextRange::from_to(start, end), text))
68}
69
70pub fn extract_ranges(text: &str) -> (Vec<TextRange>, String) {
71 let mut ranges = Vec::new();
72 let mut text = String::from(text);
73 while let Some((range, new_text)) = try_extract_range(&text) {
74 text = new_text;
75 ranges.push(range);
76 }
77
78 (ranges, text)
57} 79}
58 80
59pub fn add_cursor(text: &str, offset: TextUnit) -> String { 81pub fn add_cursor(text: &str, offset: TextUnit) -> String {
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) {
20 f: (...args: any[]) => Promise<boolean> 20 f: (...args: any[]) => Promise<boolean>
21 ) { 21 ) {
22 const defaultCmd = `default:${name}`; 22 const defaultCmd = `default:${name}`;
23 const original = async (...args: any[]) => 23 const original = (...args: any[]) =>
24 await vscode.commands.executeCommand(defaultCmd, ...args); 24 vscode.commands.executeCommand(defaultCmd, ...args);
25 25
26 registerCommand(name, async (...args: any[]) => { 26 registerCommand(name, async (...args: any[]) => {
27 const editor = vscode.window.activeTextEditor; 27 const editor = vscode.window.activeTextEditor;