-
Notifications
You must be signed in to change notification settings - Fork 172
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
Automatically install ruby-lsp-rails
as part of custom bundle
#1381
Conversation
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.
Let's write a PR description, but the changes are good
@vinistock I think we need a few more changes, e.g. ruby-lsp/lib/ruby_lsp/setup_bundler.rb Line 226 in c1ae8fd
|
Ah, yeah. True. We need to add |
282437b
to
ae97b57
Compare
lib/ruby_lsp/setup_bundler.rb
Outdated
) | ||
|
||
# If the user decided to add the `ruby-lsp` and `debug` to their Gemfile after having already run the Ruby LSP, | ||
# If the user decided to add `ruby-lsp`, `ruby-lsp-rails` and `debug` to their Gemfile after having already run the Ruby LSP, |
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.
I'll need to reformat some of these comments to due the the line length, but I'll wait until the review is done.
If there was a chance we might need to add a fourth gem here, then I would consider refactoring this, since we need to repeat the list of 3 in many places currently. |
ae97b57
to
3b5cf6e
Compare
Looks good to me |
(once this is merged, we can update the ruby-lsp-rails README to indicate that it doesn't need to be added to the Gemfile). |
ruby-lsp-rails
as part of custom bundle
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.
Let's add one test for the update case (which would've caught the logic issue).
2c2c236
to
67ee399
Compare
67ee399
to
dc41d3a
Compare
dc41d3a
to
662dfba
Compare
Motivation
With
ruby-lsp-rails
v0.3.0, we switched away from the mounted Rack application approach. This meansruby-lsp-rails
can now be used without having to add it to the app's Gemfile.Also closes #1267
Implementation
Previously we would install
debug
andruby-lsp
into the custom bundle. We now also installruby-lsp-rails
if the app uses Rails.Automated Tests
Updated
Manual Tests
Verified.