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

term/winconsole: Identify tty correctly, fix resize problem #11840

Merged
merged 1 commit into from Mar 26, 2015

Conversation

Projects
None yet
4 participants
@ahmetb
Contributor

ahmetb commented Mar 26, 2015

This change fixes a bug where stdout/stderr handles are not identified
correctly as file descriptors on windows.

Previously we used to set the window size to fixed size to fit the default
tty size on the host (80x24) because we couldn't find out why
monitorTtySize wasn't called at all. Turns out it was a bug we fixed here.
Now the attach/exec commands can correctly get the terminal size from windows.

We still do not monitorTtySize() correctly on windows and update the tty
size on the host-side, in order to fix that we'll provide a
platform-specific monitorTtySize implementation in the future.

❗️ ❗️ @jfrazelle it would be great to cherry-pick this fix to 1.6 release branch as we got plenty of feedback saying "every time I run docker.exe it resizes my terminal, it's annoying". ❗️ ❗️

Signed-off-by: Ahmet Alp Balkan ahmetalpbalkan@gmail.com
cc: @tiborvass @sachin-jayant-joshi

term/winconsole: Identify tty correctly, fix resize problem
This change fixes a bug where stdout/stderr handles are not identified
correctly.

Previously we used to set the window size to fixed size to fit the default
tty size on the host (80x24). Now the attach/exec commands can correctly
get the terminal size from windows.

We still do not `monitorTtySize()` correctly on windows and update the tty
size on the host-side, in order to fix that we'll provide a
platform-specific `monitorTtySize` implementation in the future.

Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>
@crosbymichael

This comment has been minimized.

Member

crosbymichael commented Mar 26, 2015

LGTM

1 similar comment
@jessfraz

This comment has been minimized.

Contributor

jessfraz commented Mar 26, 2015

LGTM

jessfraz pushed a commit that referenced this pull request Mar 26, 2015

Jessie Frazelle
Merge pull request #11840 from ahmetalpbalkan/win-cli/term-resize-fix
term/winconsole: Identify tty correctly, fix resize problem

@jessfraz jessfraz merged commit cf7104a into moby:master Mar 26, 2015

2 checks passed

janky Jenkins build Docker-PRs 4387 has succeeded
Details
windows Jenkins build Windows-PRs 1378 has succeeded
Details

@ahmetb ahmetb deleted the ahmetb:win-cli/term-resize-fix branch Mar 26, 2015

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