Skip to content

Add UI for Value Tracking#51974

Merged
8 commits merged intodotnet:features/value_trackingfrom
ryzngard:feature/value_tracking/ui
Mar 19, 2021
Merged

Add UI for Value Tracking#51974
8 commits merged intodotnet:features/value_trackingfrom
ryzngard:feature/value_tracking/ui

Conversation

@ryzngard
Copy link
Contributor

This adds the start of the UI for value tracking. Builds off of #51898

@ryzngard ryzngard requested a review from a team as a code owner March 19, 2021 00:24
@ryzngard ryzngard requested a review from a team March 19, 2021 00:24
@ghost ghost added the Area-IDE label Mar 19, 2021
return false;
}

var selectedSymbol = _threadingContext.JoinableTaskFactory.Run(() => GetSelectedSymbolAsync(textSpan, document, cancellationToken));
Copy link
Contributor

Choose a reason for hiding this comment

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

Have we talked to the platform folks about it being possible to have an async command handler so we don't need to use JTF for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I like this a lot better. It being fire-and-forget seems fine compared to blocking the caller on async work.

private readonly ValueTrackingTreeViewModel _viewModel;

// Needed for VSSDK003
// See https://github.com/Microsoft/VSSDK-Analyzers/blob/main/doc/VSSDK003.md
Copy link
Contributor

Choose a reason for hiding this comment

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

We should make sure that we eventually change the tool window to be async. Synchronous tool window construction is surprisingly expensive.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, plus async tool windows are not difficult to implement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I fixed this. I was close to having it, didn't realize I should just not block on window show. I followed https://github.com/madskristensen/AsyncToolWindowSample/

Copy link
Contributor

@jmarolf jmarolf left a comment

Choose a reason for hiding this comment

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

:shipit:

@MartyIX
Copy link
Contributor

MartyIX commented Mar 19, 2021

Could anyone tell me what this feature is about? It sounds like something I'm interested in one of my analyzers. Thanks to a kind soul in advance! :)

@ryzngard
Copy link
Contributor Author

@MartyIX this is related to #25994

This isn't the final UI, we're slowly making progress on this feature into a feature branch. Let me know if you have any questions!

@MartyIX
Copy link
Contributor

MartyIX commented Mar 19, 2021

Thanks Andrew! I will check the source code, it looks useful for my scenario :-)

@ryzngard
Copy link
Contributor Author

I'm going to merge this, but feel free to leave more feedback and I'll address in a separate PR

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

@ghost ghost merged commit e4455e0 into dotnet:features/value_tracking Mar 19, 2021
This pull request was closed.
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.

4 participants