style: Add cmake-format pre-commit hook #6279
Conversation
mathbunnyru
requested review from
bthomee,
Copilot and
pratikmankawde
and removed request for
Copilot
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #6279 +/- ##
=========================================
- Coverage 79.4% 79.4% -0.0%
=========================================
Files 839 839
Lines 71625 71625
Branches 8241 8236 -5
=========================================
- Hits 56861 56855 -6
- Misses 14764 14770 +6 see 4 files with indirect coverage changes 🚀 New features to boost your workflow:
|
CMakeLists.txt
Outdated
| endif () | ||
|
|
||
| ### | ||
| # |
There was a problem hiding this comment.
Maybe this section "separator" character should just be removed?
.cmake-format.yaml
Outdated
| foo: | ||
| flags: | ||
| - BAR | ||
| - BAZ | ||
| kwargs: | ||
| HEADERS: "*" | ||
| SOURCES: "*" | ||
| DEPENDS: "*" |
There was a problem hiding this comment.
Is this an actual sub-command that can be provided, and if so what does it do?
There was a problem hiding this comment.
I think it was an example from somewhere, and command foo doesn't make sense, removed it
.cmake-format.yaml
Outdated
| disable: false | ||
| _help_line_width: | ||
| - How wide to allow formatted cmake files | ||
| line_width: 120 |
There was a problem hiding this comment.
Although the line width adopted throughout the code somewhat varies from file to file and from moment to moment, it generally seems to be 80 characters. I'd suggest to therefore set this value to 80. Also in .clang-format there is ColumnLimit: 80.
.cmake-format.yaml
Outdated
| - What character to use for bulleted lists | ||
| bullet_char: "*" |
There was a problem hiding this comment.
FWIW: The cspell changes * to - in READMEs. Perhaps we should make this consistent and change to - too?
(I prefer *, but I like consistency even more.)
There was a problem hiding this comment.
I think it's prettier, not cspell.
I agree, consistency is nice, changed here
cmake/XrplCompiler.cmake
Outdated
| INTERFACE -bigobj # Increase object file max size | ||
| -fp:precise # Floating point behavior | ||
| -Gd # __cdecl calling convention | ||
| -Gm- # Minimal rebuild: disabled | ||
| -Gy- # Function level linking: disabled | ||
| -MP # Multiprocessor compilation | ||
| -openmp- # pragma omp: disabled | ||
| -errorReport:none # No error reporting to Internet | ||
| -nologo # Suppress login banner | ||
| -wd4018 # Disable signed/unsigned comparison warnings | ||
| -wd4244 # Disable float to int possible loss of data warnings | ||
| -wd4267 # Disable size_t to T possible loss of data warnings | ||
| -wd4800 # Disable C4800(int to bool performance) | ||
| -wd4503 # Decorated name length exceeded, name was truncated |
There was a problem hiding this comment.
I quite like having the comments nicely aligned vertically. Is there an option in the .cmake-format to keep it how it was?
There was a problem hiding this comment.
I haven't found such an option.
But I was able to add comments above flags, I think it's readable enough and looks nice, wdyt?
Co-authored-by: Bart <bthomee@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a cmake-format pre-commit hook to automatically format CMake files according to a consistent style guide. The configuration matches the style used in Clio. The PR applies the formatting to most CMake files in the repository, with some files explicitly excluded as exceptions to be handled in a future PR.
Changes:
- Added cmake-format pre-commit hook with configuration matching Clio's style
- Reformatted multiple CMake files with consistent spacing, indentation, and line wrapping
- Added cmake-format configuration terms to cspell dictionary
Reviewed changes
Copilot reviewed 17 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| .pre-commit-config.yaml | Adds cmake-format hook with excluded files list |
| .cmake-format.yaml | Comprehensive cmake-format configuration with 120-char line width |
| .config/cspell.config.yaml | Adds legitimate terms from cmake-format config (hwrap, kwarg, kwargs, pargs) |
| src/tests/libxrpl/CMakeLists.txt | Reformats if/endif spacing |
| cmake/deps/Boost.cmake | Reformats find_package, target_link_libraries, and if/endif blocks |
| cmake/create_symbolic_link.cmake | Reformats function and if/endif spacing, condenses multi-line comment |
| cmake/XrplVersion.cmake | Reformats foreach and if/endif spacing |
| cmake/XrplValidatorKeys.cmake | Reformats option, function calls, and FetchContent_Declare |
| cmake/XrplSettings.cmake | Reformats multiple option, if/endif, and set commands with improved comment wrapping |
| cmake/XrplSanity.cmake | Reformats set, if/endif, and message calls with improved multi-line message formatting |
| cmake/XrplSanitizers.cmake | Reformats extensive if/endif blocks, foreach loops, and message calls |
| cmake/XrplInterface.cmake | Reformats target configuration commands and if/endif blocks |
| cmake/XrplCov.cmake | Reformats if/endif and function calls with improved argument layout |
| cmake/XrplConfig.cmake | Reformats include, set, find_dependency, and if/endif blocks |
| cmake/XrplCompiler.cmake | Reformats extensive target_compile_options, target_link_libraries, and if/endif blocks |
| cmake/XrplAddTest.cmake | Reformats function definition and file globbing |
| cmake/CompilationEnv.cmake | Reformats multi-line comments and if/endif blocks |
| cmake/Ccache.cmake | Reformats if/endif blocks and multi-line comments |
| cmake/CMakeFuncs.cmake | Reformats macro/function definitions and execute_process calls |
| CMakeLists.txt | Reformats if/endif blocks, include statements, and target_link_libraries calls |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2 participants
High Level Overview of Change
I add
cmake-formatpre-commit hook.The style file is exactly like in Clio (so I would probably argue not to change it).
For now, I've added files where applying cmake-format doesn't look too bad, and added other files as exceptions - manually adjusting these files is a bit time-consuming, so I would probably do it separately, but we can benefit from cmake-format for most files already.
Context of Change
Type of Change
.gitignore, formatting, dropping support for older tooling)API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)