Commit 6dd84659489e49a10a3671ffbaf2d2b0681d99fc
1 parent
6aa58d51
TODO: solidify plans for code formatting
Showing
2 changed files
with
59 additions
and
48 deletions
TODO
| @@ -3,8 +3,9 @@ Next | @@ -3,8 +3,9 @@ Next | ||
| 3 | ==== | 3 | ==== |
| 4 | 4 | ||
| 5 | In order: | 5 | In order: |
| 6 | -* PointerHolder -> shared_ptr | 6 | +* PR #661 (overloaded getters from m-holger) |
| 7 | * code formatting | 7 | * code formatting |
| 8 | +* PointerHolder -> shared_ptr | ||
| 8 | * cmake | 9 | * cmake |
| 9 | * ABI including --json default is latest | 10 | * ABI including --json default is latest |
| 10 | * json v2 | 11 | * json v2 |
| @@ -34,62 +35,69 @@ Soon: Break ground on "Document-level work" | @@ -34,62 +35,69 @@ Soon: Break ground on "Document-level work" | ||
| 34 | Code Formatting | 35 | Code Formatting |
| 35 | =============== | 36 | =============== |
| 36 | 37 | ||
| 37 | -It would be good to have automatic code formatting to make the code | ||
| 38 | -more consistent and to make it easier for contributors. We would do a | ||
| 39 | -big commit to bring everything up to spec. Things to keep in mind: | ||
| 40 | - | ||
| 41 | -* clang-format looks promising but is a bit of a moving target; need | ||
| 42 | - to see if its output has been stable over the past few releases | ||
| 43 | - since the first one that can produce code the way I like it. I may | ||
| 44 | - have to require a minimum clang-format version. | ||
| 45 | - | ||
| 46 | -* Ideas here aim for something similar to rust's defaults but with | ||
| 47 | - adjustments to meet my existing style and preferences: | ||
| 48 | - | ||
| 49 | - * 80 columns | ||
| 50 | - | ||
| 51 | - * 4-space indent (no tabs) | ||
| 52 | - | ||
| 53 | - * Compact braces. While this is a big departure from the past and | ||
| 54 | - will create many changes, I have become accustomed to this style | ||
| 55 | - and use it across my projects in other languages these days. | ||
| 56 | - | ||
| 57 | - * No "bin packing" -- if arguments (constructor initializers, | ||
| 58 | - function arguments, etc.) don't fit on one line, do one argument | ||
| 59 | - per line | ||
| 60 | - | ||
| 61 | - * With the exception of short lambdas, no block constructs can be | ||
| 62 | - collapsed to a single line. | 38 | +Use clang-format-15. |
| 63 | 39 | ||
| 64 | - * Braces are mandatory for all control constructs (no if, while, | ||
| 65 | - etc. without braces) | 40 | +* Put a .clang-format at the top of the repository -- see below for content. |
| 41 | +* Reformat all files: | ||
| 66 | 42 | ||
| 67 | - * Space after control constructs | 43 | + for i in **/*.cc **/*.c **/*.h **/*.hh; do |
| 44 | + clang-format < $i >| $i.new && mv $i.new $i | ||
| 45 | + done | ||
| 68 | 46 | ||
| 69 | -* Try to get emacs c-style to match as closely as possible | ||
| 70 | - | ||
| 71 | -* Consider blame.ignoreRevsFile if it seems to help | 47 | +* Carefully inspect the diff. There are some places where a comment to |
| 48 | + force a line break might be in order. | ||
| 72 | 49 | ||
| 73 | -* See also https://bestpractices.coreinfrastructure.org/en | ||
| 74 | - | ||
| 75 | -* QTC::TC first two arguments have to be lexically on one line. If the | ||
| 76 | - code formatter breaks this, some QTC calls may have to be surrounded by | 50 | +* Update README-maintainer about formatting. Mention |
| 77 | 51 | ||
| 78 | // clang-format off | 52 | // clang-format off |
| 79 | // clang-format on | 53 | // clang-format on |
| 80 | 54 | ||
| 81 | - or qtest may have to be made more flexible unless the formatter has | ||
| 82 | - some rules about some places where lines shouldn't be broken. | ||
| 83 | - | ||
| 84 | -* auto_* files from generate_auto_job should be exempt from | ||
| 85 | - formatting. | 55 | + as well as the use of a comment to force a line break. |
| 56 | + | ||
| 57 | +Tentative .clang-format: | ||
| 58 | + | ||
| 59 | +``` | ||
| 60 | +--- | ||
| 61 | +Language: Cpp | ||
| 62 | +BasedOnStyle: LLVM | ||
| 63 | +AlignAfterOpenBracket: AlwaysBreak | ||
| 64 | +AlignEscapedNewlines: DontAlign | ||
| 65 | +AllowShortFunctionsOnASingleLine: None | ||
| 66 | +BinPackArguments: false | ||
| 67 | +BinPackParameters: false | ||
| 68 | +BreakConstructorInitializers: BeforeComma | ||
| 69 | +DeriveLineEnding: false | ||
| 70 | +PackConstructorInitializers: Never | ||
| 71 | +IncludeCategories: | ||
| 72 | + - Regex: '^["<](qpdf)/' | ||
| 73 | + Priority: 1 | ||
| 74 | + SortPriority: 0 | ||
| 75 | + CaseSensitive: false | ||
| 76 | + - Regex: '.*' | ||
| 77 | + Priority: 2 | ||
| 78 | + SortPriority: 0 | ||
| 79 | + CaseSensitive: false | ||
| 80 | + - Regex: '.*' | ||
| 81 | + Priority: 1 | ||
| 82 | + SortPriority: 0 | ||
| 83 | + CaseSensitive: false | ||
| 84 | +IndentCaseBlocks: true | ||
| 85 | +IndentWidth: 4 | ||
| 86 | +InsertTrailingCommas: Wrapped | ||
| 87 | +KeepEmptyLinesAtTheStartOfBlocks: false | ||
| 88 | +PointerAlignment: Left | ||
| 89 | +``` | ||
| 90 | + | ||
| 91 | +Remaining work: | ||
| 86 | 92 | ||
| 87 | -Ideally it should be possible to run formatting in CI so that pull | ||
| 88 | -requests have to be properly formatting, but if not, there needs to be | ||
| 89 | -a `make format` similar to `make spell` that I could apply after | ||
| 90 | -merging contributions and from time to time. | ||
| 91 | - | ||
| 92 | -A .clang-format file can be created at the top of the repository. | 93 | +* Try to get emacs c-style to match as closely as possible |
| 94 | +* Consider blame.ignoreRevsFile if it seems to help | ||
| 95 | +* Add a `make format` similar to `make spell` (or whatever this ends | ||
| 96 | + up being with cmake) | ||
| 97 | +* Consider a github action to check formatting. I don't want | ||
| 98 | + formatting failures to prevent all the tests from being run. | ||
| 99 | + Alternatively, add running `make format` as a release preparation | ||
| 100 | + check like `make spell`. | ||
| 93 | 101 | ||
| 94 | Output JSON v2 | 102 | Output JSON v2 |
| 95 | ============== | 103 | ============== |
| @@ -782,6 +790,8 @@ directory or that are otherwise not publicly accessible. This includes | @@ -782,6 +790,8 @@ directory or that are otherwise not publicly accessible. This includes | ||
| 782 | things sent to me by email that are specifically not public. Even so, | 790 | things sent to me by email that are specifically not public. Even so, |
| 783 | I find it useful to make reference to them in this list. | 791 | I find it useful to make reference to them in this list. |
| 784 | 792 | ||
| 793 | + * Look at https://bestpractices.coreinfrastructure.org/en | ||
| 794 | + | ||
| 785 | * Get rid of remaining assert() calls from non-test code. | 795 | * Get rid of remaining assert() calls from non-test code. |
| 786 | 796 | ||
| 787 | * Consider updating the fuzzer with code that exercises | 797 | * Consider updating the fuzzer with code that exercises |