-
-
Notifications
You must be signed in to change notification settings - Fork 10k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add vscode Ruby LSP config #17934
Add vscode Ruby LSP config #17934
Conversation
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.
Great work @Rylan12! Would love to see this working nicely.
aedffc5
to
a62d89a
Compare
a62d89a
to
9ddde17
Compare
Okay I pushed a better solution which originally I couldn't get to work right, but I found a way to do it. Now we can use portable ruby as the ruby for the LSP which avoids needing a second I've removed the docs for it because it seems silly to have a whole page for that one line, but maybe it can go somewhere else in the docs. Once I've got the sorbet LSP set up, maybe there will be enough info for a joint page. |
It might be nice to figure out if you can convince VSCode to run that command automatically for you on launch. I know you can get it to run "tasks" when you open a workspace. Let me know if you need more syntax pointers there, I've done it on another project. |
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.
Looks good as-is, good to merge as-is and iterate! Thanks @Rylan12!
You may find this snippet from an old + "rubyLsp.customRubyCommand": "source \"../../.vscode/ruby-lsp/activate.sh\"", homebrew_prefix="$(cd "$(dirname "$0")"/../../ && pwd)/"
"${homebrew_prefix}/bin/brew" install-bundler-gems --add-groups=style,vscode
export PATH="${homebrew_prefix}/Library/Homebrew/vendor/portable-ruby/current/bin:${PATH}"
export BUNDLE_WITH="style vscode" The |
Not for this PR but here's also some settings I got on this before, given I managed to get it working better than ruby-lsp: + "sorbet.enabled": true,
+ "sorbet.lspConfigs": [
+ {
+ "id": "default",
+ "name": "Brew Typecheck",
+ "description": "Default configuration",
+ "cwd": "${workspaceFolder}",
+ "command": [
+ "./bin/brew",
+ "typecheck",
+ "--lsp"
+ ]
+ }
+ ], diff --git a/Library/Homebrew/dev-cmd/typecheck.rb b/Library/Homebrew/dev-cmd/ty
pecheck.rb
index a105d4ac71..00554f7dfc 100644
--- a/Library/Homebrew/dev-cmd/typecheck.rb
+++ b/Library/Homebrew/dev-cmd/typecheck.rb
@@ -21,6 +21,8 @@ module Homebrew
switch "--suggest-typed",
depends_on: "--update",
description: "Try upgrading `typed` sigils."
+ switch "--lsp",
+ description: "Start the Sorbet LSP server."
flag "--dir=",
description: "Typecheck all files in a specific directory."
flag "--file=",
@@ -30,6 +32,9 @@ module Homebrew
"in their paths (relative to the input path passed to Sorbet)."
conflicts "--dir", "--file"
+ conflicts "--lsp", "--update"
+ conflicts "--lsp", "--update-all"
+ conflicts "--lsp", "--fix"
named_args :none
end
@@ -91,6 +96,11 @@ module Homebrew
srb_exec << "--autocorrect"
end
+ if args.lsp?
+ srb_exec << "--lsp"
+ srb_exec << "--disable-watchman" unless which("watchman", ORIGINAL_PATHS)
+ end
+
srb_exec += ["--ignore", args.ignore] if args.ignore.present?
if args.file.present? || args.dir.present?
cd("sorbet") do |
Thanks for all the tips. I'm going to merge as-is for now to get this in, and will look into the shim/task path in the meantime. I was able to get sorbet working with your suggestion, thanks @Bo98! I'll open a PR for that, too. |
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?This PR updates the VS Code configuration to use the Ruby LSP extension instead of the old one which has been deprecated. I've also added some extra documentation to help explain how to use it. This setup seems to work for me, but I would love some other VS Code users to check it and make sure it works for them too.
The basis of the approach I've taken is documented in the extension docs here.
CC: @p-linnane @krehel @Bo98