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

Support local variables for completion and definition requests #1518

Closed
wants to merge 2 commits into from

Conversation

bjarosze
Copy link
Contributor

Motivation

Support for completion and go to definition for local variables.

Implementation

Basic idea is that we don't have to index all local variables, because they are easy to pull off ad hoc. This way indexing time will stay the same.

Method responsible for locating local variables is Document#locate_local_variable_nodes. It returns all nodes that define local variables which are visible in given position.

Completion implementation is pretty straightforward. Just take all unique names of all local variables.
Definition jumps to closest definition node, e. g:

1| test = 1
2| test = 2
3| test = 3
4| puts test

Jumping to definition from line 4 jumps to line 3. From line 3 to line 2, etc.

Automated Tests

Added tests for both completion and definition.

Manual Tests

Write simple code like:

test = 1
puts t

You should see completion menu and then jump to definition.

@bjarosze bjarosze requested a review from a team as a code owner March 11, 2024 14:47
@bjarosze bjarosze requested review from egiurleo and KaanOzkan March 11, 2024 14:47
@KaanOzkan KaanOzkan requested a review from vinistock March 13, 2024 14:43
@KaanOzkan KaanOzkan removed their request for review April 8, 2024 14:09
Copy link
Contributor

This pull request is being marked as stale because there was no activity in the last 2 months

@github-actions github-actions bot added the Stale label Jun 10, 2024
@github-actions github-actions bot closed this Jun 24, 2024
@vinistock
Copy link
Member

Sorry for not getting back to you on time. I appreciate the PR to add support for this, but it conflicted with our current project's efforts and I ended up not being able to prioritize assisting you to push this over the finish line.

For completion, I created #2248, which uses Prism's own local variable information to suggest completions available in the current scope.

To provide hover and go to definition for locals, we will need to introduce the concept of a language server query. That will give us the ability to collect specific entities from an AST outside of indexing (to save memory and only do the work when necessary). Queries will also enable implementing the rename and references features.

We will likely take a look at queries some time soon, which should allow us to implement all of these features with a shared building block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants