diff options
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 { |