XAML QuickInfo update #51846
XAML QuickInfo update #51846
Conversation
|
Can you add before/after screenshots? |
|
@sharwell Done. See updated description. |
|
Nice! Does the hyperlinking work as well? |
|
No, it does not. This is for the LSP Hover handler, which I don't think supports that. |
| @@ -53,16 +52,51 @@ public static async Task<IEnumerable<TaggedText>> GetDescriptionAsync(this ISymb | |||
| return Enumerable.Empty<TaggedText>(); | |||
| } | |||
|
|
|||
| var textContentBuilder = new List<TaggedText>(); | |||
| textContentBuilder.AddRange(symbol.ToDisplayParts(s_descriptionStyle).ToTaggedText()); | |||
| var description = await symbolDisplayService.ToDescriptionPartsAsync(codeProject.Solution.Workspace, semanticModel, 0, ImmutableArray.Create(symbol), SymbolDescriptionGroups.MainDescription, cancellationToken).ConfigureAwait(false); | |||
|
|
||
| var documentation = symbol.GetDocumentationParts(semanticModel, offset, formatter, cancellationToken); | ||
| var documentation = symbol.GetDocumentationParts(semanticModel, 0, formatter, cancellationToken); |
| builder.AddRange(documentation); | ||
| } | ||
|
|
||
| var remarksDocumentation = symbol.GetRemarksDocumentationParts(semanticModel, 0, formatter, cancellationToken); |
There was a problem hiding this comment.
i'm surprpised you need to write this yourself. if you use ToDescriptionGroupsAsync, you should have a documentation group already setup for you that you can use. does that not work?
There was a problem hiding this comment.
Nope. I would have expected ToDescriptionPartsAsync with SymbolDescriptionGroups.All to give me everything but it does not, so I limited it to MainDescription and called the rest myself.
|
Overall, this looks great. Just a general question about if we can make it even simpler by leveraging more of what ISYmbolDisplayService gives you :) |
|
Thanks @CyrusNajmabadi. I agree ISymbolDisplayService should have been able to do all this for me. Right now, it does not make a difference if I pass in All groups or just MainDescription to ToDescriptionPartsAsync, both just give me the full symbol name. |
|
Thanks @CyrusNajmabadi. I agree ISymbolDisplayService should have been able to do all this for me. Right now, it does not make a difference if I pass in All groups or just MainDescription to , both just give me the full symbol name. |
|
@sharwell, @CyrusNajmabadi, anything else you need before signing off? |
|
Sorry. I need to look into this. I have to understand why we can't currently share more. |
|
@CyrusNajmabadi How about we go with this for now and I open an issue? |
|
looking now. |
|
I've opened #51961 to help out here. You'll be able to just grab the groups and build the total docs. |
|
Thanks @CyrusNajmabadi! I'll wait for it. Looks like #51961 has some (unrelated?) test failures though. |
|
@mgoertz-msft my pr went in. So you should be able to use that and just combine the groups without having to rebuild the internal structure. Thanks! |
|
@CyrusNajmabadi Shouldn't I be able to call ToDescriptionPartsAsync with DescriptionGroups.All to get everything? That still doesn't work because the builder only puts the MainDescription into the _groupsMap and the rest into the _documentationMap. |
|
Call ToDescriptionGroupsAsync instead. That gives you the merged result. You shouldn't need SymbolDisplayParts (i hope) for what you're doing. TaggedText is the better, and richer, medium for doing things like QuickInfo. |
|
I would have just called ToTaggedText() on the result. With ToDescriptionGroupsAsync I have to iterate over all the groups and merge the results myself. Isn't it still a bug that ToDescriptionPartsAsync takes a group flag as an argument but only ever supports the MainDescription? |
We can make an extension that does that for you if htat would help :)
In this case no. Because we can't actually make SymbolDisplayParts for these other sections. Specifically, these other sections use features of TaggedText that are not representable in the SYmbolDisplayPart world. SymbolDisplayParts are a subset of TaggedText. This is very much because this is an api using both compiler and IDE primitives, and it has evolved a lot over the years. it likely could be cleaned up, but it was internal and we never had the need. |
|
i think you'll want to extract the code in CommonSemanticQuickInfoProvider.CreateContentAsync where we grab all this data and merge it together and then use that common code for your new provider. |
|
actually, let me try to provide the helper. i'll make a pr for thsi. |
…layService and adding more documentation parts.
mgoertz-msft
force-pushed
the
dev/mgoertz/quickinfo
branch
from
b218ea1
to
202e9a9
Compare
|
@CyrusNajmabadi New QuickInfoUtilities helper works great, thanks, but I had to insert line breaks between the sections. |
src/VisualStudio/Xaml/Impl/Implementation/LanguageServer/Extensions/SymbolExtensions.cs
Outdated
Show resolved
Hide resolved
…sions/SymbolExtensions.cs Co-authored-by: CyrusNajmabadi <cyrus.najmabadi@gmail.com>
ghost
merged commit 
ghost
added this to the 
Updated XAML QuickInfo to show more info like C# by using ISymbolDisplayService and adding more documentation parts.
Before:
After:
