Skip to content
This repository was archived by the owner on May 4, 2024. It is now read-only.

Fix the --no-progress bugs in npm#46

Closed
iarna wants to merge 5 commits intomasterfrom
iarna/progress-enable-fixes
Closed

Fix the --no-progress bugs in npm#46
iarna wants to merge 5 commits intomasterfrom
iarna/progress-enable-fixes

Conversation

@iarna
Copy link
Copy Markdown
Contributor

@iarna iarna commented Nov 3, 2016

They were a result of log.progressEnabled getting out of sync with gauge's own internal ideas about its enabledness.

iarna added 5 commits November 3, 2016 15:08
`gauge` on the other hand, sets its default based on the TTYness of what
it's writing to.  We need to set this explicitly so that the two are in
sync.
Update gauge to get isEnabled, so we can directly introspect what guage
thinks about itself.
While we're initializing gauge with a constant now, this helps future
proof us should we ever change that decision. It ensures that our own
tracking variable is in sync with gauge from the start.
We were calling it after enabling and before disabling respectively but
this is unnecessary as `gauge` will take care of these for us.
This means calling `gauge.disable()` when pausing if progressEnabled to
ensure that the progress bar is cleared and won't be written to the screen
while the log is disabled.

And similarly calling `gauge.enable()` when resuming if progressEnabled.

And finally patching `log.enableProgress()` to ONLY skip calling
`gauge.enable()` when called while paused.
@coveralls
Copy link
Copy Markdown

coveralls commented Nov 3, 2016

Coverage Status

Coverage decreased (-0.6%) to 86.022% when pulling 687659a on iarna/progress-enable-fixes into 3ca8823 on master.

@xzyfer
Copy link
Copy Markdown

xzyfer commented Nov 13, 2016

We have recently run into this in node-sass also

@iarna
Copy link
Copy Markdown
Contributor Author

iarna commented Nov 17, 2016

This landed in 4.0.1

@iarna iarna closed this Nov 17, 2016
iarna added a commit to npm/npm that referenced this pull request Nov 17, 2016
Fix bugs where `log.progressEnabled` got out of sync with how `gauge` kept
track of these things resulting in a progressbar that couldn't be disabled.

Fixes: #14413
PR-URL: npm/npmlog#46
Credit: @iarna
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants