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.

Sign up for GitHub

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

Move listener registration to the extension activation #106

Closed
wants to merge 1 commit into from
Closed

Move listener registration to the extension activation #106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions lib/ruby_lsp/ruby_lsp_rails/code_lens.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ class CodeLens < ::RubyLsp::Listener
ResponseType = type_member { { fixed: T::Array[::RubyLsp::Interface::CodeLens] } }
BASE_COMMAND = "bin/rails test"

::RubyLsp::Requests::CodeLens.add_listener(self)

sig { override.returns(ResponseType) }
attr_reader :response

Expand Down
3 changes: 3 additions & 0 deletions lib/ruby_lsp/ruby_lsp_rails/extension.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ class Extension < ::RubyLsp::Extension

sig { override.void }
def activate
::RubyLsp::Requests::Hover.add_listener(RubyLsp::Rails::Hover)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we only register classes and not instances, I wonder if this could be moved in the Listener itself:

class RubyLsp::Rails::CodeLens < ::RubyLsp::Listener
  listen_to ::RubyLsp::Requests::CodeLens

  # ...
end

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an interesting approach, but it would lead to the same problem we're trying to avoid in this PR.

In order to disable an extension and avoid registering the listeners defined by it, we need the registration to happen in activate.

Basically, if the server extension is disabled, we just don't invoke activate. If the listener classes register themselves, then that will happen automatically when we require the Extension file and we'll have no way of stopping it.

::RubyLsp::Requests::CodeLens.add_listener(RubyLsp::Rails::CodeLens)

RubyLsp::Rails::RailsClient.instance.check_if_server_is_running!
end

Expand Down
2 changes: 0 additions & 2 deletions lib/ruby_lsp/ruby_lsp_rails/hover.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ class Hover < ::RubyLsp::Listener

ResponseType = type_member { { fixed: T.nilable(::RubyLsp::Interface::Hover) } }

::RubyLsp::Requests::Hover.add_listener(self)

sig { override.returns(ResponseType) }
attr_reader :response

Expand Down
2 changes: 2 additions & 0 deletions test/ruby_lsp_rails/code_lens_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ module RubyLsp
module Rails
class CodeLensTest < ActiveSupport::TestCase
setup do
::RubyLsp::Requests::CodeLens.add_listener(RubyLsp::Rails::CodeLens)
@message_queue = Thread::Queue.new
@store = RubyLsp::Store.new
end

def teardown
::RubyLsp::Requests::CodeLens.listeners.clear
T.must(@message_queue).close
end

Expand Down
3 changes: 3 additions & 0 deletions test/ruby_lsp_rails/extension_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ class ExtensionTest < ActiveSupport::TestCase
RubyLsp::Rails::RailsClient.stubs(instance: rails_client)
extension = Extension.new
assert_predicate(extension, :activate)
ensure
::RubyLsp::Requests::Hover.listeners.clear
::RubyLsp::Requests::CodeLens.listeners.clear
end
end
end
Expand Down