-
Notifications
You must be signed in to change notification settings - Fork 208
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 support for Ruby LSP as a built-in add-on #630
Conversation
It's failing because Ruby LSP doesn't format files outside of the workspace path. You can workaround that with: - with_server(source) do |server, uri|
+ with_server(source) do |server, _uri|
+ uri = URI(File.join(server.global_state.workspace_path, 'fake.rb'))
server.process_message(
id: 1,
- method: "textDocument/formatting",
- params: {textDocument: {uri: uri}, position: {line: 0, character: 0}}
+ method: 'textDocument/formatting',
+ params: { textDocument: { uri: uri },
+ position: { line: 0, character: 0 } }
) However, it'd then actually try to run formatting with that file, which will result to another error since the file doesn't actually exist. So you'll likely need additional setup to:
|
@st0012 thanks Stan! Got through that and have it formatting correctly now. Question: can I safely rely on Question: this test is now failing b/c the range being returned ends on line 19, column 19, even though the source string is only 2 lines long and that second line is only 5 or 6 characters. Since the addon is not providing the range, is it a bug? |
I'm unsure the cause of the test failure this introduces, but just looking at the addon stuff, does this look right? @vinistock, @st0012? |
Looking into this failure, I have very few good answers. AFAICT, if one executes The test will print this to stderr:
And then the test will fail because the shelled-out run will exit non-zero. This is a bit of a mystery to me. Like, the best explanation I can imagine is that loading Extremely odd failure |
Hey @vinistock @st0012, I'm trying to validate this actually works by setting up a project pointing at my local copy using: gem "standard", path: "/path/to/standardrb/standard" But Ruby LSP doesn't seem to pick it up as a formatter/linter option. (If I Question: do you have any idea why this might not be working when the plugin is a local path-sourced gem? Meanwhile, if I change my dependency to look at this github branch:
And then I check the output tab of VS Code, I get a stack trace without an error message:
So I'm kind of at a loss as to how to actually test the plugin locally, as a result. |
It looks like the error was caused by And for some reason the addon's error reporting mechanism didn't work as expected as the error message is not displayed. I'll give it a look when I'm back to work. |
This should be solved by Shopify/ruby-lsp#2085 |
Thank you both for the above. I fixed the constant not defined error, but even after clearing it I'm seeing the same behavior where the plugin isn't recognized as a formatter Setting value of "rubyLsp.formatter" to "standard" yields the problem:
Additionally, when I choose "format document" from the command pallete, I get "There is no formatter for 'ruby' files installed." Here's a sample repo to demo the config: |
@andyw8 ahh, yes, if it's just hard-coded now that won't really scale to third-party add-ons. I was assuming the extension was phoning home to the LSP and then updating the configuration schema dynamically (somehow? Think I was conflating it with WorkspaceConfiguration API, which I doubt can be used for that purpose.) It might be necessary to just relax that configuration property in the schema to allow it to be set to any value? |
Looking into some options: Shopify/ruby-lsp#2092 |
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.
❤️
@searls |
[UPDATE: Nevermind, got it running locally OK. Forgot I had to enable the format & diagnostic features on @andyw8 is there any way I ought to be able to test this now? I tried pointing {
"[ruby]": {
"editor.defaultFormatter": "Shopify.ruby-lsp"
},
"rubyLsp.formatter": "standard",
"rubyLsp.linters": [
"standard"
]
}
|
8ddcdc6
to
504a4e3
Compare
The result's response is nil, but (one assumes) it shouldn't be $ bundle exec m test/ruby_lsp_addon_test.rb
…ng the test server needs
…ializes successfully
…RuboCop executor directly instead of via --stdin
Just released this basic version (without code actions) as v1.38.0 |
@vinistock @andyw8 if you would be so kind, let me know when a version of the vscode extension has shipped with the |
@searls you can test it by checking out the https://github.com/Shopify/ruby-lsp/blob/main/CONTRIBUTING.md#live-debugging Everyone on the team is at a company event this week, so there may be a little delay until the next release. |
Heads up I also merged #636 which adds Code Action support (and duplicates half of Ruby LSP's rubocop integration in the process) |
This shipped today: https://github.com/Shopify/ruby-lsp/releases/tag/vscode-ruby-lsp-v0.7.6 |
Thanks @andyw8. I've verified everything works, and so have updated these docs:
I also just published a blog post that I hope you all will appreciate as supporting the vision that @vinistock and others have laid out for the Ruby LSP project. Additionally, I'd love if you highlights this addon on your repo's README along with the others, since the fact that this one is built-in will make discovery harder for Ruby LSP users (especially in the presence of our own bespoke LSP server and VS Code plugin). |
I've created Shopify/ruby-lsp#2263 to include standard in the readme 👍 |
@searls thank you very much for the blog post and your work on this. Having your support for the vision is really important and I appreciate it 🙏. |
Release plan
patchminor releaseBackground
After talking to @vinistock at Kaigi, I want to take a stab at advertising a working Ruby LSP addon from within the gem so that Ruby LSP users can get standard support without running the LSP themselves (or installing a custom extension, like our VS Code extension
I'm trying and failing to get this end-to-end working in an example app, so I'm trying to take a step back and just get this test passing. The result's response is nil, but (one assumes?) it shouldn't be. More importantly,
WrapsBuiltinLspStandardizer#run_formatting
isn't being invoked, so my attempts to set the formatter to `standard is probably not working, either.To replicate the failure, you should be able to run: