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 <[email protected]>
cheers, Joe
