Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 29, 2025

Changes: Enable LSP Go-to-Definition, Find-All-References, and Go-to-Implementation Capabilities

Allows other extensions to call into our LSP handlers, but ensures that the MEF APIs are still used for normal user invoked commands by ensuring our MEF handlers are ordered before LSP MEF handlers.

Completed:

  • Enable DefinitionProvider, ReferencesProvider, and ImplementationProvider capabilities in AlwaysActivateInProcLanguageClient.GetCapabilities() regardless of the LSP feature flag
  • Add Order(Before = ...) attribute to GoToDefinitionCommandHandler to ensure it runs before any LSP command handlers
  • Add Order(Before = ...) attribute to FindReferencesCommandHandler to ensure it runs before any LSP command handlers
  • Add Order(Before = ...) attribute to GoToImplementationCommandHandler to ensure it runs before any LSP command handlers
  • Add new constants LspGoToDefinition, LspFindReferences, and LspGoToImplementation to PredefinedCommandHandlerNames
  • Update command handler name values to exact requested strings
  • Document that LSP command handler names are externally agreed upon and cannot be changed
  • Build verified successfully

Files Modified:

  1. src/EditorFeatures/Core/LanguageServer/AlwaysActivateInProcLanguageClient.cs - Enabled DefinitionProvider, ReferencesProvider, and ImplementationProvider capabilities unconditionally
  2. src/EditorFeatures/Core/GoToDefinition/GoToDefinitionCommandHandler.cs - Added Order(Before) attribute for LSP handler
  3. src/EditorFeatures/Core/GoOrFind/FindReferences/FindReferencesCommandHandler.cs - Added Order(Before) attribute for LSP handler
  4. src/EditorFeatures/Core/GoOrFind/GoToImplementation/GoToImplementationCommandHandler.cs - Added Order(Before) attribute for LSP handler
  5. src/EditorFeatures/Core/Extensibility/Commands/PredefinedCommandHandlerNames.cs - Added LSP command handler name constants with exact values and documented they are externally agreed upon

Summary:

These changes enable clients to call the LSP go-to-definition, find-all-references, and go-to-implementation handlers by advertising the capabilities in the AlwaysActiveInProcLanguageServer. The traditional command handlers are ordered to run before any LSP-based command handlers, ensuring they remain active for normal scenarios while allowing experimentation with LSP handlers.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

… command handler ordering

Co-authored-by: dibarbet <5749229+dibarbet@users.noreply.github.com>
Copilot AI changed the title [WIP] Experiment with enabling LSP go to definition and FAR handlers Experiment: Enable LSP go-to-definition and find-all-references with command handler precedence Oct 29, 2025
Copilot AI requested a review from dibarbet October 29, 2025 20:26
Copilot finished work on behalf of dibarbet October 29, 2025 20:26
/// <summary>
/// Command handler name for LSP Go To Definition.
/// </summary>
public const string LspGoToDefinition = "LSP Go To Definition Command Handler";
Copy link

Choose a reason for hiding this comment

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

this one is "LSP GoToDefinitionCommandHandler" btw

/// <summary>
/// Command handler name for LSP Find References.
/// </summary>
public const string LspFindReferences = "LSP Find References Command Handler";
Copy link
Member

Choose a reason for hiding this comment

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

@copilot use exactly LSP FindReferenceCommandHandler as the value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to "LSP FindReferenceCommandHandler" in e37817f

Copy link
Member

Choose a reason for hiding this comment

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

can you doc that these names are agreed upon externally and cannot change?

Copy link
Member

Choose a reason for hiding this comment

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

@copilot can you update the summary documentation that these names are agreed upon externally and cannot change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated documentation for all three LSP command handler names to indicate they are agreed upon externally and cannot be changed in c63bb03

/// <summary>
/// Command handler name for LSP Go To Definition.
/// </summary>
public const string LspGoToDefinition = "LSP Go To Definition Command Handler";
Copy link
Member

Choose a reason for hiding this comment

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

@copilot use exactly LSP GoToDefinitionCommandHandler as the value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to "LSP GoToDefinitionCommandHandler" in e37817f

Co-authored-by: dibarbet <5749229+dibarbet@users.noreply.github.com>
Copilot AI requested a review from dibarbet October 29, 2025 20:35
Copilot finished work on behalf of dibarbet October 29, 2025 20:35
@hjihong
Copy link

hjihong commented Oct 30, 2025

@copilot implement the same things for the Go To Implementation capability

1 similar comment
@dibarbet
Copy link
Member

@copilot implement the same things for the Go To Implementation capability

Co-authored-by: dibarbet <5749229+dibarbet@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Oct 30, 2025

@copilot implement the same things for the Go To Implementation capability

Added Go To Implementation capability and command handler ordering in 93fcd0e. The changes include:

  • Enabled ImplementationProvider = true in AlwaysActivateInProcLanguageClient
  • Added LspGoToImplementation constant to PredefinedCommandHandlerNames
  • Added Order(Before = PredefinedCommandHandlerNames.LspGoToImplementation) to GoToImplementationCommandHandler

Copilot AI changed the title Experiment: Enable LSP go-to-definition and find-all-references with command handler precedence Experiment: Enable LSP go-to-definition, find-all-references, and go-to-implementation with command handler precedence Oct 30, 2025
Copilot finished work on behalf of dibarbet October 30, 2025 21:28
@dibarbet dibarbet changed the title Experiment: Enable LSP go-to-definition, find-all-references, and go-to-implementation with command handler precedence Enable LSP go-to-definition, find-all-references, and go-to-implementation with command handler precedence Oct 31, 2025
@dibarbet
Copy link
Member

dibarbet commented Oct 31, 2025

Tested with an additional fix on the LSP client side to ensure they are ordered after our handlers. This allows us to use VS MEF handlers for standard editor requests, but gives other extensions the ability to call our LSP handlers to retrieve definition /impl and reference information.

Current thought is to take this in dev18 to make sure that this ordering works

@dibarbet dibarbet marked this pull request as ready for review October 31, 2025 20:56
@dibarbet dibarbet requested a review from a team as a code owner October 31, 2025 20:56
…d cannot change

Co-authored-by: dibarbet <5749229+dibarbet@users.noreply.github.com>
Copilot finished work on behalf of dibarbet November 3, 2025 22:04
@dibarbet dibarbet merged commit 85dda97 into main Nov 4, 2025
25 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Nov 4, 2025
olegtk added a commit to olegtk/roslyn that referenced this pull request Nov 7, 2025
dotnet#80951 slightly broke Peek Definition by making editor pick VS LSP client implementation of IPeekableSourceProvider instead of this one. Similarly to command handlers this provider also needs to be ordered before LSP's  one.
dibarbet pushed a commit that referenced this pull request Nov 7, 2025
#80951 slightly broke Peek Definition by making editor pick VS LSP
client implementation of IPeekableSourceProvider instead of this one.
Similarly to command handlers this provider also needs to be ordered
before LSP's one.
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.

4 participants