Make LSP Hover non-experimental (removes --enable-experimental-lsp-hover)
#990
<
8000
/div>
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hover works well enough to be enabled for everyone. If there is a bug, it is easy to add a new test case to Sorbet and iterate on a solution. In real-world uses, hover does not appear to cause any issues with typechecking speed. In particular, VS Code cancels hover if you move your mouse away, so mousing over many things during a slow path won't interrupt edit batching.
DarkDimius
reviewed
Jun 21, 2019
| @@ -311,7 +311,6 @@ cxxopts::Options buildOptions() { | |||
| options.add_options("advanced")("watchman-path", | |||
| "Path to watchman executable. Defaults to using `watchman` on your PATH.", | |||
| cxxopts::value<string>()->default_value(empty.watchmanPath)); | |||
| options.add_options("advanced")("enable-experimental-lsp-hover", "Enable experimental LSP feature: Hover"); | |||
There was a problem hiding this comment.
Note that this will break people who pass this flag.
I'm fine with that(flag is experimental), but this is worth noting in commit message.
There was a problem hiding this comment.
I will edit the merge commit message to note the issue.
DarkDimius
approved these changes
Jun 21, 2019
|
Super excited for this! |
--enable-experimental-lsp-hover)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make LSP Hover non-experimental.
Hover works well enough to be enabled for everyone. If there is a bug, it is easy to add a new test case to Sorbet and iterate on a solution. The only bug I am currently aware of involves hover information for proc arguments at their declaration sites.
In real-world use, hover does not appear to cause any issues with editor responsiveness. In particular, VS Code cancels hover if you move your mouse away, so mousing over many things during a slow path won't dramatically interrupt edit batching.
Motivation
Stabilizing IDE features.
Test plan
Automated tests already exist.
Note that this is going to break those who passed
--enable-experimental-lsp-hoverbefore.