Open ryanleary opened 5 years ago
Thanks. Basically we use the Google style, so that should work. Obviously check whether there are huge differences that look like they may be some kind of bug. We do occasionally put code through cpplint.py which checks for Google style issues.
On Sat, Mar 2, 2019 at 1:31 PM Ryan Leary notifications@github.com wrote:
@danpovey https://github.com/danpovey I would like to add a .clang-format file for the project -- which we could also add so that the CI verifies that formatting is properly done.
Would you have an interest in this? If so, please review the below clang-format configuration which is the default "Google-style" which I think is a bit different from what the Kaldi format appears to be. If you could help me tweak it to automatically format Kaldi C src as you like, I think that would be great. If you're not interested, feel free to go ahead and close the issue.
Language: Cpp
BasedOnStyle: Google
AccessModifierOffset: -1 AlignAfterOpenBracket: Align AlignConsecutiveAssignments: false AlignConsecutiveDeclarations: false AlignEscapedNewlines: Left AlignOperands: true AlignTrailingComments: true AllowAllParametersOfDeclarationOnNextLine: true AllowShortBlocksOnASingleLine: false AllowShortCaseLabelsOnASingleLine: false AllowShortFunctionsOnASingleLine: All AllowShortIfStatementsOnASingleLine: true AllowShortLoopsOnASingleLine: true AlwaysBreakAfterDefinitionReturnType: None AlwaysBreakAfterReturnType: None AlwaysBreakBeforeMultilineStrings: true AlwaysBreakTemplateDeclarations: Yes BinPackArguments: true BinPackParameters: true BraceWrapping: AfterClass: false AfterControlStatement: false AfterEnum: false AfterFunction: false AfterNamespace: false AfterObjCDeclaration: false AfterStruct: false AfterUnion: false AfterExternBlock: false BeforeCatch: false BeforeElse: false IndentBraces: false SplitEmptyFunction: true SplitEmptyRecord: true SplitEmptyNamespace: true BreakBeforeBinaryOperators: None BreakBeforeBraces: Attach BreakBeforeInheritanceComma: false BreakInheritanceList: BeforeColon BreakBeforeTernaryOperators: true BreakConstructorInitializersBeforeComma: false BreakConstructorInitializers: BeforeColon BreakAfterJavaFieldAnnotations: false BreakStringLiterals: true ColumnLimit: 80 CommentPragmas: '^ IWYU pragma:' CompactNamespaces: false ConstructorInitializerAllOnOneLineOrOnePerLine: true ConstructorInitializerIndentWidth: 4 ContinuationIndentWidth: 4 Cpp11BracedListStyle: true DerivePointerAlignment: true DisableFormat: false ExperimentalAutoDetectBinPacking: false FixNamespaceComments: true ForEachMacros:
- foreach
- Q_FOREACH
- BOOST_FOREACH IncludeBlocks: Preserve IncludeCategories:
- Regex: '^<ext/.*.h>' Priority: 2
- Regex: '^<.*.h>' Priority: 1
- Regex: '^<.*' Priority: 2
- Regex: '.*' Priority: 3 IncludeIsMainRegex: '(-_)?$' IndentCaseLabels: true IndentPPDirectives: None IndentWidth: 2 IndentWrappedFunctionNames: false JavaScriptQuotes: Leave JavaScriptWrapImports: true KeepEmptyLinesAtTheStartOfBlocks: false MacroBlockBegin: '' MacroBlockEnd: '' MaxEmptyLinesToKeep: 1 NamespaceIndentation: None ObjCBinPackProtocolList: Never ObjCBlockIndentWidth: 2 ObjCSpaceAfterProperty: false ObjCSpaceBeforeProtocolList: true PenaltyBreakAssignment: 2 PenaltyBreakBeforeFirstCallParameter: 1 PenaltyBreakComment: 300 PenaltyBreakFirstLessLess: 120 PenaltyBreakString: 1000 PenaltyBreakTemplateDeclaration: 10 PenaltyExcessCharacter: 1000000 PenaltyReturnTypeOnItsOwnLine: 200 PointerAlignment: Left RawStringFormats:
- Language: Cpp Delimiters:
- cc
- CC
- cpp
- Cpp
- CPP
- 'c++'
- 'C++' CanonicalDelimiter: '' BasedOnStyle: google
- Language: TextProto Delimiters:
- pb
- PB
- proto
- PROTO EnclosingFunctions:
- EqualsProto
- EquivToProto
- PARSE_PARTIAL_TEXT_PROTO
- PARSE_TEST_PROTO
- PARSE_TEXT_PROTO
- ParseTextOrDie
- ParseTextProtoOrDie CanonicalDelimiter: '' BasedOnStyle: google ReflowComments: true SortIncludes: true SortUsingDeclarations: true SpaceAfterCStyleCast: false SpaceAfterTemplateKeyword: true SpaceBeforeAssignmentOperators: true SpaceBeforeCpp11BracedList: false SpaceBeforeCtorInitializerColon: true SpaceBeforeInheritanceColon: true SpaceBeforeParens: ControlStatements SpaceBeforeRangeBasedForLoopColon: true SpaceInEmptyParentheses: false SpacesBeforeTrailingComments: 2 SpacesInAngles: false SpacesInContainerLiterals: true SpacesInCStyleCastParentheses: false SpacesInParentheses: false SpacesInSquareBrackets: false Standard: Auto StatementMacros:
- Q_UNUSED
- QT_REQUIRE_VERSION TabWidth: 8 UseTab: Never```
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/3070, or mute the thread https://github.com/notifications/unsubscribe-auth/ADJVu8FxFmI5vrzKRzHO-6B5aTefNWGdks5vSsOFgaJpZM4baiXs .
Ok great. I'll work on setting up the CI and making tweaks to the config to see if I can come close to what is generally in the codebase.
Thanks. Don't worry too much about the .cu files or the *-ansi.h files. There is no particularly well-defined style for them.
On Sat, Mar 2, 2019 at 1:43 PM Ryan Leary notifications@github.com wrote:
Ok great. I'll work on setting up the CI and making tweaks to the config to see if I can come close to what is generally in the codebase.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/3070#issuecomment-468948183, or mute the thread https://github.com/notifications/unsubscribe-auth/ADJVu1Q12r9M5oX3BWZzg60T3sP2lageks5vSsZBgaJpZM4baiXs .
Super, excellent idea! At a quick glance, we can trim down some fat immediately:
ForEachMacros:
- foreach
- Q_FOREACH
- BOOST_FOREACH
These macros should never occur in Kaldi, ok to remove the section, just noise.
IncludeCategories:
- Regex: '^<ext/.*\.h>'
Priority: 2
We do not have any includes like `#include <ext/.....h>, this is just noise, too.
IncludeIsMainRegex: '([-_](test|unittest))?$'
This is for includes of the tested file's headers in tests, e. g. in a file 'foo-test.cc', #include "foo.h"
is considered the "main" include (priority group 0), as it is in 'foo.cc'. The regex can be simplified to '(-test)?$'
in our usage, as all tests follow the pattern 'foo-test.cc', for an arbitrary value of 'foo.cc'.
RawStringFormats:
The whole section is for checking language fragments inside C strings, not relevant to us, to the best of my knowledge.
StatementMacros:
- Q_UNUSED
- QT_REQUIRE_VERSION
These also never occur.
But in theory, we should have been using the Google style all along... So I would just start with just a 2-line file
Language: Cpp
BasedOnStyle: Google
(uncomment it) and then would see where it does not cut it. Then override only what would really produce a lot of mismatches.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
This issue has been automatically closed by a bot strictly because of inactivity. This does not mean that we think that this issue is not important! If you believe it has been closed hastily, add a comment to the issue and mention @kkm000, and I'll gladly reopen it.
@danpovey I would like to add a .clang-format file for the project -- which we could also add so that the CI verifies that formatting is properly done.
Would you have an interest in this? If so, please review the below clang-format configuration which is the default "Google-style" which I think is a bit different from what the Kaldi format appears to be. If you could help me tweak it to automatically format Kaldi C src as you like, I think that would be great. If you're not interested, feel free to go ahead and close the issue.