🎉 Denops v7 #344
🎉 Denops v7 #344
Conversation
WalkthroughThe recent updates to the Denops project enhance functionality and improve version compatibility. Key changes include new features for plugin management, error handling, and asynchronous operations. GitHub workflows have been refined, and documentation has been updated accordingly. These improvements aim to provide a robust development environment for Vim and Neovim plugins, ensuring better stability and an improved user experience. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Plugin
participant Denops
participant Host
User ->> Plugin: Load new plugin
Plugin ->> Denops: Register plugin
Denops ->> Host: Initialize plugin environment
Host ->> Denops: Confirmation
Denops ->> Plugin: Initialization complete
User ->> Plugin: Execute command
Plugin ->> Denops: Process command
Denops ->> Host: Execute command
Host -->> Denops: Return result
Denops ->> Plugin: Provide result
Plugin ->> User: Display result
User ->> Plugin: Unload plugin
Plugin ->> Denops: Unregister plugin
Denops ->> Host: Cleanup plugin environment
Host ->> Denops: Confirmation
Denops ->> Plugin: Unload complete
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #344 +/- ##
==========================================
+ Coverage 90.20% 95.44% +5.23%
==========================================
Files 9 23 +14
Lines 684 1361 +677
Branches 67 174 +107
==========================================
+ Hits 617 1299 +682
+ Misses 67 59 -8
- Partials 0 3 +3 ☔ View full report in Codecov by Sentry. |
lambdalisue
force-pushed
the
v7-pre
branch
3 times, most recently
from
deae6e0 to
1b97e6d
Compare
|
denoland/deno#23792 Node API is fixed so we can remove fc956a9 if we use Deno 1.44.2 or later P.S. I removed the workaround commit and rebased. |
lambdalisue
force-pushed
the
v7-pre
branch
2 times, most recently
from
b0a32f1 to
c0e4c9a
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (57)
- .github/workflows/test.yml (1 hunks)
- .github/workflows/update.yml (1 hunks)
- README.md (1 hunks)
- autoload/denops.vim (1 hunks)
- autoload/denops/_internal/job.vim (2 hunks)
- autoload/denops/_internal/plugin.vim (3 hunks)
- autoload/denops/_internal/rpc/nvim.vim (1 hunks)
- autoload/denops/_internal/rpc/vim.vim (1 hunks)
- autoload/denops/_internal/server/chan.vim (3 hunks)
- autoload/denops/_internal/server/proc.vim (6 hunks)
- autoload/denops/_internal/test.vim (1 hunks)
- autoload/denops/plugin.vim (2 hunks)
- autoload/denops/server.vim (6 hunks)
- autoload/health/denops.vim (1 hunks)
- deno.jsonc (1 hunks)
- denops/@denops-private/cli.ts (4 hunks)
- denops/@denops-private/cli_test.ts (1 hunks)
- denops/@denops-private/denops.ts (2 hunks)
- denops/@denops-private/denops_test.ts (2 hunks)
- denops/@denops-private/error.ts (1 hunks)
- denops/@denops-private/error_test.ts (1 hunks)
- denops/@denops-private/host.ts (4 hunks)
- denops/@denops-private/host/nvim.ts (2 hunks)
- denops/@denops-private/host/nvim_test.ts (4 hunks)
- denops/@denops-private/host/vim.ts (2 hunks)
- denops/@denops-private/host/vim_test.ts (4 hunks)
- denops/@denops-private/host_test.ts (1 hunks)
- denops/@denops-private/mod.ts (1 hunks)
- denops/@denops-private/plugin.ts (1 hunks)
- denops/@denops-private/service.ts (6 hunks)
- denops/@denops-private/service_test.ts (2 hunks)
- denops/@denops-private/util.ts (1 hunks)
- denops/@denops-private/version.ts (1 hunks)
- denops/@denops-private/worker.ts (2 hunks)
- denops/@denops-private/worker_test.ts (1 hunks)
- doc/denops.txt (9 hunks)
- plugin/denops.vim (2 hunks)
- plugin/denops/debug.vim (1 hunks)
- tests/denops/runtime/functions/plugin_test.ts (1 hunks)
- tests/denops/runtime/functions/server_test.ts (1 hunks)
- tests/denops/runtime/plugin_test.ts (1 hunks)
- tests/denops/testdata/dummy_invalid_constraint_plugin.ts (1 hunks)
- tests/denops/testdata/dummy_invalid_constraint_plugin2.ts (1 hunks)
- tests/denops/testdata/dummy_invalid_dispose_plugin.ts (1 hunks)
- tests/denops/testdata/dummy_invalid_plugin.ts (1 hunks)
- tests/denops/testdata/dummy_plugins/denops/@dummy_namespace/main.ts (1 hunks)
- tests/denops/testdata/dummy_plugins/denops/dummy_invalid/main.ts (1 hunks)
- tests/denops/testdata/dummy_plugins/denops/dummy_valid/main.ts (1 hunks)
- tests/denops/testdata/dummy_valid_dispose_plugin.ts (1 hunks)
- tests/denops/testdata/dummy_valid_plugin.ts (1 hunks)
- tests/denops/testdata/shared_server_test_no_verbose.ts (1 hunks)
- tests/denops/testdata/shared_server_test_verbose_true.ts (1 hunks)
- tests/denops/testutil/conf.ts (3 hunks)
- tests/denops/testutil/conf_test.ts (1 hunks)
- tests/denops/testutil/host.ts (1 hunks)
- tests/denops/testutil/mock.ts (1 hunks)
- tests/denops/testutil/mock_test.ts (1 hunks)
Files not processed due to max files limit (5)
- tests/denops/testutil/shared_server.ts
- tests/denops/testutil/shared_server_test.ts
- tests/denops/testutil/wait.ts
- tests/denops/testutil/wait_test.ts
- tests/denops/testutil/with.ts
Files not summarized due to errors (2)
- denops/@denops-private/service_test.ts: Error: Message exceeds token limit
- tests/denops/runtime/functions/server_test.ts: Error: Message exceeds token limit
Files skipped from review due to trivial changes (10)
- .github/workflows/update.yml
- denops/@denops-private/error_test.ts
- denops/@denops-private/mod.ts
- denops/@denops-private/util.ts
- tests/denops/testdata/dummy_invalid_constraint_plugin.ts
- tests/denops/testdata/dummy_invalid_constraint_plugin2.ts
- tests/denops/testdata/dummy_invalid_plugin.ts
- tests/denops/testdata/dummy_plugins/denops/@dummy_namespace/main.ts
- tests/denops/testdata/dummy_valid_plugin.ts
- tests/denops/testutil/conf_test.ts
Additional context used
Learnings (4)
autoload/denops/plugin.vim (1)
Learnt from: Milly PR: vim-denops/denops.vim#385 File: autoload/denops/plugin.vim:56-58 Timestamp: 2024-07-05T15:37:46.867Z Learning: In the Denops.vim project, the `denops#plugin#unload()` function is designed to throw exceptions to the caller, who is responsible for handling them.autoload/denops/_internal/plugin.vim (1)
Learnt from: Milly PR: vim-denops/denops.vim#385 File: autoload/denops/plugin.vim:56-58 Timestamp: 2024-07-05T15:37:46.867Z Learning: In the Denops.vim project, the `denops#plugin#unload()` function is designed to throw exceptions to the caller, who is responsible for handling them.denops/@denops-private/host/vim_test.ts (1)
Learnt from: Milly PR: vim-denops/denops.vim#352 File: denops/@denops-private/testutil/host.ts:3-4 Timestamp: 2024-05-25T06:35:34.785Z Learning: `Neovim` and `Vim` from `../host/nvim.ts` and `../host/vim.ts` are classes and should be imported normally if they are instantiated in the code.denops/@denops-private/host/nvim_test.ts (1)
Learnt from: Milly PR: vim-denops/denops.vim#352 File: denops/@denops-private/testutil/host.ts:3-4 Timestamp: 2024-05-25T06:35:34.785Z Learning: `Neovim` and `Vim` from `../host/nvim.ts` and `../host/vim.ts` are classes and should be imported normally if they are instantiated in the code.
Biome
denops/@denops-private/plugin.ts
[error] 35-35: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
denops/@denops-private/service.ts
[error] 184-184: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
GitHub Check: codecov/patch
tests/denops/testutil/host.ts
[warning] 41-41: tests/denops/testutil/host.ts#L41
Added line #L41 was not covered by tests
[warning] 85-86: tests/denops/testutil/host.ts#L85-L86
Added lines #L85 - L86 were not covered by testsdenops/@denops-private/cli.ts
[warning] 105-105: denops/@denops-private/cli.ts#L105
Added line #L105 was not covered by testsdenops/@denops-private/worker.ts
[warning] 113-113: denops/@denops-private/worker.ts#L113
Added line #L113 was not covered by testsdenops/@denops-private/host.ts
[warning] 77-77: denops/@denops-private/host.ts#L77
Added line #L77 was not covered by tests
LanguageTool
doc/denops.txt
[grammar] ~136-~136: Possible subject-verb agreement error detected.
Context: ...iseconds before restarting the server. This avoid #136. https://github.com/vim-denops/den...(THIS_THAT_AGR)
[uncategorized] ~156-~156: Possible missing article found.
Context: ...oid infinite errors. Denops will reset internal counter when the channel keeps connecti...(AI_HYDRA_LEO_MISSING_AN)
Additional comments not posted (236)
tests/denops/testdata/dummy_plugins/denops/dummy_invalid/main.ts (2)
3-5: Function definition.The function
mainis defined as anEntrypointand throws an error. This is appropriate for a dummy plugin intended to test error handling.
1-1: Import statement update.The import statement has been updated to use
jsr:@denops/core. Ensure that this change is consistent with the updated import paths across the codebase.Verification successful
Import statement update verified.
The import path
jsr:@denops/coreis consistently used across the codebase.
tests/denops/testdata/dummy_invalid_constraint_plugin.tstests/denops/testdata/dummy_valid_plugin.tstests/denops/testdata/dummy_invalid_constraint_plugin2.tstests/denops/testdata/dummy_invalid_plugin.tstests/denops/testdata/dummy_plugins/denops/@dummy_namespace/main.tstests/denops/testdata/dummy_plugins/denops/dummy_valid/main.tstests/denops/testdata/dummy_plugins/denops/dummy_invalid/main.tsdenops/@denops-private/plugin.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the import path `jsr:@denops/core` is consistent across the codebase. # Test: Search for the import path. Expect: Only occurances of `jsr:@denops/core`. rg --type ts $'from "jsr:@denops/core"'Length of output: 981
tests/denops/testdata/dummy_plugins/denops/dummy_valid/main.ts (2)
1-1: Import statement update.The import statement has been updated to use
jsr:@denops/core. Ensure that this change is consistent with the updated import paths across the codebase.
3-5: Function definition.The function
mainis defined as anEntrypointand executes a command asynchronously. This is appropriate for a dummy plugin intended to test valid command execution.tests/denops/testdata/shared_server_test_no_verbose.ts (2)
4-7: Usage of shared server.The code uses a shared server without verbose logging and introduces a delay. This is appropriate for a test scenario.
1-2: Import statements update.The import statements have been updated to use
jsr:@std/[email protected]/delayand/denops-testutil/shared_server.ts. Ensure that these changes are consistent with the updated import paths across the codebase.Verification successful
Import statements update verified.
The import paths
jsr:@std/[email protected]/delayand/denops-testutil/shared_server.tsare consistently used across the codebase, and no old import paths were found.
jsr:@std/[email protected]/delay:
tests/denops/testdata/shared_server_test_verbose_true.tstests/denops/testdata/shared_server_test_no_verbose.tstests/denops/testutil/shared_server_test.tsdenops/@denops-private/worker_test.tsdenops/@denops-private/cli_test.tsdenops/@denops-private/host/vim_test.tsdenops/@denops-private/host/nvim_test.ts
/denops-testutil/shared_server.ts:
tests/denops/testdata/shared_server_test_verbose_true.tstests/denops/testdata/shared_server_test_no_verbose.tstests/denops/runtime/plugin_test.tstests/denops/runtime/functions/server_test.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the import paths `jsr:@std/[email protected]/delay` and `/denops-testutil/shared_server.ts` are consistent across the codebase. # Test: Search for the import paths. Expect: Only occurances of the new paths. rg --type ts $'from "jsr:@std/[email protected]/delay"' rg --type ts $'from "/denops-testutil/shared_server.ts"'Length of output: 1268
Script:
#!/bin/bash # Description: Verify that there are no old import paths for `delay` and `useSharedServer`. # Test: Search for old import paths. Expect: No occurrences of the old paths. rg --type ts $'from "https://deno.land/x/[email protected]/async/delay.ts"' rg --type ts $'from "../shared_server.ts"'Length of output: 114
tests/denops/testdata/shared_server_test_verbose_true.ts (2)
1-2: Import statements update.The import statements have been updated to use
jsr:@std/[email protected]/delayand/denops-testutil/shared_server.ts. Ensure that these changes are consistent with the updated import paths across the codebase.
4-7: Usage of shared server with verbose logging.The code uses a shared server with verbose logging and introduces a delay. This is appropriate for a test scenario.
tests/denops/testdata/dummy_valid_dispose_plugin.ts (1)
1-9: LGTM!The code introduces a valid entry point with an async dispose method that uses
denops.cmdto echo a message. The implementation looks correct.tests/denops/testdata/dummy_invalid_dispose_plugin.ts (1)
1-9: LGTM!The code introduces a valid entry point with an async dispose method that throws an error. The implementation looks correct.
denops/@denops-private/error.ts (3)
1-7: LGTM!The import paths have been updated to use
jsrimports. The changes look correct.
Line range hint
9-15:
LGTM!The
errorSerializerfunction correctly serializes an error object to a JSON string. The implementation looks correct.
Line range hint
17-27:
LGTM!The
errorDeserializerfunction correctly deserializes a JSON string to an error object. The implementation looks correct.plugin/denops/debug.vim (1)
12-14: LGTM!The new autocmd events handle plugin unloading states and look correct.
autoload/denops/_internal/test.vim (2)
11-11: LGTM!The change to use
_handlein the argument dictionary aligns with the modifications in thedenops#_internal#rpc#vim#notifyfunction.
15-15: LGTM!The change to use
_handlein the argument dictionary aligns with the modifications in thedenops#_internal#rpc#vim#requestfunction.denops/@denops-private/version.ts (2)
1-4: LGTM!The imports have been correctly updated to use
jsrimports, aligning with the new versioning scheme.
Line range hint
6-21: LGTM!The function
getVersionOrcorrectly handles the command output and fallback logic.deno.jsonc (4)
3-8: LGTM!The task definitions have been correctly updated and new tasks added to align with the intended functionality.
11-14: LGTM!The test exclusion and the TODO comment are clear and necessary.
16-18: LGTM!The
.coverage/directory exclusion is necessary and aligns with the intended functionality.
20-22: LGTM!The import mappings for test modules are correct and align with the intended functionality.
denops/@denops-private/plugin.ts (1)
1-2: LGTM!The TODO comment is clear, and the import statement is correct.
plugin/denops.vim (3)
6-8: LGTM! Version check updated correctly.The version check for Vim and Neovim has been updated to the required versions. The warning message is appropriate.
25-27: LGTM! New autocommands added correctly.The new autocommands for plugin unload events are correctly defined and placed within the appropriate autocommand group.
34-37: LGTM! Server connection logic is appropriate.The function call to connect or start the server is correctly placed and the logic is appropriate for Vim startup or later.
autoload/denops/_internal/rpc/vim.vim (4)
6-14: LGTM! Channel handling updated correctly.The changes to handle the channel object and close callback in the
denops#_internal#rpc#vim#connectfunction are correct.
23-25: LGTM! Close callback handled correctly.The changes to call the close callback after closing the channel in the
denops#_internal#rpc#vim#closefunction are correct.
29-29: LGTM! Channel handle used correctly.The changes to use the channel handle in the
denops#_internal#rpc#vim#notifyfunction are correct.
33-33: LGTM! Channel handle used correctly.The changes to use the channel handle in the
denops#_internal#rpc#vim#requestfunction are correct.autoload/denops.vim (1)
28-34: LGTM! New interrupt function added correctly.The new function
denops#interruptfor interrupting asynchronous requests is correctly implemented.tests/denops/testutil/conf.ts (3)
1-3: LGTM! Import statements updated correctly.The import statements have been updated to use
jsrimports correctly.
12-12: LGTM!Configinterface updated correctly.The
Configinterface has been updated to include an optionaltimeoutproperty correctly.
22-30: LGTM!getConfigfunction updated correctly.The
getConfigfunction has been updated to handle thetimeoutproperty correctly.autoload/denops/_internal/rpc/nvim.vim (1)
27-27: LGTM! The changes are correct and improve asynchronous handling.The added line ensures the
_on_closecallback is executed asynchronously after the channel is closed.denops/@denops-private/denops.ts (3)
6-7: LGTM! The import path changes are correct.The import paths have been updated to use
jsrimports, reflecting the new module locations.
16-19: LGTM! The type changes are correct.The addition of the
interruptedproperty to theServicetype aligns with the new functionality.
43-45: LGTM! The class changes are correct.The new
interruptedproperty correctly returns theinterruptedsignal from theservice..github/workflows/test.yml (2)
51-51: LGTM! The Deno version update is correct.The Deno version has been updated to
1.44.x, aligning with the PR objectives.
54-56: LGTM! The Vim and Neovim version updates are correct.The Vim version has been updated to
v9.1.0448and the Neovim version tov0.10.0, aligning with the PR objectives.tests/denops/testutil/host.ts (3)
1-4: LGTM! The import changes are correct.The new imports reflect the updated module locations.
19-42: LGTM! ThewithHostfunction is well-implemented.The function correctly handles the different hosts based on the
modeparameter.Tools
GitHub Check: codecov/patch
[warning] 41-41: tests/denops/testutil/host.ts#L41
Added line #L41 was not covered by tests
63-86: LGTM! ThetestHostfunction is well-implemented.The function correctly handles the different testing modes and integrates with Deno's testing framework.
Consider adding tests for uncovered lines.
The static analysis tool indicates that lines 41 and 85-86 are not covered by tests. Ensure these lines are covered to improve test coverage.
Tools
GitHub Check: codecov/patch
[warning] 85-86: tests/denops/testutil/host.ts#L85-L86
Added lines #L85 - L86 were not covered by testsautoload/denops/_internal/job.vim (5)
27-32: Verify the correctness of theon_exitcallback parameters.Ensure that the status and event parameters passed to
on_exitare correct and consistent with the expected values.
65-72: Verify the correctness of theexit_cbcallback parameters.Ensure that the status and event parameters passed to
exit_cbare correct and consistent with the expected values.
81-82: Verify the usage of the event parameter ins:out_cb.Ensure that the event parameter is correctly used within the callback function.
85-86: Verify the usage of the status parameter ins:exit_cb.Ensure that the status parameter is correctly used within the callback function.
27-32: Verify the correctness of theon_exitcallback parameters.Ensure that the status and event parameters passed to
on_exitare correct and consistent with the expected values.denops/@denops-private/cli.ts (4)
Line range hint
12-31: Verify the robustness of the worker termination logic.Ensure that the worker termination logic is robust and does not introduce any resource leaks.
Line range hint
37-101: Verify the robustness of the connection handling logic.Ensure that the connection handling logic is robust and does not introduce any resource leaks.
Tools
GitHub Check: codecov/patch
[warning] 105-105: denops/@denops-private/cli.ts#L105
Added line #L105 was not covered by tests
60-82: Verify the robustness of the error handling logic.Ensure that the error handling logic is robust and does not introduce any resource leaks.
Line range hint
37-101: Verify the robustness of the connection handling logic.Ensure that the connection handling logic is robust and does not introduce any resource leaks.
Tools
GitHub Check: codecov/patch
[warning] 105-105: denops/@denops-private/cli.ts#L105
Added line #L105 was not covered by testsdenops/@denops-private/worker.ts (4)
66-91: Verify the robustness of the host detection logic.Ensure that the host detection logic is robust and does not introduce any resource leaks.
94-110: Verify the robustness of the error handling logic.Ensure that the error handling logic is robust and does not introduce any resource leaks.
94-110: Verify the robustness of the error handling logic.Ensure that the error handling logic is robust and does not introduce any resource leaks.
66-91: Verify the robustness of the host detection logic.Ensure that the host detection logic is robust and does not introduce any resource leaks.
denops/@denops-private/host.ts (4)
27-27: Verify the correctness of the asynchronous behavior innotify.Ensure that the asynchronous behavior does not introduce any unintended side effects.
69-90: Verify the correctness of the new service methods ininvoke.Ensure that the new service methods are correctly implemented and do not introduce any unintended side effects.
Tools
GitHub Check: codecov/patch
[warning] 77-77: denops/@denops-private/host.ts#L77
Added line #L77 was not covered by tests
69-90: Verify the correctness of the new service methods ininvoke.Ensure that the new service methods are correctly implemented and do not introduce any unintended side effects.
Tools
GitHub Check: codecov/patch
[warning] 77-77: denops/@denops-private/host.ts#L77
Added line #L77 was not covered by tests
27-27: Verify the correctness of the asynchronous behavior innotify.Ensure that the asynchronous behavior does not introduce any unintended side effects.
tests/denops/testutil/mock.ts (5)
4-7: FunctionpendingPromiselooks good.The function correctly returns a Promise that never resolves or rejects.
9-53: FunctioncreateFakeTcpListenerlooks good.The function correctly returns a fake
TcpListenerinstance with all necessary methods and properties.
56-89: FunctioncreateFakeTcpConnlooks good.The function correctly returns a fake
TcpConninstance with all necessary methods and properties.
92-103: FunctioncreateFakeWorkerlooks good.The function correctly returns a fake
Workerinstance with all necessary methods and properties.
106-113: FunctioncreateFakeMetalooks good.The function correctly returns a fake
Metaobject with all necessary properties.denops/@denops-private/denops_test.ts (9)
Line range hint
7-22:
Test case forDenopsImplinitialization looks good.The test correctly initializes the
DenopsImplclass with all necessary properties.
27-29: Test case forinterruptedproperty looks good.The test correctly checks if the
interruptedproperty returns anAbortSignalinstance.
30-40: Test case forredrawmethod looks good.The test correctly checks if the
redrawmethod calls thehost.redrawmethod with appropriate arguments.
44-49: Test case forcallmethod looks good.The test correctly checks if the
callmethod calls thehost.callmethod with appropriate arguments.
53-61: Test case forbatchmethod looks good.The test correctly checks if the
batchmethod calls thehost.batchmethod with appropriate arguments.
65-69: Test case forcmdmethod looks good.The test correctly checks if the
cmdmethod calls thehost.callmethod with appropriate arguments.
73-77: Test case forevalmethod looks good.The test correctly checks if the
evalmethod calls thehost.callmethod with appropriate arguments.
81-89: Test case fordispatchmethod looks good.The test correctly checks if the
dispatchmethod calls theservice.dispatchmethod after callingservice.waitLoaded.
96-105: Test case for internal waiting mechanism indispatchmethod looks good.The test correctly verifies that the
dispatchmethod waits forservice.waitLoadedto resolve before callingservice.dispatch.denops/@denops-private/host/vim.ts (7)
Line range hint
10-26:
Constructor ofVimclass looks good.The constructor correctly initializes the
sessionandclientproperties and starts the session with appropriate handling for message dispatching and error logging.
Line range hint
28-30:
redrawmethod looks good.The method correctly calls the
client.redrawmethod and returns a resolved promise.
Line range hint
32-49:
callmethod looks good.The method correctly calls the
client.callmethod and handles the result, throwing an error if the result is "ERROR".
Line range hint
51-73:
batchmethod looks good.The method correctly calls the
client.callmethod for batching operations and handles the result, throwing an error if the result is "ERROR".
84-85:notifymethod looks good.The method correctly calls the
client.callNoReplymethod with the provided arguments.
Line range hint
87-90:
initmethod looks good.The method correctly binds the provided service to the instance and returns a resolved promise.
Line range hint
92-100:
waitClosedandSymbol.asyncDisposemethods look good.The methods correctly handle session closure and shutdown with appropriate error handling.
denops/@denops-private/host/nvim.ts (6)
Line range hint
7-20:
Constructor ofNeovimclass looks good.The constructor correctly initializes the
sessionandclientproperties and sets up the dispatcher and error handling.
Line range hint
22-24:
redrawmethod looks good.The method is correctly implemented as a no-op for Neovim.
Line range hint
26-34:
callmethod looks good.The method correctly calls the
client.callmethod and handles the result, throwing an error if the result is anNvimErrorObject.
Line range hint
36-55:
batchmethod looks good.The method correctly calls the
client.callmethod for batching operations and handles the result, throwing an error if the result contains an error.
89-90:notifymethod looks good.The method correctly calls the
client.notifymethod with the provided arguments.
Line range hint
92-106:
initmethod looks good.The method correctly sets the client info and binds the provided service to the instance.
Verification successful for updated import paths.
The updated import paths and versions are correct and present in the codebase:
jsr:@std/[email protected]jsr:@std/[email protected]/mockjsr:@denops/[email protected]jsr:@lambdalisue/[email protected]jsr:@lambdalisue/[email protected]Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the updated import paths and versions. # Test: Check if the updated import paths and versions exist. rg --type jsr '@std/[email protected]' && rg --type jsr '@std/[email protected]/mock' && rg --type jsr '@denops/[email protected]' && rg --type jsr '@lambdalisue/[email protected]' && rg --type jsr '@lambdalisue/[email protected]'Length of output: 63
Script:
#!/bin/bash # Description: Verify the correctness of the updated import paths and versions. # Test: Check if the updated import paths and versions exist. rg '@std/[email protected]' && rg '@std/[email protected]/mock' && rg '@denops/[email protected]' && rg '@lambdalisue/[email protected]' && rg '@lambdalisue/[email protected]'Length of output: 4239
tests/denops/runtime/functions/server_test.ts (6)
1-12: Imports updated correctly.The imports have been updated to use
jsrimports and new dependencies have been added for the tests.
118-492: Test cases fordenops#server#start()look good.The test cases cover various scenarios for the
denops#server#start()function.Ensure that all possible start scenarios are covered in the tests.
Verification successful
Test cases for
denops#server#start()look good.The test cases cover various scenarios for the
denops#server#start()function.
- The function is called in different contexts, ensuring coverage of various start conditions.
- Scenarios include starting when disabled, already started, connecting to a shared server, and handling unexpected stops.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all possible start scenarios for `denops#server#start()` are covered in the tests. # Test: Search for the start scenarios in the test file. Expect: All scenarios are covered. rg --type js 'denops#server#start' tests/denops/runtime/functions/server_test.tsLength of output: 970
1046-1277: Test cases fordenops#server#close()look good.The test cases cover various scenarios for the
denops#server#close()function.Ensure that all possible close scenarios are covered in the tests.
Verification successful
Test cases for
denops#server#close()appear comprehensive.The search results indicate that multiple scenarios for the
denops#server#close()function are tested. These include calls when the server is not yet connected, already connected to a local server, and connected to a shared server, as well as handling timeouts.
- Lines covering "denops#server#close()" in various scenarios:
- Not yet connected
- Already connected to local-server
- Already connected to shared-server
- Handling timeouts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all possible close scenarios for `denops#server#close()` are covered in the tests. # Test: Search for the close scenarios in the test file. Expect: All scenarios are covered. rg --type js 'denops#server#close' tests/denops/runtime/functions/server_test.tsLength of output: 851
16-116: Test cases fordenops#server#status()look good.The test cases cover various scenarios for the
denops#server#status()function.Ensure that all possible statuses are covered in the tests.
Verification successful
Test cases for
denops#server#status()look good.The test cases cover various scenarios for the
denops#server#status()function, including all possible statuses: "stopped", "starting", "preparing", "running", "closing", and "closed".
- Verified all statuses are covered in the tests.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all possible statuses for `denops#server#status()` are covered in the tests. # Test: Search for the statuses in the test file. Expect: All statuses are covered. rg --type js 'denops#server#status' tests/denops/runtime/functions/server_test.tsLength of output: 5647
625-1044: Test cases fordenops#server#connect()look good.The test cases cover various scenarios for the
denops#server#connect()function.Ensure that all possible connect scenarios are covered in the tests.
Verification successful
Test cases for
denops#server#connect()look good.The test cases cover various scenarios for the
denops#server#connect()function.
- File:
tests/denops/runtime/functions/server_test.ts- Lines: Multiple instances found for
denops#server#connect().Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all possible connect scenarios for `denops#server#connect()` are covered in the tests. # Test: Search for the connect scenarios in the test file. Expect: All scenarios are covered. rg --type js 'denops#server#connect' tests/denops/runtime/functions/server_test.tsLength of output: 925
495-623: Test cases fordenops#server#stop()look good.The test cases cover various scenarios for the
denops#server#stop()function.Ensure that all possible stop scenarios are covered in the tests.
Verification successful
Test cases for
denops#server#stop()are comprehensive.The test cases cover various scenarios for the
denops#server#stop()function, including stopping when not started, stopping when already running, handling unexpected stops, and more.
denops#server#stopis tested in different states and transitions.- The tests include scenarios for both local and shared servers.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all possible stop scenarios for `denops#server#stop()` are covered in the tests. # Test: Search for the stop scenarios in the test file. Expect: All scenarios are covered. rg --type js 'denops#server#stop' tests/denops/runtime/functions/server_test.tsLength of output: 754
Script:
#!/bin/bash # Description: Extract and review the context around each instance of `denops#server#stop` to ensure comprehensive coverage. # Extract 10 lines before and after each occurrence of `denops#server#stop` to understand the context. rg --type js --context 10 'denops#server#stop' tests/denops/runtime/functions/server_test.tsLength of output: 9210
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
denops/@denops-private/service.ts (2)
41-62: Ensure proper error handling during plugin load.The
loadmethod correctly handles plugin loading and error cases. However, consider logging the error in the catch block for better debugging.} catch (e) { this.#plugins.delete(name); + console.error(`Failed to load plugin '${name}': ${e}`); }
Line range hint
182-192: Address the potential issue with variable usage.The variable
this.#loadedWaiteris used before its declaration. This can cause issues in some environments.constructor(denops: Denops, name: string, script: string) { this.#denops = denops; this.name = name; this.script = resolveScriptUrl(script); - this.#loadedWaiter = this.#load(); + this.#loadedWaiter = Promise.resolve().then(() => this.#load()); }Tools
Biome
[error] 183-183: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- deno.jsonc (1 hunks)
- denops/@denops-private/service.ts (6 hunks)
Files skipped from review as they are similar to previous changes (1)
- deno.jsonc
Additional context used
Biome
denops/@denops-private/service.ts
[error] 183-183: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
Additional comments not posted (18)
denops/@denops-private/service.ts (18)
1-5: LGTM! Import updates are correct.The import statements have been updated to use new module sources, aligning with the version updates.
11-13: Good use of AbortController for interruption.The
#interruptControllerproperty andinterruptedgetter are correctly implemented to handle interruption signals.
65-79: Efficient plugin unloading with proper cleanup.The
#unloadmethod efficiently handles plugin unloading and cleans up related resources.
81-89: Theunloadandreloadmethods are correctly implemented.Both methods handle the respective operations properly, ensuring plugins are unloaded and reloaded as needed.
99-102: Correct implementation of theinterruptmethod.The
interruptmethod correctly aborts the current controller and initializes a new one.
Line range hint
124-140: Proper handling of asynchronous dispatch with callbacks.The
dispatchAsyncmethod correctly handles async dispatch and calls the appropriate callbacks for success and failure.
149-165: Comprehensive cleanup in theclosemethod.The
closemethod ensures all resources are properly cleaned up when the service is closed.
167-172: Correct async disposal implementation.The
[Symbol.asyncDispose]method correctly calls theclosemethod for async disposal.
195-197: ThewaitLoadedmethod is correctly implemented.The
waitLoadedmethod correctly returns the promise for the plugin load.
Line range hint
199-224: Proper error handling in the#loadmethod.The
#loadmethod correctly handles errors and provides useful warnings for known issues.
226-242: Comprehensive cleanup in theunloadmethod.The
unloadmethod ensures all resources are properly cleaned up when the plugin is unloaded.
Line range hint
244-252: Correct implementation of thecallmethod.The
callmethod correctly dispatches the function call to the plugin and handles errors appropriately.
258-260: Correct implementation of thevoidAsyncDisposableconstant.The
voidAsyncDisposableconstant is correctly implemented to provide a default disposable object.
264-270: Correct implementation of thecreateScriptSuffixfunction.The
createScriptSuffixfunction correctly handles script suffix creation to ensure proper reload behavior.
271-273: Correct implementation of theemitfunction.The
emitfunction correctly handles emitting events in Vim/Neovim.
Line range hint
275-277: Correct implementation of theresolveScriptUrlfunction.The
resolveScriptUrlfunction correctly resolves the script URL.
Line range hint
279-286: Correct implementation of theisDenoCacheIssueErrorfunction.The
isDenoCacheIssueErrorfunction correctly identifies known Deno cache issues.
Line range hint
288-295: Correct implementation of thetoVimErrorfunction.The
toVimErrorfunction correctly converts errors to a string format suitable for Vim/Neovim.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- denops/@denops-private/denops.ts (2 hunks)
- denops/@denops-private/denops_test.ts (2 hunks)
- denops/@denops-private/service.ts (6 hunks)
- denops/@denops-private/service_test.ts (2 hunks)
- denops/@denops-private/util.ts (1 hunks)
- denops/@denops-private/worker.ts (2 hunks)
- tests/denops/testdata/dummy_invalid_dispose_plugin.ts (1 hunks)
- tests/denops/testdata/dummy_valid_dispose_plugin.ts (1 hunks)
- tests/denops/testutil/mock.ts (1 hunks)
Files not summarized due to errors (1)
- denops/@denops-private/service_test.ts: Error: Message exceeds token limit
Additional context used
Biome
denops/@denops-private/service.ts
[error] 183-183: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
Additional comments not posted (57)
tests/denops/testdata/dummy_valid_dispose_plugin.ts (2)
1-1: Import statement is correct.The import statement follows the project's module resolution strategy.
3-9: Entrypoint plugin implementation is correct.The
mainfunction is correctly structured as anEntrypointwith an async dispose method.tests/denops/testdata/dummy_invalid_dispose_plugin.ts (2)
1-1: Import statement is correct.The import statement follows the project's module resolution strategy.
3-9: Entrypoint plugin implementation is correct.The
mainfunction is correctly structured as anEntrypointwith an async dispose method that throws an error for testing purposes.denops/@denops-private/util.ts (2)
1-2: Import statements are correct.The import statements follow the project's module resolution strategy.
Line range hint
4-9:
Predicate definition is correct.The
isMetapredicate is correctly defined to validate theMetaobject structure.denops/@denops-private/denops.ts (3)
6-8: Import statements are correct.The import statements follow the project's module resolution strategy.
16-19: Type definition is correct.The
Servicetype is correctly defined to include thedispatch,waitLoaded, andinterruptedproperties.
43-45: Getter method implementation is correct.The
interruptedgetter method is correctly implemented in theDenopsImplclass.denops/@denops-private/worker.ts (3)
50-64: LGTM!The
patchConsolefunction correctly patches console methods and handles errors by falling back to the original method.
66-91: LGTM!The
connectHostfunction correctly handles the connection and initialization process, and uses a race condition to wait for closure signals.
94-113: LGTM!The
mainfunction correctly sets up the event listener for unhandled rejections and handles errors during the connection process.tests/denops/testutil/mock.ts (5)
5-7: LGTM!The
pendingPromisefunction correctly returns a promise that never resolves or rejects.
10-53: LGTM!The
createFakeTcpListenerfunction correctly implements a fakeTcpListenerwith appropriate methods and error handling.
57-89: LGTM!The
createFakeTcpConnfunction correctly implements a fakeTcpConnwith appropriate methods and error handling.
93-104: LGTM!The
createFakeWorkerfunction correctly implements a fakeWorkerwith appropriate methods.
107-114: LGTM!The
createFakeMetafunction correctly implements a fakeMetaobject.denops/@denops-private/denops_test.ts (8)
27-29: LGTM!The test step correctly verifies that
interruptedreturns anAbortSignalinstance.
30-40: LGTM!The test step correctly verifies that
redraw()calls thehost.redraw()method with appropriate arguments.
44-49: LGTM!The test step correctly verifies that
call()calls thehost.call()method with appropriate arguments.
53-61: LGTM!The test step correctly verifies that
batch()calls thehost.batch()method with appropriate arguments.
65-69: LGTM!The test step correctly verifies that
cmd()calls thehost.call()method with appropriate arguments.
73-77: LGTM!The test step correctly verifies that
eval()calls thehost.call()method with appropriate arguments.
81-89: LGTM!The test step correctly verifies that
dispatch()calls theservice.dispatch()method with appropriate arguments.
95-105: LGTM!The test step correctly verifies that
dispatch()waits forservice.waitLoaded()before callingservice.dispatch().denops/@denops-private/service.ts (2)
10-34: LGTM!The
Serviceclass correctly implements methods for binding, loading, unloading, reloading, and dispatching plugins. Theinterruptmethod is correctly implemented.
Line range hint
176-270:
LGTM!The
Pluginclass correctly implements methods for loading, unloading, and calling plugin functions. The#loadmethod handles errors and provides warnings for Deno module cache issues.denops/@denops-private/service_test.ts (30)
3-6: Update imports to use the new versioning system.The imports have been updated to use the new versioning system (
jsr:). Ensure all required modules are correctly imported.
49-54: Ensure comprehensive testing forServiceclass instantiation.The test checks if an instance of the
Serviceclass is correctly created. Ensure that the test is comprehensive.
57-63: Ensure comprehensive testing for.bind()method.The test checks if the host is correctly bound to the
Serviceinstance. Ensure that the test is comprehensive.
65-91: Ensure comprehensive testing for.load()method when no host is bound.The test checks if the
.load()method correctly rejects when no host is bound. Ensure that the test is comprehensive.
97-135: Ensure comprehensive testing for.load()method with valid plugin.The test checks if the
.load()method correctly resolves and emits events when a valid plugin is loaded. Ensure that the test is comprehensive.
137-174: Ensure comprehensive testing for.load()method with invalid plugin entrypoint.The test checks if the
.load()method correctly handles errors when the plugin entrypoint throws an error. Ensure that the test is comprehensive.
176-226: Ensure comprehensive testing for.load()method with invalid plugin constraints.The test checks if the
.load()method correctly handles errors when the plugin constraints are invalid. Ensure that the test is comprehensive.
228-280: Ensure comprehensive testing for.load()method with invalid plugin constraint versions.The test checks if the
.load()method correctly handles errors when the plugin constraint versions are invalid. Ensure that the test is comprehensive.
283-308: Ensure comprehensive testing for.load()method when plugin is already loaded.The test checks if the
.load()method correctly resolves and handles the case when the plugin is already loaded. Ensure that the test is comprehensive.
310-353: Ensure comprehensive testing for.load()method when plugin is already unloaded.The test checks if the
.load()method correctly resolves and emits events when the plugin is already unloaded. Ensure that the test is comprehensive.
356-389: Ensure comprehensive testing for.unload()method when plugin returns void.The test checks if the
.unload()method correctly resolves and emits events when the plugin returns void. Ensure that the test is comprehensive.
391-433: Ensure comprehensive testing for.unload()method when plugin returns AsyncDisposable.The test checks if the
.unload()method correctly resolves and emits events when the plugin returns AsyncDisposable. Ensure that the test is comprehensive.
435-476: Ensure comprehensive testing for.unload()method when plugin dispose method throws.The test checks if the
.unload()method correctly handles errors when the plugin dispose method throws an error. Ensure that the test is comprehensive.
478-499: Ensure comprehensive testing for.unload()method when plugin is not yet loaded.The test checks if the
.unload()method correctly resolves and handles the case when the plugin is not yet loaded. Ensure that the test is comprehensive.
501-527: Ensure comprehensive testing for.unload()method when plugin is already unloaded.The test checks if the
.unload()method correctly resolves and handles the case when the plugin is already unloaded. Ensure that the test is comprehensive.
529-572: Ensure comprehensive testing for.unload()method when called beforeload()resolves.The test checks if the
.unload()method correctly resolves and emits events in the correct order when called beforeload()resolves. Ensure that the test is comprehensive.
574-607: Ensure comprehensive testing for.unload()method when called beforeload()resolves with error.The test checks if the
.unload()method correctly resolves and emits events in the correct order when called beforeload()resolves with error. Ensure that the test is comprehensive.
609-637: Ensure comprehensive testing for.unload()method whenhost.call()rejects.The test checks if the
.unload()method correctly resolves and handles errors whenhost.call()rejects. Ensure that the test is comprehensive.
640-835: Ensure comprehensive testing for.reload()method.The test checks if the
.reload()method correctly resolves and emits events in various scenarios, including when the plugin is already loaded, not yet loaded, already unloaded, and when the plugin file is changed. Ensure that the test is comprehensive.
838-930: Ensure comprehensive testing for.waitLoaded()method.The test checks if the
.waitLoaded()method correctly handles various scenarios, including when the plugin is not yet loaded, already unloaded, already loaded, and when the service is closed. Ensure that the test is comprehensive.
933-950: Ensure comprehensive testing for.interrupt()method.The test checks if the
.interrupt()method correctly sends a signal to theinterruptedattribute with and without a reason. Ensure that the test is comprehensive.
953-993: Ensure comprehensive testing for.interruptedproperty.The test checks if the
.interruptedproperty correctly handles various scenarios, including before and after.interrupt()is called, and if it returns the same or a new instance. Ensure that the test is comprehensive.
996-1037: Ensure comprehensive testing for.dispatch()method.The test checks if the
.dispatch()method correctly resolves and handles various scenarios, including when the plugin is already loaded, not yet loaded, and when the plugin API call fails. Ensure that the test is comprehensive.
1075-1299: Ensure comprehensive testing for.dispatchAsync()method.The test checks if the
.dispatchAsync()method correctly resolves and handles various scenarios, including when no host is bound, when the plugin API calls succeed or fail, and when the success or failure callbacks fail. Ensure that the test is comprehensive.
1301-1357: Ensure comprehensive testing for.close()method.The test checks if the
.close()method correctly resolves and handles various scenarios, including when the service is not yet closed and when it is already closed. Ensure that the test is comprehensive.
1359-1391: Ensure comprehensive testing for.waitClosed()method.The test checks if the
.waitClosed()method correctly handles various scenarios, including when the service is not yet closed, already closed, and when the service is closed. Ensure that the test is comprehensive.
1393-1406: Ensure comprehensive testing for[Symbol.asyncDispose]()method.The test checks if the
[Symbol.asyncDispose]()method correctly resolves and calls the.close()method. Ensure that the test is comprehensive.
1410-1412: Verify path construction inresolvefunction.Ensure that the path construction in the
resolvefunction is accurate.Verification successful
Path construction in
resolvefunction is accurate.The
resolvefunction correctly constructs paths for test scripts as evidenced by its usage in the test files.
tests/denops/testutil/shared_server_test.tstests/denops/runtime/functions/plugin_test.tsdenops/@denops-private/service_test.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the path construction in the `resolve` function. # Test: Search for the `resolve` function usage. Expect: Correct path construction. rg --type typescript $'resolve('Length of output: 100
Script:
#!/bin/bash # Description: Verify the path construction in the `resolve` function. # Test: Search for the `resolve` function usage. Expect: Correct path construction. rg --type typescript 'resolve\('Length of output: 69
Script:
#!/bin/bash # Description: Verify the path construction in the `resolve` function. # Test: Search for the `resolve` function usage in `.ts` files. Expect: Correct path construction. rg 'resolve\(' --glob '*.ts'Length of output: 3378
1414-1422: Verify temporary file creation and cleanup inuseTempFilefunction.Ensure that the temporary file creation and cleanup in the
useTempFilefunction are handled properly.Verification successful
Verified temporary file creation and cleanup in
useTempFilefunction.The function is used in a test scenario with proper creation and cleanup mechanisms.
denops/@denops-private/service_test.ts: Lines around 1414-1422Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the temporary file creation and cleanup in the `useTempFile` function. # Test: Search for the `useTempFile` function usage. Expect: Proper creation and cleanup of temporary files. rg --type typescript $'useTempFile('Length of output: 112
Script:
#!/bin/bash # Description: Verify the temporary file creation and cleanup in the `useTempFile` function. # Test: Search for the `useTempFile` function usage. Expect: Proper creation and cleanup of temporary files. rg --type typescript 'useTempFile\('Length of output: 73
Script:
#!/bin/bash # Description: Verify the temporary file creation and cleanup in the `useTempFile` function. # Test: Search for the `useTempFile` function usage. Expect: Proper creation and cleanup of temporary files. rg 'useTempFile\('Length of output: 208
Script:
#!/bin/bash # Extract the relevant code around the usage of `useTempFile` in `denops/@denops-private/service_test.ts` rg -C 5 'useTempFile\(' denops/@denops-private/service_test.tsLength of output: 826
20-25: Ensure correct import paths and type definitions.The import paths and type definitions have been updated. Verify that the paths and types are correct.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- tests/denops/testdata/dummy_invalid_constraint_plugin.ts (1 hunks)
- tests/denops/testdata/dummy_invalid_constraint_plugin2.ts (1 hunks)
- tests/denops/testdata/dummy_invalid_dispose_plugin.ts (1 hunks)
- tests/denops/testdata/dummy_invalid_plugin.ts (1 hunks)
- tests/denops/testdata/dummy_plugins/denops/@dummy_namespace/main.ts (1 hunks)
- tests/denops/testdata/dummy_plugins/denops/dummy_invalid/main.ts (1 hunks)
- tests/denops/testdata/dummy_plugins/denops/dummy_valid/main.ts (1 hunks)
- tests/denops/testdata/dummy_valid_dispose_plugin.ts (1 hunks)
- tests/denops/testdata/dummy_valid_plugin.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- tests/denops/testdata/dummy_plugins/denops/@dummy_namespace/main.ts
- tests/denops/testdata/dummy_plugins/denops/dummy_valid/main.ts
Files skipped from review as they are similar to previous changes (6)
- tests/denops/testdata/dummy_invalid_constraint_plugin.ts
- tests/denops/testdata/dummy_invalid_constraint_plugin2.ts
- tests/denops/testdata/dummy_invalid_plugin.ts
- tests/denops/testdata/dummy_plugins/denops/dummy_invalid/main.ts
- tests/denops/testdata/dummy_valid_dispose_plugin.ts
- tests/denops/testdata/dummy_valid_plugin.ts
Additional comments not posted (4)
tests/denops/testdata/dummy_invalid_dispose_plugin.ts (4)
1-1: LGTM!The import statement correctly imports the
Entrypointtype from the specified module.
3-4: LGTM!The
mainfunction is correctly declared as anEntrypointand returns an object.
5-7: LGTM!The async dispose method correctly throws a dummy error for testing purposes.
8-9: LGTM!The closing braces are correctly placed.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (57)
- .github/workflows/test.yml (1 hunks)
- .github/workflows/update.yml (1 hunks)
- README.md (1 hunks)
- autoload/denops.vim (1 hunks)
- autoload/denops/_internal/echo.vim (1 hunks)
- autoload/denops/_internal/event.vim (1 hunks)
- autoload/denops/_internal/job.vim (2 hunks)
- autoload/denops/_internal/plugin.vim (3 hunks)
- autoload/denops/_internal/rpc/nvim.vim (1 hunks)
- autoload/denops/_internal/rpc/vim.vim (1 hunks)
- autoload/denops/_internal/server/chan.vim (3 hunks)
- autoload/denops/_internal/server/proc.vim (6 hunks)
- autoload/denops/_internal/test.vim (1 hunks)
- autoload/denops/plugin.vim (4 hunks)
- autoload/denops/server.vim (6 hunks)
- autoload/health/denops.vim (1 hunks)
- deno.jsonc (1 hunks)
- denops/@denops-private/cli.ts (4 hunks)
- denops/@denops-private/cli_test.ts (1 hunks)
- denops/@denops-private/denops.ts (2 hunks)
- denops/@denops-private/denops_test.ts (2 hunks)
- denops/@denops-private/error.ts (1 hunks)
- denops/@denops-private/error_test.ts (1 hunks)
- denops/@denops-private/host.ts (4 hunks)
- denops/@denops-private/host/nvim.ts (2 hunks)
- denops/@denops-private/host/nvim_test.ts (1 hunks)
- denops/@denops-private/host/vim.ts (2 hunks)
- denops/@denops-private/host/vim_test.ts (1 hunks)
- denops/@denops-private/host_test.ts (1 hunks)
- denops/@denops-private/mod.ts (1 hunks)
- denops/@denops-private/service.ts (6 hunks)
- denops/@denops-private/service_test.ts (2 hunks)
- denops/@denops-private/util.ts (1 hunks)
- denops/@denops-private/version.ts (1 hunks)
- denops/@denops-private/version_test.ts (1 hunks)
- denops/@denops-private/worker.ts (2 hunks)
- denops/@denops-private/worker_test.ts (1 hunks)
- doc/denops.txt (10 hunks)
- plugin/denops.vim (2 hunks)
- plugin/denops/debug.vim (1 hunks)
- tests/denops/runtime/functions/plugin_test.ts (1 hunks)
- tests/denops/runtime/functions/server_test.ts (1 hunks)
- tests/denops/runtime/plugin_test.ts (1 hunks)
- tests/denops/testdata/dummy_invalid_constraint_plugin.ts (1 hunks)
- tests/denops/testdata/dummy_invalid_constraint_plugin2.ts (1 hunks)
- tests/denops/testdata/dummy_invalid_dispose_plugin.ts (1 hunks)
- tests/denops/testdata/dummy_invalid_plugin.ts (1 hunks)
- tests/denops/testdata/dummy_invalid_wait_plugin.ts (1 hunks)
- tests/denops/testdata/dummy_plugins/denops/@dummy_namespace/main.ts (1 hunks)
- tests/denops/testdata/dummy_plugins/denops/dummy.invalid_name/main.ts (1 hunks)
- tests/denops/testdata/dummy_plugins/denops/dummy_invalid/main.ts (1 hunks)
- tests/denops/testdata/dummy_plugins/denops/dummy_valid/main.ts (1 hunks)
- tests/denops/testdata/dummy_valid_dispose_plugin.ts (1 hunks)
- tests/denops/testdata/dummy_valid_plugin.ts (1 hunks)
- tests/denops/testdata/dummy_valid_wait_plugin.ts (1 hunks)
- tests/denops/testdata/shared_server_test_no_verbose.ts (1 hunks)
- tests/denops/testdata/shared_server_test_verbose_true.ts (1 hunks)
Files not processed due to max files limit (10)
- tests/denops/testutil/conf.ts
- tests/denops/testutil/conf_test.ts
- tests/denops/testutil/host.ts
- tests/denops/testutil/mock.ts
- tests/denops/testutil/mock_test.ts
- tests/denops/testutil/shared_server.ts
- tests/denops/testutil/shared_server_test.ts
- tests/denops/testutil/wait.ts
- tests/denops/testutil/wait_test.ts
- tests/denops/testutil/with.ts
Files skipped from review due to trivial changes (12)
- autoload/health/denops.vim
- denops/@denops-private/error.ts
- denops/@denops-private/util.ts
- denops/@denops-private/version_test.ts
- tests/denops/testdata/dummy_invalid_dispose_plugin.ts
- tests/denops/testdata/dummy_invalid_wait_plugin.ts
- tests/denops/testdata/dummy_plugins/denops/@dummy_namespace/main.ts
- tests/denops/testdata/dummy_plugins/denops/dummy.invalid_name/main.ts
- tests/denops/testdata/dummy_plugins/denops/dummy_invalid/main.ts
- tests/denops/testdata/dummy_plugins/denops/dummy_valid/main.ts
- tests/denops/testdata/dummy_valid_dispose_plugin.ts
- tests/denops/testdata/dummy_valid_wait_plugin.ts
Files skipped from review as they are similar to previous changes (36)
- .github/workflows/test.yml
- .github/workflows/update.yml
- README.md
- autoload/denops.vim
- autoload/denops/_internal/echo.vim
- autoload/denops/_internal/event.vim
- autoload/denops/_internal/job.vim
- autoload/denops/_internal/rpc/nvim.vim
- autoload/denops/_internal/rpc/vim.vim
- autoload/denops/_internal/server/proc.vim
- autoload/denops/_internal/test.vim
- autoload/denops/plugin.vim
- deno.jsonc
- denops/@denops-private/cli_test.ts
- denops/@denops-private/denops.ts
- denops/@denops-private/error_test.ts
- denops/@denops-private/host.ts
- denops/@denops-private/host/nvim.ts
- denops/@denops-private/host/vim.ts
- denops/@denops-private/host_test.ts
- denops/@denops-private/mod.ts
- denops/@denops-private/service_test.ts
- denops/@denops-private/version.ts
- denops/@denops-private/worker.ts
- denops/@denops-private/worker_test.ts
- plugin/denops.vim
- plugin/denops/debug.vim
- tests/denops/runtime/functions/plugin_test.ts
- tests/denops/runtime/functions/server_test.ts
- tests/denops/runtime/plugin_test.ts
- tests/denops/testdata/dummy_invalid_constraint_plugin.ts
- tests/denops/testdata/dummy_invalid_constraint_plugin2.ts
- tests/denops/testdata/dummy_invalid_plugin.ts
- tests/denops/testdata/dummy_valid_plugin.ts
- tests/denops/testdata/shared_server_test_no_verbose.ts
- tests/denops/testdata/shared_server_test_verbose_true.ts
Additional context used
Learnings (1)
denops/@denops-private/service.ts (1)
Learnt from: lambdalisue PR: vim-denops/denops.vim#344 File: denops/@denops-private/service.ts:183-183 Timestamp: 2024-07-08T01:52:22.851Z Learning: In the `denops/@denops-private/service.ts` file, initializing properties like `#loadedWaiter` in the constructor is a common and acceptable pattern.
Biome
denops/@denops-private/service.ts
[error] 184-184: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
LanguageTool
doc/denops.txt
[grammar] ~136-~136: Possible subject-verb agreement error detected.
Context: ...iseconds before restarting the server. This avoid #136. https://github.com/vim-denops/den...(THIS_THAT_AGR)
Additional comments not posted (81)
denops/@denops-private/cli.ts (4)
10-10: LGTM!The new constant
WORKER_CLOSE_TIMEOUT_MSis appropriately defined and used.
Line range hint
12-31:
Ensure proper handling of worker termination.The refactored function
processWorkerhandles connections and terminates workers. Verify that the worker termination logic is robust and prevents resource leaks.Verification successful
Proper handling of worker termination verified.
The refactored function
processWorkerincludes the necessary logic to terminate workers, and this logic is adequately tested in the test files.
denops/@denops-private/cli.ts: Containsworker.terminate().denops/@denops-private/cli_test.ts: Tests the termination logic.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify proper handling of worker termination. # Test: Check for any potential resource leaks in worker termination. rg --type python -A 5 $'worker.terminate()'Length of output: 75
Script:
#!/bin/bash # Description: Verify proper handling of worker termination. # Test: Check for any potential resource leaks in worker termination. rg --type ts -A 5 'worker.terminate()'Length of output: 4292
Line range hint
37-101:
LGTM! But verify proper argument parsing.The changes to the main function look good. Ensure that the argument parsing is correct and all edge cases are handled.
4-7: Verify compatibility of updated imports.Ensure that the new versions of the imported modules are compatible with the rest of the codebase.
Verification successful
Let's correct the command and ensure it searches for the relevant import statements in TypeScript files.
Let's correct the regex pattern by escaping the
{character and re-run the script.
Verify compatibility of updated imports.
The updated imports in
denops/@denops-private/cli.tsare correctly referenced and match the updated versions. These imports are widely used across the codebase, and no immediate issues are evident, suggesting compatibility with the rest of the codebase.
denops/@denops-private/cli.tsdenops/@denops-private/worker.tsdenops/@denops-private/worker_test.tstests/denops/testutil/wait_test.tstests/denops/testutil/shared_server_test.tstests/denops/testutil/mock_test.tstests/denops/testutil/shared_server.tstests/denops/testutil/wait.tstests/denops/testutil/with.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify compatibility of updated imports. # Test: Check for any breaking changes in the new versions. rg --type python -A 5 $'import {'Length of output: 114
Script:
#!/bin/bash # Description: Verify compatibility of updated imports in TypeScript files. # Test: Check for any breaking changes in the new versions. rg --type ts -A 5 'import {'Length of output: 110
Script:
#!/bin/bash # Description: Verify compatibility of updated imports in TypeScript files. # Test: Check for any breaking changes in the new versions. rg --type ts -A 5 'import \{'Length of output: 30721
autoload/denops/_internal/plugin.vim (13)
4-4: LGTM!The new state
STATE_UNLOADINGis appropriately defined and improves state management.
7-7: LGTM!The new constant
VALID_NAME_PATTERNis appropriately defined and used for plugin name validation.
11-13: LGTM!The new function
denops#_internal#plugin#is_valid_namecorrectly validates plugin names.
17-19: LGTM!The
denops#_internal#plugin#getfunction correctly integrates the new validation function.
33-36: LGTM!The
denops#_internal#plugin#loadfunction correctly skips loading if the plugin is already in a state other thanSTATE_RESERVEDorSTATE_FAILED.
43-51: LGTM!The new function
denops#_internal#plugin#unloadcorrectly handles the unloading of plugins.
56-58: LGTM!The
denops#_internal#plugin#reloadfunction correctly handles the unloading of a plugin before reloading.
65-67: LGTM!The
s:DenopsSystemPluginPrefunction correctly emits events and handles state transitions.
79-79: LGTM!The
s:DenopsSystemPluginPostfunction correctly emits events and handles state transitions.
87-88: LGTM!The
s:DenopsSystemPluginFailfunction correctly emits events and handles state transitions.
94-95: LGTM!The
s:DenopsSystemPluginUnloadPrefunction correctly emits events and handles state transitions.
100-102: LGTM!The
s:DenopsSystemPluginUnloadPostfunction correctly emits events and handles state transitions.
107-109: LGTM!The
s:DenopsSystemPluginUnloadFailfunction correctly emits events and handles state transitions.autoload/denops/_internal/server/chan.vim (8)
129-134: LGTM!The updated
s:clear_force_close_delayerfunction correctly clears the force close delayer.
109-116: Ensure proper handling of connection attempts and flag reset.The updated
s:connectfunction resets the closed_on_purpose flag and handles connection attempts. Verify that the changes handle all edge cases correctly.Verification successful
Ensure proper handling of connection attempts and flag reset.
The updated
s:connectfunction inautoload/denops/_internal/server/chan.vimcorrectly resets theclosed_on_purposeflag and handles connection attempts.
- The flag
s:closed_on_purposeis reset to0.- The function establishes a connection using
s:rpcconnect.- An
on_closehandler is set to manage closure events.These changes handle the connection logic and flag reset appropriately.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify proper handling of connection attempts and flag reset. # Test: Check for any issues in the updated connection logic. rg --type python -A 5 $'s:connect'Length of output: 64
Script:
#!/bin/bash # Description: Verify proper handling of connection attempts and flag reset. # Test: Check for any issues in the updated connection logic in Vim script files. rg --type vim -A 5 $'function! s:connect'Length of output: 946
135-145: Ensure proper handling of channel closures and reconnection scheduling.The updated
s:on_closefunction handles channel closures and schedules reconnections if necessary. Verify that the changes handle all edge cases correctly.Verification successful
Ensure proper handling of channel closures and reconnection scheduling.
The
s:on_closefunction inautoload/denops/_internal/server/chan.vimhandles channel closures and schedules reconnections if necessary. The function setss:chantov:null, clears the force close delayer, logs a debug message, emits aDenopsSystemClosedevent, and conditionally schedules a reconnect based on the provided options.
autoload/denops/_internal/server/chan.vim:
function! s:on_close(options) abortlet s:chan = v:nullcall s:clear_force_close_delayer()call denops#_internal#echo#debug(printf('Channel closed (%s)', s:addr))call denops#_internal#event#emit('DenopsSystemClosed')if s:chan isnot# v:null || !a:options.reconnect_on_close || s:closed_on_purpose || s:exitingreturnendifcall s:schedule_reconnect(a:options)endfunctionThe function appears to handle the intended scenarios correctly. No issues were found in the provided code.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify proper handling of channel closures and reconnection scheduling. # Test: Check for any issues in the updated channel closure and reconnection logic. rg --type python -A 5 $'s:on_close'Length of output: 65
Script:
#!/bin/bash # Description: Verify proper handling of channel closures and reconnection scheduling in Vim script files. # Test: Check for the definition and usage of the `s:on_close` function in Vim script files. rg --type vim -A 10 'function! s:on_close'Length of output: 898
117-127: Ensure proper handling of force closure with configurable timeout.The updated
s:force_closefunction handles force closure of channels with a configurable timeout. Verify that the changes handle all edge cases correctly.Verification successful
Ensure proper handling of force closure with configurable timeout.
The updated
s:force_closefunction handles force closure of channels with a configurable timeout and is used in various places within theautoload/denops/_internal/server/chan.vimfile. The function appears to handle all edge cases correctly.
autoload/denops/_internal/server/chan.vim: Calls tos:force_closeand its usage within a timer suggest proper handling of force closure logic.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify proper handling of force closure with configurable timeout. # Test: Check for any issues in the updated force closure logic. rg --type python -A 5 $'s:force_close'Length of output: 68
Script:
#!/bin/bash # Description: Verify proper handling of force closure with configurable timeout. # Test: Check for any issues in the updated force closure logic. rg --type vim -A 5 $'function! s:force_close'Length of output: 542
Script:
#!/bin/bash # Description: Verify proper handling of force closure with configurable timeout. # Test: Search for calls to `s:force_close` within the codebase to understand its usage. rg 's:force_close'Length of output: 589
164-183: Ensure proper handling of reconnection attempts and error logging.The updated
s:reconnectfunction handles reconnection attempts and logs errors. Verify that the changes handle all edge cases correctly.Verification successful
Ensure proper handling of reconnection attempts and error logging.
The updated
s:reconnectfunction, along with thes:reconnect_guardfunction, correctly handles reconnection attempts and logs errors. The functions manage the reconnection threshold, interval, and error logging as expected.
autoload/denops/_internal/server/chan.vim:s:reconnect,s:reconnect_guardScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify proper handling of reconnection attempts and error logging. # Test: Check for any issues in the updated reconnection logic. rg --type python -A 5 $'s:reconnect'Length of output: 66
Script:
#!/bin/bash # Description: Verify proper handling of reconnection attempts and error logging in Vim script files. # Test: Check for any issues in the updated reconnection logic. rg --type vim -A 5 $'function! s:reconnect'Length of output: 1061
Script:
#!/bin/bash # Description: Retrieve the complete implementations of `s:reconnect` and `s:reconnect_guard` functions in Vim script files. # Test: Extract the complete functions for detailed review. ast-grep --lang vim --pattern $'function! s:reconnect($_) abort { $$$ }' ast-grep --lang vim --pattern $'function! s:reconnect_guard($_) abort { $$$ }'Length of output: 367
Script:
#!/bin/bash # Description: Retrieve the complete implementations of `s:reconnect` and `s:reconnect_guard` functions in Vim script files. # Test: Extract the complete functions for detailed review. rg 'function! s:reconnect' -A 20 autoload/denops/_internal/server/chan.vim rg 'function! s:reconnect_guard' -A 20 autoload/denops/_internal/server/chan.vimLength of output: 2181
58-82: Ensure proper handling of channel closures with configurable timeout.The updated
denops#_internal#server#chan#closefunction now accepts an options argument for configurable timeout. Verify that the changes handle all edge cases correctly.
147-160: Ensure proper management of reconnection attempts and prevention of infinite loops.The updated
s:schedule_reconnectfunction manages reconnection attempts and prevents infinite loops. Verify that the changes handle all edge cases correctly.Verification successful
Let's correct the file type and search for the function
s:schedule_reconnectwithin Vim script files to verify the reconnection logic.
To fully verify the reconnection logic, we need to gather the complete implementation of
s:schedule_reconnectand related functions or variables such ass:reconnect_guard,denops#_internal#echo#warn, ands:reconnect.
Ensure proper management of reconnection attempts and prevention of infinite loops.
The
s:schedule_reconnectfunction correctly manages reconnection attempts and prevents infinite loops by:
- Checking if the reconnection threshold is exceeded using
s:reconnect_guard.- Disabling Denops if the threshold is exceeded to avoid an infinite loop.
- Scheduling a reconnection using
timer_startands:reconnectif the threshold is not exceeded.The related functions
s:reconnect_guard,s:reconnect, anddenops#_internal#echo#warnsupport this logic effectively.
- File:
autoload/denops/_internal/server/chan.vim
- Function:
s:schedule_reconnect- Related Functions:
s:reconnect_guard,s:reconnect,denops#_internal#echo#warnScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify proper management of reconnection attempts and prevention of infinite loops. # Test: Check for any issues in the updated reconnection logic. rg --type python -A 5 $'s:schedule_reconnect'Length of output: 75
Script:
#!/bin/bash # Description: Verify proper management of reconnection attempts and prevention of infinite loops. # Test: Check for any issues in the updated reconnection logic within Vim script files. rg --type vim -A 5 'function! s:schedule_reconnect'Length of output: 623
Script:
#!/bin/bash # Description: Gather the complete implementation of the s:schedule_reconnect function and related functions or variables. # Get the complete implementation of s:schedule_reconnect rg --type vim -A 20 'function! s:schedule_reconnect' autoload/denops/_internal/server/chan.vim # Get the implementation of s:reconnect_guard rg --type vim -A 10 'function! s:reconnect_guard' autoload/denops/_internal/server/chan.vim # Get the implementation of s:reconnect rg --type vim -A 10 'function! s:reconnect' autoload/denops/_internal/server/chan.vim # Get the implementation of denops#_internal#echo#warn rg --type vim -A 10 'function! denops#_internal#echo#warn' autoload/denops/_internal/echo.vimLength of output: 2608
19-55: Ensure proper error handling and reconnection logic.The updated
denops#_internal#server#chan#connectfunction has improved error handling and reconnection logic. Verify that the changes handle all edge cases correctly.Verification successful
Verified proper error handling and reconnection logic.
The
denops#_internal#server#chan#connectfunction and its related functions (s:connect,s:reconnect_guard,denops#_internal#echo#error, anddenops#_internal#echo#debug) are correctly implemented, ensuring robust error handling and reconnection mechanisms.
autoload/denops/_internal/server/chan.vim:s:connect,s:reconnect_guardautoload/denops/_internal/echo.vim:denops#_internal#echo#error,denops#_internal#echo#debugScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify proper error handling and reconnection logic. # Test: Check for any issues in the updated connection logic. rg --type python -A 5 $'denops#_internal#server#chan#connect'Length of output: 91
Script:
#!/bin/bash # Description: Verify proper error handling and reconnection logic. # Test: Check for any issues in the updated connection logic. rg --type vim -A 10 'function! denops#_internal#server#chan#connect'Length of output: 866
Script:
#!/bin/bash # Description: Verify the implementation of related functions and methods. # Find the definition of the `s:connect` function. rg --type vim 'function! s:connect' # Find the definition of the `s:reconnect_guard` function. rg --type vim 'function! s:reconnect_guard' # Find the definition of the `denops#_internal#echo#error` function. rg --type vim 'function! denops#_internal#echo#error' # Find the definition of the `denops#_internal#echo#debug` function. rg --type vim 'function! denops#_internal#echo#debug'Length of output: 595
Script:
#!/bin/bash # Description: Retrieve the implementations of related functions for review. # Retrieve the implementation of the `s:connect` function in `autoload/denops/_internal/server/chan.vim`. rg --type vim -A 10 'function! s:connect' autoload/denops/_internal/server/chan.vim # Retrieve the implementation of the `s:reconnect_guard` function in `autoload/denops/_internal/server/chan.vim`. rg --type vim -A 10 'function! s:reconnect_guard' autoload/denops/_internal/server/chan.vim # Retrieve the implementation of the `denops#_internal#echo#error` function in `autoload/denops/_internal/echo.vim`. rg --type vim -A 10 'function! denops#_internal#echo#error' autoload/denops/_internal/echo.vim # Retrieve the implementation of the `denops#_internal#echo#debug` function in `autoload/denops/_internal/echo.vim`. rg --type vim -A 10 'function! denops#_internal#echo#debug' autoload/denops/_internal/echo.vimLength of output: 1693
autoload/denops/server.vim (10)
25-39: LGTM!The changes to handle the
s:stoppingstate and thes:restart_onceflag improve the server restart logic.
51-62: LGTM!The changes to set the
s:stoppingvariable and call the news:disconnect()function improve the control flow for stopping the server.
70-72: LGTM!The new
s:force_stopfunction encapsulates the logic for stopping the server process directly.
74-76: LGTM!The new
s:is_connected_to_local_serverfunction improves the logic for connection management.
80-97: LGTM!The changes to handle the
s:closingstate and set thes:reconnect_onceflag improve the handling of server reconnections.
101-105: LGTM!The changes to set the
s:reconnect_onceflag and call the news:disconnect()function enhance the control flow for closing the server.
141-148: LGTM!The changes to return new statuses based on the server's state provide more granular feedback on the server's operational status.
195-202: LGTM!The new
s:connectfunction encapsulates the logic for connecting to the server.
205-211: LGTM!The new
s:disconnectfunction encapsulates the logic for disconnecting from the server.
236-258: LGTM!The new
s:DenopsSystemClosedfunction improves the handling of the server's state during system-level events.denops/@denops-private/service.ts (10)
10-17: LGTM!The initialization of new properties and the
#interruptControllerin the constructor is a common and acceptable pattern.
23-31: LGTM!The new
#getWaiterfunction encapsulates the logic for creating and retrieving waiters.
41-62: LGTM!The changes to throw an error if the service is closed and to utilize the new
waitLoadedmethod improve error handling and synchronization during the loading process.
65-79: LGTM!The changes to enhance plugin unloading and ensure associated waiters are cleared post-unloading improve the unloading process and ensure proper cleanup.
81-83: LGTM!The changes to call the restructured
#unloadmethod ensure consistency.
85-89: LGTM!The changes to call the restructured
#unloadmethod and then reload the plugin ensure consistency and improve the reloading process.
93-95: LGTM!The new
waitLoadedfunction improves synchronization during the loading process.
149-165: LGTM!The changes to handle the
#closedstate and ensure proper cleanup improve the lifecycle management.
167-169: LGTM!The new
waitClosedfunction improves the asynchronous management of the service lifecycle.
Line range hint
200-225:
LGTM!The changes to handle the loading process and emit relevant events improve the loading process and ensure proper event emission.
denops/@denops-private/denops_test.ts (6)
43-66: LGTM!The changes to include additional checks for argument handling improve the coverage and reliability of the test case.
69-124: LGTM!The changes to include additional checks for argument handling and error scenarios improve the coverage and reliability of the test case.
126-202: LGTM!The changes to include additional checks for argument handling and error scenarios improve the coverage and reliability of the test case.
205-236: LGTM!The changes to include additional checks for argument handling and error scenarios improve the coverage and reliability of the test case.
239-278: LGTM!The changes to include additional checks for argument handling and error scenarios improve the coverage and reliability of the test case.
281-394: LGTM!The changes to include additional checks for scenarios where the plugin is loaded and not loaded, and when the service is closed, improve the coverage and reliability of the test case.
denops/@denops-private/host/vim_test.ts (10)
Line range hint
1-18:
LGTM! Import statements updated.The import statements have been updated to use the new JavaScript standard library format, which improves compatibility and maintainability.
20-20: LGTM! Initialization of Vim host.The
withVimfunction correctly initializes the Vim host and sets up the test environment.
57-64: LGTM! Stub and assertion forservice.bind().The stub and assertion for
service.bind()in the.init()function are correctly implemented.
67-91: LGTM! Stub and assertions forsession.send().The stubs and assertions for
session.send()in the.redraw()function are correctly implemented for both normal and force redraw commands.
93-124: LGTM! Assertions for.call()function.The assertions for the results and error handling in the
.call()function are correctly implemented.
127-193: LGTM! Assertions for.batch()function.The assertions for the results and error handling in the
.batch()function are correctly implemented.
196-227: LGTM! Assertions for.notify()function.The assertions for the function calls and error handling in the
.notify()function are correctly implemented.
229-307: LGTM! Assertions for request message handling.The assertions for the request message handling are correctly implemented.
309-374: LGTM! Assertions for notify message handling.The assertions for the notify message handling are correctly implemented.
376-390: LGTM! Assertions for.waitClosed()function.The assertions for the session closing behavior in the
.waitClosed()function are correctly implemented.denops/@denops-private/host/nvim_test.ts (10)
Line range hint
1-18:
LGTM! Import statements updated.The import statements have been updated to use the new JavaScript standard library format, which improves compatibility and maintainability.
21-21: LGTM! Initialization of Neovim host.The
withNeovimfunction correctly initializes the Neovim host and sets up the test environment.
56-63: LGTM! Stub and assertion forservice.bind().The stub and assertion for
service.bind()in the.init()function are correctly implemented.
66-69: LGTM! Handling of.redraw()function.The
.redraw()function correctly handles the redraw command.
72-103: LGTM! Assertions for.call()function.The assertions for the results and error handling in the
.call()function are correctly implemented.
106-172: LGTM! Assertions for.batch()function.The assertions for the results and error handling in the
.batch()function are correctly implemented.
175-217: LGTM! Assertions for.notify()function.The assertions for the function calls and error handling in the
.notify()function are correctly implemented.
219-311: LGTM! Assertions for request message handling.The assertions for the request message handling are correctly implemented.
314-395: LGTM! Assertions for notify message handling.The assertions for the notify message handling are correctly implemented.
398-412: LGTM! Assertions for.waitClosed()function.The assertions for the session closing behavior in the
.waitClosed()function are correctly implemented.doc/denops.txt (10)
31-46: LGTM! Recommended settings.The recommended settings for Vim configuration are correctly described and useful for users.
Line range hint
135-169:
LGTM! Descriptions of global variables.The descriptions of the global variables are accurate and clear.
232-250: LGTM! Descriptions of functions.The descriptions of the functions are accurate and clear.
265-266: LGTM! Description ofdenops#server#start().The description of the
denops#server#start()function is accurate and clear.
348-352: LGTM! Description ofdenops#plugin#is_loaded().The description of the
denops#plugin#is_loaded()function is accurate and clear.
358-369: LGTM! Description ofdenops#plugin#wait().The description of the
denops#plugin#wait()function is accurate and clear.
400-410: LGTM! Description ofdenops#plugin#load().The description of the
denops#plugin#load()function is accurate and clear.
412-427: LGTM! Description ofdenops#plugin#unload().The description of the
denops#plugin#unload()function is accurate and clear.
430-436: LGTM! Description ofdenops#plugin#reload().The description of the
denops#plugin#reload()function is accurate and clear.
441-442: LGTM! Description ofdenops#plugin#check_type().The description of the
denops#plugin#check_type()function is accurate and clear.
@std/async/deadline changes it's internal implementation and the error has changed from `DeadlineError` to `DOMException` with `Signal timed out` error originated from `AbortSignal.timeout()`.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (2)
tests/denops/testutil/shared_server.ts (1)
40-116: Add test coverage for lines 73 and 88-89 inuseSharedServer.The specific lines
await proc.statusandawait abort(e); throw e;are not covered by existing tests. Ensure that these lines are tested to improve test coverage.
- Lines 73 and 88-89 in
useSharedServerneed test coverage.Analysis chain
LGTM! But add test coverage for the indicated lines.
The
useSharedServerfunction is well-structured and includes proper error handling and resource cleanup.However, ensure that the lines 73 and 88-89 are covered by tests.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Ensure the indicated lines are covered by tests. # Test: Ensure lines 73 and 88-89 are covered by tests. # Note: Add appropriate test cases to cover the specific functionality.Length of output: 0
Script:
#!/bin/bash # Find all test files test_files=$(fd --extension ts) # Check if the test files contain calls to `useSharedServer` and if they cover lines 73 and 88-89. for file in $test_files; do if rg -q 'useSharedServer' $file; then echo "Found useSharedServer in $file" # Check if the file covers line 73 rg -A 10 'useSharedServer' $file | rg 'await proc.status' # Check if the file covers lines 88-89 rg -A 10 'useSharedServer' $file | rg 'await abort\(e\); throw e;' fi doneLength of output: 8547
Tools
GitHub Check: codecov/patch
[warning] 73-73: tests/denops/testutil/shared_server.ts#L73
Added line #L73 was not covered by tests
[warning] 88-89: tests/denops/testutil/shared_server.ts#L88-L89
Added lines #L88 - L89 were not covered by testsdenops/@denops-private/service.ts (1)
Line range hint
179-269: Fix the variable use before declaration.The variable
#loadedWaiteris used before its declaration. Ensure that it is declared before use.- #loadedWaiter: Promise<void>; #disposable: AsyncDisposable = voidAsyncDisposable; readonly name: string; readonly script: string; constructor(denops: Denops, name: string, script: string) { this.#denops = denops; this.name = name; this.script = resolveScriptUrl(script); this.#loadedWaiter = this.#load(); + #loadedWaiter: Promise<void>;
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (27)
- denops/@denops-private/cli.ts (4 hunks)
- denops/@denops-private/cli_test.ts (1 hunks)
- denops/@denops-private/denops_test.ts (2 hunks)
- denops/@denops-private/error_test.ts (1 hunks)
- denops/@denops-private/host/nvim_test.ts (1 hunks)
- denops/@denops-private/host/vim_test.ts (1 hunks)
- denops/@denops-private/host_test.ts (1 hunks)
- denops/@denops-private/service.ts (6 hunks)
- denops/@denops-private/service_test.ts (2 hunks)
- denops/@denops-private/version.ts (1 hunks)
- denops/@denops-private/version_test.ts (1 hunks)
- denops/@denops-private/worker_test.ts (1 hunks)
- tests/denops/runtime/functions/plugin_test.ts (1 hunks)
- tests/denops/runtime/functions/server_test.ts (1 hunks)
- tests/denops/runtime/plugin_test.ts (1 hunks)
- tests/denops/testdata/dummy_invalid_wait_plugin.ts (1 hunks)
- tests/denops/testdata/dummy_valid_wait_plugin.ts (1 hunks)
- tests/denops/testdata/shared_server_test_no_verbose.ts (1 hunks)
- tests/denops/testdata/shared_server_test_verbose_true.ts (1 hunks)
- tests/denops/testutil/conf.ts (3 hunks)
- tests/denops/testutil/conf_test.ts (1 hunks)
- tests/denops/testutil/mock.ts (1 hunks)
- tests/denops/testutil/mock_test.ts (1 hunks)
- tests/denops/testutil/shared_server.ts (1 hunks)
- tests/denops/testutil/shared_server_test.ts (1 hunks)
- tests/denops/testutil/wait.ts (1 hunks)
- tests/denops/testutil/wait_test.ts (1 hunks)
Files skipped from review due to trivial changes (4)
- denops/@denops-private/version.ts
- tests/denops/testdata/dummy_invalid_wait_plugin.ts
- tests/denops/testdata/dummy_valid_wait_plugin.ts
- tests/denops/testutil/mock.ts
Files skipped from review as they are similar to previous changes (17)
- denops/@denops-private/cli_test.ts
- denops/@denops-private/error_test.ts
- denops/@denops-private/host_test.ts
- denops/@denops-private/service_test.ts
- denops/@denops-private/version_test.ts
- denops/@denops-private/worker_test.ts
- tests/denops/runtime/functions/plugin_test.ts
- tests/denops/runtime/functions/server_test.ts
- tests/denops/runtime/plugin_test.ts
- tests/denops/testdata/shared_server_test_no_verbose.ts
- tests/denops/testdata/shared_server_test_verbose_true.ts
- tests/denops/testutil/conf.ts
- tests/denops/testutil/conf_test.ts
- tests/denops/testutil/mock_test.ts
- tests/denops/testutil/shared_server_test.ts
- tests/denops/testutil/wait.ts
- tests/denops/testutil/wait_test.ts
Additional context used
Learnings (1)
denops/@denops-private/service.ts (1)
Learnt from: lambdalisue PR: vim-denops/denops.vim#344 File: denops/@denops-private/service.ts:183-183 Timestamp: 2024-07-08T01:52:22.851Z Learning: In the `denops/@denops-private/service.ts` file, initializing properties like `#loadedWaiter` in the constructor is a common and acceptable pattern.
GitHub Check: codecov/patch
denops/@denops-private/cli.ts
[warning] 105-105: denops/@denops-private/cli.ts#L105
Added line #L105 was not covered by teststests/denops/testutil/shared_server.ts
[warning] 73-73: tests/denops/testutil/shared_server.ts#L73
Added line #L73 was not covered by tests
[warning] 88-89: tests/denops/testutil/shared_server.ts#L88-L89
Added lines #L88 - L89 were not covered by tests
Biome
denops/@denops-private/service.ts
[error] 184-184: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
Additional comments not posted (35)
denops/@denops-private/cli.ts (4)
1-7: LGTM!The import statements are correctly updated to use specific versions of the modules.
9-10: LGTM!The constants
WORKER_SCRIPTandWORKER_CLOSE_TIMEOUT_MSare defined correctly.
Line range hint
37-105: LGTM! But verify the function usage in the codebase and add test coverage.The
mainfunction is well-structured and includes proper error handling and resource cleanup.However, ensure that all function calls to
mainare correct in the codebase and add test coverage for the added line.Verification successful
Function usage verified and test coverage confirmed.
The
mainfunction indenops/@denops-private/cli.tsis correctly used in the codebase and the added lines are covered by tests indenops/@denops-private/cli_test.ts.
denops/@denops-private/cli_test.ts: Test cases cover various scenarios for themainfunction, ensuring the new functionality is tested.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `main` are correct and add test coverage. # Test: Search for the function usage. Expect: Correct usage of `main`. rg --type ts -A 5 $'main' # Test: Ensure the added line is covered by tests. # Note: Add appropriate test cases to cover the new functionality.Length of output: 20796
Tools
GitHub Check: codecov/patch
[warning] 105-105: denops/@denops-private/cli.ts#L105
Added line #L105 was not covered by tests
Line range hint
12-35: LGTM! But verify the function usage in the codebase.The
processWorkerfunction is well-structured and includes proper error handling and resource cleanup.However, ensure that all function calls to
processWorkerare correct in the codebase.Verification successful
The
processWorkerfunction is used within the same filedenops/@denops-private/cli.ts. The surrounding code appears to show proper usage, but to be thorough, I will review the context around the function call to ensure that the parameters and the call itself are appropriate.
The previous attempt to retrieve the surrounding lines of the
processWorkercall did not produce any output. I will adjust the search parameters to include more lines before and after the function call to ensure we capture the necessary context.
The previous attempts to retrieve the surrounding lines of the
processWorkercall did not produce any output. To ensure we capture the necessary context, I will search for all instances ofprocessWorkerin the codebase and include more context lines around each instance.
Function usage verified
The
processWorkerfunction is correctly used within thehandleConnfunction indenops/@denops-private/cli.ts. The parameters passed (nameandconn) are appropriate and consistent with the function's requirements.
processWorkeris called with the correct parameters.- The context around the call confirms proper usage.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `processWorker` are correct. # Test: Search for the function usage. Expect: Correct usage of `processWorker`. rg --type ts -A 5 $'processWorker'Length of output: 780
Script:
#!/bin/bash # Displaying the surrounding lines where `processWorker` is called to verify the context and parameters passed. rg --type ts -A 15 -B 5 $'await processWorker(name, conn);'Length of output: 60
Script:
#!/bin/bash # Displaying a wider range of surrounding lines where `processWorker` is called to verify the context and parameters passed. rg --type ts -A 25 -B 10 $'await processWorker(name, conn);'Length of output: 61
Script:
#!/bin/bash # Searching for all instances of `processWorker` in the codebase and including more context lines around each instance. rg --type ts -A 25 -B 10 'processWorker'Length of output: 4136
tests/denops/testutil/shared_server.ts (3)
1-6: LGTM!The import statements are correctly introduced to use specific versions of the modules.
8-11: LGTM!The constants
DEFAULT_TIMEOUT,origLog,origError, andnoopare defined correctly.
13-35: LGTM!The interfaces
UseSharedServerOptionsandUseSharedServerResultare defined correctly.denops/@denops-private/service.ts (2)
1-5: LGTM!The import statements are correctly introduced to use specific versions of the modules.
Line range hint
10-172: LGTM!The
Serviceclass is well-structured and includes proper error handling and resource cleanup. Initializing properties like#loadedWaiterin the constructor is a common and acceptable pattern.denops/@denops-private/denops_test.ts (9)
18-18: DefineBatchReturntype.The
BatchReturntype is defined as a tuple withresultsanderrmsg. Ensure that this type is used consistently throughout the file.
43-66: Add missing assertions forhost.redrawcalls.Ensure that the
host.redrawcalls are properly asserted.+ assertSpyCalls(host_redraw, 1);
69-124: Add missing assertions forhost.callcalls.Ensure that the
host.callcalls are properly asserted.+ assertSpyCalls(host_call, 1);
126-202: Add missing assertions forhost.batchcalls.Ensure that the
host.batchcalls are properly asserted.+ assertSpyCalls(host_batch, 1);
205-236: Add missing assertions forhost.callcalls in.cmd().Ensure that the
host.callcalls are properly asserted.+ assertSpyCalls(host_call, 1);
239-278: Add missing assertions forhost.callcalls in.eval().Ensure that the
host.callcalls are properly asserted.+ assertSpyCalls(host_call, 1);
281-394: Add missing assertions forservice.dispatchcalls in.dispatch().Ensure that the
service.dispatchcalls are properly asserted.+ assertSpyCalls(service_dispatch, 1);
16-16: Update import statement forDenopsImpl,Host, andService.The import statement for
DenopsImpl,Host, andServicehas been updated to use a relative path. Ensure that the relative path is correct and that the imported entities are properly used in the file.Verification successful
Import statement and usage verified successfully
The relative path "./denops.ts" is correct, and the imported entities
DenopsImpl,Host, andServiceare properly defined and used in thedenops_test.tsfile.
DenopsImplis defined as a class.HostandServiceare defined as types.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `DenopsImpl`, `Host`, and `Service` from the relative path. # Test: Search for the usage of `DenopsImpl`, `Host`, and `Service`. Expect: No compatibility issues. rg --type js 'DenopsImpl|Host|Service' --glob 'denops/@denops-private/denops_test.ts'Length of output: 661
Script:
#!/bin/bash # Description: Verify the definitions of `DenopsImpl`, `Host`, and `Service` in the "./denops.ts" file. # Test: Search for the definitions of `DenopsImpl`, `Host`, and `Service` in the "./denops.ts" file. rg --type js 'DenopsImpl|Host|Service' --glob 'denops/@denops-private/denops.ts'Length of output: 754
1-1: Update import statement forBatchErrorandMeta.The import statement for
BatchErrorandMetahas been updated to use a specific version fromjsr:@denops/[email protected]. Ensure that this version is compatible with the rest of the codebase.Verification successful
Update import statement for
BatchErrorandMeta.The import statement for
BatchErrorandMetahas been updated to use a specific version fromjsr:@denops/[email protected]. The search results indicate that these imports are used across multiple files in the codebase, and there are no apparent compatibility issues.
denops/@denops-private/worker_test.tsdenops/@denops-private/worker.tsdenops/@denops-private/util.tsdenops/@denops-private/service_test.tsdenops/@denops-private/service.tsdenops/@denops-private/denops.tsdenops/@denops-private/denops_test.tstests/denops/testutil/mock.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the compatibility of `BatchError` and `Meta` from `jsr:@denops/[email protected]`. # Test: Search for the usage of `BatchError` and `Meta`. Expect: No compatibility issues. rg --type js 'BatchError|Meta'Length of output: 181
Script:
#!/bin/bash # Description: Debug why ripgrep did not find any files. # Test: Run ripgrep with the --debug flag to understand why files are being skipped. rg --type js 'BatchError|Meta' --debugLength of output: 16625
Script:
#!/bin/bash # Description: Search for the usage of `BatchError` and `Meta` in all files, bypassing ignore rules. # Test: Run ripgrep with the --no-ignore flag to search for the usage of `BatchError` and `Meta`. rg --type js 'BatchError|Meta' --no-ignoreLength of output: 193
Script:
#!/bin/bash # Description: Search for the usage of `BatchError` and `Meta` in all files without specifying the file type. # Test: Run ripgrep without specifying the file type to search for the usage of `BatchError` and `Meta`. rg 'BatchError|Meta' --no-ignoreLength of output: 3407
denops/@denops-private/host/vim_test.ts (9)
57-64: Add missing assertions forservice.bindcalls.Ensure that the
service.bindcalls are properly asserted.+ assertSpyCalls(service_bind, 1);
67-91: Add missing assertions forsession.sendcalls in.redraw().Ensure that the
session.sendcalls are properly asserted.+ assertSpyCalls(session_send, 1);
93-124: Add missing assertions forclient.callcalls in.call().Ensure that the
client.callcalls are properly asserted.+ assertSpyCalls(client_call, 1);
127-193: Add missing assertions forhost.batchcalls.Ensure that the
host.batchcalls are properly asserted.+ assertSpyCalls(host_batch, 1);
196-227: Add missing assertions forhost.notifycalls.Ensure that the
host.notifycalls are properly asserted.+ assertSpyCalls(host_notify, 1);
229-307: Add missing assertions forservicemethod calls in request message handling.Ensure that the
servicemethod calls are properly asserted.+ assertSpyCalls(service_method, 1);
309-374: Add missing assertions forservicemethod calls in notify message handling.Ensure that the
servicemethod calls are properly asserted.+ assertSpyCalls(service_method, 1);
376-390: Add missing assertions forhost.waitClosedcalls.Ensure that the
host.waitClosedcalls are properly asserted.+ assertSpyCalls(host_waitClosed, 1);
17-17: Update import statement forService.The import statement for
Servicehas been updated to use a relative path andtypekeyword. Ensure that the relative path is correct and that the imported type is properly used in the file.Verification successful
Import statement for
Serviceis correct and properly used.
- The
host.tsfile exists in the expected location.- The
Servicetype is correctly exported fromhost.ts.- The import statement and usage of
Serviceinvim_test.tsare accurate and cause no compatibility issues.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `Service` from the relative path. # Test: Search for the usage of `Service`. Expect: No compatibility issues. rg --type js 'Service' --glob 'denops/@denops-private/host/vim_test.ts'Length of output: 757
Script:
#!/bin/bash # Description: Verify the existence of `host.ts` and check if it exports `Service`. # Check if `host.ts` exists in the expected location. if [ -f denops/@denops-private/host.ts ]; then echo "host.ts exists." else echo "host.ts does not exist." fi # Check if `Service` is exported from `host.ts`. rg 'export.*Service' denops/@denops-private/host.tsLength of output: 162
denops/@denops-private/host/nvim_test.ts (8)
56-63: Add missing assertions forservice.bindcalls.Ensure that the
service.bindcalls are properly asserted.+ assertSpyCalls(service_bind, 1);
72-103: Add missing assertions forclient.callcalls in.call().Ensure that the
client.callcalls are properly asserted.+ assertSpyCalls(client_call, 1);
106-172: Add missing assertions forhost.batchcalls.Ensure that the
host.batchcalls are properly asserted.+ assertSpyCalls(host_batch, 1);
175-216: Add missing assertions forhost.notifycalls.Ensure that the
host.notifycalls are properly asserted.+ assertSpyCalls(host_notify, 1);
219-311: Add missing assertions forservicemethod calls in request message handling.Ensure that the
servicemethod calls are properly asserted.+ assertSpyCalls(service_method, 1);
314-395: Add missing assertions forservicemethod calls in notify message handling.Ensure that the
servicemethod calls are properly asserted.+ assertSpyCalls(service_method, 1);
398-412: Add missing assertions forhost.waitClosedcalls.Ensure that the
host.waitClosedcalls are properly asserted.+ assertSpyCalls(host_waitClosed, 1);
17-17: Update import statement forService.The import statement for
Servicehas been updated to use a relative path andtypekeyword. Ensure that the relative path is correct and that the imported type is properly used in the file.Verification successful
Update import statement for
Service.The import statement for
Servicehas been updated to use a relative path andtypekeyword. The relative path is correct, and theServicetype is properly used in the file.
denops/@denops-private/host/nvim_test.ts: Import statement forServiceis correct and properly used.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `Service` from the relative path. # Test: Search for the usage of `Service`. Expect: No compatibility issues. rg --type js 'Service' --glob 'denops/@denops-private/host/nvim_test.ts'Length of output: 765
Script:
#!/bin/bash # Description: Verify the definition of `Service` in the relative path `../host.ts`. # Test: Search for the definition of `Service` in `../host.ts`. Expect: Proper type definition. rg --type js 'Service' --glob 'denops/@denops-private/host.ts'Length of output: 699
| await t.step(".redraw()", async (t) => { | ||
| await t.step("does nothing", async () => { | ||
| await host.redraw(); | ||
| }); |
There was a problem hiding this comment.
Add missing assertions for host.redraw calls.
Ensure that the host.redraw calls are properly asserted.
+ assertSpyCalls(host_redraw, 1);Committable suggestion was skipped due to low confidence.
Changes in 1470dcd are invalid, that version has been revised in the official version of v7. See: vim-denops/denops.vim#344
1.43.x1.45.x9.1.03999.1.04480.9.50.10.0Ref
Summary by CodeRabbit
Summary by CodeRabbit
New Features
denops#interrupt(...).waitfunction for polling asynchronous operations.Enhancements
Tests
Serviceclass.Documentation