From d14b6871a5bd81efdc9ff0b1ae75a1ba7bc7f2b8 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 15 Jul 2016 10:11:14 +0200 Subject: [PATCH] Cleanup of colour code. Use is_a_tty. (#1621) * Cleanup of colour code. Use is_a_tty. * Fix test build. * Another fix. --- Cargo.lock | 11 +++++++++++ Cargo.toml | 1 + ethcore/src/client/client.rs | 4 ++-- ethcore/src/miner/miner.rs | 4 ++-- ethcore/src/service.rs | 2 +- parity/configuration.rs | 2 +- parity/informant.rs | 3 ++- parity/main.rs | 11 ++++++----- parity/setup_log.rs | 24 ++++++++++++++++-------- parity/signer.rs | 3 +-- rpc/src/v1/tests/mocked/ethcore.rs | 2 +- util/src/log.rs | 27 ++------------------------- 12 files changed, 46 insertions(+), 48 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f7e9a95dd..8bc8c86ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -21,6 +21,7 @@ dependencies = [ "ethsync 1.3.0", "fdlimit 0.1.0", "hyper 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)", + "isatty 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "json-ipc-server 0.2.4 (git+https://github.com/ethcore/json-ipc-server.git)", "lazy_static 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", @@ -585,6 +586,16 @@ dependencies = [ "xmltree 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "isatty" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "kernel32-sys 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", + "libc 0.2.12 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "itertools" version = "0.4.13" diff --git a/Cargo.toml b/Cargo.toml index 77ea5e9c9..a4de88a27 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ semver = "0.2" ansi_term = "0.7" lazy_static = "0.2" regex = "0.1" +isatty = "0.1" ctrlc = { git = "https://github.com/ethcore/rust-ctrlc.git" } fdlimit = { path = "util/fdlimit" } ethcore = { path = "ethcore" } diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 344add17b..34f369729 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -35,7 +35,7 @@ use util::rlp::{RlpStream, Rlp, UntrustedRlp}; use util::journaldb; use util::journaldb::JournalDB; use util::kvdb::*; -use util::{Applyable, Stream, View, PerfTimer, Itertools, Colour}; +use util::{Stream, View, PerfTimer, Itertools, Colour}; use util::{Mutex, RwLock}; // other @@ -618,7 +618,7 @@ impl Client { } } *previous_enode = Some(url.into()); - info!(target: "mode", "Public node URL: {}", url.apply(Colour::White.bold())); + info!(target: "mode", "Public node URL: {}", Colour::White.bold().paint(url)); } } diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index b3f2765e2..97ba6c082 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -116,7 +116,7 @@ impl GasPriceCalibrator { let wei_per_usd: f32 = 1.0e18 / usd_per_eth; let gas_per_tx: f32 = 21000.0; let wei_per_gas: f32 = wei_per_usd * usd_per_tx / gas_per_tx; - info!(target: "miner", "Updated conversion rate to Ξ1 = {} ({} wei/gas)", format!("US${}", usd_per_eth).apply(Colour::White.bold()), format!("{}", wei_per_gas).apply(Colour::Yellow.bold())); + info!(target: "miner", "Updated conversion rate to Ξ1 = {} ({} wei/gas)", Colour::White.bold().paint(format!("US${}", usd_per_eth)), Colour::Yellow.bold().paint(format!("{}", wei_per_gas))); set_price(U256::from_dec_str(&format!("{:.0}", wei_per_gas)).unwrap()); }) { self.next_calibration = Instant::now() + self.options.recalibration_period; @@ -775,7 +775,7 @@ impl MinerService for Miner { let n = sealed.header().number(); let h = sealed.header().hash(); try!(chain.import_sealed_block(sealed)); - info!(target: "miner", "Mined block imported OK. #{}: {}", format!("{}", n).apply(Colour::White.bold()), h.hex().apply(Colour::White.bold())); + info!(target: "miner", "Mined block imported OK. #{}: {}", Colour::White.bold().paint(format!("{}", n)), Colour::White.bold().paint(h.hex())); Ok(()) }) } diff --git a/ethcore/src/service.rs b/ethcore/src/service.rs index 3a5d555d8..a09e9c887 100644 --- a/ethcore/src/service.rs +++ b/ethcore/src/service.rs @@ -54,7 +54,7 @@ impl ClientService { let io_service = try!(IoService::::start()); panic_handler.forward_from(&io_service); - info!("Configured for {} using {} engine", spec.name.clone().apply(Colour::White.bold()), spec.engine.name().apply(Colour::Yellow.bold())); + info!("Configured for {} using {} engine", Colour::White.bold().paint(spec.name.clone()), Colour::Yellow.bold().paint(spec.engine.name())); let client = try!(Client::new(config, spec, db_path, miner, io_service.channel())); panic_handler.forward_from(client.deref()); let client_io = Arc::new(ClientIoHandler { diff --git a/parity/configuration.rs b/parity/configuration.rs index f41784bc1..cbb21db27 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -195,7 +195,7 @@ impl Configuration { let wei_per_usd: f32 = 1.0e18 / usd_per_eth; let gas_per_tx: f32 = 21000.0; let wei_per_gas: f32 = wei_per_usd * usd_per_tx / gas_per_tx; - info!("Using a fixed conversion rate of Ξ1 = {} ({} wei/gas)", format!("US${}", usd_per_eth).apply(White.bold()), format!("{}", wei_per_gas).apply(Yellow.bold())); + info!("Using a fixed conversion rate of Ξ1 = {} ({} wei/gas)", White.bold().paint(format!("US${}", usd_per_eth)), Yellow.bold().paint(format!("{}", wei_per_gas))); GasPricer::Fixed(U256::from_dec_str(&format!("{:.0}", wei_per_gas)).unwrap()) } } diff --git a/parity/informant.rs b/parity/informant.rs index 32d730295..e8dc14d9a 100644 --- a/parity/informant.rs +++ b/parity/informant.rs @@ -20,6 +20,7 @@ use self::ansi_term::Style; use std::time::{Instant, Duration}; use std::ops::{Deref, DerefMut}; +use isatty::{stdout_isatty}; use ethsync::{SyncStatus, NetworkConfiguration}; use util::{Uint, RwLock}; use ethcore::client::*; @@ -91,7 +92,7 @@ impl Informant { let mut write_report = self.report.write(); let report = client.report(); - let paint = |c: Style, t: String| match self.with_color { + let paint = |c: Style, t: String| match self.with_color && stdout_isatty() { true => format!("{}", c.paint(t)), false => t, }; diff --git a/parity/main.rs b/parity/main.rs index 2417f062a..1566aed73 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -53,6 +53,7 @@ extern crate ansi_term; #[macro_use] extern crate lazy_static; extern crate regex; +extern crate isatty; #[cfg(feature = "dapps")] extern crate ethcore_dapps; @@ -82,7 +83,7 @@ use std::thread::sleep; use std::time::Duration; use rustc_serialize::hex::FromHex; use ctrlc::CtrlC; -use util::{H256, ToPretty, PayloadInfo, Bytes, Colour, Applyable, version, journaldb}; +use util::{H256, ToPretty, PayloadInfo, Bytes, Colour, version, journaldb}; use util::panics::{MayPanic, ForwardPanic, PanicHandler}; use ethcore::client::{BlockID, BlockChainClient, ClientConfig, get_db_path, BlockImportError, ChainNotify, Mode}; @@ -192,18 +193,18 @@ fn execute_client(conf: Configuration, spec: Spec, client_config: ClientConfig) // Raise fdlimit unsafe { ::fdlimit::raise_fd_limit(); } - info!("Starting {}", format!("{}", version()).apply(Colour::White.bold())); - info!("Using state DB journalling strategy {}", match client_config.pruning { + info!("Starting {}", Colour::White.bold().paint(format!("{}", version()))); + info!("Using state DB journalling strategy {}", Colour::White.bold().paint(match client_config.pruning { journaldb::Algorithm::Archive => "archive", journaldb::Algorithm::EarlyMerge => "light", journaldb::Algorithm::OverlayRecent => "fast", journaldb::Algorithm::RefCounted => "basic", - }.apply(Colour::White.bold())); + })); // Display warning about using experimental journaldb types match client_config.pruning { journaldb::Algorithm::EarlyMerge | journaldb::Algorithm::RefCounted => { - warn!("Your chosen strategy is {}! You can re-run with --pruning to change.", "unstable".apply(Colour::Red.bold())); + warn!("Your chosen strategy is {}! You can re-run with --pruning to change.", Colour::Red.bold().paint("unstable")); } _ => {} } diff --git a/parity/setup_log.rs b/parity/setup_log.rs index fb45bd549..fc88a6745 100644 --- a/parity/setup_log.rs +++ b/parity/setup_log.rs @@ -19,11 +19,12 @@ use std::env; use std::sync::Arc; use std::fs::File; use std::io::Write; +use isatty::{stderr_isatty}; use time; use env_logger::LogBuilder; use regex::Regex; use util::RotatingLogger; -use util::log::{Applyable, Colour}; +use util::log::Colour; /// Sets up the logger pub fn setup_log(init: &Option, enable_color: bool, log_to_file: &Option) -> Arc { @@ -47,19 +48,26 @@ pub fn setup_log(init: &Option, enable_color: bool, log_to_file: &Option builder.parse(s); } - let logs = Arc::new(RotatingLogger::new(levels, enable_color)); + let enable_color = enable_color && stderr_isatty(); + let logs = Arc::new(RotatingLogger::new(levels)); let logger = logs.clone(); let maybe_file = log_to_file.as_ref().map(|f| File::create(f).unwrap_or_else(|_| die!("Cannot write to log file given: {}", f))); let format = move |record: &LogRecord| { let timestamp = time::strftime("%Y-%m-%d %H:%M:%S %Z", &time::now()).unwrap(); - let format = if max_log_level() <= LogLevelFilter::Info { - format!("{}{}", timestamp.apply(Colour::Black.bold()), record.args()) + let with_color = if max_log_level() <= LogLevelFilter::Info { + format!("{}{}", Colour::Black.bold().paint(timestamp), record.args()) } else { - format!("{}{}:{}: {}", timestamp.apply(Colour::Black.bold()), record.level(), record.target(), record.args()) + format!("{}{}:{}: {}", Colour::Black.bold().paint(timestamp), record.level(), record.target(), record.args()) + }; + + let removed_color = kill_color(with_color.as_ref()); + + let ret = match enable_color { + true => with_color, + false => removed_color.clone(), }; - let removed_color = kill_color(format.as_ref()); if let Some(mut file) = maybe_file.as_ref() { // ignore errors - there's nothing we can do let _ = file.write_all(removed_color.as_bytes()); @@ -67,7 +75,7 @@ pub fn setup_log(init: &Option, enable_color: bool, log_to_file: &Option } logger.append(removed_color); - format + ret }; builder.format(format); builder.init().unwrap(); @@ -84,7 +92,7 @@ fn kill_color(s: &str) -> String { #[test] fn should_remove_colour() { let before = "test"; - let after = kill_color(&before.apply(Colour::Red.bold())); + let after = kill_color(&Colour::Red.bold().paint(before)); assert_eq!(after, "test"); } diff --git a/parity/signer.rs b/parity/signer.rs index 8cc63a857..aaad81389 100644 --- a/parity/signer.rs +++ b/parity/signer.rs @@ -20,7 +20,6 @@ use std::path::PathBuf; use ansi_term::Colour; use util::panics::{ForwardPanic, PanicHandler}; use util::path::restrict_permissions_owner; -use util::Applyable; use rpc_apis; use ethcore_signer as signer; use die::*; @@ -60,7 +59,7 @@ pub fn new_token(path: String) -> io::Result<()> { let mut codes = try!(signer::AuthCodes::from_file(&path)); let code = try!(codes.generate_new()); try!(codes.to_file(&path)); - println!("This key code will authorise your System Signer UI: {}", code.apply(Colour::White.bold())); + info!("This key code will authorise your System Signer UI: {}", Colour::White.bold().paint(code)); Ok(()) } diff --git a/rpc/src/v1/tests/mocked/ethcore.rs b/rpc/src/v1/tests/mocked/ethcore.rs index cbdddc2b0..5b88e8756 100644 --- a/rpc/src/v1/tests/mocked/ethcore.rs +++ b/rpc/src/v1/tests/mocked/ethcore.rs @@ -32,7 +32,7 @@ fn client_service() -> Arc { } fn logger() -> Arc { - Arc::new(RotatingLogger::new("rpc=trace".to_owned(), false)) + Arc::new(RotatingLogger::new("rpc=trace".to_owned())) } fn settings() -> Arc { diff --git a/util/src/log.rs b/util/src/log.rs index b4169f91c..97f3e347f 100644 --- a/util/src/log.rs +++ b/util/src/log.rs @@ -17,35 +17,13 @@ //! Common log helper functions use std::env; -use std::borrow::Cow; use rlog::LogLevelFilter; use env_logger::LogBuilder; -use std::sync::atomic::{Ordering, AtomicBool}; use arrayvec::ArrayVec; pub use ansi_term::{Colour, Style}; use parking_lot::{RwLock, RwLockReadGuard}; -lazy_static! { - static ref USE_COLOR: AtomicBool = AtomicBool::new(false); -} - -/// Something which can be apply()ed. -pub trait Applyable: AsRef { - /// Apply the style `c` to ourself, returning us styled in that manner. - fn apply(&self, c: Style) -> Cow; -} - -impl> Applyable for T { - fn apply(&self, c: Style) -> Cow { - let s = self.as_ref(); - match USE_COLOR.load(Ordering::Relaxed) { - true => Cow::Owned(format!("{}", c.paint(s))), - false => Cow::Borrowed(s), - } - } -} - lazy_static! { static ref LOG_DUMMY: bool = { let mut builder = LogBuilder::new(); @@ -81,8 +59,7 @@ impl RotatingLogger { /// Creates new `RotatingLogger` with given levels. /// It does not enforce levels - it's just read only. - pub fn new(levels: String, enable_color: bool) -> Self { - USE_COLOR.store(enable_color, Ordering::Relaxed); + pub fn new(levels: String) -> Self { RotatingLogger { levels: levels, logs: RwLock::new(ArrayVec::<[_; LOG_SIZE]>::new()), @@ -111,7 +88,7 @@ mod test { use super::RotatingLogger; fn logger() -> RotatingLogger { - RotatingLogger::new("test".to_owned(), false) + RotatingLogger::new("test".to_owned()) } #[test]