Add support for Ruby LSP as a built-in add-on #630
Conversation
The result's response is nil, but (one assumes) it shouldn't be $ bundle exec m test/ruby_lsp_addon_test.rb
|
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 |
…ng the test server needs
searls
changed the title
|
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 |
| end | ||
|
|
||
| def init! | ||
| @config = ::Standard::BuildsConfig.new.call(ARGV) |
There was a problem hiding this comment.
ARGV here is going to be the set of arguments passed to the ruby-lsp executable, which are likely not recognized by standard. Could that lead to any issues? Would it be better to pass an empty array every time?
| @config = ::Standard::BuildsConfig.new.call(ARGV) | ||
| @standardizer = ::Standard::Lsp::Standardizer.new( | ||
| @config, | ||
| ::Standard::Lsp::Logger.new |
There was a problem hiding this comment.
Does this logger have any chance to print to stdout? In the LSP context, nothing can be printed to STDOUT or else is breaks editor/server communication. Everything has to be printed to STDERR only.
| end | ||
|
|
||
| def run_formatting(uri, document) | ||
| @standardizer.format(uri_to_path(uri), document.source) |
There was a problem hiding this comment.
We have a helper that patches a new method into URI objects to get the correct file system paths, which has some fixes for Windows (the URI gem has some bugs).
| @standardizer.format(uri_to_path(uri), document.source) | |
| @standardizer.format(uri.to_standardized_path, document.source) |
There's only one caveat. If you created a Ruby file and it is currently unsaved, you can invoke formatting manually. In those cases, to_standardized_path returns nil. I'm not sure how Standard will handle that.
| end | ||
|
|
||
| def run_diagnostic(uri, document) | ||
| offenses = @standardizer.offenses(uri_to_path(uri), document.source) |
There was a problem hiding this comment.
| offenses = @standardizer.offenses(uri_to_path(uri), document.source) | |
| offenses = @standardizer.offenses(uri.to_standardized_path, document.source) |
| when "refactor", "info" | ||
| RubyLsp::Constant::DiagnosticSeverity::HINT | ||
| else # the above cases fully cover what RuboCop sends at this time | ||
| logger.puts "Unknown severity: #{severity.inspect}" |
There was a problem hiding this comment.
Does this print to STDOUT? If so, it will throw the LSP into an infinite loop.
| start: RubyLsp::Interface::Position.new(line: loc[:start_line] - 1, character: loc[:start_column] - 1), | ||
| end: RubyLsp::Interface::Position.new(line: loc[:last_line] - 1, character: loc[:last_column] - 1) | ||
| ) | ||
| # TODO: do I need something like this? |
There was a problem hiding this comment.
If you want to support quickfixes through code actions, then yes you'll need to provide those as part of data.
But you can always start without it and add later.
|
|
||
| # lifted from: | ||
| # https://github.com/Shopify/ruby-lsp/blob/4c1906172add4d5c39c35d3396aa29c768bfb898/lib/ruby_lsp/requests/support/rubocop_diagnostic.rb#L84 | ||
| def code_description(cop_name) |
There was a problem hiding this comment.
I should warn you that a contributor is looking into changing this slightly to allow for RuboCop extensions to also have their documentation URLs displayed on diagnostics.
Likely not something that needs to be addressed here, just an FYI Shopify/ruby-lsp#2076.
Release plan
Background
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_formattingisn'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: