config migrate: detect missing attributes within existing sections #40

New issue

Open

opened 2026-05-08 15:47:57 +02:00 by heiko · 1 comment

heiko commented

2026-05-08 15:47:57 +02:00

Owner

Goal

Extend gogogo config migrate to detect missing attributes within existing sections, not just entirely-missing top-level sections.

Today: a user with formats: but no formats.deb.section gets no migration help — migrate only flags whole-section gaps.
Want: migrate should also surface missing leaves like formats.deb.section, build.test.skip, etc.

Shape of the change

Schema model

Replace []Unit{Name string} (in internal/cmd/migrate/schema.go) with a flat list of typed paths:

type Field struct {
    Path []string  // e.g. ["build", "test", "run"]
}

Source of truth: reflect over internal/config.Config. It already has yaml tags. That makes the Go struct authoritative; default.yml becomes example/comment source only.

Detection (`diff.go`)

Generalize missingUnits → missingFields:

Parse user content to yaml.Node, walk it once to build the set of dotted paths the user has.
For each Field, classify:
- whole — the root segment isn't present at all (today's case)
- partial — root present, but the path isn't (new case)
Idempotency: extend today's # name: line scan to # dotted.path: lines.

Emission (`emit.go`)

Both classes go in the existing trailing # ============ block — append-only, so user formatting in existing sections stays untouched. The two cases differ only in what extractSection looks up:

whole → today's behavior (find #name: in default.yml, emit children)
partial → find the matching nested line (build: → commands:) and emit just that subtree's example, prefixed # build.commands: as the idempotency marker.

Tradeoffs

Append-only stays append-only. Missing nested attrs land at the bottom, not inline next to their section. That's what today does for whole sections. In-place insertion would mean rewriting via yaml.Node, which loses comments and re-flows the file — not worth it.
Reflect-based schema means drift between default.yml and config.Config shows up as "example not found" stubs in migrate output — useful self-test.
Empty values count as present. formats.deb.depends: [] written by the user is "considered" — we don't second-guess explicit choices.

Decisions (resolved during planning)

Renamed/deprecated fields: out of scope. Different feature; separate ticket if ever needed.
Lists: presence-of-key counts as present. No recursion into list elements.
Indentation tracking in extractSection: required for the path-aware variant. Trickiest piece — needs a small dedicated test fixture.

Test plan

missingFields table-test: empty user, partial sections, full config, with and without idempotency markers.
Reflect schema test: snapshot the field list against an expected fixture so additions to config.Config show up as test diffs.
End-to-end Run test with a user config that has formats: but no formats.deb — expect a # formats.deb: block to be appended.
Existing migrate tests must still pass — the whole-section path matches today's behavior.

Files affected

internal/cmd/migrate/schema.go — replace Unit with Field, build via reflection over config.Config.
internal/cmd/migrate/diff.go — missingFields, dotted-path idempotency scan.
internal/cmd/migrate/emit.go — path-aware extractSection (indentation tracking).
internal/cmd/migrate/migrate.go — call sites; public Run/NeedsUpdate/CheckIdempotent API stays the same.
internal/cmd/migrate/migrate_test.go — extend tests; add fixture for nested partial cases.

## Goal Extend `gogogo config migrate` to detect missing **attributes** within existing sections, not just entirely-missing top-level sections. Today: a user with `formats:` but no `formats.deb.section` gets no migration help — `migrate` only flags whole-section gaps. Want: `migrate` should also surface missing leaves like `formats.deb.section`, `build.test.skip`, etc. ## Shape of the change ### Schema model Replace `[]Unit{Name string}` (in `internal/cmd/migrate/schema.go`) with a flat list of typed paths: ```go type Field struct { Path []string // e.g. ["build", "test", "run"] } ``` **Source of truth**: reflect over `internal/config.Config`. It already has yaml tags. That makes the Go struct authoritative; `default.yml` becomes example/comment source only. ### Detection (`diff.go`) Generalize `missingUnits` → `missingFields`: 1. Parse user content to `yaml.Node`, walk it once to build the set of dotted paths the user has. 2. For each `Field`, classify: - **whole** — the root segment isn't present at all (today's case) - **partial** — root present, but the path isn't (new case) 3. Idempotency: extend today's `# name:` line scan to `# dotted.path:` lines. ### Emission (`emit.go`) Both classes go in the existing trailing `# ============` block — append-only, so user formatting in existing sections stays untouched. The two cases differ only in what `extractSection` looks up: - **whole** → today's behavior (find `#name:` in `default.yml`, emit children) - **partial** → find the matching nested line (`build:` → `commands:`) and emit just that subtree's example, prefixed `# build.commands:` as the idempotency marker. ## Tradeoffs - **Append-only stays append-only.** Missing nested attrs land at the bottom, not inline next to their section. That's what today does for whole sections. In-place insertion would mean rewriting via `yaml.Node`, which loses comments and re-flows the file — not worth it. - **Reflect-based schema** means drift between `default.yml` and `config.Config` shows up as "example not found" stubs in migrate output — useful self-test. - **Empty values count as present.** `formats.deb.depends: []` written by the user is "considered" — we don't second-guess explicit choices. ## Decisions (resolved during planning) - **Renamed/deprecated fields**: out of scope. Different feature; separate ticket if ever needed. - **Lists**: presence-of-key counts as present. No recursion into list elements. - **Indentation tracking in `extractSection`**: required for the path-aware variant. Trickiest piece — needs a small dedicated test fixture. ## Test plan - `missingFields` table-test: empty user, partial sections, full config, with and without idempotency markers. - Reflect schema test: snapshot the field list against an expected fixture so additions to `config.Config` show up as test diffs. - End-to-end `Run` test with a user config that has `formats:` but no `formats.deb` — expect a `# formats.deb:` block to be appended. - Existing migrate tests must still pass — the whole-section path matches today's behavior. ## Files affected - `internal/cmd/migrate/schema.go` — replace `Unit` with `Field`, build via reflection over `config.Config`. - `internal/cmd/migrate/diff.go` — `missingFields`, dotted-path idempotency scan. - `internal/cmd/migrate/emit.go` — path-aware `extractSection` (indentation tracking). - `internal/cmd/migrate/migrate.go` — call sites; public `Run`/`NeedsUpdate`/`CheckIdempotent` API stays the same. - `internal/cmd/migrate/migrate_test.go` — extend tests; add fixture for nested partial cases.

heiko added the

Kind/Enhancement

label

2026-05-08 15:47:57 +02:00

heiko referenced this issue from a commit

2026-05-08 23:16:17 +02:00

fix(migrate): detect missing attributes within existing sections

heiko referenced this issue from a commit

2026-05-08 23:16:17 +02:00

test(migrate): add comprehensive test coverage for missingAttributes

heiko referenced this issue

2026-05-08 23:18:57 +02:00