-
-
Notifications
You must be signed in to change notification settings - Fork 275
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
Don't call stream TTY methods on streams that are not TTYs #82
Conversation
index.js
Outdated
@@ -77,6 +77,7 @@ class Ora { | |||
} | |||
|
|||
for (let i = 0; i < this.linesToClear; i++) { | |||
if ( ! this.stream.isTTY) continue; |
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.
The check should be on line 75
@sindresorhus done. Did it in that first place so that .linesToClear would still be reset to 0. This way it won't. just FYI. Happy to do it the way you prefer. |
Is travis failing because of my change or something else? |
For travis failure, I made a PR (#84) that was merged. |
@sindresorhus I think you were a little fast, I wanted him to make a rebase to check that everything works fine. There is a problem with the tests. I can adapt the code for the tests to pass correctly |
@forresst Yeah, I know. I decided to just fix it myself instead of waiting. Want to get out a new release today. |
@sindresorhus Perfect ! (sorry for spam) |
No worries, you couldn't have known that :) |
moveCursor, clearLine, etc are not methods that every stream will have. Only streams that are specifically TTY streams.
I think by default this.stream is set to process.stderr. In unix it's very possible for stderr to not be a TTY.
When running some node thing in the background (with
concurrently
), I got an exception.