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

App cancellation token #54

Merged
merged 1 commit into from Jan 23, 2018

Conversation

Projects
None yet
3 participants
@danielmarbach
Contributor

danielmarbach commented Nov 13, 2017

This would add cancellation token support to the async App methods

@@ -464,40 +464,47 @@ public void Show()
/// The current application directory.
/// </summary>
/// <returns></returns>
public async Task<string> GetAppPathAsync()
public async Task<string> GetAppPathAsync(CancellationToken cancellationToken = default(CancellationToken))

This comment has been minimized.

@danielmarbach

danielmarbach Nov 13, 2017

Contributor

With CSharp 7.1 support this could be reduced to = default but I didn't want to change too many things at the same time

@danielmarbach

danielmarbach Nov 13, 2017

Contributor

With CSharp 7.1 support this could be reduced to = default but I didn't want to change too many things at the same time

BridgeConnector.Socket.On("appGetAppPathCompleted", (path) =>
var taskCompletionSource = new TaskCompletionSource<string>();
using(cancellationToken.Register(() => taskCompletionSource.TrySetCanceled()))

This comment has been minimized.

@danielmarbach

danielmarbach Nov 13, 2017

Contributor

I deliberately choose here the non state based overload. This leads to closure allocations but since the BridgeConnector has the same problem I figured it is OK

@danielmarbach

danielmarbach Nov 13, 2017

Contributor

I deliberately choose here the non state based overload. This leads to closure allocations but since the BridgeConnector has the same problem I figured it is OK

Show outdated Hide outdated ElectronNET.API/App.cs
Show outdated Hide outdated ElectronNET.API/App.cs
@robertmuehsig

This comment has been minimized.

Show comment
Hide comment
@robertmuehsig

robertmuehsig Nov 13, 2017

Collaborator

Looking good - what do you think @GregorBiswanger?

Collaborator

robertmuehsig commented Nov 13, 2017

Looking good - what do you think @GregorBiswanger?

@GregorBiswanger

This comment has been minimized.

Show comment
Hide comment
@GregorBiswanger

GregorBiswanger Nov 14, 2017

Member

@danielmarbach @robertmuehsig I think this is important for a clean and solid code... Thank you @danielmarbach for your support... Can you implement it on the other Electron.cs API classes too? :)

Member

GregorBiswanger commented Nov 14, 2017

@danielmarbach @robertmuehsig I think this is important for a clean and solid code... Thank you @danielmarbach for your support... Can you implement it on the other Electron.cs API classes too? :)

@danielmarbach danielmarbach changed the base branch from develop to master Nov 20, 2017

@danielmarbach danielmarbach changed the title from WIP App cancellation token to App cancellation token Nov 20, 2017

@danielmarbach

This comment has been minimized.

Show comment
Hide comment
@danielmarbach

danielmarbach Nov 20, 2017

Contributor

App cancellation token ready for review

Contributor

danielmarbach commented Nov 20, 2017

App cancellation token ready for review

@danielmarbach

This comment has been minimized.

Show comment
Hide comment
@danielmarbach

danielmarbach Dec 18, 2017

Contributor

Should or could this be merged?

Contributor

danielmarbach commented Dec 18, 2017

Should or could this be merged?

@robertmuehsig

This comment has been minimized.

Show comment
Hide comment
@robertmuehsig
Collaborator

robertmuehsig commented Jan 18, 2018

@GregorBiswanger GregorBiswanger merged commit c8aec57 into ElectronNET:master Jan 23, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@GregorBiswanger

This comment has been minimized.

Show comment
Hide comment
@GregorBiswanger

GregorBiswanger Jan 23, 2018

Member

Thank you @danielmarbach... 👍

Member

GregorBiswanger commented Jan 23, 2018

Thank you @danielmarbach... 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment