Skip to content
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

Allow turning off ERB support through setting #2311

Merged
merged 1 commit into from
Jul 18, 2024
Merged

Conversation

vinistock
Copy link
Member

@vinistock vinistock commented Jul 15, 2024

Motivation

Closes #2282

If the user is working on a project with a dependency that prevents the Ruby LSP server from upgrading, the server will not have ERB support.

Unfortunately, mutating the document selector after the client has already launched doesn't stop it from receiving requests for the removed language IDs. And we have no way of knowing if the server version supports ERB before actually launching. It's a chicken and egg problem.

Implementation

I believe the best we can do is the following (implemented in this PR):

  1. After launching the server, we check if there's ERB support. If there is, then the server is up to date and we don't have to do anything
  2. Otherwise, we print a warning to output tab (should it be a dialog?) and then we restart the server, passing a new option to turn off ERB support before launch (which then has the desired effect of preventing requests for ERB files)

Concerns

Should we even do this? This means that anyone on old server versions will always need to boot the server twice until they manage to upgrade to newer versions.

@vinistock vinistock added bugfix This PR will fix an existing bug vscode This pull request should be included in the VS Code extension's release notes labels Jul 15, 2024
@vinistock vinistock self-assigned this Jul 15, 2024
@vinistock vinistock requested a review from a team as a code owner July 15, 2024 18:27
@vinistock vinistock requested review from andyw8 and st0012 July 15, 2024 18:27
@st0012
Copy link
Member

st0012 commented Jul 15, 2024

should it be a dialog?

Yeah I think it should be dialog

Should we even do this?

If the user still can not upgrade the server after seeing the warning, to revert the erb file behaviour the only alternative would be to downgrade the extension version? If that's the case, then I think restarting the server twice seems more acceptable.

@vinistock
Copy link
Member Author

I really dislike the double-start on launch, so I want to propose that we expose a setting to turn off ERB support as an escape hatch for users that cannot upgrade the server. After a few months, we can get rid of the setting and enable ERB support generally.

vscode/src/client.ts Show resolved Hide resolved
@vinistock vinistock changed the title Remove ERB from document selector for outdated servers Allow turning off ERB support through setting Jul 18, 2024
@vinistock vinistock enabled auto-merge (squash) July 18, 2024 13:48
@vinistock vinistock merged commit b92c560 into main Jul 18, 2024
33 checks passed
@vinistock vinistock deleted the vs/avoid_erb_failures branch July 18, 2024 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR will fix an existing bug vscode This pull request should be included in the VS Code extension's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VSCode: ERB file false positive problems
2 participants