Cleanup of colour code. Use is_a_tty. (#1621)

* Cleanup of colour code. Use is_a_tty.

* Fix test build.

* Another fix.

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"

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" }

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));
 	}
 }
 

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(())
 		})
 	}

ethcore/src/service.rs

@@ -54,7 +54,7 @@ impl ClientService {
 		let io_service = try!(IoService::<ClientIoMessage>::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 {

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())
 					}
 				}

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,
 		};

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"));
 		}
 		_ => {}
 	}

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<String>, enable_color: bool, log_to_file: &Option<String>) -> Arc<RotatingLogger> {
@@ -47,19 +48,26 @@ pub fn setup_log(init: &Option<String>, 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<String>, 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");
 }
 

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(())
 }
 

rpc/src/v1/tests/mocked/ethcore.rs

@@ -32,7 +32,7 @@ fn client_service() -> Arc<TestBlockChainClient> {
 }
 
 fn logger() -> Arc<RotatingLogger> {
-	Arc::new(RotatingLogger::new("rpc=trace".to_owned(), false))
+	Arc::new(RotatingLogger::new("rpc=trace".to_owned()))
 }
 
 fn settings() -> Arc<NetworkSettings> {

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<str> {
-	/// Apply the style `c` to ourself, returning us styled in that manner.
-	fn apply(&self, c: Style) -> Cow<str>;
-}
-
-impl<T: AsRef<str>> Applyable for T {
-    fn apply(&self, c: Style) -> Cow<str> {
-        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]