Skip to content

Migrate workspace symbol to use Rubydex#4019

Open
vinistock wants to merge 1 commit into03-03-migrate_go_to_definition_to_use_rubydexfrom
03-19-migrate_workspace_symbol_to_rubydex
Open

Migrate workspace symbol to use Rubydex#4019
vinistock wants to merge 1 commit into03-03-migrate_go_to_definition_to_use_rubydexfrom
03-19-migrate_workspace_symbol_to_rubydex

Conversation

@vinistock
Copy link
Member

Motivation

This PR migrates workspace symbol to use Rubydex.

Implementation

  1. Started using Graph#search to match symbols
  2. After doing some research, I realized that no language server excludes private/protected symbols from the results. Additionally, there's a proposal open to include visibility as a new field in workspace symbols. I think excluding based on visibility was not the right choice, so I removed that filter
  3. I added the to_lsp_kind to our definition patches

Automated Tests

I removed a redundant test and expanded the one about private/protected definitions.

@vinistock vinistock requested review from alexcrocha and st0012 March 19, 2026 16:47
@vinistock vinistock self-assigned this Mar 19, 2026
@vinistock vinistock requested a review from a team as a code owner March 19, 2026 16:47
@vinistock vinistock added server This pull request should be included in the server gem's release notes other Changes that aren't bugfixes, enhancements or breaking changes labels Mar 19, 2026
@vinistock vinistock mentioned this pull request Mar 19, 2026
20 tasks
@vinistock vinistock force-pushed the 03-19-migrate_workspace_symbol_to_rubydex branch from b182366 to 23fc40e Compare March 20, 2026 20:03
@vinistock vinistock force-pushed the 03-03-migrate_go_to_definition_to_use_rubydex branch from 005200d to f3b0fcc Compare March 20, 2026 20:04
@global_state = global_state
@query = query
@index = global_state.index #: RubyIndexer::Index
@graph = global_state.graph #: Rubydex::Graph
Copy link
Member

Choose a reason for hiding this comment

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

Since we don't need @global_state anymore, does it make sense to just pass graph in?

declaration.definitions.each do |definition|
location = definition.location
uri = URI(location.uri)
file_path = uri.full_path
Copy link
Member

Choose a reason for hiding this comment

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

Not blocking, but does it make sense to make this Definition#full_path?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

other Changes that aren't bugfixes, enhancements or breaking changes 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