ruby-lsp icon indicating copy to clipboard operation
ruby-lsp copied to clipboard

Enhance version manager config to accept more fields

Open vinistock opened this issue 1 year ago • 3 comments

Motivation

Step towards https://github.com/Shopify/ruby-lsp/issues/1822

Because each version manager has its own customizations, I believe we need to offer more settings to match the behaviour in the editor. We currently only allow setting the identifier of the manager, but that's not always enough (e.g.: see the issue).

Implementation

The idea is to change the versionManager setting so that it becomes an object and then we nest the identifier inside of that object. After this goes in, then we'll be able to add extra configuration fields, like the version manager executable path.

All of the changes are solely to migrate versionManager from a string to an object that looks like { identifier: "chruby" }.

The only other change is the automated migration in extension.ts, where we read all of the existing configuration values and update them automatically to match the new format. That code is temporary and can be removed after it has been there for a while.

Automated Tests

Adapted our existing tests.

Manual Tests

  1. Start the extension on this branch
  2. Open your JSON settings
  3. Verify that your version manager setting was migrated to the new format
  4. verify the server boots properly

vinistock avatar Mar 26 '24 13:03 vinistock

For reference, the older Ruby extension used that pattern for Ruby linter configuration: https://github.com/rubyide/vscode-ruby/blob/main/packages/vscode-ruby-client/package.json#L283-L343

You can see how rubocop is either a boolean or object with rich fields, so either "ruby.lint": { "rubocop": true } or "ruby.lint": { "rubocop": { "command": "foo" } } is valid configuration.

paracycle avatar Mar 26 '24 18:03 paracycle

So we'd avoid the identifier key, and do this?

"rubyLsp.rubyVersionManager": { "chruby": { "foo": "bar" } }

andyw8 avatar Mar 26 '24 18:03 andyw8

Talked with Ufuk on Slack. Unfortunately, if we use the string or object types, we lose the ability to provide enum completions.

In fact, you can add the enum and it does show the right completions, but then it considers the { identifier: } version of the setting as invalid, which makes the experience a bit odd.

With the current implementation, old settings get migrated automatically and you get completion every step of the way, so I think this is a nicer approach.

vinistock avatar Mar 26 '24 19:03 vinistock