Skip to content

Add preferPrimaryDefinition setting for go to definition - #4169

Closed
sucicfilip wants to merge 1 commit into
Shopify:mainfrom
sucicfilip:feature/prefer-primary-definition
Closed

Add preferPrimaryDefinition setting for go to definition#4169
sucicfilip wants to merge 1 commit into
Shopify:mainfrom
sucicfilip:feature/prefer-primary-definition

Conversation

@sucicfilip

Copy link
Copy Markdown

Motivation

In apps that reopen classes across many files (packwerk monoliths, engines, etc.), go to definition on a constant always opens a picker listing every file where the class is reopened. Most of the time I just want the file that actually defines the class, and picking it out of the list gets tedious fast.

Implementation

Adds an opt-in featuresConfiguration.definition.preferPrimaryDefinition setting, wired through the existing featuresConfiguration mechanism in GlobalState.

When the setting is enabled and a constant resolves to multiple namespace entries, the definition listener keeps only the entries whose file path matches the fully qualified constant name following the Zeitwerk convention. For example, in this repository RubyIndexer is reopened in over 20 files, but only lib/ruby_indexer/ruby_indexer.rb matches the convention, so that is the single definition returned. If no entry matches the convention, all entries are returned as before. Methods and plain constants are not affected. The filter only applies to classes and modules.

Automated Tests

Added two tests to DefinitionExpectationsTest: one asserting that only the convention-matching file is returned when the setting is on, and one asserting the fallback to all entries when no file matches.

Manual Tests

  1. Point the VS Code extension at this branch (e.g. "rubyLsp.bundleGemfile" or a path: entry in your project's Gemfile)
  2. Add "rubyLsp.featuresConfiguration": { "definition": { "preferPrimaryDefinition": true } } to your settings and restart the Ruby LSP
  3. Open this repository and trigger go to definition on a RubyIndexer reference (it is reopened in over 20 files under lib/). You should land directly in lib/ruby_indexer/ruby_indexer.rb instead of getting a picker
  4. Disable the setting and repeat. You should get the usual picker with every definition

@sucicfilip
sucicfilip force-pushed the feature/prefer-primary-definition branch from 489faf2 to 0c1ebd6 Compare July 20, 2026 11:35
@sucicfilip
sucicfilip marked this pull request as ready for review July 20, 2026 11:36
@sucicfilip
sucicfilip requested a review from a team as a code owner July 20, 2026 11:36
@sucicfilip
sucicfilip force-pushed the feature/prefer-primary-definition branch from 0c1ebd6 to 4da6df0 Compare July 23, 2026 12:14
@vinistock

Copy link
Copy Markdown
Member

Thank you for the proposal. While, I understand the point you make in the description and am familiar with Rails'/Zeitwerk's convention, from Ruby's perspective there's no distinction between the different definitions and the file name is irrelevant. Additionally, the current behaviour is also consistent with language servers for other languages.

For someone working on multiple Ruby projects, say a few gems and some Rails apps, forgetting that the setting is enabled or working on a workspace where it's configured in .vscode/settings.json could be confusing as it might appear like the LSP is not correctly identifying valid definitions of the same class.

It also creates a discrepancy between the features. If we omit other definitions in go to definition, why should they continue to appear on hover or completionItem/resolve? It ends up being a choice between features that don't seem to match or completely ignoring a valid definition, which makes it seem like the analysis is incorrect.

If it was possible to sort the results and put the matching file name first, that'd be a nicer solution, but the specification doesn't allow for that since you can have multiple language servers connected at the same time and editors wouldn't know how to order the results between them.

@vinistock vinistock closed this Jul 24, 2026
@sucicfilip

Copy link
Copy Markdown
Author

Ah, I understand. Thank you for the clarification. I tried out Rubymine and got used to this, but ultimately came back to Zed + Ruby LSP😄

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants