Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add UI for Value Tracking #51974

Merged
8 commits merged into from
Mar 19, 2021

Conversation

ryzngard
Copy link
Contributor

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

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
Member

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