Page MenuHomePhabricator

[clang-format]: Add option to insert space after locical not operator
ClosedPublic

Authored by reuk on Apr 5 2019, 8:40 AM.

Details

Summary

This patch aims to support the following rule from the Juce coding standards:

The ! operator should always be followed by a space, e.g. if (! foo)

Leaving a space after ! stops it from blending into the rest of the expression, which makes it easier to tell at a glance what the expression is doing.

Patch by Reuben Thomas

Diff Detail

Event Timeline

reuk created this revision.Apr 5 2019, 8:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 5 2019, 8:40 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
reuk updated this revision to Diff 193889.Apr 5 2019, 8:42 AM

just a few nits, apart form that it LG

clang/include/clang/Format/Format.h
1728

nit: end the sentence with a "."

1929

nit: so I recently observed that these are alphabetic, move this up to line 1916

clang/lib/Format/Format.cpp
493

nit: these are alphabetic

clang/unittests/Format/FormatTest.cpp
9730

could you add an example where someone does !! would it appear as ! ! ( or '!!'

I actually also tend to change doc/ClangFormatStyleOptions.rst too

reuk updated this revision to Diff 194055.Apr 7 2019, 5:00 AM

Updated with fixes. Thanks for pointing out that the options are ordered alphabetically, I hadn't noticed that!

reuk added a comment.Apr 7 2019, 5:01 AM

I updated ClangFormatStyleOptions.rst by hand, is there a way to do that automatically instead (for future patches)?

I updated ClangFormatStyleOptions.rst by hand, is there a way to do that automatically instead (for future patches)?

I do it by hand, I know there is a python script out there, go with this for now, perhaps I'll try and find out how to do it automatically and then submit any differences if there are any (I'm sure there are some)

MyDeveloperDay accepted this revision.Apr 7 2019, 5:30 AM

LGTM (minor nit)

clang/lib/Format/TokenAnnotator.cpp
2835–2836

nit: you don't need to {}

This revision is now accepted and ready to land.Apr 7 2019, 5:30 AM
reuk updated this revision to Diff 194057.Apr 7 2019, 5:33 AM

Fixed formatting nit.

reuk closed this revision.Apr 8 2019, 5:57 AM

Closed by https://reviews.llvm.org/rG91f60b44958f, https://reviews.llvm.org/rL357908, https://reviews.llvm.org/rC357908 (sorry, I forgot to update the body of the commit message to close this automatically)