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
Replace JTF.RunAsync() with exception-safe wrappers #11152
Conversation
}).FileAndForget(); | ||
|
||
return; | ||
SelectedDiff.InvokeAndForget(() => SelectedDiff.ViewChangesAsync(item, openWithDiffTool: OpenWithDiffTool, cancellationToken: _viewChangesSequence.Next())); |
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.
The issue with this change it makes it close to impossible to mock the threading context. The current implementation allows us to replace the static ThreadHelper
reference with an injected instance that provides JTF, but in the new implementation we won't be able to do that...
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.
The injection of a JTF instance should be no big deal: The class TaskManager
provides / encapsulates the JTF instance.
With the initial implementation, it would look like:
_taskManager.InvokeAndForget(SelectedDiff, () => SelectedDiff.ViewChangesAsync(...));
With another argument added to the extension method, it can be written as before (preferred):
SelectedDiff.InvokeAndForget(() => SelectedDiff.ViewChangesAsync(...), _taskManager);
Furthermore, I should feed every already available CancellationToken
to InvokeAndForget()
, too - but in another follow-up PR:
CancellationToken cancellationToken = _viewChangesSequence.Next();
SelectedDiff.InvokeAndForget(() => SelectedDiff.ViewChangesAsync(..., cancellationToken), _taskManager, cancellationToken);
On hold: The test execution will improve / work again after merging the 3 today's PRs. |
needs #11162 |
3decf0c
to
b48bdfb
Compare
b48bdfb
to
d6d3ad4
Compare
d6d3ad4
to
224359c
Compare
Follow-up to #11136 (and #11137 (comment))
inspired by d5b8f33
Proposed changes
JoinableTaskFactory.RunAsync()
with exception-safe wrappersFileAndForget
orInvokeAndForget
, respectivelyGitUIExtensions.InvokeSync()
andGitUIExtensions.InvokeAsync()
Screenshots
N/A
Test methodology
Please do not squash merge ❗
✒️ I contribute this code under The Developer Certificate of Origin.