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

Refactor exec() wrapper to handle Unicode #141

Closed
wants to merge 1 commit into from
Closed

Refactor exec() wrapper to handle Unicode #141

wants to merge 1 commit into from

Conversation

dimaqq
Copy link

@dimaqq dimaqq commented Jun 22, 2022

Hopefully fixes #140
I'm assuming that execImpl doesn't reuse the buffers they pass to the listeners.
If it does, a different solution is needed.

OTOH, maybe the action can use getExecOutput from https://github.com/actions/toolkit/blob/main/packages/exec/src/exec.ts directly?

Hopefully fixes #140 
I'm assuming that `execImpl` doesn't reuse the buffers they pass to the listeners.
If it does, a different solution is needed.
@dimaqq
Copy link
Author

dimaqq commented Oct 6, 2022

@dorny poke 🙏🏼

@dorny
Copy link
Owner

dorny commented Oct 11, 2022

Thanks @dimaqq and sorry for my very late reply. I was inactive on the project for some time.
Using getExecOutput seems to be the correct way. The function was not available at the time this action was created.
I will do some quick tests with it.

@dorny
Copy link
Owner

dorny commented Oct 11, 2022

The getExecOutput works well. Thanks for the suggestion :)
Merged in #162 so I'm closing this PR now.

@dorny dorny closed this Oct 11, 2022
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.

Possibly bad unicode handling in exec() function wrapper
2 participants