dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.98k stars 4.03k forks source link

Extend GetDataTipText to support hover on expressions #72729

Open WardenGnaw opened 6 months ago

WardenGnaw commented 6 months ago

Background and Motivation

In order to improve the experience of hovering over expressions in Visual Studio, the proposal here is for Roslyn to provide us the text that includes the full expression and potentially the whole statement with subexpressions that we could use to evaluate.

Proposed API

we are asking for IVsTextViewFilter.GetDataTipText to be extended to include the function and its parameters in the returned pbstrText.

For example, hovering over the F in in bar.Foo("a", 1, 0x20).Bar('x', 3, Baz()), should return the text bar.Foo("a", 1, 0x20).

If we have chained method, we are looking for a new API building on top of IVsTextViewFilter.GetDataTipText that will give us the function call chain.

For example, hovering over the F in in bar.Foo("a", 1, 0x20).Bar('x', 3, Baz()), should return the text bar.Foo("a", 1, 0x20).Bar('x', 3, Baz()).

interface IVsTextViewFilter : IUnknown
{
...
+         HRESULT     GetDataTipText ([in, out] TextSpan * pSpan, bool includeDownstreamExpressions, [out] BSTR **ppbstrExpressions);
}

Usage Examples

bar.Foo("a", 1, 0x20).Bar('x', 3, Baz())
    ^ TextSpan
GetDataTipText(aspan, false, out string[] expressions);
Debug.Assert("bar", expressions[0]);
Debug.Assert("Foo(\"a\", 1, 0x20)", expressions[1]);

GetDataTipText(aspan, true, out string[] expressions);
Debug.Assert("bar", expressions[0]);
Debug.Assert("Foo(\"a\", 1, 0x20)", expressions[1]);
Debug.Assert("Bar('x', 3, Baz())", expressions[2]);

Alternative Designs

None

Risks

Not sure

CyrusNajmabadi commented 6 months ago

We should not do this with IVsTextViewFilter. It's already problematic how that API works (using legacy VS concepts in a synchronous, non-cancellable fashion). New feature work should be done with modern approaches, including:

  1. MEF
  2. Normal VS concepts (ITextSnapshot's and the like) and int positions.
  3. Async
  4. Cancellable

We could also consider this being an IAsyncEnumerable if the debugger would find it valuable to consume values one at a time. But returning an array of results is also fine.

we are asking for IVsTextViewFilter.GetDataTipText to be extended to include the function and its parameters in the returned pbstrText.

I don't think the result should be a string. We can just return an array of spans, indicating the sub-expressions of the total expression that could be independently invoked to get results to present in some fashion. This will also prevent unnecessary allocations (which could likely be very large given normal user expressions).

dibarbet commented 6 months ago

(Triaging) @WardenGnaw what is the priority on this from the debugger side?