-
Notifications
You must be signed in to change notification settings - Fork 448
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
cli: do not prompt when there is no tty available on stdin #2570
Conversation
There was a problem hiding this 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 :-)
This is a bit of a drive-by. I noticed that inside travis snapcraft 3 hangs with the following message: ``` Support for 'LXD' needs to be set up. Would you like to do that it now? [y/N]: ``` It seems like we should only prompt here if stdin is attached to a tty and otherwise just fail early. This should fix hangs in e.g. scripts.
Interesting, last I read |
@sergiusens When I played around with redirection it seems like click.confirm() does not take tty into account. But caveats apply, I did not look deeply and did not report an upstream bug (probably a good idea though). Go wild on this, this PR is just a drive-by as I was slightly annoyed by this hang in travis. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contribution, we will look into consolidating these scenarios into a follow-up PR
Codecov Report
@@ Coverage Diff @@
## master #2570 +/- ##
==========================================
+ Coverage 89.05% 89.06% +<.01%
==========================================
Files 205 205
Lines 13987 13988 +1
Branches 2118 2118
==========================================
+ Hits 12456 12458 +2
+ Misses 1081 1080 -1
Partials 450 450
Continue to review full report at Codecov.
|
This is a bit of a drive-by. I noticed that inside travis
snapcraft 3 hangs with the following message:
It seems like we should only prompt here if stdin is attached
to a tty and otherwise just fail early. This should fix hangs
in e.g. scripts.