-
-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
👍 Add denops#cache#update()
function to update Deno module cache of denops itself and denops plugins in runtimepath
#342
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 15 minutes and 43 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 recent updates revolve around refining the Denops Vim environment, enhancing plugin management, and optimizing caching mechanisms for improved performance. The workflow configuration now triggers more precisely based on specific file modifications, contributing to a more streamlined development experience. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- .github/workflows/test.yml (2 hunks)
- autoload/denops/_internal/plugin.vim (1 hunks)
- autoload/denops/cache.vim (1 hunks)
- autoload/denops/plugin.vim (4 hunks)
- doc/denops.txt (1 hunks)
Additional Context Used
LanguageTool (4)
doc/denops.txt (4)
Near line 60: The verb form ‘install’ does not seem to be suitable in this context.
Context: ...2345 < Note that there is a plugin that install startup scripts of the shared server fo...
Near line 102: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...hown. This variable must be configured prior to denops initialization. Default: 0 *g:...
Near line 124: Possible missing article found.
Context: ...oid infinite errors. Denops will reset internal counter when the process keeps running ...
Near line 361: Possible missing comma found.
Context: ...atically called on |User| |DenopsReady| autocmd invoked by |denops#server#start...
Additional comments not posted (15)
autoload/denops/_internal/plugin.vim (2)
1-12
: LGTM! The function correctly collects plugin scripts from the runtime path.
14-24
: LGTM! The function correctly finds and returns a specific plugin or throws an error if not found.autoload/denops/cache.vim (3)
4-35
: LGTM! The cache update function is well-implemented with options handling and job management.
37-43
: LGTM! The stderr handler function effectively formats and displays error messages.
45-47
: LGTM! The job exit handler correctly cleans up the job variable..github/workflows/test.yml (2)
8-13
: LGTM! The workflow paths are correctly specified to trigger on relevant file changes.
85-90
: LGTM! The new caching step correctly updates the cache using thedenops#cache#update()
function.autoload/denops/plugin.vim (7)
4-5
: LGTM! The function correctly checks if a plugin is loaded by name.
Line range hint
8-36
: Ensure all edge cases are handled in the plugin wait function, especially regarding server status and timeouts.Verification successful
The detailed examination of the
denops#_internal#wait#for
anddenops#server#status
functions confirms that thedenops#plugin#wait
function is equipped to handle various edge cases, including server status checks and timeout management. The implementation includes mechanisms to gracefully manage timeouts and check server readiness, which aligns with the concerns raised in the review comment.
- The
denops#_internal#wait#for
function includes timeout handling and interruption mechanisms.- The
denops#server#status
function provides accurate server state management, crucial for deciding whether waiting is feasible.Based on this analysis, the function appears to handle all edge cases as required.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all edge cases are handled in the plugin wait function. # Test: Search for the function usage. Expect: Proper handling of all scenarios. rg --type vimscript $'denops#plugin#wait'Length of output: 550
Script:
#!/bin/bash # Description: Verify the implementation details of `denops#_internal#wait#for` and `denops#server#status` to ensure edge cases are handled. # Test: Search for the implementation of `denops#_internal#wait#for` and `denops#server#status`. ast-grep --lang vimscript --pattern $'function denops#_internal#wait#for($_, $_, $_) { $$$ }' ast-grep --lang vimscript --pattern $'function denops#server#status() { $$$ }'Length of output: 412
Script:
#!/bin/bash # Description: Verify the implementation details of `denops#_internal#wait#for` and `denops#server#status` using `rg` since `ast-grep` does not support Vimscript. # Test: Search for the implementation of `denops#_internal#wait#for` and `denops#server#status` using `rg`. rg --type vimscript -- 'function! denops#_internal#wait#for' rg --type vimscript -- 'function! denops#server#status'Length of output: 293
Script:
#!/bin/bash # Description: Extract the content of `denops#_internal#wait#for` and `denops#server#status` to analyze their implementation details. # Test: Use `cat` to display the content of the functions. cat autoload/denops/_internal/wait.vim cat autoload/denops/server.vimLength of output: 6742
44-52
: LGTM! The asynchronous wait function is correctly implemented with proper callback management.
61-70
: Marked as deprecated: Ensure users are aware of the newdenops#plugin#load()
function as a replacement.
73-75
: LGTM! The plugin load function correctly handles the loading process and notifies the server.
80-81
: LGTM! The plugin reload function correctly handles the reload process and notifies the server.
87-90
: LGTM! The plugin discovery function correctly collects and loads plugins from the runtime path.doc/denops.txt (1)
203-212
: Ensure the documentation fordenops#cache#update()
is clear and includes all necessary details about the options.Verification successful
The documentation for
denops#cache#update()
includes a clear description of the function's purpose, the options available, and practical command-line examples for both Vim and Neovim. This comprehensive approach not only explains the function but also provides users with actionable examples on how to use it, enhancing both the clarity and utility of the documentation.
- The function's purpose is well-defined, explaining that it updates the Deno module cache for denops and its plugins.
- The available option "reload" is clearly mentioned with its effect.
- The inclusion of command-line examples for both Vim and Neovim is particularly useful for users who might want to run these commands directly from their CLI, making the documentation practical and user-friendly.
Overall, the documentation appears to be complete and clear, providing all necessary details along with practical usage examples.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the documentation for `denops#cache#update()` is complete and clear. # Test: Check the documentation content for completeness. cat doc/denops.txt | grep 'denops#cache#update'Length of output: 237
Script:
#!/bin/bash # Description: Check the surrounding context of `denops#cache#update()` in the documentation to ensure integration and clarity. # Display lines around the occurrences to understand the context better. grep -C 5 'denops#cache#update' doc/denops.txtLength of output: 743
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Out of diff range and nitpick comments (3)
doc/denops.txt (3)
Line range hint
60-60
: Consider changing "install" to "installs" to correct the verb form for singular subject agreement.- Note that there is a plugin that install startup scripts of the shared server for each platform. + Note that there is a plugin that installs startup scripts of the shared server for each platform.
Line range hint
102-102
: Consider simplifying "prior to" to "before" for conciseness.- This variable must be configured prior to denops initialization. + This variable must be configured before denops initialization.
Line range hint
124-124
: Consider adding "the" before "internal counter" to correct the missing article.- Denops will reset internal counter when the process keeps running more than this interval. + Denops will reset the internal counter when the process keeps running more than this interval.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- .github/workflows/test.yml (2 hunks)
- autoload/denops.vim (2 hunks)
- autoload/denops/_internal/plugin.vim (1 hunks)
- autoload/denops/cache.vim (1 hunks)
- autoload/denops/plugin.vim (4 hunks)
- doc/denops.txt (6 hunks)
Files skipped from review as they are similar to previous changes (4)
- .github/workflows/test.yml
- autoload/denops/_internal/plugin.vim
- autoload/denops/cache.vim
- autoload/denops/plugin.vim
Additional Context Used
LanguageTool (3)
doc/denops.txt (3)
Near line 60: The verb form ‘install’ does not seem to be suitable in this context.
Context: ...2345 < Note that there is a plugin that install startup scripts of the shared server fo...
Near line 102: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...hown. This variable must be configured prior to denops initialization. Default: 0 *g:...
Near line 124: Possible missing article found.
Context: ...oid infinite errors. Denops will reset internal counter when the process keeps running ...
Additional comments not posted (3)
autoload/denops.vim (3)
1-4
: LGTM! The implementation ofdenops#notify
correctly uses the internal server channel for dispatching notifications.
8-11
: LGTM! Thedenops#request
function is well-implemented, ensuring that it waits for a response after sending a request.
Line range hint
15-24
: LGTM! The asynchronous request handling indenops#request_async
is correctly implemented with one-time callbacks.
4637b3c
to
64cfdc9
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #342 +/- ##
=======================================
Coverage 90.01% 90.01%
=======================================
Files 9 9
Lines 661 661
Branches 62 62
=======================================
Hits 595 595
Misses 66 66 ☔ View full report in Codecov by Sentry. |
SSIA
Summary by CodeRabbit
New Features
Refactor
Documentation
Chores