From 687035657d071f2b71fbb2a3a00a353294e710c7 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Sat, 21 Jun 2014 18:53:07 -0700 Subject: [PATCH] Terminal colors --- libs/hammer.rs | 2 +- src/bin/cargo-compile.rs | 2 +- src/bin/cargo.rs | 59 +++++++++++++++++++--------------- src/cargo/core/mod.rs | 5 +++ src/cargo/core/shell.rs | 57 ++++++++++++++++++--------------- src/cargo/lib.rs | 62 ++++++++++++++++++++++-------------- src/cargo/ops/cargo_rustc.rs | 18 +++++++---- src/cargo/util/errors.rs | 10 ++---- tests/support/mod.rs | 7 ++-- tests/test_cargo_compile.rs | 7 ++-- tests/test_shell.rs | 24 ++++++++++---- 11 files changed, 149 insertions(+), 104 deletions(-) diff --git a/libs/hammer.rs b/libs/hammer.rs index 6d26b74a6..6c442daa3 160000 --- a/libs/hammer.rs +++ b/libs/hammer.rs @@ -1 +1 @@ -Subproject commit 6d26b74a66ce16f9d146a407a2384aa6ef431f7c +Subproject commit 6c442daa3550d791333c4e382587d63fd12c89d2 diff --git a/src/bin/cargo-compile.rs b/src/bin/cargo-compile.rs index c4cff306e..d410131e1 100644 --- a/src/bin/cargo-compile.rs +++ b/src/bin/cargo-compile.rs @@ -22,7 +22,7 @@ pub struct Options { manifest_path: Option } -hammer_config!(Options) +hammer_config!(Options "Compile the current project") fn main() { execute_main_without_stdin(execute); diff --git a/src/bin/cargo.rs b/src/bin/cargo.rs index dde0dbe27..ffe3f454a 100644 --- a/src/bin/cargo.rs +++ b/src/bin/cargo.rs @@ -11,7 +11,7 @@ use hammer::{FlagConfig,FlagConfiguration}; use std::os; use std::io::process::{Command,InheritFd,ExitStatus,ExitSignal}; use serialize::Encodable; -use cargo::{NoFlags,execute_main_without_stdin,handle_error}; +use cargo::{GlobalFlags, NoFlags, execute_main_without_stdin, handle_error}; use cargo::util::important_paths::find_project; use cargo::util::{CliError, CliResult, Require, config, human}; @@ -37,32 +37,41 @@ fn execute() { Err(err) => return handle_error(err, false) }; - if cmd == "config-for-key".to_str() { - log!(4, "cmd == config-for-key"); - execute_main_without_stdin(config_for_key) - } - else if cmd == "config-list".to_str() { - log!(4, "cmd == config-list"); - execute_main_without_stdin(config_list) - } - else if cmd == "locate-project".to_str() { - log!(4, "cmd == locate-project"); - execute_main_without_stdin(locate_project) - } - else { - let command = Command::new(format!("cargo-{}", cmd)) - .args(args.as_slice()) - .stdin(InheritFd(0)) - .stdout(InheritFd(1)) - .stderr(InheritFd(2)) - .status(); + match cmd.as_slice() { + "config-for-key" => { + log!(4, "cmd == config-for-key"); + execute_main_without_stdin(config_for_key) + }, + "config-list" => { + log!(4, "cmd == config-list"); + execute_main_without_stdin(config_list) + }, + "locate-project" => { + log!(4, "cmd == locate-project"); + execute_main_without_stdin(locate_project) + }, + "--help" | "-h" | "help" | "-?" => { + println!("Commands:"); + println!(" compile # compile the current project\n"); - match command { - Ok(ExitStatus(0)) => (), - Ok(ExitStatus(i)) | Ok(ExitSignal(i)) => { - handle_error(CliError::new("", i as uint), false) + let (_, options) = hammer::usage::(false); + println!("Options (for all commands):\n\n{}", options); + }, + _ => { + let command = Command::new(format!("cargo-{}", cmd)) + .args(args.as_slice()) + .stdin(InheritFd(0)) + .stdout(InheritFd(1)) + .stderr(InheritFd(2)) + .status(); + + match command { + Ok(ExitStatus(0)) => (), + Ok(ExitStatus(i)) | Ok(ExitSignal(i)) => { + handle_error(CliError::new("", i as uint), false) + } + Err(_) => handle_error(CliError::new("No such subcommand", 127), false) } - Err(_) => handle_error(CliError::new("No such subcommand", 127), false) } } } diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index 9d041299e..fd6ff3211 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -30,6 +30,11 @@ pub use self::summary::{ Summary }; +pub use self::shell::{ + Shell, + ShellConfig +}; + pub use self::dependency::Dependency; pub use self::version_req::VersionReq; diff --git a/src/cargo/core/shell.rs b/src/cargo/core/shell.rs index 984bc7c9c..32d0f8ed0 100644 --- a/src/cargo/core/shell.rs +++ b/src/cargo/core/shell.rs @@ -1,8 +1,8 @@ use term; use term::{Terminal,color}; -use term::color::Color; +use term::color::{Color, BLACK}; use term::attr::Attr; -use std::io::IoResult; +use std::io::{IoResult, stderr}; pub struct ShellConfig { pub color: bool, @@ -10,31 +10,33 @@ pub struct ShellConfig { pub tty: bool } -enum AdequateTerminal { - NoColor(T), - Color(Box>) +enum AdequateTerminal { + NoColor(Box), + Color(Box>>) } -pub struct Shell { - terminal: AdequateTerminal, +pub struct Shell { + terminal: AdequateTerminal, config: ShellConfig } -impl Shell { - pub fn create(out: T, config: ShellConfig) -> Option> { +impl Shell { + pub fn create(out: Box, config: ShellConfig) -> Shell { if config.tty && config.color { - let term: Option> = Terminal::new(out); + let term: Option>> = Terminal::new(out); term.map(|t| Shell { - terminal: Color(box t as Box>), + terminal: Color(box t as Box>>), config: config + }).unwrap_or_else(|| { + Shell { terminal: NoColor(box stderr() as Box), config: config } }) } else { - Some(Shell { terminal: NoColor(out), config: config }) + Shell { terminal: NoColor(out), config: config } } } pub fn verbose(&mut self, - callback: |&mut Shell| -> IoResult<()>) -> IoResult<()> { + callback: |&mut Shell| -> IoResult<()>) -> IoResult<()> { if self.config.verbose { return callback(self) } @@ -42,22 +44,25 @@ impl Shell { Ok(()) } - pub fn say(&mut self, message: T, color: Color) -> IoResult<()> { + pub fn say(&mut self, message: T, color: Color) -> IoResult<()> { try!(self.reset()); - try!(self.fg(color)); - try!(self.write_line(message.as_slice())); + if color != BLACK { try!(self.fg(color)); } + try!(self.write_line(message.to_str().as_slice())); try!(self.reset()); try!(self.flush()); Ok(()) } } -impl Terminal for Shell { - fn new(out: T) -> Option> { - Shell::create(out, ShellConfig { - color: true, - verbose: false, - tty: false, +impl Terminal> for Shell { + fn new(out: Box) -> Option { + Some(Shell { + terminal: NoColor(out), + config: ShellConfig { + color: true, + verbose: false, + tty: false, + } }) } @@ -96,18 +101,18 @@ impl Terminal for Shell { } } - fn unwrap(self) -> T { + fn unwrap(self) -> Box { fail!("Can't unwrap a Shell"); } - fn get_ref<'a>(&'a self) -> &'a T { + fn get_ref<'a>(&'a self) -> &'a Box { match self.terminal { Color(ref c) => c.get_ref(), NoColor(ref w) => w } } - fn get_mut<'a>(&'a mut self) -> &'a mut T { + fn get_mut<'a>(&'a mut self) -> &'a mut Box { match self.terminal { Color(ref mut c) => c.get_mut(), NoColor(ref mut w) => w @@ -115,7 +120,7 @@ impl Terminal for Shell { } } -impl Writer for Shell { +impl Writer for Shell { fn write(&mut self, buf: &[u8]) -> IoResult<()> { match self.terminal { Color(ref mut c) => c.write(buf), diff --git a/src/cargo/lib.rs b/src/cargo/lib.rs index f2ed95c1b..694543808 100644 --- a/src/cargo/lib.rs +++ b/src/cargo/lib.rs @@ -21,7 +21,12 @@ extern crate hamcrest; use serialize::{Decoder, Encoder, Decodable, Encodable, json}; use std::io; -use hammer::{FlagDecoder, FlagConfig, UsageDecoder, HammerError, FlagConfiguration}; +use std::io::stderr; +use std::io::stdio::stderr_raw; +use hammer::{FlagDecoder, FlagConfig, UsageDecoder, HammerError}; + +use core::{Shell, ShellConfig}; +use term::color::{RED, BLACK}; pub use util::{CargoError, CliError, CliResult, human}; @@ -62,25 +67,12 @@ trait FlagParse : FlagConfig { fn decode_flags(d: &mut FlagDecoder) -> Result; } -trait FlagUsage : FlagConfig { - fn decode_usage(d: &mut UsageDecoder) -> Result; -} - -//impl> FlagParse for T {} -//impl> FlagUsage for T {} - impl> FlagParse for T { fn decode_flags(d: &mut FlagDecoder) -> Result { Decodable::decode(d) } } -impl> FlagUsage for T { - fn decode_usage(d: &mut UsageDecoder) -> Result { - Decodable::decode(d) - } -} - trait RepresentsFlags : FlagParse + Decodable {} impl> RepresentsFlags for T {} @@ -150,10 +142,16 @@ pub fn execute_main_without_stdin<'a, Err(e) => handle_error(e, true), Ok(val) => { if val.help { - println!("Usage:\n"); + let (desc, options) = hammer::usage::(true); - print!("{}", hammer::usage::(true)); - print!("{}", hammer::usage::(false)); + desc.map(|d| println!("{}\n", d)); + + println!("Options:\n"); + + print!("{}", options); + + let (_, options) = hammer::usage::(false); + print!("{}", options); } else { process_executed(call(exec, val.rest.as_slice()), val) } @@ -180,21 +178,37 @@ pub fn process_executed<'a, pub fn handle_error(err: CliError, verbose: bool) { log!(4, "handle_error; err={}", err); - let CliError { error, exit_code, .. } = err; - let _ = write!(&mut std::io::stderr(), "{}", error); + + let CliError { error, exit_code, unknown, .. } = err; + + let tty = stderr_raw().isatty(); + let stderr = box stderr() as Box; + + let config = ShellConfig { color: true, verbose: false, tty: tty }; + let mut shell = Shell::create(stderr, config); + + if unknown { + let _ = shell.say("An unknown error occurred", RED); + } else { + let _ = shell.say(error.to_str(), RED); + } + + if unknown && !verbose { + let _ = shell.say("\nTo learn more, run the command again with --verbose.", BLACK); + } if verbose { - error.cause().map(handle_cause); + handle_cause(error, &mut shell); } std::os::set_exit_status(exit_code as int); } -fn handle_cause(err: &CargoError) { - println!("\nCaused by:"); - println!(" {}", err.description()); +fn handle_cause(err: &CargoError, shell: &mut Shell) { + let _ = shell.say("\nCaused by:", BLACK); + let _ = shell.say(format!(" {}", err.description()), BLACK); - err.cause().map(handle_cause); + err.cause().map(|e| handle_cause(e, shell)); } fn args() -> Vec { diff --git a/src/cargo/ops/cargo_rustc.rs b/src/cargo/ops/cargo_rustc.rs index 99ff2cf8e..334467c3a 100644 --- a/src/cargo/ops/cargo_rustc.rs +++ b/src/cargo/ops/cargo_rustc.rs @@ -1,6 +1,6 @@ use std::os::args; use std::io; -use std::io::File; +use std::io::{File, IoError}; use std::str; use core::{Package, PackageSet, Target}; @@ -31,8 +31,14 @@ pub fn compile_packages(pkg: &Package, deps: &PackageSet) -> CargoResult<()> { // First ensure that the destination directory exists debug!("creating target dir; path={}", target_dir.display()); - try!(mk_target(&target_dir)); - try!(mk_target(&deps_target_dir)); + + try!(mk_target(&target_dir).chain_error(|| + internal(format!("Couldn't create the target directory for {} at {}", + pkg.get_name(), target_dir.display())))); + + try!(mk_target(&deps_target_dir).chain_error(|| + internal(format!("Couldn't create the directory for dependencies for {} at {}", + pkg.get_name(), deps_target_dir.display())))); let mut cx = Context { dest: &deps_target_dir, @@ -119,10 +125,8 @@ fn is_fresh(dep: &Package, loc: &Path, Ok((old_fingerprint == new_fingerprint, new_fingerprint)) } -fn mk_target(target: &Path) -> CargoResult<()> { - io::fs::mkdir_recursive(target, io::UserRWX).chain_error(|| { - internal("could not create target directory") - }) +fn mk_target(target: &Path) -> Result<(), IoError> { + io::fs::mkdir_recursive(target, io::UserRWX) } fn compile_custom(pkg: &Package, cmd: &str, cx: &Context) -> CargoResult<()> { diff --git a/src/cargo/util/errors.rs b/src/cargo/util/errors.rs index c3c1ea4fa..fb937f01f 100644 --- a/src/cargo/util/errors.rs +++ b/src/cargo/util/errors.rs @@ -240,6 +240,7 @@ pub type CliResult = Result; #[deriving(Show)] pub struct CliError { pub error: Box, + pub unknown: bool, pub exit_code: uint } @@ -263,13 +264,8 @@ impl CliError { } pub fn from_boxed(error: Box, code: uint) -> CliError { - let error = if error.is_human() { - error - } else { - human("An unknown error occurred").with_cause(error) - }; - - CliError { error: error, exit_code: code } + let human = error.is_human(); + CliError { error: error, exit_code: code, unknown: !human } } } diff --git a/tests/support/mod.rs b/tests/support/mod.rs index a0ad6aa3e..b1bc4c0ba 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -301,13 +301,14 @@ impl ham::SelfDescribing for ShellWrites { } } -impl<'a> ham::Matcher<&'a mut shell::Shell> for ShellWrites { - fn matches(&self, actual: &mut shell::Shell) +impl<'a> ham::Matcher<&'a [u8]> for ShellWrites { + fn matches(&self, actual: &[u8]) -> ham::MatchResult { use term::Terminal; - let actual = std::str::from_utf8_lossy(actual.get_ref().get_ref()); + println!("{}", actual); + let actual = std::str::from_utf8_lossy(actual); let actual = actual.to_str(); ham::expect(actual == self.expected, actual) } diff --git a/tests/test_cargo_compile.rs b/tests/test_cargo_compile.rs index c005a1e8e..f3a03a3c8 100644 --- a/tests/test_cargo_compile.rs +++ b/tests/test_cargo_compile.rs @@ -45,7 +45,7 @@ test!(cargo_compile_with_invalid_manifest { assert_that(p.cargo_process("cargo-compile"), execs() .with_status(101) - .with_stderr("Cargo.toml is not a valid manifest")); + .with_stderr("Cargo.toml is not a valid manifest\n")); }) test!(cargo_compile_without_manifest { @@ -55,7 +55,7 @@ test!(cargo_compile_without_manifest { execs() .with_status(102) .with_stderr("Could not find Cargo.toml in this directory or any \ - parent directory")); + parent directory\n")); }) test!(cargo_compile_with_invalid_code { @@ -73,7 +73,7 @@ src/foo.rs:1:1: 1:8 error: expected item but found `invalid` src/foo.rs:1 invalid rust code! ^~~~~~~ Could not execute process \ -`rustc src/foo.rs --crate-type bin --out-dir {} -L {} -L {}` (status=101)", +`rustc src/foo.rs --crate-type bin --out-dir {} -L {} -L {}` (status=101)\n", target.display(), target.display(), target.join("deps").display()).as_slice())); @@ -386,6 +386,7 @@ test!(custom_build_failure { Could not execute process `{}` (status=101) --- stderr task '
' failed at 'nope', src/foo.rs:2 + ", build.root().join("target/foo").display()))); }) diff --git a/tests/test_shell.rs b/tests/test_shell.rs index 4450055bc..6a83dd7aa 100644 --- a/tests/test_shell.rs +++ b/tests/test_shell.rs @@ -1,6 +1,6 @@ use support::{ResultTest,Tap,shell_writes}; use hamcrest::{assert_that}; -use std::io::{MemWriter,IoResult}; +use std::io::{MemWriter, BufWriter, IoResult}; use std::str::from_utf8_lossy; use cargo::core::shell::{Shell,ShellConfig}; use term::{Terminal,TerminfoTerminal,color}; @@ -8,27 +8,37 @@ use term::{Terminal,TerminfoTerminal,color}; fn setup() { } +fn writer(buf: &mut [u8]) -> Box { + box BufWriter::new(buf) as Box +} + test!(non_tty { let config = ShellConfig { color: true, verbose: true, tty: false }; - Shell::create(MemWriter::new(), config).assert().tap(|shell| { + let mut buf: Vec = Vec::from_elem(9, 0 as u8); + + Shell::create(writer(buf.as_mut_slice()), config).tap(|shell| { shell.say("Hey Alex", color::RED).assert(); - assert_that(shell, shell_writes("Hey Alex\n")); + assert_that(buf.as_slice(), shell_writes("Hey Alex\n")); }); }) test!(color_explicitly_disabled { let config = ShellConfig { color: false, verbose: true, tty: true }; - Shell::create(MemWriter::new(), config).assert().tap(|shell| { + let mut buf: Vec = Vec::from_elem(9, 0 as u8); + + Shell::create(writer(buf.as_mut_slice()), config).tap(|shell| { shell.say("Hey Alex", color::RED).assert(); - assert_that(shell, shell_writes("Hey Alex\n")); + assert_that(buf.as_slice(), shell_writes("Hey Alex\n")); }); }) test!(colored_shell { let config = ShellConfig { color: true, verbose: true, tty: true }; - Shell::create(MemWriter::new(), config).assert().tap(|shell| { + let mut buf: Vec = Vec::from_elem(22, 0 as u8); + + Shell::create(writer(buf.as_mut_slice()), config).tap(|shell| { shell.say("Hey Alex", color::RED).assert(); - assert_that(shell, shell_writes(colored_output("Hey Alex\n", + assert_that(buf.as_slice(), shell_writes(colored_output("Hey Alex\n", color::RED).assert())); }); })