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

Add hover for global variables #2691

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

snutij
Copy link
Contributor

@snutij snutij commented Oct 8, 2024

Motivation

Iteration of #2644
Handle global variables in hover requests.

Implementation

Follow the existing pattern: first, listen to on_global_variable_read_node_enter. Then, search for entries in the index with the variable name, and finally, push the documentation for entries to the response_builder.

Automated Tests

A very basic test; I tried not to be too coupled to the RBS implementation, asserting only the first three words.

Manual Tests

Write $DEBUG within a file, then hover over it. You should see documentation from the RBS declaration.

Kapture.2024-10-08.at.21.44.12.mp4

@snutij snutij requested a review from a team as a code owner October 8, 2024 19:51
@@ -13,6 +13,7 @@ class Hover
Prism::ConstantReadNode,
Prism::ConstantWriteNode,
Prism::ConstantPathNode,
Prism::GlobalVariableReadNode,
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also handle GlobalVariableWriteNode.

(there's also GlobalVariableAndWriteNode and a few others, but they are very rarely used).

Copy link
Contributor Author

@snutij snutij Oct 9, 2024

Choose a reason for hiding this comment

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

I'm wondering, after reading your feedback, if I missed this point with the indexer part as well.

# a new comment
$DEBUG = "foo"

Here, on hover, if I'm not mistaken, we want to display both comments: the one from the RBS declarations and the one from our own implementation. If yes, then I need to handle all these kinds of nodes within RubyIndexer::DeclarationListener, not only on RBS side, which would also allow features to work for user global variable like:

# a comment
$foo = "bar"

WDYT?

@andyw8 andyw8 added enhancement New feature or request server This pull request should be included in the server gem's release notes labels Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request server This pull request should be included in the server gem's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants