Finish generalizing ToggleComments to support block comments

Max Brunsfeld created

Change summary

crates/editor/src/editor.rs       | 102 +++++++++++++++++++++++++++++++-
crates/language/src/buffer.rs     |   1 
crates/language/src/syntax_map.rs |  47 ++++++++------
3 files changed, 126 insertions(+), 24 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -4492,6 +4492,7 @@ impl Editor {
             let mut last_toggled_row = None;
             let snapshot = this.buffer.read(cx).read(cx);
             let empty_str: Arc<str> = "".into();
+            let mut suffixes_inserted = Vec::new();
 
             fn comment_prefix_range(
                 snapshot: &MultiBufferSnapshot,
@@ -4569,7 +4570,6 @@ impl Editor {
                     continue;
                 };
 
-                let mut all_selection_lines_are_comments = true;
                 selection_edit_ranges.clear();
 
                 // If multiple selections contain a given row, avoid processing that
@@ -4586,12 +4586,17 @@ impl Editor {
                     };
                 last_toggled_row = Some(end_row);
 
+                if start_row > end_row {
+                    continue;
+                }
+
                 // If the language has line comments, toggle those.
                 if let Some(full_comment_prefix) = language.line_comment_prefix() {
                     // Split the comment prefix's trailing whitespace into a separate string,
                     // as that portion won't be used for detecting if a line is a comment.
                     let comment_prefix = full_comment_prefix.trim_end_matches(' ');
                     let comment_prefix_whitespace = &full_comment_prefix[comment_prefix.len()..];
+                    let mut all_selection_lines_are_comments = true;
 
                     for row in start_row..=end_row {
                         if snapshot.is_line_blank(row) {
@@ -4633,7 +4638,6 @@ impl Editor {
                 {
                     let comment_prefix = full_comment_prefix.trim_end_matches(' ');
                     let comment_prefix_whitespace = &full_comment_prefix[comment_prefix.len()..];
-
                     let prefix_range = comment_prefix_range(
                         snapshot.deref(),
                         start_row,
@@ -4653,6 +4657,7 @@ impl Editor {
                             full_comment_prefix.clone(),
                         ));
                         edits.push((suffix_range.end..suffix_range.end, comment_suffix.clone()));
+                        suffixes_inserted.push((end_row, comment_suffix.len()));
                     } else {
                         edits.push((prefix_range, empty_str.clone()));
                         edits.push((suffix_range, empty_str.clone()));
@@ -4667,7 +4672,33 @@ impl Editor {
                 buffer.edit(edits, None, cx);
             });
 
-            let selections = this.selections.all::<usize>(cx);
+            // Adjust selections so that they end before any comment suffixes that
+            // were inserted.
+            let mut suffixes_inserted = suffixes_inserted.into_iter().peekable();
+            let mut selections = this.selections.all::<Point>(cx);
+            let snapshot = this.buffer.read(cx).read(cx);
+            for selection in &mut selections {
+                while let Some((row, suffix_len)) = suffixes_inserted.peek().copied() {
+                    match row.cmp(&selection.end.row) {
+                        Ordering::Less => {
+                            suffixes_inserted.next();
+                            continue;
+                        }
+                        Ordering::Greater => break,
+                        Ordering::Equal => {
+                            if selection.end.column == snapshot.line_len(row) {
+                                if selection.is_empty() {
+                                    selection.start.column -= suffix_len as u32;
+                                }
+                                selection.end.column -= suffix_len as u32;
+                            }
+                            break;
+                        }
+                    }
+                }
+            }
+
+            drop(snapshot);
             this.change_selections(Some(Autoscroll::Fit), cx, |s| s.select(selections));
         });
     }
@@ -10975,6 +11006,7 @@ mod tests {
             buffer.set_language(Some(html_language), cx);
         });
 
+        // Toggle comments for empty selections
         cx.set_state(
             &r#"
                 <p>A</p>ˇ
@@ -10983,7 +11015,6 @@ mod tests {
             "#
             .unindent(),
         );
-
         cx.update_editor(|editor, cx| editor.toggle_comments(&ToggleComments, cx));
         cx.assert_editor_state(
             &r#"
@@ -10993,6 +11024,69 @@ mod tests {
             "#
             .unindent(),
         );
+        cx.update_editor(|editor, cx| editor.toggle_comments(&ToggleComments, cx));
+        cx.assert_editor_state(
+            &r#"
+                <p>A</p>ˇ
+                <p>B</p>ˇ
+                <p>C</p>ˇ
+            "#
+            .unindent(),
+        );
+
+        // Toggle comments for mixture of empty and non-empty selections, where
+        // multiple selections occupy a given line.
+        cx.set_state(
+            &r#"
+                <p>A«</p>
+                <p>ˇ»B</p>ˇ
+                <p>C«</p>
+                <p>ˇ»D</p>ˇ
+            "#
+            .unindent(),
+        );
+
+        cx.update_editor(|editor, cx| editor.toggle_comments(&ToggleComments, cx));
+        cx.assert_editor_state(
+            &r#"
+                <!-- <p>A«</p>
+                <p>ˇ»B</p>ˇ -->
+                <!-- <p>C«</p>
+                <p>ˇ»D</p>ˇ -->
+            "#
+            .unindent(),
+        );
+        cx.update_editor(|editor, cx| editor.toggle_comments(&ToggleComments, cx));
+        cx.assert_editor_state(
+            &r#"
+                <p>A«</p>
+                <p>ˇ»B</p>ˇ
+                <p>C«</p>
+                <p>ˇ»D</p>ˇ
+            "#
+            .unindent(),
+        );
+
+        // Toggle comments when different languages are active for different
+        // selections.
+        cx.set_state(
+            &r#"
+                ˇ<script>
+                    ˇvar x = new Y();
+                ˇ</script>
+            "#
+            .unindent(),
+        );
+        cx.foreground().run_until_parked();
+        cx.update_editor(|editor, cx| editor.toggle_comments(&ToggleComments, cx));
+        cx.assert_editor_state(
+            &r#"
+                <!-- ˇ<script> -->
+                    // ˇvar x = new Y();
+                <!-- ˇ</script> -->
+            "#
+            .unindent(),
+        );
     }
 
     #[gpui::test]

crates/language/src/buffer.rs 🔗

@@ -1840,6 +1840,7 @@ impl BufferSnapshot {
         let offset = position.to_offset(self);
         self.syntax
             .layers_for_range(offset..offset, &self.text)
+            .filter(|l| l.node.end_byte() > offset)
             .last()
             .map(|info| info.language)
             .or(self.language.as_ref())

crates/language/src/syntax_map.rs 🔗

@@ -525,19 +525,19 @@ impl SyntaxSnapshot {
     }
 
     #[cfg(test)]
-    pub fn layers(&self, buffer: &BufferSnapshot) -> Vec<SyntaxLayerInfo> {
-        self.layers_for_range(0..buffer.len(), buffer)
+    pub fn layers<'a>(&'a self, buffer: &'a BufferSnapshot) -> Vec<SyntaxLayerInfo> {
+        self.layers_for_range(0..buffer.len(), buffer).collect()
     }
 
     pub fn layers_for_range<'a, T: ToOffset>(
-        &self,
+        &'a self,
         range: Range<T>,
-        buffer: &BufferSnapshot,
-    ) -> Vec<SyntaxLayerInfo> {
+        buffer: &'a BufferSnapshot,
+    ) -> impl 'a + Iterator<Item = SyntaxLayerInfo> {
         let start = buffer.anchor_before(range.start.to_offset(buffer));
         let end = buffer.anchor_after(range.end.to_offset(buffer));
 
-        let mut cursor = self.layers.filter::<_, ()>(|summary| {
+        let mut cursor = self.layers.filter::<_, ()>(move |summary| {
             if summary.max_depth > summary.min_depth {
                 true
             } else {
@@ -547,21 +547,26 @@ impl SyntaxSnapshot {
             }
         });
 
-        let mut result = Vec::new();
+        // let mut result = Vec::new();
         cursor.next(buffer);
-        while let Some(layer) = cursor.item() {
-            result.push(SyntaxLayerInfo {
-                language: &layer.language,
-                depth: layer.depth,
-                node: layer.tree.root_node_with_offset(
-                    layer.range.start.to_offset(buffer),
-                    layer.range.start.to_point(buffer).to_ts_point(),
-                ),
-            });
-            cursor.next(buffer)
-        }
+        std::iter::from_fn(move || {
+            if let Some(layer) = cursor.item() {
+                let info = SyntaxLayerInfo {
+                    language: &layer.language,
+                    depth: layer.depth,
+                    node: layer.tree.root_node_with_offset(
+                        layer.range.start.to_offset(buffer),
+                        layer.range.start.to_point(buffer).to_ts_point(),
+                    ),
+                };
+                cursor.next(buffer);
+                Some(info)
+            } else {
+                None
+            }
+        })
 
-        result
+        // result
     }
 }
 
@@ -1848,7 +1853,9 @@ mod tests {
         range: Range<Point>,
         expected_layers: &[&str],
     ) {
-        let layers = syntax_map.layers_for_range(range, &buffer);
+        let layers = syntax_map
+            .layers_for_range(range, &buffer)
+            .collect::<Vec<_>>();
         assert_eq!(
             layers.len(),
             expected_layers.len(),