public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: John Brooks <john@fastquake.com>
Cc: Andy Whitcroft <apw@canonical.com>,
	linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v2] checkpatch: Change format of --color argument to --color[=WHEN]
Date: Tue, 06 Jun 2017 12:21:22 -0700	[thread overview]
Message-ID: <1496776882.1968.27.camel@perches.com> (raw)
In-Reply-To: <1496768832-31004-1-git-send-email-john@fastquake.com>

On Tue, 2017-06-06 at 13:07 -0400, John Brooks wrote:
> 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(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4b9569fa931b..372d541c2c46 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -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) {

  reply	other threads:[~2017-06-06 19:24 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-05 22:27 [PATCH] checkpatch: Change format of --color argument to --color[=WHEN] John Brooks
2017-06-05 23:10 ` Joe Perches
2017-06-05 23:28   ` John Brooks
2017-06-06  5:48   ` Adam Borowski
2017-06-06 17:07   ` [PATCH v2] " John Brooks
2017-06-06 19:21     ` Joe Perches [this message]
2017-06-06 19:56       ` John Brooks
2017-06-06 20:03         ` Joe Perches
2017-06-07  1:50           ` [PATCH V3] " Joe Perches
2017-06-07 13:41             ` John Brooks

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1496776882.1968.27.camel@perches.com \
    --to=joe@perches.com \
    --cc=akpm@linux-foundation.org \
    --cc=apw@canonical.com \
    --cc=john@fastquake.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox