feat: add macro editor cursor navigation commands#1113
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds four editor cursor-movement commands (file/line start/end), integrates them into the enum, UI, and MacroChoiceEngine dispatch, and includes unit tests for engine dispatch and command behavior (including missing active view error handling). Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "CommandSequenceEditor (UI)"
participant Engine as "MacroChoiceEngine"
participant Cmd as "MoveCursor Command"
participant App as "Obsidian App / Editor"
UI->>Engine: user selects/saves editor command (EditorCommandType)
Engine->>Cmd: map type → instantiate/choose command
Engine->>Cmd: invoke static run(app)
Cmd->>App: query active markdown view / editor
Cmd->>App: setCursor(line, ch)
App-->>Cmd: cursor updated
Cmd-->>Engine: completed
Engine-->>UI: update state / confirm
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying quickadd with
|
| Latest commit: |
b9de75c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://eace0f38.quickadd.pages.dev |
| Branch Preview URL: | https://304-feature-request-extend-e.quickadd.pages.dev |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/engine/MacroChoiceEngine.editorCommands.test.ts (1)
42-136: Optional: extract spy setup from each test intobeforeEachto reduce duplication.The same four-spy block (lines 43–54, 67–78, 91–102, 115–126) is copy-pasted into every test case. Since
vi.restoreAllMocks()already runs between tests, moving setup tobeforeEacheliminates the repetition without any behavioural change.♻️ Suggested refactor
+ let fileStartSpy: ReturnType<typeof vi.spyOn>; + let fileEndSpy: ReturnType<typeof vi.spyOn>; + let lineStartSpy: ReturnType<typeof vi.spyOn>; + let lineEndSpy: ReturnType<typeof vi.spyOn>; + beforeEach(() => { vi.restoreAllMocks(); + fileStartSpy = vi + .spyOn(MoveCursorToFileStartCommand, "run") + .mockImplementation(() => undefined); + fileEndSpy = vi + .spyOn(MoveCursorToFileEndCommand, "run") + .mockImplementation(() => undefined); + lineStartSpy = vi + .spyOn(MoveCursorToLineStartCommand, "run") + .mockImplementation(() => undefined); + lineEndSpy = vi + .spyOn(MoveCursorToLineEndCommand, "run") + .mockImplementation(() => undefined); }); it("dispatches MoveCursorToFileStart", async () => { - const fileStartSpy = vi - .spyOn(MoveCursorToFileStartCommand, "run") - .mockImplementation(() => undefined); - const fileEndSpy = vi - .spyOn(MoveCursorToFileEndCommand, "run") - .mockImplementation(() => undefined); - const lineStartSpy = vi - .spyOn(MoveCursorToLineStartCommand, "run") - .mockImplementation(() => undefined); - const lineEndSpy = vi - .spyOn(MoveCursorToLineEndCommand, "run") - .mockImplementation(() => undefined); - const app = await callExecuteEditorCommand( EditorCommandType.MoveCursorToFileStart ); // ... remaining tests follow the same pattern🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/engine/MacroChoiceEngine.editorCommands.test.ts` around lines 42 - 136, Tests duplicate the same four spy setups in each it block; move that repeated setup into a beforeEach to DRY the tests. Add a beforeEach that spies on MoveCursorToFileStartCommand.run, MoveCursorToFileEndCommand.run, MoveCursorToLineStartCommand.run and MoveCursorToLineEndCommand.run (using vi.spyOn(...).mockImplementation(() => undefined)) so each test can call callExecuteEditorCommand(EditorCommandType...) and assert the appropriate spy was/was not called; keep vi.restoreAllMocks() between tests as-is.src/gui/MacroGUIs/CommandSequenceEditor.ts (1)
339-355: Consider human-readable labels for the new multi-word dropdown entries.The existing short entries (
Copy,Cut,Paste) happen to be readable as raw enum strings, but the new compound names render asMoveCursorToFileStart,MoveCursorToFileEnd, etc. — a single PascalCase blob. Passing a separate display string as the second argument to.addOption()would improve clarity for users:✨ Optional: use readable display labels
- .addOption( - EditorCommandType.MoveCursorToFileStart, - EditorCommandType.MoveCursorToFileStart - ) - .addOption( - EditorCommandType.MoveCursorToFileEnd, - EditorCommandType.MoveCursorToFileEnd - ) - .addOption( - EditorCommandType.MoveCursorToLineStart, - EditorCommandType.MoveCursorToLineStart - ) - .addOption( - EditorCommandType.MoveCursorToLineEnd, - EditorCommandType.MoveCursorToLineEnd - ) + .addOption( + EditorCommandType.MoveCursorToFileStart, + "Move cursor to file start" + ) + .addOption( + EditorCommandType.MoveCursorToFileEnd, + "Move cursor to file end" + ) + .addOption( + EditorCommandType.MoveCursorToLineStart, + "Move cursor to line start" + ) + .addOption( + EditorCommandType.MoveCursorToLineEnd, + "Move cursor to line end" + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gui/MacroGUIs/CommandSequenceEditor.ts` around lines 339 - 355, The dropdown options using EditorCommandType enums (e.g., EditorCommandType.MoveCursorToFileStart, MoveCursorToFileEnd, MoveCursorToLineStart, MoveCursorToLineEnd) are displayed as raw PascalCase; update the addOption calls that add these values (the chain that calls .addOption(...)) to pass a human-readable label as the second argument (for example "Move Cursor to File Start", "Move Cursor to File End", "Move Cursor to Line Start", "Move Cursor to Line End") so the UI shows friendly text while still storing the enum value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/engine/MacroChoiceEngine.editorCommands.test.ts`:
- Around line 42-136: Tests duplicate the same four spy setups in each it block;
move that repeated setup into a beforeEach to DRY the tests. Add a beforeEach
that spies on MoveCursorToFileStartCommand.run, MoveCursorToFileEndCommand.run,
MoveCursorToLineStartCommand.run and MoveCursorToLineEndCommand.run (using
vi.spyOn(...).mockImplementation(() => undefined)) so each test can call
callExecuteEditorCommand(EditorCommandType...) and assert the appropriate spy
was/was not called; keep vi.restoreAllMocks() between tests as-is.
In `@src/gui/MacroGUIs/CommandSequenceEditor.ts`:
- Around line 339-355: The dropdown options using EditorCommandType enums (e.g.,
EditorCommandType.MoveCursorToFileStart, MoveCursorToFileEnd,
MoveCursorToLineStart, MoveCursorToLineEnd) are displayed as raw PascalCase;
update the addOption calls that add these values (the chain that calls
.addOption(...)) to pass a human-readable label as the second argument (for
example "Move Cursor to File Start", "Move Cursor to File End", "Move Cursor to
Line Start", "Move Cursor to Line End") so the UI shows friendly text while
still storing the enum value.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/engine/MacroChoiceEngine.editorCommands.test.ts (2)
43-60: Consolidate the twobeforeEachblocks into one.Two back-to-back
beforeEachhooks at the samedescribelevel are valid but needlessly split.vi.restoreAllMocks()and the spy setup are a single logical unit.♻️ Proposed consolidation
- beforeEach(() => { - vi.restoreAllMocks(); - }); - - beforeEach(() => { - fileStartSpy = vi + beforeEach(() => { + vi.restoreAllMocks(); + fileStartSpy = vi .spyOn(MoveCursorToFileStartCommand, "run") .mockImplementation(() => undefined); fileEndSpy = vi .spyOn(MoveCursorToFileEndCommand, "run") .mockImplementation(() => undefined); lineStartSpy = vi .spyOn(MoveCursorToLineStartCommand, "run") .mockImplementation(() => undefined); lineEndSpy = vi .spyOn(MoveCursorToLineEndCommand, "run") .mockImplementation(() => undefined); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/engine/MacroChoiceEngine.editorCommands.test.ts` around lines 43 - 60, Combine the two consecutive beforeEach blocks into a single beforeEach that first calls vi.restoreAllMocks() and then sets up the spies for MoveCursorToFileStartCommand.run, MoveCursorToFileEndCommand.run, MoveCursorToLineStartCommand.run, and MoveCursorToLineEndCommand.run (the fileStartSpy, fileEndSpy, lineStartSpy, lineEndSpy mocks), so mock restoration and spy setup occur in one logical initialization step.
37-104: Consider adding a test for thedefault(unhandled type) branch.
executeEditorCommandthrows on an unknownEditorCommandType(the exhaustivenevercheck). None of the four tests exercise this path, so a regression (e.g. a new enum member added without a matching case) would go undetected by this suite.➕ Suggested additional test
+ it("throws for an unhandled editor command type", async () => { + await expect( + callExecuteEditorCommand("__unknown__" as unknown as EditorCommandType) + ).rejects.toThrow("Unhandled editor command type"); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/engine/MacroChoiceEngine.editorCommands.test.ts` around lines 37 - 104, Add a test that exercises the default/unhandled branch by calling executeEditorCommand (via callExecuteEditorCommand) with an invalid EditorCommandType value (e.g., a bogus value cast to EditorCommandType) and assert it throws; specifically, add an async test that awaits expect(callExecuteEditorCommand(invalidValue as unknown as EditorCommandType)).rejects.toThrow() to ensure the exhaustive never check in executeEditorCommand is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/engine/MacroChoiceEngine.editorCommands.test.ts`:
- Around line 43-60: Combine the two consecutive beforeEach blocks into a single
beforeEach that first calls vi.restoreAllMocks() and then sets up the spies for
MoveCursorToFileStartCommand.run, MoveCursorToFileEndCommand.run,
MoveCursorToLineStartCommand.run, and MoveCursorToLineEndCommand.run (the
fileStartSpy, fileEndSpy, lineStartSpy, lineEndSpy mocks), so mock restoration
and spy setup occur in one logical initialization step.
- Around line 37-104: Add a test that exercises the default/unhandled branch by
calling executeEditorCommand (via callExecuteEditorCommand) with an invalid
EditorCommandType value (e.g., a bogus value cast to EditorCommandType) and
assert it throws; specifically, add an async test that awaits
expect(callExecuteEditorCommand(invalidValue as unknown as
EditorCommandType)).rejects.toThrow() to ensure the exhaustive never check in
executeEditorCommand is covered.
chhoumann
deleted the
304-feature-request-extend-editor-commands-for-macors
branch
|
🎉 This PR is included in version 2.12.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MacroChoiceEngineValidation
bun run testbun run build-with-lintevalharness:obsidian vault=dev dev:errorsreported no captured errorsCloses #304
Summary by CodeRabbit
New Features
Tests