Skip to content
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

Print output with verbose option #884

Merged
merged 1 commit into from
Mar 4, 2024
Merged

Print output with verbose option #884

merged 1 commit into from
Mar 4, 2024

Conversation

ehmicky
Copy link
Collaborator

@ehmicky ehmicky commented Mar 3, 2024

Fixes #751.

// Also, we cannot use the real PID if we want to print it before `child_process.spawn()` is run.
// As a pro, it is shorter than a normal PID and never re-uses the same id.
// As a con, it cannot be used to send signals.
let VERBOSE_ID = 0n;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the number printed next to the timestamp.

// Files and streams can produce big outputs, which we don't want to print.
// We could print `stdio[3+]` but it often is redirected to files and streams, with the same issue.
// So we only print stdout and stderr.
const fdUsesVerbose = ([{fdNumber}]) => fdNumber === 1 || fdNumber === 2;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comments in this file describe the reasons behind some of the restrictions for the verbose: 'full' option described in readme.md.

// They can also lead to double printing when passing file descriptor integers or `process.std*`.
// This only leaves with `pipe` and `overlapped`.
const shouldLogOutput = (stdioStreams, {encoding}, isSync, {verbose}) => verbose === 'full'
&& !isSync
Copy link
Collaborator Author

@ehmicky ehmicky Mar 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

execaSync() cannot currently use this because it cannot use transforms and this feature uses transforms, for implementation reasons.

However, I am planning on adding support for both to execaSync() and will add an issue shortly.

@ehmicky
Copy link
Collaborator Author

ehmicky commented Mar 3, 2024

Fixed merge conflict.

@sindresorhus sindresorhus merged commit 6fda30f into main Mar 4, 2024
14 checks passed
@sindresorhus sindresorhus deleted the verbose-output branch March 4, 2024 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improved verbose option
2 participants