sc-informant: Respect --disable-log-color (#3009)

Changes `sc-informant` to respect the `--disable-log-color` CLI flag.

Closes: #2795

---------

Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>

prdoc/pr_3009.prdoc

@@ -0,0 +1,10 @@
+title: "sc-informant: Respect `--disable-log-color`"
+
+doc:
+  - audience: Node Operator
+    description: |
+      Fixes some places that weren't respecting the `--disable-log-color` CLI flag.
+
+crates:
+  - name: "sc-informant"
+  - name: "sc-service"

substrate/client/cli/src/config.rs

@@ -27,8 +27,8 @@ use names::{Generator, Name};
 use sc_service::{
 	config::{
 		BasePath, Configuration, DatabaseSource, KeystoreConfig, NetworkConfiguration,
-		NodeKeyConfig, OffchainWorkerConfig, PrometheusConfig, PruningMode, Role, RpcMethods,
-		TelemetryEndpoints, TransactionPoolOptions, WasmExecutionMethod,
+		NodeKeyConfig, OffchainWorkerConfig, OutputFormat, PrometheusConfig, PruningMode, Role,
+		RpcMethods, TelemetryEndpoints, TransactionPoolOptions, WasmExecutionMethod,
 	},
 	BlocksPruning, ChainSpec, TracingReceiver,
 };
@@ -516,7 +516,7 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 			announce_block: self.announce_block()?,
 			role,
 			base_path,
-			informant_output_format: Default::default(),
+			informant_output_format: OutputFormat { enable_color: !self.disable_log_color()? },
 			runtime_cache_size,
 		})
 	}

substrate/client/informant/src/display.rs

@@ -142,37 +142,20 @@ impl<B: BlockT> InformantDisplay<B> {
 					("⚙️ ", format!("Preparing{}", speed), format!(", target=#{target}")),
 			};
 
-		if self.format.enable_color {
-			info!(
-				target: "substrate",
-				"{} {}{} ({} peers), best: #{} ({}), finalized #{} ({}), {} {}",
-				level,
-				Colour::White.bold().paint(&status),
-				target,
-				Colour::White.bold().paint(format!("{}", num_connected_peers)),
-				Colour::White.bold().paint(format!("{}", best_number)),
-				best_hash,
-				Colour::White.bold().paint(format!("{}", finalized_number)),
-				info.chain.finalized_hash,
-				Colour::Green.paint(format!("⬇ {}", TransferRateFormat(avg_bytes_per_sec_inbound))),
-				Colour::Red.paint(format!("⬆ {}", TransferRateFormat(avg_bytes_per_sec_outbound))),
-			)
-		} else {
-			info!(
-				target: "substrate",
-				"{} {}{} ({} peers), best: #{} ({}), finalized #{} ({}), ⬇ {} ⬆ {}",
-				level,
-				status,
-				target,
-				num_connected_peers,
-				best_number,
-				best_hash,
-				finalized_number,
-				info.chain.finalized_hash,
-				TransferRateFormat(avg_bytes_per_sec_inbound),
-				TransferRateFormat(avg_bytes_per_sec_outbound),
-			)
-		}
+		info!(
+			target: "substrate",
+			"{} {}{} ({} peers), best: #{} ({}), finalized #{} ({}), {} {}",
+			level,
+			self.format.print_with_color(Colour::White.bold(), status),
+			target,
+			self.format.print_with_color(Colour::White.bold(), num_connected_peers),
+			self.format.print_with_color(Colour::White.bold(), best_number),
+			best_hash,
+			self.format.print_with_color(Colour::White.bold(), finalized_number),
+			info.chain.finalized_hash,
+			self.format.print_with_color(Colour::Green, format!("⬇ {}", TransferRateFormat(avg_bytes_per_sec_inbound))),
+			self.format.print_with_color(Colour::Red, format!("⬆ {}", TransferRateFormat(avg_bytes_per_sec_outbound))),
+		)
 	}
 }
 

substrate/client/informant/src/lib.rs

@@ -18,7 +18,7 @@
 
 //! Console informant. Prints sync progress and block events. Runs on the calling thread.
 
-use ansi_term::Colour;
+use ansi_term::{Colour, Style};
 use futures::prelude::*;
 use futures_timer::Delay;
 use log::{debug, info, trace};
@@ -51,6 +51,47 @@ impl Default for OutputFormat {
 	}
 }
 
+enum ColorOrStyle {
+	Color(Colour),
+	Style(Style),
+}
+
+impl From<Colour> for ColorOrStyle {
+	fn from(value: Colour) -> Self {
+		Self::Color(value)
+	}
+}
+
+impl From<Style> for ColorOrStyle {
+	fn from(value: Style) -> Self {
+		Self::Style(value)
+	}
+}
+
+impl ColorOrStyle {
+	fn paint(&self, data: String) -> impl Display {
+		match self {
+			Self::Color(c) => c.paint(data),
+			Self::Style(s) => s.paint(data),
+		}
+	}
+}
+
+impl OutputFormat {
+	/// Print with color if `self.enable_color == true`.
+	fn print_with_color(
+		&self,
+		color: impl Into<ColorOrStyle>,
+		data: impl ToString,
+	) -> impl Display {
+		if self.enable_color {
+			color.into().paint(data.to_string()).to_string()
+		} else {
+			data.to_string()
+		}
+	}
+}
+
 /// Builds the informant and returns a `Future` that drives the informant.
 pub async fn build<B: BlockT, C, N, S>(client: Arc<C>, network: N, syncing: S, format: OutputFormat)
 where
@@ -89,11 +130,14 @@ where
 
 	futures::select! {
 		() = display_notifications.fuse() => (),
-		() = display_block_import(client).fuse() => (),
+		() = display_block_import(client, format).fuse() => (),
 	};
 }
 
-fn display_block_import<B: BlockT, C>(client: Arc<C>) -> impl Future<Output = ()>
+fn display_block_import<B: BlockT, C>(
+	client: Arc<C>,
+	format: OutputFormat,
+) -> impl Future<Output = ()>
 where
 	C: UsageProvider<B> + HeaderMetadata<B> + BlockchainEvents<B>,
 	<C as HeaderMetadata<B>>::Error: Display,
@@ -117,11 +161,11 @@ where
 				match maybe_ancestor {
 					Ok(ref ancestor) if ancestor.hash != *last_hash => info!(
 						"♻️  Reorg on #{},{} to #{},{}, common ancestor #{},{}",
-						Colour::Red.bold().paint(format!("{}", last_num)),
+						format.print_with_color(Colour::Red.bold(), last_num),
 						last_hash,
-						Colour::Green.bold().paint(format!("{}", n.header.number())),
+						format.print_with_color(Colour::Green.bold(), n.header.number()),
 						n.hash,
-						Colour::White.bold().paint(format!("{}", ancestor.number)),
+						format.print_with_color(Colour::White.bold(), ancestor.number),
 						ancestor.hash,
 					),
 					Ok(_) => {},
@@ -146,7 +190,7 @@ where
 			info!(
 				target: "substrate",
 				"✨ Imported #{} ({})",
-				Colour::White.bold().paint(format!("{}", n.header.number())),
+				format.print_with_color(Colour::White.bold(), n.header.number()),
 				n.hash,
 			);
 		}

substrate/client/service/src/config.rs

@@ -18,8 +18,11 @@
 
 //! Service configuration.
 
+use prometheus_endpoint::Registry;
+use sc_chain_spec::ChainSpec;
 pub use sc_client_db::{BlocksPruning, Database, DatabaseSource, PruningMode};
 pub use sc_executor::{WasmExecutionMethod, WasmtimeInstantiationStrategy};
+pub use sc_informant::OutputFormat;
 pub use sc_network::{
 	config::{
 		MultiaddrWithPeerId, NetworkConfiguration, NodeKeyConfig, NonDefaultSetConfig, ProtocolId,
@@ -30,9 +33,6 @@ pub use sc_network::{
 	},
 	Multiaddr,
 };
-
-use prometheus_endpoint::Registry;
-use sc_chain_spec::ChainSpec;
 pub use sc_telemetry::TelemetryEndpoints;
 pub use sc_transaction_pool::Options as TransactionPoolOptions;
 use sp_core::crypto::SecretString;
@@ -134,7 +134,7 @@ pub struct Configuration {
 	/// Base path of the configuration. This is shared between chains.
 	pub base_path: BasePath,
 	/// Configuration of the output format that the informant uses.
-	pub informant_output_format: sc_informant::OutputFormat,
+	pub informant_output_format: OutputFormat,
 	/// Maximum number of different runtime versions that can be cached.
 	pub runtime_cache_size: u8,
 }