Home Original page

Update `Formatter\show_table` to use `Runner->in_color` rather than `shouldColorize` by Dan-Q · Pull Request #5804 · wp-cli/wp-cli

I don't love adding a dependency on WP_CLI::get_runner() inside of this Formatter class.

My first thought was to add a new class-level $in_color variable that can be passed when instantiated:

diff --git a/php/WP_CLI/Formatter.php b/php/WP_CLI/Formatter.php
index 8dd354f5..74fd714c 100644
--- a/php/WP_CLI/Formatter.php
+++ b/php/WP_CLI/Formatter.php
@@ -33,7 +33,7 @@ class Formatter {
 	 * @param string|bool $prefix Check if fields have a standard prefix.
 	 * False indicates empty prefix.
 	 */
-	public function __construct( &$assoc_args, $fields = null, $prefix = false ) {
+	public function __construct( &$assoc_args, $fields = null, $prefix = false, $in_color = true ) {
 		$format_args = [
 			'format' => 'table',
 			'fields' => $fields,
@@ -53,8 +53,9 @@ class Formatter {
 
 		$format_args['fields'] = array_map( 'trim', $format_args['fields'] );
 
-		$this->args   = $format_args;
-		$this->prefix = $prefix;
+		$this->args     = $format_args;
+		$this->prefix   = $prefix;
+		$this->in_color = $in_color
 	}
 
 	/**

However, this would just put the odd dependency elsewhere:

function format_items( $format, $items, $fields ) {
$assoc_args = [
'format' => $format,
'fields' => $fields,
];
$formatter = new Formatter( $assoc_args );
$formatter->display_items( $items );
}

Because Formatter already calls WP_CLI::print_value(), Utils\write_csv(), etc., I think it's fine to call out to WP_CLI:: or a utility function. We could add an in_color() method like this:

diff --git a/php/class-wp-cli.php b/php/class-wp-cli.php
index c775bbc3..67d29c7a 100644
--- a/php/class-wp-cli.php
+++ b/php/class-wp-cli.php
@@ -728,6 +728,20 @@ class WP_CLI {
 		unset( self::$deferred_additions[ $name ] );
 	}
 
+	/**
+	 * Whether or not WP-CLI is currently in color.
+	 *
+	 * @param bool|null $set Whether or not to set the status.
+	 * @return bool
+	 */
+	public static function in_color( $set = null ) {
+		static $in_color;
+		if ( ! is_null( $set ) ) {
+			$in_color = (bool) $set;
+		}
+		return $in_color;
+	}
+
 	/**
 	 * Display informational message without prefix, and ignore `--quiet`.
 	 *

We'd then use it like this:

diff --git a/php/WP_CLI/Formatter.php b/php/WP_CLI/Formatter.php
index 8dd354f5..7f2500f5 100644
--- a/php/WP_CLI/Formatter.php
+++ b/php/WP_CLI/Formatter.php
@@ -301,7 +301,7 @@ class Formatter {
 	private static function show_table( $items, $fields, $ascii_pre_colorized = false ) {
 		$table = new Table();
 
-		$enabled = Colors::shouldColorize();
+		$enabled = WP_CLI::in_color();
 		if ( $enabled ) {
 			Colors::disable( true );
 		}
diff --git a/php/WP_CLI/Runner.php b/php/WP_CLI/Runner.php
index 9e7ceb27..f4d7948c 100644
--- a/php/WP_CLI/Runner.php
+++ b/php/WP_CLI/Runner.php
@@ -938,6 +938,7 @@ class Runner {
 		} else {
 			$this->colorize = $this->config['color'];
 		}
+		WP_CLI::in_color( $this->colorize );
 	}
 
 	public function init_logger() {

This is all somewhat spaghetti logic, but I think what I propose is marginally better than calling WP_CLI::get_runner() directly.

Thoughts?