Fix logging (#1590)

* Strip colour for everywhere except the terminal.

* Log to file.

Fixes #1560.

* Fix indentation.

[ci:skip]

* Remove unnecessary clone()ing.

* Update setup_log.rs

* remove unnecessary mutex in logging (#1601)

Cargo.lock

@@ -22,9 +22,11 @@ dependencies = [
  "fdlimit 0.1.0",
  "hyper 0.8.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)",
  "num_cpus 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)",
  "number_prefix 0.2.5 (registry+https://github.com/rust-lang/crates.io-index)",
+ "regex 0.1.68 (registry+https://github.com/rust-lang/crates.io-index)",
  "rpassword 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)",
  "rustc_version 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)",

Cargo.toml

@@ -36,6 +36,8 @@ ethcore-ipc = { path = "ipc/rpc" }
 ethcore-ipc-hypervisor = { path = "ipc/hypervisor" }
 json-ipc-server = { git = "https://github.com/ethcore/json-ipc-server.git" }
 ansi_term = "0.7"
+lazy_static = "0.2"
+regex = "0.1"
 
 [target.'cfg(windows)'.dependencies]
 winapi = "0.2"

parity/cli.rs

@@ -264,6 +264,8 @@ Legacy Options:
 Miscellaneous Options:
   -l --logging LOGGING     Specify the logging level. Must conform to the same
                            format as RUST_LOG.
+  --log-file FILENAME      Specify a filename into which logging should be
+                           directed.
   --no-color               Don't use terminal color codes in output.
   -v --version             Show information about version.
   -h --help                Show this screen.
@@ -339,7 +341,7 @@ pub struct Args {
 	pub flag_author: Option<String>,
 	pub flag_usd_per_tx: String,
 	pub flag_usd_per_eth: String,
-  pub flag_price_update_period: String,
+	pub flag_price_update_period: String,
 	pub flag_gas_floor_target: String,
 	pub flag_gas_cap: String,
 	pub flag_extra_data: Option<String>,
@@ -351,6 +353,7 @@ pub struct Args {
 	pub flag_to: String,
 	pub flag_format: Option<String>,
 	pub flag_jitvm: bool,
+	pub flag_log_file: Option<String>,
 	pub flag_no_color: bool,
 	pub flag_no_network: bool,
 	// legacy...

parity/main.rs

@@ -50,6 +50,9 @@ extern crate ethcore_rpc;
 
 extern crate ethcore_signer;
 extern crate ansi_term;
+#[macro_use]
+extern crate lazy_static;
+extern crate regex;
 
 #[cfg(feature = "dapps")]
 extern crate ethcore_dapps;
@@ -184,7 +187,7 @@ fn execute_client(conf: Configuration, spec: Spec, client_config: ClientConfig)
 	let panic_handler = PanicHandler::new_in_arc();
 
 	// Setup logging
-	let logger = setup_log::setup_log(&conf.args.flag_logging, conf.have_color());
+	let logger = setup_log::setup_log(&conf.args.flag_logging, conf.have_color(), &conf.args.flag_log_file);
 	// Raise fdlimit
 	unsafe { ::fdlimit::raise_fd_limit(); }
 
@@ -342,7 +345,7 @@ fn execute_export(conf: Configuration) {
 	let panic_handler = PanicHandler::new_in_arc();
 
 	// Setup logging
-	let _logger = setup_log::setup_log(&conf.args.flag_logging, conf.have_color());
+	let _logger = setup_log::setup_log(&conf.args.flag_logging, conf.have_color(), &conf.args.flag_log_file);
 	// Raise fdlimit
 	unsafe { ::fdlimit::raise_fd_limit(); }
 
@@ -403,7 +406,7 @@ fn execute_import(conf: Configuration) {
 	let panic_handler = PanicHandler::new_in_arc();
 
 	// Setup logging
-	let _logger = setup_log::setup_log(&conf.args.flag_logging, conf.have_color());
+	let _logger = setup_log::setup_log(&conf.args.flag_logging, conf.have_color(), &conf.args.flag_log_file);
 	// Raise fdlimit
 	unsafe { ::fdlimit::raise_fd_limit(); }
 

parity/setup_log.rs

@@ -17,12 +17,16 @@
 
 use std::env;
 use std::sync::Arc;
+use std::fs::File;
+use std::io::Write;
 use time;
 use env_logger::LogBuilder;
+use regex::Regex;
 use util::RotatingLogger;
+use util::log::{Applyable, Colour};
 
 /// Sets up the logger
-pub fn setup_log(init: &Option<String>, enable_color: bool) -> Arc<RotatingLogger> {
+pub fn setup_log(init: &Option<String>, enable_color: bool, log_to_file: &Option<String>) -> Arc<RotatingLogger> {
 	use rlog::*;
 
 	let mut levels = String::new();
@@ -45,14 +49,24 @@ pub fn setup_log(init: &Option<String>, enable_color: bool) -> Arc<RotatingLogge
 
 	let logs = Arc::new(RotatingLogger::new(levels, enable_color));
 	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, record.args())
+			format!("{}{}", timestamp.apply(Colour::Black.bold()), record.args())
 		} else {
-			format!("{}{}:{}: {}", timestamp, record.level(), record.target(), record.args())
+			format!("{}{}:{}: {}", timestamp.apply(Colour::Black.bold()), record.level(), record.target(), record.args())
 		};
-		logger.append(format.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());
+			let _ = file.write_all(b"\n");
+		}
+		logger.append(removed_color);
+
 		format
     };
 	builder.format(format);
@@ -60,3 +74,23 @@ pub fn setup_log(init: &Option<String>, enable_color: bool) -> Arc<RotatingLogge
 	logs
 }
 
+fn kill_color(s: &str) -> String {
+	lazy_static! {
+		static ref RE: Regex = Regex::new("\x1b\\[[^m]+m").unwrap();
+	}
+	RE.replace_all(s, "")
+}
+
+#[test]
+fn should_remove_colour() {
+	let before = "test";
+	let after = kill_color(&before.apply(Colour::Red.bold()));
+	assert_eq!(after, "test");
+}
+
+#[test]
+fn should_remove_multiple_colour() {
+	let t = format!("{} {}", Colour::Red.bold().paint("test"), Colour::White.normal().paint("again"));
+	let after = kill_color(&t);
+	assert_eq!(after, "test again");
+}