flux-overlay: replace status --no-color option with --color=WHEN

Problem: The flux-overlay status command requires manual disabling of
colorized output using a --no-color option, but other Flux commands
use the semi-standard --color=WHEN option.

Replace --no-color with `-L, --color=WHEN`, where WHEN can be one of
'auto', 'never', or 'always'. The default is 'auto' if the option
is not specified, or 'always' if the option is specified without
an argument.

Since --color=auto is now the default, color will not be displayed
if stdout is not connected to a tty. Therefore the use of --no-color
in the testsuite can simply be removed.

Author: grondo

src/cmd/builtin/overlay.c

@@ -11,6 +11,7 @@
 #if HAVE_CONFIG_H
 # include <config.h>
 #endif
+#include <unistd.h>
 #include <jansson.h>
 #include <flux/core.h>
 
@@ -57,8 +58,9 @@ static struct optparse_option status_opts[] = {
       .usage = "Do not fill in presumed state of nodes that are"
                " inaccessible behind offline/lost overlay parents",
     },
-    { .name = "no-color", .has_arg = 0,
-      .usage = "Do not use color to highlight offline/lost nodes",
+    { .name = "color", .key = 'L', .has_arg = 2, .arginfo = "WHEN",
+      .usage = "Colorize output when supported; WHEN can be 'always' "
+               "(default if omitted), 'never', or 'auto' (default)."
     },
     { .name = "wait", .key = 'w', .has_arg = 1, .arginfo = "STATE",
       .usage = "Wait until subtree enters STATE before reporting"
@@ -77,6 +79,7 @@ static struct optparse_option disconnect_opts[] = {
 struct status {
     flux_t *h;
     int verbose;
+    int color;
     double timeout;
     optparse_t *opt;
     struct timespec start;
@@ -173,7 +176,7 @@ static const char *status_colorize (struct status *ctx,
 {
     static char buf[128];
 
-    if (!optparse_hasopt (ctx->opt, "no-color")) {
+    if (ctx->color) {
         if (streq (status, "lost") && !ghost) {
             snprintf (buf, sizeof (buf), "%s%s%s",
                       ansi_red, status, ansi_default);
@@ -555,6 +558,24 @@ static bool validate_wait (const char *wait)
     return true;
 }
 
+static int status_use_color (optparse_t *p)
+{
+    const char *when;
+    int color;
+
+    if (!(when = optparse_get_str (p, "color", "auto")))
+        when = "always";
+    if (streq (when, "always"))
+        color = 1;
+    else if (streq (when, "never"))
+        color = 0;
+    else if (streq (when, "auto"))
+        color = isatty (STDOUT_FILENO) ? 1 : 0;
+    else
+        log_msg_exit ("Invalid argument to --color: '%s'", when);
+    return color;
+}
+
 static int subcmd_status (optparse_t *p, int ac, char *av[])
 {
     int rank = optparse_get_int (p, "rank", 0);
@@ -563,6 +584,7 @@ static int subcmd_status (optparse_t *p, int ac, char *av[])
 
     ctx.h = builtin_get_flux_handle (p);
     ctx.verbose = optparse_get_int (p, "verbose", 0);
+    ctx.color = status_use_color (p);
     ctx.timeout = optparse_get_duration (p, "timeout", default_timeout);
     if (ctx.timeout == 0)
         ctx.timeout = -1.0; // disabled

t/t3303-system-healthcheck.t

@@ -91,7 +91,7 @@ test_expect_success 'flux overlay status -vv works' '
 
 test_expect_success 'flux overlay status shows rank 3 offline' '
 	echo "3 fake3: offline" >health.exp &&
-	flux overlay status --timeout=0 --no-pretty --no-color \
+	flux overlay status --timeout=0 --no-pretty \
 		| grep fake3 >health.out &&
 	test_cmp health.exp health.out
 '
@@ -109,14 +109,14 @@ test_expect_success 'flux overlay status -vv' '
 '
 
 test_expect_success 'flux overlay status: 0,1:partial, 3:offline' '
-	flux overlay status --timeout=0 --no-color --no-pretty  >health2.out &&
+	flux overlay status --timeout=0 --no-pretty  >health2.out &&
 	grep "0 fake0: partial" health2.out &&
 	grep "1 fake1: partial" health2.out &&
 	grep "3 fake3: offline" health2.out
 '
 
 test_expect_success 'flux overlay status: 0-1:partial, 3,7-8:offline' '
-	flux overlay status --timeout=0 --no-color --no-pretty >health3.out &&
+	flux overlay status --timeout=0 --no-pretty >health3.out &&
 	grep "0 fake0: partial" health3.out &&
 	grep "1 fake1: partial" health3.out &&
 	grep "3 fake3: offline" health3.out &&
@@ -125,7 +125,7 @@ test_expect_success 'flux overlay status: 0-1:partial, 3,7-8:offline' '
 '
 
 test_expect_success 'flux overlay status: 0,1:partial, 3,7-8:offline, rest:full' '
-	flux overlay status --timeout=0 --no-color --no-pretty >health4.out &&
+	flux overlay status --timeout=0 --no-pretty >health4.out &&
 	grep "0 fake0: partial" health4.out &&
 	grep "1 fake1: partial" health4.out &&
 	grep "3 fake3: offline" health4.out &&
@@ -234,7 +234,7 @@ test_expect_success 'stop broker 12' '
 '
 
 test_expect_success 'flux overlay status prints connection timed out on 12' '
-	flux overlay status --no-pretty --no-color >status.out &&
+	flux overlay status --no-pretty >status.out &&
 	grep "fake12: Connection timed out" status.out
 '
 

t/t3305-system-rpctrack-up.t

@@ -97,7 +97,7 @@ test_expect_success 'report health status' '
 '
 test_expect_success 'health status for rank 6 is lost' '
 	echo "6 fake6: lost" >status.exp &&
-	flux overlay status --timeout=0 --down --no-color --no-pretty \
+	flux overlay status --timeout=0 --down --no-pretty \
 		| grep fake6 >status.out &&
 	test_cmp status.exp status.out
 '