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

Upgraded version of Azure/go-ansiterm #27

Merged
merged 2 commits into from
Jun 10, 2021
Merged

Upgraded version of Azure/go-ansiterm #27

merged 2 commits into from
Jun 10, 2021

Conversation

n4j
Copy link
Contributor

@n4j n4j commented Jun 8, 2021

Upgraded version of Azure/go-ansiterm from v0.0.0-20170929234023-d6e3b3328b78 to v0.0.0-20210608035416-43c61cb656b4 which fixes issue #26, kubernetes/kubernetes#102404 and docker/for-win#9770

Latest version of go-ansiterm is compatible with the stdin, stdout and stderr flags passed to winterm.GetStdFile function from package golang.org/x/sys/windows and it's also backwards compatible with syscall package as well

Upgraded version of Azure/go-ansiterm from v0.0.0-20170929234023-d6e3b3328b78 to v0.0.0-20210608035416-43c61cb656b4 which fixes issue moby#26, kubernetes/kubernetes#102404 and docker/for-win#9770

Latest version of go-ansiterm is compatible with the stdin, stdout and stderr flags passed to `winterm.GetStdFile`  function from package `golang.org/x/sys/windows` and it's also backwards compatible with `syscall` package as well

Signed-off-by: n4j <neerajx86@gmail.com>
@n4j
Copy link
Contributor Author

n4j commented Jun 8, 2021

@tonistiigi , @thaJeztah , @kolyshkin Can anyone of you please review this request?

This is the PR which fixes the issues mentioned in the PR comments

@thaJeztah
Copy link
Member

Full diff: Azure/go-ansiterm@d6e3b33...43c61cb

Thanks! I actually had docker/cli#2911 (same issue) open in my browser to look at this code

@thaJeztah
Copy link
Member

Hmmm... I think there's a bug in Azure/go-ansiterm#31. I don't think that will work as expected; left a comment on that PR Azure/go-ansiterm#31 (review)

@thaJeztah
Copy link
Member

Opened Azure/go-ansiterm#32 with a fix

@thaJeztah
Copy link
Member

Azure/go-ansiterm#32 was merged; can you update to the latest version of https://github.com/Azure/go-ansiterm ?

Signed-off-by: n4j <neerajx86@gmail.com>
@n4j
Copy link
Contributor Author

n4j commented Jun 9, 2021

@thaJeztah Done! Thanks for your diligence!!!

@thaJeztah
Copy link
Member

New diff is Azure/go-ansiterm@d6e3b33...2377c96

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@thaJeztah
Copy link
Member

@cpuguy83 @kolyshkin PTAL

Copy link
Collaborator

@dims dims left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Member

Let's get this one merged; thanks for reviewing @dims

@thaJeztah thaJeztah merged commit 9d4ed18 into moby:master Jun 10, 2021
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.

3 participants