optimad / bitpit

Open source library for scientific HPC
http://optimad.github.io/bitpit/
GNU Lesser General Public License v3.0
117 stars 34 forks source link

Allow using clang-format for format the code #291

Closed andrea-iob closed 1 year ago

andrea-iob commented 2 years ago

In order to keep the code style consistent among all bitpit modules, I've added to the root of the repository the configuration file needed by clang-format. The presence of this configuration file allows to use clang-format to automatically format the code:

clang-format --style=file -i <FILENAME>

The configuration filed is based on the LLVM coding standards (https://llvm.org/docs/CodingStandards.html). The changes with respect to the base configuration are minor: the width of the indentation and the placement of the braces. Here the difference between the base LLVM configuration:

@@ -29,27 +29,27 @@
 BinPackArguments: true
 BinPackParameters: true
 BraceWrapping:
-  AfterCaseLabel:  false
-  AfterClass:      false
+  AfterCaseLabel:  true
+  AfterClass:      true
   AfterControlStatement: Never
-  AfterEnum:       false
-  AfterFunction:   false
+  AfterEnum:       true
+  AfterFunction:   true
   AfterNamespace:  false
   AfterObjCDeclaration: false
-  AfterStruct:     false
-  AfterUnion:      false
+  AfterStruct:     true
+  AfterUnion:      true
   AfterExternBlock: false
-  BeforeCatch:     false
+  BeforeCatch:     true
   BeforeElse:      false
-  BeforeLambdaBody: false
-  BeforeWhile:     false
+  BeforeLambdaBody: true
+  BeforeWhile:     true
   IndentBraces:    false
   SplitEmptyFunction: true
   SplitEmptyRecord: true
   SplitEmptyNamespace: true
 BreakBeforeBinaryOperators: None
 BreakBeforeConceptDeclarations: true
-BreakBeforeBraces: Attach
+BreakBeforeBraces: Custom
 BreakBeforeInheritanceComma: false
 BreakInheritanceList: BeforeColon
 BreakBeforeTernaryOperators: true
@@ -57,7 +57,7 @@
 BreakConstructorInitializers: BeforeColon
 BreakAfterJavaFieldAnnotations: false
 BreakStringLiterals: true
-ColumnLimit:     80
+ColumnLimit:     0
 CommentPragmas:  '^ IWYU pragma:'
 QualifierAlignment: Leave
 CompactNamespaces: false
@@ -104,12 +104,12 @@
 IndentPPDirectives: None
 IndentExternBlock: AfterExternBlock
 IndentRequires:  false
-IndentWidth:     2
+IndentWidth:     4
 IndentWrappedFunctionNames: false
 InsertTrailingCommas: None
 JavaScriptQuotes: Leave
 JavaScriptWrapImports: true
-KeepEmptyLinesAtTheStartOfBlocks: true
+KeepEmptyLinesAtTheStartOfBlocks: false
 LambdaBodyIndentation: Signature
 MacroBlockBegin: ''
 MacroBlockEnd:   ''
@@ -179,7 +179,7 @@
 StatementMacros:
   - Q_UNUSED
   - QT_REQUIRE_VERSION
-TabWidth:        8
+TabWidth:        4
 UseCRLF:         false
 UseTab:          Never
 WhitespaceSensitiveMacros:

If needed, the configuration can be changed (with the current one I get a code style very similar to the one we are currently using in some modules).

I've also added some scripts that allow to setup a pre-commit git hook to automatically format the code. The hook will inspect the code to be commited and, if it doesn't respect the code style, it will allow to automatically fix the coding style. This script can also be called manually to format individual files or staged changes. More information about the script here. The pre-commit hook can only be installed locally (see previous link), github doesn't support hooks. To enforce coding style on github, we can setup a github action that performs the check and allow merging a pull request only if the check is satisfied.

If needed, I can run clang-format on the whole code base or we can choose to apply the enforce the coding style only to the newly added code.

andrea-iob commented 2 years ago

If you want to test different clang-format configurations, you can try this configurator.

andrea-iob commented 2 years ago

There is a plugin for Eclipse that integrates the clang-format tool as an alternative C/C++ code formatter.

marcocisternino commented 2 years ago

Several VSCode extension to clang-format are available (xaver's one is the most downloaded)

marcocisternino commented 2 years ago

We have a code style guide here. @roccoarpa , I'm not sure but I think we tried to respect it in immerflow. @andrea-iob , correct me if I'm wrong. Do we want to change this style?

roccoarpa commented 2 years ago

We have a code style guide here. @roccoarpa , I'm not sure but I think we tried to respect it in immerflow. @andrea-iob , correct me if I'm wrong. Do we want to change this style?

Sorry, my bad, i totally misunderstood. Please forget my previous comment (i'm going to delete it). Please proceed how you feel more appropriate.

edoardolombardi commented 2 years ago

Cool. Thanks @andrea-iob, I think this tool should be useful for all our developments.

Sorry, my bad, i totally misunderstood. Please forget my previous comment (i'm going to delete it). Please proceed how you feel more appropriate.

Maybe I'm wrong, but I think that your deleted comment @roccoarpa was not off topic.

In particular referring to

We have a code style guide here. @roccoarpa , I'm not sure but I think we tried to respect it in immerflow. @andrea-iob , correct me if I'm wrong. Do we want to change this style?

a couple of questions:

If not, I agree with @roccoarpa, maybe an internal meeting can be useful to inform all the developers, to discuss and to agree on a common coding style and to adopt it in all our Optimad codes.

PS: I noticed that @andrea-iob already started a chat with some of us in this direction.

andrea-iob commented 2 years ago

I've enabled alignment of consecutive macros, assignments, and bitfields (AlignConsecutiveMacros, AlignConsecutiveAssignments, AlignConsecutiveBitFields).

roccoarpa commented 2 years ago

Hello there: we run some test with the new formatting together with @kgkara, and the current format layout (with = alignment active) is fine for us. Anyway, I want to point out the question of the version of clang-format. I don't remember with which version this .clang-format yaml file is compliant. I used the version 12 (on Ubuntu newer version are not available with apt for now) and this file triggers error of unrecognized keys. I transferred the modifications on a clang12 compliant yaml (reported below) and this works.

Let us know which version of clang-format will be officially supported in the end.


Language:        Cpp
# BasedOnStyle:  LLVM
AccessModifierOffset: -2
AlignAfterOpenBracket: Align
AlignConsecutiveMacros: Consecutive
AlignConsecutiveAssignments: Consecutive
AlignConsecutiveBitFields: Consecutive
AlignConsecutiveDeclarations: None
AlignEscapedNewlines: Right
AlignOperands:   Align
AlignTrailingComments: true
AllowAllArgumentsOnNextLine: true
AllowAllConstructorInitializersOnNextLine: true
AllowAllParametersOfDeclarationOnNextLine: true
AllowShortEnumsOnASingleLine: true
AllowShortBlocksOnASingleLine: Never
AllowShortCaseLabelsOnASingleLine: false
AllowShortFunctionsOnASingleLine: All
AllowShortLambdasOnASingleLine: All
AllowShortIfStatementsOnASingleLine: Never
AllowShortLoopsOnASingleLine: false
AlwaysBreakAfterDefinitionReturnType: None
AlwaysBreakAfterReturnType: None
AlwaysBreakBeforeMultilineStrings: false
AlwaysBreakTemplateDeclarations: MultiLine
AttributeMacros:
  - __capability
BinPackArguments: true
BinPackParameters: true
BraceWrapping:
  AfterCaseLabel:  true
  AfterClass:      true
  AfterControlStatement: Never
  AfterEnum:       true
  AfterFunction:   true
  AfterNamespace:  false
  AfterObjCDeclaration: false
  AfterStruct:     true
  AfterUnion:      true
  AfterExternBlock: false
  BeforeCatch:     false
  BeforeElse:      false
  BeforeLambdaBody: true
  BeforeWhile:     false
  IndentBraces:    false
  SplitEmptyFunction: true
  SplitEmptyRecord: true
  SplitEmptyNamespace: true
BreakBeforeBinaryOperators: None
BreakBeforeConceptDeclarations: true
BreakBeforeBraces: Custom
BreakBeforeInheritanceComma: false
BreakInheritanceList: BeforeColon
BreakBeforeTernaryOperators: true
BreakConstructorInitializersBeforeComma: false
BreakConstructorInitializers: BeforeColon
BreakAfterJavaFieldAnnotations: false
BreakStringLiterals: true
ColumnLimit:     0
CommentPragmas:  '^ IWYU pragma:'
CompactNamespaces: false
ConstructorInitializerAllOnOneLineOrOnePerLine: false
ConstructorInitializerIndentWidth: 4
ContinuationIndentWidth: 4
Cpp11BracedListStyle: true
DeriveLineEnding: true
DerivePointerAlignment: false
DisableFormat:   false
EmptyLineBeforeAccessModifier: LogicalBlock
ExperimentalAutoDetectBinPacking: false
FixNamespaceComments: true
ForEachMacros:
  - foreach
  - Q_FOREACH
  - BOOST_FOREACH
StatementAttributeLikeMacros:
  - Q_EMIT
IncludeBlocks:   Preserve
IncludeCategories:
  - Regex:           '^"(llvm|llvm-c|clang|clang-c)/'
    Priority:        2
    SortPriority:    0
    CaseSensitive:   false
  - Regex:           '^(<|"(gtest|gmock|isl|json)/)'
    Priority:        3
    SortPriority:    0
    CaseSensitive:   false
  - Regex:           '.*'
    Priority:        1
    SortPriority:    0
    CaseSensitive:   false
IncludeIsMainRegex: '(Test)?$'
IncludeIsMainSourceRegex: ''
IndentCaseLabels: false
IndentCaseBlocks: false
IndentGotoLabels: true
IndentPPDirectives: None
IndentExternBlock: AfterExternBlock
IndentRequires:  false
IndentWidth:     4
IndentWrappedFunctionNames: false
InsertTrailingCommas: None
JavaScriptQuotes: Leave
JavaScriptWrapImports: true
KeepEmptyLinesAtTheStartOfBlocks: false
MacroBlockBegin: ''
MacroBlockEnd:   ''
MaxEmptyLinesToKeep: 1
NamespaceIndentation: None
ObjCBinPackProtocolList: Auto
ObjCBlockIndentWidth: 2
ObjCBreakBeforeNestedBlockParam: true
ObjCSpaceAfterProperty: false
ObjCSpaceBeforeProtocolList: true
PenaltyBreakAssignment: 2
PenaltyBreakBeforeFirstCallParameter: 19
PenaltyBreakComment: 300
PenaltyBreakFirstLessLess: 120
PenaltyBreakString: 1000
PenaltyBreakTemplateDeclaration: 10
PenaltyExcessCharacter: 1000000
PenaltyReturnTypeOnItsOwnLine: 60
PenaltyIndentedWhitespace: 0
PointerAlignment: Right
ReflowComments:  true
SortIncludes:    true
SortJavaStaticImport: Before
SortUsingDeclarations: true
SpaceAfterCStyleCast: true
SpaceAfterLogicalNot: false
SpaceAfterTemplateKeyword: true
SpaceBeforeAssignmentOperators: true
SpaceBeforeCaseColon: false
SpaceBeforeCpp11BracedList: false
SpaceBeforeCtorInitializerColon: true
SpaceBeforeInheritanceColon: true
SpaceBeforeParens: ControlStatements
SpaceAroundPointerQualifiers: Default
SpaceBeforeRangeBasedForLoopColon: true
SpaceInEmptyBlock: false
SpaceInEmptyParentheses: false
SpacesBeforeTrailingComments: 1
SpacesInAngles:  false
SpacesInConditionalStatement: false
SpacesInContainerLiterals: true
SpacesInCStyleCastParentheses: false
SpacesInParentheses: false
SpacesInSquareBrackets: false
SpaceBeforeSquareBrackets: false
BitFieldColonSpacing: Both
Standard:        Latest
StatementMacros:
  - Q_UNUSED
  - QT_REQUIRE_VERSION
TabWidth:        4
UseCRLF:         false
UseTab:          Never
WhitespaceSensitiveMacros:
  - STRINGIZE
  - PP_STRINGIZE
  - BOOST_PP_STRINGIZE
  - NS_SWIFT_NAME
  - CF_SWIFT_NAME
... 
andrea-iob commented 2 years ago

Let us know which version of clang-format will be officially supported in the end.

Let's use version 12. I will replace the .clang-format file with yours.

andrea-iob commented 2 years ago

I've updated the .clang-format file.

andrea-iob commented 2 years ago

For each module/test I added a target named clang-format-<name> that will format all the sources of that module/test. There is also a global target, called clang-format, that depends on all other clang-format targets.

andrea-iob commented 2 years ago

I updated the style guide and I did the following changes to the configuration file:

 ---
 Language:        Cpp
 # BasedOnStyle:  LLVM
-AccessModifierOffset: -2
+AccessModifierOffset: -4
 AlignAfterOpenBracket: Align
 AlignConsecutiveMacros: Consecutive
 AlignConsecutiveAssignments: Consecutive
 AlignConsecutiveBitFields: Consecutive
 AlignConsecutiveDeclarations: None
 AlignEscapedNewlines: Right
 AlignOperands:   Align
-AlignTrailingComments: true
+AlignTrailingComments: false
 AllowAllArgumentsOnNextLine: true
 AllowAllConstructorInitializersOnNextLine: true
 AllowAllParametersOfDeclarationOnNextLine: true
 AllowShortEnumsOnASingleLine: true
 AllowShortBlocksOnASingleLine: Never
@@ -121,17 +121,17 @@ PenaltyBreakString: 1000
 PenaltyBreakTemplateDeclaration: 10
 PenaltyExcessCharacter: 1000000
 PenaltyReturnTypeOnItsOwnLine: 60
 PenaltyIndentedWhitespace: 0
 PointerAlignment: Right
-ReflowComments:  true
+ReflowComments:  false
 SortIncludes:    true
 SortJavaStaticImport: Before
 SortUsingDeclarations: true
 SpaceAfterCStyleCast: true
 SpaceAfterLogicalNot: false
-SpaceAfterTemplateKeyword: true
+SpaceAfterTemplateKeyword: false
 SpaceBeforeAssignmentOperators: true
 SpaceBeforeCaseColon: false
 SpaceBeforeCpp11BracedList: false
 SpaceBeforeCtorInitializerColon: true
 SpaceBeforeInheritanceColon: true

If there are no objections, I'm going to merge this pull request this week. After that, I will create additional pull requests for enforcing the coding style of the single bitpit modules.