cycle-spacing
#2513
cycle-spacing
#2513
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements the cycle-spacing command, which provides Emacs-style whitespace cycling functionality. The command cycles through three states when invoked repeatedly: replacing multiple spaces/tabs with a single space, deleting all whitespace, and restoring the original spacing.
Key Changes:
- Implements core
CycleSpacingcommand class with state management and interruption handling - Adds comprehensive test coverage for various scenarios including multi-cursor support, tabs, mixed whitespace, and edge cases
- Registers command bindings across multiple keybinding configurations
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/commands/edit.ts | Implements the CycleSpacing command class with cycling logic, state management, and cursor positioning |
| src/emulator.ts | Registers the CycleSpacing command in the emulator's command list |
| src/extension.ts | Binds the cycleSpacing command to make it available as a VS Code command |
| package.json | Adds keybindings for the cycleSpacing command across different meta prefix configurations |
| keybindings/move-edit.json | Adds the base keybinding entry for cycleSpacing using meta+space |
| src/test/suite/commands/edit.cycle-spacing.test.ts | Comprehensive test suite covering basic cycling, multi-cursor, tabs, interruption handling, and edge cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let from = cursorChar; | ||
| while (from > 0) { | ||
| const char = lineText[from - 1]; | ||
| if (char !== " " && char !== "\t") { | ||
| break; | ||
| } | ||
| from -= 1; | ||
| } |
There was a problem hiding this comment.
The whitespace detection logic is duplicated in two while loops. Consider extracting this into a helper function like isWhitespace(char: string): boolean to improve maintainability and make the code more DRY.
| const newPos = new vscode.Position(line, original.from + original.before.length); | ||
| return new Selection(newPos, newPos); | ||
| } | ||
| } |
There was a problem hiding this comment.
This fallback case returns the current selection unchanged, but it's unclear when this would be reached since all three states (0, 1, 2) are explicitly handled. Consider adding a comment explaining when this fallback occurs or refactoring to make the logic clearer.
| } | |
| } | |
| // This fallback should never be reached because all three states (0, 1, 2) are explicitly handled above. | |
| // If reached, it indicates an unexpected state; returning the current selection unchanged as a safeguard. |
No description provided.