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

feat(packager): always show command output, enhance formatting #196

Merged
merged 7 commits into from
Apr 17, 2024

Conversation

lucasfernog-crabnebula
Copy link
Member

It's hard to understand when e.g. a cargo build fails because we do not show its output without the --verbose flag. This PR changes the tracing event level to INFO so it's always available, and enhances the formatting to remove extra fields, dates, and log level.

@amr-crabnebula
Copy link
Collaborator

amr-crabnebula commented Apr 15, 2024

enhances the formatting to remove extra fields, dates, and log level.

Usage of compact showed level and dates for things other than shell output so I pushed a change to remove those as well

@amr-crabnebula
Copy link
Collaborator

It's hard to understand when e.g. a cargo build fails because we do not show its output without the --verbose flag. This PR changes the tracing event level to INFO so it's always available

I don't think this should be INFO by default at all, showing only on debug is probably better, however I changed stderr to be tracing::error! instead so if it fails it will show the errors correctly, let me know what you think @lucasfernog-crabnebula

Comment on lines +288 to +290
if std::env::var_os("CARGO_TERM_COLOR").is_none() {
std::env::set_var("CARGO_TERM_COLOR", "always");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't this be set by the user invoking the packager? or at least kept as auto so it automatically detects whether the current terminal has color support or not?

Copy link
Member Author

Choose a reason for hiding this comment

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

if we do not force this there's no colored output at all :(

@lucasfernog-crabnebula
Copy link
Member Author

It's hard to understand when e.g. a cargo build fails because we do not show its output without the --verbose flag. This PR changes the tracing event level to INFO so it's always available

I don't think this should be INFO by default at all, showing only on debug is probably better, however I changed stderr to be tracing::error! instead so if it fails it will show the errors correctly, let me know what you think @lucasfernog-crabnebula

I think for some commands this can be debug only, but for the cargo build one it should definitely be info IMO.

@lucasfernog-crabnebula
Copy link
Member Author

using error level for stderr is tricky, some CLIs use it for progress output for instance

@amr-crabnebula
Copy link
Collaborator

alright, I have changed it to always show output of beforePackagingCommand

@amr-crabnebula amr-crabnebula merged commit 1375380 into main Apr 17, 2024
21 of 22 checks passed
@amr-crabnebula amr-crabnebula deleted the feat/enhance-shell-output branch April 17, 2024 13:30
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.

2 participants