feat: add restart language server command#25
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 18 minutes and 13 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe pull request adds a restart command for the HSML language server to the VS Code extension, refactors the client startup logic into a reusable helper, updates the README to clarify syntax highlighting and language-server capabilities, and introduces corresponding test mocks and test cases. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Shinigami92
force-pushed
the
feat-restart-language-server
branch
from
3a844c7 to
7cf572f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/extension.test.ts (1)
44-55: Merge this into the existingdescribe('activate', ...)block.There's already a
describe('activate', ...)block starting at line 15. This creates a duplicate test suite with the same name, which can cause confusion in test output and violates DRY.Proposed fix
Move the test case into the existing
describe('activate', ...)block at lines 15-42:describe('activate', () => { // ... existing tests ... + + it('should register restart command', async () => { + const { commands } = await import('vscode'); + const { activate } = await import('../client/src/extension.js'); + await activate(mockContext); + + expect(commands.registerCommand).toHaveBeenCalledWith( + 'hsml.restartLanguageServer', + expect.any(Function), + ); + }); }); - -describe('activate', () => { - it('should register restart command', async () => { - const { commands } = await import('vscode'); - const { activate } = await import('../client/src/extension.js'); - await activate(mockContext); - - expect(commands.registerCommand).toHaveBeenCalledWith( - 'hsml.restartLanguageServer', - expect.any(Function), - ); - }); -});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/extension.test.ts` around lines 44 - 55, This creates a duplicate describe('activate', ...) suite; move the it('should register restart command', async () => { ... }) test into the existing describe('activate', ...) block (the one that already wraps tests starting around the earlier lines) so there's only one describe for activate; keep the body that imports activate and vscode commands, calls await activate(mockContext), and asserts commands.registerCommand was called with 'hsml.restartLanguageServer' and expect.any(Function) (referencing activate, mockContext, and the commands.registerCommand assertion to locate the test).tests/__mocks__/vscode-languageclient-node.ts (1)
3-5: Consider makingmockStartreturn a Promise for consistency.The new
mockRestartcorrectly returnsPromise.resolve(), matching theLanguageClientAPI. However,mockStartreturnsundefined, which is inconsistent sinceclient/src/extension.tsnow usesawait client.start(). Whileawait undefinedworks in JavaScript, aligning the mock with the actual API signature improves test fidelity.Proposed fix
-export const mockStart = vi.fn(); +export const mockStart = vi.fn(() => Promise.resolve());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/__mocks__/vscode-languageclient-node.ts` around lines 3 - 5, mockStart currently returns undefined while mockStop and mockRestart return promises; update the mock so mockStart returns a resolved Promise to match the LanguageClient API and the usage in client/src/extension.ts (where client.start() is awaited). Locate mockStart in the tests/__mocks__/vscode-languageclient-node.ts mock file and change its factory to return a Promise.resolve() (consistent with mockStop/mockRestart) so tests reflect the real async signature.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/extension.test.ts`:
- Around line 57-73: Add a test for the "no client" branch by resetting module
state with vi.resetModules() so the module-level client is undefined, then mock
resolveServerPath (e.g. resolveServerPath.mockResolvedValue('hsml')), import the
commands and the mocked languageclient (mockStart) and the extension module,
retrieve the registered 'hsml.restartLanguageServer' callback like the existing
test, invoke it, and assert that startClient (mockStart) was called instead of
client.restart; use the same pattern of looking up commands.registerCommand mock
calls to get the callback so the test exercises startClient when client is
absent.
---
Nitpick comments:
In `@tests/__mocks__/vscode-languageclient-node.ts`:
- Around line 3-5: mockStart currently returns undefined while mockStop and
mockRestart return promises; update the mock so mockStart returns a resolved
Promise to match the LanguageClient API and the usage in client/src/extension.ts
(where client.start() is awaited). Locate mockStart in the
tests/__mocks__/vscode-languageclient-node.ts mock file and change its factory
to return a Promise.resolve() (consistent with mockStop/mockRestart) so tests
reflect the real async signature.
In `@tests/extension.test.ts`:
- Around line 44-55: This creates a duplicate describe('activate', ...) suite;
move the it('should register restart command', async () => { ... }) test into
the existing describe('activate', ...) block (the one that already wraps tests
starting around the earlier lines) so there's only one describe for activate;
keep the body that imports activate and vscode commands, calls await
activate(mockContext), and asserts commands.registerCommand was called with
'hsml.restartLanguageServer' and expect.any(Function) (referencing activate,
mockContext, and the commands.registerCommand assertion to locate the test).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eed4c710-4c1c-43cb-adc7-e9cafe8b91c2
📒 Files selected for processing (6)
README.mdclient/src/extension.tspackage.jsontests/__mocks__/vscode-languageclient-node.tstests/__mocks__/vscode.tstests/extension.test.ts
1 participant
Summary by CodeRabbit
Release Notes
New Features
Documentation