build: make uncrustify enforce the project's actual code style - #121
Conversation
uncrustify.cfg was a verbatim `--show-config` dump of Uncrustify 0.78.1 with
zero customizations, so every rule sat at its default (mostly "ignore") and a
full run reformatted 0 of 71 files. Replace it with a 166-line config that
encodes the style already dominant in src/ and samples/: K&R braces, hard tabs
at width 8, "if (" with a space, "foo()" without, pointer star bound to the
name, casts without a trailing space, "case" at the "switch" level and
preprocessor directives in column 1.
Also:
- add .gitattributes so line endings are LF in the repository (src/ held 57
CRLF files against 14 LF ones), matching the config's `newlines = lf`
- rewrite run-uncrustify.sh: discover files through `git ls-files` so build
output and submodules drop out on their own, cover samples/ as well as src/,
match .uncrustifyignore as a path prefix instead of a loose substring (the
file listed only lib/ paths while the script scanned src/ only, so it never
matched anything), and add a --check mode
- add an `uncrustify-check` CMake target and a `format` CI job pinned to
Uncrustify 0.83.0
Applies the .gitattributes rules added in the previous commit. Whitespace only: no file's content changes when compared with CR stripped.
Whitespace only. Verified semantics-preserving: the C token stream of each of the 69 changed files is byte-identical to its previous revision.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52394b117d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| elif git rev-parse --is-inside-work-tree >/dev/null 2>&1; then | ||
| # Tracked files only: skips build output and submodule contents. | ||
| local roots=() r | ||
| for r in "${ROOTS[@]}"; do roots+=("$r/*.c" "$r/*.h"); done |
There was a problem hiding this comment.
When a PR changes a C++ source, the new format job still succeeds without checking it because the job invokes ./run-uncrustify.sh --check without filenames and this pathspec collects only .c and .h files. In particular, the built samples/shapes/main.cpp and its tracked shapes.hpp header are omitted even though they currently contain constructs that the new configuration would reformat, so include .cpp and .hpp in default discovery.
Useful? React with 👍 / 👎.
1 participant
Problem
uncrustify.cfgwas a verbatimuncrustify --show-configdump from version 0.78.1, with zero customizations. Comparing it option by option against the defaults, the only difference was a duplicatedsp_paren_parenline. Since nearly every rule ships asignore, running the formatter changed nothing:So: there was a
make uncrustifytarget, there was a.uncrustifyignore, there was a script — and none of it enforced any style. Security theater.Other problems found:
.uncrustifyignorewas dead code. It listed onlylib/paths, but the script scannedsrc/only. No pattern ever matched.src/, no.gitattributes, and the config'snewlines = autokept the mix alive.samples/was never formatted.for file in $files, ignore matching by loose substring, no check mode.Changes
uncrustify.cfg— 152 KB / 850 options → 166 lines. Only the rules that describe the style, derived by counting occurrences in the existing code:{on the same line)if (with a spacefoo(without onechar *p(int)xwithout a space} elseon one linecaseat theswitchlevel#includein column 1code_width = 0— the p99 line is 116 columns; automatic wrapping would only cause damage. There is a comment at the top of the file warning never to paste a--show-configdump over it, which is exactly what flattened the previous config..gitattributes(new) —* text=auto eol=lf, CRLF kept for.bat, binaries marked.run-uncrustify.sh— rewritten:git ls-files, which drops build output and submodules on its own (the oldfindsaw 88 files, the real count is 85 —samples/cbuild/CMakeFiles/.../CMakeCCompilerId.cand friends were in scope)samples/as well assrc/.uncrustifyignorenow matches as a prefix of the repo-relative path--checkmode (writes nothing, exits 1),--help, accepts individual filesset -euo pipefail, NUL-delimited arraysCMakeLists.txt— newuncrustify-checktarget..github/workflows/Build.yml— aformatjob with Uncrustify pinned to 0.83.0, built from the tag and cached.apt installwould bring a moving version and the check would start failing on its own after an upgrade..git-blame-ignore-revs— the two whitespace commits, sogit blamestays useful.Verification
The reformatting is semantics-preserving.
diff -wflagged 24 files, but it does not forgive line joins (} else). I compared the C token streams before and after, using a tokenizer that handles strings, chars, comments and line continuations:Idempotent — a second pass changes nothing.
Builds —
makeexits 0 with no new warnings under-Wall,make installOK, and theaudio,shapesandadhocsamples build.Blame preserved — lines remain attributed to their original authors.
How to review
The commits are split by nature; worth reviewing one at a time:
34d7fea20816b6d49cd335bf440052394b1For the formatting diff without the noise:
git diff -w 20816b6 5bf4400.