-
Notifications
You must be signed in to change notification settings - Fork 273
Reformat sources with clang-format #1324
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
f18979a
Add clang-format config
reuk 6ce0dba
Add travis style check
reuk 554cb54
Adjust cpplint to disable whitespace checks by default
reuk 1dcc82c
Convert COMPILING to markdown format
reuk 8482b35
Add information about using clang-format
reuk 9d4b827
Update coding standard
reuk 55e6594
Fixup cpplint.py
reuk a24ac3d
Fixup compiling.md with more clang-format install instructions
reuk 2110cd1
Make formatting stage non-blocking on Travis
reuk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| --- | ||
| AccessModifierOffset: '-2' | ||
| AlignAfterOpenBracket: AlwaysBreak | ||
| AlignConsecutiveAssignments: 'false' | ||
| AlignConsecutiveDeclarations: 'false' | ||
| AlignEscapedNewlinesLeft: 'false' | ||
| AlignOperands: 'true' | ||
| AlignTrailingComments: 'true' | ||
| AllowAllParametersOfDeclarationOnNextLine: 'false' | ||
| AllowShortBlocksOnASingleLine: 'false' | ||
| AllowShortCaseLabelsOnASingleLine: 'false' | ||
| AllowShortFunctionsOnASingleLine: None | ||
| AllowShortIfStatementsOnASingleLine: 'false' | ||
| AllowShortLoopsOnASingleLine: 'false' | ||
| AlwaysBreakAfterReturnType: None | ||
| AlwaysBreakBeforeMultilineStrings: 'true' | ||
| AlwaysBreakTemplateDeclarations: 'true' | ||
| BinPackArguments: 'false' | ||
| BinPackParameters: 'false' | ||
| BreakBeforeBinaryOperators: None | ||
| BreakBeforeBraces: Allman | ||
| BreakBeforeTernaryOperators: 'true' | ||
| BreakConstructorInitializersBeforeComma: 'false' | ||
| ColumnLimit: '80' | ||
| ConstructorInitializerAllOnOneLineOrOnePerLine: 'true' | ||
| ConstructorInitializerIndentWidth: '2' | ||
| ContinuationIndentWidth: '2' | ||
| Cpp11BracedListStyle: 'true' | ||
| DerivePointerAlignment: 'false' | ||
| DisableFormat: 'false' | ||
| ExperimentalAutoDetectBinPacking: 'false' | ||
| ForEachMacros: [ | ||
| 'forall_rw_range_set_r_objects', | ||
| 'Forall_rw_range_set_r_objects', | ||
| 'forall_rw_range_set_w_objects', | ||
| 'Forall_rw_range_set_w_objects', | ||
| 'forall_rw_set_r_entries', | ||
| 'Forall_rw_set_r_entries', | ||
| 'forall_rw_set_w_entries', | ||
| 'Forall_rw_set_w_entries', | ||
| 'forall_goto_functions', | ||
| 'Forall_goto_functions', | ||
| 'forall_goto_program_instructions', | ||
| 'Forall_goto_program_instructions', | ||
| 'forall_objects', | ||
| 'Forall_objects', | ||
| 'forall_valid_objects', | ||
| 'Forall_valid_objects', | ||
| 'forall_nodes', | ||
| 'Forall_nodes', | ||
| 'forall_literals', | ||
| 'Forall_literals', | ||
| 'forall_operands', | ||
| 'Forall_operands', | ||
| 'forall_expr', | ||
| 'Forall_expr', | ||
| 'forall_expr_list', | ||
| 'Forall_expr_list', | ||
| 'forall_symbolptr_list', | ||
| 'Forall_symbolptr_list', | ||
| 'forall_guard', | ||
| 'Forall_guard', | ||
| 'forall_irep', | ||
| 'Forall_irep', | ||
| 'forall_named_irep', | ||
| 'Forall_named_irep', | ||
| 'forall_value_list', | ||
| 'Forall_value_list', | ||
| 'forall_symbols', | ||
| 'Forall_symbols', | ||
| 'forall_symbol_base_map', | ||
| 'Forall_symbol_base_map', | ||
| 'forall_symbol_module_map', | ||
| 'Forall_symbol_module_map', | ||
| 'forall_subtypes', | ||
| 'Forall_subtypes'] | ||
| IndentCaseLabels: 'false' | ||
| IndentWidth: '2' | ||
| IndentWrappedFunctionNames: 'false' | ||
| KeepEmptyLinesAtTheStartOfBlocks: 'false' | ||
| Language: Cpp | ||
| MaxEmptyLinesToKeep: '1' | ||
| NamespaceIndentation: None | ||
| PenaltyBreakString: 10000 | ||
| PointerAlignment: Right | ||
| ReflowComments: 'false' | ||
| SortIncludes: 'false' | ||
| SpaceAfterCStyleCast: 'false' | ||
| SpaceBeforeAssignmentOperators: 'true' | ||
| SpaceBeforeParens: Never | ||
| SpaceInEmptyParentheses: 'false' | ||
| SpacesBeforeTrailingComments: '1' | ||
| SpacesInAngles: 'false' | ||
| SpacesInCStyleCastParentheses: 'false' | ||
| SpacesInContainerLiterals: 'false' | ||
| SpacesInParentheses: 'false' | ||
| SpacesInSquareBrackets: 'false' | ||
| Standard: Cpp11 | ||
| TabWidth: '2' | ||
| UseTab: Never | ||
|
|
||
| ... |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unsure as to whether this should be another commit or not. In some ways it would be nice to roll the tool out locally, let people use it, then add it to CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really understand the drawbacks of enabling this straight away. Could you give some examples?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may wrongly block other PRs from passing CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless there's a way of enabling travis checks just for new branches, I believe this will be an issue whenever we enable this check. In any case, the formatting can be corrected using the instructions in the COMPILING.md (the 'retroactively formatting individual commits' section).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@reuk would this step block a PR? The current linter, if fails, doesn't fail the PR. This shouldn't either, since there may be unforseen problems.
However, providing it doesn't then I would have thought turning it on in this PR would be the quickest way to find out if there are issues (as opposed to relying on people doing it locally).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment it does block, but maybe if we made it non-blocking like the linter that would be a good solution - we can always make it blocking in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking is good. It seems like the kind of thing that it might take a little while to get used to and to get to love, so making it blocking on day 1 seems a bit hasty. There may also be unforseen things that we need to iron out and I'd rather not block commits on CI while that happens.