aboutsummaryrefslogtreecommitdiff
path: root/cmd/krino
diff options
context:
space:
mode:
Diffstat (limited to 'cmd/krino')
-rw-r--r--cmd/krino/commands_test.go15
-rw-r--r--cmd/krino/exclude_test.go4
-rw-r--r--cmd/krino/history_test.go87
-rw-r--r--cmd/krino/log.go10
-rw-r--r--cmd/krino/main.go14
-rw-r--r--cmd/krino/main_test.go12
-rw-r--r--cmd/krino/matching_test.go47
-rw-r--r--cmd/krino/render.go52
-rw-r--r--cmd/krino/render_test.go10
-rw-r--r--cmd/krino/review.go17
-rw-r--r--cmd/krino/review_test.go12
-rw-r--r--cmd/krino/sort.go135
-rw-r--r--cmd/krino/sort_test.go48
-rw-r--r--cmd/krino/undo.go67
14 files changed, 260 insertions, 270 deletions
diff --git a/cmd/krino/commands_test.go b/cmd/krino/commands_test.go
index b8364e6..f681a0f 100644
--- a/cmd/krino/commands_test.go
+++ b/cmd/krino/commands_test.go
@@ -10,12 +10,11 @@ import (
)
// home gives each test its own HOME with no XDG overrides, and points the
-// package's stdin seam at something guaranteed non-terminal (fix round
-// 2026-09-12/item 4): every test that reaches cmdSort's terminal check or
-// the interactive review must not depend on what the ambient test binary's
-// stdin happens to be - if that were ever a real terminal, such a test
-// would silently fall through to the interactive prompt and block on a
-// keypress instead of failing.
+// package's stdin seam at something guaranteed non-terminal: every test
+// that reaches cmdSort's terminal check or the interactive review must not
+// depend on what the ambient test binary's stdin happens to be - if that
+// were ever a real terminal, such a test would silently fall through to
+// the interactive prompt and block on a keypress instead of failing.
func home(t *testing.T) string {
t.Helper()
h := t.TempDir()
@@ -78,8 +77,8 @@ func TestInitNewCheck(t *testing.T) {
}
}
-// TestConfigFlagExpandsTilde is item B: -c=~/... is not expanded by the
-// shell (the ~ comes after =), so krino must expand it itself.
+// TestConfigFlagExpandsTilde: -c=~/... is not expanded by the shell (the ~
+// comes after =), so krino must expand it itself.
func TestConfigFlagExpandsTilde(t *testing.T) {
h := home(t)
cwd := t.TempDir()
diff --git a/cmd/krino/exclude_test.go b/cmd/krino/exclude_test.go
index f76be0c..2c77438 100644
--- a/cmd/krino/exclude_test.go
+++ b/cmd/krino/exclude_test.go
@@ -137,7 +137,7 @@ func TestSkipSummaryCountsTooBig(t *testing.T) {
// TestVerboseListsDirectoriesLeftOutOfTheWalk: a rule's destination inside
// the directory is not walked, so its files never show in any count; -v
-// says so, and only for directories that exist (triage 4).
+// says so, and only for directories that exist.
func TestVerboseListsDirectoriesLeftOutOfTheWalk(t *testing.T) {
h := home(t)
dl := filepath.Join(h, "dl")
@@ -167,7 +167,7 @@ func TestVerboseListsDirectoriesLeftOutOfTheWalk(t *testing.T) {
}
// TestExplainShowsAnUndecidedRule: explain names a rule it could not decide
-// "undecided", not "no" (plan 12).
+// "undecided", not "no".
func TestExplainShowsAnUndecidedRule(t *testing.T) {
h := home(t)
dl := filepath.Join(h, "dl")
diff --git a/cmd/krino/history_test.go b/cmd/krino/history_test.go
index 9c409bc..d85766f 100644
--- a/cmd/krino/history_test.go
+++ b/cmd/krino/history_test.go
@@ -47,8 +47,8 @@ func TestLogListsRunsAndUndoReverses(t *testing.T) {
if _, out, _ = runCLI(t, "log"); !strings.Contains(out, "undone") {
t.Errorf("log does not mark the run undone:\n%s", out)
}
- // Plain undo after an undo continues the run it undid (review M10):
- // everything came back, so nothing is left and nothing moves.
+ // Plain undo after an undo continues the run it undid: everything came
+ // back, so nothing is left and nothing moves.
if code, out, errOut := runCLI(t, "undo", "-y"); code != 0 || !strings.Contains(out, "0 applied") {
t.Errorf("undo after a complete undo: %d\n%s\n%s", code, out, errOut)
}
@@ -75,21 +75,21 @@ func TestUndoDryRunChangesNothing(t *testing.T) {
}
}
-// TestUndoFailsImmediatelyWithHeldLock is fix round 2026-09-12, item 1: an
-// undo moves files just as an apply does, so it needs the same per-directory
-// guard sort's TestSecondRunFailsImmediatelyWithYes already pins for the
-// forward path (spec §11: "a second krino on the same directory ... fails
-// immediately with -y"). The lock file is held under the config NAME "dl",
-// not any filesystem path - UndoFile.Dir is the journal's `dir` column,
-// which is the directory's name from krino.conf, not its root.
+// TestUndoFailsImmediatelyWithHeldLock: an undo moves files just as an
+// apply does, so it needs the same per-directory guard sort's
+// TestSecondRunFailsImmediatelyWithYes already pins for the forward path
+// (spec §11: "a second krino on the same directory ... fails immediately
+// with -y"). The lock file is held under the config NAME "dl", not any
+// filesystem path - UndoFile.Dir is the journal's `dir` column, which is
+// the directory's name from krino.conf, not its root.
//
-// Ruling (fix round 2026-09-12, follow-up): the lock is acquired before the
-// plan is even shown, matching cmdSort's own window (acquired before
-// Plan/review, held across both) rather than only around ApplyUndo - so
-// this also asserts the refusal is noticed before any plan output reaches
-// stdout. A version of this test that only checked the exit code and
-// stderr would pass equally whether the lock were taken early or late, and
-// so would not be pinning the thing this ruling is actually about.
+// The lock is acquired before the plan is even shown, matching cmdSort's
+// own window (acquired before Plan/review, held across both) rather than
+// only around ApplyUndo - so this also asserts the refusal is noticed
+// before any plan output reaches stdout. A version of this test that only
+// checked the exit code and stderr would pass equally whether the lock
+// were taken early or late, and so would not be pinning the thing this
+// test is actually meant to catch.
func TestUndoFailsImmediatelyWithHeldLock(t *testing.T) {
h := matchingFixture(t)
if code, _, errOut := runCLI(t, "-y"); code != 0 {
@@ -131,12 +131,12 @@ func undoFiles(rels ...string) []engine.UndoFile {
return out
}
-// TestFinalizeUndoPlanMarksUnapprovedAsDeclined is the wiring point for fix
-// round 2026-09-12, item 2: a refused file rides through untouched (its own
-// Refused reason is what ApplyUndo checks first), an approved file rides
-// through untouched too, and anything else - explicitly declined, or never
-// reached because [d]/[q] cut a per-file review short - comes out with
-// Declined set rather than being dropped from the plan.
+// TestFinalizeUndoPlanMarksUnapprovedAsDeclined: a refused file rides
+// through untouched (its own Refused reason is what ApplyUndo checks
+// first), an approved file rides through untouched too, and anything else
+// - explicitly declined, or never reached because [d]/[q] cut a per-file
+// review short - comes out with Declined set rather than being dropped
+// from the plan.
func TestFinalizeUndoPlanMarksUnapprovedAsDeclined(t *testing.T) {
up := &engine.UndoPlan{Run: "r1", Files: []engine.UndoFile{
{File: "a"}, // index 0: approved
@@ -239,19 +239,18 @@ func TestReviewUndoRefusedFileNotPrompted(t *testing.T) {
}
}
-// TestPrintUndoPlan is fix wave item 3 (Important), rebuilding fix round
-// 2026-09-12's own golden test: that version hand-built its UndoSteps,
-// including a Dst on the undo-copy step the real code never sets (Dst is
-// deliberately left "" - trash.Put only chooses the entry name at execution
-// time), so it was structurally incapable of catching the bug it was meant
-// to guard against - an undo-copy row rendering as a bare
-// "undo-copy → " with nothing said about what it would do to the
+// TestPrintUndoPlan rebuilds an earlier golden test that hand-built its
+// UndoSteps, including a Dst on the undo-copy step the real code never
+// sets (Dst is deliberately left "" - trash.Put only chooses the entry
+// name at execution time), so it was structurally incapable of catching
+// the bug it was meant to guard against - an undo-copy row rendering as a
+// bare "undo-copy → " with nothing said about what it would do to the
// user's backup copy, the single most destructive step an undo plan takes.
-// The same lesson as Task 9's review: a hand-assembled fixture hides a test
-// that cannot detect a broken copy-undo. This version runs a REAL forward
-// apply (copy then move, so mkdir, copy and move all appear in one file's
-// own chain) and a REAL PlanUndo, editing one file's result afterward so
-// the plan also carries a genuinely refused row, then renders that.
+// A hand-assembled fixture hides a test that cannot detect a broken
+// copy-undo. This version runs a REAL forward apply (copy then move, so
+// mkdir, copy and move all appear in one file's own chain) and a REAL
+// PlanUndo, editing one file's result afterward so the plan also carries a
+// genuinely refused row, then renders that.
func TestPrintUndoPlan(t *testing.T) {
h := home(t)
dl := filepath.Join(h, "dl")
@@ -315,9 +314,9 @@ func TestPrintUndoPlan(t *testing.T) {
"undo-move → ~/dl/inv1.pdf\n",
"undo-copy ~/backup/inv1.pdf → trash\n",
"undo-mkdir ~/backup\n",
- // Minor 4 / fix wave item 5: the refusal reason must be abbreviated
- // against $HOME exactly like every step cell above it, not printed
- // as a raw absolute path.
+ // The refusal reason must be abbreviated against $HOME exactly
+ // like every step cell above it, not printed as a raw absolute
+ // path.
"refused: ~/dl/Work/notes.pdf changed since the run\n",
} {
if !strings.Contains(out, want) {
@@ -412,8 +411,8 @@ func TestGlobalYesBeforeUndo(t *testing.T) {
}
// TestUndoWithoutRunContinuesTheLastUndo: when the most recent run is an
-// undo that could not finish, plain `krino undo` offers what that undo left
-// instead of refusing because the last run is an undo (review M10).
+// undo that could not finish, plain `krino undo` offers what that undo
+// left instead of refusing because the last run is an undo.
func TestUndoWithoutRunContinuesTheLastUndo(t *testing.T) {
h := home(t)
dl := filepath.Join(h, "dl")
@@ -471,9 +470,9 @@ func TestUndoWithoutRunContinuesTheLastUndo(t *testing.T) {
}
// TestReviewUndoMatchesReview: undo's per-file review behaves as review's
-// does (review cli F2): n is recorded, each choice is echoed in red, and w
-// leaves the files it never reached out of the plan, counted as not
-// reviewed rather than logged as declined.
+// does: n is recorded, each choice is echoed in red, and w leaves the
+// files it never reached out of the plan, counted as not reviewed rather
+// than logged as declined.
func TestReviewUndoMatchesReview(t *testing.T) {
out := new(strings.Builder)
files := undoFiles("a", "b", "c")
@@ -501,7 +500,7 @@ func TestReviewUndoMatchesReview(t *testing.T) {
// TestMinAgeRejectedOutsideSortAndExplain: --min-age only changes sorting
// and explain; any other command refuses it rather than silently ignoring
-// a mistyped value, and an empty value is an error (review cli F4).
+// a mistyped value, and an empty value is an error.
func TestMinAgeRejectedOutsideSortAndExplain(t *testing.T) {
matchingFixture(t)
for _, args := range [][]string{
@@ -524,7 +523,7 @@ func TestMinAgeRejectedOutsideSortAndExplain(t *testing.T) {
// TestIgnoredGlobalFlagsAreRefused: a global flag a command does not use is
// refused instead of silently ignored, so "krino -n new ..." or "krino -n
-// init" - meant as a preview - cannot write config (re-review cli F4).
+// init" - meant as a preview - cannot write config.
func TestIgnoredGlobalFlagsAreRefused(t *testing.T) {
h := home(t)
if code, _, errOut := runCLI(t, "-n", "init"); code != 2 || !strings.Contains(errOut, "-n") {
diff --git a/cmd/krino/log.go b/cmd/krino/log.go
index bfe61a2..e3e9dd1 100644
--- a/cmd/krino/log.go
+++ b/cmd/krino/log.go
@@ -74,8 +74,8 @@ func cmdLog(g *globals, args []string, stdout, stderr io.Writer) int {
if err != nil {
// journal.Runs (via Engine.Runs) fails closed on a genuine read
// error - permissions, a corrupt file - and that must stay
- // distinguishable from the ordinary "no log yet" case below rather
- // than collapsing into the same message (dispatch notes).
+ // distinguishable from the ordinary "no log yet" case below
+ // rather than collapsing into the same message.
if errors.Is(err, os.ErrNotExist) {
fmt.Fprintln(stdout, "nothing logged yet")
return 0
@@ -84,9 +84,9 @@ func cmdLog(g *globals, args []string, stdout, stderr io.Writer) int {
return 1
}
// A log file can exist and still hold no runs (a real run with nothing
- // actionable still opens the journal - dispatch notes). That is just as
- // ordinary as no log file at all: same message, same exit 0, and no
- // table header is printed over zero rows.
+ // actionable still opens the journal). That is just as ordinary as no
+ // log file at all: same message, same exit 0, and no table header is
+ // printed over zero rows.
if len(runs) == 0 {
fmt.Fprintln(stdout, "nothing logged yet")
return 0
diff --git a/cmd/krino/main.go b/cmd/krino/main.go
index 973711e..5c504c8 100644
--- a/cmd/krino/main.go
+++ b/cmd/krino/main.go
@@ -101,13 +101,13 @@ func run(args []string, stdout, stderr io.Writer) int {
return cmd(g, rest[1:], stdout, stderr)
}
}
- // Ruling 2026-09-12/4: Go's flag package stops parsing at the first
- // non-flag argument, so "krino dl -n" leaves "-n" in rest as a second
- // directory name instead of a flag - the dry run is silently never
- // honoured. A leftover argument that still looks like a flag is a
- // usage error rather than a guess; there is deliberately no second
- // pass that re-parses trailing flags, which would make "krino --
- // -weird-dir" ambiguous between the two syntaxes.
+ // Go's flag package stops parsing at the first non-flag argument, so
+ // "krino dl -n" leaves "-n" in rest as a second directory name
+ // instead of a flag - the dry run is silently never honoured. A
+ // leftover argument that still looks like a flag is a usage error
+ // rather than a guess; there is deliberately no second pass that
+ // re-parses trailing flags, which would make "krino -- -weird-dir"
+ // ambiguous between the two syntaxes.
for _, a := range rest {
if strings.HasPrefix(a, "-") {
fmt.Fprintf(stderr, "krino: %s: flags must come before directory names\n", a)
diff --git a/cmd/krino/main_test.go b/cmd/krino/main_test.go
index 6e4df44..6d1d30e 100644
--- a/cmd/krino/main_test.go
+++ b/cmd/krino/main_test.go
@@ -41,8 +41,8 @@ func TestBadFlag(t *testing.T) {
}
}
-// TestCommandsAreReserved is item E: every subcommand name must also be a
-// reserved directory name, so a directory can never shadow a command.
+// TestCommandsAreReserved: every subcommand name must also be a reserved
+// directory name, so a directory can never shadow a command.
func TestCommandsAreReserved(t *testing.T) {
for name := range commands {
if !config.Reserved[name] {
@@ -51,10 +51,10 @@ func TestCommandsAreReserved(t *testing.T) {
}
}
-// TestSortNoConfig: apply is implemented as of Task 7, so running with no
-// config at all now fails the same way every other command does — "not
-// found" from engine.Load, not the old "not implemented yet" hard stop this
-// test used to pin (removed as part of Task 7; see cmd/krino/sort.go).
+// TestSortNoConfig: running with no config at all fails the same way every
+// other command does — "not found" from engine.Load, not the old "not
+// implemented yet" hard stop this test used to pin before apply was
+// implemented (see cmd/krino/sort.go).
func TestSortNoConfig(t *testing.T) {
home(t)
code, _, errOut := runCLI(t)
diff --git a/cmd/krino/matching_test.go b/cmd/krino/matching_test.go
index 754e577..4ce2205 100644
--- a/cmd/krino/matching_test.go
+++ b/cmd/krino/matching_test.go
@@ -180,10 +180,10 @@ func TestCheckListsExtractors(t *testing.T) {
}
}
-// TestDryRunWarningsSortedByRel is controller ruling 2026-09-12: the
-// warnings section is one Rel-sorted list across matched and unmatched
-// files, not matched files followed by unmatched files - a reader scans it
-// by name and has no way to see which group a file fell into. "cover.pdf"
+// TestDryRunWarningsSortedByRel: the warnings section is one Rel-sorted
+// list across matched and unmatched files, not matched files followed by
+// unmatched files - a reader scans it by name and has no way to see which
+// group a file fell into. "cover.pdf"
// matches "pdfs" but still carries the warning "acme" recorded before it
// gave up; "brochure.doc" never matches at all. Their Rel order
// ("brochure.doc" < "cover.pdf") is the reverse of matched-then-unmatched
@@ -230,7 +230,7 @@ func TestDryRunWarningsSortedByRel(t *testing.T) {
}
}
-// TestDirectoryWarningAfterHeader: C3. A directory-level warning must be
+// TestDirectoryWarningAfterHeader: a directory-level warning must be
// emitted after its own header line, not before it, so on a terminal (both
// streams sharing one tty, hence stdout and stderr driven into the same
// buffer here to observe their relative order) it reads as describing the
@@ -272,8 +272,8 @@ func TestDirectoryWarningAfterHeader(t *testing.T) {
}
}
-// TestSortSkipsMissingRootAndContinues: C5, the exit-1 skip path. A
-// directory whose root has vanished since it was configured is skipped
+// TestSortSkipsMissingRootAndContinues: the exit-1 skip path. A directory
+// whose root has vanished since it was configured is skipped
// with one line on stderr naming it, but every other directory is still
// processed, with a blank line still separating their two outputs, and
// the run as a whole exits 1.
@@ -311,8 +311,8 @@ func TestSortSkipsMissingRootAndContinues(t *testing.T) {
}
}
-// TestDirectoryWarningNotCountedInWarningsField: C5. A directory-level
-// warning is printed as "krino: NAME: <warning>" on stderr, but is not one
+// TestDirectoryWarningNotCountedInWarningsField: a directory-level warning
+// is printed as "krino: NAME: <warning>" on stderr, but is not one
// of the per-file warnings the "N warnings" field in the summary line
// counts.
func TestDirectoryWarningNotCountedInWarningsField(t *testing.T) {
@@ -352,14 +352,14 @@ func TestDirectoryWarningNotCountedInWarningsField(t *testing.T) {
}
}
-// TestLongNameNotPaddedLayoutIntact: C5, the 40-character cap. A file name
+// TestLongNameNotPaddedLayoutIntact: the 40-character cap. A file name
// longer than the 40-character column cap is left unpadded (not truncated,
// not stretched further), while a short name alongside it is still padded
// out to the full 40-column cap — the layout stays a clean two-column grid
// even though one row's first cell overruns it.
-// TestApplyWithYesMovesFiles is brief 7's basic apply-path test: -y applies
-// the plan with no prompt, the file actually moves, the outcome line says
-// so, and the run is logged with both boundaries.
+// TestApplyWithYesMovesFiles is the basic apply-path test: -y applies the
+// plan with no prompt, the file actually moves, the outcome line says so,
+// and the run is logged with both boundaries.
func TestApplyWithYesMovesFiles(t *testing.T) {
h := matchingFixture(t)
code, out, errOut := runCLI(t, "-y")
@@ -382,9 +382,9 @@ func TestApplyWithYesMovesFiles(t *testing.T) {
}
}
-// TestDryRunLogsNothing is Ruling 2: journal.Open must never be called at
-// all in dry-run mode, since it materialises both the state directory and
-// an empty log file as a side effect of merely opening it.
+// TestDryRunLogsNothing: journal.Open must never be called at all in
+// dry-run mode, since it materialises both the state directory and an
+// empty log file as a side effect of merely opening it.
func TestDryRunLogsNothing(t *testing.T) {
h := matchingFixture(t)
if code, _, errOut := runCLI(t, "-n"); code != 0 {
@@ -425,11 +425,10 @@ func TestSecondRunFailsImmediatelyWithYes(t *testing.T) {
}
}
-// TestFlagsMustPrecedeDirectoryNames is Ruling 4: Go's flag package stops
-// parsing at the first non-flag argument, so "krino dl -n" would otherwise
-// silently take "-n" as a second directory name and never honour the dry
-// run. A leftover argument starting with "-" is a usage error instead of a
-// guess.
+// TestFlagsMustPrecedeDirectoryNames: Go's flag package stops parsing at
+// the first non-flag argument, so "krino dl -n" would otherwise silently
+// take "-n" as a second directory name and never honour the dry run. A
+// leftover argument starting with "-" is a usage error instead of a guess.
func TestFlagsMustPrecedeDirectoryNames(t *testing.T) {
home(t)
code, _, errOut := runCLI(t, "dl", "-n")
@@ -438,8 +437,8 @@ func TestFlagsMustPrecedeDirectoryNames(t *testing.T) {
}
}
-// TestNoColourEscapeToNonTerminal is Ruling 7's one pinned guarantee: a plan
-// piped to a file or read by another tool must be plain text. tui.Colour(w)
+// TestNoColourEscapeToNonTerminal pins one guarantee: a plan piped to a
+// file or read by another tool must be plain text. tui.Colour(w)
// already returns false for anything that is not a terminal *os.File, and
// runCLI's stdout is a bytes.Buffer, so this holds end to end through the
// real command path, not just at the helper that decides it.
@@ -502,7 +501,7 @@ func TestLongNameGetsItsOwnLine(t *testing.T) {
// TestDryRunJSONCarriesExclusionsAndWarnings: the JSON plan says which
// exclude set a file aside and carries the warnings matching raised, for
-// matched and unmatched files alike, as the text plan does (triage 28h).
+// matched and unmatched files alike, as the text plan does.
func TestDryRunJSONCarriesExclusionsAndWarnings(t *testing.T) {
h := home(t)
dl := filepath.Join(h, "dl")
diff --git a/cmd/krino/render.go b/cmd/krino/render.go
index 5b75df3..ca7dad7 100644
--- a/cmd/krino/render.go
+++ b/cmd/krino/render.go
@@ -35,17 +35,17 @@ const minWrap = 10
// never wraps, which keeps a plan piped to a file one field per line.
func printPlan(w io.Writer, dp *engine.DirPlan, verbose bool, p palette, width int) {
r := dp.Result
- // C1 (plan 2): scanned counts matched, unmatched and skipped alike, not
- // just matched plus unmatched - spec §8.2's worked example is "266
- // scanned" against "41 to act on" and "not acted on: 3 busy · 12
- // ignored · 210 unmatched", and 41+3+12+210 = 266.
+ // scanned counts matched, unmatched and skipped alike, not just matched
+ // plus unmatched - spec §8.2's worked example is "266 scanned" against
+ // "41 to act on" and "not acted on: 3 busy · 12 ignored · 210
+ // unmatched", and 41+3+12+210 = 266.
scanned := len(r.Matched) + len(r.Unmatched) + len(r.Skipped)
- // B2: warning lines come from both the match itself (fm.Warnings) and
- // the chains plan.Build produced (Chain.Warnings, e.g. "moved more than
+ // warning lines come from both the match itself (fm.Warnings) and the
+ // chains plan.Build produced (Chain.Warnings, e.g. "moved more than
// once") - both computed once here so the count and the section below
// agree on the exact same list.
lines := collectWarnings(r, dp.Chains)
- // D12: dp.Elapsed spans Match plus Build, unlike r.Elapsed, which stops
+ // dp.Elapsed spans Match plus Build, unlike r.Elapsed, which stops
// before Build ever runs - the label says "planning", so the number
// must cover all of it.
counts := fmt.Sprintf("%d scanned · %d to act on · %d warnings · %.2fs", scanned, countActing(dp.Chains), warnedCount(lines), dp.Elapsed.Seconds())
@@ -131,14 +131,14 @@ func excludedLines(r *engine.Result, chains []plan.Chain) []string {
// chainActing reports whether c has at least one step that will actually
// run - the single definition of "actionable" that countActing,
-// actionableChains (sort.go) and chainOutcomes (sort.go) all share (fix
-// wave item 4 / Minor 5). Before this fix, countActing and actionableChains
-// each kept their own copy of this question and disagreed: countActing
-// excluded an all-skipped chain (len(Steps) > 0, but every step's Skip is
-// set) while actionableChains's own len(Steps) > 0 check included it, so a
-// directory could print "N scanned · 0 to act on" and then still ask the
-// user to approve a file it had just said there were none of - and on
-// approval, log a run-start/run-end pair holding only "skipped" entries.
+// actionableChains (sort.go) and chainOutcomes (sort.go) all share.
+// countActing and actionableChains used to each keep their own copy of
+// this question and disagree: countActing excluded an all-skipped chain
+// (len(Steps) > 0, but every step's Skip is set) while actionableChains's
+// own len(Steps) > 0 check included it, so a directory could print "N
+// scanned · 0 to act on" and then still ask the user to approve a file it
+// had just said there were none of - and on approval, log a
+// run-start/run-end pair holding only "skipped" entries.
func chainActing(c plan.Chain) bool {
for _, s := range c.Steps {
if s.Skip == "" {
@@ -149,9 +149,9 @@ func chainActing(c plan.Chain) bool {
}
// countActing reports how many chains have at least one step that will
-// actually run. C1/ruling 2026-09-12: a rule with no actions is an
-// exclusion, and a chain every one of whose steps is skipped is not about
-// to do anything either - neither must inflate "to act on".
+// actually run. A rule with no actions is an exclusion, and a chain every
+// one of whose steps is skipped is not about to do anything either -
+// neither must inflate "to act on".
func countActing(chains []plan.Chain) int {
n := 0
for _, c := range chains {
@@ -186,7 +186,7 @@ func printBlocks(w io.Writer, chains []plan.Chain, root string, p palette, width
fmt.Fprintln(w)
head := " " + padLeft(strconv.Itoa(i), numW) + " "
// A long name continues at the value column, never at the label
- // column, so its text cannot pass for a step line (triage 28l).
+ // column, so its text cannot pass for a step line.
for _, l := range wrapped(head, display(c.File.Rel), indent+labelWidth+1, width, plainText) {
fmt.Fprintln(w, l)
}
@@ -328,8 +328,8 @@ func wrapText(s string, max int) []string {
// character (CJK), none for a combining mark, one for the rest - format
// characters included: a terminal may draw one (a soft hyphen), and
// counting a column too many only wraps early, while one too few runs past
-// the edge (triage 28j, plan 11 review L4). A terminal can still draw some
-// characters wider (emoji, ambiguous-width letters); see KNOWN LIMITATIONS.
+// the edge. A terminal can still draw some characters wider (emoji,
+// ambiguous-width letters); see KNOWN LIMITATIONS.
func cols(s string) int {
n := 0
for _, r := range s {
@@ -362,9 +362,9 @@ func destText(s plan.Step, root string) string {
dir := filepath.Dir(s.Dst)
if rel, ok := relToRoot(root, dir); ok {
if rel == "" {
- // D10: rel is "" exactly when dir is root itself (relToRoot's
- // own case below); rendering that as bare rel+"/" would print
- // "/", which reads as the filesystem root rather than "this
+ // rel is "" exactly when dir is root itself (relToRoot's own
+ // case below); rendering that as bare rel+"/" would print "/",
+ // which reads as the filesystem root rather than "this
// directory".
return "./"
}
@@ -376,7 +376,7 @@ func destText(s plan.Step, root string) string {
// relToRoot returns dir relative to root (slash-separated) when dir is
// root itself or lies inside it; ok is false when dir lies outside root,
// including when the two cannot be related at all (e.g. one relative, one
-// absolute). C3: root itself counts as "inside" here (rel is "", ok true) -
+// absolute). root itself counts as "inside" here (rel is "", ok true) -
// unlike internal/engine/match.go's excludeDirs, which asks a different
// question (what may a rule exclude from the walk) and treats root as
// outside it; do not "unify" the two.
@@ -405,7 +405,7 @@ func padLeft(s string, w int) string {
// colWidth returns the widest string in ss, in runes, capped at max when
// max is positive; 0 leaves it uncapped. Shares relWidth's rune-counting
-// rule (C4): a name carrying diacritics must not misalign its column.
+// rule: a name carrying diacritics must not misalign its column.
func colWidth(ss []string, max int) int {
w := 0
for _, s := range ss {
diff --git a/cmd/krino/render_test.go b/cmd/krino/render_test.go
index 1b2f8cc..7c2072e 100644
--- a/cmd/krino/render_test.go
+++ b/cmd/krino/render_test.go
@@ -31,7 +31,7 @@ import (
//
// "excluded.txt" matched a rule with no actions (an exclusion, spec §4.5):
// it has zero steps, so it must not appear in the table and must not
-// inflate "to act on" (ruling 2026-09-12).
+// inflate "to act on".
func TestPrintPlan(t *testing.T) {
h := home(t)
root := filepath.Join(h, "downloads")
@@ -78,7 +78,7 @@ func TestPrintPlan(t *testing.T) {
dp := &engine.DirPlan{
Dir: &engine.Dir{Name: "downloads", Root: root},
Chains: []plan.Chain{scan001, fv123, setup, excluded},
- Elapsed: 420 * time.Millisecond, // D12: the counts line renders DirPlan.Elapsed (Match plus Build), not Result.Elapsed alone
+ Elapsed: 420 * time.Millisecond, // the counts line renders DirPlan.Elapsed (Match plus Build), not Result.Elapsed alone
Result: &engine.Result{
Matched: []engine.FileMatch{
{File: scan.File{Rel: "scan001.pdf"}, Warnings: []string{"acme: content unreadable: needs pdftotext, not installed"}},
@@ -380,7 +380,7 @@ func TestSkipSummaryLineAccountsForEveryFile(t *testing.T) {
// TestWrappedNameCannotFakeAStepLine: a long name's continuation lines start
// at the value column, not at the column step labels use, so a name holding
-// "rename → x" cannot pass for a step of its own block (triage 28l).
+// "rename → x" cannot pass for a step of its own block.
func TestWrappedNameCannotFakeAStepLine(t *testing.T) {
home(t)
name := strings.Repeat("a", 30) + " rename → evil.pdf"
@@ -401,7 +401,7 @@ func TestWrappedNameCannotFakeAStepLine(t *testing.T) {
// TestColumnsCountWideAndCombiningCharacters: widths and wrapping count
// terminal columns - two for a CJK character, none for a combining mark -
// so a name in a wide script neither overruns the terminal nor misaligns
-// its column (triage 28j).
+// its column.
func TestColumnsCountWideAndCombiningCharacters(t *testing.T) {
if n := cols("漢字"); n != 4 {
t.Errorf("cols(漢字) = %d, want 4", n)
@@ -411,7 +411,7 @@ func TestColumnsCountWideAndCombiningCharacters(t *testing.T) {
}
// A format character is counted as one column: a terminal may draw it
// (a soft hyphen), and counting one too many only wraps a line early,
- // while one too few lets it run past the edge (plan 11 review L4).
+ // while one too few lets it run past the edge.
if n := cols("a\u00adb"); n != 3 {
t.Errorf("cols(a + soft hyphen + b) = %d, want 3", n)
}
diff --git a/cmd/krino/review.go b/cmd/krino/review.go
index 8253405..671cbab 100644
--- a/cmd/krino/review.go
+++ b/cmd/krino/review.go
@@ -58,9 +58,9 @@ func (k keyReader) Read(p []byte) (int, error) {
// decided and stops. approved holds every file decided, true for yes and
// false for no; a file [w] left unreviewed is absent. replaced holds the
// files [t] or [d] chose to trash or delete instead of what the rules
-// planned; replaceChains applies it. Enter is ignored at both prompts. root is the directory being reviewed - passed only to
-// reviewPerFile's destination rendering (review finding 1, fix round
-// 2026-09-12); nothing here uses it directly.
+// planned; replaceChains applies it. Enter is ignored at both prompts. root
+// is the directory being reviewed - passed only to reviewPerFile's
+// destination rendering; nothing here uses it directly.
func reviewChains(in io.Reader, out io.Writer, chains []plan.Chain, root string, p palette) (map[string]bool, map[string]plan.Kind, rune, error) {
fmt.Fprintln(out)
for _, l := range wrapped("", "[a] apply all [c] choose per file [s] skip this directory [q] quit", 0, widthPolicy(out), p.keys) {
@@ -108,10 +108,9 @@ func reviewChains(in io.Reader, out io.Writer, chains []plan.Chain, root string,
// other key asks about the same file again); every choice is echoed in red
// on its own line; [w] stops asking and applies whatever was already chosen,
// leaving the rest unreviewed - reported back as end 'w'; [q] aborts the
-// review entirely, discarding even files already marked yes - end 'q'. root is
-// passed down so a destination inside root renders root-relative and one
-// outside it renders ~-abbreviated, exactly as in the plan (review finding
-// 1, fix round 2026-09-12).
+// review entirely, discarding even files already marked yes - end 'q'. root
+// is passed down so a destination inside root renders root-relative and one
+// outside it renders ~-abbreviated, exactly as in the plan.
func reviewPerFile(in io.Reader, out io.Writer, chains []plan.Chain, root string, p palette) (approved map[string]bool, replaced map[string]plan.Kind, end rune, err error) {
approved = map[string]bool{}
replaced = map[string]plan.Kind{}
@@ -122,8 +121,8 @@ func reviewPerFile(in io.Reader, out io.Writer, chains []plan.Chain, root string
continue
}
- // The heading wraps with its continuation past the label column, so
- // a long name cannot pass for a step line (plan 11 review L4).
+ // The heading wraps with its continuation past the label column,
+ // so a long name cannot pass for a step line.
fmt.Fprintln(out)
for _, l := range wrapped("", fmt.Sprintf("[%d/%d] %s", i+1, len(chains), display(c.File.Rel)), 7+labelWidth+1, widthPolicy(out), plainText) {
fmt.Fprintln(out, l)
diff --git a/cmd/krino/review_test.go b/cmd/krino/review_test.go
index fc175cb..0731ba9 100644
--- a/cmd/krino/review_test.go
+++ b/cmd/krino/review_test.go
@@ -111,11 +111,11 @@ func TestInvalidKeyReprompts(t *testing.T) {
}
}
-// TestPerFileDestinationIsRootRelative is review finding 1 (fix round
-// 2026-09-12): reviewPerFile must render a destination the same way the
-// directory-level table does (render.go's destText) - root-relative for a
-// destination inside root, ~-abbreviated for one outside it - not always
-// abbreviated because root was never passed through to actionCell at all.
+// TestPerFileDestinationIsRootRelative: reviewPerFile must render a
+// destination the same way the directory-level table does (render.go's
+// destText) - root-relative for a destination inside root, ~-abbreviated
+// for one outside it - not always abbreviated because root was never
+// passed through to actionCell at all.
// Both halves are pinned: getting only the "inside" half right would still
// let an outside-root destination silently regress to some other form.
func TestPerFileDestinationIsRootRelative(t *testing.T) {
@@ -330,7 +330,7 @@ func TestStopAfterApply(t *testing.T) {
// TestReviewHeadingsCannotFakeAStepLine: a long name in the per-file
// heading of review and of undo wraps with its continuation past the column
-// step labels use, so the name cannot pass for a step (plan 11 review L4).
+// step labels use, so the name cannot pass for a step.
func TestReviewHeadingsCannotFakeAStepLine(t *testing.T) {
old := widthPolicy
t.Cleanup(func() { widthPolicy = old })
diff --git a/cmd/krino/sort.go b/cmd/krino/sort.go
index c2038fd..12812c7 100644
--- a/cmd/krino/sort.go
+++ b/cmd/krino/sort.go
@@ -26,13 +26,12 @@ import (
// stdin is os.Stdin, threaded through this seam rather than referenced
// directly: cmdSort's terminal check, installSignalHandler and reviewDir
// all read it, and a test must never depend on what the ambient test
-// binary's stdin happens to be (fix round 2026-09-12/item 4). If it were
-// ever a real terminal, code that only worked by assuming otherwise would
-// fall through to the interactive prompt and block the test suite on a
-// keypress - the same kind of hang the lock-cancellation test was built to
-// never risk. Tests point this at something guaranteed non-terminal
-// (commands_test.go's home helper) instead of relying on a claim about
-// what go test does with stdin.
+// binary's stdin happens to be. If it were ever a real terminal, code that
+// only worked by assuming otherwise would fall through to the interactive
+// prompt and block the test suite on a keypress - the same kind of hang
+// the lock-cancellation test was built to never risk. Tests point this at
+// something guaranteed non-terminal (commands_test.go's home helper)
+// instead of relying on a claim about what go test does with stdin.
var stdin = os.Stdin
// stdinIsTerminal is the check that review can prompt at all, a seam so a
@@ -46,11 +45,11 @@ var stdinIsTerminal = func() bool { return term.IsTerminal(int(stdin.Fd())) }
// (and cannot drift) across the three places it applies.
const zeroOutcome = "0 applied · 0 failed · 0 declined"
-// cmdSort plans and, from Task 7, applies the included directories: flags
-// are checked before any config is read, a bad config stops the whole run
-// before scanning (spec §11), and one engine.Session - its log, run id and
-// claims - covers every directory in the run. See docs/design.md
-// §8.2-§8.4 and §11 for the flow this follows.
+// cmdSort plans and applies the included directories: flags are checked
+// before any config is read, a bad config stops the whole run before
+// scanning (spec §11), and one engine.Session - its log, run id and claims
+// - covers every directory in the run. See docs/design.md §8.2-§8.4 and
+// §11 for the flow this follows.
func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
if g.yes && g.dry {
return usageError(stderr, "-y and -n cannot be used together")
@@ -80,9 +79,9 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
return usageError(stderr, "refusing to prompt: stdin is not a terminal (use -y or -n)")
}
- // Ruling 5: internal/tui deliberately does not trap signals - a package
- // that installs process-wide handlers as a side effect of reading one
- // key would surprise every caller. It lands here because cmdSort must
+ // internal/tui deliberately does not trap signals - a package that
+ // installs process-wide handlers as a side effect of reading one key
+ // would surprise every caller. It lands here because cmdSort must
// install one anyway: spec §11 says Ctrl-C finishes the current step,
// logs it, and stops, which means the ctx passed to Apply below must be
// cancelled on SIGINT. The same handler also restores the terminal on
@@ -96,8 +95,8 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
// The run itself - the log, its run id and the claims every directory
// shares - belongs to the engine, so the GUI runs a directory exactly
- // as this does (GUI design §1.3). Ruling 2: a dry session opens no log,
- // since journal.Open creates the state directory and an empty krino.log
+ // as this does (GUI design §1.3). A dry session opens no log, since
+ // journal.Open creates the state directory and an empty krino.log
// merely by being called.
sess, err := e.NewSession(g.dry)
if err != nil {
@@ -125,9 +124,8 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
// Spec §3/§11: a second krino on the same directory waits for the
// lock, or fails immediately with -y, so a cron job never piles up
// behind a stuck run. lock.Acquire takes ctx precisely so that wait
- // is not unbounded in practice (fix round 2026-09-12/item 1): a
- // signal cancels it and Acquire returns ctx.Err() promptly instead
- // of polling forever.
+ // is not unbounded in practice: a signal cancels it and Acquire
+ // returns ctx.Err() promptly instead of polling forever.
l, err := sess.Lock(ctx, d, !g.yes)
if err != nil {
if interrupted(err) {
@@ -176,15 +174,15 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
}
if g.json {
// --json is only ever reached with -n (checked above), and
- // Ruling 7 is explicit that JSON must never be paged, so
- // this returns before any of the paging/review code below.
+ // JSON must never be paged, so this returns before any of
+ // the paging/review code below.
jsonDirs = append(jsonDirs, plan.NewJSONDir(d.Name, d.Root, dp.Chains, fileNotes(dp.Result), dp.Result.Warnings))
return false
}
- // Ruling 6: the plan goes through tui.Page - taller than the
- // terminal, it is shown through $PAGER and the prompt follows
- // once the pager exits (spec §8.2) - for -n as much as for the
+ // The plan goes through tui.Page - taller than the terminal, it
+ // is shown through $PAGER and the prompt follows once the
+ // pager exits (spec §8.2) - for -n as much as for the
// interactive and -y paths; a dry run that scrolls 200 files
// off the top of the terminal is exactly the case the pager
// exists for. printPlan is reused as-is (render.go), never
@@ -228,8 +226,8 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
case 's':
// Spec §8.2: [s] applies nothing in this directory and
// moves on - no Apply call at all, so nothing is logged
- // for it either (Ruling 1: this is a chosen outcome, not a
- // failure, and must not set exit 1).
+ // for it either: this is a chosen outcome, not a failure,
+ // and must not set exit 1.
fmt.Fprintln(stdout, zeroOutcome)
return false
case 'q':
@@ -254,11 +252,11 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
}
res, aerr := sess.Apply(ctx, toApply, approved)
if aerr != nil {
- // Interrupted mid-apply (fix round 2026-09-12/item 2):
- // treated exactly like the cancelled lock wait above - not
- // a failure ("context canceled" is a Go-ism, not something
- // to show a user who just pressed Ctrl-C). The ctx.Err()
- // check at the end of this function turns it into exit 130.
+ // Interrupted mid-apply: treated exactly like the
+ // cancelled lock wait above - not a failure ("context
+ // canceled" is a Go-ism, not something to show a user who
+ // just pressed Ctrl-C). The ctx.Err() check at the end of
+ // this function turns it into exit 130.
if !interrupted(aerr) {
fmt.Fprintf(stderr, "krino: %s: %v\n", d.Name, aerr)
exit = 1
@@ -266,8 +264,8 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
return stopAfterApply(action, aerr)
}
fmt.Fprintln(stdout, withNotReviewed(outcome(p, res.Applied, res.Failed, res.Declined), notReviewed))
- // Ruling 1: only an actual step failure makes the run exit 1
- // here - a directory the user declined or skipped must not.
+ // Only an actual step failure makes the run exit 1 here - a
+ // directory the user declined or skipped must not.
if res.Failed > 0 {
exit = 1
}
@@ -305,12 +303,12 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
}
// actionableChains returns the chains of dp.Chains that have at least one
-// step that will actually run (chainActing, render.go) - fix wave item 4 /
-// Minor 5: this used to be a separate len(c.Steps) > 0 check, which
-// disagreed with render.go's countActing over a chain every one of whose
-// steps is skipped, so a directory could report "0 to act on" and then
-// still offer such a chain for approval. Converged on chainActing, this is
-// now also stricter than the filter engine.Apply's own forward-path loop
+// step that will actually run (chainActing, render.go). This used to be a
+// separate len(c.Steps) > 0 check, which disagreed with render.go's
+// countActing over a chain every one of whose steps is skipped, so a
+// directory could report "0 to act on" and then still offer such a chain
+// for approval. Converged on chainActing, this is now also stricter than
+// the filter engine.Apply's own forward-path loop
// applies (internal/engine/apply.go's Apply, still len(c.Steps) > 0): an
// all-skipped chain is simply never a candidate for approval here, so it
// can never reach Apply with approved == true, and Apply's own loop -
@@ -328,12 +326,12 @@ func actionableChains(chains []plan.Chain) []plan.Chain {
// installSignalHandler arranges for SIGINT and SIGTERM to cancel cancel
// and, if stdin is a terminal, restore it to the state it was in when this
-// was called (Ruling 5). The returned func stops the handler and must be
-// called once the run is over, or its goroutine and signal registration
-// outlive cmdSort.
+// was called. The returned func stops the handler and must be called once
+// the run is over, or its goroutine and signal registration outlive
+// cmdSort.
//
-// Fix round 2026-09-12/item 2: the handler loops rather than servicing one
-// signal and exiting. A single-shot select left signal.Notify's
+// The handler loops rather than servicing one signal and exiting. A
+// single-shot select left signal.Notify's
// registration in place (which suppresses Go's default terminate) with no
// goroutine left reading the channel, so a second Ctrl-C landed in the
// buffered channel unread and a third was dropped outright - together with
@@ -356,8 +354,8 @@ func installSignalHandler(cancel context.CancelFunc) func() {
sig := make(chan os.Signal, 1)
signal.Notify(sig, os.Interrupt, syscall.SIGTERM)
// A hangup stops krino like SIGTERM - unless it was started ignoring
- // hangups (nohup), which must go on meaning "keep running" (re-review
- // pa F3): asking for SIGHUP would re-enable it.
+ // hangups (nohup), which must go on meaning "keep running": asking for
+ // SIGHUP would re-enable it.
if !signal.Ignored(syscall.SIGHUP) {
signal.Notify(sig, syscall.SIGHUP)
}
@@ -374,20 +372,19 @@ func installSignalHandler(cancel context.CancelFunc) func() {
signals++
if signals >= 2 {
// Deliberate exception to "the lock is released on
- // every path" (fix round 2026-09-12/item 3, by
- // design, documented on review): every deferred
- // cleanup in cmdSort - including the held directory's
- // lock.Release - is skipped here. That is intentional:
- // this is the user's escape hatch when a clean
- // shutdown was already asked for once (the first
- // signal) and not delivered, so trying to unwind
- // cleanly a second time is exactly what would make the
- // hatch unreliable. It is safe to skip that unwind:
- // journal.Append flushes each line as it writes, so
- // nothing buffered is lost by exiting immediately, and
- // an abandoned lock file is reclaimed automatically by
- // Task 4's stale-pid takeover the next time anything
- // tries to acquire it (lock.go's tryAcquire).
+ // every path": every deferred cleanup in cmdSort -
+ // including the held directory's lock.Release - is
+ // skipped here. That is intentional: this is the
+ // user's escape hatch when a clean shutdown was
+ // already asked for once (the first signal) and not
+ // delivered, so trying to unwind cleanly a second time
+ // is exactly what would make the hatch unreliable. It
+ // is safe to skip that unwind: journal.Append flushes
+ // each line as it writes, so nothing buffered is lost
+ // by exiting immediately, and an abandoned lock file is
+ // reclaimed automatically by the stale-pid takeover
+ // the next time anything tries to acquire it (lock.go's
+ // tryAcquire).
os.Exit(130)
}
case <-done:
@@ -497,17 +494,17 @@ func printSkipped(w io.Writer, skipped []scan.Skipped) {
}
}
-// skipReasonOrder is plan 2's reviewed order for the skip reasons the last
-// line reports, before "unmatched".
+// skipReasonOrder is the order the skip reasons the last line reports,
+// before "unmatched".
var skipReasonOrder = []scan.Reason{scan.Ignored, scan.Busy, scan.TooNew, scan.TooBig, scan.Symlink, scan.NotRegular, scan.Unreadable}
// skipSummaryLine builds the "not acted on: N ignored · N busy · ... · N
// unmatched" line per spec §8.2's item format ("<count> <label>", not
// "<label>: <count>"), only the non-zero counts, or "" when every count is
-// zero. Ordering is plan 2's reviewed skipReasonOrder, with "unmatched"
-// last: the spec's own worked example shows only three of the seven
-// categories and states no ordering rule, so its incidental order is not
-// adopted, only its item format and unmatched's trailing position.
+// zero. Ordering follows skipReasonOrder, with "unmatched" last: the
+// spec's own worked example shows only three of the seven categories and
+// states no ordering rule, so its incidental order is not adopted, only
+// its item format and unmatched's trailing position.
func skipSummaryLine(r *engine.Result, chains []plan.Chain, verbose bool) string {
counts := map[scan.Reason]int{}
for _, s := range r.Skipped {
@@ -600,8 +597,8 @@ func interrupted(err error) bool {
}
// stopAfterApply reports whether krino stops once a directory has been
-// applied: an interrupt stops it, and so does [w], whether or not its apply
-// succeeded (review cli F3) - no later directory is planned or asked about.
+// applied: an interrupt stops it, and so does [w], whether or not its
+// apply succeeded - no later directory is planned or asked about.
func stopAfterApply(action rune, err error) bool {
return interrupted(err) || action == 'w'
}
diff --git a/cmd/krino/sort_test.go b/cmd/krino/sort_test.go
index 4f3e29f..4e68cb5 100644
--- a/cmd/krino/sort_test.go
+++ b/cmd/krino/sort_test.go
@@ -31,9 +31,9 @@ func TestRelWidthAndPadCellCountRunes(t *testing.T) {
}
}
-// TestActionableChainsAgreesWithCountActing is fix wave item 4 / Minor 5:
-// countActing (render.go) and actionableChains used to disagree over a
-// chain every one of whose steps is skipped (len(Steps) > 0, but every
+// TestActionableChainsAgreesWithCountActing: countActing (render.go) and
+// actionableChains used to disagree over a chain every one of whose steps
+// is skipped (len(Steps) > 0, but every
// step's own Skip is set) - countActing already excluded it from "to act
// on", while actionableChains's own len(Steps) > 0 check still offered it
// for approval, so a directory could print "N scanned · 0 to act on" and
@@ -53,17 +53,17 @@ func TestActionableChainsAgreesWithCountActing(t *testing.T) {
}
}
-// TestAllSkippedDirectoryReportsZeroAndLogsNothing is fix wave item 4 /
-// Minor 5 and 6, end to end. Before the fix: a directory whose one file
-// matches a rule under (on-conflict skip) - so its single step's own Skip
-// is set ("target exists") - printed "0 to act on" (countActing) and then,
-// with -y, still ran that chain through Apply anyway (actionableChains'
-// own len(Steps) > 0 check approved it regardless), logging a
-// run-start/run-end pair holding only a "skipped" entry while the outcome
-// line read "0 applied · 0 failed · 0 declined" for a file that had just
-// been silently processed. After the fix, the chain is never offered for
-// approval, Apply is never even called for this directory, and the journal
-// gains nothing at all.
+// TestAllSkippedDirectoryReportsZeroAndLogsNothing, end to end: a
+// directory whose one file matches a rule under (on-conflict skip) - so
+// its single step's own Skip is set ("target exists") - must report "0 to
+// act on" (countActing) and, with -y, must not run that chain through
+// Apply anyway: an all-skipped chain offered for approval regardless
+// (as actionableChains' own len(Steps) > 0 check alone would allow) would
+// log a run-start/run-end pair holding only a "skipped" entry while the
+// outcome line read "0 applied · 0 failed · 0 declined" for a file that
+// had just been silently processed. Instead the chain is never offered
+// for approval, Apply is never even called for this directory, and the
+// journal gains nothing at all.
func TestAllSkippedDirectoryReportsZeroAndLogsNothing(t *testing.T) {
h := home(t)
dl := filepath.Join(h, "dl")
@@ -225,7 +225,7 @@ func TestDryRunShowsNeverDeletedSkip(t *testing.T) {
if !strings.Contains(out, "skipped: a duplicate is never deleted") {
t.Errorf("plan lacks the skipped delete:\n%s", out)
}
- // explain shows it too, for the copy that is the duplicate (triage 30a).
+ // explain shows it too, for the copy that is the duplicate.
_, outA, _ := runCLI(t, "explain", filepath.Join(dl, "a.pdf"))
_, outB, _ := runCLI(t, "explain", filepath.Join(dl, "b.pdf"))
if strings.Count(outA+outB, "a duplicate is never deleted") != 1 {
@@ -235,8 +235,8 @@ func TestDryRunShowsNeverDeletedSkip(t *testing.T) {
// TestLaterDirectoryIsNotBlockedByAnEarlierOnesClaims: in a real run each
// directory is applied before the next is planned, so the disk is the
-// truth; a path an earlier directory moved a file away from, or planned and
-// did not apply, is not "taken" for a later one (triage 28i).
+// truth; a path an earlier directory moved a file away from, or planned
+// and did not apply, is not "taken" for a later one.
func TestLaterDirectoryIsNotBlockedByAnEarlierOnesClaims(t *testing.T) {
h := home(t)
old := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
@@ -300,8 +300,8 @@ func devFullFixture(t *testing.T) {
}
// TestApplyErrorExitsOne: an error applying a directory - here the log
-// cannot be written - makes krino exit 1 (triage 34m: removing that exit
-// code left every test passing).
+// cannot be written - makes krino exit 1; removing that exit code left
+// every other test passing.
func TestApplyErrorExitsOne(t *testing.T) {
devFullFixture(t)
code, _, errOut := runCLI(t, "-y", "d1")
@@ -311,9 +311,9 @@ func TestApplyErrorExitsOne(t *testing.T) {
}
// TestWriteStopsKrinoWhenApplyFails: [w] in review stops krino after its
-// directory even when applying it fails - the next directory is not planned
-// or asked about (review cli F3), driven through the real command with a
-// pipe standing in for the terminal (triage 28m).
+// directory even when applying it fails - the next directory is not
+// planned or asked about, driven through the real command with a pipe
+// standing in for the terminal.
func TestWriteStopsKrinoWhenApplyFails(t *testing.T) {
devFullFixture(t)
r, w, err := os.Pipe()
@@ -337,7 +337,7 @@ func TestWriteStopsKrinoWhenApplyFails(t *testing.T) {
// TestLaterDirectoryNeverOverwritesAnEarlierOnesResult: what an earlier
// directory of the run put somewhere stays claimed, so a later directory's
// (on-conflict overwrite) takes a free name instead of trashing it - as a
-// dry run of the same two directories shows (plan 11 review M1).
+// dry run of the same two directories shows.
func TestLaterDirectoryNeverOverwritesAnEarlierOnesResult(t *testing.T) {
h := home(t)
old := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
@@ -369,7 +369,7 @@ func TestLaterDirectoryNeverOverwritesAnEarlierOnesResult(t *testing.T) {
// TestOnlyWhereFilesEndedUpStaysClaimed: a path an earlier directory's file
// passed through and left - renamed, then moved on - is free for a later
-// directory; only where files ended up stays claimed (plan 11 re-check).
+// directory; only where files ended up stays claimed.
func TestOnlyWhereFilesEndedUpStaysClaimed(t *testing.T) {
h := home(t)
old := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
diff --git a/cmd/krino/undo.go b/cmd/krino/undo.go
index 5a7db4b..85b74b5 100644
--- a/cmd/krino/undo.go
+++ b/cmd/krino/undo.go
@@ -26,9 +26,9 @@ func init() { commands["undo"] = cmdUndo }
// the most recent one otherwise. Since the newest run in the log can never
// itself be marked Undone - that would require a still-later run to have
// reversed it - "the most recent run" and "the most recent run that has not
-// been undone" are the same run in every case, including the one this
-// task's own test exercises: undoing an undo run a second time with no RUN
-// argument targets that very undo run, which PlanUndo then refuses by name.
+// been undone" are the same run in every case, including the case tested
+// below: undoing an undo run a second time with no RUN argument targets
+// that very undo run, which PlanUndo then refuses by name.
//
// Undo builds a plan like any other, shown and approved the same way (spec
// §10) - reviewUndoDir/-Files/-PerFile below are undo's own counterpart to
@@ -74,10 +74,9 @@ func cmdUndo(g *globals, args []string, stdout, stderr io.Writer) int {
}
// Installed here, before any paging or review, not just around
- // ApplyUndo: Ruling 5 (Task 7) is that SIGTERM landing between
- // keystrokes or during the pager needs the terminal restored, and that
- // window starts as soon as this command might show something on a
- // terminal.
+ // ApplyUndo: SIGTERM landing between keystrokes or during the pager
+ // needs the terminal restored, and that window starts as soon as this
+ // command might show something on a terminal.
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
stopSignals := installSignalHandler(cancel)
@@ -101,9 +100,9 @@ func cmdUndo(g *globals, args []string, stdout, stderr io.Writer) int {
return 0
}
runID = runs[0].ID
- // The most recent run is itself an undo: continue it, by planning the
- // run it undid again - reversals it completed are not offered twice
- // (review M10). Naming an undo run explicitly is still refused.
+ // The most recent run is itself an undo: continue it, by planning
+ // the run it undid again - reversals it completed are not offered
+ // twice. Naming an undo run explicitly is still refused.
if runs[0].UndoOf != "" {
runID = runs[0].UndoOf
}
@@ -128,9 +127,8 @@ func cmdUndo(g *globals, args []string, stdout, stderr io.Writer) int {
return 1
}
- // Fix round 2026-09-12 (widened per the coordinator's follow-up
- // ruling): an undo moves files just as an apply does, so it needs
- // cmdSort's same per-directory guard (spec §11: a second krino on the
+ // An undo moves files just as an apply does, so it needs cmdSort's
+ // same per-directory guard (spec §11: a second krino on the
// same directory waits for the lock, or fails immediately with -y),
// held across the SAME window cmdSort holds its own lock across - the
// plan display and the review, not just the apply. Failing before the
@@ -165,8 +163,8 @@ func cmdUndo(g *globals, args []string, stdout, stderr io.Writer) int {
var buf bytes.Buffer
printUndoPlan(&buf, up)
text := colourRefused(buf.String(), p)
- // Ruling 6 (Task 7), carried over: the plan goes through tui.Page for
- // -n as much as for -y and the interactive path.
+ // The plan goes through tui.Page for -n as much as for -y and the
+ // interactive path.
if err := show(g, stdout, text); err != nil {
fmt.Fprintf(stderr, "krino: %v\n", err)
return 1
@@ -204,9 +202,9 @@ func cmdUndo(g *globals, args []string, stdout, stderr io.Writer) int {
toApply, notReviewed := finalizeUndoPlan(up, approved, action)
- // Ruling 2 (Task 7), carried over: journal.Open creates the state
- // directory and the log file as a side effect of merely being called,
- // so the session opens it only now, when something will actually be
+ // journal.Open creates the state directory and the log file as a side
+ // effect of merely being called, so the session opens it only now,
+ // when something will actually be
// applied - never for -n (returned above), and not merely because -y
// or a review session ran, unlike cmdSort, which opens before it knows
// whether anything is actionable (a difference forced by cmdSort not
@@ -281,18 +279,17 @@ func releaseUndoLocks(locks []*lock.Lock) []error {
// finalizeUndoPlan builds the *engine.UndoPlan ApplyUndo actually runs,
// preserving up.Files' own order: every refused file rides along unchanged
// (ApplyUndo declines these itself, silently, exactly as it already does
-// when handed the unfiltered plan - spec §10's refusal is not this task's
-// to make noisier); every actionable file approved marks true rides along
+// when handed the unfiltered plan - spec §10's refusal is not made any
+// noisier here); every actionable file approved marks true rides along
// unchanged too. A file the user said no to, or left unmarked when [d] or
-// [q] cut a per-file review short, is not dropped - fix round 2026-09-12,
-// item 2 of Task 8's review: spec §9 says a declined file is logged even
-// though nothing happens to it, the same as the forward path already does,
-// so it is kept with Declined set, which tells ApplyUndo to log its steps
-// as declined rather than reverse them.
+// [q] cut a per-file review short, is not dropped: spec §9 says a
+// declined file is logged even though nothing happens to it, the same as
+// the forward path already does, so it is kept with Declined set, which
+// tells ApplyUndo to log its steps as declined rather than reverse them.
//
// After [w] (action 'w'), a reversible file the review never reached is
// left out of the plan entirely, as review's [w] leaves a forward file
-// unlogged, and counted in notReviewed (review cli F2).
+// unlogged, and counted in notReviewed.
func finalizeUndoPlan(up *engine.UndoPlan, approved map[int]bool, action rune) (plan *engine.UndoPlan, notReviewed int) {
out := &engine.UndoPlan{Run: up.Run, Cleanup: up.Cleanup}
for i, f := range up.Files {
@@ -374,15 +371,15 @@ func reviewUndoFiles(in io.Reader, out io.Writer, files []engine.UndoFile, p pal
// its reason and reverses nothing of it regardless of anything chosen here
// - but it still gets its own [i/N] line, so the numbering accounts for
// every file in the plan, not just the reversible ones. It behaves as
-// review.go's reviewPerFile does (review cli F2): a no is recorded as false,
-// each choice is echoed in red, [w] ends with 'w' leaving unreached files
-// out of approved, and [q] ends with 'q'.
+// review.go's reviewPerFile does: a no is recorded as false, each choice is
+// echoed in red, [w] ends with 'w' leaving unreached files out of
+// approved, and [q] ends with 'q'.
func reviewUndoPerFile(in io.Reader, out io.Writer, files []engine.UndoFile, p palette) (approved map[int]bool, end rune, err error) {
approved = map[int]bool{}
yesRest := false
for i, f := range files {
// Heading and steps wrap past the action column, so a long name or
- // path cannot pass for a step line (plan 11 review L4).
+ // path cannot pass for a step line.
width := widthPolicy(out)
fmt.Fprintln(out)
for _, l := range wrapped("", fmt.Sprintf("[%d/%d] %s/%s", i+1, len(files), display(f.Dir), display(f.File)), 7+undoStepWidth+2, width, plainText) {
@@ -468,11 +465,11 @@ const undoStepWidth = len("undo-displace")
// one (a sibling directory not yet empty for undo-mkdir - spec §10's one
// case where a step's own failure does not refuse its whole file), the
// directory removed for undo-mkdir (no destination to show), the file being
-// trashed for undo-copy (fix wave item 3: its Dst is deliberately empty -
-// trash.Put only chooses the entry name at execution time - so this is the
-// one action with no path to point an arrow at; before this fix the cell
-// rendered as a bare "undo-copy → ", the plan's one row that said
-// nothing about what it would do to the user's file), or an arrow to where
+// trashed for undo-copy (its Dst is deliberately empty - trash.Put only
+// chooses the entry name at execution time - so this is the one action
+// with no path to point an arrow at; rendering it as a bare "undo-copy
+// → " would be the plan's one row that says nothing about what it would do
+// to the user's file), or an arrow to where
// the step puts the file back, ~-abbreviated - undo has no single root the
// way a sort plan does (one run can span several directories), so there is
// no root-relative form to render here the way actionCell has.
generated by cgit v1.3 (git 2.53.0) at 2026-09-19 15:30:14 +0000