Skip to content

Move code back from quick info to SymbolDisplay.#51961

Merged
CyrusNajmabadi merged 9 commits intodotnet:mainfrom
CyrusNajmabadi:symbolDisplay
Mar 20, 2021
Merged

Move code back from quick info to SymbolDisplay.#51961
CyrusNajmabadi merged 9 commits intodotnet:mainfrom
CyrusNajmabadi:symbolDisplay

Conversation

@CyrusNajmabadi
Copy link
Contributor

This will help with #51846, removing unnecessary copying between xaml and Roslyn's QI providers.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner March 18, 2021 18:00
@ghost ghost added the Area-IDE label Mar 18, 2021
AddDocumentationContent(firstSymbol);
}

private void AddDocumentationContent(ISymbol symbol)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moving back from qi to symbol-display.

var sections = ImmutableArray.CreateBuilder<QuickInfoSection>(initialCapacity: groups.Count);

void AddSection(string kind, ImmutableArray<TaggedText> taggedParts)
=> sections.Add(QuickInfoSection.Create(kind, taggedParts));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

helper local functions moved to bottom.

{
var result = new Dictionary<SymbolDescriptionGroups, ImmutableArray<TaggedText>>(_documentationMap);
foreach (var (group, parts) in _groupMap)
result[group] = parts.ToTaggedText();
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this. Why are we replacing documentation text with symbol text?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there are two maps. one that goes from group->symboparts, and one that goes from group->taggedtext. the maps cover different groups. So this just produces a final merged dictionary that is entirely from group->taggedtext.


var remarksDocumentationContent = GetRemarksDocumentationContent(workspace, symbol, groups, semanticModel, token, formatter, cancellationToken);
if (!remarksDocumentationContent.IsDefaultOrEmpty)
var remarksDocumentation = GetRemarksDocumentationContent(workspace, groups, semanticModel);
Copy link
Member

@genlu genlu Mar 18, 2021

Choose a reason for hiding this comment

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

nit: might as well just inline this since it's a very simple method and to be consistent with code below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure.

Copy link
Member

@genlu genlu left a comment

Choose a reason for hiding this comment

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

LGTM, except the one question I have

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Auto-approval

@CyrusNajmabadi CyrusNajmabadi merged commit ad3ecc5 into dotnet:main Mar 20, 2021
@CyrusNajmabadi CyrusNajmabadi deleted the symbolDisplay branch March 20, 2021 09:11
@ghost ghost added this to the Next milestone Mar 20, 2021
@allisonchou allisonchou modified the milestones: Next, 16.10.P2 Mar 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants