-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
chore(chisel
): enforce common::shell
for chisel binary
#9177
base: master
Are you sure you want to change the base?
Conversation
chisel
): enforce common::shell for chisel binarychisel
): enforce common::shell
for chisel binary
@@ -205,7 +207,7 @@ async fn main_args(args: Chisel) -> eyre::Result<()> { | |||
} | |||
|
|||
// Print welcome header | |||
println!("Welcome to Chisel! Type `{}` to show available commands.", "!help".green()); | |||
sh_println!("Welcome to Chisel! Type `{}` to show available commands.", "!help")?; |
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.
cc @DaniPopes in #9109 you noted messages ought not to be painted in color except for the warn / error
keyword, is removing this here correct? Where yellow
is used for warnings we use sh_warn!
, for red
(if an error) we use sh_err!
and success (or used in an random way) should not be painted green, correct?
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.
Yeah but that's for err and warn, and here it's in a REPL too, so it's fine
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.
lgtm!
Motivation
Enforces
common::shell
as added in #9109 for thechisel
binarySolution
Splits out the enforcement of the common shell per binary / large crate to avoid huge diffs making reviews easier