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

Allow more requests to be extended #808

Closed
vinistock opened this issue Jul 10, 2023 · 4 comments
Closed

Allow more requests to be extended #808

vinistock opened this issue Jul 10, 2023 · 4 comments
Assignees
Labels
addons Tasks related to Ruby LSP addons enhancement New feature or request pinned This issue or pull request is pinned and won't be marked as stale

Comments

@vinistock
Copy link
Member

vinistock commented Jul 10, 2023

In #782, we encapsulated the logic to execute multiple listeners in the base implementations. Let's generalize this even more to be implemented in the parent Listener class and let's expose a configuration to allow requests to be extended.

Something like this

class Hover < Listener
  extensible!
end

Then let's allow the following requests to be enhanced by extensions:

@vinistock vinistock added the enhancement New feature or request label Jul 10, 2023
@vinistock vinistock added this to the 2023-Q3 milestone Jul 10, 2023
@vinistock vinistock added the addons Tasks related to Ruby LSP addons label Jul 10, 2023
@vinistock vinistock added the pinned This issue or pull request is pinned and won't be marked as stale label Jul 20, 2023
@st0012 st0012 self-assigned this Aug 29, 2023
@st0012
Copy link
Member

st0012 commented Sep 11, 2023

Today at the DX stream meeting we talked about that on some requests, we may need to expose more info or even make certain data mutable to our extensions. I think semantic highlight may be one of them.

Consider this:

class Foo < ApplicationRecord
  before_save :bar
end

Currently before_save is marked as a method with no modifiers by ruby-lsp. If ruby-lsp-rails wants to enhance it by assigning modifiers (e.g. declaration), the current way of merging response won't be able to achieve it.

@vinistock can you check if my understanding on this case is correct. And if it is, should we still make it extensible before we discuss further about the data flow between ruby-lsp/extensions?

@vinistock
Copy link
Member Author

Yes, your understanding is correct. We'll indeed need to think of a better strategy if two extensions want to modify the same semantic token. Let's put this on pause while we plan of a better abstraction for merging responses from extensions.

@st0012
Copy link
Member

st0012 commented Sep 12, 2023

@vinistock vinistock modified the milestones: 2023-Q3, 2023-Q4 Oct 25, 2023
@st0012
Copy link
Member

st0012 commented Feb 21, 2024

I think this has been completed as the response building mechanism has been completely redesigned and individual requests can decide how their responses should be exposed to addons pretty easily.

@st0012 st0012 closed this as completed Feb 21, 2024
vinistock added a commit that referenced this issue Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addons Tasks related to Ruby LSP addons enhancement New feature or request pinned This issue or pull request is pinned and won't be marked as stale
Projects
None yet
Development

No branches or pull requests

2 participants