Shopify / ruby-lsp-rails

A Ruby LSP addon for Rails
https://shopify.github.io/ruby-lsp-rails/
MIT License
515 stars 21 forks source link

Option to disable `ruby-lsp-rails` server-dependent addon features [low-prio] #382

Open Splines opened 1 month ago

Splines commented 1 month ago

I have checked that this feature is not already implemented

Use case

We employ a Docker setup for our e-learning plattform MaMpf. We make use of the awesome Ruby LSP VSCode extension.

In our logs (Output pane of VSCode), we find that Server dependent features will not be available. This is reasonable due to these lines in the ruby-lsp-rails addon since a connection to our database cannot be established as the database is running inside a Docker container, while the Ruby LSP extension only accesses our local files (in a WSL environment).

Description

As we don't need the server-dependent features of Ruby LSP (*), we propose introducing an option to disable automatic bundling/activation of the ruby-lsp-rails addon/gem such that Ruby LSP does not try to communicate/instantiate our Rails app and instead only relies on static analysis of the code at hand.

(*) Of course, these additional features would be nice to have, but the static analysis is sufficient for us. Or rather, spawning a new Docker container every time one saves a Ruby file would be way too slow. And the setup would evolve quite some effort to get the Ruby LSP commands routed to the Docker container correctly, that we don't see the benefits for right now. And note that development inside a docker container as described in your VSCode extension Readme (section title "Developing on containers") is not an option for us.

Implementation

Include a check in setup_bundler.rb and make it available as flag in the VSCode settings.

Priority

For us, this issue is of low priority, as it's just a warning/error we don't want to see in the logs, but apart from that, everything works fine for us.

andyw8 commented 1 month ago

Hi @Splines, thanks for the report.

I assume you are able to run things through Docker with something like:

docker-compose run -it myapp rails runner "puts 'hello'"

So, if there was a way to customize the Rails runner command path, maybe that would help in solving this?

Splines commented 1 month ago

Yes, we can indeed run any rails commands (e.g. rails console) inside our "main" docker container using docker compose exec -it <containername> <command> (or docker compose run -it <containername> <command> to start a new container for interactively executing the specified command).

For RSpec tests, we use the Ruby Test Explorer extension and specify in its settings that it should execute a custom python script when running the tests to make sure they are executed inside a new docker test container. While the execution of the tests itself is quite fast, the startup of the container takes quite a few seconds (haven't measured, but my guess is around 15 to 20s on my i7 4 core, 16 GB RAM machine and that feels like an eternity).

That's just why I fear that even with a customized Rails runner command path for the Ruby LSP extension, we'd rather not use a "dockerized" command as I don't want to wait 15s every time I change something in a Ruby file in VSCode in order to be able to use the features of Ruby LSP. Hence, I think I'd prefer an option to only use static language features of the language server and deactivate the "dynamic" ones that rely on executing actual Ruby commands against the project. Not using Docker at all is not an option for us as it's just too much of a hassle to set up every component of our whole infrastructure individually (e.g. database, mail server and so forth...).

andyw8 commented 1 month ago

Just to clarify, for rails runner we start it but then keep it active, so you shouldn't have an issue with having to wait each time.

If you want to try that approach, you could try temporarily modify this line: https://github.com/Shopify/ruby-lsp-rails/blob/23edfb1e670494159bfbf0f6316deaa74266372a/lib/ruby_lsp/ruby_lsp_rails/runner_client.rb#L55

We previously did offer a way to disable the server addon features via an initializer but we deprecated it when we changed the server approach in https://github.com/Shopify/ruby-lsp-rails/pull/256. We can considered making that option available again if necessary.

Splines commented 1 month ago

Thanks for your support, the following modification of this line works for us:

stdin, stdout, stderr, wait_thread = Bundler.with_original_env do
    Dir.chdir("./docker/development/") do
        Open3.popen3("docker", "compose", "run", "--rm", "-T",
            '--entrypoint=""', "-v", "#{__dir__}/server.rb:/root/tmp/ruby-lsp-server.rb",
            "mampf", "rails", "runner", "/root/tmp/ruby-lsp-server.rb", 'start')
    end
end

The container is called mampf. I had to memory-map the server.rb into the Docker container via the -v option. After restarting the Ruby LSP server, I can see that a new docker container is created (it's spun up really quickly). After that, I hovered over an ActiveRecord model in our codebase and can now see the schema definition of it 🙌

So, if there was a way to customize the Rails runner command path, maybe that would help in solving this?

Yes, that'd be awesome. Note that we'd have to change directory inside such a command (as done via Dir.chdir above).

Splines commented 1 month ago

Note that the feature Jump to the declaration of a route seems to not work inside our config/routes.rb file. Is Ctrl + Click supposed to work here?

andyw8 commented 1 month ago

@Splines you should be able to Go To Definition from where the route is called, e.g. users_path or users_url in a controller, and it will take to routes.rb. But perhaps this line would not work with your Docker setup, since the path will be for the Docker instance and not your local filesystem.

Splines commented 1 month ago

Unfortunately, it fails due to the condition ActionDispatch::Routing::Mapper.respond_to?(:route_source_locations) && ActionDispatch::Routing::Mapper.route_source_locations not being satisfied. And yes, the line you mentioned would probably return an incorrect path that one would have to translate back to the local filesystem path.

I have to be honest and say that the features I deem relevant for a nice development experience already come shipped with Ruby LSP, even without ruby-lsp-rails, i.e. getting semantic highlighting, jumping to definitions, seeing an outline, getting documentation links etc. While the extra features provided by Ruby LSP Rails would be nice to have, personally I can live without them. Having setup our app in Docker containers, it's of course more effort to get Ruby LSP Rails to work nicely if communication has to happen with the Rails app.

We previously did offer a way to disable the server addon features via an initializer but we deprecated it when we changed the server approach in https://github.com/Shopify/ruby-lsp-rails/pull/256. We can considered making that option available again if necessary.

I feel like that'd be the preferred way for us based on my first little experiments here. If you could make this option available again somehow, it'd be much appreciated. For now, we will continue to ignore the Server dependent features will not be available message and the accompanying error in the logs.

andyw8 commented 1 month ago

(I'll move this to the ruby-lsp-rails repo since that's where the change would be needed)

andyw8 commented 1 month ago

@Splines we discussed this within the team recently. Have you looked into using the Docker extension with VS Code? We suspect that may be the 'proper' solution for the issue here.

Dreikantbrot commented 1 month ago

Just throwing in my 2 cents here, since I would like to see some way to disable ruby-lsp-rails, too.

In my case we use PostgreSQL in development, since we depend on some of its more specific features. Every time we need to completely reset the database (via db:reset or some other custom Rake task), the database cannot be dropped, due to ruby-lsp-rails still maintaining a connection to the DB. So, for now, everytime we need to do that, we need to stop the entire Ruby LSP server via VSCode manually just do reset the database, which can become quite tedious in some cases.

I would rather forego the features ruby-lsp-rails offers altogether, but I'd still like to use Ruby LSP itself, since it fixes some annoying syntax highlighting issues of other providers such as Solargraph.