Fix for comment indentation

Richard Feldman created

Change summary

crates/editor/src/editor.rs       | 118 ++++++++++++++++++++------------
crates/editor/src/editor_tests.rs |  52 +-------------
2 files changed, 77 insertions(+), 93 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -9743,6 +9743,14 @@ impl Editor {
         let mut edits = Vec::new();
         let mut prev_edited_row = 0;
         let mut row_delta = 0;
+
+        // Check if we have multiple cursors all at the beginning of their lines (column 0)
+        // This is specifically for fixing the issue where indenting multiple comment lines
+        // results in inconsistent indentation
+        let empty_selections: Vec<_> = selections.iter().filter(|s| s.is_empty()).collect();
+        let multiple_cursors_at_line_start =
+            empty_selections.len() > 1 && empty_selections.iter().all(|s| s.head().column == 0);
+
         for selection in &mut selections {
             if selection.start.row != prev_edited_row {
                 row_delta = 0;
@@ -9761,67 +9769,87 @@ impl Editor {
             if let Some(suggested_indent) =
                 suggested_indents.get(&MultiBufferRow(cursor.row)).copied()
             {
-                // Don't do anything if already at suggested indent
-                // and there is any other cursor which is not
-                if has_some_cursor_in_whitespace
-                    && cursor.column == current_indent.len
-                    && current_indent.len == suggested_indent.len
-                {
-                    continue;
-                }
+                // Skip suggested indent logic when we have multiple cursors at the beginning
+                // of lines to ensure consistent indentation across multiple lines
+                if !(multiple_cursors_at_line_start && cursor.column == 0) {
+                    // Don't do anything if already at suggested indent
+                    // and there is any other cursor which is not
+                    if has_some_cursor_in_whitespace
+                        && cursor.column == current_indent.len
+                        && current_indent.len == suggested_indent.len
+                    {
+                        continue;
+                    }
 
-                // Adjust line and move cursor to suggested indent
-                // if cursor is not at suggested indent
-                if cursor.column < suggested_indent.len
-                    && cursor.column <= current_indent.len
-                    && current_indent.len <= suggested_indent.len
-                {
-                    selection.start = Point::new(cursor.row, suggested_indent.len);
-                    selection.end = selection.start;
-                    if row_delta == 0 {
-                        edits.extend(Buffer::edit_for_indent_size_adjustment(
-                            cursor.row,
-                            current_indent,
-                            suggested_indent,
-                        ));
-                        row_delta = suggested_indent.len - current_indent.len;
+                    // Adjust line and move cursor to suggested indent
+                    // if cursor is not at suggested indent
+                    if cursor.column < suggested_indent.len
+                        && cursor.column <= current_indent.len
+                        && current_indent.len <= suggested_indent.len
+                    {
+                        selection.start = Point::new(cursor.row, suggested_indent.len);
+                        selection.end = selection.start;
+                        if row_delta == 0 {
+                            edits.extend(Buffer::edit_for_indent_size_adjustment(
+                                cursor.row,
+                                current_indent,
+                                suggested_indent,
+                            ));
+                            row_delta = suggested_indent.len - current_indent.len;
+                        }
+                        continue;
                     }
-                    continue;
-                }
 
-                // If current indent is more than suggested indent
-                // only move cursor to current indent and skip indent
-                if cursor.column < current_indent.len && current_indent.len > suggested_indent.len {
-                    selection.start = Point::new(cursor.row, current_indent.len);
-                    selection.end = selection.start;
-                    continue;
+                    // If current indent is more than suggested indent
+                    // only move cursor to current indent and skip indent
+                    if cursor.column < current_indent.len
+                        && current_indent.len > suggested_indent.len
+                    {
+                        selection.start = Point::new(cursor.row, current_indent.len);
+                        selection.end = selection.start;
+                        continue;
+                    }
                 }
             }
 
             // Otherwise, insert a hard or soft tab.
             let settings = buffer.language_settings_at(cursor, cx);
-            let tab_size = if settings.hard_tabs {
+            let indent_size = if settings.hard_tabs {
                 IndentSize::tab()
             } else {
                 let tab_size = settings.tab_size.get();
-                let indent_remainder = snapshot
-                    .text_for_range(Point::new(cursor.row, 0)..cursor)
-                    .flat_map(str::chars)
-                    .fold(row_delta % tab_size, |counter: u32, c| {
-                        if c == '\t' {
-                            0
-                        } else {
-                            (counter + 1) % tab_size
-                        }
-                    });
+                // When we have multiple cursors at the beginning of their lines,
+                // don't use row_delta in the calculation to ensure consistent indentation
+                let indent_remainder = if multiple_cursors_at_line_start && cursor.column == 0 {
+                    0
+                } else {
+                    snapshot
+                        .text_for_range(Point::new(cursor.row, 0)..cursor)
+                        .flat_map(str::chars)
+                        .fold(row_delta % tab_size, |counter: u32, c| {
+                            if c == '\t' {
+                                0
+                            } else {
+                                (counter + 1) % tab_size
+                            }
+                        })
+                };
 
                 let chars_to_next_tab_stop = tab_size - indent_remainder;
                 IndentSize::spaces(chars_to_next_tab_stop)
             };
-            selection.start = Point::new(cursor.row, cursor.column + row_delta + tab_size.len);
+
+            // When we have multiple cursors at line start, don't apply row_delta
+            let new_column = if multiple_cursors_at_line_start && cursor.column == 0 {
+                cursor.column + indent_size.len
+            } else {
+                cursor.column + row_delta + indent_size.len
+            };
+
+            selection.start = Point::new(cursor.row, new_column);
             selection.end = selection.start;
-            edits.push((cursor..cursor, tab_size.chars().collect::<String>()));
-            row_delta += tab_size.len;
+            edits.push((cursor..cursor, indent_size.chars().collect::<String>()));
+            row_delta += indent_size.len;
         }
 
         self.transact(window, cx, |this, window, cx| {

crates/editor/src/editor_tests.rs 🔗

@@ -3470,11 +3470,6 @@ async fn test_indent_outdent(cx: &mut TestAppContext) {
 
 #[gpui::test]
 async fn test_indent_yaml_comments_with_multiple_cursors(cx: &mut TestAppContext) {
-    // Test for issue #33761: Indenting comments behaves strangely with multiple cursors
-    // When pressing TAB with multiple cursors at the beginning of comment lines,
-    // the indentation should be consistent across all lines, but instead each line
-    // gets indented based on the existing whitespace after the comment character.
-
     init_test(cx, |_| {});
 
     let mut cx = EditorTestContext::new(cx).await;
@@ -3492,8 +3487,6 @@ async fn test_indent_yaml_comments_with_multiple_cursors(cx: &mut TestAppContext
     ));
     cx.update_buffer(|buffer, cx| buffer.set_language(Some(yaml_language), cx));
 
-    // Set up the test case from the issue
-    // Each line has a cursor at the beginning (position 0)
     cx.set_state(indoc! {"
         ˇ#     ingress:
         ˇ#         api:
@@ -3507,8 +3500,7 @@ async fn test_indent_yaml_comments_with_multiple_cursors(cx: &mut TestAppContext
     // Press tab to indent all lines
     cx.update_editor(|e, window, cx| e.tab(&Tab, window, cx));
 
-    // Expected behavior: all lines should be indented by the same amount (2 spaces)
-    let _expected = indoc! {"
+    cx.assert_editor_state(indoc! {"
           ˇ#     ingress:
           ˇ#         api:
           ˇ#             enabled: false
@@ -3516,40 +3508,11 @@ async fn test_indent_yaml_comments_with_multiple_cursors(cx: &mut TestAppContext
           ˇ#           console:
           ˇ#               enabled: false
           ˇ#               pathType: Prefix
-    "};
-
-    // Actual behavior (BUG): lines are indented by different amounts
-    // The indentation appears to be calculated based on the whitespace
-    // after the comment character, resulting in inconsistent indentation
-
-    // What actually happens:
-    // - Line 1 (#     ingress:) gets 2 spaces added
-    // - Line 2 (#         api:) gets 4 spaces added
-    // - Line 3 (#             enabled: false) gets 6 spaces added
-    // - Line 4 (#             pathType: Prefix) gets 6 spaces added
-    // - Line 5 (#           console:) gets 6 spaces added
-    // - Line 6 (#               enabled: false) gets 8 spaces added
-    // - Line 7 (#               pathType: Prefix) gets 8 spaces added
-
-    cx.assert_editor_state(indoc! {"
-          ˇ#     ingress:
-            ˇ#         api:
-              ˇ#             enabled: false
-              ˇ#             pathType: Prefix
-              ˇ#           console:
-                ˇ#               enabled: false
-                ˇ#               pathType: Prefix
-    "});
-
-    // This test demonstrates the bug described in issue #33761
-    // The test currently expects the buggy behavior to pass.
-    // Once the bug is fixed, replace the assertion above with:
-    // cx.assert_editor_state(_expected);
+    "});
 }
 
 #[gpui::test]
 async fn test_indent_yaml_comments_simple(cx: &mut TestAppContext) {
-    // Simplified test for issue #33761 showing the core problem
     init_test(cx, |_| {});
 
     let mut cx = EditorTestContext::new(cx).await;
@@ -3567,7 +3530,6 @@ async fn test_indent_yaml_comments_simple(cx: &mut TestAppContext) {
     ));
     cx.update_buffer(|buffer, cx| buffer.set_language(Some(yaml_language), cx));
 
-    // Simple test with just two lines showing the problem
     cx.set_state(indoc! {"
         ˇ# foo
         ˇ#     bar
@@ -3575,18 +3537,14 @@ async fn test_indent_yaml_comments_simple(cx: &mut TestAppContext) {
 
     cx.update_editor(|e, window, cx| e.tab(&Tab, window, cx));
 
-    // Expected: both lines indented by 2 spaces
-    // Actual: first line indented by 2, second by 4
     cx.assert_editor_state(indoc! {"
           ˇ# foo
-            ˇ#     bar
+          ˇ#     bar
     "});
 }
 
 #[gpui::test]
-async fn test_indent_yaml_comments_expected_behavior(cx: &mut TestAppContext) {
-    // This test shows what the EXPECTED behavior should be for issue #33761
-    // Currently this test will FAIL because of the bug
+async fn test_indent_yaml_comments_full(cx: &mut TestAppContext) {
     init_test(cx, |_| {});
 
     let mut cx = EditorTestContext::new(cx).await;
@@ -3604,7 +3562,6 @@ async fn test_indent_yaml_comments_expected_behavior(cx: &mut TestAppContext) {
     ));
     cx.update_buffer(|buffer, cx| buffer.set_language(Some(yaml_language), cx));
 
-    // Set up the test case from the issue
     cx.set_state(indoc! {"
         ˇ#     ingress:
         ˇ#         api:
@@ -3617,7 +3574,6 @@ async fn test_indent_yaml_comments_expected_behavior(cx: &mut TestAppContext) {
 
     cx.update_editor(|e, window, cx| e.tab(&Tab, window, cx));
 
-    // EXPECTED: All lines should be indented by the same amount (2 spaces)
     cx.assert_editor_state(indoc! {"
           ˇ#     ingress:
           ˇ#         api: