-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
libnpmexec: allow custom stdio option value. #6976
base: latest
Are you sure you want to change the base?
Conversation
@@ -31,7 +32,9 @@ const run = async ({ | |||
}, | |||
} | |||
|
|||
npmlog.disableProgress() | |||
if (stdio === 'inherit') { | |||
npmlog.disableProgress() |
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.
This is done so that the output from npm doesn't interlace with the output of your script. We always want to disable progress bars during exec.
This will need a test |
scriptShell, | ||
}) | ||
} finally { | ||
npmlog.enableProgress() | ||
if (stdio === 'inherit') { |
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.
this one needs to be removed also
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 idea is to make the libnpmexec don’t output npm logs.
npm log output is interlacing with the output of the application executing libnpmexec.
Just passing the stdio value through is not enough.
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.
This isn't npm log output, this is progress bars specifically.
To suppress all logging we need npmlog.pause
and npmlog.resume
, but these should be independent of enabling/disabling progress.
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.
I don't want to install npmlog directly to pause it.
What about a flag to ignore npmlog?
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.
You already have it
const npmlog = require('npmlog') |
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.
I mean, I have ora as progress bar.
npmlog and ora are mangling the output.
I need to install and import npmlog in my project to pause the npmlog.
And that does not guarantee that the npmlog is muted due to hoisting.
Looks good other than that rogue |
libnpmexec
forces 'inherit' value in stdio option.Others stdio values are required for non interactive usage.
References