From a76226343e71b9e121a53b4452612f6d7c8f8d72 Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Mon, 20 Jul 2026 22:15:00 -0400 Subject: [PATCH] build(pre-commit): stop the whitespace fixers rewriting vendored trees MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `trailing-whitespace`, `end-of-file-fixer`, and `mixed-line-ending` REWRITE files rather than merely checking them, and had no exclusion at all. A `pre-commit run --all-files` therefore silently reformatted 41 files across trees this repo deliberately keeps byte-identical to upstream: the vendored TriCNES C# under crates/rustynes-test-harness/golden/tricnes/, the vendored rcheevos C under crates/rustynes-cheevos/vendor/, the immutable ref-docs/ corpus, an upstream font licence (assets/RustyNES_Icon/OFL.txt), and upstream test-ROM READMEs. That is a real defect, not a cosmetic one. Those trees exist precisely so they can be diffed against upstream and so provenance/licence text stays verbatim; rewriting them destroys that property, and it does so during a command the contributing docs actively recommend (line 4 of this file). It also drags unrelated churn into whatever change happened to run the hooks broadly. .markdownlintignore already exempted most of these paths for markdownlint. This is the missing equivalent for the fixers, shared across all three via a YAML anchor so the three lists cannot drift apart. Scope is deliberately NARROWER than .markdownlintignore: only content we did not author (vendored / upstream / immutable reference). Frozen-but-ours trees — docs/archive/, to-dos/archive/, to-dos/plans/, docs/monetization/ — stay in scope, because they are our own prose and whitespace hygiene on them is harmless. Excluding them would over-fit the rule to "things markdownlint skips" rather than to the actual invariant being protected. Verified by re-running the exact command that caused the damage: it now reports 41 fewer modified files, touching only scripts/diag/celldiff.py — our own script, with two genuine trailing-whitespace slips, fixed here so `pre-commit run --all-files` is clean from a clean tree. --- .pre-commit-config.yaml | 28 ++++++++++++++++++++++++++++ scripts/diag/celldiff.py | 4 ++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b4dcb51f..db8e2d22 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -8,9 +8,34 @@ repos: - repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.5.0 hooks: + # The three hooks below REWRITE files rather than just checking them, so + # they must not touch trees we do not author. Without this exclusion a + # `pre-commit run --all-files` silently reformats vendored upstream source + # and the immutable reference corpus — observed: 41 files across the + # vendored TriCNES C#, the vendored rcheevos C, `ref-docs/`, an upstream + # font licence, and upstream test-ROM READMEs. That destroys the "kept + # pristine / byte-identical to upstream" property those trees exist for, + # and pollutes any diff that happens to run the hooks broadly. + # + # `.markdownlintignore` already exempts most of these for markdownlint; + # this is the equivalent for the whitespace fixers, which had no exclusion + # at all. Scope is deliberately narrower than `.markdownlintignore`: only + # content we did not write (vendored / upstream / immutable reference). + # Frozen-but-ours trees (`docs/archive/`, `to-dos/plans/`, …) stay in + # scope — they are our prose, and whitespace hygiene on them is harmless. - id: trailing-whitespace args: [--markdown-linebreak-ext=md] + exclude: &vendored_trees | + (?x)^( + ref-docs/ + |ref-proj/ + |crates/rustynes-test-harness/golden/tricnes/ + |crates/rustynes-cheevos/vendor/ + |tests/roms/ + |assets/RustyNES_Icon/OFL\.txt$ + ) - id: end-of-file-fixer + exclude: *vendored_trees - id: check-yaml # mkdocs.yml uses Material for MkDocs' documented `!!python/name:` YAML # tags (pymdownx.emoji icon rendering), which check-yaml's safe_load @@ -35,6 +60,9 @@ repos: - id: check-merge-conflict - id: mixed-line-ending args: [--fix=lf] + # Third rewriting hook — same exclusion. Upstream `.resx` / `.csproj` + # files in the vendored TriCNES tree legitimately carry CRLF. + exclude: *vendored_trees # Markdown linting - repo: https://github.com/igorshubovych/markdownlint-cli diff --git a/scripts/diag/celldiff.py b/scripts/diag/celldiff.py index fef7e480..50745450 100644 --- a/scripts/diag/celldiff.py +++ b/scripts/diag/celldiff.py @@ -45,7 +45,7 @@ def rnorm(acc): # Walk both from landmark, report first structural divergences (addr or normalized-kind) mlist=mes[mi:]; rlist=rny[ri:] -# Mesen has NO Idle rows; RustyNES emits I rows. To align, drop RustyNES I rows? +# Mesen has NO Idle rows; RustyNES emits I rows. To align, drop RustyNES I rows? # But I rows ARE cycles. Mesen logs a read every cycle (no idle). So an I in Rusty # where Mesen has a read = the structural divergence we hunt. Keep both, align by index. divs=0 @@ -57,7 +57,7 @@ def rnorm(acc): if ma!=ra or mk2!=rk2: print(f"off={off:5d} | MES {mk}{('('+mk+')') if mk in 'HDAG' else ''} {ma:04X}={mv:02X} | RNY {racc} {ra:04X}={rd:02X} indmc={indmc}") divs+=1 - if divs>=40: + if divs>=40: print("... (capped at 40)") break print(f"total divergences shown (cap 40 of first {n} aligned cycles)")