diff options
27 files changed, 2423 insertions, 117 deletions
diff --git a/cmd/krino/matching_test.go b/cmd/krino/matching_test.go index eaf9494..614edc2 100644 --- a/cmd/krino/matching_test.go +++ b/cmd/krino/matching_test.go @@ -4,6 +4,7 @@ package main import ( "bytes" + "encoding/json" "os" "path/filepath" "strings" @@ -61,12 +62,12 @@ func TestDryRun(t *testing.T) { t.Fatalf("exit %d: %s", code, errOut) } for _, want := range []string{ - "krino: dl ~/dl\n8 scanned · 4 matched · 2 warnings · ", - "\n inv1.txt acme: type txt, content \"acme ltd\"\n", - "\n notes.txt rest: not matched, type txt\n", - "\n report (1).pdf dups: duplicate of report.pdf\n", + "krino: dl ~/dl\n8 scanned · 4 to act on · 2 warnings · ", + "\n 1 inv1.txt move → Work/Acme/ acme type txt, content \"acme ltd\"\n", + "\n 2 notes.txt move → Other/ rest not matched, type txt\n", + "\n 4 report (1).pdf trash dups duplicate of report.pdf\n", "\nwarnings\n brochure.doc acme: content unreadable: needs antiword or catdoc, not installed\n", - "\nnot matched: 2 · ignored: 1 · busy: 1 (-v lists them)\n", + "\nnot acted on: 1 ignored · 1 busy · 2 unmatched (-v lists them)\n", } { if !strings.Contains(out, want) { t.Errorf("output lacks %q:\n%s", want, out) @@ -74,11 +75,38 @@ func TestDryRun(t *testing.T) { } } +// TestDryRunJSON is the --json counterpart of TestDryRun: with -n it now +// prints the plan instead of refusing, and the document parses. +func TestDryRunJSON(t *testing.T) { + matchingFixture(t) + code, out, errOut := runCLI(t, "-n", "--json") + if code != 0 { + t.Fatalf("exit %d: %s", code, errOut) + } + for _, want := range []string{`"action": "move"`, `"rel": "inv1.txt"`} { + if !strings.Contains(out, want) { + t.Errorf("json output lacks %s:\n%s", want, out) + } + } + var doc struct { + Version int `json:"version"` + Dirs []struct { + Name string `json:"name"` + } `json:"dirs"` + } + if err := json.Unmarshal([]byte(out), &doc); err != nil { + t.Fatalf("output does not parse as JSON: %v\n%s", err, out) + } + if doc.Version != 1 || len(doc.Dirs) != 1 || doc.Dirs[0].Name != "dl" { + t.Errorf("document = %+v", doc) + } +} + func TestDryRunVerbose(t *testing.T) { matchingFixture(t) _, out, _ := runCLI(t, "-n", "-v") for _, want := range []string{ - "not matched: 2 · ignored: 1 · busy: 1\n", + "not acted on: 1 ignored · 1 busy · 2 unmatched\n", "\nnot matched\n brochure.doc\n report.pdf\n", "\nskipped\n movie.mkv busy\n movie.mkv.part ignored\n", } { @@ -121,7 +149,7 @@ func TestSortFlags(t *testing.T) { }{ {[]string{"-y", "-n"}, "-y and -n cannot be used together"}, {nil, "applying files is not implemented yet; use -n to see what would happen"}, - {[]string{"-n", "--json"}, "--json is not implemented yet"}, + {[]string{"--json"}, "--json is not implemented yet"}, // --json without -n stays an error } for _, tt := range tests { if code, _, errOut := runCLI(t, tt.args...); code != 2 || !strings.Contains(errOut, tt.want) { @@ -355,7 +383,7 @@ func TestLongNameNotPaddedLayoutIntact(t *testing.T) { conf := ` (path "~/dl") (min-age 0s) -(rule "r" (when (type txt)) (stop)) +(rule "r" (when (type txt)) (move "Other")) ` if err := os.WriteFile(filepath.Join(h, ".config/krino/dirs/dl.conf"), []byte(conf), 0o644); err != nil { t.Fatal(err) @@ -365,10 +393,10 @@ func TestLongNameNotPaddedLayoutIntact(t *testing.T) { if code != 0 { t.Fatalf("exit %d: %s", code, errOut) } - if want := "\n " + long + " r: type txt\n"; !strings.Contains(out, want) { - t.Errorf("long name should be unpadded (exactly two trailing spaces before the rule column):\n%s\nwant substring:\n%s", out, want) + if want := "\n 1 " + long + " move → Other/ r type txt\n"; !strings.Contains(out, want) { + t.Errorf("long name should be unpadded (exactly two trailing spaces before the actions column):\n%s\nwant substring:\n%s", out, want) } - if want := "\n " + short + strings.Repeat(" ", 40-len(short)) + " r: type txt\n"; !strings.Contains(out, want) { + if want := "\n 2 " + short + strings.Repeat(" ", 40-len(short)) + " move → Other/ r type txt\n"; !strings.Contains(out, want) { t.Errorf("short name should still be padded to the 40-column cap:\n%s\nwant substring:\n%s", out, want) } } diff --git a/cmd/krino/render.go b/cmd/krino/render.go new file mode 100644 index 0000000..b27d4d1 --- /dev/null +++ b/cmd/krino/render.go @@ -0,0 +1,258 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package main + +import ( + "fmt" + "io" + "path/filepath" + "strconv" + "strings" + "unicode/utf8" + + "krino/internal/engine" + "krino/internal/plan" + "krino/internal/xdg" +) + +// actionKindWidth is the field an action cell's kind word is left-padded +// to before its arrow: the widest of the three kinds that carry a +// destination ("copy", "move", "rename"). Trash and DELETE permanently +// have no destination, hence no arrow to align, so they are not padded. +const actionKindWidth = len("rename") + +// printPlan renders one directory's plan the way krino -n shows it, per +// spec §8.2, below the header line cmdSort has already written: a counts +// line, the numbered action table, the warnings section and the "not +// acted on" line, each present only when it has something to show. No +// colour, pager or prompt: those arrive with the review UI in plan 4, +// which wraps this same function, hence its plain (io.Writer, *DirPlan, +// bool) signature. +func printPlan(w io.Writer, dp *engine.DirPlan, verbose bool) { + 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 := 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 + // 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 + // before Build ever runs - the label says "planning", so the number + // must cover all of it. + fmt.Fprintf(w, "%d scanned · %d to act on · %d warnings · %.2fs\n", scanned, countActing(dp.Chains), warnedCount(lines), dp.Elapsed.Seconds()) + + if rows := planRows(dp.Chains, dp.Dir.Root); len(rows) > 0 { + fmt.Fprintln(w) + printPlanTable(w, rows) + } + + if len(lines) > 0 { + fmt.Fprintln(w) + fmt.Fprintln(w, "warnings") + printWarnings(w, lines) + } + + if line := skipSummaryLine(r, dp.Chains, verbose); line != "" { + fmt.Fprintln(w) + fmt.Fprintln(w, line) + } + + if verbose { + if len(r.Unmatched) > 0 { + fmt.Fprintln(w) + fmt.Fprintln(w, "not matched") + for _, fm := range r.Unmatched { + fmt.Fprintf(w, " %s\n", fm.File.Rel) + } + } + if len(r.Skipped) > 0 { + fmt.Fprintln(w) + fmt.Fprintln(w, "skipped") + printSkipped(w, r.Skipped) + } + } +} + +// 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". +func countActing(chains []plan.Chain) int { + n := 0 + for _, c := range chains { + for _, s := range c.Steps { + if s.Skip == "" { + n++ + break + } + } + } + return n +} + +// planRow is one line of the action table: a file's first step (num and +// file set) or a continuation line (both blank). chainIdx (D15) is the +// index into the DirPlan's own Chains slice this row was built from, kept +// alongside the display strings so plan 4's per-file approval ("row 2 is +// chain index 1", spec §8.3) does not have to re-derive the mapping later; +// it changes no rendering here and is not itself asserted by a test. +type planRow struct { + num, file, actions, rule, reason string + chainIdx int +} + +// planRows turns the chains that have at least one step into table rows, +// per spec §8.2: a file's first step shares its numbered row; later steps +// sit on continuation lines with the number and file columns blank. A +// chain with no steps at all - an exclusion, or a rule that only stops - +// has nothing to show and contributes no row and no number. root is the +// directory being planned, threaded down to destText so a destination +// inside it renders root-relative rather than home-abbreviated. +func planRows(chains []plan.Chain, root string) []planRow { + var rows []planRow + n := 0 + for ci, c := range chains { + if len(c.Steps) == 0 { + continue + } + n++ + for i, s := range c.Steps { + row := planRow{actions: actionCell(s, root), rule: s.Rule, reason: s.Reason, chainIdx: ci} + if i == 0 { + row.num = strconv.Itoa(n) + row.file = c.File.Rel + } + rows = append(rows, row) + } + } + return rows +} + +// actionCell renders one step's action column. A step with Skip set shows +// its reason in place of the destination. Trash and DELETE permanently +// have no destination to show - even when the step came from a duplicate +// rule, the reason column already says what it is a duplicate of, so the +// action column is just the kind word. Everything else (copy, move, +// rename) shows the kind, padded so every arrow in the table lines up, +// then its destination; Displaces adds a trailing note. +func actionCell(s plan.Step, root string) string { + kind := s.Kind.String() + if s.Skip != "" { + return padCell(kind, actionKindWidth) + " skipped: " + s.Skip + } + switch s.Kind { + case plan.Trash, plan.DeletePermanent: + return kind + } + cell := padCell(kind, actionKindWidth) + " → " + destText(s, root) + if s.Displaces != "" { + cell += " (replaces the existing file)" + } + return cell +} + +// destText renders a copy/move/rename step's destination, per spec §8.2: +// for rename, just the new base name. For copy and move, a directory with +// a trailing "/" so it reads as one - root-relative when it lies inside +// the directory being planned (spec's own worked example: "Work/Acme/"), +// abbreviated against $HOME otherwise (the same example's +// "~/backup/invoices/2026/", outside the root entirely). +func destText(s plan.Step, root string) string { + if s.Kind == plan.Rename { + return filepath.Base(s.Dst) + } + 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 + // directory". + return "./" + } + return rel + "/" + } + return xdg.Abbrev(dir) + "/" +} + +// 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) - +// 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. +func relToRoot(root, dir string) (rel string, ok bool) { + r, err := filepath.Rel(root, dir) + if err != nil || r == ".." || strings.HasPrefix(r, ".."+string(filepath.Separator)) { + return "", false + } + if r == "." { + return "", true // dir is root itself + } + return filepath.ToSlash(r), true +} + +// printPlanTable prints rows in the table layout of spec §8.2: #, file, +// actions and rule are padded to what is actually shown in this section, +// in runes, exactly as relWidth/padCell already do elsewhere; the file +// column is capped at 40 and the actions column at 46. The reason is the +// last column and is never padded. The row number is right-aligned +// (padLeft, not padCell) so the "#" column stays flush as it widens past +// a single digit - the layout plan 4's review UI inherits unchanged. +func printPlanTable(w io.Writer, rows []planRow) { + nums := make([]string, len(rows)) + files := make([]string, len(rows)) + actions := make([]string, len(rows)) + rules := make([]string, len(rows)) + for i, r := range rows { + nums[i], files[i], actions[i], rules[i] = r.num, r.file, r.actions, r.rule + } + // D13: numW and ruleW are left uncapped, unlike fileW and actionsW. The + // row number is at most a few digits regardless of how large a + // directory is, and a rule name is a config author's own identifier, + // not scan noise from a user's file names - truncating a name someone + // deliberately chose would only make the row harder to trace back to + // its rule, so there is nothing here worth capping. + numW := colWidth(nums, 0) + fileW := relWidth(files) + actionsW := colWidth(actions, 46) + ruleW := colWidth(rules, 0) + + fmt.Fprintf(w, " %s %s %s rule\n", padLeft("#", numW), padCell("file", fileW), padCell("actions", actionsW)) + for _, r := range rows { + fmt.Fprintf(w, " %s %s %s %s %s\n", padLeft(r.num, numW), padCell(r.file, fileW), padCell(r.actions, actionsW), padCell(r.rule, ruleW), r.reason) + } +} + +// padLeft pads s to width w (runes, not bytes) with leading spaces, +// right-aligning it; s already at or beyond w is left unpadded. Used only +// for the row-number column - every other column reads left-aligned, per +// padCell. +func padLeft(s string, w int) string { + n := utf8.RuneCountInString(s) + if n >= w { + return s + } + return strings.Repeat(" ", w-n) + s +} + +// 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. +func colWidth(ss []string, max int) int { + w := 0 + for _, s := range ss { + if n := utf8.RuneCountInString(s); n > w { + w = n + } + } + if max > 0 && w > max { + w = max + } + return w +} diff --git a/cmd/krino/render_test.go b/cmd/krino/render_test.go new file mode 100644 index 0000000..0a6ae26 --- /dev/null +++ b/cmd/krino/render_test.go @@ -0,0 +1,283 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package main + +import ( + "bytes" + "fmt" + "path/filepath" + "strings" + "testing" + "time" + + "krino/internal/engine" + "krino/internal/plan" + "krino/internal/scan" +) + +// TestPrintPlan is the golden render test of spec §8.2. The DirPlan is +// built by hand, not by running a scan, so the expected output cannot +// drift with a fixture: it asserts the header row, a numbered row, a +// continuation line, the "DELETE permanently" capitalisation and the +// counts line. +// +// The two destinations pin both branches of destText: scan001.pdf's and +// fv_123.pdf's "acme" step lands inside root and renders root-relative +// ("Work/Acme/2026/"), while fv_123.pdf's "backup" step lands under $HOME +// but outside root and stays home-abbreviated ("~/backup/invoices/2026/") +// - spec §8.2's own worked example draws exactly this distinction. +// +// "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). +func TestPrintPlan(t *testing.T) { + h := home(t) + root := filepath.Join(h, "downloads") + + scan001 := plan.Chain{ + File: scan.File{Rel: "scan001.pdf"}, + Steps: []plan.Step{ + { + Kind: plan.Move, + Rule: "acme", + Src: filepath.Join(root, "scan001.pdf"), + Dst: filepath.Join(root, "Work", "Acme", "2026", "scan001.pdf"), + Reason: `content "acme ltd"`, + }, + }, + } + fv123 := plan.Chain{ + File: scan.File{Rel: "fv_123.pdf"}, + Steps: []plan.Step{ + { + Kind: plan.Copy, + Rule: "backup", + Src: filepath.Join(root, "fv_123.pdf"), + Dst: filepath.Join(h, "backup", "invoices", "2026", "fv_123.pdf"), + Reason: `content "invoice"`, + }, + { + Kind: plan.Move, + Rule: "acme", + Src: filepath.Join(h, "backup", "invoices", "2026", "fv_123.pdf"), + Dst: filepath.Join(root, "Work", "Acme", "2026", "fv_123.pdf"), + Reason: `name \bacme\b`, + }, + }, + } + setup := plan.Chain{ + File: scan.File{Rel: "setup-1.2.deb"}, + Steps: []plan.Step{ + {Kind: plan.DeletePermanent, Rule: "old-pkgs", Src: filepath.Join(root, "setup-1.2.deb"), Reason: "age 94d"}, + }, + } + excluded := plan.Chain{File: scan.File{Rel: "excluded.txt"}} // matched a stop-only rule: no actions, no steps + + 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 + Result: &engine.Result{ + Matched: []engine.FileMatch{ + {File: scan.File{Rel: "scan001.pdf"}, Warnings: []string{"acme: content unreadable: needs pdftotext, not installed"}}, + {File: scan.File{Rel: "fv_123.pdf"}}, + {File: scan.File{Rel: "setup-1.2.deb"}}, + {File: scan.File{Rel: "excluded.txt"}}, + }, + Unmatched: []engine.FileMatch{{File: scan.File{Rel: "unmatched.txt"}}}, + Skipped: []scan.Skipped{{Rel: "busy.tmp", Reason: scan.Busy}}, + }, + } + + var buf bytes.Buffer + printPlan(&buf, dp, false) + out := buf.String() + + for _, want := range []string{ + "6 scanned · 3 to act on · 1 warnings · 0.42s\n", + " # file actions rule\n", + " 1 scan001.pdf move → Work/Acme/2026/ acme content \"acme ltd\"\n", + " 2 fv_123.pdf copy → ~/backup/invoices/2026/ backup content \"invoice\"\n", + " move → Work/Acme/2026/ acme name \\bacme\\b\n", + " 3 setup-1.2.deb DELETE permanently old-pkgs age 94d\n", + "warnings\n scan001.pdf acme: content unreadable: needs pdftotext, not installed\n", + "not acted on: 1 busy · 1 excluded · 1 unmatched (-v lists them)\n", + } { + if !strings.Contains(out, want) { + t.Errorf("output lacks %q:\n%s", want, out) + } + } + if strings.Contains(out, "excluded.txt") { + t.Errorf("excluded.txt has no steps and must not appear in the table:\n%s", out) + } +} + +// TestPrintPlanSkippedStep: a step with Skip set shows its reason in place +// of the destination, and a Displaces step notes that it replaces the +// existing file. +func TestPrintPlanSkippedStep(t *testing.T) { + home(t) // isolate HOME even though these paths do not use it + dp := &engine.DirPlan{ + Dir: &engine.Dir{Name: "dl", Root: "/r"}, + Chains: []plan.Chain{ + { + File: scan.File{Rel: "a.pdf"}, + Steps: []plan.Step{ + {Kind: plan.Copy, Rule: "backup", Src: "/r/a.pdf", Dst: "/backup/a.pdf", Skip: "target exists"}, + }, + }, + { + File: scan.File{Rel: "b.pdf"}, + Steps: []plan.Step{ + {Kind: plan.Move, Rule: "acme", Src: "/r/b.pdf", Dst: "/r/Work/b.pdf", Displaces: "/r/Work/b.pdf"}, + }, + }, + }, + Result: &engine.Result{ + Matched: []engine.FileMatch{{File: scan.File{Rel: "a.pdf"}}, {File: scan.File{Rel: "b.pdf"}}}, + }, + } + var buf bytes.Buffer + printPlan(&buf, dp, false) + out := buf.String() + if !strings.Contains(out, "copy skipped: target exists") { + t.Errorf("skipped step should show its reason in place of the destination:\n%s", out) + } + if !strings.Contains(out, "move → Work/ (replaces the existing file)") { + t.Errorf("a Displaces step should note it replaces the existing file:\n%s", out) + } + if !strings.Contains(out, "2 scanned · 1 to act on · 0 warnings ·") { + t.Errorf("a.pdf's only step is skipped, so it must not count as \"to act on\":\n%s", out) + } +} + +// TestPrintPlanRowNumberAlignment pins the table layout plan 4's review UI +// inherits: with 11 acted-on files the row-number column has to widen past +// a single digit, and the row number is right-aligned so "#" stays flush. +// The fixture also carries a file name past the 40-character cap and a +// rule name noticeably longer than the rest, exercising the file and rule +// columns' own per-section widths at the same time. +func TestPrintPlanRowNumberAlignment(t *testing.T) { + h := home(t) + root := filepath.Join(h, "dl") + long := strings.Repeat("z", 42) + ".txt" // 46 runes: past the 40-column cap + + names := make([]string, 11) + for i := range names { + names[i] = fmt.Sprintf("f%02d.txt", i+1) + } + names[10] = long // row 11 carries the long name + + var chains []plan.Chain + var matched []engine.FileMatch + for i, name := range names { + rule := "r" + if i == 5 { + rule = "a-noticeably-longer-rule-name" + } + chains = append(chains, plan.Chain{ + File: scan.File{Rel: name}, + Steps: []plan.Step{ + {Kind: plan.Move, Rule: rule, Src: filepath.Join(root, name), Dst: filepath.Join(root, "Out", name), Reason: "type txt"}, + }, + }) + matched = append(matched, engine.FileMatch{File: scan.File{Rel: name}}) + } + + dp := &engine.DirPlan{ + Dir: &engine.Dir{Name: "dl", Root: root}, + Chains: chains, + Result: &engine.Result{Matched: matched}, + } + + var buf bytes.Buffer + printPlan(&buf, dp, false) + out := buf.String() + + for _, want := range []string{ + " # file actions rule\n", + " 1 f01.txt move → Out/ r type txt\n", + " 6 f06.txt move → Out/ a-noticeably-longer-rule-name type txt\n", + " 10 f10.txt move → Out/ r type txt\n", + " 11 " + long + " move → Out/ r type txt\n", + } { + if !strings.Contains(out, want) { + t.Errorf("output lacks %q:\n%s", want, out) + } + } +} + +// TestPrintPlanCountsFileOnceWithBothWarningKinds: B2 - a file may carry +// both a match warning (Result.Matched[i].Warnings) and a chain warning +// (Chain.Warnings, e.g. "moved more than once"). Both must reach the +// warnings section, but the counts line's "N warnings" counts files with at +// least one warning, not warning lines, so this one file must still count +// as 1, not 2. +func TestPrintPlanCountsFileOnceWithBothWarningKinds(t *testing.T) { + h := home(t) + root := filepath.Join(h, "dl") + dp := &engine.DirPlan{ + Dir: &engine.Dir{Name: "dl", Root: root}, + Chains: []plan.Chain{ + { + File: scan.File{Rel: "a.pdf"}, + Steps: []plan.Step{ + {Kind: plan.Move, Rule: "r1", Src: filepath.Join(root, "a.pdf"), Dst: filepath.Join(root, "Out", "a.pdf")}, + {Kind: plan.Move, Rule: "r2", Src: filepath.Join(root, "Out", "a.pdf"), Dst: filepath.Join(root, "Out2", "a.pdf")}, + }, + Warnings: []string{"moved more than once; a (stop) is probably missing"}, + }, + }, + Result: &engine.Result{ + Matched: []engine.FileMatch{ + {File: scan.File{Rel: "a.pdf"}, Warnings: []string{"r1: content unreadable: needs pdftotext, not installed"}}, + }, + }, + } + var buf bytes.Buffer + printPlan(&buf, dp, false) + out := buf.String() + + for _, want := range []string{ + "1 scanned · 1 to act on · 1 warnings ·", + " a.pdf r1: content unreadable: needs pdftotext, not installed\n", + " a.pdf moved more than once; a (stop) is probably missing\n", + } { + if !strings.Contains(out, want) { + t.Errorf("output lacks %q:\n%s", want, out) + } + } + if strings.Contains(out, "2 warnings") { + t.Errorf("one file with two warnings must count once, not twice:\n%s", out) + } +} + +// TestSkipSummaryLineAccountsForEveryFile pins that the footer's categories +// add up to "scanned". The real downloads folder reported "267 scanned · 172 +// to act on" while saying nothing about the other 95, which had matched an +// exclusion rule carrying no actions: they were neither acted on, nor +// unmatched, nor skipped by the walk. +func TestSkipSummaryLineAccountsForEveryFile(t *testing.T) { + r := &engine.Result{ + Matched: make([]engine.FileMatch, 4), + Unmatched: make([]engine.FileMatch, 2), + Skipped: []scan.Skipped{{Rel: "a.part", Reason: scan.Ignored}, {Rel: "b.iso", Reason: scan.Busy}}, + } + chains := []plan.Chain{ + {Steps: []plan.Step{{Kind: plan.Move, Dst: "/r/W/x"}}}, // acting + {}, // excluded: matched an action-less rule + {}, // excluded + {Steps: []plan.Step{{Kind: plan.Move, Skip: "target exists"}}}, // every step skipped + } + got := skipSummaryLine(r, chains, false) + want := "not acted on: 1 ignored · 1 busy · 2 excluded · 1 all steps skipped · 2 unmatched (-v lists them)" + if got != want { + t.Errorf("line =\n%q\nwant\n%q", got, want) + } + scanned := len(r.Matched) + len(r.Unmatched) + len(r.Skipped) + excluded, allSkipped := chainOutcomes(chains) + if acting := countActing(chains); acting+excluded+allSkipped+len(r.Unmatched)+len(r.Skipped) != scanned { + t.Errorf("categories do not sum to scanned: %d acting + %d excluded + %d all-skipped + %d unmatched + %d skipped != %d", + acting, excluded, allSkipped, len(r.Unmatched), len(r.Skipped), scanned) + } +} diff --git a/cmd/krino/sort.go b/cmd/krino/sort.go index fe88cf8..0ba82bc 100644 --- a/cmd/krino/sort.go +++ b/cmd/krino/sort.go @@ -4,6 +4,7 @@ package main import ( "context" + "encoding/json" "fmt" "io" "os" @@ -12,17 +13,18 @@ import ( "unicode/utf8" "krino/internal/engine" + "krino/internal/plan" "krino/internal/scan" "krino/internal/xdg" ) // cmdSort plans and applies the included directories. Only -n (dry run) is -// implemented; applying arrives in plan 3. +// implemented; applying arrives in plan 4. 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") } - if g.json { + if g.json && !g.dry { fmt.Fprintln(stderr, "krino: --json is not implemented yet") return 2 } @@ -39,106 +41,56 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int { exit := 0 printed := false + jsonDirs := []plan.JSONDir{} // never nil: the document's "dirs" must marshal as [], not null + // A3: one Claims for the whole run, shared across every directory's + // Plan call below, so two directories that both plan a move to the + // same destination resolve the collision at planning time instead of + // each independently believing it owns that path. + claims := plan.NewClaims() for _, d := range e.Dirs { if fi, err := os.Stat(d.Root); err != nil || !fi.IsDir() { fmt.Fprintf(stderr, "krino: skipping %s: %s is not a directory\n", d.Name, xdg.Abbrev(d.Root)) exit = 1 continue } - r, err := e.Match(context.Background(), d) + dp, err := e.Plan(context.Background(), d, claims) if err != nil { fmt.Fprintf(stderr, "krino: skipping %s: %v\n", d.Name, err) exit = 1 continue } - if printed { - fmt.Fprintln(stdout) + if !g.json { + if printed { + fmt.Fprintln(stdout) + } + printed = true + fmt.Fprintf(stdout, "krino: %s %s\n", d.Name, xdg.Abbrev(d.Root)) } - printed = true - fmt.Fprintf(stdout, "krino: %s %s\n", d.Name, xdg.Abbrev(d.Root)) // C3: directory-level warnings go to stderr after the header // line above, not before it, so on a terminal they read as // describing the directory just named instead of floating above it. - for _, w := range r.Warnings { + for _, w := range dp.Result.Warnings { fmt.Fprintf(stderr, "krino: %s: %s\n", d.Name, w) } - printResult(stdout, r, g.verbose) - } - return exit -} - -// printResult renders one directory's match result the way krino -n shows -// it, below the header line cmdSort has already written: a summary line, -// then the matched, warnings and skip-count sections, each present only -// when it has something to show. Plan 3 reuses this for the outcome of an -// actual run. -func printResult(w io.Writer, r *engine.Result, verbose bool) { - // C1: 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 - so scanned counts matched, unmatched and skipped - // alike, not just matched plus unmatched. - scanned := len(r.Matched) + len(r.Unmatched) + len(r.Skipped) - warned := 0 - for _, fm := range r.Matched { - if len(fm.Warnings) > 0 { - warned++ - } - } - for _, fm := range r.Unmatched { - if len(fm.Warnings) > 0 { - warned++ - } - } - fmt.Fprintf(w, "%d scanned · %d matched · %d warnings · %.2fs\n", scanned, len(r.Matched), warned, r.Elapsed.Seconds()) - - if len(r.Matched) > 0 { - fmt.Fprintln(w) - printMatched(w, r.Matched) - } - - if lines := collectWarnings(r); len(lines) > 0 { - fmt.Fprintln(w) - fmt.Fprintln(w, "warnings") - printWarnings(w, lines) - } - - if line := skipSummaryLine(r, verbose); line != "" { - fmt.Fprintln(w) - fmt.Fprintln(w, line) - } - - if verbose { - if len(r.Unmatched) > 0 { - fmt.Fprintln(w) - fmt.Fprintln(w, "not matched") - for _, fm := range r.Unmatched { - fmt.Fprintf(w, " %s\n", fm.File.Rel) - } - } - if len(r.Skipped) > 0 { - fmt.Fprintln(w) - fmt.Fprintln(w, "skipped") - printSkipped(w, r.Skipped) + if g.json { + jsonDirs = append(jsonDirs, plan.NewJSONDir(d.Name, d.Root, dp.Chains, dp.Result.Warnings)) + continue } + printPlan(stdout, dp, g.verbose) } -} -// printMatched lists each matched file, its Rel padded to the widest shown -// (capped at 40), then each matching rule as "name: reasons", rules joined -// by "; ". -func printMatched(w io.Writer, matched []engine.FileMatch) { - rels := make([]string, len(matched)) - for i, fm := range matched { - rels[i] = fm.File.Rel - } - width := relWidth(rels) - for _, fm := range matched { - parts := make([]string, len(fm.Rules)) - for i, rm := range fm.Rules { - parts[i] = rm.Rule.Name + ": " + strings.Join(rm.Reasons, ", ") + if g.json { + b, err := json.MarshalIndent(plan.NewJSON(jsonDirs), "", " ") + if err != nil { + // Unreachable in practice: every field the document carries + // marshals cleanly (strings, times, ints). + fmt.Fprintf(stderr, "krino: %v\n", err) + return 1 } - fmt.Fprintf(w, " %s %s\n", padCell(fm.File.Rel, width), strings.Join(parts, "; ")) + stdout.Write(b) + fmt.Fprintln(stdout) } + return exit } // warnLine is one file's warning, for the warnings section. @@ -152,22 +104,48 @@ type warnLine struct { // section by file name and has no way to tell which group a file fell // into, so grouping by match state is invisible structure that would only // show up as an odd order. A file's own warnings (when it has more than -// one) stay in the order they were recorded. -func collectWarnings(r *engine.Result) []warnLine { +// one) stay in the order they were recorded: its match warnings (if any) +// first, then its chain warnings (B2) - match happens before planning, so +// that is also the order they were actually produced in. chains supplies +// the chain-level warnings (e.g. "moved more than once"), keyed by +// Chain.File.Rel; every chain's file is necessarily also in r.Matched (only +// matched files ever reach plan.Build), so it is visited exactly once here. +func collectWarnings(r *engine.Result, chains []plan.Chain) []warnLine { files := make([]engine.FileMatch, 0, len(r.Matched)+len(r.Unmatched)) files = append(files, r.Matched...) files = append(files, r.Unmatched...) sort.Slice(files, func(i, j int) bool { return files[i].File.Rel < files[j].File.Rel }) + chainWarnings := make(map[string][]string, len(chains)) + for _, c := range chains { + if len(c.Warnings) > 0 { + chainWarnings[c.File.Rel] = c.Warnings + } + } + var out []warnLine for _, fm := range files { for _, w := range fm.Warnings { out = append(out, warnLine{fm.File.Rel, w}) } + for _, w := range chainWarnings[fm.File.Rel] { + out = append(out, warnLine{fm.File.Rel, w}) + } } return out } +// warnedCount counts the distinct files behind lines: B2's "N warnings" in +// the counts line must count a file once even when it carries both a match +// warning and a chain warning, not once per warning line. +func warnedCount(lines []warnLine) int { + seen := make(map[string]bool, len(lines)) + for _, l := range lines { + seen[l.rel] = true + } + return len(seen) +} + // printWarnings lists one line per warning, Rel padded to the widest shown // (capped at 40). func printWarnings(w io.Writer, lines []warnLine) { @@ -194,37 +172,83 @@ func printSkipped(w io.Writer, skipped []scan.Skipped) { } } -// skipReasonOrder is the order the last line reports skip reasons in, -// after "not matched". +// skipReasonOrder is plan 2's reviewed order for the skip reasons the last +// line reports, before "unmatched". var skipReasonOrder = []scan.Reason{scan.Ignored, scan.Busy, scan.TooNew, scan.Symlink, scan.NotRegular, scan.Unreadable} -// skipSummaryLine builds the "not matched: N · ignored: N ..." line, only -// the non-zero counts, or "" when every count is zero. -func skipSummaryLine(r *engine.Result, verbose bool) string { +// 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. +func skipSummaryLine(r *engine.Result, chains []plan.Chain, verbose bool) string { counts := map[scan.Reason]int{} for _, s := range r.Skipped { counts[s.Reason]++ } var parts []string - if n := len(r.Unmatched); n > 0 { - parts = append(parts, fmt.Sprintf("not matched: %d", n)) - } for _, reason := range skipReasonOrder { if n := counts[reason]; n > 0 { - parts = append(parts, fmt.Sprintf("%s: %d", reason.String(), n)) + parts = append(parts, fmt.Sprintf("%d %s", n, reason.String())) } } + // excluded and allSkipped (chainOutcomes) close the same arithmetic gap: + // without them, a file matching only an action-less rule, or one whose + // every step was skipped, is neither "to act on", unmatched, nor a walk + // skip, so it appears nowhere. + excluded, allSkipped := chainOutcomes(chains) + if excluded > 0 { + parts = append(parts, fmt.Sprintf("%d excluded", excluded)) + } + if allSkipped > 0 { + parts = append(parts, fmt.Sprintf("%d all steps skipped", allSkipped)) + } + if n := len(r.Unmatched); n > 0 { + parts = append(parts, fmt.Sprintf("%d unmatched", n)) + } if len(parts) == 0 { return "" } - line := strings.Join(parts, " · ") + line := "not acted on: " + strings.Join(parts, " · ") if !verbose { line += " (-v lists them)" } return line } +// chainOutcomes counts two of skipSummaryLine's categories over chains: +// excluded is chains with no steps at all (a file that matched only rules +// carrying no actions - spec §4.5: "a rule with only (stop) is an +// exclusion"); allSkipped is chains with at least one step, none of them +// unskipped (every step's Skip is set). Neither is "to act on", neither is +// unmatched, and neither is a walk skip, so without these two counts they +// appear nowhere: the real downloads folder reported "267 scanned · 172 to +// act on" and said nothing at all about the other 95. With them the +// arithmetic always closes - scanned = to act on + excluded + all steps +// skipped + unmatched + walk skips - as spec §8.2's own example does. +func chainOutcomes(chains []plan.Chain) (excluded, allSkipped int) { + for _, c := range chains { + if len(c.Steps) == 0 { + excluded++ + continue + } + acting := false + for _, s := range c.Steps { + if s.Skip == "" { + acting = true + break + } + } + if !acting { + allSkipped++ + } + } + return excluded, allSkipped +} + // relWidth returns the column width for a list of Rel names: the widest in // runes (C4: not bytes, or a name carrying diacritics misaligns its // column), capped at 40. diff --git a/docs/design.md b/docs/design.md index 888ab34..092738a 100644 --- a/docs/design.md +++ b/docs/design.md @@ -313,8 +313,19 @@ When the target already exists: A `copy` whose target has identical content is skipped as "already there", whatever the policy, so a backup rule can run every time. +A step whose target is the file itself — a rule whose `DEST` resolves to the +directory the file already sits in, or a `rename` to the name it already has — +is skipped as "already there" as well, whatever the policy. Without that the +file's own existence would read as a conflict with itself and the plan would +rename it to `stem_1` on every run. + Conflicts between files in the same plan are resolved when planning, so the -plan shows final names. The executor re-checks at execution time; if the +plan shows final names. An in-plan claim is never displaced: when `overwrite` +finds a target another step of the same plan has already claimed, it takes the +next free name instead, because that path will hold the other step's output by +the time either runs, and displacing it would destroy that step's result. The +claim set spans the whole run, so two configured directories cannot plan the +same final name. The executor re-checks at execution time; if the name has to change, the log records the actual name. ## 8. Plan, review, approval diff --git a/internal/cond/compile.go b/internal/cond/compile.go index 4fdc75f..590d61a 100644 --- a/internal/cond/compile.go +++ b/internal/cond/compile.go @@ -362,6 +362,28 @@ func (c *compiler) compileMatched(n *sexp.Node) *node { return &node{kind: kMatched, pos: n.Pos, label: "matched", cost: costCheap} } +// collectNameGroups walks n and its children — and and or included, not +// excluded — appending the capture-group count of every pattern of every +// kName node reachable without crossing a not, in compile order. B1: a +// name test under a not never supplies Result.Captures (eval.go's negated +// tracking takes captures only when !negated), so it must not count toward +// checkCaptures' "does some name test in this rule have enough groups" +// either - a rule combining a capturing name test with an unrelated +// (not (name ...)) must still pass. +func collectNameGroups(n *node, out *[]int) { + if n == nil || n.kind == kNot { + return + } + if n.kind == kName { + for _, p := range n.patterns { + *out = append(*out, p.re.NumSubexp()) + } + } + for _, ch := range n.children { + collectNameGroups(ch, out) + } +} + // quotedExampleErr records the shared "takes X in quotes: write (test // "arg")" diagnostic used by name, path, content and duplicate. func (c *compiler) quotedExampleErr(a *sexp.Node, test, noun string) { diff --git a/internal/cond/compile_test.go b/internal/cond/compile_test.go index e68a4d8..2a07ad6 100644 --- a/internal/cond/compile_test.go +++ b/internal/cond/compile_test.go @@ -124,6 +124,23 @@ func TestEmptyChildrenGuard(t *testing.T) { } } +// TestNameGroups: B1b - NameGroups() reports the capture-group count of +// every name test reachable without crossing a not, and skips one reachable +// only under a not (B1): here the outer (name ...) has two groups and the +// (not (name ...)) one has one, but the result must carry only the outer's +// count. +func TestNameGroups(t *testing.T) { + c, errs := Compile("d.conf", nodes(t, `(and (name "inv-(\d+)-(\d+)") (not (name "draft-(\d+)")))`), Options{}) + if len(errs) > 0 { + t.Fatal(errs) + } + got := c.NameGroups() + want := []int{2} + if !reflect.DeepEqual(got, want) { + t.Errorf("NameGroups() = %v, want %v", got, want) + } +} + func TestGroupsMatchSpec(t *testing.T) { want := map[string]string{ "image": "jpg jpeg png gif webp bmp tif tiff heic heif avif svg ico raw cr2 nef arw dng", diff --git a/internal/cond/types.go b/internal/cond/types.go index 81c2f45..9455f63 100644 --- a/internal/cond/types.go +++ b/internal/cond/types.go @@ -93,6 +93,18 @@ type node struct { dirs []string } +// NameGroups returns the number of capture groups of every name test in the +// condition that can ever supply captures, in compile order: a name test +// nested inside and/or counts however deep, but one inside a not does not +// (B1) - it can never be the source of Result.Captures, so it must not be +// asked to justify a rule's use of {N} either. Empty when the rule has no +// such name test. +func (c *Cond) NameGroups() []int { + var out []int + collectNameGroups(c.root, &out) + return out +} + // groups maps a (type ...) group name to the extensions it expands to, // spec Appendix A. var groups = map[string][]string{ diff --git a/internal/dup/dup.go b/internal/dup/dup.go index 502ef31..e3c7424 100644 --- a/internal/dup/dup.go +++ b/internal/dup/dup.go @@ -381,6 +381,47 @@ func readAt(f *os.File, off int64) ([]byte, error) { return buf[:n], nil } +// SameContent reports whether a and b hold identical content: a stat and +// size check first, then the same partial/full hash comparison Lookup uses +// for scanned candidates. Neither file needs to have been scanned or +// indexed; this is the one place content identity is decided, so callers +// outside this package must not hash a second way. +func SameContent(a, b string) (bool, error) { + ai, err := os.Stat(a) + if err != nil { + return false, err + } + bi, err := os.Stat(b) + if err != nil { + return false, err + } + if ai.Size() != bi.Size() { + return false, nil + } + + aPartial, err := computePartialHash(a) + if err != nil { + return false, err + } + bPartial, err := computePartialHash(b) + if err != nil { + return false, err + } + if aPartial != bPartial { + return false, nil + } + + aFull, err := computeFullHash(a) + if err != nil { + return false, err + } + bFull, err := computeFullHash(b) + if err != nil { + return false, err + } + return aFull == bFull, nil +} + // computeFullHash hashes the whole file. func computeFullHash(path string) ([sha256.Size]byte, error) { f, err := os.Open(path) diff --git a/internal/dup/dup_test.go b/internal/dup/dup_test.go index fb4e64d..5d2621c 100644 --- a/internal/dup/dup_test.go +++ b/internal/dup/dup_test.go @@ -262,6 +262,29 @@ func TestLookupFailsWhenSubjectUnreadable(t *testing.T) { } } +// TestSameContentEqualSizeDifferentContent: two files of identical size but +// different bytes must not be reported as the same content — the partial +// hash (not just the size check) has to separate them. +func TestSameContentEqualSizeDifferentContent(t *testing.T) { + d := t.TempDir() + a := filepath.Join(d, "a.bin") + b := filepath.Join(d, "b.bin") + one := []byte("acme-invoice-01") + two := []byte("acme-invoice-02") + if len(one) != len(two) { + t.Fatal("fixture bug: files must be the same size") + } + if err := os.WriteFile(a, one, 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(b, two, 0o644); err != nil { + t.Fatal(err) + } + if ok, err := SameContent(a, b); err != nil || ok { + t.Errorf("SameContent(a, b) = %v, %v; want false, nil", ok, err) + } +} + // fakeDirEntry is an fs.DirEntry whose Info() returns a canned result, for // exercising addEntry's Info()-failure handling directly (A3) — a real // filepath.WalkDir gives no hook to inject a stat failure deterministically diff --git a/internal/engine/engine.go b/internal/engine/engine.go index eec9a60..ea61e8c 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -6,6 +6,7 @@ package engine import ( + "fmt" "os" "time" @@ -13,6 +14,7 @@ import ( "krino/internal/config" "krino/internal/extract" "krino/internal/ignore" + "krino/internal/plan" ) // Engine holds a loaded, compiled configuration: everything a front end @@ -84,6 +86,10 @@ func Load(mainFile string, names ...string) (*Engine, []*config.Diag) { errs = append(errs, cerrs...) continue } + if diag := checkCaptures(d.File, r, c); diag != nil { + errs = append(errs, diag) + continue + } dir.Rules = append(dir.Rules, &Rule{Name: r.Name, Conf: r, Settings: rs, Cond: c}) } dir.ContentVariants = contentVariants(dir.Rules) @@ -102,6 +108,38 @@ func Load(mainFile string, names ...string) (*Engine, []*config.Diag) { }, nil } +// checkCaptures validates a compiled rule's actions against the capture +// groups its own name tests can supply (spec 7.3): a rule using {N} needs a +// name test at all, and every name test in it needs at least N groups. It +// reports only the first offending action, so one config mistake yields one +// diagnostic. +func checkCaptures(file string, r *config.Rule, c *cond.Cond) *config.Diag { + groups := c.NameGroups() + for _, a := range r.Actions { + n, err := plan.MaxIndex(a.Arg) + if err != nil || n == 0 { + continue + } + if len(groups) == 0 { + return &config.Diag{File: file, Pos: a.Pos, Msg: fmt.Sprintf("rule %q: {%d} needs a name test to capture from", r.Name, n)} + } + for _, g := range groups { + if g < n { + return &config.Diag{File: file, Pos: a.Pos, Msg: fmt.Sprintf("rule %q: {%d} but a name test has only %s", r.Name, n, captureGroups(g))} + } + } + } + return nil +} + +// captureGroups renders a capture-group count with correct singular/plural. +func captureGroups(n int) string { + if n == 1 { + return "1 capture group" + } + return fmt.Sprintf("%d capture groups", n) +} + // dedupeNames returns names with every repeat after its first occurrence // removed, order preserved. func dedupeNames(names []string) []string { diff --git a/internal/engine/engine_test.go b/internal/engine/engine_test.go index 5c0536f..8dcfcb3 100644 --- a/internal/engine/engine_test.go +++ b/internal/engine/engine_test.go @@ -138,6 +138,55 @@ func TestCheck(t *testing.T) { } } +// TestLoadRejectsUnsuppliedCaptures: a rule using {N} must be able to get it +// from its own name tests (spec 7.3) — adapted from the brief to this +// package's actual sandbox/writeConfig/Load helpers (writeConfig takes a +// main file body and a dirs map; there is no separate engineLoad, Load is +// called directly). +func TestLoadRejectsUnsuppliedCaptures(t *testing.T) { + tests := []struct{ rule, want string }{ + {`(rule "a" (when (type pdf)) (move "Work/{1}"))`, + `rule "a": {1} needs a name test to capture from`}, + {`(rule "a" (when (name "inv-(\d+)")) (move "Work/{2}"))`, + `rule "a": {2} but a name test has only 1 capture group`}, + {`(rule "a" (when (or (name "x-(\d+)-(\d+)") (name "y-(\d+)"))) (rename "{2}"))`, + `rule "a": {2} but a name test has only 1 capture group`}, + } + for _, tt := range tests { + h := sandbox(t) + main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": `(path "/tmp") +` + tt.rule}) + _, errs := Load(main, "dl") + joined := "" + for _, d := range errs { + joined += d.Error() + "\n" + } + if len(errs) != 1 || !strings.Contains(joined, tt.want) { + t.Errorf("rule %s: errs %v, want %q", tt.rule, errs, tt.want) + } + } +} + +// TestLoadAcceptsSuppliedCaptures: a rule whose name test has enough groups +// for every {N} it uses loads clean. +func TestLoadAcceptsSuppliedCaptures(t *testing.T) { + for _, rule := range []string{ + `(rule "a" (when (name "inv-(\d+)-(\d+)")) (move "Work/{2}/{1}"))`, + // B1a: a name test reachable only under a (not ...) must not count + // toward this rule's own captures (B1), but it must also not make + // the rule itself invalid - the outer (name ...) alone already + // supplies the two groups {2} needs. + `(rule "a" (when (and (name "inv-(\d+)-(\d+)") (not (name "draft")))) (move "Work/{2}"))`, + } { + h := sandbox(t) + main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": `(path "/tmp") +` + rule}) + if _, errs := Load(main, "dl"); len(errs) != 0 { + t.Errorf("rule %s: unexpected diagnostics: %v", rule, errs) + } + } +} + // TestContentVariantsComputedAtLoad: B2 plumbing. Load computes each // directory's distinct (ignoreCase, fold) content-test variants from its // rules' resolved settings: a rule with no content test contributes diff --git a/internal/engine/facts.go b/internal/engine/facts.go index 60380f3..423bfd9 100644 --- a/internal/engine/facts.go +++ b/internal/engine/facts.go @@ -13,6 +13,7 @@ import ( "krino/internal/cond" "krino/internal/dup" "krino/internal/norm" + "krino/internal/plan" "krino/internal/scan" "krino/internal/xdg" ) @@ -166,7 +167,7 @@ func (f *facts) Duplicate(dirs []string) (string, bool, error) { root := f.run.d.Root resolved := make([]string, len(dirs)) for i, raw := range dirs { - resolved[i] = resolveDir(raw, root) + resolved[i] = plan.ResolveDir(raw, root) } sorted := append([]string(nil), resolved...) sort.Strings(sorted) @@ -183,16 +184,6 @@ func (f *facts) Duplicate(dirs []string) (string, bool, error) { return displayOriginal(orig, root), true, nil } -// resolveDir expands a leading ~ and joins a relative directory to root, -// cleaned. -func resolveDir(raw, root string) string { - p := xdg.Expand(raw) - if !filepath.IsAbs(p) { - p = filepath.Join(root, p) - } - return filepath.Clean(p) -} - // displayOriginal reports orig relative to root when it lies inside root, // else as an absolute path. func displayOriginal(orig, root string) string { diff --git a/internal/engine/match.go b/internal/engine/match.go index 0693bd5..e68a6e8 100644 --- a/internal/engine/match.go +++ b/internal/engine/match.go @@ -16,6 +16,7 @@ import ( "krino/internal/cond" "krino/internal/config" + "krino/internal/plan" "krino/internal/scan" "krino/internal/xdg" ) @@ -312,7 +313,12 @@ func isBusy(path string, suffixes []string) bool { // excludeDirs computes the directories Match and Explain never enter: each // rule's copy/move destination, the Trash, and the directory holding the -// main config file - each kept only when it lies strictly inside d's root. +// main config file - each kept only when it lies strictly inside d's root +// (C3: root itself is not "inside" it here - a rule cannot exclude the very +// directory being scanned. cmd/krino/render.go's relToRoot answers a +// different question, whether a destination is root or beneath it for +// display purposes, and there root does count as inside; the two are each +// correct for their own question, so do not "unify" them). // A destination with no template placeholder excludes exactly that // directory; a destination with a placeholder excludes only the static // part before its first "{", cut back to a full path component (its last @@ -348,7 +354,7 @@ func (e *Engine) excludeDirs(d *Dir) []string { prefix = "" } } - add(resolveDir(prefix, root)) + add(plan.ResolveDir(prefix, root)) } } add(filepath.Join(xdg.DataHome(), "Trash")) diff --git a/internal/engine/plan.go b/internal/engine/plan.go new file mode 100644 index 0000000..979bec1 --- /dev/null +++ b/internal/engine/plan.go @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package engine + +import ( + "context" + "time" + + "krino/internal/plan" +) + +// DirPlan is one directory's plan: the match Result it was built from (for +// the counts and warnings a front end renders), the chains plan.Build +// produced from its matched files, and Elapsed (D12), which - unlike +// Result.Elapsed, stopped inside Match before plan.Build ever runs - spans +// both Match and Build, so a front end reporting how long planning took is +// not undercounting it. It lives here, not in internal/plan, because +// internal/plan must not import internal/engine (engine already imports +// plan). +type DirPlan struct { + Dir *Dir + Chains []plan.Chain + Result *Result + Elapsed time.Duration +} + +// Plan matches d, then turns every matched file into a chain per spec +// §7.1. Unmatched files never reach plan.Build: they have no rules, hence +// no steps, and Result.Unmatched already says they were not matched. +// Chains keep Match's Rel order, since Matched is already sorted that way +// and Build's output lines up with its input position for position. claims +// is the run-scoped claim set (A3): the caller creates one *plan.Claims per +// run and passes the same instance to every directory's Plan call, so two +// directories claiming one destination resolve the collision instead of +// both silently landing on it. +func (e *Engine) Plan(ctx context.Context, d *Dir, claims *plan.Claims) (*DirPlan, error) { + started := time.Now() + r, err := e.Match(ctx, d) + if err != nil { + return nil, err + } + + inputs := make([]plan.Input, len(r.Matched)) + for i, fm := range r.Matched { + rules := make([]plan.RuleMatch, len(fm.Rules)) + for j, rm := range fm.Rules { + rules[j] = plan.RuleMatch{ + Name: rm.Rule.Name, + Actions: rm.Rule.Conf.Actions, + Settings: rm.Rule.Settings, + Captures: rm.Captures, + Reasons: rm.Reasons, + } + } + inputs[i] = plan.Input{File: fm.File, Rules: rules} + } + + chains := plan.Build(d.Root, inputs, e.Now(), plan.OS{}, claims) + return &DirPlan{Dir: d, Chains: chains, Result: r, Elapsed: time.Since(started)}, nil +} diff --git a/internal/engine/plan_test.go b/internal/engine/plan_test.go new file mode 100644 index 0000000..a169168 --- /dev/null +++ b/internal/engine/plan_test.go @@ -0,0 +1,146 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package engine + +import ( + "context" + "fmt" + "os" + "path/filepath" + "testing" + "time" + + "krino/internal/plan" +) + +func TestPlanBuildsChainsFromMatchedFiles(t *testing.T) { + h := sandbox(t) + os.Mkdir(filepath.Join(h, "dl"), 0o755) + old := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + for _, n := range []string{"z.pdf", "a.pdf", "notes.txt"} { + p := filepath.Join(h, "dl", n) + if err := os.WriteFile(p, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(p, old, old); err != nil { + t.Fatal(err) + } + } + main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": ` +(path "~/dl") +(min-age 0s) +(rule "pdfs" (when (type pdf)) (move "PDF")) +`}) + e, errs := Load(main, "dl") + if len(errs) > 0 { + t.Fatal(errs) + } + + dp, err := e.Plan(context.Background(), e.Dirs[0], plan.NewClaims()) + if err != nil { + t.Fatal(err) + } + if dp.Dir != e.Dirs[0] { + t.Errorf("Dir = %v, want %v", dp.Dir, e.Dirs[0]) + } + if dp.Result == nil || len(dp.Result.Matched) != 2 || len(dp.Result.Unmatched) != 1 { + t.Fatalf("Result = %+v", dp.Result) + } + + // Chains keep Match's Rel order: "a.pdf" before "z.pdf", regardless of + // walk or map-iteration order. + if len(dp.Chains) != 2 { + t.Fatalf("got %d chains, want 2 (unmatched notes.txt must not appear)", len(dp.Chains)) + } + if dp.Chains[0].File.Rel != "a.pdf" || dp.Chains[1].File.Rel != "z.pdf" { + t.Fatalf("chains not in Rel order: %s, %s", dp.Chains[0].File.Rel, dp.Chains[1].File.Rel) + } + + c := dp.Chains[0] + if len(c.Steps) != 1 { + t.Fatalf("a.pdf steps = %+v", c.Steps) + } + s := c.Steps[0] + want := filepath.Join(h, "dl", "PDF", "a.pdf") + if s.Kind != plan.Move || s.Rule != "pdfs" || s.Dst != want || s.Skip != "" { + t.Errorf("step = %+v, want Move to %s", s, want) + } +} + +// TestPlanSharesClaimsAcrossDirectories: A3 - plan.Claims is created once +// by the caller and threaded through every Engine.Plan call of a run, so +// two configured directories cannot plan the same final name. Two +// directories each hold a file named "x.txt" and each carry a rule moving +// it to one shared destination outside both roots; calling Plan for both +// with the *same* claims must resolve the second directory's step onto +// "x_1.txt" rather than let it collide on "x.txt" too. +func TestPlanSharesClaimsAcrossDirectories(t *testing.T) { + h := sandbox(t) + shared := filepath.Join(h, "elsewhere") + for _, d := range []string{"d1", "d2"} { + if err := os.Mkdir(filepath.Join(h, d), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(h, d, "x.txt"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + } + dirConf := func(path string) string { + return fmt.Sprintf(`(path %q) +(min-age 0s) +(rule "r" (when (type txt)) (move %q)) +`, path, shared) + } + main := writeConfig(t, h, `(include "d1" "d2")`, map[string]string{ + "d1": dirConf("~/d1"), + "d2": dirConf("~/d2"), + }) + e, errs := Load(main, "d1", "d2") + if len(errs) > 0 { + t.Fatal(errs) + } + + claims := plan.NewClaims() + dp1, err := e.Plan(context.Background(), e.Dirs[0], claims) + if err != nil { + t.Fatal(err) + } + dp2, err := e.Plan(context.Background(), e.Dirs[1], claims) + if err != nil { + t.Fatal(err) + } + + if len(dp1.Chains) != 1 || len(dp1.Chains[0].Steps) != 1 { + t.Fatalf("d1 chains = %+v", dp1.Chains) + } + if len(dp2.Chains) != 1 || len(dp2.Chains[0].Steps) != 1 { + t.Fatalf("d2 chains = %+v", dp2.Chains) + } + got1 := dp1.Chains[0].Steps[0].Dst + got2 := dp2.Chains[0].Steps[0].Dst + want1 := filepath.Join(shared, "x.txt") + want2 := filepath.Join(shared, "x_1.txt") + if got1 != want1 { + t.Errorf("d1 dst = %s, want %s", got1, want1) + } + if got2 != want2 { + t.Errorf("d2 dst = %s, want %s (claims must be shared with d1's plan)", got2, want2) + } +} + +func TestPlanReturnsMatchError(t *testing.T) { + h := sandbox(t) + main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": ` +(path "~/dl") +(rule "r" (when (type pdf)) (move "PDF")) +`}) + e, errs := Load(main, "dl") + if len(errs) > 0 { + t.Fatal(errs) + } + // dl's root ~/dl does not exist: scan.Walk fails, and Plan must + // propagate that error rather than paper over it with an empty plan. + if _, err := e.Plan(context.Background(), e.Dirs[0], plan.NewClaims()); err == nil { + t.Error("want an error when the root does not exist, got nil") + } +} diff --git a/internal/plan/chain.go b/internal/plan/chain.go new file mode 100644 index 0000000..bfa2484 --- /dev/null +++ b/internal/plan/chain.go @@ -0,0 +1,222 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "fmt" + "path/filepath" + "sort" + "strings" + "time" + + "krino/internal/config" + "krino/internal/scan" + "krino/internal/xdg" +) + +// Claims is the set of destination paths already spoken for, shared across +// every directory planned in one run (A3): two directories competing for +// one destination must resolve the collision at planning time, which needs +// one Claims threaded through every Build call of that run, not a fresh one +// per call. +type Claims struct { + taken claimed +} + +// NewClaims returns an empty Claims, ready to pass to Build. +func NewClaims() *Claims { + return &Claims{taken: claimed{}} +} + +// Input is one file and the rules that matched it, in match order. +type Input struct { + File scan.File + Rules []RuleMatch +} + +// Build turns each file's matching rules into a chain. root is the +// directory's absolute root; now is the start of the run; d is consulted to +// resolve destination conflicts (§7.4). claims is the run-scoped claim set +// (A3): pass the same *Claims to every Build call of one run (every +// directory included) so two directories claiming one destination resolve +// the collision instead of both silently landing on it. claims must not be +// nil - a caller with nothing to share yet still calls NewClaims() itself, +// so an accidentally-unshared claim set can never happen by simply +// forgetting the argument. Files whose chain has no steps are returned with +// an empty Steps slice, so the caller can tell "matched a rule that does +// nothing" from "not matched". +// +// Conflicts are resolved as each step is created, and a claimed destination +// is shared across every file's chain, not just its own: two files +// competing for one name must resolve the same way on every run, so +// resolution proceeds in File.Rel order regardless of the order in which +// in is given. The returned slice still lines up with in, position for +// position. +func Build(root string, in []Input, now time.Time, d Disk, claims *Claims) []Chain { + if claims == nil { + panic("plan: Build requires a non-nil Claims (see NewClaims)") + } + order := make([]int, len(in)) + for i := range order { + order[i] = i + } + sort.SliceStable(order, func(i, j int) bool { + return in[order[i]].File.Rel < in[order[j]].File.Rel + }) + + chains := make([]Chain, len(in)) + for _, i := range order { + chains[i] = buildOne(root, in[i], now, d, claims.taken) + } + return chains +} + +// buildOne builds the chain for a single file. claim is shared with every +// other file processed by the same Build call. +func buildOne(root string, in Input, now time.Time, d Disk, claim claimed) Chain { + c := Chain{File: in.File} + cur := in.File.Path + + var deletedBy string + moves := 0 + warnedMove := false + + for _, rule := range in.Rules { + reason := strings.Join(rule.Reasons, ", ") + for _, a := range rule.Actions { + kind := stepKind(a.Kind) + + if deletedBy != "" { + c.Steps = append(c.Steps, Step{ + Kind: kind, + Rule: rule.Name, + Src: cur, + Reason: reason, + Skip: "deleted by rule " + deletedBy, + Conflict: rule.Settings.OnConflict, + }) + continue + } + + facts := Facts{ + Name: filepath.Base(cur), + Captures: rule.Captures, + ModTime: in.File.ModTime, + Now: now, + } + + step := Step{Kind: kind, Rule: rule.Name, Src: cur, Reason: reason, Conflict: rule.Settings.OnConflict} + + switch a.Kind { + case config.Copy, config.Move: + // D9: a placeholder failure (below) leaves step.Dst empty - + // there was never a destination to compute at all - while a + // conflict-policy skip (via resolveConflict, right after) + // always sets step.Dst: even when the step will not run, + // its would-be destination is a real, already-resolved + // path worth showing. + dest, err := expandDir(a.Arg, facts, root) + if err != nil { + step.Skip = err.Error() + break + } + dst := filepath.Join(dest, filepath.Base(cur)) + resolved, skip, displaces := resolveConflict(kind, rule.Settings.OnConflict, cur, dst, d, claim) + step.Dst = resolved + step.Skip = skip + step.Displaces = displaces + if skip == "" { + // C4: the path cur is about to be vacated from (on a + // move) enters neither claim nor any "freed" set, so + // Disk.Exists still reports it occupied for the rest of + // this plan and a later file wanting that exact name + // gets a gratuitous _1. This errs safe - it never lets + // a name be claimed before its file has actually + // vacated it - and stays; do not "fix" it by weakening + // the disk check. + claim[resolved] = true + if a.Kind == config.Move { + cur = resolved + moves++ + if moves > 1 && !warnedMove { + c.Warnings = append(c.Warnings, "moved more than once; a (stop) is probably missing") + warnedMove = true + } + } + } + + case config.Rename: + name, err := Expand(a.Arg, facts) + if err != nil { + step.Skip = err.Error() + break + } + if strings.ContainsRune(name, '/') { + step.Skip = `rename produced a name containing "/"` + break + } + dst := filepath.Join(filepath.Dir(cur), name) + resolved, skip, displaces := resolveConflict(kind, rule.Settings.OnConflict, cur, dst, d, claim) + step.Dst = resolved + step.Skip = skip + step.Displaces = displaces + if skip == "" { + cur = resolved + claim[resolved] = true + } + + case config.Delete, config.DeletePermanent: + deletedBy = rule.Name + } + + c.Steps = append(c.Steps, step) + } + } + + return c +} + +// stepKind maps a config.ActionKind onto its plan.Kind, exhaustively +// (D4): an unrecognised ActionKind panics rather than silently reading as +// config.Delete, matching config.ActionKind.String()'s own exhaustive style +// with an explicit fallback. +func stepKind(k config.ActionKind) Kind { + switch k { + case config.Copy: + return Copy + case config.Move: + return Move + case config.Rename: + return Rename + case config.DeletePermanent: + return DeletePermanent + case config.Delete: + return Trash + } + panic(fmt.Sprintf("plan: unknown config.ActionKind %d", int(k))) +} + +// expandDir expands raw (a DEST argument) against facts, then resolves it +// the same way the engine resolves an extra directory: ~ expands, a +// relative path joins root, and the result is cleaned. +func expandDir(raw string, facts Facts, root string) (string, error) { + expanded, err := Expand(raw, facts) + if err != nil { + return "", err + } + return ResolveDir(expanded, root), nil +} + +// ResolveDir expands a leading ~ and joins a relative directory to root, +// cleaned. C1: this is the one place that decides where a rule's +// destination resolves to; internal/engine calls it too (its own directory +// walk needs to agree on the same paths), rather than keeping a second, +// separately-maintained copy - plan is the lower layer (engine imports +// plan, so plan must never import engine), so the decision belongs here. +func ResolveDir(raw, root string) string { + p := xdg.Expand(raw) + if !filepath.IsAbs(p) { + p = filepath.Join(root, p) + } + return filepath.Clean(p) +} diff --git a/internal/plan/chain_test.go b/internal/plan/chain_test.go new file mode 100644 index 0000000..b6e3dfe --- /dev/null +++ b/internal/plan/chain_test.go @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "path/filepath" + "testing" + "time" + + "krino/internal/config" + "krino/internal/scan" +) + +func file(root, rel string) scan.File { + return scan.File{ + Path: filepath.Join(root, rel), + Rel: rel, + Name: filepath.Base(rel), + Size: 10, + ModTime: time.Date(2026, 8, 15, 12, 0, 0, 0, time.UTC), + } +} + +func act(k config.ActionKind, arg string) config.Action { return config.Action{Kind: k, Arg: arg} } + +func TestBuildChain(t *testing.T) { + root := "/r" + now := time.Date(2026, 9, 12, 0, 0, 0, 0, time.UTC) + in := []Input{{ + File: file(root, "inv1.pdf"), + Rules: []RuleMatch{ + {Name: "backup", Actions: []config.Action{act(config.Copy, "/backup/{mtime:%Y}")}}, + {Name: "acme", Actions: []config.Action{ + act(config.Rename, "{mtime:%Y-%m-%d}_{name}"), + act(config.Move, "Work/Acme/{mtime:%Y}"), + }}, + }, + }} + got := Build(root, in, now, NoDisk{}, NewClaims()) + if len(got) != 1 || len(got[0].Steps) != 3 { + t.Fatalf("got %d chains / %d steps", len(got), len(got[0].Steps)) + } + want := []Step{ + {Kind: Copy, Rule: "backup", Src: "/r/inv1.pdf", Dst: "/backup/2026/inv1.pdf"}, + {Kind: Rename, Rule: "acme", Src: "/r/inv1.pdf", Dst: "/r/2026-08-15_inv1.pdf"}, + {Kind: Move, Rule: "acme", Src: "/r/2026-08-15_inv1.pdf", Dst: "/r/Work/Acme/2026/2026-08-15_inv1.pdf"}, + } + for i, w := range want { + g := got[0].Steps[i] + if g.Kind != w.Kind || g.Rule != w.Rule || g.Src != w.Src || g.Dst != w.Dst || g.Skip != "" { + t.Errorf("step %d = %+v; want %+v", i, g, w) + } + } + if len(got[0].Warnings) != 0 { + t.Errorf("unexpected warnings: %v", got[0].Warnings) + } +} + +func TestBuildDeleteEndsChain(t *testing.T) { + in := []Input{{ + File: file("/r", "old.iso"), + Rules: []RuleMatch{ + {Name: "dups", Actions: []config.Action{act(config.Delete, "")}}, + {Name: "archive", Actions: []config.Action{act(config.Move, "Archive")}}, + }, + }} + steps := Build("/r", in, time.Now(), NoDisk{}, NewClaims())[0].Steps + if len(steps) != 2 || steps[0].Kind != Trash || steps[0].Dst != "" { + t.Fatalf("steps = %+v", steps) + } + if steps[1].Skip != "deleted by rule dups" { + t.Errorf("step after delete: Skip = %q", steps[1].Skip) + } +} + +func TestBuildWarnsOnTwoMoves(t *testing.T) { + in := []Input{{ + File: file("/r", "x.pdf"), + Rules: []RuleMatch{ + {Name: "a", Actions: []config.Action{act(config.Move, "A")}}, + {Name: "b", Actions: []config.Action{act(config.Move, "B")}}, + }, + }} + c := Build("/r", in, time.Now(), NoDisk{}, NewClaims())[0] + if len(c.Warnings) != 1 || c.Warnings[0] != "moved more than once; a (stop) is probably missing" { + t.Errorf("warnings = %v", c.Warnings) + } + if c.Steps[1].Src != "/r/A/x.pdf" { + t.Errorf("second move reads from %q, want the path after the first move", c.Steps[1].Src) + } +} + +func TestBuildBadPlaceholderSkipsOneStep(t *testing.T) { + in := []Input{{ + File: file("/r", "x.pdf"), + Rules: []RuleMatch{{Name: "a", Actions: []config.Action{ + act(config.Move, "Work/{1}"), + act(config.Rename, "ok-{name}"), + }}}, + }} + c := Build("/r", in, time.Now(), NoDisk{}, NewClaims())[0] + if c.Steps[0].Skip == "" { + t.Errorf("step with {1} and no captures should be skipped: %+v", c.Steps[0]) + } + if c.Steps[1].Skip != "" || c.Steps[1].Dst != "/r/ok-x.pdf" { + t.Errorf("chain should continue from the unchanged path: %+v", c.Steps[1]) + } +} + +func TestBuildRenameWithSlash(t *testing.T) { + in := []Input{{ + File: file("/r", "x.pdf"), + Rules: []RuleMatch{{Name: "a", Actions: []config.Action{act(config.Rename, "sub/{name}")}}}, + }} + c := Build("/r", in, time.Now(), NoDisk{}, NewClaims())[0] + if c.Steps[0].Skip != `rename produced a name containing "/"` { + t.Errorf("Skip = %q", c.Steps[0].Skip) + } +} + +// 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 +// "to act on" count and the JSON document's empty steps array both rest on +// this. +func TestBuildKeepsSteplessChains(t *testing.T) { + in := []Input{ + {File: file("/r", "excluded.txt"), Rules: []RuleMatch{{Name: "only-stop"}}}, + {File: file("/r", "untouched.txt")}, + } + chains := Build("/r", in, time.Now(), NoDisk{}, NewClaims()) + if len(chains) != 2 { + t.Fatalf("got %d chains, want one per Input", len(chains)) + } + for i, c := range chains { + if len(c.Steps) != 0 { + t.Errorf("chain %d: got %d steps, want none", i, len(c.Steps)) + } + if c.File.Rel != in[i].File.Rel { + t.Errorf("chain %d: File.Rel = %q, want %q (positional alignment)", i, c.File.Rel, in[i].File.Rel) + } + } +} diff --git a/internal/plan/conflict.go b/internal/plan/conflict.go new file mode 100644 index 0000000..8747669 --- /dev/null +++ b/internal/plan/conflict.go @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "fmt" + "os" + "path/filepath" + + "krino/internal/config" + "krino/internal/dup" +) + +// Disk is what Build needs from the filesystem to resolve conflicts, so +// tests can supply a stub and Build stays pure otherwise. +type Disk interface { + Exists(path string) bool + SameContent(a, b string) (bool, error) +} + +// OS is the real filesystem. +type OS struct{} + +// Exists reports whether path names an existing file or directory, without +// following a symlink at path itself: a dangling or otherwise unwanted +// symlink still counts as "something is there". +func (OS) Exists(path string) bool { + _, err := os.Lstat(path) + return err == nil +} + +// SameContent delegates to internal/dup, the one place content identity is +// decided. +func (OS) SameContent(a, b string) (bool, error) { + return dup.SameContent(a, b) +} + +// NoDisk is the empty filesystem: nothing exists, and nothing is ever the +// same content. For tests that are not about conflicts. +type NoDisk struct{} + +func (NoDisk) Exists(string) bool { return false } +func (NoDisk) SameContent(string, string) (bool, error) { return false, nil } + +// claimed is the set of destination paths already spoken for by an earlier +// step of this plan. +type claimed map[string]bool + +// resolveConflict decides what a step whose destination is contested does, +// per spec §7.4. src is the step's source (its current path, before this +// step runs); dst is the target the action computed. It returns the +// resolved destination (possibly unchanged), a Skip reason (non-empty when +// 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. + if dst == src { + return dst, "already there", "" + } + + onDisk := d.Exists(dst) + + if kind == Copy && onDisk { + // A SameContent error (the source vanished, a permission problem, + // ...) is treated the same as "different content": the step falls + // 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. + if same, err := d.SameContent(src, dst); err == nil && same { + return dst, "already there", "" + } + } + + if !onDisk && !c[dst] { + return dst, "", "" + } + + switch policy { + case config.ConflictSkip: + return dst, "target exists", "" + case config.ConflictOverwrite: + if onDisk && !c[dst] { + // 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. + return dst, "", dst + } + // Either claimed in-plan only (nothing on disk to displace — an + // in-plan claim is never displaced), or on disk but already + // claimed by an earlier step of this plan (displacing it again + // would destroy that step's output): either way the two chains + // cannot share one destination, so fall back to a free name, + // exactly as suffix would. A step that takes a free name + // displaces nothing. + resolved, skip := suffixed(dst, d, c) + return resolved, skip, "" + default: // config.ConflictSuffix + resolved, skip := suffixed(dst, d, c) + return resolved, skip, "" + } +} + +// 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. +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). +func suffixed(dst string, d Disk, c claimed) (resolved, skip string) { + dir, base := filepath.Split(dst) + stem, ext := splitExt(base) + for n := 1; n <= maxSuffixAttempts; n++ { + candidate := filepath.Join(dir, fmt.Sprintf("%s_%d%s", stem, n, ext)) + if !d.Exists(candidate) && !c[candidate] { + return candidate, "" + } + } + return "", "too many conflicting names" +} diff --git a/internal/plan/conflict_test.go b/internal/plan/conflict_test.go new file mode 100644 index 0000000..bdc37af --- /dev/null +++ b/internal/plan/conflict_test.go @@ -0,0 +1,224 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "os" + "path/filepath" + "testing" + "time" + + "krino/internal/config" +) + +// fakeDisk reports the paths it was given as existing, and equality of +// content by exact string match on a separate map. +type fakeDisk struct { + exists map[string]bool + same map[[2]string]bool +} + +func (f fakeDisk) Exists(p string) bool { return f.exists[p] } +func (f fakeDisk) SameContent(a, b string) (bool, error) { + return f.same[[2]string{a, b}], nil +} + +// TestConflictAlreadyThereMove is A1: a move whose destination resolves to +// the directory the file is already sitting in must be a no-op, not a +// rename - probe: "(rule "byyear" (when (type txt)) (move "{mtime:%Y}"))" +// over "dl/2026/a.txt" planned "a.txt -> a_1.txt" on run 1 and, on run 2, +// both "a.txt -> a_2.txt" and "a_1.txt -> a_1_1.txt": every run renamed the +// whole destination tree and added a generation, because the file's own +// existence at dst read as a conflict with itself. +func TestConflictAlreadyThereMove(t *testing.T) { + d := fakeDisk{exists: map[string]bool{"/r/2026/a.txt": true}} + in := []Input{{File: file("/r", "2026/a.txt"), Rules: []RuleMatch{ + {Name: "byyear", Actions: []config.Action{act(config.Move, "2026")}}}}} + s := Build("/r", in, time.Now(), d, NewClaims())[0].Steps[0] + if s.Dst != "/r/2026/a.txt" || s.Skip != "already there" || s.Displaces != "" { + t.Errorf("step = %+v; want a no-op at the file's own path", s) + } +} + +// TestConflictAlreadyThereRename is A1's rename counterpart: +// (rename "{name}") over a file already named that must also be a no-op. +func TestConflictAlreadyThereRename(t *testing.T) { + d := fakeDisk{exists: map[string]bool{"/r/2026/a.txt": true}} + in := []Input{{File: file("/r", "2026/a.txt"), Rules: []RuleMatch{ + {Name: "byyear", Actions: []config.Action{act(config.Rename, "{name}")}}}}} + s := Build("/r", in, time.Now(), d, NewClaims())[0].Steps[0] + if s.Dst != "/r/2026/a.txt" || s.Skip != "already there" || s.Displaces != "" { + t.Errorf("step = %+v; want a no-op at the file's own path", s) + } +} + +// TestConflictAlreadyThereOverwriteNoDisplace is A2: the same already-there +// case under (on-conflict overwrite) must not record the file as its own +// Displaces - spec §7.4's overwrite moves the existing target to Trash +// first, then proceeds; applied literally here that trashes the user's file +// and then moves from a path that no longer exists, so the file survives +// only in Trash. A1's guard (dst == src, checked before the policy switch) +// fixes this too, since it runs before overwrite's own branch is ever +// reached. +func TestConflictAlreadyThereOverwriteNoDisplace(t *testing.T) { + d := fakeDisk{exists: map[string]bool{"/r/2026/a.txt": true}} + over := config.ConflictOverwrite + in := []Input{{File: file("/r", "2026/a.txt"), Rules: []RuleMatch{ + {Name: "byyear", Settings: config.Resolved{OnConflict: over}, Actions: []config.Action{act(config.Move, "2026")}}}}} + s := Build("/r", in, time.Now(), d, NewClaims())[0].Steps[0] + if s.Skip != "already there" || s.Displaces != "" { + t.Errorf("step = %+v; want Skip \"already there\" and empty Displaces", s) + } +} + +func TestConflictSuffix(t *testing.T) { + d := fakeDisk{exists: map[string]bool{"/r/Work/x.pdf": true, "/r/Work/x_1.pdf": true}} + in := []Input{{File: file("/r", "x.pdf"), Rules: []RuleMatch{ + {Name: "a", Actions: []config.Action{act(config.Move, "Work")}}}}} + s := Build("/r", in, time.Now(), d, NewClaims())[0].Steps[0] + if s.Dst != "/r/Work/x_2.pdf" || s.Skip != "" { + t.Errorf("step = %+v; want Dst /r/Work/x_2.pdf", s) + } +} + +func TestConflictSkipKeepsCurrentPath(t *testing.T) { + d := fakeDisk{exists: map[string]bool{"/r/Work/x.pdf": true}} + skip := config.ConflictSkip + in := []Input{{File: file("/r", "x.pdf"), Rules: []RuleMatch{ + {Name: "a", Settings: config.Resolved{OnConflict: skip}, Actions: []config.Action{ + act(config.Move, "Work"), + act(config.Rename, "later-{name}"), + }}}}} + steps := Build("/r", in, time.Now(), d, NewClaims())[0].Steps + if steps[0].Skip != "target exists" { + t.Errorf("skip step = %+v", steps[0]) + } + if steps[1].Src != "/r/x.pdf" || steps[1].Dst != "/r/later-x.pdf" { + t.Errorf("chain must continue from the unchanged path: %+v", steps[1]) + } +} + +func TestConflictOverwriteRecordsDisplaced(t *testing.T) { + d := fakeDisk{exists: map[string]bool{"/r/Work/x.pdf": true}} + over := config.ConflictOverwrite + in := []Input{{File: file("/r", "x.pdf"), Rules: []RuleMatch{ + {Name: "a", Settings: config.Resolved{OnConflict: over}, Actions: []config.Action{act(config.Move, "Work")}}}}} + s := Build("/r", in, time.Now(), d, NewClaims())[0].Steps[0] + if s.Dst != "/r/Work/x.pdf" || s.Displaces != "/r/Work/x.pdf" { + t.Errorf("step = %+v", s) + } +} + +// TestConflictOverwriteTwoFilesOnDisk: two files collide on one path that +// already exists on disk, both under overwrite. Only the first may displace +// the pre-existing file; the second must take a free name and displace +// nothing, or applying both later would trash the first file's own output. +func TestConflictOverwriteTwoFilesOnDisk(t *testing.T) { + d := fakeDisk{exists: map[string]bool{"/r/Work/x.pdf": true}} + over := config.ConflictOverwrite + in := []Input{ + {File: file("/r", "a/x.pdf"), Rules: []RuleMatch{ + {Name: "r", Settings: config.Resolved{OnConflict: over}, Actions: []config.Action{act(config.Move, "Work")}}}}, + {File: file("/r", "b/x.pdf"), Rules: []RuleMatch{ + {Name: "r", Settings: config.Resolved{OnConflict: over}, Actions: []config.Action{act(config.Move, "Work")}}}}, + } + chains := Build("/r", in, time.Now(), d, NewClaims()) + first, second := chains[0].Steps[0], chains[1].Steps[0] + if first.Dst != "/r/Work/x.pdf" || first.Displaces != "/r/Work/x.pdf" { + t.Errorf("first step = %+v; want Dst and Displaces /r/Work/x.pdf", first) + } + if second.Dst != "/r/Work/x_1.pdf" || second.Displaces != "" { + t.Errorf("second step = %+v; want Dst /r/Work/x_1.pdf and empty Displaces", second) + } +} + +// TestConflictOverwriteTwoFilesClaimedOnly: same collision, but nothing is +// on disk — the two files claim the same name only in-plan. Neither may +// displace (an in-plan claim is never displaced); the second must fall back +// to a free name with no Displaces recorded. +func TestConflictOverwriteTwoFilesClaimedOnly(t *testing.T) { + over := config.ConflictOverwrite + in := []Input{ + {File: file("/r", "a/x.pdf"), Rules: []RuleMatch{ + {Name: "r", Settings: config.Resolved{OnConflict: over}, Actions: []config.Action{act(config.Move, "Work")}}}}, + {File: file("/r", "b/x.pdf"), Rules: []RuleMatch{ + {Name: "r", Settings: config.Resolved{OnConflict: over}, Actions: []config.Action{act(config.Move, "Work")}}}}, + } + chains := Build("/r", in, time.Now(), NoDisk{}, NewClaims()) + first, second := chains[0].Steps[0], chains[1].Steps[0] + if first.Dst != "/r/Work/x.pdf" || first.Displaces != "" { + t.Errorf("first step = %+v; want Dst /r/Work/x.pdf and empty Displaces", first) + } + if second.Dst != "/r/Work/x_1.pdf" || second.Displaces != "" { + t.Errorf("second step = %+v; want Dst /r/Work/x_1.pdf and empty Displaces", second) + } +} + +func TestCopyAlreadyThere(t *testing.T) { + d := fakeDisk{ + exists: map[string]bool{"/backup/x.pdf": true}, + same: map[[2]string]bool{{"/r/x.pdf", "/backup/x.pdf"}: true}, + } + in := []Input{{File: file("/r", "x.pdf"), Rules: []RuleMatch{ + {Name: "b", Actions: []config.Action{act(config.Copy, "/backup")}}}}} + s := Build("/r", in, time.Now(), d, NewClaims())[0].Steps[0] + if s.Skip != "already there" { + t.Errorf("step = %+v; want Skip \"already there\"", s) + } +} + +func TestTwoFilesOneTarget(t *testing.T) { + in := []Input{ + {File: file("/r", "a/x.pdf"), Rules: []RuleMatch{ + {Name: "r", Actions: []config.Action{act(config.Move, "Work")}}}}, + {File: file("/r", "b/x.pdf"), Rules: []RuleMatch{ + {Name: "r", Actions: []config.Action{act(config.Move, "Work")}}}}, + } + chains := Build("/r", in, time.Now(), NoDisk{}, NewClaims()) + if chains[0].Steps[0].Dst != "/r/Work/x.pdf" || chains[1].Steps[0].Dst != "/r/Work/x_1.pdf" { + t.Errorf("in-plan collision: %q and %q", chains[0].Steps[0].Dst, chains[1].Steps[0].Dst) + } +} + +func TestSameContentReal(t *testing.T) { + dir := t.TempDir() + a := filepath.Join(dir, "a") + b := filepath.Join(dir, "b") + c := filepath.Join(dir, "c") + if err := os.WriteFile(a, []byte("same bytes"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(b, []byte("same bytes"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(c, []byte("other bytes"), 0o644); err != nil { + t.Fatal(err) + } + if ok, err := (OS{}).SameContent(a, b); err != nil || !ok { + t.Errorf("identical files: %v %v", ok, err) + } + if ok, _ := (OS{}).SameContent(a, c); ok { + t.Error("different files reported identical") + } +} + +// stubAlwaysExists is C2's stub Disk: every path reports as existing (and +// nothing is ever the same content), so suffixed() can never find a free +// name and must give up instead of spinning forever. +type stubAlwaysExists struct{} + +func (stubAlwaysExists) Exists(string) bool { return true } +func (stubAlwaysExists) SameContent(string, string) (bool, error) { return false, nil } + +// TestSuffixedCapsAttempts is C2: suffixed() gives up after +// maxSuffixAttempts, reporting Skip "too many conflicting names" and no +// Dst, rather than looping forever against a Disk that never reports a free +// name. +func TestSuffixedCapsAttempts(t *testing.T) { + in := []Input{{File: file("/r", "x.pdf"), Rules: []RuleMatch{ + {Name: "a", Actions: []config.Action{act(config.Move, "Work")}}}}} + s := Build("/r", in, time.Now(), stubAlwaysExists{}, NewClaims())[0].Steps[0] + if s.Skip != "too many conflicting names" || s.Dst != "" { + t.Errorf("step = %+v; want Skip \"too many conflicting names\" and empty Dst", s) + } +} diff --git a/internal/plan/index.go b/internal/plan/index.go new file mode 100644 index 0000000..abe073f --- /dev/null +++ b/internal/plan/index.go @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "fmt" + "strconv" + "strings" +) + +// MaxIndex returns the highest {N} used in s, 0 when none. It reports the +// same errors Expand does for a malformed placeholder: an unclosed +// placeholder, or {0} (capture groups are numbered from 1). It shares +// Expand's scanner shape: "{{" and "}}" each emit one literal brace, and any +// other "{" opens a placeholder that runs to the next "}". +func MaxIndex(s string) (int, error) { + max := 0 + for i := 0; i < len(s); { + switch { + case s[i] == '{' && i+1 < len(s) && s[i+1] == '{': + i += 2 + case s[i] == '}' && i+1 < len(s) && s[i+1] == '}': + i += 2 + case s[i] == '{': + end := strings.IndexByte(s[i+1:], '}') + if end < 0 { + return 0, fmt.Errorf("unclosed placeholder") + } + body := s[i+1 : i+1+end] + if n, err := strconv.Atoi(body); err == nil { + if n == 0 { + return 0, fmt.Errorf("capture groups are numbered from 1") + } + if n >= 1 && n <= 9 && n > max { + max = n + } + } + i += end + 2 + default: + i++ + } + } + return max, nil +} diff --git a/internal/plan/index_test.go b/internal/plan/index_test.go new file mode 100644 index 0000000..b157f56 --- /dev/null +++ b/internal/plan/index_test.go @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "strings" + "testing" +) + +func TestMaxIndex(t *testing.T) { + for in, want := range map[string]int{ + "Work/Acme": 0, + "{name}": 0, + "{1}": 1, + "Work/{2}/{1}": 2, + "{stem}-{9}{ext}": 9, + "{{1}}": 0, + "{mtime:%Y}/{3}-{1}": 3, + } { + got, err := MaxIndex(in) + if err != nil || got != want { + t.Errorf("MaxIndex(%q) = %d, %v; want %d", in, got, err, want) + } + } + if _, err := MaxIndex("{name"); err == nil || !strings.Contains(err.Error(), "unclosed placeholder") { + t.Errorf("unclosed: %v", err) + } +} diff --git a/internal/plan/json.go b/internal/plan/json.go new file mode 100644 index 0000000..0b4f1bc --- /dev/null +++ b/internal/plan/json.go @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import "time" + +// jsonNote is carried in every JSON document, warning readers that the +// shape is not yet stable. +const jsonNote = "the shape of this document is unstable before krino 1.0" + +// JSON is the --json document. The shape is unstable before 1.0 and says +// so in its own "note" field. +type JSON struct { + Version int `json:"version"` + Note string `json:"note"` + Dirs []JSONDir `json:"dirs"` +} + +// JSONDir is one directory's plan. +type JSONDir struct { + Name string `json:"name"` + Root string `json:"root"` + Files []JSONFile `json:"files"` + Warnings []string `json:"warnings,omitempty"` +} + +// JSONFile is one file's chain. +type JSONFile struct { + Rel string `json:"rel"` + Size int64 `json:"size"` + ModTime time.Time `json:"mtime"` + Steps []JSONStep `json:"steps"` + Warnings []string `json:"warnings,omitempty"` +} + +// JSONStep is one step of a chain. D14: Reason is carried because the text +// plan already shows it and a machine reader should be able to see why a +// file matched too; Conflict (the rule's on-conflict policy) is deliberately +// not: it is a config detail, and its outcome is already visible through +// dst, displaces and skip. +type JSONStep struct { + Action string `json:"action"` + Rule string `json:"rule"` + Src string `json:"src"` + Dst string `json:"dst,omitempty"` + Displaces string `json:"displaces,omitempty"` + Reason string `json:"reason,omitempty"` + Skip string `json:"skip,omitempty"` +} + +// actionNames maps a Kind onto its JSON action name. This is its own +// mapping, independent of Kind.String(): the display form renders "DELETE +// permanently", which must never reach a machine reader. These names match +// the log's action names in spec §9, so a later `krino log` and a --json +// plan can be grepped together. +var actionNames = map[Kind]string{ + Copy: "copy", + Move: "move", + Rename: "rename", + Trash: "trash", + DeletePermanent: "delete", +} + +// NewJSON builds the top-level --json document over dirs. Version and Note +// are set here, in the one place jsonNote's wording already lives: it is +// unexported, so a struct literal built outside this package would +// silently ship an empty "note" and break the document's own contract. +func NewJSON(dirs []JSONDir) JSON { + return JSON{Version: 1, Note: jsonNote, Dirs: dirs} +} + +// NewJSONDir converts one directory's chains. +func NewJSONDir(name, root string, chains []Chain, warnings []string) JSONDir { + files := make([]JSONFile, 0, len(chains)) + for _, ch := range chains { + steps := make([]JSONStep, 0, len(ch.Steps)) + for _, s := range ch.Steps { + steps = append(steps, JSONStep{ + Action: actionNames[s.Kind], + Rule: s.Rule, + Src: s.Src, + Dst: s.Dst, + Displaces: s.Displaces, + Reason: s.Reason, + Skip: s.Skip, + }) + } + files = append(files, JSONFile{ + Rel: ch.File.Rel, + Size: ch.File.Size, + ModTime: ch.File.ModTime, + Steps: steps, + Warnings: ch.Warnings, + }) + } + return JSONDir{Name: name, Root: root, Files: files, Warnings: warnings} +} diff --git a/internal/plan/json_test.go b/internal/plan/json_test.go new file mode 100644 index 0000000..86c181c --- /dev/null +++ b/internal/plan/json_test.go @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "encoding/json" + "strings" + "testing" + "time" +) + +func TestJSONDir(t *testing.T) { + chains := []Chain{{ + File: file("/r", "x.pdf"), + Steps: []Step{ + {Kind: Copy, Rule: "b", Src: "/r/x.pdf", Dst: "/backup/x.pdf", Reason: `content "acme ltd"`}, + {Kind: Move, Rule: "a", Src: "/r/x.pdf", Dst: "/r/W/x.pdf", Displaces: "/r/W/x.pdf"}, + {Kind: Rename, Rule: "r", Src: "/r/W/x.pdf", Dst: "/r/W/2026-x.pdf"}, + {Kind: Trash, Rule: "t", Src: "/r/W/2026-x.pdf"}, + {Kind: DeletePermanent, Rule: "z", Src: "/r/W/x.pdf", Skip: "deleted by rule a"}, + }, + Warnings: []string{"moved more than once; a (stop) is probably missing"}, + }} + b, err := json.MarshalIndent(JSON{Version: 1, Note: jsonNote, Dirs: []JSONDir{NewJSONDir("dl", "/r", chains, nil)}}, "", " ") + if err != nil { + t.Fatal(err) + } + out := string(b) + for _, want := range []string{ + `"version": 1`, + `"note": "the shape of this document is unstable before krino 1.0"`, + `"name": "dl"`, + `"rel": "x.pdf"`, + `"action": "copy"`, + // D14: the reason a rule matched is carried into the JSON document too. + `"reason": "content \"acme ltd\""`, + // D7: rename and trash were previously covered only by inspection, so + // an edit garbling either name would have passed silently. + `"action": "rename"`, + `"action": "trash"`, + `"action": "move"`, + `"displaces": "/r/W/x.pdf"`, + `"action": "delete"`, + `"skip": "deleted by rule a"`, + } { + if !strings.Contains(out, want) { + t.Errorf("json lacks %s:\n%s", want, out) + } + } + if strings.Contains(out, `"dst": ""`) { + t.Error("empty dst must be omitted") + } + var round JSON + if err := json.Unmarshal(b, &round); err != nil { + t.Fatalf("does not round-trip: %v", err) + } + if !round.Dirs[0].Files[0].ModTime.Equal(time.Date(2026, 8, 15, 12, 0, 0, 0, time.UTC)) { + t.Errorf("mtime did not survive: %v", round.Dirs[0].Files[0].ModTime) + } +} + +func TestJSONDirEmptyStepsAndFilesAreArraysNotNull(t *testing.T) { + stepless := []Chain{{File: file("/r", "y.pdf")}} + dir := NewJSONDir("dl", "/r", stepless, nil) + b, err := json.Marshal(dir) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(b), `"steps":null`) { + t.Errorf("stepless file's steps must be [], not null: %s", b) + } + if !strings.Contains(string(b), `"steps":[]`) { + t.Errorf("stepless file's steps should marshal as []: %s", b) + } + + empty := NewJSONDir("dl", "/r", nil, nil) + b, err = json.Marshal(empty) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(b), `"files":null`) { + t.Errorf("empty dir's files must be [], not null: %s", b) + } + if !strings.Contains(string(b), `"files":[]`) { + t.Errorf("empty dir's files should marshal as []: %s", b) + } +} diff --git a/internal/plan/placeholder.go b/internal/plan/placeholder.go new file mode 100644 index 0000000..b947236 --- /dev/null +++ b/internal/plan/placeholder.go @@ -0,0 +1,155 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +// Package plan turns matched rules into concrete actions: placeholder +// expansion, chains, conflict resolution and the JSON plan representation. +package plan + +import ( + "errors" + "fmt" + "strconv" + "strings" + "time" +) + +// Facts are the values a placeholder can reference. +type Facts struct { + Name string // the file's current base name + Captures []string // [0] whole match, [1:] groups, from the rule's first true name test + ModTime time.Time + Now time.Time +} + +// splitExt splits name on its last dot, which does not count when it is the +// first character: "a.tar.gz" -> "a.tar", ".gz"; ".bashrc" -> ".bashrc", "". +func splitExt(name string) (stem, ext string) { + i := strings.LastIndexByte(name, '.') + if i <= 0 { + return name, "" + } + return name[:i], name[i:] +} + +// Expand replaces every placeholder in s. It returns an error naming the +// first placeholder it could not expand. It scans s once: "{{" and "}}" +// each emit one literal brace, and any other "{" opens a placeholder that +// runs to the next "}". +func Expand(s string, f Facts) (string, error) { + var b strings.Builder + for i := 0; i < len(s); { + switch { + case s[i] == '{' && i+1 < len(s) && s[i+1] == '{': + b.WriteByte('{') + i += 2 + case s[i] == '}' && i+1 < len(s) && s[i+1] == '}': + b.WriteByte('}') + i += 2 + case s[i] == '{': + end := strings.IndexByte(s[i+1:], '}') + if end < 0 { + return "", errors.New("unclosed placeholder") + } + body := s[i+1 : i+1+end] + val, err := expandOne(body, f) + if err != nil { + return "", err + } + // D2: val is appended with WriteString, after the scan has + // already moved past this placeholder - it is never re-passed + // through this loop, so a "}}" or "{{" inside a capture's own + // text is never collapsed the way one written in the template + // itself would be. A refactor to scan-then-replace would + // silently undo this; TestExpandCaptureNotRescanned pins it. + b.WriteString(val) + i += end + 2 + default: + b.WriteByte(s[i]) + i++ + } + } + return b.String(), nil +} + +// expandOne expands the body of a single {...} placeholder (without the +// braces) into its replacement text. +func expandOne(body string, f Facts) (string, error) { + switch body { + case "name": + return f.Name, nil + case "stem": + stem, _ := splitExt(f.Name) + return stem, nil + case "ext": + _, ext := splitExt(f.Name) + return ext, nil + } + + if verb, format, ok := strings.Cut(body, ":"); ok { + switch verb { + case "mtime": + return strftime(verb, format, f.ModTime) + case "now": + return strftime(verb, format, f.Now) + } + return "", fmt.Errorf("unknown placeholder {%s}", body) + } + + if n, err := strconv.Atoi(body); err == nil { + if n == 0 { + return "", errors.New("capture groups are numbered from 1") + } + // D3: spec §7.3 defines the syntax as {1}...{9}, the same window + // MaxIndex enforces; without this check {10} and up bypass + // checkCaptures entirely (MaxIndex never sees them as capture + // uses) and fail only here, at expansion time. + if n > 9 { + return "", fmt.Errorf("unknown placeholder {%s}", body) + } + if n < 0 || n >= len(f.Captures) { + return "", fmt.Errorf("no capture group %d", n) + } + return f.Captures[n], nil + } + + return "", fmt.Errorf("unknown placeholder {%s}", body) +} + +// strftime renders a strftime subset (%Y %m %d %H %M %S %j %%) of t. verb is +// the placeholder's own verb ("mtime" or "now"), named in error messages so +// they point at what the config author actually wrote (B3) instead of +// hardcoding "mtime" for a {now:...} format error. +func strftime(verb, format string, t time.Time) (string, error) { + var b strings.Builder + for i := 0; i < len(format); i++ { + c := format[i] + if c != '%' { + b.WriteByte(c) + continue + } + i++ + if i >= len(format) { + return "", fmt.Errorf("unknown time format %%%c in {%s:...}", format[i-1], verb) + } + switch format[i] { + case 'Y': + fmt.Fprintf(&b, "%04d", t.Year()) + case 'm': + fmt.Fprintf(&b, "%02d", int(t.Month())) + case 'd': + fmt.Fprintf(&b, "%02d", t.Day()) + case 'H': + fmt.Fprintf(&b, "%02d", t.Hour()) + case 'M': + fmt.Fprintf(&b, "%02d", t.Minute()) + case 'S': + fmt.Fprintf(&b, "%02d", t.Second()) + case 'j': + fmt.Fprintf(&b, "%03d", t.YearDay()) + case '%': + b.WriteByte('%') + default: + return "", fmt.Errorf("unknown time format %%%c in {%s:...}", format[i], verb) + } + } + return b.String(), nil +} diff --git a/internal/plan/placeholder_test.go b/internal/plan/placeholder_test.go new file mode 100644 index 0000000..e155430 --- /dev/null +++ b/internal/plan/placeholder_test.go @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "strings" + "testing" + "time" +) + +func facts() Facts { + return Facts{ + Name: "Invoice_2026-08.pdf", + Captures: []string{"2026-08", "2026", "a}}b"}, + ModTime: time.Date(2026, 8, 15, 14, 30, 45, 0, time.UTC), + Now: time.Date(2026, 9, 12, 9, 0, 0, 0, time.UTC), + } +} + +func TestExpand(t *testing.T) { + tests := []struct{ in, want string }{ + {"Work/Acme", "Work/Acme"}, + {"{name}", "Invoice_2026-08.pdf"}, + {"{stem}", "Invoice_2026-08"}, + {"{ext}", ".pdf"}, + {"{stem}{ext}", "Invoice_2026-08.pdf"}, + {"Work/{mtime:%Y}", "Work/2026"}, + {"{mtime:%Y-%m-%d}", "2026-08-15"}, + {"{mtime:%H%M%S}", "143045"}, + {"{mtime:%j}", "227"}, + {"{now:%Y-%m-%d}", "2026-09-12"}, + {"{1}", "2026"}, + // D2: 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. + {"{2}", "a}}b"}, + {"{{literal}}", "{literal}"}, + {"100{{%}}", "100{%}"}, + {"{mtime:%Y%%}", "2026%"}, + {"Photos/{mtime:%Y}/{stem}-{1}{ext}", "Photos/2026/Invoice_2026-08-2026.pdf"}, + } + for _, tt := range tests { + got, err := Expand(tt.in, facts()) + if err != nil || got != tt.want { + t.Errorf("Expand(%q) = %q, %v; want %q", tt.in, got, err, tt.want) + } + } +} + +func TestExpandNoExtension(t *testing.T) { + f := facts() + f.Name = "README" + for in, want := range map[string]string{"{stem}": "README", "{ext}": ""} { + if got, err := Expand(in, f); err != nil || got != want { + t.Errorf("Expand(%q) on README = %q, %v; want %q", in, got, err, want) + } + } + f.Name = ".bashrc" + if got, _ := Expand("{stem}", f); got != ".bashrc" { + t.Errorf("dotfile stem = %q, want .bashrc", got) + } + if got, _ := Expand("{ext}", f); got != "" { + t.Errorf("dotfile ext = %q, want empty", got) + } + f.Name = "archive.tar.gz" + if got, _ := Expand("{stem}|{ext}", f); got != "archive.tar|.gz" { + t.Errorf("double extension = %q, want archive.tar|.gz", got) + } +} + +func TestExpandErrors(t *testing.T) { + tests := []struct{ in, want string }{ + {"{7}", "no capture group 7"}, + {"{0}", "capture groups are numbered from 1"}, + {"{whatever}", "unknown placeholder {whatever}"}, + {"{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"), + // 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 + // rejected the same way an unknown placeholder is. + {"{10}", "unknown placeholder {10}"}, + } + for _, tt := range tests { + _, err := Expand(tt.in, facts()) + if err == nil || !strings.Contains(err.Error(), tt.want) { + t.Errorf("Expand(%q) error = %v; want %q", tt.in, err, tt.want) + } + } +} diff --git a/internal/plan/step.go b/internal/plan/step.go new file mode 100644 index 0000000..e4f280f --- /dev/null +++ b/internal/plan/step.go @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "fmt" + + "krino/internal/config" + "krino/internal/scan" +) + +// Kind is what a step does. +type Kind int + +const ( + Copy Kind = iota + Move + Rename + Trash // (delete) + DeletePermanent // (delete permanent) +) + +// String is for display only; Task 5's JSON representation defines its own +// action names. +func (k Kind) String() string { + switch k { + case Copy: + return "copy" + case Move: + return "move" + case Rename: + return "rename" + case Trash: + return "trash" + case DeletePermanent: + return "DELETE permanently" + } + return fmt.Sprintf("Kind(%d)", int(k)) +} + +// Step is one action to carry out, already resolved to absolute paths. +type Step struct { + Kind Kind + Rule string // the rule that contributed it + Src string // absolute path the step reads from + Dst string // absolute path the file has after the step; "" for the two deletes + Reason string // the rule's match reasons, for display + Skip string // non-empty: this step will not run, and why + Conflict config.Conflict // the contributing rule's on-conflict policy + Displaces string // overwrite only: the existing file that must be trashed first +} + +// Chain is one file's steps, in order. +type Chain struct { + File scan.File + Steps []Step + Warnings []string +} + +// RuleMatch is one matching rule's contribution to a file's chain. +// internal/plan must not import internal/engine (engine imports plan), so +// the engine converts its own types into these. +type RuleMatch struct { + Name string + Actions []config.Action + Settings config.Resolved + Captures []string + Reasons []string +} |