diff options
| author | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-09-17 12:11:42 +0200 |
|---|---|---|
| committer | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-09-17 12:11:42 +0200 |
| commit | bddbd74e4a73e8e32bcf648efd1cac5655f6d0cd (patch) | |
| tree | 187e6a1fb722d9ab97d2076f69f997f5d371e943 /internal/plan | |
| parent | cd7425b81f963a948f0abe7df3f9e58e190c2b78 (diff) | |
| download | krino-bddbd74e4a73e8e32bcf648efd1cac5655f6d0cd.tar.gz krino-bddbd74e4a73e8e32bcf648efd1cac5655f6d0cd.zip | |
comments that explain the code, not how it was written
About 340 comments cited the development process: task and plan
numbers, fix waves, rulings, reviewers, and the author in the third
person with a date. None of that exists outside the work itself, so to
a reader it pointed at nothing. Each one now states the engineering
reason it was standing in front of; where a comment was provenance and
nothing else, it is gone.
References to docs/design.md and docs/gui-design.md by section stay:
both ship with the repository. The design documents lose their
amendment diaries - CHANGELOG.md is that record - and the GUI's says
plainly that the window has gone further than the document.
Only comments changed. Every .go file was parsed and its code printed
with comments stripped, before and after: the two hashes are identical
across all 175 files.
Diffstat (limited to 'internal/plan')
| -rw-r--r-- | internal/plan/chain_test.go | 6 | ||||
| -rw-r--r-- | internal/plan/conflict.go | 47 | ||||
| -rw-r--r-- | internal/plan/placeholder_test.go | 6 | ||||
| -rw-r--r-- | internal/plan/step.go | 2 |
4 files changed, 30 insertions, 31 deletions
diff --git a/internal/plan/chain_test.go b/internal/plan/chain_test.go index 28c208f..0c89987 100644 --- a/internal/plan/chain_test.go +++ b/internal/plan/chain_test.go @@ -118,9 +118,9 @@ func TestBuildRenameWithSlash(t *testing.T) { } } -// TestBuildKeepsSteplessChains is D6: Build returns one Chain per Input even -// when a file's rules contribute no actions, so a caller can tell "matched a -// rule that does nothing" (an exclusion) from "not matched at all". Plan 3's +// TestBuildKeepsSteplessChains: Build returns one Chain per Input even +// when a file's rules contribute no actions, so a caller can tell "matched +// a rule that does nothing" (an exclusion) from "not matched at all". The // "to act on" count and the JSON document's empty steps array both rest on // this. func TestBuildKeepsSteplessChains(t *testing.T) { diff --git a/internal/plan/conflict.go b/internal/plan/conflict.go index c660a8b..7cebc23 100644 --- a/internal/plan/conflict.go +++ b/internal/plan/conflict.go @@ -62,13 +62,13 @@ type claimed map[string]bool // the step must not run) and Displaces (non-empty only for overwrite of a // file that exists on disk). func resolveConflict(kind Kind, policy config.Conflict, src, dst string, d Disk, c claimed) (resolved, skip, displaces string) { - // A1/A2: the file is already where this step would put it, so its own - // existence must not read as a conflict with itself. Without this guard a - // move or rename plans a rename to stem_1 and every later run adds - // another generation; under (on-conflict overwrite) the step records the - // file as its own Displaces, which plan 4 would trash before moving from - // a path that no longer exists. Checked before the policy switch, so - // overwrite never reaches its own branch. + // The file is already where this step would put it, so its own + // existence must not read as a conflict with itself. Without this + // guard a move or rename plans a rename to stem_1 and every later run + // adds another generation; under (on-conflict overwrite) the step + // records the file as its own Displaces, which would then be trashed + // before moving from a path that no longer exists. Checked before the + // policy switch, so overwrite never reaches its own branch. if dst == src { return dst, "already there", "" } @@ -81,9 +81,9 @@ func resolveConflict(kind Kind, policy config.Conflict, src, dst string, d Disk, // through to the ordinary conflict policy below instead of failing // outright. A wrong "different" verdict costs at worst an // unnecessary suffixed copy, never data loss, so resolving the - // conflict anyway is an acceptable trade-off here (D8) - a caller - // that wants the failure itself visible would need it surfaced as - // a chain warning instead. + // conflict anyway is an acceptable trade-off here - a caller that + // wants the failure itself visible would need it surfaced as a + // chain warning instead. if same, err := d.SameContent(src, dst); err == nil && same { return dst, "already there", "" } @@ -98,17 +98,17 @@ func resolveConflict(kind Kind, policy config.Conflict, src, dst string, d Disk, return dst, "target exists", "" case config.ConflictOverwrite: if onDisk && !c[dst] { - // Only a regular file is ever trashed to make room (review M4): - // a directory or link of the same name stays, and so does the + // Only a regular file is ever trashed to make room: a + // directory or link of the same name stays, and so does the // step - skipped, saying why. if !d.Regular(dst) { return dst, "target is not a regular file", "" } - // The existing file is trashed first (plan 4). Only the first - // step to reach this path may displace it: once another step - // in this same plan has already claimed dst, that path will - // hold that step's own output by the time this one runs, so - // displacing it again would destroy it. + // The existing file is trashed first. Only the first step to + // reach this path may displace it: once another step in this + // same plan has already claimed dst, that path will hold that + // step's own output by the time this one runs, so displacing + // it again would destroy it. return dst, "", dst } // Either claimed in-plan only (nothing on disk to displace — an @@ -124,21 +124,20 @@ func resolveConflict(kind Kind, policy config.Conflict, src, dst string, d Disk, resolved, skip := suffixed(dst, d, c) return resolved, skip, "" } - // Every policy has its own branch (review cli F13): a new one must not - // quietly plan as another. + // Every policy has its own branch: a new one must not quietly plan as + // another. panic(fmt.Sprintf("plan: unknown config.Conflict %d", int(policy))) } // maxSuffixAttempts bounds suffixed(): it is unbounded by design and -// terminates on a real filesystem, but C2 - without a cap, a Disk that -// always reports existence (or a directory A1 had been filling before its -// fix) turns planning quadratic instead of failing fast. +// terminates on a real filesystem, but without a cap, a Disk that always +// reports existence (or a directory being filled without the "already +// there" guard above) turns planning quadratic instead of failing fast. const maxSuffixAttempts = 10000 // suffixed finds the first stem_N.ext (N starting at 1) that is free: // neither on disk nor already claimed by an earlier step in this plan. It -// gives up after maxSuffixAttempts, returning a Skip reason and no path -// (C2). +// gives up after maxSuffixAttempts, returning a Skip reason and no path. func suffixed(dst string, d Disk, c claimed) (resolved, skip string) { dir, base := filepath.Split(dst) stem, ext := splitExt(base) diff --git a/internal/plan/placeholder_test.go b/internal/plan/placeholder_test.go index e155430..531a51f 100644 --- a/internal/plan/placeholder_test.go +++ b/internal/plan/placeholder_test.go @@ -30,7 +30,7 @@ func TestExpand(t *testing.T) { {"{mtime:%j}", "227"}, {"{now:%Y-%m-%d}", "2026-09-12"}, {"{1}", "2026"}, - // D2: an expanded value that itself contains "}}" must reach the + // An expanded value that itself contains "}}" must reach the // output unchanged. Expand's single-pass scanner jumps past a // placeholder's closing brace, so written bytes are never re-scanned; // a refactor to scan-then-replace would silently re-collapse them. @@ -77,11 +77,11 @@ func TestExpandErrors(t *testing.T) { {"{name", "unclosed placeholder"}, {"{mtime:%Q}", "unknown time format %Q in {mtime:...}"}, {"{mtime}", "unknown placeholder {mtime}"}, - // B3: the error must name the placeholder actually written ("now"), + // The error must name the placeholder actually written ("now"), // not hardcode "mtime" - the {mtime:%Q} case above passes either // way, which is why that defect survived. {"{now:%Q}", "unknown time format %Q in {now:...}"}, - // D3: {1}...{9} is the syntax (spec §7.3); {10} and up must be + // {1}...{9} is the syntax (spec §7.3); {10} and up must be // rejected the same way an unknown placeholder is. {"{10}", "unknown placeholder {10}"}, } diff --git a/internal/plan/step.go b/internal/plan/step.go index e4f280f..b3e34c1 100644 --- a/internal/plan/step.go +++ b/internal/plan/step.go @@ -20,7 +20,7 @@ const ( DeletePermanent // (delete permanent) ) -// String is for display only; Task 5's JSON representation defines its own +// String is for display only; the JSON representation defines its own // action names. func (k Kind) String() string { switch k { |
