[ovs-dev,v2,0/3] checkpatch: the comment years!

This series tries to get checkpatch a little better at skipping
stylistic things within comments, as well as growing a new feature
to actually spell check words in the comments.

Patch 1 just cleans up the patch line-type state machine a little.

Patch 2 introduces a c/c++ comment detection state machine filter,
and plugs the infix operator whitespace detector to it.  Turns out it's
the best way of detecting infix operators in comments.

I didn't *heavily* stress test this machine, but I did use checkpatch -2000
to get some samples as well as building up some test strings.  BONUS TODO:
At some point, it would be cool to have a suite of test patches to act as
unit tests...  or even just unit tests...

Patch 3 introduces an opt-in spell checker.  This can aid in reducing any
mispellings.  I sent something like this before, but it wasn't opt-in
at the time and it was probably a bit harder to understand then.

v1->v2:
  * Fixed flake8 errors.
  * Fixed the comment state machine (since it missed a number of edge cases).
  * Although it's submitted on April 1, it's not an april fools joke..

Aaron Conole (3):
  checkpatch: introduce constants for the parse states
  checkpatch: filter comment contents
  checkpatch: add a comment spell-checker

 utilities/checkpatch.py | 182 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 172 insertions(+), 10 deletions(-)

On Sun, Apr 01, 2018 at 11:06:51AM -0400, Aaron Conole wrote:
> This series tries to get checkpatch a little better at skipping
> stylistic things within comments, as well as growing a new feature
> to actually spell check words in the comments.
> 
> Patch 1 just cleans up the patch line-type state machine a little.
> 
> Patch 2 introduces a c/c++ comment detection state machine filter,
> and plugs the infix operator whitespace detector to it.  Turns out it's
> the best way of detecting infix operators in comments.
> 
> I didn't *heavily* stress test this machine, but I did use checkpatch -2000
> to get some samples as well as building up some test strings.  BONUS TODO:
> At some point, it would be cool to have a suite of test patches to act as
> unit tests...  or even just unit tests...
> 
> Patch 3 introduces an opt-in spell checker.  This can aid in reducing any
> mispellings.  I sent something like this before, but it wasn't opt-in
> at the time and it was probably a bit harder to understand then.
> 
> v1->v2:
>   * Fixed flake8 errors.
>   * Fixed the comment state machine (since it missed a number of edge cases).
>   * Although it's submitted on April 1, it's not an april fools joke..

Thanks Aaron!  I applied this series to master.  I also edited my
pre-applypatch hook to add the -S option.  (I wonder whether it would be
worthwhile to enable -S by default if enchant is installed.)

Ben Pfaff <blp@ovn.org> writes:

> On Sun, Apr 01, 2018 at 11:06:51AM -0400, Aaron Conole wrote:
>> This series tries to get checkpatch a little better at skipping
>> stylistic things within comments, as well as growing a new feature
>> to actually spell check words in the comments.
>> 
>> Patch 1 just cleans up the patch line-type state machine a little.
>> 
>> Patch 2 introduces a c/c++ comment detection state machine filter,
>> and plugs the infix operator whitespace detector to it.  Turns out it's
>> the best way of detecting infix operators in comments.
>> 
>> I didn't *heavily* stress test this machine, but I did use checkpatch -2000
>> to get some samples as well as building up some test strings.  BONUS TODO:
>> At some point, it would be cool to have a suite of test patches to act as
>> unit tests...  or even just unit tests...
>> 
>> Patch 3 introduces an opt-in spell checker.  This can aid in reducing any
>> mispellings.  I sent something like this before, but it wasn't opt-in
>> at the time and it was probably a bit harder to understand then.
>> 
>> v1->v2:
>>   * Fixed flake8 errors.
>>   * Fixed the comment state machine (since it missed a number of edge cases).
>>   * Although it's submitted on April 1, it's not an april fools joke..
>
> Thanks Aaron!  I applied this series to master.  I also edited my
> pre-applypatch hook to add the -S option.  (I wonder whether it would be
> worthwhile to enable -S by default if enchant is installed.)

Thanks Ben.

I considered making it 'auto-on', but given it may generate false
positives, I chose to make it 'opt-in' for now.  There's no reason that
can't be revisited after some soak time with others (including myself).

I would probably also enable it for the log message once enough
confidence is built up in the checker (if that makes sense).