Skip to content
This repository has been archived by the owner on Sep 26, 2021. It is now read-only.

Fix tty errors using winpty #221

Merged
merged 1 commit into from Oct 7, 2015
Merged

Fix tty errors using winpty #221

merged 1 commit into from Oct 7, 2015

Conversation

jmorganca
Copy link
Contributor

Fixes #183 and #220. Creates an alias for docker that uses winpty when using the Docker Quickstart Terminal.

Credit for fix: @gmanfunky. Thank you!

cc @nathanleclaire @mchiang0610. What are your thoughts?

Interestingly enough, this also works for docker-compose @aanand @dnephin

Signed-off-by: Jeffrey Morgan <jmorganca@gmail.com>
@nathanleclaire
Copy link
Contributor

Very interesting... would be good to get a chance to try it out so I'll have to look into making a test build. I don't know much about winpty so it's hard to comment at a high level.

@gmanfunky
Copy link

winpty comes with the Git for Windows 2.5 distro we are using. It convinces unix-leaning interactive console commands to play more nicely with some terminals such as the mintty git-bash.exe uses.

See: https://github.com/rprichard/winpty
and git-for-windows/build-extra@ceda40c

I believe this is a sane workaround for what might ultimately be fixable by using different golang console routines in docker.

I'll give it some more good exercise tomorrow too.

cd

docker () {
winpty docker $@

Choose a reason for hiding this comment

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

Not exactly sure why, but the Git for Windows peeps explicitly call program.exe in their winpty convenience aliases. (git-for-windows/build-extra@ceda40c)

It might be less likely to run into a conflict with any other docker aliases users may set.

Do you think we should follow the {name}.exe convention?

@aanand
Copy link

aanand commented Oct 7, 2015

Very interesting. Is it as simple as running winpty <command>? I wonder if it would fix interactive docker-compose run.

@thaJeztah
Copy link
Member

Will this resolve moby/moby#12469 as well? (It's a long thread, so I don't know if all cases are covered here)

@jmorganca
Copy link
Contributor Author

I'll merge this and update 1.8.2d to use it.

I've done some pretty extensive testing of this on windows with both the docker and docker-compose windows builds and it seems to work well. @gmanfunky has done quite a bit of testing as well. We can always revert if there's a show stopper.

jmorganca pushed a commit that referenced this pull request Oct 7, 2015
Fix tty errors using winpty
@jmorganca jmorganca merged commit f250c71 into docker-archive:master Oct 7, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants