Denops v6 #302
Denops v6 #302
Conversation
|
Warning Rate Limit Exceeded@lambdalisue has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 14 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe project underwent a significant update, primarily focused on improving compatibility with newer versions of Deno, Vim, and Neovim. Changes include updating version constants, refining testing workflows, and enhancing error handling. Autoload functions and internal logic were refactored for efficiency, and the plugin system was restructured to better manage communication and task execution. All updates aim to streamline the development process and ensure smoother integration with the updated software versions. Changes
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 Configration File (
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #302 +/- ##
==========================================
- Coverage 96.47% 89.66% -6.82%
==========================================
Files 3 9 +6
Lines 85 648 +563
Branches 6 61 +55
==========================================
+ Hits 82 581 +499
- Misses 3 67 +64 ☔ View full report in Codecov by Sentry. |
|
I have tested it in Windows environment. |
lambdalisue
force-pushed
the
v6-pre
branch
12 times, most recently
from
1357251
to
8dc5e89
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (31)
- .github/workflows/test.yml (2 hunks)
- README.md (2 hunks)
- autoload/denops.vim (1 hunks)
- autoload/denops/_internal/echo.vim (2 hunks)
- autoload/denops/_internal/test.vim (1 hunks)
- autoload/denops/plugin.vim (1 hunks)
- autoload/health/denops.vim (1 hunks)
- deno.jsonc (1 hunks)
- denops/@denops-private/cli.ts (1 hunks)
- denops/@denops-private/denops.ts (2 hunks)
- denops/@denops-private/denops_test.ts (1 hunks)
- denops/@denops-private/error.ts (1 hunks)
- denops/@denops-private/error_test.ts (1 hunks)
- denops/@denops-private/host.ts (1 hunks)
- denops/@denops-private/host/nvim.ts (4 hunks)
- denops/@denops-private/host/nvim_test.ts (1 hunks)
- denops/@denops-private/host/vim.ts (4 hunks)
- denops/@denops-private/host/vim_test.ts (1 hunks)
- denops/@denops-private/host_test.ts (1 hunks)
- denops/@denops-private/service.ts (2 hunks)
- denops/@denops-private/service_test.ts (1 hunks)
- denops/@denops-private/testdata/dummy_invalid_plugin.ts (1 hunks)
- denops/@denops-private/testdata/dummy_valid_plugin.ts (1 hunks)
- denops/@denops-private/testutil/cli.ts (1 hunks)
- denops/@denops-private/testutil/conf.ts (1 hunks)
- denops/@denops-private/testutil/conf_test.ts (1 hunks)
- denops/@denops-private/testutil/with.ts (1 hunks)
- denops/@denops-private/util.ts (1 hunks)
- denops/@denops-private/version.ts (1 hunks)
- denops/@denops-private/worker.ts (1 hunks)
- plugin/denops.vim (2 hunks)
Files skipped from review due to trivial changes (1)
- autoload/health/denops.vim
Additional comments: 77
denops/@denops-private/testdata/dummy_invalid_plugin.ts (1)
- 1-5: The code change introduces a dummy plugin that throws an error when executed. This is likely used for testing error handling within the Denops framework.
denops/@denops-private/testdata/dummy_valid_plugin.ts (1)
- 1-10: The code change introduces a valid dummy plugin for testing purposes. It defines a dispatcher with a test function that echoes a message, ensuring the plugin's basic functionality can be verified.
denops/@denops-private/util.ts (2)
- 1-2: Updated import versions for
Meta,is, andPredicateto ensure compatibility with the latest module versions.- 1-1: Ensure that the updated version of
denops_core(v6.0.5) is compatible with the rest of the codebase and that no breaking changes affect the functionality.deno.jsonc (1)
- 4-8: The tasks in
deno.jsonchave been updated to reflect changes in the Deno CLI options and to introduce a new task for test coverage. Ensure that the removal of the--unstableflag does not affect any features that were previously relying on it.denops/@denops-private/testutil/cli.ts (1)
- 1-22: The
mainfunction incli.tshas been refactored to use a newADDR_ENV_NAMEconstant and to handle connections to the Denops server. Ensure that the environment variable is correctly set and used across the codebase.denops/@denops-private/error.ts (1)
- 1-7: The import of the
unknownutilmodule has been updated to versionv3.13.0, and new functions from theerrorutilmodule have been introduced. Ensure that these changes are compatible with the existing error handling logic.denops/@denops-private/version.ts (1)
- 9-23: The
getVersionOrfunction has been updated to return aSemVerobject or a fallback value asynchronously. Ensure that the error handling and command execution logic are correctly implemented to accommodate this change.autoload/denops/_internal/test.vim (1)
- 1-17: The code introduces functions for testing purposes that wrap the
denops#_internal#rpc#nvim#notifyanddenops#_internal#rpc#nvim#requestfunctions for Neovim and their Vim counterparts. Ensure that these functions are used correctly in the test environment and do not interfere with the production code.autoload/denops/_internal/echo.vim (2)
- 8-10: Added a new function
denops#_internal#echo#logfor logging purposes. Ensure that this function is used consistently across the codebase for logging messages.- 20-20: Modified the
denops#_internal#echo#infofunction to calls:echomsg('Title', a:000)instead ofs:echomsg('None', a:000). Verify that this change is reflected in the user interface where informational messages are displayed.plugin/denops.vim (1)
- 6-8: The version check requirements for Vim and Neovim have been updated. Ensure that these new version requirements are communicated to users and that they do not cause issues for users on older versions who may not be able to update.
autoload/denops.vim (1)
- 19-25: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-22]
The
denops#request_asyncfunction has been modified to usedenops#_internal#server#chan#notifyinstead ofdenops#_internal#server#chan#request. This change affects the control flow and likely the handling of responses. Ensure that all asynchronous requests are still handled correctly after this change.denops/@denops-private/testutil/conf.ts (1)
- 1-45: The
getConfigfunction has been updated to handle environment variables and default values for the Denops path and Vim/Neovim executables. Ensure that these changes do not affect the expected behavior of the test environment.denops/@denops-private/cli.ts (1)
- 1-72: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-92]
The
cli.tsfile has been refactored to use version ranges for imports and to handle connections with a worker-based approach. Ensure that the worker script URL is correctly resolved and that the worker-based connection handling is robust and error-free.denops/@denops-private/denops.ts (1)
- 1-57: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-98]
Significant modifications have been made to the
DenopsImplclass, including changes to the import statements, the structure of classes, and the way methods are called. New type definitions forHostandServicehave been introduced. Ensure that these changes are consistent with the overall architecture and design principles of the Denops framework.denops/@denops-private/testutil/conf_test.ts (1)
- 1-76: The tests for the
removeTrailingSepfunction have been updated to handle different operating systems. Ensure that these tests cover all edge cases and that the function behaves correctly on all supported platforms.denops/@denops-private/error_test.ts (1)
- 1-77: Tests for the
errorSerializeranderrorDeserializerfunctions have been added. Ensure that these tests adequately cover the serialization and deserialization logic and that any edge cases are handled correctly..github/workflows/test.yml (2)
- 56-60: The Deno and Vim versions have been updated in the GitHub Actions workflow. Ensure that these version updates do not introduce any compatibility issues with the testing environment.
- 91-91: The test command has been updated to
deno task test:coverage. Verify that the coverage task is configured correctly and that it generates accurate coverage reports.denops/@denops-private/host.ts (1)
- 1-97: The
Hostinterface and related types and functions have been defined or updated. Ensure that these changes are consistent with the expected behavior of the host in the Denops framework and that they do not introduce any breaking changes.denops/@denops-private/worker.ts (1)
- 1-93: The
worker.tsfile has been updated to handle the initialization and termination of the Denops service within a worker context. Ensure that the worker is correctly terminated to avoid resource leaks and that error handling is robust.denops/@denops-private/testutil/with.ts (1)
- 1-102: The
withVimandwithNeovimfunctions have been updated to handle test environments for Vim and Neovim. Ensure that these functions correctly set up and tear down the test environment without leaving any processes or resources hanging.denops/@denops-private/denops_test.ts (1)
- 1-106: Tests for the
DenopsImplclass have been added, covering various methods such asredraw,call,batch,cmd,eval, anddispatch. Ensure that these tests are comprehensive and that they accurately reflect the behavior of theDenopsImplclass.denops/@denops-private/host_test.ts (1)
- 1-126: Tests for the
invokefunction inhost.tshave been added. Ensure that these tests cover all scenarios, including valid and invalid arguments, and that they correctly assert the behavior of theinvokefunction.denops/@denops-private/host/vim.ts (8)
- 1-7: Imports from
unknownutilandvim_channel_commandmodules have been updated. Ensure that the updated versions are compatible with the rest of the codebase.- 21-21: The
onMessagehandler forthis.#sessionis correctly handling both success and error cases when dispatching messages. However, ensure that the error logging is comprehensive and that it doesn't leak any sensitive information.- 47-53: The error message in the
callmethod provides a clear indication of the potential issue with 'denops.vim' not being in 'runtimepath'. This is good for debugging purposes.- 63-80: The
batchmethod has similar error handling to thecallmethod, which is consistent and maintains readability.- 83-85: The
notifymethod is implemented correctly to send notifications without waiting for a response.- 87-90: The
initmethod correctly binds theServiceinstance to the host. Ensure that theServiceinstance is always initialized before callinginit.- 105-114: The
#dispatchmethod is private and correctly throws an error for unexpected messages. Ensure that the error message is clear and actionable.- 122-134: The utility functions
isCallReturn,isBatchReturn,isVoidMessage, andisInvokeMessageare using type guards fromunknownutilto ensure the correctness of the message formats.denops/@denops-private/host/nvim.ts (8)
- 1-8: Imports from
unknownutilandmessagepack_rpcmodules have been updated. Ensure that the updated versions are compatible with the rest of the codebase.- 27-36: The dispatcher method
invokeis correctly checking for the presence of#servicebefore attempting to callinvoke. This prevents potential runtime errors.- 38-39: The
nvim_error_eventhandler logs errors to the console. Ensure that the error logging is comprehensive and that it doesn't leak any sensitive information.- 59-72: The
callmethod has been refactored to handle errors and formatting differently. Ensure that the error handling is consistent with the rest of the codebase and that the error messages are clear.- 77-90: The
batchmethod has been refactored similarly to thecallmethod. Ensure that the error handling is consistent and that the error messages provide enough information for debugging.- 95-97: The
notifymethod is implemented correctly to send notifications without waiting for a response.- 99-120: The
initmethod correctly sets the client information and binds theServiceinstance to the host. Ensure that theServiceinstance is always initialized before callinginit.- 136-151: The utility functions
isNvimErrorObjectandisNvimCallAtomicReturnare using type guards fromunknownutilto ensure the correctness of the message formats.README.md (2)
- 6-8: The badges in the README have been updated to reflect the new supported versions of Deno, Vim, and Neovim. Ensure that these versions are indeed supported and that the links point to the correct versions.
- 6-8: The section on versioning has been removed from the README. Ensure that this information is either outdated or relocated to a more appropriate place in the documentation.
denops/@denops-private/service.ts (11)
- 1-8: Imports have been updated to use specific versions, which is good for ensuring compatibility and preventing issues with future changes to dependencies.
- 14-17: The
Serviceclass now uses private fields and methods, which is a good practice for encapsulation and maintainability.- 26-44: The
loadmethod has been refactored to return a promise. Ensure that all calls to this method properly handle the promise, either withawaitor with.then()and.catch().- 49-61: The
reloadmethod now returns a promise. Ensure that all calls to this method properly handle the promise, and that the reloading logic is correctly implemented.- 64-69: The
#dispatchmethod is private and correctly throws an error for unloaded plugins. Ensure that the error message is clear and actionable.- 74-76: The
dispatchmethod correctly wraps the private#dispatchcall in a try-catch block to convert errors to a Vim-friendly format.- 80-104: The
dispatchAsyncmethod correctly handles both success and failure cases when dispatching messages asynchronously. Ensure that the error handling is comprehensive and that it doesn't leak any sensitive information.- 109-111: The
disposemethod no longer returns a promise, which is consistent with the Disposable interface. Ensure that all usages ofdisposedo not expect a promise.- 114-140: The new
Pluginclass handles plugin loading and calling. Ensure that the plugin lifecycle is correctly managed and that errors during plugin loading are handled appropriately.- 143-148: The
emitfunction correctly emits autocmd events. Ensure that the event names are consistent and that they are used correctly throughout the codebase.- 158-166: The utility functions
resolveScriptUrlandtoVimErrorare unchanged and should continue to work as expected.denops/@denops-private/host/vim_test.ts (2)
- 1-17: Imports for testing utilities have been updated. Ensure that the updated versions are compatible with the test cases and that they provide the necessary functionality for testing.
- 18-154: The test cases for the
Vimclass cover various scenarios, including error handling and message dispatching. Ensure that the tests are comprehensive and that they cover all new changes to theVimclass.denops/@denops-private/host/nvim_test.ts (2)
- 1-17: Imports for testing utilities have been updated. Ensure that the updated versions are compatible with the test cases and that they provide the necessary functionality for testing.
- 18-167: The test cases for the
Neovimclass cover various scenarios, including error handling and message dispatching. Ensure that the tests are comprehensive and that they cover all new changes to theNeovimclass.autoload/denops/plugin.vim (1)
- 168-173: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The autocmd events
DenopsSystemPluginWorkerPreandDenopsSystemPluginWorkerPosthave been removed. Ensure that this change is intentional and that it does not affect the plugin initialization flow.denops/@denops-private/service_test.ts (18)
- 17-20: The use of
new URLwithimport.meta.urlis correct for referencing test data files in a Deno project.- 22-28: The
metaobject is correctly structured to represent metadata for the test environment.- 29-33: The
hostobject usesunimplemented()for methods that are not relevant to the test, which is a good use of theerrorutillibrary to explicitly mark unimplemented functionality.- 36-41: The test case correctly asserts that an error is thrown when trying to load a plugin without a bound host.
- 44-58: The test case for
dispatchAsync()properly handles the rejection when no host is bound, aligning with the expected behavior.- 62-62: Binding the
hostto theserviceis necessary before proceeding with further tests that require a host. This is done correctly.- 64-92: The test case for
load()correctly stubs thehost.callmethod and asserts that the autocmd events are emitted as expected.- 95-118: The test case for loading a plugin that fails to initialize properly checks that the
DenopsSystemPluginFailautocmd event is emitted, which is a good practice for error handling in plugin systems.- 122-140: The test case for loading an already loaded plugin correctly asserts that no additional calls are made and logs the appropriate message.
- 143-171: The
reload()test case is similar to theload()test case and correctly checks for the autocmd events during the plugin reload process.- 174-191: The test case for
reload()when the plugin is not loaded yet correctly asserts that no calls are made and the expected log message is output.- 195-209: The test case for
dispatch()correctly asserts that the plugin API is called as expected.- 212-220: The test case for
dispatch()when the plugin is not loaded yet correctly rejects and matches the expected error message.- 223-247: The test case for
dispatch()when the API call fails correctly rejects and matches the expected error message.- 251-280: The test case for
dispatchAsync()when the API call succeeds correctly asserts that the success callback is called.- 284-320: The test case for
dispatchAsync()when the API call fails correctly asserts that the failure callback is called.- 324-368: The test case for
dispatchAsync()when the success callback fails correctly asserts that the error is logged.- 372-413: The test case for
dispatchAsync()when the failure callback itself fails correctly asserts that the error is logged.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- README.md (4 hunks)
Additional comments: 14
README.md (14)
- 4-4: The subtitle change is more concise and clear.
- 6-8: Updated version badges reflect the new minimum supported versions for Deno, Vim, and Neovim.
- 21-30: Introduction of pronunciation and links to related technologies is a good addition for clarity and resourcefulness.
- 34-35: The instruction to install the latest Deno is clear and directs users to the official manual.
- 38-40: The check for
denocommand's executability from Vim/Neovim is a practical step for troubleshooting.- 47-52: Providing an alternative method to specify the Deno executable path is helpful for users with non-standard setups.
- 54-59: Instructions for installing
denops.vimusing a plugin manager are straightforward.- 61-71: The addition of a test plugin installation and verification step is a good practice to ensure correct setup.
- 85-118: The explanation of the shared server concept and setup instructions are clear and could improve user experience.
- 120-132: The Windows-specific advice regarding antivirus software and Deno's cache directory is a valuable tip for troubleshooting performance issues.
- 78-137: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [134-152]
The support policy section is informative and sets clear expectations for version support.
- 153-170: The section for plugin developers provides useful resources and community support options.
- 174-182: Acknowledging the inspiration from
coc.nvimand providing links to the community and related technologies is a nice touch.- 190-192: The licensing section is standard and clear.
|
It works for me. |
The latest Vim version is 9.1 thus we should support the latest version of Vim 9.0. vim-win32-installer provides 9.0.2189 as the latest Vim 9.0 so we decided to support that version.
The most recent version of Neovim is 0.9.5, so we provide support for version 0.9.4 and newer. Please note that while we mention supporting the two most recent minor versions of Neovim, the software has not reached version 1.0. Therefore, we consider the patch version to be equivalent to the minor version for the purpose of support.
`Service` itself does not need to expose `meta` and `host` so directly assign these instances to `DenopsImpl`.
Remove `Invoker` class and use `Service` directly with common `invoke` function.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 32
Configuration used: CodeRabbit UI
Files selected for processing (31)
- .github/workflows/test.yml (2 hunks)
- README.md (3 hunks)
- autoload/denops.vim (1 hunks)
- autoload/denops/_internal/echo.vim (2 hunks)
- autoload/denops/_internal/test.vim (1 hunks)
- autoload/denops/plugin.vim (1 hunks)
- autoload/health/denops.vim (1 hunks)
- deno.jsonc (1 hunks)
- denops/@denops-private/cli.ts (1 hunks)
- denops/@denops-private/denops.ts (2 hunks)
- denops/@denops-private/denops_test.ts (1 hunks)
- denops/@denops-private/error.ts (1 hunks)
- denops/@denops-private/error_test.ts (1 hunks)
- denops/@denops-private/host.ts (1 hunks)
- denops/@denops-private/host/nvim.ts (4 hunks)
- denops/@denops-private/host/nvim_test.ts (1 hunks)
- denops/@denops-private/host/vim.ts (4 hunks)
- denops/@denops-private/host/vim_test.ts (1 hunks)
- denops/@denops-private/host_test.ts (1 hunks)
- denops/@denops-private/service.ts (2 hunks)
- denops/@denops-private/service_test.ts (1 hunks)
- denops/@denops-private/testdata/dummy_invalid_plugin.ts (1 hunks)
- denops/@denops-private/testdata/dummy_valid_plugin.ts (1 hunks)
- denops/@denops-private/testutil/cli.ts (1 hunks)
- denops/@denops-private/testutil/conf.ts (1 hunks)
- denops/@denops-private/testutil/conf_test.ts (1 hunks)
- denops/@denops-private/testutil/with.ts (1 hunks)
- denops/@denops-private/util.ts (1 hunks)
- denops/@denops-private/version.ts (1 hunks)
- denops/@denops-private/worker.ts (1 hunks)
- plugin/denops.vim (2 hunks)
Files skipped from review due to trivial changes (2)
- autoload/denops/plugin.vim
- autoload/health/denops.vim
Additional comments: 31
autoload/denops/_internal/test.vim (1)
- 1-17: No issues found.
autoload/denops/_internal/echo.vim (2)
- 8-10: No issues found.
- 20-20: No issues found.
plugin/denops.vim (1)
- 6-8: No issues found.
autoload/denops.vim (1)
- 22-22: No issues found.
.github/workflows/test.yml (3)
- 56-60: No issues found.
- 91-91: No issues found.
- 98-98: No issues found.
denops/@denops-private/host/vim.ts (2)
- 1-8: Update the dependency versions to
unknownutilandvim_channel_commandto ensure compatibility with the new versions.- 105-114: Ensure proper error handling and validation for the
#dispatchmethod to prevent unexpected runtime errors.Verification successful
The provided shell script output confirms that the
#dispatchmethod indenops/@denops-private/host/vim.tsincludes error handling through the use ofthrow new Errorfor different scenarios:
- When no service is registered in the host, it throws an error stating, "No service is registered in the host."
- It also handles unexpected JSON channel messages by throwing an error with the message content for debugging purposes.
These checks ensure that the method has proper error handling mechanisms to prevent unexpected runtime errors related to service invocation and message processing. Therefore, the review comment regarding ensuring proper error handling and validation for the
#dispatchmethod to prevent unexpected runtime errors has been addressed in the code.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify proper error handling in #dispatch method grep -C3 "throw new Error" denops/@denops-private/host/vim.tsLength of output: 1348
denops/@denops-private/host/nvim.ts (2)
- 1-5: Update the dependency versions to
unknownutiland ensure compatibility with the new versions.- 136-145: Ensure proper error handling and validation for the
nvim_error_eventmessage to prevent unexpected runtime errors.Verification successful
The provided shell script output shows that there is explicit error handling for
nvim_error_eventmessages within thedenops/@denops-private/host/nvim.tsfile. The error handling mechanism logs the error to the console usingconsole.error, which is a basic form of error reporting. This indicates that there is a mechanism in place to handle and report errors related tonvim_error_event.Given this context, it appears that the original review comment concerning ensuring proper error handling and validation for the
nvim_error_eventmessage to prevent unexpected runtime errors has been addressed to some extent by logging the error. However, the depth of error handling (beyond logging) and whether it fully prevents unexpected runtime errors cannot be fully assessed without a deeper understanding of the overall error handling strategy and implementation details within the application.Based on the information available, it seems appropriate to conclude that the review comment has been addressed in terms of identifying and logging errors related to
nvim_error_event. Further analysis on the effectiveness of this error handling strategy in preventing runtime errors would require a more comprehensive review of the application's error handling mechanisms.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify proper error handling for nvim_error_event message grep -C3 "console.error" denops/@denops-private/host/nvim.tsLength of output: 484
denops/@denops-private/service.ts (6)
- 1-8: Ensure all new imports are used effectively and check for any unnecessary imports after refactoring.
- 14-23: The introduction of the
Serviceclass with a constructor acceptingMetais a good practice for dependency injection.- 80-90: Ensure
dispatchAsyncmethod properly handles errors and uses callbacks correctly to avoid unhandled promises.- 109-111: Validate disposal logic in
disposemethod to ensure all resources are cleaned up properly.- 114-140: Review the
Pluginclass for proper error handling and lifecycle management of plugins.- 161-166: Check the implementation of
toVimErrorto ensure it correctly formats errors for Vim/Neovim.denops/@denops-private/host/vim_test.ts (2)
- 1-17: Ensure all new imports are used effectively and check for any unnecessary imports after refactoring.
- 18-154: Review the test cases for
Vimclass to ensure they cover all new functionality and error handling paths.denops/@denops-private/host/nvim_test.ts (2)
- 1-17: Ensure all new imports are used effectively and check for any unnecessary imports after refactoring.
- 18-168: Review the test cases for
Neovimclass to ensure they cover all new functionality and error handling paths.README.md (6)
- 6-8: Update the badges for Deno, Vim, and Neovim to reflect the new supported versions.
- 85-120: The introduction of the shared server feature to improve plugin startup time is well-documented and provides clear instructions for setup.
- 124-133: The guidance for Windows users regarding antivirus software and Deno's cache directory is helpful and well-explained.
- 135-151: Ensure the support policy section is updated to reflect the current policy and supported versions.
- 156-173: Review the section for plugin developers to ensure it provides clear guidance and useful resources.
- 177-195: Ensure the miscellaneous section, including developer acknowledgments and license information, is accurate and up-to-date.
denops/@denops-private/service_test.ts (3)
- 1-16: Ensure all new imports are used effectively and check for any unnecessary imports after refactoring.
- 17-21: Verify the test data files
dummy_valid_plugin.tsanddummy_invalid_plugin.tsare correctly implemented for testing.Verification successful
The provided script outputs show the contents of both
dummy_valid_plugin.tsanddummy_invalid_plugin.tsfiles, which are intended for testing within thedenopsframework.
dummy_valid_plugin.tsdefines an asyncmainfunction that sets up a dispatcher with atestmethod and executes a couple of commands usingdenops.cmd. This implementation seems correctly designed to simulate a valid Denops plugin, demonstrating how it interacts with the Denops framework by registering a dispatcher and executing commands.
dummy_invalid_plugin.tsdefines amainfunction that immediately throws an error. This is a straightforward way to simulate a plugin that fails upon execution, which is useful for testing error handling within the Denops framework.Based on the provided context and the contents of these files, it appears that both test data files are implemented in a manner that aligns with their intended purpose:
dummy_valid_plugin.tsto simulate a correctly functioning plugin anddummy_invalid_plugin.tsto simulate a plugin that fails. This suggests that the verification process for the implementation of these test data files is successful.* 22-416: Review the test cases for `Service` class to ensure they cover all new functionality, error handling paths, and plugin lifecycle management.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the implementation of test data files cat denops/@denops-private/testdata/dummy_valid_plugin.ts cat denops/@denops-private/testdata/dummy_invalid_plugin.tsLength of output: 627
|
|
||
| export class Vim implements Host { | ||
| #session: Session; | ||
| #client: Client; | ||
| #invoker?: Invoker; | ||
| #service?: Service; |
There was a problem hiding this comment.
Ensure that all usages of #service are checked for undefined before accessing its methods to prevent runtime errors.
- #service?: Service;
+ #service: Service | undefined;Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| #service?: Service; | |
| #service: Service | undefined; |
| if (result === "ERROR") { | ||
| // Not sure why but Vim just returns "ERROR" when 'denops.vim' is not in 'runtimepath'. | ||
| // This makes debugging nightmare so we throw an error with better message here. | ||
| throw new Error( | ||
| `Vim just returns "ERROR" for 'denops#api#vim#call()'. Check if 'denops.vim' exist in 'runtimepath' properly`, | ||
| ); |
There was a problem hiding this comment.
The error message for call method when result is "ERROR" could be more specific to help with debugging.
- `Vim just returns "ERROR" for 'denops#api#vim#call()'. Check if 'denops.vim' exist in 'runtimepath' properly`,
+ `Vim returned "ERROR" for 'denops#api#vim#call()'. Ensure 'denops.vim' is correctly included in 'runtimepath'.`,Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| if (result === "ERROR") { | |
| // Not sure why but Vim just returns "ERROR" when 'denops.vim' is not in 'runtimepath'. | |
| // This makes debugging nightmare so we throw an error with better message here. | |
| throw new Error( | |
| `Vim just returns "ERROR" for 'denops#api#vim#call()'. Check if 'denops.vim' exist in 'runtimepath' properly`, | |
| ); | |
| if (result === "ERROR") { | |
| // Not sure why but Vim just returns "ERROR" when 'denops.vim' is not in 'runtimepath'. | |
| // This makes debugging nightmare so we throw an error with better message here. | |
| throw new Error( | |
| `Vim returned "ERROR" for 'denops#api#vim#call()'. Ensure 'denops.vim' is correctly included in 'runtimepath'.`, | |
| ); |
| if (result === "ERROR") { | ||
| // Not sure why but Vim just returns "ERROR" when 'denops.vim' is not in 'runtimepath'. | ||
| // This makes debugging nightmare so we throw an error with better message here. | ||
| throw new Error( | ||
| `Vim just returns "ERROR" for 'denops#api#vim#batch()'. Check if 'denops.vim' exist in 'runtimepath' properly`, | ||
| ); |
There was a problem hiding this comment.
Similar to the previous comment, refine the error message for clarity in the batch method.
- `Vim just returns "ERROR" for 'denops#api#vim#batch()'. Check if 'denops.vim' exist in 'runtimepath' properly`,
+ `Vim returned "ERROR" for 'denops#api#vim#batch()'. Ensure 'denops.vim' is correctly included in 'runtimepath'.`,Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| if (result === "ERROR") { | |
| // Not sure why but Vim just returns "ERROR" when 'denops.vim' is not in 'runtimepath'. | |
| // This makes debugging nightmare so we throw an error with better message here. | |
| throw new Error( | |
| `Vim just returns "ERROR" for 'denops#api#vim#batch()'. Check if 'denops.vim' exist in 'runtimepath' properly`, | |
| ); | |
| if (result === "ERROR") { | |
| // Not sure why but Vim just returns "ERROR" when 'denops.vim' is not in 'runtimepath'. | |
| // This makes debugging nightmare so we throw an error with better message here. | |
| throw new Error( | |
| `Vim returned "ERROR" for 'denops#api#vim#batch()'. Ensure 'denops.vim' is correctly included in 'runtimepath'.`, | |
| ); |
| @@ -0,0 +1,5 @@ | |||
| import type { Denops } from "https://deno.land/x/denops_core@v6.0.5/mod.ts"; | |||
There was a problem hiding this comment.
Missing module version specification in the import statement.
- import type { Denops } from "https://deno.land/x/denops_core@v6.0.5/mod.ts";
+ import type { Denops } from "https://deno.land/x/denops_core/mod.ts";Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| import type { Denops } from "https://deno.land/x/denops_core@v6.0.5/mod.ts"; | |
| import type { Denops } from "https://deno.land/x/denops_core/mod.ts"; |
| @@ -0,0 +1,10 @@ | |||
| import type { Denops } from "https://deno.land/x/denops_core@v6.0.5/mod.ts"; | |||
There was a problem hiding this comment.
Missing module version specification in the import statement.
- import type { Denops } from "https://deno.land/x/denops_core@v6.0.5/mod.ts";
+ import type { Denops } from "https://deno.land/x/denops_core/mod.ts";Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| import type { Denops } from "https://deno.land/x/denops_core@v6.0.5/mod.ts"; | |
| import type { Denops } from "https://deno.land/x/denops_core/mod.ts"; |
| import type { Meta } from "https://deno.land/x/denops_core@v6.0.5/mod.ts"; | ||
| import { | ||
| assertSpyCall, | ||
| stub, | ||
| } from "https://deno.land/std@0.211.0/testing/mock.ts"; | ||
| import { DenopsImpl, Host, Service } from "./denops.ts"; |
There was a problem hiding this comment.
Missing module version specification in the import statement.
- import type { Meta } from "https://deno.land/x/denops_core@v6.0.5/mod.ts";
- import {
- assertSpyCall,
- stub,
- } from "https://deno.land/std@0.211.0/testing/mock.ts";
- import { DenopsImpl, Host, Service } from "./denops.ts";
+ import type { Meta } from "https://deno.land/x/denops_core/mod.ts";
+ import {
+ assertSpyCall,
+ stub,
+ } from "https://deno.land/std/testing/mock.ts";
+ import { DenopsImpl, Host, Service } from "./denops.ts"; // Ensure all imports specify a versionCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| import type { Meta } from "https://deno.land/x/denops_core@v6.0.5/mod.ts"; | |
| import { | |
| assertSpyCall, | |
| stub, | |
| } from "https://deno.land/std@0.211.0/testing/mock.ts"; | |
| import { DenopsImpl, Host, Service } from "./denops.ts"; | |
| import type { Meta } from "https://deno.land/x/denops_core/mod.ts"; | |
| import { | |
| assertSpyCall, | |
| stub, | |
| } from "https://deno.land/std/testing/mock.ts"; | |
| import { DenopsImpl, Host, Service } from "./denops.ts"; // Ensure all imports specify a version |
| import { assertThrows } from "https://deno.land/std@0.211.0/assert/mod.ts"; | ||
| import { | ||
| assertSpyCall, | ||
| assertSpyCalls, | ||
| stub, | ||
| } from "https://deno.land/std@0.211.0/testing/mock.ts"; | ||
| import { AssertError } from "https://deno.land/x/unknownutil@v3.13.0/mod.ts"; | ||
| import { invoke, Service } from "./host.ts"; | ||
| import { unimplemented } from "https://deno.land/x/errorutil@v0.1.1/mod.ts"; | ||
|
|
There was a problem hiding this comment.
Missing module version specification in the import statement.
- import { assertThrows } from "https://deno.land/std@0.211.0/assert/mod.ts";
- import {
- assertSpyCall,
- assertSpyCalls,
- stub,
- } from "https://deno.land/std@0.211.0/testing/mock.ts";
- import { AssertError } from "https://deno.land/x/unknownutil@v3.13.0/mod.ts";
- import { invoke, Service } from "./host.ts";
- import { unimplemented } from "https://deno.land/x/errorutil@v0.1.1/mod.ts";
+ import { assertThrows } from "https://deno.land/std/assert/mod.ts";
+ import {
+ assertSpyCall,
+ assertSpyCalls,
+ stub,
+ } from "https://deno.land/std/testing/mock.ts";
+ import { AssertError } from "https://deno.land/x/unknownutil/mod.ts";
+ import { invoke, Service } from "./host.ts";
+ import { unimplemented } from "https://deno.land/x/errorutil/mod.ts"; // Ensure all imports specify a versionCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| import { assertThrows } from "https://deno.land/std@0.211.0/assert/mod.ts"; | |
| import { | |
| assertSpyCall, | |
| assertSpyCalls, | |
| stub, | |
| } from "https://deno.land/std@0.211.0/testing/mock.ts"; | |
| import { AssertError } from "https://deno.land/x/unknownutil@v3.13.0/mod.ts"; | |
| import { invoke, Service } from "./host.ts"; | |
| import { unimplemented } from "https://deno.land/x/errorutil@v0.1.1/mod.ts"; | |
| import { assertThrows } from "https://deno.land/std/assert/mod.ts"; | |
| import { | |
| assertSpyCall, | |
| assertSpyCalls, | |
| stub, | |
| } from "https://deno.land/std/testing/mock.ts"; | |
| import { AssertError } from "https://deno.land/x/unknownutil/mod.ts"; | |
| import { invoke, Service } from "./host.ts"; | |
| import { unimplemented } from "https://deno.land/x/errorutil/mod.ts"; |
| async #dispatch(name: string, fn: string, args: unknown[]): Promise<unknown> { | ||
| const plugin = this.#plugins.get(name); | ||
| if (!plugin) { | ||
| throw new Error(`No plugin '${name}' is loaded`); | ||
| } | ||
| return await plugin.call(fn, ...args); |
There was a problem hiding this comment.
Validate plugin existence before attempting to call its methods in #dispatch to prevent runtime errors.
+ if (!this.#plugins.has(name)) {
+ throw new Error(`Plugin '${name}' not found`);
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| async #dispatch(name: string, fn: string, args: unknown[]): Promise<unknown> { | |
| const plugin = this.#plugins.get(name); | |
| if (!plugin) { | |
| throw new Error(`No plugin '${name}' is loaded`); | |
| } | |
| return await plugin.call(fn, ...args); | |
| async #dispatch(name: string, fn: string, args: unknown[]): Promise<unknown> { | |
| if (!this.#plugins.has(name)) { | |
| throw new Error(`Plugin '${name}' not found`); | |
| } | |
| const plugin = this.#plugins.get(name); | |
| if (!plugin) { | |
| throw new Error(`No plugin '${name}' is loaded`); | |
| } | |
| return await plugin.call(fn, ...args); |
| return await this.#dispatch(name, fn, args); | ||
| } catch (e) { | ||
| // NOTE: | ||
| // Vim/Neovim does not handle JavaScript Error instance thus use string instead | ||
| throw `${e.stack ?? e.toString()}`; | ||
| throw toVimError(e); |
There was a problem hiding this comment.
Improve error handling in dispatch method by catching and re-throwing errors with more context.
- throw toVimError(e);
+ throw new Error(`Error dispatching to plugin '${name}': ${toVimError(e)}`);Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| return await this.#dispatch(name, fn, args); | |
| } catch (e) { | |
| // NOTE: | |
| // Vim/Neovim does not handle JavaScript Error instance thus use string instead | |
| throw `${e.stack ?? e.toString()}`; | |
| throw toVimError(e); | |
| return await this.#dispatch(name, fn, args); | |
| } catch (e) { | |
| throw new Error(`Error dispatching to plugin '${name}': ${toVimError(e)}`); |
| async function emit(denops: Denops, name: string): Promise<void> { | ||
| try { | ||
| await denops.cmd(`doautocmd <nomodeline> User ${name}`); | ||
| } catch (e) { | ||
| console.error(`Failed to emit ${name}: ${e}`); | ||
| } |
There was a problem hiding this comment.
Ensure emit function properly handles errors and does not suppress them silently.
- console.error(`Failed to emit ${name}: ${e}`);
+ throw new Error(`Failed to emit event '${name}': ${e}`);Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| async function emit(denops: Denops, name: string): Promise<void> { | |
| try { | |
| await denops.cmd(`doautocmd <nomodeline> User ${name}`); | |
| } catch (e) { | |
| console.error(`Failed to emit ${name}: ${e}`); | |
| } | |
| async function emit(denops: Denops, name: string): Promise<void> { | |
| try { | |
| await denops.cmd(`doautocmd <nomodeline> User ${name}`); | |
| } catch (e) { | |
| throw new Error(`Failed to emit event '${name}': ${e}`); | |
| } |
For Users
Changes seen by users using the Denops plugin are mostly limited to updates of supported versions, with no significant changes other than that.
Updates of Supported Versions
In Denops v6, the following versions are supported:
Approximately 50% Reduction in Memory Usage
The memory usage of the Denops server has been reduced by approximately half compared to v5. For more details, refer to Memory (RSS) usage difference between denops v5 and v6.
Improved Communication Speed with Vim/Neovim
Communication speed with Vim/Neovim has been enhanced, especially noticeable in Neovim, where under specific conditions, you can issue about 1.8 times more commands compared to v5. For detailed results, see Communication performance difference between denops v5 and v6.
Reduced Impact on Initialization Time Due to the Number of Plugins
The issue of increasing initialization time as the number of plugins dependent on Denops grows has been resolved. However, users who do not utilize a large number (around 100) of Denops plugins will not perceive a significant difference. For detailed results, refer to Startup performance difference between v5 and v6.
For Denops Plugin Developers
Plugins are no longer isolated by Worker threads
Until v5, each plugin was processed in a dedicated Worker thread, but from v6 onwards, processing is done in a Worker thread per Vim/Neovim instance. Be cautious if your code uses global variables, and consider using the
denops.context (Record<PropertyKey, unknown>)provided for each plugin instead.Results of
console.log()always returned to Vim/NeovimUntil v5, the output location of
console.log()differed between Local server and Shared server, but now it is always displayed on the Vim/Neovim side in both cases.Tip
console.log()is intended for temporary use during development. If you want to display messages to users as part of the plugin's functionality, continue to usedenops.cmd("echo ...")or theechofunction indenops_std.For Denops Developers
denops_corehas been separated into a separate repositoryThe type information of Denops, which was managed in the
denops/@denopsdirectory until v5, has been separated into a separate repository called deno-denops-core. With this change, the version update policy of "no releases other than updates under thedenops/@denopsdirectory" has been abolished, and version updates will follow semantic versioning.Lower-level test utilities included, separate from
denops_testA module named
denops/@denops-private/testutilhas been added to directly test theHostused in the internal implementation of Denops. As a result, the parts that were tested usingdenops_testhave been removed, and testing is now self-contained indenops.vim.Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Refactor
Documentation
Chores
Tests