🐛 Do NOT throw an error on denops#server#close
#338
Conversation
WalkthroughThe changes in the Changes
Assessment against linked issues
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 ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #338 +/- ##
=======================================
Coverage 89.81% 89.81%
=======================================
Files 9 9
Lines 648 648
Branches 61 61
=======================================
Hits 582 582
Misses 66 66 ☔ View full report in Codecov by Sentry. |
lambdalisue
force-pushed
the
do-not-throw-error-on-server-close
branch
from
4c825aa
to
3170593
Compare
The documentation said that `denops#server#close` does not throw an error even if the server is not connected. However, it throws an error when the server is not connected. This commit fixes the behavior. Close #338
![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 (1)
- autoload/denops/server.vim (2 hunks)
Additional comments not posted (2)
autoload/denops/server.vim (2)
32-32: Simplified control flow indenops#server#restartlooks good.Verification successful
The output from the shell script does not provide sufficient information to verify the behavior of the
denops#server#restartfunction during runtime. It only shows where the function and related configurations are defined in the code. To ensure that the function does not cause unnecessary restarts when the server is already stopped, further investigation is required.--- Due to the limitation of `ast-grep` with Vimscript, we need to use an alternative method to verify the behavior of the `denops#server#restart` function. We will use `rg` to search for any conditional logic within the function that might prevent unnecessary restarts.
The review of the
denops#server#restartfunction indicates that it includes conditional checks to prevent unnecessary restarts when the server is already stopped. The conditionif denops#server#status() ==# 'stopped'within the function is particularly relevant, as it directly checks the server's status before proceeding with actions that would restart the server. This aligns with the intended simplification and control flow improvements in the function.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `denops#server#restart` does not cause unnecessary restarts when the server is already stopped. # Test: Check for multiple restarts in logs when the server is stopped. Expect: No multiple restarts. rg --type vimscript $'denops#server#restart'Length of output: 677
Script:
#!/bin/bash # Description: Check the actual behavior of `denops#server#restart` to ensure it does not restart the server unnecessarily when it's already stopped. # Test: Search for conditions or checks within the `denops#server#restart` function that prevent unnecessary restarts. ast-grep --lang vimscript --pattern $'function! denops#server#restart() { $$$ if ($_) $$$ }'Length of output: 218
Script:
#!/bin/bash # Description: Examine the `denops#server#restart` function for any conditional checks that prevent unnecessary restarts. # Test: Use rg to search for conditional statements within the `denops#server#restart` function. rg --type vimscript 'if' autoload/denops/server.vimLength of output: 648
59-61: Preventative check indenops#server#closeenhances robustness by avoiding errors on non-existent channels.
|
Oops. Rebase failure. |
The documentation said that `denops#server#close` does not throw an error even if the server is not connected. However, it throws an error when the server is not connected. This commit fixes the behavior. Close #338
lambdalisue
force-pushed
the
do-not-throw-error-on-server-close
branch
from
a59f5fd
to
dbab491
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- autoload/denops/server.vim (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- autoload/denops/server.vim
Close #337
Summary by CodeRabbit
denops#server#restart()anddenops#server#reconnect()functions for better control flow.