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

Set stty size #102

Closed
wants to merge 2 commits into from
Closed

Set stty size #102

wants to merge 2 commits into from

Conversation

warrenvw
Copy link
Contributor

@warrenvw warrenvw commented Feb 8, 2018

Without this change, in the stackstorm container stty size returns 0 0 and potentially only part of the terminal window is used. Use stty to set the actual number of cols and rows so the entire terminal window is available for use.

@arm4b
Copy link
Member

arm4b commented Feb 14, 2018

The code change is not so obvious for me to understand it quick. Might need a few code comments what code does and why.

On a side note, maybe setting env TERM=xterm may help in a Dockerfile?
usually fixes several Docker problems like that

@warrenvw
Copy link
Contributor Author

I built an image with ENV TERM xterm added to the Dockerfile, and removed entrypoint.d/sttysize.sh. Connecting to the container, stty size still returns 0 0 and as above, only the first 24 rows and 80 cols of the terminal are used.

With the fix in commit 4966244, the entire terminal is used. However, I'll continue looking for a simpler solution. If none is available, then I'll document the changes or choose not to merge.

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

👍

Looks good to me. As said before, just would be good to include a few code comments, since the impact of the change is not obvious.

And thanks for trying alternatives.

Copy link
Contributor

@vutny vutny left a comment

Choose a reason for hiding this comment

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

This is very interesting...
But I never encountered any issues with terminal size in stackstorm container, neither locally nor over SSH.

Could it be a problem of older Docker or some terminal app? I always use docker-compose exec stackstorm bash to enter the container.

@arm4b arm4b added the stale label Jul 31, 2019
@arm4b arm4b closed this in 6cc70fd Jul 17, 2020
@arm4b arm4b deleted the wvw/sttysize branch July 17, 2020 16:16
transhapHigsn pushed a commit to DiligenceVault/st2-docker that referenced this pull request Jun 8, 2021
You can find the old deprecated version in `DEPRECATED/all-in-one` branch archive: https://github.com/StackStorm/st2-docker/tree/DEPRECATED/all-in-one

Closes StackStorm#22, closes StackStorm#23, closes StackStorm#26, closes StackStorm#29, closes StackStorm#34, closes StackStorm#41, closes StackStorm#43, closes StackStorm#92, closes StackStorm#112, closes StackStorm#117, closes StackStorm#125, closes StackStorm#133, closes StackStorm#141, closes StackStorm#145, closes StackStorm#151, closes StackStorm#163, closes StackStorm#187,
closes StackStorm#188, closes StackStorm#189, closes StackStorm#190
Closes StackStorm#162, closes StackStorm#138, closes StackStorm#108, closes StackStorm#102, closes StackStorm#65
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants