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 completion support for locals #2248

Merged
merged 1 commit into from
Jul 11, 2024
Merged

Conversation

vinistock
Copy link
Member

Motivation

Add completion support for locals.

Implementation

There are two parts to the implementation:

  1. We need to start remembering the full nesting nodes inside node context. This is needed because we need to traverse the exact surrounding scopes to determine which locals are available inside the target
  2. Started including local variables as part of method completion. This may seem counter intuitive, but until you finish typing the entire name of the local variable, it is actually considered as a method call (since Prism hasn't yet matched it to a local)

Automated Tests

Added tests.

Manual Tests

Launch the LSP on this branch and verify you get locals as completion suggestions.

Screen.Recording.2024-07-02.at.5.55.03.PM.mov

@vinistock vinistock added enhancement New feature or request server This pull request should be included in the server gem's release notes labels Jul 2, 2024
@vinistock vinistock self-assigned this Jul 2, 2024
@vinistock vinistock requested a review from a team as a code owner July 2, 2024 21:59
@vinistock vinistock requested review from andyw8 and st0012 July 2, 2024 21:59
@vinistock vinistock force-pushed the vs/support_local_completion branch from 7f3d6d1 to 44dadfa Compare July 2, 2024 22:00
@vinistock vinistock force-pushed the vs/support_local_completion branch from 44dadfa to c05f9d9 Compare July 4, 2024 20:35
@vinistock vinistock force-pushed the vs/support_local_completion branch from c05f9d9 to d25280e Compare July 5, 2024 20:41
Copy link

@amomchilov amomchilov left a comment

Choose a reason for hiding this comment

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

Neat!


@node_context.locals_for_scope.each do |local|
local_name = local.to_s
next unless local_name.start_with?(name)

Choose a reason for hiding this comment

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

Future feature idea: fuzzy matching?

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 think VS Code already does that. There are two ways to return completion items: a complete list or an incomplete one.

For the incomplete one, the server is responsible for re-computing all candidates one very keystroke (in this scenario, fuzzy matching on the server would indeed be very nice).

For the complete one, we return all matches for the first character and the client (VS Code) will handle the fuzzy matching on all candidates, which is what we currently do for performance reasons. Anything we can delegate to the client means less work for the server.

@vinistock vinistock merged commit 816e188 into main Jul 11, 2024
35 checks passed
@vinistock vinistock deleted the vs/support_local_completion branch July 11, 2024 14:17
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