feat: add `foundry_common::shell` to unify log behavior
Motivation
To unify the shell implementation for improved consistency.
Solution
- Implements a common global shell for all output, correctly rendering to
stdouton logs (sh_println!) andstderron warnings and errors (sh_eprintln!,sh_warn!,sh_err!). - Adds a global
--quiet/-qflag, easily checked against withshell::is_quiet()for conditional logic - Aims to keep the diff as low as possible whilst addressing the removal of existing methods like
p_println,cli_warn,shell::printlnetc..
Changes from initial common shell implementation:
- Macros now implement an
unwrapdirectly on them, avoiding the need to pass / introduce?anywhere. sh_println!/sh_eprintln!/sh_warn!now all listen to the--quietflag- Regular output logs are rendered with
sh_println!as opposed tosh_eprintln! - Warnings are rendered to
stderroutput
To check / fix from https://github.com/foundry-rs/foundry/pull/6569:
- [ ] Evaluate https://github.com/foundry-rs/foundry/issues/3981.
- [ ] Evaluate https://github.com/foundry-rs/foundry/issues/2433.
- [ ] Evaluate https://github.com/foundry-rs/foundry/issues/1976.
- [ ] Evaluate https://github.com/foundry-rs/foundry/issues/7055.
- [ ] Evaluate https://github.com/foundry-rs/foundry/issues/8568
Follow ups:
- [ ] Introduce a global
--jsonflag, similar to the global--quietflag. - [ ] With
shell::Format::Jsonenabled set the verbosity toquiet, disabling all output except for errors. - [ ] Introduce a
sh_json!macro that renders JSON to the console ifshell::Format::Jsonis active, either pretty or not. This could be automatically a no-op in anything except when the--jsonflag is passed. - [ ] Make sure all commands and modes are
--jsoncompatible, related: https://github.com/foundry-rs/foundry/issues/8794. With the current proposed change Anvil for example now has a--jsonflag but is not yet compatible. What--jsonmeans in context dependent to the command / tool. Goal is that it becomes much easier to chain Foundry into other tools. - [ ] Re-add additional macros like
noteandstatusand implement - [ ] Enable Clippy lint, search and replace.
if possible, we can do this in smaller chunks because this pr is a conflict magnet
if possible, we can do this in smaller chunks because this pr is a conflict magnet
Noted, I've tried to keep the changes to a minimum but due to the nature of the PR there are a lot of small changes across many files. I've not replaced any println!'s that are not strictly necessary (in regards to their expected behaviour w/ the --quiet flag). ~~One thing that is possibly not strictly necessary but I feel makes a lot of sense to add is the global --json flag as there is a lot of conditional logic dependent on it.~~ Removed the global --json flag for now.
Moving back to draft, splitting out proposed global --json flag as it contained a breaking change and needs more consideration
Applied fixes from review cc @DaniPopes, would you mind checking one more time?
To avoid visual regression, applied a change to style warning message content in yellow and error message content in red.
Applied a minor fix for conditional rendering of failed tests when --junit flag is passed to avoid modifying the global shell behavior
To avoid visual regression, applied a change to style warning message content in yellow and error message content in red.
This is not right, only the "warn"/"error" should be styled
Reverted the style change as result of internal poll, now renders as follows again: