From 78f0ce9a36428cd6acb31a54f376527fa53c08e1 Mon Sep 17 00:00:00 2001 From: l3ops Date: Wed, 9 Nov 2022 16:51:21 +0100 Subject: [PATCH 1/2] feat(rome_cli): add a `--force-colors` argument --- crates/rome_cli/src/commands/help.rs | 1 + crates/rome_cli/src/lib.rs | 23 +++++++--- crates/rome_cli/src/main.rs | 3 +- crates/rome_cli/tests/main.rs | 38 +++++++++++++++++ crates/rome_console/src/lib.rs | 42 ++++++++++++++----- .../rome_js_semantic/src/tests/assertions.rs | 12 +++--- xtask/lintdoc/src/main.rs | 4 +- 7 files changed, 98 insertions(+), 25 deletions(-) diff --git a/crates/rome_cli/src/commands/help.rs b/crates/rome_cli/src/commands/help.rs index 70817f7da60..4eca954f232 100644 --- a/crates/rome_cli/src/commands/help.rs +++ b/crates/rome_cli/src/commands/help.rs @@ -19,6 +19,7 @@ const MAIN: Markup = markup! { ""OPTIONS:"" ""--no-colors"" Disable the formatting of markup (print everything as plain text) + ""--force-colors"" Force the formatting of markup using ANSI, even if the console output is determined to be incompatible ""--use-server"" Connect to a running instance of the Rome daemon server ""--version"" Show the Rome version information and quit ""--files-max-size"" The maximum allowed size for source code files in bytes (default: 1MB) diff --git a/crates/rome_cli/src/lib.rs b/crates/rome_cli/src/lib.rs index 6fd16369b28..6456fa3a9ca 100644 --- a/crates/rome_cli/src/lib.rs +++ b/crates/rome_cli/src/lib.rs @@ -7,7 +7,7 @@ //! execute the traversal of directory and files, based on the command that were passed. pub use pico_args::Arguments; -use rome_console::EnvConsole; +use rome_console::{ColorMode, EnvConsole}; use rome_flags::FeatureFlags; use rome_fs::OsFileSystem; use rome_service::{App, DynRef, Workspace, WorkspaceRef}; @@ -45,16 +45,29 @@ pub struct CliSession<'app> { } impl<'app> CliSession<'app> { - pub fn new(workspace: &'app dyn Workspace, mut args: Arguments) -> Self { + pub fn new(workspace: &'app dyn Workspace, mut args: Arguments) -> Result { let no_colors = args.contains("--no-colors"); - Self { + let force_colors = args.contains("--force-colors"); + let colors = match (no_colors, force_colors) { + (true, false) => ColorMode::Disabled, + (false, true) => ColorMode::Enabled, + (false, false) => ColorMode::Auto, + (true, true) => { + return Err(Termination::IncompatibleArguments( + "--no-colors", + "--force-colors", + )) + } + }; + + Ok(Self { app: App::new( DynRef::Owned(Box::new(OsFileSystem)), - DynRef::Owned(Box::new(EnvConsole::new(no_colors))), + DynRef::Owned(Box::new(EnvConsole::new(colors))), WorkspaceRef::Borrowed(workspace), ), args, - } + }) } /// Main function to run Rome CLI diff --git a/crates/rome_cli/src/main.rs b/crates/rome_cli/src/main.rs index 8e920252b2b..53e5bcbe815 100644 --- a/crates/rome_cli/src/main.rs +++ b/crates/rome_cli/src/main.rs @@ -35,5 +35,6 @@ fn main() -> Result<(), Termination> { workspace::server() }; - CliSession::new(&*workspace, args).run() + let session = CliSession::new(&*workspace, args)?; + session.run() } diff --git a/crates/rome_cli/tests/main.rs b/crates/rome_cli/tests/main.rs index 9c5b596e79e..c5cec585529 100644 --- a/crates/rome_cli/tests/main.rs +++ b/crates/rome_cli/tests/main.rs @@ -52,6 +52,7 @@ mod help { mod main { use super::*; use rome_diagnostics::MAXIMUM_DISPLAYABLE_DIAGNOSTICS; + use rome_service::workspace; #[test] fn unknown_command() { @@ -181,6 +182,43 @@ mod main { _ => panic!("run_cli returned {result:?} for a malformed, expected an error"), } } + + #[test] + fn no_colors() { + let workspace = workspace::server(); + let args = Arguments::from_vec(vec![OsString::from("--no-colors")]); + let result = CliSession::new(&*workspace, args).and_then(|session| session.run()); + + assert!(result.is_ok(), "run_cli returned {result:?}"); + } + + #[test] + fn force_colors() { + let workspace = workspace::server(); + let args = Arguments::from_vec(vec![OsString::from("--force-colors")]); + let result = CliSession::new(&*workspace, args).and_then(|session| session.run()); + + assert!(result.is_ok(), "run_cli returned {result:?}"); + } + + #[test] + fn incompatible_colors() { + let workspace = workspace::server(); + let args = Arguments::from_vec(vec![ + OsString::from("--no-colors"), + OsString::from("--force-colors"), + ]); + + let result = CliSession::new(&*workspace, args).and_then(|session| session.run()); + + match result { + Err(Termination::IncompatibleArguments(lhs, rhs)) => { + assert_eq!(lhs, "--no-colors"); + assert_eq!(rhs, "--force-colors"); + } + _ => panic!("run_cli returned {result:?} for a malformed, expected an error"), + } + } } mod init { diff --git a/crates/rome_console/src/lib.rs b/crates/rome_console/src/lib.rs index 13597622c3d..8ef0ab76dd2 100644 --- a/crates/rome_console/src/lib.rs +++ b/crates/rome_console/src/lib.rs @@ -63,17 +63,37 @@ pub struct EnvConsole { r#in: Stdin, } +pub enum ColorMode { + /// Always print color using either ANSI or the Windows Console API + Enabled, + /// Never print colors + Disabled, + /// Print colors if stdout / stderr are determined to be TTY / Console + /// streams, and the `TERM=dumb` and `NO_COLOR` environment variables are + /// not set + Auto, +} + impl EnvConsole { - pub fn new(no_colors: bool) -> Self { - let out_mode = if no_colors || !atty::is(atty::Stream::Stdout) { - ColorChoice::Never - } else { - ColorChoice::Auto - }; - let err_mode = if no_colors || !atty::is(atty::Stream::Stderr) { - ColorChoice::Never - } else { - ColorChoice::Auto + pub fn new(colors: ColorMode) -> Self { + let (out_mode, err_mode) = match colors { + ColorMode::Enabled => (ColorChoice::Always, ColorChoice::Always), + ColorMode::Disabled => (ColorChoice::Never, ColorChoice::Never), + ColorMode::Auto => { + let stdout = if atty::is(atty::Stream::Stdout) { + ColorChoice::Auto + } else { + ColorChoice::Never + }; + + let stderr = if atty::is(atty::Stream::Stderr) { + ColorChoice::Auto + } else { + ColorChoice::Never + }; + + (stdout, stderr) + } }; Self { @@ -86,7 +106,7 @@ impl EnvConsole { impl Default for EnvConsole { fn default() -> Self { - Self::new(false) + Self::new(ColorMode::Auto) } } diff --git a/crates/rome_js_semantic/src/tests/assertions.rs b/crates/rome_js_semantic/src/tests/assertions.rs index ebdb25f4d38..0d68f0c5186 100644 --- a/crates/rome_js_semantic/src/tests/assertions.rs +++ b/crates/rome_js_semantic/src/tests/assertions.rs @@ -108,7 +108,7 @@ pub fn assert(code: &str, test_name: &str) { let r = rome_js_parser::parse(code, FileId::zero(), SourceType::tsx()); if r.has_errors() { - let mut console = EnvConsole::new(false); + let mut console = EnvConsole::default(); for diag in r.into_diagnostics() { let error = diag .with_file_path(FileId::zero()) @@ -756,7 +756,7 @@ fn error_assertion_not_attached_to_a_declaration( .with_file_path((test_name.to_string(), FileId::zero())) .with_file_source_code(code); - let mut console = EnvConsole::new(false); + let mut console = EnvConsole::default(); console.log(markup! { {PrintDiagnostic(&error)} }); @@ -777,7 +777,7 @@ fn error_declaration_pointing_to_unknown_scope( .with_file_path((test_name.to_string(), FileId::zero())) .with_file_source_code(code); - let mut console = EnvConsole::new(false); + let mut console = EnvConsole::default(); console.log(markup! { {PrintDiagnostic(&error)} }); @@ -802,7 +802,7 @@ fn error_assertion_name_clash( .with_file_path((test_name.to_string(), FileId::zero())) .with_file_source_code(code); - let mut console = EnvConsole::new(false); + let mut console = EnvConsole::default(); console.log(markup! { {PrintDiagnostic(&error)} }); @@ -825,7 +825,7 @@ fn error_scope_end_assertion_points_to_non_existing_scope_start_assertion( .with_file_path((file_name.to_string(), FileId::zero())) .with_file_source_code(code); - let mut console = EnvConsole::new(false); + let mut console = EnvConsole::default(); console.log(markup! { {PrintDiagnostic(&error)} }); @@ -852,7 +852,7 @@ fn error_scope_end_assertion_points_to_the_wrong_scope_start( .with_file_path((file_name.to_string(), FileId::zero())) .with_file_source_code(code); - let mut console = EnvConsole::new(false); + let mut console = EnvConsole::default(); console.log(markup! { {PrintDiagnostic(&error)} }); diff --git a/xtask/lintdoc/src/main.rs b/xtask/lintdoc/src/main.rs index 74471273323..48d3fe92fdb 100644 --- a/xtask/lintdoc/src/main.rs +++ b/xtask/lintdoc/src/main.rs @@ -474,7 +474,7 @@ fn assert_lint( if test.expect_diagnostic { // Print all diagnostics to help the user if all_diagnostics.len() > 1 { - let mut console = rome_console::EnvConsole::new(false); + let mut console = rome_console::EnvConsole::default(); for diag in all_diagnostics.iter() { console.print( rome_console::LogLevel::Error, @@ -492,7 +492,7 @@ fn assert_lint( ); } else { // Print all diagnostics to help the user - let mut console = rome_console::EnvConsole::new(false); + let mut console = rome_console::EnvConsole::default(); for diag in all_diagnostics.iter() { console.print( rome_console::LogLevel::Error, From dc5ca279353cb46f938fbb0e5392bfa23ad2cab8 Mon Sep 17 00:00:00 2001 From: l3ops Date: Mon, 14 Nov 2022 14:29:37 +0100 Subject: [PATCH 2/2] merge `--no-colors` and `--force-colors` into a single `--colors` argument --- crates/rome_cli/src/commands/help.rs | 3 +- crates/rome_cli/src/lib.rs | 42 ++++++++++++++++++++-------- crates/rome_cli/tests/main.rs | 16 ++++------- website/src/pages/cli.mdx | 6 ++-- 4 files changed, 42 insertions(+), 25 deletions(-) diff --git a/crates/rome_cli/src/commands/help.rs b/crates/rome_cli/src/commands/help.rs index 4eca954f232..acc2cb1f127 100644 --- a/crates/rome_cli/src/commands/help.rs +++ b/crates/rome_cli/src/commands/help.rs @@ -18,8 +18,7 @@ const MAIN: Markup = markup! { - ""help"" Prints this help message ""OPTIONS:"" - ""--no-colors"" Disable the formatting of markup (print everything as plain text) - ""--force-colors"" Force the formatting of markup using ANSI, even if the console output is determined to be incompatible + ""--colors="" Set the formatting mode for markup: \"off\" prints everything as plain text, \"force\" forces the formatting of markup using ANSI even if the console output is determined to be incompatible ""--use-server"" Connect to a running instance of the Rome daemon server ""--version"" Show the Rome version information and quit ""--files-max-size"" The maximum allowed size for source code files in bytes (default: 1MB) diff --git a/crates/rome_cli/src/lib.rs b/crates/rome_cli/src/lib.rs index 6456fa3a9ca..32371277a68 100644 --- a/crates/rome_cli/src/lib.rs +++ b/crates/rome_cli/src/lib.rs @@ -6,6 +6,8 @@ //! to parse commands and arguments, redirect the execution of the commands and //! execute the traversal of directory and files, based on the command that were passed. +use std::str::FromStr; + pub use pico_args::Arguments; use rome_console::{ColorMode, EnvConsole}; use rome_flags::FeatureFlags; @@ -46,18 +48,36 @@ pub struct CliSession<'app> { impl<'app> CliSession<'app> { pub fn new(workspace: &'app dyn Workspace, mut args: Arguments) -> Result { - let no_colors = args.contains("--no-colors"); - let force_colors = args.contains("--force-colors"); - let colors = match (no_colors, force_colors) { - (true, false) => ColorMode::Disabled, - (false, true) => ColorMode::Enabled, - (false, false) => ColorMode::Auto, - (true, true) => { - return Err(Termination::IncompatibleArguments( - "--no-colors", - "--force-colors", - )) + enum ColorsArg { + Off, + Force, + } + + impl FromStr for ColorsArg { + type Err = String; + + fn from_str(s: &str) -> Result { + match s { + "off" => Ok(Self::Off), + "force" => Ok(Self::Force), + _ => Err(format!( + "value {s:?} is not valid for the --colors argument" + )), + } } + } + + let colors = + args.opt_value_from_str("--colors") + .map_err(|source| Termination::ParseError { + argument: "--colors", + source, + })?; + + let colors = match colors { + Some(ColorsArg::Off) => ColorMode::Disabled, + Some(ColorsArg::Force) => ColorMode::Enabled, + None => ColorMode::Auto, }; Ok(Self { diff --git a/crates/rome_cli/tests/main.rs b/crates/rome_cli/tests/main.rs index c5cec585529..a80c3cf5dd9 100644 --- a/crates/rome_cli/tests/main.rs +++ b/crates/rome_cli/tests/main.rs @@ -186,7 +186,7 @@ mod main { #[test] fn no_colors() { let workspace = workspace::server(); - let args = Arguments::from_vec(vec![OsString::from("--no-colors")]); + let args = Arguments::from_vec(vec![OsString::from("--colors=off")]); let result = CliSession::new(&*workspace, args).and_then(|session| session.run()); assert!(result.is_ok(), "run_cli returned {result:?}"); @@ -195,26 +195,22 @@ mod main { #[test] fn force_colors() { let workspace = workspace::server(); - let args = Arguments::from_vec(vec![OsString::from("--force-colors")]); + let args = Arguments::from_vec(vec![OsString::from("--colors=force")]); let result = CliSession::new(&*workspace, args).and_then(|session| session.run()); assert!(result.is_ok(), "run_cli returned {result:?}"); } #[test] - fn incompatible_colors() { + fn invalid_colors() { let workspace = workspace::server(); - let args = Arguments::from_vec(vec![ - OsString::from("--no-colors"), - OsString::from("--force-colors"), - ]); + let args = Arguments::from_vec(vec![OsString::from("--colors=other")]); let result = CliSession::new(&*workspace, args).and_then(|session| session.run()); match result { - Err(Termination::IncompatibleArguments(lhs, rhs)) => { - assert_eq!(lhs, "--no-colors"); - assert_eq!(rhs, "--force-colors"); + Err(Termination::ParseError { argument, .. }) => { + assert_eq!(argument, "--colors"); } _ => panic!("run_cli returned {result:?} for a malformed, expected an error"), } diff --git a/website/src/pages/cli.mdx b/website/src/pages/cli.mdx index 464d5b128cc..80a176a8998 100644 --- a/website/src/pages/cli.mdx +++ b/website/src/pages/cli.mdx @@ -53,9 +53,11 @@ Stop the Rome [daemon](/internals/architecture#deamon) server ## Common Options -### `--no-colors` +### `--colors=` -Disable the formatting of markup (print everything as plain text) +Set the formatting mode for markup: `off` prints everything as plain text, +`force` forces the formatting of markup using ANSI even if the console output +is determined to be incompatible ### `--use-server`