checkpatch: Change format of --color argument to --color[=WHEN]

PATCH Superseded John Brooks 3 participants 10 messages Jun 5, 2017 – Jun 7, 2017 Checkpatch

Share & tools

John Brooks

author

2017-06-05 22:53 UTC #0 lore raw

PATCH 1/1 checkpatch: Change format of --color argument to --color[=WHEN]

The boolean --color argument did not offer the ability to force colourized output even if stdout is not a terminal. Change the format of the argument to the familiar --color[=WHEN] construct as seen in common Linux utilities such as ls and dmesg, which allows the user to specify whether to colourize output always, never, or only when the output is a terminal ("auto").

Because the option is no longer boolean, --nocolor (or --no-color) is no longer available. Users of the old negative option should use --color=never instead.

Signed-off-by John Brooks <[email protected]>

---
 scripts/checkpatch.pl | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

scripts/checkpatch.pl +17 -6

index 4b9569f..8099609 100755
@@ -56,7 +56,7 @@ my $codespell = 0;
 my $codespellfile = "/usr/share/codespell/dictionary.txt";
 my $conststructsfile = "$D/const_structs.checkpatch";
 my $typedefsfile = "";
-my $color = 1;
+my $color = "auto";
 my $allow_c99_comments = 1;
 
 sub help {

@@ -115,7 +115,8 @@ Options:
                              (default:/usr/share/codespell/dictionary.txt)
   --codespellfile            Use this codespell dictionary
   --typedefsfile             Read additional types from this file
-  --color                    Use colors when output is STDOUT (default: on)
+  --color[=WHEN]             Use colors 'always', 'never', or only when output
+                             is a terminal ('auto'). Default is 'auto'.
   -h, --help, --version      display this help and exit
 
 When FILE is - read standard input.

@@ -211,7 +212,7 @@ GetOptions(
 	'codespell!'	=> \$codespell,
 	'codespellfile=s'	=> \$codespellfile,
 	'typedefsfile=s'	=> \$typedefsfile,
-	'color!'	=> \$color,
+	'color:s'	=> \$color,
 	'h|help'	=> \$help,
 	'version'	=> \$help
 ) or help(1);

@@ -237,6 +238,16 @@ if ($#ARGV < 0) {
 	push(@ARGV, '-');
 }
 
+if ($color eq "always") {
+	$color = 1;
+} elsif ($color eq "never") {
+	$color = 0;
+} elsif ($color eq "auto") {
+	$color = (-t STDOUT);
+} else {
+	die "Invalid color mode: $color\n";
+}
+
 sub hash_save_array_words {
 	my ($hashRef, $arrayRef) = @_;

@@ -1881,7 +1892,7 @@ sub report {
 		return 0;
 	}
 	my $output = '';
-	if (-t STDOUT && $color) {
+	if ($color) {
 		if ($level eq 'ERROR') {
 			$output .= RED;
 		} elsif ($level eq 'WARNING') {

@@ -1892,10 +1903,10 @@ sub report {
 	}
 	$output .= $prefix . $level . ':';
 	if ($show_types) {
-		$output .= BLUE if (-t STDOUT && $color);
+		$output .= BLUE if ($color);
 		$output .= "$type:";
 	}
-	$output .= RESET if (-t STDOUT && $color);
+	$output .= RESET if ($color);
 	$output .= ' ' . $msg . "\n";
 
 	if ($showfile) {

Signature · 1 line

2.7.4

Joe Perches

maintainer

2017-06-05 23:10 UTC #1 lore raw

Quoted 2 lines · John Brooks

The boolean --color argument did not offer the ability to force colourized output even if stdout is not a terminal.

OK, but why is colorizing output not to terminals desired?

Quoted 8 lines

Change the format of the argument
to the familiar --color[=WHEN] construct as seen in common Linux utilities such as ls and dmesg, which allows the user to specify whether to colourize output always, never, or only when the output is a terminal ("auto").
Because the option is no longer boolean, --nocolor (or --no-color) is no longer available. Users of the old negative option should use --color=never instead.

In general, I don't mind, but perhaps this option name could/should change.

As is, this also causes a previous command line that worked with --color to fail

$ ./scripts/checkpatch.pl --color foo.patch
Invalid color mode: foo.patch

John Brooks

author

2017-06-05 23:28 UTC #2 lore raw

Quoted 5 lines · Joe Perches

Quoted 2 lines · John Brooks
The boolean --color argument did not offer the ability to force colourized output even if stdout is not a terminal.
OK, but why is colorizing output not to terminals desired?

For example, to retain coloured output when using a pager (such as less -R). Which is convenient for viewing/searching lengthy output from larger patch sets, or when one is using something that interferes with the ability to scroll such as screen, tmux, or mosh.

Quoted 10 lines

Quoted 9 lines
Change the format of the argument
to the familiar --color[=WHEN] construct as seen in common Linux utilities such as ls and dmesg, which allows the user to specify whether to colourize output always, never, or only when the output is a terminal ("auto").
Because the option is no longer boolean, --nocolor (or --no-color) is no longer available. Users of the old negative option should use --color=never instead.

Quoted 9 lines

In general, I don't mind, but perhaps this option name could/should change.
As is, this also causes a previous command line that worked with --color to fail
$ ./scripts/checkpatch.pl --color foo.patch
Invalid color mode: foo.patch

Oh, that's pretty bad. I should have thought of that, sorry. I'll see what I can do to stop it from eating other arguments.

--
John Brooks

Adam Borowski

2017-06-06 05:48 UTC #3 lore raw

In reply to Joe Perches

Quoted 5 lines · Joe Perches

Quoted 2 lines · John Brooks
The boolean --color argument did not offer the ability to force colourized output even if stdout is not a terminal.
OK, but why is colorizing output not to terminals desired?

* You may post-process the output somehow. grep, sed, some highlighter...
* The output may go to less -R, ansi2html, etc.

I've made a tool that does what you want, "pipetty" (Debian package colorized-logs, in stretch and jessie-backports), but that's a dirty hack. Lying about isatty() works for programs that check STDOUT but it's notorious to instead look at STDIN, which can't be fooled in a reliable way. Thus, it's better to standardize on --color={always,auto,never}.

Meow!

Signature · 4 lines

⢀⣴⠾⠻⢶⣦⠀ A tit a day keeps the vet away.
⣾⠁⢰⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ (Rejoice as my small-animal-murder-machine got unbroken after
⠈⠳⣄⠀⠀⠀⠀ nearly two years of no catch!)

John Brooks

author

2017-06-06 17:07 UTC #4 lore raw

PATCH v2 1/1 checkpatch: Change format of --color argument to --color[=WHEN]

In reply to Joe Perches

Because the option is no longer boolean, --nocolor (or --no-color) is no longer available. Users of the old negative option should use --color=never instead.

v2 (Joe Perches):
- Prevent --color from consuming other args in e.g. ./scripts/checkpatch.pl --color foo.patch

Signed-off-by John Brooks <[email protected]>

---
 scripts/checkpatch.pl | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

scripts/checkpatch.pl +25 -6

index 4b9569f..9d03ae0 100755
@@ -56,7 +56,7 @@ my $codespell = 0;
 my $codespellfile = "/usr/share/codespell/dictionary.txt";
 my $conststructsfile = "$D/const_structs.checkpatch";
 my $typedefsfile = "";
-my $color = 1;
+my $color = "auto";
 my $allow_c99_comments = 1;
 
 sub help {

@@ -115,7 +115,8 @@ Options:
                              (default:/usr/share/codespell/dictionary.txt)
   --codespellfile            Use this codespell dictionary
   --typedefsfile             Read additional types from this file
-  --color                    Use colors when output is STDOUT (default: on)
+  --color[=WHEN]             Use colors 'always', 'never', or only when output
+                             is a terminal ('auto'). Default is 'auto'.
   -h, --help, --version      display this help and exit
 
 When FILE is - read standard input.

@@ -181,6 +182,14 @@ if (-f $conf) {
 	unshift(@ARGV, @conf_args) if @conf_args;
 }
 
+# Perl's Getopt::Long allows options to take optional arguments after a space.
+# Prevent --color by itself from consuming other arguments
+foreach (@ARGV) {
+	if ($_ eq "--color") {
+		$_ = "--color=$color";
+	}
+}
+
 GetOptions(
 	'q|quiet+'	=> \$quiet,
 	'tree!'		=> \$tree,

@@ -211,7 +220,7 @@ GetOptions(
 	'codespell!'	=> \$codespell,
 	'codespellfile=s'	=> \$codespellfile,
 	'typedefsfile=s'	=> \$typedefsfile,
-	'color!'	=> \$color,
+	'color:s'	=> \$color,
 	'h|help'	=> \$help,
 	'version'	=> \$help
 ) or help(1);

@@ -237,6 +246,16 @@ if ($#ARGV < 0) {
 	push(@ARGV, '-');
 }
 
+if ($color eq "always") {
+	$color = 1;
+} elsif ($color eq "never") {
+	$color = 0;
+} elsif ($color eq "auto") {
+	$color = (-t STDOUT);
+} else {
+	die "Invalid color mode: $color\n";
+}
+
 sub hash_save_array_words {
 	my ($hashRef, $arrayRef) = @_;

@@ -1881,7 +1900,7 @@ sub report {
 		return 0;
 	}
 	my $output = '';
-	if (-t STDOUT && $color) {
+	if ($color) {
 		if ($level eq 'ERROR') {
 			$output .= RED;
 		} elsif ($level eq 'WARNING') {

@@ -1892,10 +1911,10 @@ sub report {
 	}
 	$output .= $prefix . $level . ':';
 	if ($show_types) {
-		$output .= BLUE if (-t STDOUT && $color);
+		$output .= BLUE if ($color);
 		$output .= "$type:";
 	}
-	$output .= RESET if (-t STDOUT && $color);
+	$output .= RESET if ($color);
 	$output .= ' ' . $msg . "\n";
 
 	if ($showfile) {

Signature · 1 line

2.7.4

Joe Perches

maintainer

2017-06-06 19:24 UTC #5 lore raw

Quoted 9 lines · John Brooks

The boolean --color argument did not offer the ability to force colourized output even if stdout is not a terminal. Change the format of the argument to the familiar --color[=WHEN] construct as seen in common Linux utilities such as ls and dmesg, which allows the user to specify whether to colourize output always, never, or only when the output is a terminal ("auto").
Because the option is no longer boolean, --nocolor (or --no-color) is no longer available. Users of the old negative option should use --color=never instead.

It is possible to add --nocolor and --no-color to the arguments for GetOptions to keep the old behavior intact.

I think this works:

---
 scripts/checkpatch.pl | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

scripts/checkpatch.pl +29 -6

index 4b9569fa931b..372d541c2c46 100755
@@ -56,7 +56,7 @@ my $codespell = 0;
 my $codespellfile = "/usr/share/codespell/dictionary.txt";
 my $conststructsfile = "$D/const_structs.checkpatch";
 my $typedefsfile = "";
-my $color = 1;
+my $color = "auto";
 my $allow_c99_comments = 1;
 
 sub help {

@@ -115,7 +115,8 @@ Options:
                              (default:/usr/share/codespell/dictionary.txt)
   --codespellfile            Use this codespell dictionary
   --typedefsfile             Read additional types from this file
-  --color                    Use colors when output is STDOUT (default: on)
+  --color[=WHEN]             Use colors 'always', 'never', or only when output
+                             is a terminal ('auto'). Default is 'auto'.
   -h, --help, --version      display this help and exit
 
 When FILE is - read standard input.

@@ -181,6 +182,14 @@ if (-f $conf) {
 	unshift(@ARGV, @conf_args) if @conf_args;
 }
 
+# Perl's Getopt::Long allows options to take optional arguments after a space.
+# Prevent --color by itself from consuming other arguments
+foreach (@ARGV) {
+	if ($_ eq "--color" || $_ eq "-color") {
+		$_ = "--color=$color";
+	}
+}
+
 GetOptions(
 	'q|quiet+'	=> \$quiet,
 	'tree!'		=> \$tree,

@@ -211,7 +220,9 @@ GetOptions(
 	'codespell!'	=> \$codespell,
 	'codespellfile=s'	=> \$codespellfile,
 	'typedefsfile=s'	=> \$typedefsfile,
-	'color!'	=> \$color,
+	'color=s'	=> \$color,
+	'-no-color!'	=> \$color,	#keep old behaviors of -nocolor
+	'-nocolor!'	=> \$color,	#keep old behaviors of -nocolor
 	'h|help'	=> \$help,
 	'version'	=> \$help
 ) or help(1);

@@ -237,6 +248,18 @@ if ($#ARGV < 0) {
 	push(@ARGV, '-');
 }
 
+if ($color =~ /^[01]$/) {
+	$color = !$color;
+} elsif ($color =~ /^always$/i) {
+	$color = 1;
+} elsif ($color =~ /^never$/i) {
+	$color = 0;
+} elsif ($color =~ /^auto$/i) {
+	$color = (-t STDOUT);
+} else {
+	die "Invalid color mode: $color\n";
+}
+
 sub hash_save_array_words {
 	my ($hashRef, $arrayRef) = @_;

@@ -1881,7 +1904,7 @@ sub report {
 		return 0;
 	}
 	my $output = '';
-	if (-t STDOUT && $color) {
+	if ($color) {
 		if ($level eq 'ERROR') {
 			$output .= RED;
 		} elsif ($level eq 'WARNING') {

@@ -1892,10 +1915,10 @@ sub report {
 	}
 	$output .= $prefix . $level . ':';
 	if ($show_types) {
-		$output .= BLUE if (-t STDOUT && $color);
+		$output .= BLUE if ($color);
 		$output .= "$type:";
 	}
-	$output .= RESET if (-t STDOUT && $color);
+	$output .= RESET if ($color);
 	$output .= ' ' . $msg . "\n";
 
 	if ($showfile) {

John Brooks

author

2017-06-06 19:56 UTC #6 lore raw

Quoted 69 lines · Joe Perches

Quoted 9 lines · John Brooks

The boolean --color argument did not offer the ability to force colourized output even if stdout is not a terminal. Change the format of the argument to the familiar --color[=WHEN] construct as seen in common Linux utilities such as ls and dmesg, which allows the user to specify whether to colourize output always, never, or only when the output is a terminal ("auto").
Because the option is no longer boolean, --nocolor (or --no-color) is no longer available. Users of the old negative option should use --color=never instead.

It is possible to add --nocolor and --no-color to the arguments for GetOptions to keep the old behavior intact.

I think this works:

---
 scripts/checkpatch.pl | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

scripts/checkpatch.pl +14 -3

index 4b9569fa931b..372d541c2c46 100755
@@ -56,7 +56,7 @@ my $codespell = 0;
 my $codespellfile = "/usr/share/codespell/dictionary.txt";
 my $conststructsfile = "$D/const_structs.checkpatch";
 my $typedefsfile = "";
-my $color = 1;
+my $color = "auto";
 my $allow_c99_comments = 1;
 
 sub help {

@@ -115,7 +115,8 @@ Options:
                              (default:/usr/share/codespell/dictionary.txt)
   --codespellfile            Use this codespell dictionary
   --typedefsfile             Read additional types from this file
-  --color                    Use colors when output is STDOUT (default: on)
+  --color[=WHEN]             Use colors 'always', 'never', or only when output
+                             is a terminal ('auto'). Default is 'auto'.
   -h, --help, --version      display this help and exit
 
 When FILE is - read standard input.

@@ -181,6 +182,14 @@ if (-f $conf) {
 	unshift(@ARGV, @conf_args) if @conf_args;
 }
 
+# Perl's Getopt::Long allows options to take optional arguments after a space.
+# Prevent --color by itself from consuming other arguments
+foreach (@ARGV) {
+	if ($_ eq "--color" || $_ eq "-color") {
+		$_ = "--color=$color";
+	}
+}
+
 GetOptions(
 	'q|quiet+'	=> \$quiet,
 	'tree!'		=> \$tree,

@@ -211,7 +220,9 @@ GetOptions(
 	'codespell!'	=> \$codespell,
 	'codespellfile=s'	=> \$codespellfile,
 	'typedefsfile=s'	=> \$typedefsfile,
-	'color!'	=> \$color,
+	'color=s'	=> \$color,
+	'-no-color!'	=> \$color,	#keep old behaviors of -nocolor
+	'-nocolor!'	=> \$color,	#keep old behaviors of -nocolor
 	'h|help'	=> \$help,
 	'version'	=> \$help
 ) or help(1);

scripts/checkpatch.pl if ($#ARGV < 0) { · line 249

Quoted 2 lines

scripts/checkpatch.pl +0 -0

@@ -237,6 +248,18 @@ if ($#ARGV < 0) {
 	push(@ARGV, '-');
 }

Good changes overall. Does one want the leading dash and trailing bang here, however? I don't know what the leading dash does (I would guess it makes it store 0 into the variable? I can't find anything in the perldoc), but the bang makes the option negatable, which would allow you to do --nonocolor/ --nono-color, and that may not make sense here.

Quoted 28 lines

scripts/checkpatch.pl +13 -1

+if ($color =~ /^[01]$/) {
+	$color = !$color;
+} elsif ($color =~ /^always$/i) {
+	$color = 1;
+} elsif ($color =~ /^never$/i) {
+	$color = 0;
+} elsif ($color =~ /^auto$/i) {
+	$color = (-t STDOUT);
+} else {
+	die "Invalid color mode: $color\n";
+}
+
 sub hash_save_array_words {
 	my ($hashRef, $arrayRef) = @_;

@@ -1881,7 +1904,7 @@ sub report {
 		return 0;
 	}
 	my $output = '';
-	if (-t STDOUT && $color) {
+	if ($color) {
 		if ($level eq 'ERROR') {
 			$output .= RED;
 		} elsif ($level eq 'WARNING') {

@@ -1892,10 +1915,10 @@ sub report {
 	}
 	$output .= $prefix . $level . ':';

scripts/checkpatch.pl sub report { · line 1924

Quoted 10 lines

scripts/checkpatch.pl +2 -2

 	if ($show_types) {
-		$output .= BLUE if (-t STDOUT && $color);
+		$output .= BLUE if ($color);
 		$output .= "$type:";
 	}
-	$output .= RESET if (-t STDOUT && $color);
+	$output .= RESET if ($color);
 	$output .= ' ' . $msg . "\n";
 
 	if ($showfile) {

Everything else looks good to me.

Thanks
John

Joe Perches

maintainer Acked-by

2017-06-06 20:03 UTC #7 lore raw

Quoted 58 lines · John Brooks

Quoted 57 lines · Joe Perches
Quoted 9 lines · John Brooks
The boolean --color argument did not offer the ability to force colourized output even if stdout is not a terminal. Change the format of the argument to the familiar --color[=WHEN] construct as seen in common Linux utilities such as ls and dmesg, which allows the user to specify whether to colourize output always, never, or only when the output is a terminal ("auto").
Because the option is no longer boolean, --nocolor (or --no-color) is no longer available. Users of the old negative option should use --color=never instead.
It is possible to add --nocolor and --no-color to the arguments for GetOptions to keep the old behavior intact.
I think this works:
---
 scripts/checkpatch.pl | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)
scripts/checkpatch.pl +11 -2
index 4b9569fa931b..372d541c2c46 100755
@@ -56,7 +56,7 @@ my $codespell = 0;
 my $codespellfile = "/usr/share/codespell/dictionary.txt";
 my $conststructsfile = "$D/const_structs.checkpatch";
 my $typedefsfile = "";
-my $color = 1;
+my $color = "auto";
 my $allow_c99_comments = 1;
 
 sub help {
@@ -115,7 +115,8 @@ Options:
                              (default:/usr/share/codespell/dictionary.txt)
   --codespellfile            Use this codespell dictionary
   --typedefsfile             Read additional types from this file
-  --color                    Use colors when output is STDOUT (default: on)
+  --color[=WHEN]             Use colors 'always', 'never', or only when output
+                             is a terminal ('auto'). Default is 'auto'.
   -h, --help, --version      display this help and exit
 
 When FILE is - read standard input.
@@ -181,6 +182,14 @@ if (-f $conf) {
 	unshift(@ARGV, @conf_args) if @conf_args;
 }
 
+# Perl's Getopt::Long allows options to take optional arguments after a space.
+# Prevent --color by itself from consuming other arguments
+foreach (@ARGV) {
+	if ($_ eq "--color" || $_ eq "-color") {
+		$_ = "--color=$color";
+	}
+}
+
 GetOptions(
 	'q|quiet+'	=> \$quiet,
 	'tree!'		=> \$tree,

Quoted 8 lines

scripts/checkpatch.pl GetOptions( · line 225

Quoted 8 lines

scripts/checkpatch.pl +3 -1

@@ -211,7 +220,9 @@ GetOptions(
 	'codespell!'	=> \$codespell,
 	'codespellfile=s'	=> \$codespellfile,
 	'typedefsfile=s'	=> \$typedefsfile,
-	'color!'	=> \$color,
+	'color=s'	=> \$color,
+	'-no-color!'	=> \$color,	#keep old behaviors of -nocolor
+	'-nocolor!'	=> \$color,	#keep old behaviors of -nocolor

[]

Quoted 2 lines

Good changes overall. Does one want the leading dash and trailing bang here, however?

You're probably right about the trailing bang.

Quoted 3 lines

I don't know what the leading dash does (I would guess it makes it
store 0 into the variable?
I can't find anything in the perldoc)

No idea. Me neither.

Quoted 3 lines

but the bang
makes the option negatable, which would allow you to do --nonocolor/ --nono-color, and that may not make sense here.

Right. The bang should be removed.

When you submit a V3 with the appropriate changes,

Acked-by Joe Perches <[email protected]>

cheers, Joe

Joe Perches

maintainer

2017-06-07 01:50 UTC #8 lore raw

PATCH v3 1/1 checkpatch: Change format of --color argument to --color[=WHEN]

From: John Brooks <[email protected]>

The boolean --color argument did not offer the ability to force colourized output even if stdout is not a terminal. Change the format of the argument to the familiar --color[=WHEN] construct as seen in common Linux utilities such as git, ls and dmesg, which allows the user to specify whether to colourize output "always", "never", or "auto" when the output is a terminal. The default is "auto".

The old command-line uses of --color and --no-color are unchanged.

Signed-off-by John Brooks <[email protected]>

Signed-off-by Joe Perches <[email protected]>

---
 scripts/checkpatch.pl | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

scripts/checkpatch.pl +29 -6

index 7fcaf5ca997b..619851cfd872 100755
@@ -57,7 +57,7 @@ my $codespell = 0;
 my $codespellfile = "/usr/share/codespell/dictionary.txt";
 my $conststructsfile = "$D/const_structs.checkpatch";
 my $typedefsfile = "";
-my $color = 1;
+my $color = "auto";
 my $allow_c99_comments = 1;
 
 sub help {

@@ -116,7 +116,8 @@ Options:
                              (default:/usr/share/codespell/dictionary.txt)
   --codespellfile            Use this codespell dictionary
   --typedefsfile             Read additional types from this file
-  --color                    Use colors when output is STDOUT (default: on)
+  --color[=WHEN]             Use colors 'always', 'never', or only when output
+                             is a terminal ('auto'). Default is 'auto'.
   -h, --help, --version      display this help and exit
 
 When FILE is - read standard input.

@@ -182,6 +183,14 @@ if (-f $conf) {
 	unshift(@ARGV, @conf_args) if @conf_args;
 }
 
+# Perl's Getopt::Long allows options to take optional arguments after a space.
+# Prevent --color by itself from consuming other arguments
+foreach (@ARGV) {
+	if ($_ eq "--color" || $_ eq "-color") {
+		$_ = "--color=$color";
+	}
+}
+
 GetOptions(
 	'q|quiet+'	=> \$quiet,
 	'tree!'		=> \$tree,

@@ -212,7 +221,9 @@ GetOptions(
 	'codespell!'	=> \$codespell,
 	'codespellfile=s'	=> \$codespellfile,
 	'typedefsfile=s'	=> \$typedefsfile,
-	'color!'	=> \$color,
+	'color=s'	=> \$color,
+	'no-color'	=> \$color,	#keep old behaviors of -nocolor
+	'nocolor'	=> \$color,	#keep old behaviors of -nocolor
 	'h|help'	=> \$help,
 	'version'	=> \$help
 ) or help(1);

@@ -238,6 +249,18 @@ if ($#ARGV < 0) {
 	push(@ARGV, '-');
 }
 
+if ($color =~ /^[01]$/) {
+	$color = !$color;
+} elsif ($color =~ /^always$/i) {
+	$color = 1;
+} elsif ($color =~ /^never$/i) {
+	$color = 0;
+} elsif ($color =~ /^auto$/i) {
+	$color = (-t STDOUT);
+} else {
+	die "Invalid color mode: $color\n";
+}
+
 sub hash_save_array_words {
 	my ($hashRef, $arrayRef) = @_;

@@ -1882,7 +1905,7 @@ sub report {
 		return 0;
 	}
 	my $output = '';
-	if (-t STDOUT && $color) {
+	if ($color) {
 		if ($level eq 'ERROR') {
 			$output .= RED;
 		} elsif ($level eq 'WARNING') {

@@ -1893,10 +1916,10 @@ sub report {
 	}
 	$output .= $prefix . $level . ':';
 	if ($show_types) {
-		$output .= BLUE if (-t STDOUT && $color);
+		$output .= BLUE if ($color);
 		$output .= "$type:";
 	}
-	$output .= RESET if (-t STDOUT && $color);
+	$output .= RESET if ($color);
 	$output .= ' ' . $msg . "\n";
 
 	if ($showfile) {

Signature · 1 line

2.10.0.rc2.1.g053435c

John Brooks

author

2017-06-07 13:41 UTC #9 lore raw

Quoted 9 lines · Joe Perches

From: John Brooks <[email protected]>
The boolean --color argument did not offer the ability to force colourized output even if stdout is not a terminal. Change the format of the argument to the familiar --color[=WHEN] construct as seen in common Linux utilities such as git, ls and dmesg, which allows the user to specify whether to colourize output "always", "never", or "auto" when the output is a terminal. The default is "auto".

Quoted 4 lines

The old command-line uses of --color and --no-color are unchanged.
Signed-off-by John Brooks <[email protected]>
Signed-off-by Joe Perches <[email protected]>

Thanks for doing the V3 for me :)
I was going to but had other work to do last night.

John