Use first line comment prefix when toggling comments (cherry-pick #10335) (#10339)

gcp-cherry-pick-bot[bot] , Max Brunsfeld , and Marshall created 2 years ago

Cherry-picked Use first line comment prefix when toggling comments
(#10335)

This fixed an issue introduced in
https://github.com/zed-industries/zed/pull/10126, where, when toggling
comments in a language with multiple line comment prefixes (e.g. Gleam,
Erlang) Zed would insert the *last* prefix instead of the first.

Release Notes:

- Fixed an issue where the `toggle comments` command inserted the wrong
line comment prefix in some languages (preview only).

Co-authored-by: Marshall <[e-mail]>

Co-authored-by: Max Brunsfeld <[e-mail]>
Co-authored-by: Marshall <[e-mail]>

Change summary

crates/editor/src/editor.rs           | 58 +++++++++++-----------------
crates/editor/src/editor_tests.rs     | 21 ++++++++++
crates/languages/src/rust/config.toml |  2 
3 files changed, 44 insertions(+), 37 deletions(-)

Detailed changes

crates/editor/src/editor.rs

@@ -7096,22 +7096,13 @@ impl Editor {
                     .line_comment_prefixes()
                     .filter(|prefixes| !prefixes.is_empty())
                 {
-                    // 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.
-                    struct Comment {
-                        full_prefix: Arc<str>,
-                        trimmed_prefix_len: usize,
-                    }
-                    let prefixes: SmallVec<[Comment; 4]> = full_comment_prefixes
+                    let first_prefix = full_comment_prefixes
+                        .first()
+                        .expect("prefixes is non-empty");
+                    let prefix_trimmed_lengths = full_comment_prefixes
                         .iter()
-                        .map(|full_prefix| {
-                            let trimmed_prefix_len = full_prefix.trim_end_matches(' ').len();
-                            Comment {
-                                trimmed_prefix_len,
-                                full_prefix: full_prefix.clone(),
-                            }
-                        })
-                        .collect();
+                        .map(|p| p.trim_end_matches(' ').len())
+                        .collect::<SmallVec<[usize; 4]>>();
 
                     let mut all_selection_lines_are_comments = true;
 
@@ -7120,28 +7111,25 @@ impl Editor {
                             continue;
                         }
 
-                        let Some((prefix, prefix_range)) = prefixes
+                        let prefix_range = full_comment_prefixes
                             .iter()
-                            .map(|prefix| {
-                                (
-                                    prefix,
-                                    comment_prefix_range(
-                                        snapshot.deref(),
-                                        row,
-                                        &prefix.full_prefix[..prefix.trimmed_prefix_len],
-                                        &prefix.full_prefix[prefix.trimmed_prefix_len..],
-                                    ),
+                            .zip(prefix_trimmed_lengths.iter().copied())
+                            .map(|(prefix, trimmed_prefix_len)| {
+                                comment_prefix_range(
+                                    snapshot.deref(),
+                                    row,
+                                    &prefix[..trimmed_prefix_len],
+                                    &prefix[trimmed_prefix_len..],
                                 )
                             })
-                            .max_by_key(|(_, range)| range.end.column - range.start.column)
-                        else {
-                            // There has to be at least one prefix.
-                            break;
-                        };
+                            .max_by_key(|range| range.end.column - range.start.column)
+                            .expect("prefixes is non-empty");
+
                         if prefix_range.is_empty() {
                             all_selection_lines_are_comments = false;
                         }
-                        selection_edit_ranges.push((prefix_range, prefix.full_prefix.clone()));
+
+                        selection_edit_ranges.push(prefix_range);
                     }
 
                     if all_selection_lines_are_comments {
@@ -7149,17 +7137,17 @@ impl Editor {
                             selection_edit_ranges
                                 .iter()
                                 .cloned()
-                                .map(|(range, _)| (range, empty_str.clone())),
+                                .map(|range| (range, empty_str.clone())),
                         );
                     } else {
                         let min_column = selection_edit_ranges
                             .iter()
-                            .map(|(range, _)| range.start.column)
+                            .map(|range| range.start.column)
                             .min()
                             .unwrap_or(0);
-                        edits.extend(selection_edit_ranges.iter().map(|(range, prefix)| {
+                        edits.extend(selection_edit_ranges.iter().map(|range| {
                             let position = Point::new(range.start.row, min_column);
-                            (position..position, prefix.clone())
+                            (position..position, first_prefix.clone())
                         }));
                     }
                 } else if let Some((full_comment_prefix, comment_suffix)) =

crates/editor/src/editor_tests.rs

@@ -6507,7 +6507,7 @@ async fn test_toggle_comment(cx: &mut gpui::TestAppContext) {
     let mut cx = EditorTestContext::new(cx).await;
     let language = Arc::new(Language::new(
         LanguageConfig {
-            line_comments: vec!["// ".into()],
+            line_comments: vec!["// ".into(), "//! ".into(), "/// ".into()],
             ..Default::default()
         },
         Some(tree_sitter_rust::language()),
@@ -6601,6 +6601,25 @@ async fn test_toggle_comment(cx: &mut gpui::TestAppContext) {
             // c();ˇ»
         }
     "});
+
+    // If a selection includes multiple comment prefixes, all lines are uncommented.
+    cx.set_state(indoc! {"
+        fn a() {
+            «// a();
+            /// b();
+            //! c();ˇ»
+        }
+    "});
+
+    cx.update_editor(|e, cx| e.toggle_comments(&ToggleComments::default(), cx));
+
+    cx.assert_editor_state(indoc! {"
+        fn a() {
+            «a();
+            b();
+            c();ˇ»
+        }
+    "});
 }
 
 #[gpui::test]

crates/languages/src/rust/config.toml

@@ -1,7 +1,7 @@
 name = "Rust"
 grammar = "rust"
 path_suffixes = ["rs"]
-line_comments = [ "/// ", "//! ", "// "]
+line_comments = ["// ", "/// ", "//! "]
 autoclose_before = ";:.,=}])>"
 brackets = [
     { start = "{", end = "}", close = true, newline = true },