From a651029197ab65aac6287a817715c4ae992fa53c Mon Sep 17 00:00:00 2001 From: Fletcher Nichol Date: Thu, 15 Sep 2016 11:32:30 -0600 Subject: [PATCH 1/2] [hab] Update analytics code to use the UI subsystem. This change ensures that analytics opt in and opt out activities are being colored (or not colored) appropriately. Signed-off-by: Fletcher Nichol --- components/common/src/ui.rs | 2 ++ components/hab/src/analytics.rs | 23 +++++++++++------------ components/hab/src/command/cli.rs | 4 ++-- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/components/common/src/ui.rs b/components/common/src/ui.rs index 5e8f9a8617..2c89fb6fdc 100644 --- a/components/common/src/ui.rs +++ b/components/common/src/ui.rs @@ -29,6 +29,7 @@ pub enum Status { Applying, Cached, Creating, + Deleting, Downloading, Encrypting, Installed, @@ -48,6 +49,7 @@ impl Status { Status::Applying => ('↑', "Applying".into(), Colour::Green), Status::Cached => ('☑', "Cached".into(), Colour::Green), Status::Creating => ('Ω', "Creating".into(), Colour::Green), + Status::Deleting => ('☒', "Deleting".into(), Colour::Green), Status::Downloading => ('↓', "Downloading".into(), Colour::Green), Status::Encrypting => ('☛', "Encypting".into(), Colour::Green), Status::Installed => ('✓', "Installed".into(), Colour::Green), diff --git a/components/hab/src/analytics.rs b/components/hab/src/analytics.rs index 500e4ebeca..821881acef 100644 --- a/components/hab/src/analytics.rs +++ b/components/hab/src/analytics.rs @@ -190,8 +190,8 @@ use std::io::{Read, Write}; use std::path::Path; use std::time::{UNIX_EPOCH, SystemTime}; -use ansi_term::Colour::{Blue, Green, Yellow}; use clap; +use common::ui::{Status, UI}; use hcore; use http_client::ApiClient; use url::Url; @@ -343,23 +343,23 @@ pub fn instrument_clap_error(err: &clap::Error) { /// * If the parent directory cannot be created /// * If an opt-out if exists but cannot be deleted /// * If an opt-in file cannot be created -pub fn opt_in(analytics_path: &Path, origin_generated: bool) -> Result<()> { - println!("{}", Yellow.bold().paint("» Opting in to analytics")); +pub fn opt_in(ui: &mut UI, analytics_path: &Path, origin_generated: bool) -> Result<()> { + try!(ui.begin("Opting in to analytics")); // Create the parent directory which will contain the opt-in file try!(fs::create_dir_all(analytics_path)); // Get the path to the opt-out file let opt_out_path = analytics_path.join(OPTED_OUT_METAFILE); // If the opt-out file exists, delete it from disk if opt_out_path.exists() { - println!("{} {}", Green.paint("☒ Deleting"), opt_out_path.display()); + try!(ui.status(Status::Deleting, opt_out_path.display())); try!(fs::remove_file(&opt_out_path)); } // Get the path to the opt-in file let opt_in_path = analytics_path.join(OPTED_IN_METAFILE); - println!("{} {}", Green.paint("☑ Creating"), opt_in_path.display()); + try!(ui.status(Status::Creating, opt_in_path.display())); // Create the opt-in file let _ = try!(File::create(opt_in_path)); - println!("{}", Blue.paint("★ Analytics opted in, thank you!")); + try!(ui.end("Analytics opted in, thank you!")); // Record an event that the setup subcommand was invoked record_event(Event::Subcommand, &format!("{}--{}--{}", PRODUCT, "cli", "setup")); @@ -386,24 +386,23 @@ pub fn opt_in(analytics_path: &Path, origin_generated: bool) -> Result<()> { /// * If the parent directory cannot be created /// * If an opt-in if exists but cannot be deleted /// * If an opt-out file cannot be created -pub fn opt_out(analytics_path: &Path) -> Result<()> { - println!("{}", Yellow.bold().paint("» Opting out of analytics")); +pub fn opt_out(ui: &mut UI, analytics_path: &Path) -> Result<()> { + try!(ui.begin("Opting out of analytics")); // Create the parent directory which will contain the opt-in file try!(fs::create_dir_all(analytics_path)); // Get the path to the opt-in file let opt_in_path = analytics_path.join(OPTED_IN_METAFILE); // If the opt-in file exists, delete it from disk if opt_in_path.exists() { - println!("{} {}", Green.paint("☒ Deleting"), opt_in_path.display()); + try!(ui.status(Status::Deleting, opt_in_path.display())); try!(fs::remove_file(&opt_in_path)); } // Get the path to the opt-out file let opt_out_path = analytics_path.join(OPTED_OUT_METAFILE); - println!("{} {}", Green.paint("☑ Creating"), opt_out_path.display()); + try!(ui.status(Status::Creating, opt_out_path.display())); // Create the opt-out file let _ = try!(File::create(opt_out_path)); - println!("{}", - Blue.paint("★ Analytics opted out, we salute you just the same!")); + try!(ui.end("Analytics opted out, we salute you just the same!")); // Return an empty Ok, representing a successful operation Ok(()) } diff --git a/components/hab/src/command/cli.rs b/components/hab/src/command/cli.rs index 6de57568a6..273b39ab7c 100644 --- a/components/hab/src/command/cli.rs +++ b/components/hab/src/command/cli.rs @@ -196,13 +196,13 @@ pub mod setup { } fn opt_in_analytics(ui: &mut UI, analytics_path: &Path, generated_origin: bool) -> Result<()> { - let result = analytics::opt_in(analytics_path, generated_origin); + let result = analytics::opt_in(ui, analytics_path, generated_origin); try!(ui.br()); result } fn opt_out_analytics(ui: &mut UI, analytics_path: &Path) -> Result<()> { - let result = analytics::opt_out(analytics_path); + let result = analytics::opt_out(ui, analytics_path); try!(ui.br()); result } From 77cb638bb5a33da3a5d048208131c49db55a7822 Mon Sep 17 00:00:00 2001 From: Fletcher Nichol Date: Thu, 15 Sep 2016 12:30:14 -0600 Subject: [PATCH 2/2] [hab] Add warn & fatal methods to UI to remove remaining coloring. This change removes the final raw colored `println!()` calls in favor of 2 new methods: `UI#fatal()` and `UI#warn()`. Signed-off-by: Fletcher Nichol --- components/common/src/ui.rs | 38 ++++++++++++++++++- components/hab/src/command/origin.rs | 1 - components/hab/src/command/pkg.rs | 33 +++++++--------- components/hab/src/command/sup.rs | 8 ++-- components/hab/src/main.rs | 57 +++++++++++++--------------- 5 files changed, 81 insertions(+), 56 deletions(-) diff --git a/components/common/src/ui.rs b/components/common/src/ui.rs index 2c89fb6fdc..d345df7dcd 100644 --- a/components/common/src/ui.rs +++ b/components/common/src/ui.rs @@ -53,7 +53,7 @@ impl Status { Status::Downloading => ('↓', "Downloading".into(), Colour::Green), Status::Encrypting => ('☛', "Encypting".into(), Colour::Green), Status::Installed => ('✓', "Installed".into(), Colour::Green), - Status::Missing => ('∵', "Missing".into(), Colour::Cyan), + Status::Missing => ('∵', "Missing".into(), Colour::Red), Status::Signed => ('✓', "Signed".into(), Colour::Cyan), Status::Signing => ('☛', "Signing".into(), Colour::Cyan), Status::Uploaded => ('✓', "Uploaded".into(), Colour::Green), @@ -104,6 +104,42 @@ impl UI { Ok(()) } + pub fn warn(&mut self, message: T) -> Result<()> { + let ref mut stream = self.shell.err; + match stream.is_colored() { + true => { + try!(write!(stream, + "{}\n", + Colour::Yellow.bold().paint(format!("∅ {}", message.to_string())))); + } + false => { + try!(write!(stream, "∅ {}\n", message.to_string())); + } + } + try!(stream.flush()); + Ok(()) + } + + pub fn fatal(&mut self, message: T) -> Result<()> { + let ref mut stream = self.shell.err; + match stream.is_colored() { + true => { + try!(write!(stream, + "{}\n", + Colour::Red.bold() + .paint(format!("✗✗✗\n✗✗✗ {}\n✗✗✗", + message.to_string())))); + } + false => { + try!(write!(stream, + "✗✗✗\n✗✗✗ {}\n✗✗✗\n", + message.to_string())); + } + } + try!(stream.flush()); + Ok(()) + } + pub fn progress(&mut self) -> Option { if self.shell.out.is_a_terminal() { Some(ProgressBar::default()) diff --git a/components/hab/src/command/origin.rs b/components/hab/src/command/origin.rs index 3cb6b91079..99737a3f22 100644 --- a/components/hab/src/command/origin.rs +++ b/components/hab/src/command/origin.rs @@ -92,7 +92,6 @@ pub mod key { Ok(()) } - // TODO fin: use UI to signal whether or not to use progress bars fn download_key(ui: &mut UI, depot_client: &Client, nwr: &str, diff --git a/components/hab/src/command/pkg.rs b/components/hab/src/command/pkg.rs index 6ce90df233..2b9086a49f 100644 --- a/components/hab/src/command/pkg.rs +++ b/components/hab/src/command/pkg.rs @@ -160,8 +160,8 @@ pub mod export { inner::start(ui, ident, format) } - pub fn format_for(value: &str) -> Result { - inner::format_for(value) + pub fn format_for(ui: &mut UI, value: &str) -> Result { + inner::format_for(ui, value) } #[cfg(target_os = "linux")] @@ -182,7 +182,7 @@ pub mod export { use error::{Error, Result}; use super::ExportFormat; - pub fn format_for(value: &str) -> Result { + pub fn format_for(_ui: &mut UI, value: &str) -> Result { match value { "docker" => { let format = ExportFormat { @@ -239,20 +239,18 @@ pub mod export { #[cfg(not(target_os = "linux"))] mod inner { - use ansi_term::Colour::Yellow; use error::{Error, Result}; use common::UI; use hcore::package::PackageIdent; use std::env; use super::ExportFormat; - // TODO fin: add a warning or error method on `UI` to handle messages below - pub fn format_for(value: &str) -> Result { - let msg = format!("∅ Exporting {} packages from this operating system is not yet \ + pub fn format_for(ui: &mut UI, value: &str) -> Result { + try!(ui.warn(format!("∅ Exporting {} packages from this operating system is not yet \ supported. Try running this command again on a 64-bit Linux \ operating system.\n", - value); - println!("{}", Yellow.bold().paint(msg)); + value))); + try!(ui.br()); let e = Error::UnsupportedExportFormat(value.to_string()); Err(e) } @@ -260,10 +258,9 @@ pub mod export { pub fn start(_ui: &mut UI, _ident: &PackageIdent, _format: &ExportFormat) -> Result<()> { let subcmd = env::args().nth(1).unwrap_or("".to_string()); let subsubcmd = env::args().nth(2).unwrap_or("".to_string()); - let msg = format!("∅ Exporting packages from this operating system is not yet \ - supported. Try running this command again on a 64-bit Linux \ - operating system.\n"); - println!("{}", Yellow.bold().paint(msg)); + try!(ui.warn("Exporting packages from this operating system is not yet supported. Try \ + running this command again on a 64-bit Linux operating system.")); + try!(ui.br()); Err(Error::SubcommandNotSupported(format!("{} {}", subcmd, subsubcmd))) } @@ -442,7 +439,6 @@ pub mod upload { use std::path::{Path, PathBuf}; - use ansi_term::Colour::Red; use common::ui::{Status, UI}; use depot_client::{self, Client}; use hcore::crypto::artifact::get_artifact_header; @@ -580,11 +576,10 @@ pub mod upload { } } } else { - // TODO fin: replace with a warning or error `UI` method call - println!("{} artifact for {} was not found in {}", - Red.bold().paint("✗ Missing"), - ident.archive_name().unwrap(), - archives_dir.display()); + try!(ui.status(Status::Missing, + format!("artifact for {} was not found in {}", + ident.archive_name().unwrap(), + archives_dir.display()))); return Err(Error::FileNotFound(archives_dir.to_string_lossy() .into_owned())); } diff --git a/components/hab/src/command/sup.rs b/components/hab/src/command/sup.rs index fa7b903843..6538f62fad 100644 --- a/components/hab/src/command/sup.rs +++ b/components/hab/src/command/sup.rs @@ -75,17 +75,15 @@ mod inner { use std::env; use std::ffi::OsString; - use ansi_term::Colour::Yellow; use common::ui::UI; use error::{Error, Result}; pub fn start(_ui: &mut UI, _args: Vec) -> Result<()> { let subcmd = env::args().nth(1).unwrap_or("".to_string()); - let msg = format!("∅ Launching a native Supervisor on this operating system is not yet \ - supported. Try running this command again on a 64-bit Linux \ - operating system.\n"); - println!("{}", Yellow.bold().paint(msg)); + try!(ui.warn("Launching a native Supervisor on this operating system is not yet supported. \ + Try running this command again on a 64-bit Linux operating system.")); + try!(ui.br()); Err(Error::SubcommandNotSupported(subcmd)) } } diff --git a/components/hab/src/main.rs b/components/hab/src/main.rs index fdeee51f7e..2319f5bea5 100644 --- a/components/hab/src/main.rs +++ b/components/hab/src/main.rs @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -extern crate ansi_term; #[macro_use] extern crate clap; extern crate env_logger; @@ -29,7 +28,6 @@ use std::path::Path; use std::str::FromStr; use std::thread; -use ansi_term::Colour::Red; use clap::ArgMatches; use common::ui::UI; @@ -62,17 +60,16 @@ const MAX_FILE_UPLOAD_SIZE_BYTES: u64 = 4096; fn main() { env_logger::init().unwrap(); + let mut ui = UI::default(); thread::spawn(|| analytics::instrument_subcommand()); - if let Err(e) = start() { - println!("{}", - Red.bold().paint(format!("✗✗✗\n✗✗✗ {}\n✗✗✗", e))); + if let Err(e) = start(&mut ui) { + ui.fatal(e).unwrap(); std::process::exit(1) } } -fn start() -> Result<()> { - let mut ui = UI::default(); - try!(exec_subcommand_if_called(&mut ui)); +fn start(ui: &mut UI) -> Result<()> { + try!(exec_subcommand_if_called(ui)); let (args, remaining_args) = raw_parse_args(); debug!("clap cli args: {:?}", &args); @@ -83,35 +80,35 @@ fn start() -> Result<()> { e.exit(); }); match app_matches.subcommand() { - ("apply", Some(m)) => try!(sub_config_apply(&mut ui, m)), + ("apply", Some(m)) => try!(sub_config_apply(ui, m)), ("cli", Some(matches)) => { match matches.subcommand() { - ("setup", Some(_)) => try!(sub_cli_setup(&mut ui)), + ("setup", Some(_)) => try!(sub_cli_setup(ui)), _ => unreachable!(), } } ("config", Some(matches)) => { match matches.subcommand() { - ("apply", Some(m)) => try!(sub_config_apply(&mut ui, m)), + ("apply", Some(m)) => try!(sub_config_apply(ui, m)), _ => unreachable!(), } } ("file", Some(matches)) => { match matches.subcommand() { - ("upload", Some(m)) => try!(sub_file_upload(&mut ui, m)), + ("upload", Some(m)) => try!(sub_file_upload(ui, m)), _ => unreachable!(), } } - ("install", Some(m)) => try!(sub_pkg_install(&mut ui, m)), + ("install", Some(m)) => try!(sub_pkg_install(ui, m)), ("origin", Some(matches)) => { match matches.subcommand() { ("key", Some(m)) => { match m.subcommand() { - ("download", Some(sc)) => try!(sub_origin_key_download(&mut ui, sc)), + ("download", Some(sc)) => try!(sub_origin_key_download(ui, sc)), ("export", Some(sc)) => try!(sub_origin_key_export(sc)), - ("generate", Some(sc)) => try!(sub_origin_key_generate(&mut ui, sc)), - ("import", Some(_)) => try!(sub_origin_key_import(&mut ui)), - ("upload", Some(sc)) => try!(sub_origin_key_upload(&mut ui, sc)), + ("generate", Some(sc)) => try!(sub_origin_key_generate(ui, sc)), + ("import", Some(_)) => try!(sub_origin_key_import(ui)), + ("upload", Some(sc)) => try!(sub_origin_key_upload(ui, sc)), _ => unreachable!(), } } @@ -120,18 +117,18 @@ fn start() -> Result<()> { } ("pkg", Some(matches)) => { match matches.subcommand() { - ("binlink", Some(m)) => try!(sub_pkg_binlink(&mut ui, m)), - ("build", Some(m)) => try!(sub_pkg_build(&mut ui, m)), + ("binlink", Some(m)) => try!(sub_pkg_binlink(ui, m)), + ("build", Some(m)) => try!(sub_pkg_build(ui, m)), ("exec", Some(m)) => try!(sub_pkg_exec(m, remaining_args)), - ("export", Some(m)) => try!(sub_pkg_export(&mut ui, m)), + ("export", Some(m)) => try!(sub_pkg_export(ui, m)), ("hash", Some(m)) => try!(sub_pkg_hash(m)), - ("install", Some(m)) => try!(sub_pkg_install(&mut ui, m)), + ("install", Some(m)) => try!(sub_pkg_install(ui, m)), ("path", Some(m)) => try!(sub_pkg_path(m)), ("provides", Some(m)) => try!(sub_pkg_provides(m)), ("search", Some(m)) => try!(sub_pkg_search(m)), - ("sign", Some(m)) => try!(sub_pkg_sign(&mut ui, m)), - ("upload", Some(m)) => try!(sub_pkg_upload(&mut ui, m)), - ("verify", Some(m)) => try!(sub_pkg_verify(&mut ui, m)), + ("sign", Some(m)) => try!(sub_pkg_sign(ui, m)), + ("upload", Some(m)) => try!(sub_pkg_upload(ui, m)), + ("verify", Some(m)) => try!(sub_pkg_verify(ui, m)), _ => unreachable!(), } } @@ -140,8 +137,8 @@ fn start() -> Result<()> { ("key", Some(m)) => { match m.subcommand() { ("export", Some(sc)) => try!(sub_ring_key_export(sc)), - ("import", Some(_)) => try!(sub_ring_key_import(&mut ui)), - ("generate", Some(sc)) => try!(sub_ring_key_generate(&mut ui, sc)), + ("import", Some(_)) => try!(sub_ring_key_import(ui)), + ("generate", Some(sc)) => try!(sub_ring_key_generate(ui, sc)), _ => unreachable!(), } } @@ -152,19 +149,19 @@ fn start() -> Result<()> { match matches.subcommand() { ("key", Some(m)) => { match m.subcommand() { - ("generate", Some(sc)) => try!(sub_service_key_generate(&mut ui, sc)), + ("generate", Some(sc)) => try!(sub_service_key_generate(ui, sc)), _ => unreachable!(), } } _ => unreachable!(), } } - ("setup", Some(_)) => try!(sub_cli_setup(&mut ui)), + ("setup", Some(_)) => try!(sub_cli_setup(ui)), ("user", Some(matches)) => { match matches.subcommand() { ("key", Some(m)) => { match m.subcommand() { - ("generate", Some(sc)) => try!(sub_user_key_generate(&mut ui, sc)), + ("generate", Some(sc)) => try!(sub_user_key_generate(ui, sc)), _ => unreachable!(), } } @@ -396,7 +393,7 @@ fn sub_pkg_exec(m: &ArgMatches, cmd_args: Vec) -> Result<()> { fn sub_pkg_export(ui: &mut UI, m: &ArgMatches) -> Result<()> { let ident = try!(PackageIdent::from_str(m.value_of("PKG_IDENT").unwrap())); let format = &m.value_of("FORMAT").unwrap(); - let export_fmt = try!(command::pkg::export::format_for(&format)); + let export_fmt = try!(command::pkg::export::format_for(ui, &format)); command::pkg::export::start(ui, &ident, &export_fmt) }