Return the SignatureHelp items nearest to the cursor #73606
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
|
Oohh, this might fix dotnet/razor#6335 too ❤️ |
Maybe, a lot of things have changed since that issue was opened. |
src/Features/LanguageServer/Protocol/Handler/SignatureHelp/SignatureHelpHandler.cs
Outdated
Show resolved
Hide resolved
src/Features/LanguageServer/Protocol/Handler/SignatureHelp/SignatureHelpHandler.cs
Outdated
Show resolved
Hide resolved
src/Features/LanguageServer/Protocol/Handler/SignatureHelp/SignatureHelpHandler.cs
Outdated
Show resolved
Hide resolved
src/Features/CSharp/Portable/SignatureHelp/CSharpSignatureHelpService.cs
Outdated
Show resolved
Hide resolved
src/Features/CSharp/Portable/SignatureHelp/CSharpSignatureHelpService.cs
Outdated
Show resolved
Hide resolved
src/Features/CSharp/Portable/SignatureHelp/CSharpSignatureHelpService.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/SignatureHelp/SignatureHelpServiceWithProviders.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/SignatureHelp/SignatureHelpServiceWithProviders.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/SignatureHelp/SignatureHelpServiceWithProviders.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs
Outdated
Show resolved
Hide resolved
src/Features/VisualBasic/Portable/SignatureHelp/VisualBasicSignatureHelpService.vb
Outdated
Show resolved
Hide resolved
src/Features/CSharp/Portable/SignatureHelp/CSharpSignatureHelpService.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/SignatureHelp/SignatureHelpServiceWithProviders.cs
Outdated
Show resolved
Hide resolved
| @@ -174,66 +176,6 @@ private static bool DisplayPartsMatch(SignatureHelpItem i1, SignatureHelpItem i2 | |||
|
|
|||
| private static bool CompareParts(TaggedText p1, TaggedText p2) | |||
| => p1.ToString() == p2.ToString(); | |||
|
|
|||
| private static async Task<(ISignatureHelpProvider provider, SignatureHelpItems items)> ComputeItemsAsync( | |||
There was a problem hiding this comment.
Moved into the SignatureHelpService.
There was a problem hiding this comment.
i'm assuming it didn't change (beyond anything mechanical). if it did, please call that out.
There was a problem hiding this comment.
I will do a better job of this going forward. Added comments below.
src/Features/Core/Portable/SignatureHelp/ISignatureHelpService.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs
Outdated
Show resolved
Hide resolved
|
@CyrusNajmabadi Ready for another review when you have time. |
src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/SignatureHelp/SignatureHelpService.cs
Outdated
Show resolved
Hide resolved
| var extensionManager = document.Project.Solution.Services.GetRequiredService<IExtensionManager>(); | ||
|
|
||
| ISignatureHelpProvider? bestProvider = null; | ||
| SignatureHelpItems? bestItems = null; |
There was a problem hiding this comment.
i'm assuming this was a move, and didn't change.
There was a problem hiding this comment.
Ah, yeah I meant more generally the logic moved but not verbatim. On a suggestion from DavidB it uses the extension manager when invoking providers as well as inlines the IsBetter method.
| // | ||
| // Then invoking sig help will only show the items for "new Bar(" and not also | ||
| // the items for "Goo(..." | ||
| if (bestItems is not null && items.ApplicableSpan.Start < bestItems.ApplicableSpan.Start) |
There was a problem hiding this comment.
This is the moved IsBetter method logic inlined.
| var extensionManager = document.Project.Solution.Services.GetRequiredService<IExtensionManager>(); | ||
|
|
||
| ISignatureHelpProvider? bestProvider = null; | ||
| SignatureHelpItems? bestItems = null; |
There was a problem hiding this comment.
Ah, yeah I meant more generally the logic moved but not verbatim. On a suggestion from DavidB it uses the extension manager when invoking providers as well as inlines the IsBetter method.
| @@ -174,66 +176,6 @@ private static bool DisplayPartsMatch(SignatureHelpItem i1, SignatureHelpItem i2 | |||
|
|
|||
| private static bool CompareParts(TaggedText p1, TaggedText p2) | |||
| => p1.ToString() == p2.ToString(); | |||
|
|
|||
| private static async Task<(ISignatureHelpProvider provider, SignatureHelpItems items)> ComputeItemsAsync( | |||
There was a problem hiding this comment.
I will do a better job of this going forward. Added comments below.
The LSP SignatureHelpHandler only ever considered the SignatureHelpItems returned from the first provider to give us items. This change moves us to considering all providers and returning the help items with the nearest applicable span.
fixes dotnet/vscode-csharp#6670