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

Include comments when dispatching listener events #2054

Open
vinistock opened this issue May 16, 2024 · 4 comments
Open

Include comments when dispatching listener events #2054

vinistock opened this issue May 16, 2024 · 4 comments
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

There are scenarios where addons might need to have access to comments that are close to nodes they are interested in.

For example, for RBS inline annotations, an addon could provide completion, go to definition, hover and semantic highlighting. But currently, the listeners cannot receive any comments content, because the comments are separate from the AST in Prism's parse result.

I believe the easiest way to provide this functionality to listeners might be to create a custom dispatcher that stores the comment information and passes that along to listeners in a way that allows them to easily access it.

Scenario: imagine proper highlighting and the ability to navigate through constants in these inline RBS annotations.

class Foo
  attr_reader :name # ::String

  # :: (String, Integer?) -> void
  def bar(a, b)
  end
end
@vinistock vinistock added enhancement New feature or request addons Tasks related to Ruby LSP addons pinned This issue or pull request is pinned and won't be marked as stale labels May 16, 2024
@Super-Xray
Copy link
Contributor

image
Excuse me, it seems that this function has been implemented, though it can only read the annotation which at specific position('test', but not '::String').

@vinistock
Copy link
Member Author

Sorry, that's unrelated. We decided to only index documentation on top of declaration for performance reasons, but one thing we'd like to explore is to make documentation fetching lazy (which will also save lots of memory in the index).

This issue is related to addon listeners not having any means to access comment contents to provide other features, like allowing you to go to definition on a type defined in a comment.

@Super-Xray
Copy link
Contributor

Super-Xray commented Jun 18, 2024

Thanks for your explanation, so this issue's work is about saving memory by fetching lazy in this hover function, in order to support other functions (in the future) such as go to definition on a type defined in a comment?

I guess the related code is at https://github.com/Shopify/ruby-lsp/blob/main/lib/ruby_lsp/requests/support/common.rb#L92 and https://github.com/Shopify/ruby-lsp/blob/main/lib/ruby_lsp/listeners/hover.rb#L97. It seems like the comment come from @index.resolve_method(message, @node_context.fully_qualified_name). Is it right? I'm interested in this work.

@vinistock
Copy link
Member Author

this issue's work is about saving memory by fetching lazy in this hover function

No. Currently, listener objects that register with Prism::Dispatcher can only handle node events, like on_call_node_enter. However, they have no way of accessing comments, because they aren't nodes and we don't give listeners access to comments in any way.

That prevents addons from being able to add functionality based on comments. In the example of the PR description, a Steep addon for type checking is currently unable to provide go to definition in an inline comment signature, because it has no access to any comments.

The work to make comment fetching lazy is a different effort.

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