linux-kernel - Re: [PATCH v2] checkpatch: fix TYPO_SPELLING check for words with apostrophe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f5c7211fc8521248fcb7409f95afadda932a7fe7.camel@perches.com>
Date:   Tue, 01 Dec 2020 10:29:31 -0800
From:   Joe Perches <joe@...ches.com>
To:     Dwaipayan Ray <dwaipayanray1@...il.com>
Cc:     linux-kernel-mentees@...ts.linuxfoundation.org,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>,
        Peilin Ye <yepeilin.cs@...il.com>
Subject: Re: [PATCH v2] checkpatch: fix TYPO_SPELLING check for words with
 apostrophe

On Tue, 2020-12-01 at 23:39 +0530, Dwaipayan Ray wrote:
> On Tue, Dec 1, 2020 at 11:32 PM Joe Perches <joe@...ches.com> wrote:
> > On Tue, 2020-12-01 at 12:53 +0530, Dwaipayan Ray wrote:
> > > checkpatch reports a false TYPO_SPELLING warning for some words
> > > containing an apostrophe when run with --codespell option.
> > > 
> > > A false positive is "doesn't". Occurrence of the word causes
> > > checkpatch to emit the following warning:
> > > 
> > > "WARNING: 'doesn'' may be misspelled - perhaps 'doesn't'?"
> > > 
> > > Modify the regex pattern to be more in line with the codespell
> > > default word matching regex. This fixes the word capture and
> > > avoids the false warning.
> > > 
> > > Suggested-by: Joe Perches <joe@...ches.com>
> > > Reported-by: Peilin Ye <yepeilin.cs@...il.com>
> > > Signed-off-by: Dwaipayan Ray <dwaipayanray1@...il.com>
> > > ---
> > > Changes in v2:
> > > - Use the default codespell word regex.
> > > - Modify commit message to specify --codespell usage
> > > 
> > >  scripts/checkpatch.pl | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > []
> > > @@ -3106,7 +3106,7 @@ sub process {
> > >  # Check for various typo / spelling mistakes
> > >               if (defined($misspellings) &&
> > >                   ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) {
> > > -                     while ($rawline =~ /(?:^|[^a-z@])($misspellings)(?:\b|$|[^a-z@])/gi) {
> > > +                     while ($rawline =~ /(?:^|[^\w\-'`])($misspellings)(?:[^\w\-'`]|$)/gi) {
> > 
> > This regex seems to work well, thanks.
> > 
> > >                               my $typo = $1;
> > 
> > A trivial improvement might be to highlight the location of the
> > misspelled word with a caret using
> > 
> >                                 my $blank = copy_spacing($rawline);
> >                                 my $ptr = substr($blank, 0, $-[0] + 1) . "^";
> >                                 my $hereptr = "$hereline$ptr\n";
> > 
> > >                               my $typo_fix = $spelling_fix{lc($typo)};
> > >                               $typo_fix = ucfirst($typo_fix) if ($typo =~ /^[A-Z]/);
> > 
> > and using $hereptr and not $hereline when emitting the message.
> > 
> Sure I will do that.
> Is the color highlighting also supported in most systems?

I believe so, yes.

Color is only used when the output is to stdout and not redirected
and not otherwise specified on the checkpatch command line.

> Maybe a red colored word would be nice to notice as well.

Maybe, but that might make the code around the message output of
$hereptr a bit obscure.  Presumably the code would not use $hereline
but something else new and specific for this message.

The output is done via the report() function so adding optional color
for only a portion of the new $hereptr string might be a lot of code.

If you try it, maybe make the misspelled word color match the message
level and not just be red.  Also the caret line wouldn't have much 
value with that added color highlighting.

Try it and see what you prefer.