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

Add ruby support via solargraph #46

Merged
merged 21 commits into from
Jan 29, 2025

Conversation

sbrudz
Copy link
Contributor

@sbrudz sbrudz commented Jan 22, 2025

This PR adds support for the ruby language by adding the solargraph LSP server.

We're also interested in adding ruby-lsp as an alternative server in a future PR. Not sure of the preferred way to support multiple LSP servers for the same language, though. If you have any recommendations, please let me know.

@sbrudz
Copy link
Contributor Author

sbrudz commented Jan 22, 2025

@microsoft-github-policy-service agree company="Def Method"

@LakshyAAAgrawal
Copy link
Collaborator

Dear @sbrudz

Thanks for the amazing PR! Could you please have a look at the following error being thrown in the automated ruby tests (https://github.com/microsoft/multilspy/actions/runs/12918692497/job/36099508872?pr=46#step:5:205)? Can we try to make it so that it works gracefully across versions?

Please let me know your bandwidth and availability, and thanks a lot for your contribution!

Once this issue is addressed, I will be able to merge the PR.

@sbrudz sbrudz force-pushed the add-ruby-solargraph branch from d6f3a0d to 1b934d3 Compare January 27, 2025 15:52
@sbrudz
Copy link
Contributor Author

sbrudz commented Jan 27, 2025

Thanks for reviewing!

I've removed the part of the code that checked the .ruby-version file, so it should now work more gracefully across ruby versions.

I tested this on another repo locally and solargraph was able to handle indexing older ruby code while running via a newer ruby version without any issues.

@sbrudz
Copy link
Contributor Author

sbrudz commented Jan 27, 2025

I'll take a look at the test failure in commit 1b934d3.

@LakshyAAAgrawal
Copy link
Collaborator

We're also interested in adding ruby-lsp as an alternative server in a future PR. Not sure of the preferred way to support multiple LSP servers for the same language, though. If you have any recommendations, please let me know.

This is excellent. Please have a look at #39 , and let's discuss there the appropriate way to support multiple programming language <> multiple LSPs.

@sbrudz
Copy link
Contributor Author

sbrudz commented Jan 27, 2025

The tests should all be passing now. Let me know if you'd like me to squash some of the commits to clean things up.

I'll take a look at #39 tomorrow.

@LakshyAAAgrawal LakshyAAAgrawal merged commit 65a3aa0 into microsoft:main Jan 29, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants