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

Groups > linux.kernel > #1659166

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

Show all headers | View raw

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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[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

csiph-web