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

Command.ExecuteAsync stdout and stderr content now gets returned #58

Merged
merged 4 commits into from Sep 1, 2023

Conversation

YanikCeulemans
Copy link
Contributor

This is an initial attempt at fixing the Command.ExecuteAsync not properly returning the the stdout content. I have updated the tests to return Async<Unit> so the results are awaited.

Fixes #57

fixed command execute async not returning stdout content.
@YanikCeulemans
Copy link
Contributor Author

I have not yet tested the failure cases when the command fails to execute or times out given a cancellation token.

Please feel free to comment / edit wherever necessary.

Thanks in advance for your time.

@CaptnCodr
Copy link
Owner

CaptnCodr commented Sep 1, 2023

Thank you for your PR.
After reviewing and trying out and running tests locally the CancelAfter operation doesn't work anymore.
Handlers OutputDataReceived and ErrorDataReceived were added for this purpose.

Do you have another idea how to solve this?

@CaptnCodr
Copy link
Owner

I have something here. I apply it to this PR soon.

CaptnCodr and others added 2 commits September 1, 2023 10:49
Co-Authored-By: Yanik Ceulemans <4895411+yanikceulemans@users.noreply.github.com>
@CaptnCodr
Copy link
Owner

CaptnCodr commented Sep 1, 2023

Does this look good to you?

@YanikCeulemans
Copy link
Contributor Author

Looks good to me! I ran the tests on my macOS machine and they all passed.

Thank you very much for your time and effort.

@CaptnCodr CaptnCodr merged commit 09f2b69 into CaptnCodr:main Sep 1, 2023
3 checks passed
@CaptnCodr
Copy link
Owner

@YanikCeulemans this fix has been deployed in v1.10.1 and is already available at Nuget.
Thank you for your contribution! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Command.executeAsync does not await output
2 participants