-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Delete task extension used only by the interactive window. #76450
Conversation
@@ -41,36 +41,35 @@ internal ResetInteractive(EditorOptionsService editorOptionsService, Func<string | |||
_createImport = createImport; | |||
} | |||
|
|||
internal Task ExecuteAsync(IInteractiveWindow interactiveWindow, string title) | |||
internal async Task ExecuteAsync(IInteractiveWindow interactiveWindow, string title) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
caller of this was going an await anyways, so there was no reason i could figure out for this to be written as task returning, with task continuations, instead of just normal async/await.
// Now, we're going to do a bunch of async operations. So create a wait | ||
// indicator so the user knows something is happening, and also so they cancel. | ||
var uiThreadOperationExecutor = GetUIThreadOperationExecutor(); | ||
using var context = uiThreadOperationExecutor.BeginExecute(title, EditorFeaturesWpfResources.Building_Project, allowCancellation: true, showProgress: false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a normal using now, so we don't need an explicit dispose.
projectNamespaces, | ||
projectDirectory, | ||
platform, | ||
context).ConfigureAwait(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
intentionally CA(True) since hte original code did a ContinueWith with FromCurrentSynchronizationContext.
finally | ||
{ | ||
// Once we're done resetting focus the REPL window. | ||
ExecutionCompleted?.Invoke(this, new EventArgs()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in a finally, so we always do this regardless of cancellation (Teh semantics of SafeContinueWith).
@tmat ptal |
@dotnet/roslyn-ide @jasonmalinowski ptal |
Followup to #76448