Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1658203 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH] checkpatch: Change format of --color argument to --color[=WHEN] John Brooks <john@fastquake.com> - 2017-06-06 01:00 +0200
    Re: [PATCH] checkpatch: Change format of --color argument to  --color[=WHEN] Joe Perches <joe@perches.com> - 2017-06-06 01:20 +0200
      Re: [PATCH] checkpatch: Change format of --color argument to  --color[=WHEN] John Brooks <john@fastquake.com> - 2017-06-06 01:30 +0200
      Re: [PATCH] checkpatch: Change format of --color argument to  --color[=WHEN] Adam Borowski <kilobyte@angband.pl> - 2017-06-06 07:50 +0200
      [PATCH v2] checkpatch: Change format of --color argument to --color[=WHEN] John Brooks <john@fastquake.com> - 2017-06-06 19:10 +0200
        Re: [PATCH v2] checkpatch: Change format of --color argument to  --color[=WHEN] Joe Perches <joe@perches.com> - 2017-06-06 21:40 +0200
          Re: [PATCH v2] checkpatch: Change format of --color argument to  --color[=WHEN] John Brooks <john@fastquake.com> - 2017-06-06 22:00 +0200
            Re: [PATCH v2] checkpatch: Change format of --color argument to  --color[=WHEN] Joe Perches <joe@perches.com> - 2017-06-06 22:10 +0200
              [PATCH V3] checkpatch: Change format of --color argument to --color[=WHEN] Joe Perches <joe@perches.com> - 2017-06-07 04:00 +0200
                Re: [PATCH V3] checkpatch: Change format of --color argument to  --color[=WHEN] John Brooks <john@fastquake.com> - 2017-06-07 15:50 +0200

#1658203 — [PATCH] 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 <john@fastquake.com>
---
 scripts/checkpatch.pl | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4b9569f..8099609 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.
@@ -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) {
-- 
2.7.4

[toc] | [next] | [standalone]

#1658213 — Re: [PATCH] checkpatch: Change format of --color argument to --color[=WHEN]

On Mon, 2017-06-05 at 18:27 -0400, John Brooks wrote:
> 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?

> 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

[toc] | [prev] | [next] | [standalone]

#1658218 — Re: [PATCH] checkpatch: Change format of --color argument to --color[=WHEN]

On Mon, Jun 05, 2017 at 04:10:30PM -0700, Joe Perches wrote:
> On Mon, 2017-06-05 at 18:27 -0400, John Brooks wrote:
> > 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.

> 
> > 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
> 

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

[toc] | [prev] | [next] | [standalone]

#1658359 — Re: [PATCH] checkpatch: Change format of --color argument to --color[=WHEN]

On Mon, Jun 05, 2017 at 04:10:30PM -0700, Joe Perches wrote:
> On Mon, 2017-06-05 at 18:27 -0400, John Brooks wrote:
> > 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!
-- 
⢀⣴⠾⠻⢶⣦⠀ A tit a day keeps the vet away.
⣾⠁⢰⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ (Rejoice as my small-animal-murder-machine got unbroken after
⠈⠳⣄⠀⠀⠀⠀ nearly two years of no catch!)

[toc] | [prev] | [next] | [standalone]

#1658952 — [PATCH v2] 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.

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

Signed-off-by: John Brooks <john@fastquake.com>
---
 scripts/checkpatch.pl | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4b9569f..9d03ae0 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") {
+		$_ = "--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) {
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1659150 — Re: [PATCH v2] checkpatch: Change format of --color argument to --color[=WHEN]

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) {

[toc] | [prev] | [next] | [standalone]

#1659162 — Re: [PATCH v2] checkpatch: Change format of --color argument to --color[=WHEN]

On Tue, Jun 06, 2017 at 12:21:22PM -0700, Joe Perches wrote:
> 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, '-');
>  }

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.

>  
> +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) {

Everything else looks good to me.

Thanks
John

[toc] | [prev] | [next] | [standalone]

#1659166 — Re: [PATCH v2] checkpatch: Change format of --color argument to --color[=WHEN]

On Tue, 2017-06-06 at 19:56 +0000, John Brooks wrote:
> On Tue, Jun 06, 2017 at 12:21:22PM -0700, Joe Perches wrote:
> > 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

[]

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

You're probably right about the trailing bang.

>  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.

> 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 <joe@perches.com>

cheers, Joe

[toc] | [prev] | [next] | [standalone]

#1659338 — [PATCH V3] checkpatch: Change format of --color argument to --color[=WHEN]

From: John Brooks <john@fastquake.com>

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 <john@fastquake.com>
Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7fcaf5ca997b..619851cfd872 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -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) {
-- 
2.10.0.rc2.1.g053435c

[toc] | [prev] | [next] | [standalone]

#1659815 — Re: [PATCH V3] checkpatch: Change format of --color argument to --color[=WHEN]

On Tue, Jun 06, 2017 at 06:50:06PM -0700, Joe Perches wrote:
> From: John Brooks <john@fastquake.com>
> 
> 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 <john@fastquake.com>
> Signed-off-by: Joe Perches <joe@perches.com>

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

John

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web