refactor: split source files that exceeded the ~300-line ceiling
Mechanical moves only — operations, store, history_view, and settings_view are now split along their existing seams so every file stays within the 250+20% guideline. Document the new layout in ARCHITECTURE.md and close the ROADMAP item. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
mixeme
+39
-4
@@ -274,8 +274,9 @@ the moment the window opens.
|
||||
|
||||
### `jobs_view.go` file structure
|
||||
|
||||
The size guideline for a file in this project is ~250 lines.
|
||||
`src/ui/jobs_view.go` is split across six files along these seams:
|
||||
The size guideline for a file in this project is ~250 lines; up to 20% over
|
||||
(~300 lines) is acceptable. `src/ui/jobs_view.go` is split across six files
|
||||
along these seams:
|
||||
|
||||
| File | Contents |
|
||||
|------|----------|
|
||||
@@ -299,11 +300,45 @@ list's highlight at the selected job.
|
||||
|
||||
### `settings_view.go` file structure
|
||||
|
||||
`src/ui/settings_view.go` is split across three files the same way, once its
|
||||
`src/ui/settings_view.go` is split across four files the same way, once its
|
||||
own size passed the guideline:
|
||||
|
||||
| File | Contents |
|
||||
|------|----------|
|
||||
| `settings_view.go` | `settingsView` — field construction, save, load, validate; the Theme label translation helpers |
|
||||
| `settings_view.go` | `settingsView` — thin entry point; theme label translation helpers |
|
||||
| `settings_view_form.go` | `buildSettingsForm` — widget construction, save, load, cancel, and defaults handlers |
|
||||
| `settings_view_layout.go` | `newSettingsLayout`, `settingsSection`, `settingsRow` — the two-column arrangement and the button row |
|
||||
| `settings_view_helpers.go` | Pure helpers — `fyneVersion`, `mustParseURL`, `settingsFolderPath`, `openFolder`, `chooseFile`/`chooseJSONFile`, `chooseFolder` (`chooseFile` also backs `job_dialog.go`'s command browser) |
|
||||
|
||||
### `operations.go` file structure
|
||||
|
||||
`src/app/operations.go` is split across four files along the public API,
|
||||
locked helpers, and pure validation seams:
|
||||
|
||||
| File | Contents |
|
||||
|------|----------|
|
||||
| `operations.go` | Job mutators (`CreateJob` … `SetEnabled`); shared constants (`maxJobLogs`, `timestampLayout`, `errJobNotFound`) |
|
||||
| `operations_settings.go` | Config mutators — `SetGlobalPause`, `SetJobListView`, `ShouldNotifyOnFailure`, `UpdateSettings` |
|
||||
| `operations_locked.go` | `*Locked` state helpers (`refreshNextRunLocked` … `nextIDLocked`); `prependLog`, `uiRecord` |
|
||||
| `operations_validate.go` | Pure validators and normalizers — `normalizeJob`, `validateJob`, `hasFileName`, `validateConfig` |
|
||||
|
||||
### `store.go` file structure
|
||||
|
||||
`src/storage/store.go` is split across three files. Path resolution for the
|
||||
executable directory lives in `paths.go`; config-relative path resolution stays
|
||||
with the store API:
|
||||
|
||||
| File | Contents |
|
||||
|------|----------|
|
||||
| `store.go` | `Store` struct, `OpenStore`, `PeekKeepRunningInTray`, save API, `ResolveConfiguredPath`, `applyConfigPaths`, atomic JSON writes |
|
||||
| `store_config.go` | `loadOrCreateConfig` — config load, defaults, and migration shims |
|
||||
| `store_jobs.go` | `LoadJobsFile`, `loadOrCreateJobs`, `normalizeJobs`, sample jobs, platform-specific demo commands |
|
||||
|
||||
### `history_view.go` file structure
|
||||
|
||||
`src/ui/history_view.go` is split across two files:
|
||||
|
||||
| File | Contents |
|
||||
|------|----------|
|
||||
| `history_view_columns.go` | Pure column-width helpers — `textWidth` through `historyColumnWidths`, sample sets, `historyContentValues` |
|
||||
| `history_view.go` | `historyLog`, `historyHeader`, `newHistoryView`, cell text, `newEvent`, `logFileName` |
|
||||
|
||||
+9
-1
@@ -94,7 +94,8 @@ History and overlap queues, and a Jobs selection that follows the job.**
|
||||
`TestQuoteLeadingWindowsProgramPathPicksEarliestBoundedExtension`), the
|
||||
deliberately-uncovered list covers everything the profile reports at 0%, and
|
||||
the coverage figure records how to read the total rather than the per-package
|
||||
lines. `ROADMAP.md` — the over-the-guideline table was re-measured.
|
||||
lines. `ROADMAP.md` — the over-the-guideline table was re-measured; the split
|
||||
is now landed and the open item removed.
|
||||
- README's scheduler wording caught up with the 0.11.2 rename of "Pause all" to
|
||||
**Disable auto**, and its notification description matches what the app sends.
|
||||
- `STANDARDS.md` records the rules the review settled: no file I/O under
|
||||
@@ -132,6 +133,13 @@ History and overlap queues, and a Jobs selection that follows the job.**
|
||||
330-line constructor whose dozen closures shared seven mutable locals is now
|
||||
widgets reading one named state object — which is what made the selection fix
|
||||
above a change in one place instead of five.
|
||||
- Four more source files over the ~300-line ceiling (250 + 20%) were split in
|
||||
one pass: `operations.go` into job mutators, config mutators
|
||||
(`operations_settings.go`), `*Locked` helpers, and pure validators; `store.go` into the store API, config load, and jobs load;
|
||||
`history_view.go` into column-width helpers and the table widget; and
|
||||
`settings_view.go` into a thin entry point plus `settings_view_form.go` for
|
||||
field construction and save/load handlers. `run.go` and `service.go` stay
|
||||
as-is — both are within the ceiling.
|
||||
- `Service.Store()` is replaced by typed `Service.Config()` and `Service.Paths()`
|
||||
accessors that copy under the lock, so the UI no longer reaches into a shared
|
||||
`*storage.Store`. The Jobs pause control is now driven by `refreshView`
|
||||
|
||||
@@ -139,55 +139,6 @@ Design notes / open questions:
|
||||
Service exposes import/export operations; the UI only picks the file and
|
||||
shows the outcome.
|
||||
|
||||
### Split the files that are over the size guideline
|
||||
|
||||
[ARCHITECTURE.md](ARCHITECTURE.md) sets a ~250-line guideline per source file
|
||||
and records the `jobs_view.go` and `settings_view.go` splits as the worked
|
||||
examples. `jobs_view.go` was split again in 1.0.3 — into view, state, list, and
|
||||
toolbar — because the selection defect it carried was a symptom of the size
|
||||
(one 330-line constructor over seven shared locals). Six non-test files are
|
||||
over the guideline:
|
||||
|
||||
| File | Lines |
|
||||
|------|-------|
|
||||
| `src/app/operations.go` | 529 |
|
||||
| `src/storage/store.go` | 382 |
|
||||
| `src/ui/history_view.go` | 355 |
|
||||
| `src/ui/settings_view.go` | 326 |
|
||||
| `src/app/run.go` | 275 |
|
||||
| `src/app/service.go` | 252 |
|
||||
|
||||
The remaining six are deliberately deferred rather than done piecemeal: a
|
||||
split touches every reader of the file, and doing them in one pass keeps the
|
||||
seams consistent instead of settling each one its own way. Splitting is
|
||||
also the kind of change that reads as pure movement while quietly dropping a
|
||||
function, so it wants one careful pass, not a hurried one per file.
|
||||
|
||||
Seams visible today, as a starting point rather than a decision:
|
||||
|
||||
- **`operations.go`** — the worst overage and the clearest split: the public
|
||||
mutating operations (`CreateJob` … `UpdateSettings`), the `…Locked` state
|
||||
helpers that only they call, and the pure validators and normalizers
|
||||
(`normalizeJob`, `validateJob`, `hasFileName`, `validateConfig`) are three
|
||||
distinct jobs already sitting in three consecutive blocks.
|
||||
- **`history_view.go`** — the column-measuring helpers (`textWidth` through
|
||||
`historyColumnWidths`) are pure, already unit-tested, and independent of the
|
||||
table they size.
|
||||
- **`store.go`** — path resolution, the config load/normalize path, and the jobs
|
||||
load/normalize path are three separate concerns in one file.
|
||||
- **`run.go`**, **`settings_view.go`**, **`service.go`** — barely over. Worth
|
||||
re-measuring at the time; if a pass elsewhere has shrunk them, leave them
|
||||
alone rather than splitting for the sake of the number. The counts above move
|
||||
a few lines either way with any edit, so re-measure before acting on them
|
||||
rather than treating the table as current.
|
||||
|
||||
The `jobs_view.go` pass is the worked example for the rest: the constructor was
|
||||
broken up along the state it shared, not along line count, and the split landed
|
||||
with the selection fix rather than promising it separately.
|
||||
|
||||
Scope note: the guideline is about source files. Test files are much larger and
|
||||
that is fine — a table-driven test file grows with the cases it covers.
|
||||
|
||||
### Window size persistence *(frozen)*
|
||||
|
||||
Window size is currently **not** saved on quit or close. Saving was disabled
|
||||
|
||||
Reference in New Issue
Block a user