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

Only display code lens when the test framework is rails #318

Merged
merged 2 commits into from
Apr 3, 2024

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Apr 3, 2024

Currently, ruby-lsp-rails also sends code lens even when the test library is not rails. This creates problem when the user has both ruby-lsp-rspec and ruby-lsp-rails installed and the test framework is rspec, as the duplicated code lens will be cause none of them to display.

I also removed obsolete test setups that create/remove message queues manually. They're not used after the with_server is adopted.

Currently, ruby-lsp-rails also sends code lens even when the test
library is not rails. This creates problem when the user has both
ruby-lsp-rspec and ruby-lsp-rails installed and the test framework
is rspec, as the duplicated code lens will be cause none of them to display.
@st0012 st0012 added the bugfix This PR fixes an existing bug label Apr 3, 2024
@st0012 st0012 self-assigned this Apr 3, 2024
@st0012 st0012 requested a review from a team as a code owner April 3, 2024 16:34
@st0012 st0012 requested review from andyw8 and Morriar April 3, 2024 16:34
@@ -47,6 +47,8 @@ def deactivate
).void
end
def create_code_lens_listener(response_builder, uri, dispatcher)
return unless T.must(@global_state).test_library == "rails"
Copy link
Member Author

Choose a reason for hiding this comment

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

I decided to skip the entire listener because it's only used for test code lends atm.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's probably fine for now, but we'll need to re-think for @gmcgibbon's work in #239

Copy link
Member Author

Choose a reason for hiding this comment

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

I may be worth creating a separate listener for that feature now that we have conditions like this.

@st0012 st0012 merged commit 4e50add into main Apr 3, 2024
54 checks passed
@st0012 st0012 deleted the display-code-lens-conditionally branch April 3, 2024 16:44
@vinistock
Copy link
Member

We still need to improve the test framework detection logic in the Ruby LSP to fully fix the Ruby LSP RSpec issue though, right?

@st0012
Copy link
Member Author

st0012 commented Apr 3, 2024

@vinistock Yeah that's true. I'm working on it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants