Add action parameter comment_empty_lines for editor::ToggleComments (#63961)

# Objective

- Fixes: https://github.com/zed-industries/zed/issues/60738
- Re-iteration of https://github.com/zed-industries/zed/pull/63918 with
suggested changes

## Solution

- Added the parameter `comment_empty_lines` to `editor::ToggleComments`
set to `false` by default.

## Testing

- Tested manually. See showcase below.
- Added two automated tests for the added feature.
- Tested on MacOS.

## Self-Review Checklist:

- [X] I've reviewed my own diff for quality, security, and reliability
- [X] Unsafe blocks (if any) have justifying comments
- [X] The content adheres to Zed's UI standards
([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
and
[icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md)
guidelines)
- [X] Tests cover the new/changed behavior
- [X] Performance impact has been considered and is acceptable

## References
The action's behavior was observed across Emacs, VSCode, and Intellij,
and the corresponding keymaps were set accordingly.


https://github.com/user-attachments/assets/36c1a27a-357c-468a-b56b-59ef31265329

https://github.com/user-attachments/assets/de755753-9de1-4771-980f-177810fb85ef

https://github.com/user-attachments/assets/c8ecd261-2bba-4345-89f4-c1d708d8b6d2

## Showcase

Below is the screen recording of these three tests:

1. Behavior of toggling comments when `comment_empty_lines` is `true`.
2. Switching the parameter value to true after commenting a block and
un-commenting the same block does not re-comment the block due to an
empty line in between.
3. Menu bar item is still present, albeit it's behavior is mapped to the
default value of the parameters of the bound action, instead of what is
inside the keybindings file. This (bug?) was present even before. See
the attached clip below.

1.
https://github.com/user-attachments/assets/a9cd2a2c-437b-41a3-bd8e-c4c3a1f350cd
2.
https://github.com/user-attachments/assets/4bbfb77c-9019-410e-9d0a-952d4438d2a7
3.
https://github.com/user-attachments/assets/ca300e2d-2106-4bb7-865b-5d2ab89cd0f2


https://github.com/user-attachments/assets/ec1e47b6-ff99-461e-93af-af925be4d914

---

Release Notes:

- Added `comment_empty_lines` parameter to `editor::ToggleComments` for
multiline selections.
Use `["editor::ToggleComments", {"comment_empty_lines": true}]` to
comment blank lines too (default for Zed keymap now), or
`["editor::ToggleComments", {"comment_empty_lines": false}]` to skip
them (default for VSCode keymap).

---------

Co-authored-by: Kirill Bulatov <kirill@zed.dev>

assets/keymaps/default-linux.json

@@ -547,8 +547,8 @@
       "ctrl-k ctrl-shift-d": ["editor::SelectPrevious", { "replace_newest": true }], // editor.action.moveSelectionToPreviousFindMatch
       "ctrl-k ctrl-i": "editor::Hover",
       "ctrl-k ctrl-b": "editor::BlameHover",
-      "ctrl-/": ["editor::ToggleComments", { "advance_downwards": false }],
-      "ctrl-k ctrl-c": ["editor::ToggleComments", { "advance_downwards": false }],
+      "ctrl-/": ["editor::ToggleComments", { "advance_downwards": false, "comment_empty_lines": true }],
+      "ctrl-k ctrl-c": ["editor::ToggleComments", { "advance_downwards": false, "comment_empty_lines": true }],
       "ctrl-k ctrl-/": "editor::ToggleBlockComments",
       "shift-alt-a": "editor::ToggleBlockComments",
       "f8": ["editor::GoToDiagnostic", { "severity": { "min": "hint", "max": "error" } }],

assets/keymaps/default-macos.json

@@ -589,7 +589,7 @@
       "cmd-k ctrl-cmd-d": ["editor::SelectPrevious", { "replace_newest": true }], // editor.action.moveSelectionToPreviousFindMatch
       "cmd-k cmd-i": "editor::Hover",
       "cmd-k cmd-b": "editor::BlameHover",
-      "cmd-/": ["editor::ToggleComments", { "advance_downwards": false }],
+      "cmd-/": ["editor::ToggleComments", { "advance_downwards": false, "comment_empty_lines": true }],
       "cmd-k cmd-/": "editor::ToggleBlockComments",
       "shift-alt-a": "editor::ToggleBlockComments",
       "f8": ["editor::GoToDiagnostic", { "severity": { "min": "hint", "max": "error" } }],

assets/keymaps/default-windows.json

@@ -550,8 +550,8 @@
       "ctrl-k ctrl-i": "editor::Hover",
       "ctrl-k ctrl-b": "editor::BlameHover",
       "ctrl-k ctrl-f": "editor::FormatSelections",
-      "ctrl-/": ["editor::ToggleComments", { "advance_downwards": false }],
-      "ctrl-k ctrl-c": ["editor::ToggleComments", { "advance_downwards": false }],
+      "ctrl-/": ["editor::ToggleComments", { "advance_downwards": false, "comment_empty_lines": true }],
+      "ctrl-k ctrl-c": ["editor::ToggleComments", { "advance_downwards": false, "comment_empty_lines": true }],
       "ctrl-k ctrl-/": "editor::ToggleBlockComments",
       "shift-alt-a": "editor::ToggleBlockComments",
       "f8": ["editor::GoToDiagnostic", { "severity": { "min": "hint", "max": "error" } }],

assets/keymaps/linux/emacs.json

@@ -52,8 +52,8 @@
       "alt-l": "editor::ConvertToLowerCase", // downcase-word
       "alt-c": "editor::ConvertToUpperCamelCase", // capitalize-word
       "ctrl-t": "editor::Transpose", // transpose-chars
-      "alt-;": ["editor::ToggleComments", { "advance_downwards": false }],
-      "ctrl-x ctrl-;": "editor::ToggleComments",
+      "alt-;": ["editor::ToggleComments", { "advance_downwards": false, "comment_empty_lines": false }],
+      "ctrl-x ctrl-;": ["editor::ToggleComments", { "advance_downwards": false, "comment_empty_lines": false }],
       "alt-.": "editor::GoToDefinition", // xref-find-definitions
       "alt-?": "editor::FindAllReferences", // xref-find-references
       "alt-,": "pane::GoBack", // xref-pop-marker-stack

assets/keymaps/linux/jetbrains.json

@@ -35,7 +35,7 @@
       "alt-shift-g": "editor::SplitSelectionIntoLines",
       "alt-j": ["editor::SelectNext", { "replace_newest": false }],
       "alt-shift-j": ["editor::SelectPrevious", { "replace_newest": false }],
-      "ctrl-/": ["editor::ToggleComments", { "advance_downwards": true }],
+      "ctrl-/": ["editor::ToggleComments", { "advance_downwards": true, "comment_empty_lines": true }],
       "ctrl-w": "editor::SelectLargerSyntaxNode",
       "ctrl-shift-w": "editor::SelectSmallerSyntaxNode",
       "shift-alt-up": "editor::MoveLineUp",

assets/keymaps/linux/vscode.json

@@ -13,6 +13,8 @@
       "alt-z": "editor::ToggleSoftWrap", // Toggle Word Wrap
       "ctrl-shift-space": "editor::ShowSignatureHelp", // Trigger Parameter Hints
       "ctrl-shift-a": "editor::ToggleBlockComments", // Toggle Block Comment
+      "ctrl-k ctrl-c": ["editor::ToggleComments", { "advance_downwards": false, "comment_empty_lines": false }], // Add Line Comment
+      "ctrl-/": ["editor::ToggleComments", { "advance_downwards": false, "comment_empty_lines": false }],
       "ctrl-k f12": "editor::GoToDefinitionSplit", // Open Definition to the Side
     },
   },

assets/keymaps/macos/emacs.json

@@ -46,8 +46,8 @@
       "alt-l": "editor::ConvertToLowerCase", // downcase-word
       "alt-c": "editor::ConvertToUpperCamelCase", // capitalize-word
       "ctrl-t": "editor::Transpose", // transpose-chars
-      "alt-;": ["editor::ToggleComments", { "advance_downwards": false }],
-      "ctrl-x ctrl-;": "editor::ToggleComments",
+      "alt-;": ["editor::ToggleComments", { "advance_downwards": false, "comment_empty_lines": false }],
+      "ctrl-x ctrl-;": ["editor::ToggleComments", { "advance_downwards": false, "comment_empty_lines": false }],
       "alt-.": "editor::GoToDefinition", // xref-find-definitions
       "alt-?": "editor::FindAllReferences", // xref-find-references
       "alt-,": "pane::GoBack", // xref-pop-marker-stack

assets/keymaps/macos/jetbrains.json

@@ -35,7 +35,7 @@
       "ctrl-g": ["editor::SelectNext", { "replace_newest": false }],
       "ctrl-shift-g": "editor::UndoSelection",
       "ctrl-cmd-g": ["editor::SelectPrevious", { "replace_newest": false }],
-      "cmd-/": ["editor::ToggleComments", { "advance_downwards": true }],
+      "cmd-/": ["editor::ToggleComments", { "advance_downwards": true, "comment_empty_lines": true }],
       "alt-up": "editor::SelectLargerSyntaxNode",
       "alt-down": "editor::SelectSmallerSyntaxNode",
       "shift-alt-up": "editor::MoveLineUp",

assets/keymaps/macos/vscode.json

@@ -14,7 +14,8 @@
       "cmd-shift-space": "editor::ShowSignatureHelp", // Trigger Parameter Hints
       "ctrl-shift-cmd-right": "editor::SelectLargerSyntaxNode", // Expand AST Selection
       "ctrl-shift-cmd-left": "editor::SelectSmallerSyntaxNode", // Shrink AST Selection
-      "cmd-k cmd-c": ["editor::ToggleComments", { "advance_downwards": false }], // Add Line Comment (closest analog)
+      "cmd-k cmd-c": ["editor::ToggleComments", { "advance_downwards": false, "comment_empty_lines": false }], // Add Line Comment (closest analog)
+      "cmd-/": ["editor::ToggleComments", { "advance_downwards": false, "comment_empty_lines": false }],
       "cmd-k f12": "editor::GoToDefinitionSplit", // Open Definition to the Side
     },
   },

assets/keymaps/vim.json

@@ -504,7 +504,7 @@
       "*": "buffer_search::UseSelectionForFind",
       "shift-x": "editor::SelectLine",
       "_": "vim::HelixTrimSelections",
-      "ctrl-c": "editor::ToggleComments",
+      "ctrl-c": ["editor::ToggleComments", { "advance_downwards": false, "comment_empty_lines": false }],
       "alt-o": "editor::SelectLargerSyntaxNode",
       "alt-i": "editor::SelectSmallerSyntaxNode",
       "alt-p": "editor::SelectPreviousSyntaxNode",
@@ -562,7 +562,7 @@
       "space r": "editor::Rename",
       "space a": "editor::ToggleCodeActions",
       "space h": "editor::SelectAllMatches",
-      "space c": "editor::ToggleComments",
+      "space c": ["editor::ToggleComments", { "advance_downwards": false, "comment_empty_lines": false }],
       "space p": "editor::Paste",
       "space y": "editor::Copy",
       "space /": "pane::DeploySearch",

crates/editor/src/actions.rs

@@ -140,7 +140,7 @@ pub struct ConfirmCodeAction {
 }
 
 /// Toggles comment markers for the selected lines.
-#[derive(PartialEq, Clone, Deserialize, Default, JsonSchema, Action)]
+#[derive(PartialEq, Clone, Deserialize, JsonSchema, Action)]
 #[action(namespace = editor)]
 #[serde(deny_unknown_fields)]
 pub struct ToggleComments {
@@ -148,6 +148,22 @@ pub struct ToggleComments {
     pub advance_downwards: bool,
     #[serde(default)]
     pub ignore_indent: bool,
+    /// Whether to add comment markers to blank lines inside a multi-line
+    /// selection. A line of only whitespace counts as blank. Defaults to true.
+    #[serde(default = "default_true")]
+    pub comment_empty_lines: bool,
+}
+
+// `Default` is written out rather than derived because `comment_empty_lines`
+// defaults to true.
+impl Default for ToggleComments {
+    fn default() -> Self {
+        Self {
+            advance_downwards: false,
+            ignore_indent: false,
+            comment_empty_lines: true,
+        }
+    }
 }
 
 /// Toggles block comment markers for the selected text.

crates/editor/src/editor_tests.rs

@@ -23271,7 +23271,7 @@ async fn test_toggle_comment(cx: &mut TestAppContext) {
         }
     "});
 
-    // If a selection span multiple lines, empty lines are not toggled.
+    // If a selection spans multiple lines, empty lines are also toggled.
     cx.set_state(indoc! {"
         fn a() {
             «a();
@@ -23284,9 +23284,9 @@ async fn test_toggle_comment(cx: &mut TestAppContext) {
 
     cx.assert_editor_state(indoc! {"
         fn a() {
-            // «a();
-
-            // c();ˇ»
+        //     «a();
+        //•
+        //     c();ˇ»
         }
     "});
 
@@ -23326,6 +23326,7 @@ async fn test_toggle_comment_ignore_indent(cx: &mut TestAppContext) {
     let toggle_comments = &ToggleComments {
         advance_downwards: false,
         ignore_indent: true,
+        comment_empty_lines: false,
     };
 
     // If multiple selections intersect a line, the line is only toggled once.
@@ -23435,6 +23436,160 @@ async fn test_toggle_comment_ignore_indent(cx: &mut TestAppContext) {
     "});
 }
 
+#[gpui::test]
+async fn test_toggle_comment_commenting_blank_lines(cx: &mut TestAppContext) {
+    init_test(cx, |_| {});
+    let mut cx = EditorTestContext::new(cx).await;
+    // A prefix without a trailing space keeps every expectation below free of
+    // trailing whitespace, which would otherwise be stripped on save and break
+    // the assertions on commented blank lines.
+    let language = Arc::new(Language::new(
+        LanguageConfig {
+            line_comments: vec!["//".into()],
+            ..Default::default()
+        },
+        Some(tree_sitter_rust::LANGUAGE.into()),
+    ));
+    cx.update_buffer(|buffer, cx| buffer.set_language(Some(language), cx));
+
+    let toggle_comments = &ToggleComments::default();
+
+    cx.set_state(indoc! {"
+        «fn a() {
+            b();
+
+            c();
+        }ˇ»
+    "});
+
+    cx.update_editor(|e, window, cx| e.toggle_comments(toggle_comments, window, cx));
+
+    cx.assert_editor_state(indoc! {"
+        //«fn a() {
+        //    b();
+        //
+        //    c();
+        //}ˇ»
+    "});
+
+    // Toggling again removes the prefix from every line, blank ones included.
+    cx.update_editor(|e, window, cx| e.toggle_comments(toggle_comments, window, cx));
+
+    cx.assert_editor_state(indoc! {"
+        «fn a() {
+            b();
+
+            c();
+        }ˇ»
+    "});
+
+    // All prefixes in a block go at one shared column so they line up: the
+    // smallest indent among the rows being commented. A blank line contributes
+    // 0 and drags that to 0 - shifting every line in the block left. VS Code
+    // does the same.
+    cx.set_state(indoc! {"
+        fn a() {
+            «b();
+
+            c();ˇ»
+        }
+    "});
+
+    cx.update_editor(|e, window, cx| e.toggle_comments(toggle_comments, window, cx));
+
+    cx.assert_editor_state(indoc! {"
+        fn a() {
+        //    «b();
+        //
+        //    c();ˇ»
+        }
+    "});
+}
+
+#[gpui::test]
+async fn test_toggle_comment_uncomments_after_parameter_change(cx: &mut TestAppContext) {
+    init_test(cx, |_| {});
+    let mut cx = EditorTestContext::new(cx).await;
+    let language = Arc::new(Language::new(
+        LanguageConfig {
+            line_comments: vec!["//".into()],
+            ..Default::default()
+        },
+        Some(tree_sitter_rust::LANGUAGE.into()),
+    ));
+    cx.update_buffer(|buffer, cx| buffer.set_language(Some(language), cx));
+
+    let skip_blank_lines = &ToggleComments {
+        comment_empty_lines: false,
+        ..Default::default()
+    };
+    let comment_blank_lines = &ToggleComments::default();
+
+    cx.set_state(indoc! {"
+        «fn a() {
+            b();
+
+            c();
+        }ˇ»
+    "});
+
+    // Comment with one binding: the blank line gets no marker.
+    cx.update_editor(|e, window, cx| e.toggle_comments(skip_blank_lines, window, cx));
+
+    cx.assert_editor_state(indoc! {"
+        //«fn a() {
+        //    b();
+
+        //    c();
+        //}ˇ»
+    "});
+
+    // Toggle with the other binding. The blank line still has no marker, but it must
+    // not make the block look uncommented: the markers are removed, not doubled.
+    cx.update_editor(|e, window, cx| e.toggle_comments(comment_blank_lines, window, cx));
+
+    cx.assert_editor_state(indoc! {"
+        «fn a() {
+            b();
+
+            c();
+        }ˇ»
+    "});
+}
+
+#[gpui::test]
+async fn test_toggle_comment_selection_of_only_blank_lines(cx: &mut TestAppContext) {
+    init_test(cx, |_| {});
+    let mut cx = EditorTestContext::new(cx).await;
+    let language = Arc::new(Language::new(
+        LanguageConfig {
+            line_comments: vec!["//".into()],
+            ..Default::default()
+        },
+        Some(tree_sitter_rust::LANGUAGE.into()),
+    ));
+    cx.update_buffer(|buffer, cx| buffer.set_language(Some(language), cx));
+
+    let toggle_comments = &ToggleComments::default();
+
+    // No line carries a marker, so there is nothing to remove: comment them.
+    cx.set_state(indoc! {"
+        fn a() {
+        «
+
+        ˇ»}
+    "});
+
+    cx.update_editor(|e, window, cx| e.toggle_comments(toggle_comments, window, cx));
+
+    cx.assert_editor_state(indoc! {"
+        fn a() {
+        //«
+        //
+        ˇ»}
+    "});
+}
+
 #[gpui::test]
 async fn test_advance_downward_on_toggle_comment(cx: &mut TestAppContext) {
     init_test(cx, |_| {});
@@ -23457,6 +23612,7 @@ async fn test_advance_downward_on_toggle_comment(cx: &mut TestAppContext) {
     let toggle_comments = &ToggleComments {
         advance_downwards: true,
         ignore_indent: false,
+        comment_empty_lines: false,
     };
 
     // Single cursor on one line -> advance

crates/editor/src/input.rs

@@ -1416,6 +1416,7 @@ impl Editor {
             let empty_str: Arc<str> = Arc::default();
             let mut suffixes_inserted = Vec::new();
             let ignore_indent = action.ignore_indent;
+            let comment_empty_lines = action.comment_empty_lines;
 
             fn comment_prefix_range(
                 snapshot: &MultiBufferSnapshot,
@@ -1545,11 +1546,13 @@ impl Editor {
                         .map(|p| p.trim_end_matches(' ').len())
                         .collect::<SmallVec<[usize; 4]>>();
 
-                    let mut all_selection_lines_are_comments = true;
+                    let mut commented_lines = 0;
+                    let mut uncommented_lines = 0;
 
                     for row in start_row.0..=end_row.0 {
                         let row = MultiBufferRow(row);
-                        if start_row < end_row && snapshot.is_line_blank(row) {
+                        let is_blank = start_row < end_row && snapshot.is_line_blank(row);
+                        if !comment_empty_lines && is_blank {
                             continue;
                         }
 
@@ -1568,14 +1571,30 @@ impl Editor {
                             .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;
+                        // Blank rows are left out of the tally. They never carry a marker,
+                        // so counting them would make a commented block that contains one
+                        // look uncommented, and it could then never be uncommented. VS Code
+                        // and IntelliJ exclude them here for the same reason.
+                        // Without this, commenting a block with an empty line in between,
+                        // and then changing the action parameter of
+                        // ToggleComments with `comment_empty_lines: true` would add comment to the block,
+                        // even though it should be uncommented
+                        if !is_blank {
+                            if prefix_range.is_empty() {
+                                uncommented_lines += 1;
+                            } else {
+                                commented_lines += 1;
+                            }
                         }
 
                         selection_edit_ranges.push(prefix_range);
                     }
 
-                    if all_selection_lines_are_comments {
+                    // Remove markers only when at least one row was counted as commented
+                    // and none were counted as uncommented.
+                    let should_uncomment = uncommented_lines == 0 && commented_lines > 0;
+
+                    if should_uncomment {
                         edits.extend(
                             selection_edit_ranges
                                 .iter()