Fixes progress display on non-color tty (#4647) #4697
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If the output does not support color, then each render of the progress bar is added to a single line, which wraps over multiple lines. As a fallback, a simple carriage return is used to move to the start of the line, and space characters to clear the line.
|
This change will increase the build size from 9.94 MB to 9.94 MB, an increase of 969 bytes (0%)
|
1 similar comment
|
This change will increase the build size from 9.94 MB to 9.94 MB, an increase of 969 bytes (0%)
|
|
Works for me (#4647 reporter here)! |
|
@arcanis @nwholloway just updated to latest master with this commit in it. Running on a mac in my CI: Seems like This error is on macOS 10.12, Node 8.6. |
skevy
pushed a commit
to expo/yarn
that referenced
this pull request
Oct 16, 2017
…kg#4647) (yarnpkg#4697)" This reverts commit 4c38ca7.
|
I suspect that 'process.stdout.columns' is returning a negative number. I'm not sure why Node would believe it to be a tty, but not able to get the window size. @skevy Would you like to try 0ef88f7 to see if this fixes the problem for you? I would be interested in knowing what the job output in your CI environment looked like previous to my first change, and after this change. |
skevy
pushed a commit
to expo/yarn
that referenced
this pull request
Oct 25, 2017
…kg#4647) (yarnpkg#4697)" This reverts commit 4c38ca7.
joaolucasl
pushed a commit
to joaolucasl/yarn
that referenced
this pull request
Oct 27, 2017
If the output does not support color, then each render of the progress bar is added to a single line, which wraps over multiple lines. As a fallback, a simple carriage return is used to move to the start of the line, and space characters to clear the line.
BYK
pushed a commit
that referenced
this pull request
Oct 31, 2017
**Summary** My recent pull request was to improve the appearance of the progress bar for non-color terminals (PR #4697). However, @skevy reported a RangeError when running with macOS 10.12, with Node 8.6. This would have been caused by process.stdout.columns returning a negative number. In this case, this just assumes a default width of 100 characters (as in spinner-progress.js). **Test plan** I have not been able to reproduce the condition where `process.tty.columns` returns a negative number, so have verified the logic by considering key cases, e.g., `undefined > 0`, `-1 > 0`.
calvinhuang
pushed a commit
to calvinhuang/yarn
that referenced
this pull request
Nov 9, 2017
**Summary** My recent pull request was to improve the appearance of the progress bar for non-color terminals (PR yarnpkg#4697). However, @skevy reported a RangeError when running with macOS 10.12, with Node 8.6. This would have been caused by process.stdout.columns returning a negative number. In this case, this just assumes a default width of 100 characters (as in spinner-progress.js). **Test plan** I have not been able to reproduce the condition where `process.tty.columns` returns a negative number, so have verified the logic by considering key cases, e.g., `undefined > 0`, `-1 > 0`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If the output does not support color, then each render of the progress bar is added to a single line, which wraps over multiple lines.
As a fallback, a simple carriage return is used to move to the start of the line, and space characters to clear the line.
Summary
This is a minor cosmetic output on the formatting of the progress output.
I came across the issue raised when looking for problems to address for Hacktoberfest.
Test plan
I verified by running
yarn checkto provide output with the progress bar under the following conditions:The output from the 3 tests can be seen in the following screenshot.
This screenshot shows the non-color test with the active progress bar.