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

convox run handles non-tty stdin #110

Merged
merged 2 commits into from Oct 16, 2015

Conversation

Projects
None yet
2 participants
@bobzoller
Copy link
Contributor

bobzoller commented Oct 15, 2015

fixes #108

@@ -40,7 +40,6 @@ func cmdRun(c *cli.Context) {

fd := os.Stdin.Fd()
stdinState, err := terminal.GetState(int(fd))
defer terminal.Restore(int(fd), stdinState)

This comment has been minimized.

@csquared

csquared Oct 15, 2015

Contributor

IIRC, this was for handling CTRL+C

This comment has been minimized.

@csquared

csquared Oct 16, 2015

Contributor

maybe we can we conditionally defer?

This comment has been minimized.

@bobzoller

bobzoller Oct 16, 2015

Contributor

ah, got it. (couldn't figure out what that was for!) I'll add it back in and put the conditional around it. and a comment ;)

@bobzoller

This comment has been minimized.

Copy link
Contributor

bobzoller commented Oct 16, 2015

@csquared what do you think about this? extracting a function keeps the logic simple, and you don't need the double terminal.Restore because the one defer covers both cases.

@csquared

This comment has been minimized.

Copy link
Contributor

csquared commented Oct 16, 2015

This looks good- nice use of defer! Will merge when back at keyboard

@csquared

This comment has been minimized.

Copy link
Contributor

csquared commented Oct 16, 2015

👍

csquared added a commit that referenced this pull request Oct 16, 2015

Merge pull request #110 from goodeggs/run-non-tty
convox run handles non-tty stdin

@csquared csquared merged commit 63561fe into convox:master Oct 16, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.001%) to 60.502%
Details

@bobzoller bobzoller deleted the goodeggs:run-non-tty branch Oct 16, 2015

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