mirror of
https://github.com/kuhyx/screen-locker.git
synced 2026-11-10 19:27:15 +01:00
Enforce the 250-line cap on C++ and markdown
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzxdWYdwRAtMg7ki6TKxuU
This commit is contained in:
parent
9e71954348
commit
e47f42eb49
@ -240,7 +240,9 @@ repos:
|
||||
# the gate it was being refactored to satisfy. ts/tsx joined on
|
||||
# 2026-08-29 with web/, for exactly the same reason: a language absent
|
||||
# from this list is a directory the cap silently does not apply to.
|
||||
types_or: [python, shell, dart, ts, tsx]
|
||||
# c/c++ and markdown joined on 2026-11-01 (the Linux runner had grown
|
||||
# to 258; prose is under the same cap per the 250-line rule).
|
||||
types_or: [python, shell, dart, ts, tsx, c, c++, markdown]
|
||||
exclude: '^web/(node_modules|dist|coverage)/'
|
||||
- id: no-silent-failures
|
||||
name: Block silent failures (swallowed exceptions and silent sys.exit(0) must say why)
|
||||
|
||||
@ -1,95 +0,0 @@
|
||||
# Refactor TODO — enforce the 250-line file cap
|
||||
|
||||
> **This file is a ready-to-use prompt.** Paste it to Claude, or open this repo
|
||||
> and say "do refactor_claude_todo". It is self-contained: everything needed to
|
||||
> execute is below. Generated 2026-08-14 from a measured survey of every repo.
|
||||
|
||||
## Goal
|
||||
|
||||
Every file in this repo must be **at most 250 lines** — source, tests, and
|
||||
prose (`.md`/`.txt`/`.rst`/`.tex`) alike — and must **stay** that way forever,
|
||||
enforced by a gate that fails the commit, not by a note anyone can ignore.
|
||||
|
||||
Why: a file that cannot be read in one piece forces re-reads and partial edits,
|
||||
which is the single largest avoidable cost in an LLM-assisted workflow. Aim by
|
||||
churn, not size alone — refactoring pays where code is read and changed often
|
||||
(Fowler, _refactoring economic benefit_).
|
||||
|
||||
## Scope in this repo
|
||||
|
||||
<!-- BEGIN GENERATED VIOLATIONS -->
|
||||
|
||||
- **10 files** currently exceed 250 lines.
|
||||
- **1,828 lines** over the cap in total (the work left to do); longest file is **740** lines.
|
||||
|
||||
ROI = lines x commits in the last year. Work top-down; a long file nobody edits
|
||||
has near-zero payoff and should not be first.
|
||||
|
||||
| lines | commits/yr | kind | file |
|
||||
| ----: | ---------: | :--- | :--- |
|
||||
| 503 | 13 | code | `stronglift_replacement/workout_app/lib/screens/settings_screen.dart` |
|
||||
| 633 | 9 | code | `stronglift_replacement/workout_app/lib/screens/workout_screen.dart` |
|
||||
| 740 | 7 | code | `stronglift_replacement/workout_app/test/services/workout_sync_service_test.dart` |
|
||||
| 322 | 10 | code | `stronglift_replacement/workout_app/lib/services/workout_sync_service.dart` |
|
||||
| 383 | 7 | code | `stronglift_replacement/workout_app/test/screens/home_screen_test.dart` |
|
||||
| 260 | 9 | code | `stronglift_replacement/workout_app/lib/screens/home_screen.dart` |
|
||||
| 386 | 5 | code | `stronglift_replacement/workout_app/test/screens/history_screen_test.dart` |
|
||||
| 362 | 4 | code | `stronglift_replacement/workout_app/test/screens/workout_screen_test.dart` |
|
||||
| 313 | 2 | code | `stronglift_replacement/workout_app/lib/screens/manual_workout_screen.dart` |
|
||||
| 426 | 1 | code | `stronglift_replacement/workout_app/test/services/progression_sync_service_test.dart` |
|
||||
|
||||
<!-- END GENERATED VIOLATIONS -->
|
||||
|
||||
|
||||
Exempt (do NOT split these):
|
||||
|
||||
- generated files — `*.g.dart`, `*.freezed.dart`, `*.gr.dart`, `**/l10n/generated/**`,
|
||||
anything with a `GENERATED` header
|
||||
- markup — `.html`, `.css`, `.scss`
|
||||
- data files — `.json`, `.yaml`, `.csv`, wordlists and other data-ish `.txt`
|
||||
(mean line length under 25 chars)
|
||||
|
||||
|
||||
## How to split
|
||||
|
||||
- **Python** — extract cohesive helpers into sibling modules; keep the public
|
||||
API and imports stable.
|
||||
- **Shell** — split into `lib/*.sh` sourced by a thin entry script. Keep
|
||||
`set -euo pipefail` in each.
|
||||
- **Dart / TypeScript** — extract widgets/components into their own files.
|
||||
- **Tests** — split by test-group into sibling files
|
||||
(`foo.test.ts` -> `foo.parsing.test.ts`, `foo.render.test.ts`). Coverage must
|
||||
not drop.
|
||||
- **Docs** — split into topic files under `docs/` with an index. For an
|
||||
oversized `CLAUDE.md`, move detail into referenced docs so the
|
||||
always-loaded part shrinks.
|
||||
|
||||
**Do not** game the cap: no one-lining, no deleting tests, no moving code into
|
||||
an exempt extension, no `# noqa`-style suppressions.
|
||||
|
||||
## Make it permanent (required — this is the point)
|
||||
|
||||
A refactor without a gate silently regrows. Before this task is done:
|
||||
|
||||
1. Wire the shared gate `~/src/utils/scripts/check_file_length.sh` into this repo's
|
||||
`.pre-commit-config.yaml` as a local hook. If the repo has no pre-commit
|
||||
config, add a minimal one.
|
||||
2. The hook checks **files in the commit** (not the whole tree), so unrelated
|
||||
commits never break, and it **fails** — exit 1, not a warning.
|
||||
3. No baseline file and no allowlist. Those are suppressions.
|
||||
4. If this repo has CI (`.github/workflows`), add the same check there so it
|
||||
also fails on push.
|
||||
|
||||
## Done condition
|
||||
|
||||
- `bash ~/src/utils/scripts/check_file_length.sh --all` from this repo root exits 0.
|
||||
- The repo's own test suite and coverage bar are still green.
|
||||
- `pre-commit run --files <changed files>` passes.
|
||||
- A deliberately over-250-line test file, staged, makes `git commit` **fail**.
|
||||
- For a deployed daemon/app: the entry point still actually runs.
|
||||
|
||||
## Verify
|
||||
|
||||
Run the suite, then actually run the locker entry point — this repo deploys from the working tree, so a broken import is a live outage.
|
||||
|
||||
REMOVE ME AFTER FINISH
|
||||
@ -1,178 +0,0 @@
|
||||
# Finish TODO — the last 10 Dart files over 250 lines
|
||||
|
||||
> **This file is a ready-to-use prompt.** Open this repo and say
|
||||
> "do todo_finish_dart_250_prompt". It is self-contained.
|
||||
> Generated 2026-08-15 at the end of a session that took the repo from 41
|
||||
> files over the cap to 10. All 10 remaining are Dart.
|
||||
|
||||
## Goal
|
||||
|
||||
`bash ~/src/utils/scripts/check_file_length.sh --all` from the repo root exits 0.
|
||||
|
||||
Everything else in `refactor_claude_todo.md` still applies (no gaming the cap,
|
||||
no deleting tests, no suppressions). This file only adds what the previous
|
||||
session learned the hard way.
|
||||
|
||||
## Start here (do this first, it costs one command)
|
||||
|
||||
```bash
|
||||
cd ~/src/screen-locker/stronglift_replacement/workout_app
|
||||
env -u GIT_DIR flutter test 2>&1 | tail -1 # MUST print: +402: All tests passed!
|
||||
```
|
||||
|
||||
**402 is the invariant.** Dart has no glob-parametrized test like the Python
|
||||
side's `test_no_popup_widgets`, so the count never legitimately changes. Any
|
||||
other number means a lost, duplicated, or non-compiling test — stop and fix
|
||||
before continuing. A file that fails to *load* shows as
|
||||
`Failing tests: ... loading X` and drops the count, so check the number, not
|
||||
just the absence of red.
|
||||
|
||||
Never use `git stash` (blocked by a hook). To compare against HEAD:
|
||||
`git worktree add --detach /tmp/wt HEAD` … `git worktree remove /tmp/wt`.
|
||||
|
||||
## The 10 files
|
||||
|
||||
| lines | file |
|
||||
| ----: | :--- |
|
||||
| 740 | `test/services/workout_sync_service_test.dart` |
|
||||
| 633 | `lib/screens/workout_screen.dart` |
|
||||
| 503 | `lib/screens/settings_screen.dart` |
|
||||
| 426 | `test/services/progression_sync_service_test.dart` |
|
||||
| 386 | `test/screens/history_screen_test.dart` |
|
||||
| 383 | `test/screens/home_screen_test.dart` |
|
||||
| 362 | `test/screens/workout_screen_test.dart` |
|
||||
| 322 | `lib/services/workout_sync_service.dart` |
|
||||
| 313 | `lib/screens/manual_workout_screen.dart` |
|
||||
| 260 | `lib/screens/home_screen.dart` |
|
||||
|
||||
Re-run `python3 scripts/refresh_refactor_todo.py` for the live set.
|
||||
|
||||
## Part 1 — the 4 test files
|
||||
|
||||
`scripts/split_dart_test.py` works and is committed. Usage:
|
||||
|
||||
```bash
|
||||
python3 scripts/split_dart_test.py <file> <out1>:<boundaryIdx> [<out2>:<idx> ...]
|
||||
```
|
||||
|
||||
Boundary indices are positions in the list of top-level `group(` /
|
||||
`testWidgets(` / `test(` lines. Each output takes `boundaries[idx:next_idx]`;
|
||||
the source keeps everything before the first.
|
||||
|
||||
**Its working envelope — stay inside it:**
|
||||
|
||||
1. **One pass, from the pristine file.** Specify every output up front. Do NOT
|
||||
split an already-split file: indices are recomputed against the new file and
|
||||
the trailer is re-copied, which is how content went missing last time.
|
||||
2. **Zero top-level declarations after `main()`.** Check with:
|
||||
`awk -v m=$(grep -n '^void main(' F | cut -d: -f1) 'NR>m && /^[A-Za-z_]/' F`
|
||||
The splitter copies the trailer, but the detection is unreliable when
|
||||
helpers sit *between* groups.
|
||||
3. **No local function declarations in the preamble.** `history_screen_test.dart`
|
||||
defines `_pump`, `_wrap`, `_seed` as locals inside `main()`; both halves get
|
||||
the copy but the second half's references resolve wrong. This is why that
|
||||
file is still on the list.
|
||||
|
||||
**All 4 remaining files are OUTSIDE the envelope** (that's why they're left):
|
||||
|
||||
- `workout_sync_service_test.dart` — 2 post-`main` declarations
|
||||
(`_manualWorkoutSurvivalTests`, `_cutoverTests`), plus `_fakeFirebase`,
|
||||
`_manual`, `_PutCall` used across the split point.
|
||||
- `home_screen_test.dart` — 4 post-`main` declarations (fake sync classes
|
||||
overriding `syncNow`).
|
||||
- `progression_sync_service_test.dart` — 1 post-`main` declaration.
|
||||
- `history_screen_test.dart` — the local-function preamble above.
|
||||
|
||||
**The fix, and it is its own committed step:** move those shared helpers and
|
||||
fakes into a sibling `test/services/_sync_test_fixtures.dart` (or
|
||||
`test/screens/_home_test_fixtures.dart`) and `import` it — the same shape as
|
||||
`screen_locker/tests/_workout_sync_fixtures.py` on the Python side. Commit
|
||||
that move, verified at 402, *before* splitting. Then the file is inside the
|
||||
envelope and the splitter handles it.
|
||||
|
||||
**One file at a time → verify 402 → commit.** Batching means a wrong count
|
||||
doesn't tell you which file caused it, and the revert tangles.
|
||||
|
||||
## Part 2 — the 6 `lib/` files
|
||||
|
||||
Two patterns are proven and committed; copy them.
|
||||
|
||||
**A. Private widget classes → `part` file.** Used on `history_screen`,
|
||||
`settings_screen`, `home_screen`, `exercise_tile`, `github_mirror_screen`.
|
||||
Dart privacy is library-scoped, so `part` keeps `_Foo` classes private —
|
||||
making them public would trip `public_member_api_docs` and change the API.
|
||||
A `part` also gets no `SF:` entry in lcov, so the coverage gate's
|
||||
"every lib file must appear" guard has nothing new to miss.
|
||||
|
||||
**B. Methods → `extension` in a `part` file.** Used on `storage_service`
|
||||
(5 files), `progression_sync_service` (4), `backup_service`,
|
||||
`workout_sync_service`. Dart can't continue a class body across files, but an
|
||||
extension in a part still reaches `_privateField`.
|
||||
|
||||
### Three hard constraints — each one cost a debugging cycle
|
||||
|
||||
1. **`setState` is `@protected`.** It cannot be called from an extension. Any
|
||||
State method that calls `setState` must stay in the class. This is the main
|
||||
blocker on `home_screen.dart` (260) and `settings_screen.dart` (503).
|
||||
2. **Extension methods dispatch STATICALLY.** A method a test fake overrides
|
||||
must stay in the class. `_FakeSync extends WorkoutSyncService` overrides
|
||||
`pushManual` and `readMergedManualPayloads`; with those in an extension the
|
||||
*real* implementations ran and four manual-workout tests hung on
|
||||
`pumpAndSettle` instead of failing an assertion. Before moving any public
|
||||
method, grep: `grep -rn "extends <ClassName>" -A15 test/`.
|
||||
This is why `workout_sync_service.dart` is stuck at 322 — its public
|
||||
surface is what fakes bind to.
|
||||
3. **Static members need qualifying inside an extension**:
|
||||
`BackupService._baseDir`, `ProgressionSyncService._stateToJson`. The
|
||||
analyzer catches this (`unqualified_reference_to_static_member_of_extended_type`).
|
||||
|
||||
### What's actually left to do in `lib/`
|
||||
|
||||
- `workout_screen.dart` (633) — **not yet attempted.** Check it for private
|
||||
widget classes first (pattern A); that's the cheap win.
|
||||
- `manual_workout_screen.dart` (313) — not yet attempted.
|
||||
- `settings_screen.dart` (503) — widgets already extracted. Going lower needs
|
||||
the ~190-line `build()` broken up: extract the SYNC / OFFLINE BACKUP list
|
||||
sections into private `StatelessWidget`s taking what they read as
|
||||
constructor params (they never call `setState`, so they're eligible). A
|
||||
previous attempt to slice them out as a raw widget-list getter produced
|
||||
unbalanced brackets — build the widget class properly, don't slice text.
|
||||
- `home_screen.dart` (260, only 10 over) — same shape, one cohesive `build()`.
|
||||
- `workout_sync_service.dart` (322) — see constraint 2. Only private helpers
|
||||
can move; getting under 250 means changing the public surface, which is a
|
||||
design decision, not a move. **Ask the user before doing that.**
|
||||
|
||||
## Verify (all of it, every batch)
|
||||
|
||||
```bash
|
||||
cd ~/src/screen-locker/stronglift_replacement/workout_app
|
||||
env -u GIT_DIR dart analyze lib/ # must be "No issues found!"
|
||||
env -u GIT_DIR flutter test 2>&1 | tail -1 # must be +402
|
||||
cd ~/src/screen-locker && pre-commit run # includes the 100% Flutter coverage gate
|
||||
```
|
||||
|
||||
`dart analyze` is a **no-op on test files** — `analysis_options.yaml` excludes
|
||||
`test/**`. For those, the test count and the compile it implies are the only
|
||||
signal.
|
||||
|
||||
## Done condition
|
||||
|
||||
- `bash ~/src/utils/scripts/check_file_length.sh --all` exits 0.
|
||||
- 402 Dart tests pass; `flutter` coverage still 100%.
|
||||
- `pre-commit run --all-files` passes.
|
||||
- The app still runs on the phone: use the `phone-deploy` skill
|
||||
(`bash ~/.claude/scripts/phone_deploy.sh <app-dir> --shot <path>`), then
|
||||
**read the screenshot yourself** and confirm real progression data renders —
|
||||
weights and "Next: Workout A/B", not seeded defaults. That is what proves
|
||||
the `storage_service` part-extensions still resolve.
|
||||
|
||||
## Environment note (not your bug)
|
||||
|
||||
`python3 -m scripts.verify_lock_popup_safety` fails locally with
|
||||
`RecoveryLoop.__init__() takes 3 positional arguments but 6 were given`.
|
||||
The code matches the pinned gatelock 0.4.0; this machine has 0.5.0 installed.
|
||||
Resync with `pip install -r requirements.txt`. **Do not "fix" the code to
|
||||
match the installed version** — that would break CI. See the comment in
|
||||
`scripts/_popup_form_check.py`.
|
||||
|
||||
REMOVE ME AFTER FINISH
|
||||
@ -38,7 +38,6 @@ android {
|
||||
}
|
||||
|
||||
defaultConfig {
|
||||
// TODO: Specify your own unique Application ID (https://developer.android.com/studio/build/application-id.html).
|
||||
applicationId = "com.kuhy.workout_app"
|
||||
// You can update the following values to match your application needs.
|
||||
// For more information, see: https://flutter.dev/to/review-gradle-config.
|
||||
|
||||
@ -125,13 +125,8 @@ static void my_application_activate(GApplication* application) {
|
||||
GtkWindow* window =
|
||||
GTK_WINDOW(gtk_application_window_new(GTK_APPLICATION(application)));
|
||||
|
||||
// Use a header bar when running in GNOME as this is the common style used
|
||||
// by applications and is the setup most users will be using (e.g. Ubuntu
|
||||
// desktop).
|
||||
// If running on X and not using GNOME then just use a traditional title bar
|
||||
// in case the window manager does more exotic layout, e.g. tiling.
|
||||
// If running on Wayland assume the header bar will work (may need changing
|
||||
// if future cases occur).
|
||||
// Header bar only under GNOME (and Wayland); X11 WMs such as i3 tile, so
|
||||
// they get a plain title bar. Lock mode never shows either.
|
||||
gboolean use_header_bar = !lock_mode_enabled;
|
||||
#ifdef GDK_WINDOWING_X11
|
||||
GdkScreen* screen = gtk_window_get_screen(window);
|
||||
@ -211,19 +206,11 @@ static gboolean my_application_local_command_line(GApplication* application,
|
||||
|
||||
// Implements GApplication::startup.
|
||||
static void my_application_startup(GApplication* application) {
|
||||
// MyApplication* self = MY_APPLICATION(object);
|
||||
|
||||
// Perform any actions required at application startup.
|
||||
|
||||
G_APPLICATION_CLASS(my_application_parent_class)->startup(application);
|
||||
}
|
||||
|
||||
// Implements GApplication::shutdown.
|
||||
static void my_application_shutdown(GApplication* application) {
|
||||
// MyApplication* self = MY_APPLICATION(object);
|
||||
|
||||
// Perform any actions required at application shutdown.
|
||||
|
||||
G_APPLICATION_CLASS(my_application_parent_class)->shutdown(application);
|
||||
}
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user