diff options
35 files changed, 1353 insertions, 226 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 43ab3e5..da39c06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,20 @@ ## Unreleased +## 0.0.5 — 2026-09-14 + +- Keyword cache: for each file whose text it extracts, krino records which + content keywords the text contains, in `~/.cache/krino/NAME.cache`, and + answers content tests from it while the file is unchanged. A rerun over + the same files extracts nothing: on a real downloads folder a dry run + went from 12.8 s to 0.13 s. No text and no file names are stored. A + changed file, a new keyword, or a changed extraction tool means reading + again; failures are never cached. `krino check` shows where the cache is. +- Choosing per file: `t` sends the file to the Trash and `d` deletes it + permanently, after a `y` to confirm, instead of what its rules planned; + both are logged under the rule `(review)`. "Done, apply chosen so far" + moves from `d` to `w`, in undo's review too. + ## 0.0.4 — 2026-09-14 - `max-size` setting: files larger than it are skipped as "too big", in @@ -127,10 +127,12 @@ key at a time, no Enter needed: move → Filed/Acme/ rule acme because name "acme" - [y] yes [n] no [a] yes to this and all remaining [d] done, apply chosen so far [q] quit, apply nothing + [y] yes [n] no [a] yes to this and all remaining [t] trash [d] delete permanently [w] write, apply chosen so far [q] quit, apply nothing ``` -Approval is per file: a file's whole chain runs, or none of it. A README +Approval is per file: a file's whole chain runs, or none of it. `t` sends +the file to the Trash and `d` deletes it permanently (after a `y`) instead +of what the rules planned; `w` applies what you chose so far. A README can't paste a session it didn't actually run in a terminal, so here we apply directly with `-y`, which shows the same plan and applies it without asking: diff --git a/cmd/krino/cache_test.go b/cmd/krino/cache_test.go new file mode 100644 index 0000000..65526b3 --- /dev/null +++ b/cmd/krino/cache_test.go @@ -0,0 +1,47 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// TestRunWritesKeywordCache: a dry run over a directory with a content rule +// leaves a private cache under ~/.cache/krino, and check says where it is. +func TestRunWritesKeywordCache(t *testing.T) { + h := home(t) + dl := filepath.Join(h, "dl") + os.MkdirAll(dl, 0o755) + p := filepath.Join(dl, "a.txt") + os.WriteFile(p, []byte("invoice from acme ltd"), 0o644) + old := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + os.Chtimes(p, old, old) + if code, _, errOut := runCLI(t, "init"); code != 0 { + t.Fatal(errOut) + } + if code, _, errOut := runCLI(t, "new", "dl", dl); code != 0 { + t.Fatal(errOut) + } + rules := "(path \"~/dl\")\n(rule \"acme\" (when (content \"acme ltd\")) (move \"Acme\"))\n" + os.WriteFile(filepath.Join(h, ".config", "krino", "dirs", "dl.conf"), []byte(rules), 0o644) + + for i := 0; i < 2; i++ { + if code, out, errOut := runCLI(t, "-n"); code != 0 || !strings.Contains(out, "a.txt") { + t.Fatalf("run %d: exit %d\n%s\n%s", i, code, out, errOut) + } + } + fi, err := os.Stat(filepath.Join(h, ".cache", "krino", "dl.cache")) + if err != nil { + t.Fatal(err) + } + if fi.Mode().Perm() != 0o600 { + t.Errorf("cache mode %o, want 600", fi.Mode().Perm()) + } + if _, out, _ := runCLI(t, "check"); !strings.Contains(out, "cache: ~/.cache/krino\n") { + t.Errorf("check does not show the cache directory:\n%s", out) + } +} diff --git a/cmd/krino/check.go b/cmd/krino/check.go index dfe3a1a..4463c73 100644 --- a/cmd/krino/check.go +++ b/cmd/krino/check.go @@ -28,6 +28,7 @@ func cmdCheck(g *globals, args []string, stdout, stderr io.Writer) int { fmt.Fprintf(stdout, "config: %s\n", xdg.Abbrev(r.MainFile)) fmt.Fprintf(stdout, "log: %s\n", xdg.Abbrev(r.LogFile)) + fmt.Fprintf(stdout, "cache: %s\n", xdg.Abbrev(cacheDir())) if len(r.Dirs) == 0 { fmt.Fprintln(stdout, "no directories included; add one with: krino new NAME PATH") } diff --git a/cmd/krino/common.go b/cmd/krino/common.go index 4552441..11e73c5 100644 --- a/cmd/krino/common.go +++ b/cmd/krino/common.go @@ -5,6 +5,7 @@ package main import ( "fmt" "io" + "path/filepath" "strings" "time" @@ -30,6 +31,11 @@ func minAgeOverride(g *globals) (d time.Duration, set bool, err error) { return d, true, nil } +// cacheDir is where every directory's keyword cache lives (spec §6.1). +func cacheDir() string { + return filepath.Join(xdg.CacheHome(), "krino") +} + // applyMinAge sets every loaded directory's min-age to d, for this run only. func applyMinAge(e *engine.Engine, d time.Duration) { for _, dir := range e.Dirs { diff --git a/cmd/krino/explain.go b/cmd/krino/explain.go index 6f8fa26..8fade24 100644 --- a/cmd/krino/explain.go +++ b/cmd/krino/explain.go @@ -39,6 +39,7 @@ func cmdExplain(g *globals, args []string, stdout, stderr io.Writer) int { if setMinAge { applyMinAge(e, minAge) } + e.CacheDir = cacheDir() x, err := e.Explain(context.Background(), xdg.Expand(fs.Arg(0))) if err != nil { diff --git a/cmd/krino/history_test.go b/cmd/krino/history_test.go index f76ac66..5d86b6f 100644 --- a/cmd/krino/history_test.go +++ b/cmd/krino/history_test.go @@ -182,6 +182,22 @@ func TestReviewUndoChoosePerFile(t *testing.T) { } } +// TestReviewUndoWriteStopsAsking: [w] applies what was chosen so far; undo +// offers no [t] or [d], so those are rejected keys there. +func TestReviewUndoWriteStopsAsking(t *testing.T) { + out := new(strings.Builder) + approved, action, err := reviewUndoFiles(strings.NewReader("ctdyw"), out, undoFiles("a", "b", "c"), palette{}) + if err != nil { + t.Fatal(err) + } + if action != 'c' || !approved[0] || approved[1] || approved[2] { + t.Errorf("approved = %v action = %q; want only index 0", approved, action) + } + if !strings.Contains(out.String(), "'t' is not y, n, a, w or q") || strings.Contains(out.String(), "[t]") { + t.Errorf("undo review should reject t and not offer it:\n%s", out) + } +} + // TestReviewUndoRefusedFileNotPrompted is spec §10: a file PlanUndo already // refused is shown (with its reason - covered end to end by // TestUndoRefusesChangedDestination-style flows through cmdUndo) but never diff --git a/cmd/krino/review.go b/cmd/krino/review.go index 66e17b9..29b3915 100644 --- a/cmd/krino/review.go +++ b/cmd/krino/review.go @@ -21,7 +21,7 @@ import ( // directory-level table does (root-relative inside root, ~-abbreviated // outside it) instead of always falling back to the abbreviated form. p // styles the prompts and the per-file steps (spec §8.2). -func reviewDir(out io.Writer, chains []plan.Chain, root string, p palette) (map[string]bool, rune, error) { +func reviewDir(out io.Writer, chains []plan.Chain, root string, p palette) (map[string]bool, map[string]plan.Kind, rune, error) { return reviewChains(keyReader{stdin}, out, chains, root, p) } @@ -47,17 +47,18 @@ func (k keyReader) Read(p []byte) (int, error) { // // menu, and, for [c], the per-file // -// [y] yes [n] no [a] yes to this and all remaining [d] done, apply chosen so far [q] quit, apply nothing +// [y] yes [n] no [a] yes to this and all remaining [t] trash [d] delete permanently [w] write, apply chosen so far [q] quit, apply nothing // // prompt. action is always one of 'a', 'c', 's' or 'q': a [c] session's own // [q] ("quit, apply nothing") folds into the same 'q' the caller already // handles for the top-level menu, and approved is emptied to match - even a // file already marked yes in that session is discarded, per spec §8.3's -// wording ("apply nothing"), unlike [d] ("apply chosen so far"), which -// keeps it. root is the directory being reviewed - passed only to +// wording ("apply nothing"), unlike [w] ("apply chosen so far"), which +// keeps it. replaced holds the files [t] or [d] chose to trash or delete +// instead of what the rules planned; replaceChains applies it. root is the directory being reviewed - passed only to // reviewPerFile's destination rendering (review finding 1, fix round // 2026-09-12); nothing here uses it directly. -func reviewChains(in io.Reader, out io.Writer, chains []plan.Chain, root string, p palette) (map[string]bool, rune, error) { +func reviewChains(in io.Reader, out io.Writer, chains []plan.Chain, root string, p palette) (map[string]bool, map[string]plan.Kind, rune, error) { fmt.Fprintln(out) for _, l := range wrapped("", "[a] apply all [c] choose per file [s] skip this directory [q] quit", 0, widthPolicy(out), p.keys) { fmt.Fprintln(out, l) @@ -65,24 +66,24 @@ func reviewChains(in io.Reader, out io.Writer, chains []plan.Chain, root string, for { key, err := readKey(in) if err != nil { - return nil, 0, err + return nil, nil, 0, err } switch key { case 'a': - return approveAll(chains), 'a', nil + return approveAll(chains), nil, 'a', nil case 's': - return map[string]bool{}, 's', nil + return map[string]bool{}, nil, 's', nil case 'q': - return map[string]bool{}, 'q', nil + return map[string]bool{}, nil, 'q', nil case 'c': - approved, quit, err := reviewPerFile(in, out, chains, root, p) + approved, replaced, quit, err := reviewPerFile(in, out, chains, root, p) if err != nil { - return nil, 0, err + return nil, nil, 0, err } if quit { - return map[string]bool{}, 'q', nil + return map[string]bool{}, nil, 'q', nil } - return approved, 'c', nil + return approved, replaced, 'c', nil default: fmt.Fprintf(out, "%q is not a, c, s or q\n", key) } @@ -94,14 +95,18 @@ func reviewChains(in io.Reader, out io.Writer, chains []plan.Chain, root string, // Each file shows the same block body the plan shows (stepLines): every // step, its rule and its reason, wrapped to the terminal. [y]/[n] decide // just that file; [a] approves it and every remaining file without asking -// again; [d] stops asking and applies whatever was already chosen, +// again; [t] and [d] approve it with its chain replaced by one trash or +// permanent delete step, [d] only after a y to its own confirmation (any +// other key asks about the same file again); [w] stops asking and applies +// whatever was already chosen, // declining the rest; [q] aborts the review entirely, discarding even files // already marked yes - reported back to reviewChains via quit=true. root is // passed down so a destination inside root renders root-relative and one // outside it renders ~-abbreviated, exactly as in the plan (review finding // 1, fix round 2026-09-12). -func reviewPerFile(in io.Reader, out io.Writer, chains []plan.Chain, root string, p palette) (approved map[string]bool, quit bool, err error) { +func reviewPerFile(in io.Reader, out io.Writer, chains []plan.Chain, root string, p palette) (approved map[string]bool, replaced map[string]plan.Kind, quit bool, err error) { approved = map[string]bool{} + replaced = map[string]plan.Kind{} yesRest := false for i, c := range chains { if yesRest { @@ -113,14 +118,17 @@ func reviewPerFile(in io.Reader, out io.Writer, chains []plan.Chain, root string for _, l := range stepLines(c, 7, root, p, widthPolicy(out)) { fmt.Fprintln(out, l) } - for _, l := range wrapped(" ", perFileKeys, 2, widthPolicy(out), p.keys) { - fmt.Fprintln(out, l) - } - + showKeys := true for { + if showKeys { + for _, l := range wrapped(" ", perFileKeys, 2, widthPolicy(out), p.keys) { + fmt.Fprintln(out, l) + } + showKeys = false + } key, kerr := readKey(in) if kerr != nil { - return nil, false, kerr + return nil, nil, false, kerr } switch key { case 'y': @@ -130,23 +138,60 @@ func reviewPerFile(in io.Reader, out io.Writer, chains []plan.Chain, root string case 'a': approved[c.File.Rel] = true yesRest = true + case 't': + approved[c.File.Rel] = true + replaced[c.File.Rel] = plan.Trash case 'd': - return approved, false, nil + fmt.Fprintf(out, " delete %s permanently? [y/N] ", c.File.Rel) + confirm, kerr := readKey(in) + if kerr != nil { + return nil, nil, false, kerr + } + fmt.Fprintln(out) + if confirm != 'y' { + fmt.Fprintln(out, " not deleted") + showKeys = true + continue + } + approved[c.File.Rel] = true + replaced[c.File.Rel] = plan.DeletePermanent + case 'w': + return approved, replaced, false, nil case 'q': - return nil, true, nil + return nil, nil, true, nil default: - fmt.Fprintf(out, "%q is not y, n, a, d or q\n", key) + fmt.Fprintf(out, "%q is not y, n, a, t, d, w or q\n", key) continue } break } } - return approved, false, nil + return approved, replaced, false, nil } -// perFileKeys is the per-file prompt of spec §8.3, shared by review and -// undo, and wrapped to the terminal like every other long line. -const perFileKeys = "[y] yes [n] no [a] yes to this and all remaining [d] done, apply chosen so far [q] quit, apply nothing" +// perFileKeys is the per-file prompt of spec §8.3, wrapped to the terminal +// like every other long line. Undo's has no [t] or [d] (undoPerFileKeys). +const perFileKeys = "[y] yes [n] no [a] yes to this and all remaining [t] trash [d] delete permanently [w] write, apply chosen so far [q] quit, apply nothing" + +// reviewRule is the rule name a step chosen in review is planned and logged +// under, in parentheses so no configured rule can be mistaken for it. +const reviewRule = "(review)" + +// replaceChains returns chains with the chain of every file in replaced +// swapped for one step of the chosen kind on the file itself, under +// reviewRule: [t] and [d] set aside what the rules planned. chains itself +// is not modified. +func replaceChains(chains []plan.Chain, replaced map[string]plan.Kind) []plan.Chain { + out := make([]plan.Chain, len(chains)) + for i, c := range chains { + if k, ok := replaced[c.File.Rel]; ok { + c.Steps = []plan.Step{{Kind: k, Rule: reviewRule, Src: c.File.Path, Reason: "chosen in review"}} + c.Warnings = nil + } + out[i] = c + } + return out +} // readKey reads the single byte reviewChains treats as one keypress. Over // the real terminal that byte already came from tui.ReadKey (via diff --git a/cmd/krino/review_test.go b/cmd/krino/review_test.go index a387022..19a1d01 100644 --- a/cmd/krino/review_test.go +++ b/cmd/krino/review_test.go @@ -22,7 +22,7 @@ func chains(rels ...string) []plan.Chain { func TestChoosePerFile(t *testing.T) { // c enters per-file mode, then y n y for three files. - approved, action, err := reviewChains(strings.NewReader("cyny"), new(strings.Builder), chains("a", "b", "c"), "", palette{}) + approved, _, action, err := reviewChains(strings.NewReader("cyny"), new(strings.Builder), chains("a", "b", "c"), "", palette{}) if err != nil { t.Fatal(err) } @@ -35,31 +35,31 @@ func TestChoosePerFile(t *testing.T) { } func TestApplyAllAndSkip(t *testing.T) { - approved, action, _ := reviewChains(strings.NewReader("a"), new(strings.Builder), chains("a", "b"), "", palette{}) + approved, _, action, _ := reviewChains(strings.NewReader("a"), new(strings.Builder), chains("a", "b"), "", palette{}) if action != 'a' || len(approved) != 2 { t.Errorf("[a] = %q %v; want every file approved", action, approved) } - approved, action, _ = reviewChains(strings.NewReader("s"), new(strings.Builder), chains("a", "b"), "", palette{}) + approved, _, action, _ = reviewChains(strings.NewReader("s"), new(strings.Builder), chains("a", "b"), "", palette{}) if action != 's' || len(approved) != 0 { t.Errorf("[s] = %q %v; want nothing approved", action, approved) } } -func TestPerFileDoneStopsAsking(t *testing.T) { - // c, y for the first, then d: apply what was chosen so far. - approved, _, _ := reviewChains(strings.NewReader("cyd"), new(strings.Builder), chains("a", "b", "c"), "", palette{}) +func TestPerFileWriteStopsAsking(t *testing.T) { + // c, y for the first, then w: apply what was chosen so far. + approved, _, _, _ := reviewChains(strings.NewReader("cyw"), new(strings.Builder), chains("a", "b", "c"), "", palette{}) if !approved["a"] || approved["b"] || approved["c"] { t.Errorf("approved = %v; want only a", approved) } } // TestPerFileQuitAppliesNothing: spec §8.3's [q] on the per-file prompt is -// "quit, apply nothing" - stronger than [d], which keeps what was already +// "quit, apply nothing" - stronger than [w], which keeps what was already // chosen. It folds into the same top-level 'q' the caller already handles // for the directory-level menu (spec §8.2's [q]), and discards even a file // already marked yes. func TestPerFileQuitAppliesNothing(t *testing.T) { - approved, action, err := reviewChains(strings.NewReader("cyq"), new(strings.Builder), chains("a", "b"), "", palette{}) + approved, _, action, err := reviewChains(strings.NewReader("cyq"), new(strings.Builder), chains("a", "b"), "", palette{}) if err != nil { t.Fatal(err) } @@ -74,7 +74,7 @@ func TestPerFileQuitAppliesNothing(t *testing.T) { // TestPerFileYesToAllRemaining: spec §8.3's [a] mid-review approves the // current file and every remaining one without asking again. func TestPerFileYesToAllRemaining(t *testing.T) { - approved, action, err := reviewChains(strings.NewReader("ca"), new(strings.Builder), chains("a", "b", "c"), "", palette{}) + approved, _, action, err := reviewChains(strings.NewReader("ca"), new(strings.Builder), chains("a", "b", "c"), "", palette{}) if err != nil { t.Fatal(err) } @@ -88,7 +88,7 @@ func TestPerFileYesToAllRemaining(t *testing.T) { // same prompt is read again. func TestInvalidKeyReprompts(t *testing.T) { out := new(strings.Builder) - approved, action, err := reviewChains(strings.NewReader("zs"), out, chains("a"), "", palette{}) + approved, _, action, err := reviewChains(strings.NewReader("zs"), out, chains("a"), "", palette{}) if err != nil { t.Fatal(err) } @@ -115,7 +115,7 @@ func TestPerFileDestinationIsRootRelative(t *testing.T) { {File: scan.File{Rel: "outside.txt"}, Steps: []plan.Step{{Kind: plan.Move, Dst: "/home/x/backup/outside.txt"}}}, } out := new(strings.Builder) - if _, _, err := reviewChains(strings.NewReader("cyy"), out, cs, root, palette{}); err != nil { + if _, _, _, err := reviewChains(strings.NewReader("cyy"), out, cs, root, palette{}); err != nil { t.Fatal(err) } text := out.String() @@ -134,7 +134,7 @@ func TestPerFileShowsTheWholeBlock(t *testing.T) { {Kind: plan.Move, Rule: "acme", Dst: "/w/a.pdf", Reason: `content "acme ltd"`}, }}} out := new(strings.Builder) - if _, _, err := reviewChains(strings.NewReader("cy"), out, cs, "", palette{}); err != nil { + if _, _, _, err := reviewChains(strings.NewReader("cy"), out, cs, "", palette{}); err != nil { t.Fatal(err) } want := "\n[1/1] a.pdf\n move → /w/\n rule acme\n because content \"acme ltd\"\n" @@ -154,7 +154,7 @@ func TestPerFileWrapsToTheTerminal(t *testing.T) { {Kind: plan.Move, Rule: "acme", Dst: "/w/some/deeply/nested/destination/directory/for/invoices/a.pdf", Reason: `content "acme ltd"`}, }}} out := new(strings.Builder) - if _, _, err := reviewChains(strings.NewReader("cy"), out, cs, "", palette{}); err != nil { + if _, _, _, err := reviewChains(strings.NewReader("cy"), out, cs, "", palette{}); err != nil { t.Fatal(err) } var prompt string @@ -170,3 +170,67 @@ func TestPerFileWrapsToTheTerminal(t *testing.T) { t.Errorf("wrapped prompt reads %q, want %q\n%s", prompt, perFileKeys, out) } } + +// TestPerFileTrashAndDelete: [t] and [d] (confirmed with y) replace what the +// rules planned for that file with one step, and approve it. +func TestPerFileTrashAndDelete(t *testing.T) { + out := new(strings.Builder) + approved, replaced, action, err := reviewChains(strings.NewReader("ctdyn"), out, chains("a", "b", "c"), "", palette{}) + if err != nil { + t.Fatal(err) + } + if action != 'c' || !approved["a"] || !approved["b"] || approved["c"] { + t.Errorf("approved = %v action = %q; want a and b", approved, action) + } + if len(replaced) != 2 || replaced["a"] != plan.Trash || replaced["b"] != plan.DeletePermanent { + t.Errorf("replaced = %v; want a trash, b deleted permanently", replaced) + } + if !strings.Contains(out.String(), "delete b permanently? [y/N]") { + t.Errorf("no confirmation asked:\n%s", out) + } +} + +// TestPerFileDeleteNeedsConfirmation: any key but y after [d] deletes +// nothing and asks about the same file again. +func TestPerFileDeleteNeedsConfirmation(t *testing.T) { + out := new(strings.Builder) + approved, replaced, _, err := reviewChains(strings.NewReader("cdny"), out, chains("a"), "", palette{}) + if err != nil { + t.Fatal(err) + } + if len(replaced) != 0 || !approved["a"] { + t.Errorf("approved = %v replaced = %v; want a approved as planned, nothing replaced", approved, replaced) + } + if strings.Count(out.String(), perFileKeys) != 2 { + t.Errorf("the prompt should be shown again after a cancelled delete:\n%s", out) + } +} + +// TestPerFileQuitDiscardsReplacements: [q] after [t] applies nothing, the +// trash included. +func TestPerFileQuitDiscardsReplacements(t *testing.T) { + approved, replaced, action, _ := reviewChains(strings.NewReader("ctq"), new(strings.Builder), chains("a", "b"), "", palette{}) + if action != 'q' || len(approved) != 0 || len(replaced) != 0 { + t.Errorf("approved = %v replaced = %v action = %q; want nothing", approved, replaced, action) + } +} + +// TestReplaceChains: a replaced file's chain becomes one step of the chosen +// kind on the file itself, under the rule name (review); other chains keep +// their steps. +func TestReplaceChains(t *testing.T) { + cs := chains("a", "b") + cs[0].File.Path = "/dl/a" + cs[0].Steps = append(cs[0].Steps, plan.Step{Kind: plan.Copy, Dst: "/w/copy"}) + got := replaceChains(cs, map[string]plan.Kind{"a": plan.DeletePermanent}) + want := plan.Step{Kind: plan.DeletePermanent, Rule: "(review)", Src: "/dl/a", Reason: "chosen in review"} + if len(got[0].Steps) != 1 || got[0].Steps[0] != want { + t.Errorf("a's steps = %+v; want only %+v", got[0].Steps, want) + } + if len(got[1].Steps) != 1 || got[1].Steps[0].Kind != plan.Move { + t.Errorf("b's steps changed: %+v", got[1].Steps) + } + if len(cs[0].Steps) != 2 { + t.Errorf("replaceChains changed its input: %+v", cs[0].Steps) + } +} diff --git a/cmd/krino/sort.go b/cmd/krino/sort.go index 9e714ba..3e5d951 100644 --- a/cmd/krino/sort.go +++ b/cmd/krino/sort.go @@ -69,6 +69,7 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int { if setMinAge { applyMinAge(e, minAge) } + e.CacheDir = cacheDir() p := palette{on: colourOn(g, stdout)} // Spec §8.4: with neither -y nor -n, krino asks; asking a non-terminal @@ -213,12 +214,13 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int { } var approved map[string]bool + var replaced map[string]plan.Kind var action rune if g.yes { approved, action = approveAll(actionable), 'a' } else { var rerr error - approved, action, rerr = reviewDir(stdout, actionable, d.Root, p) + approved, replaced, action, rerr = reviewDir(stdout, actionable, d.Root, p) if rerr != nil { fmt.Fprintf(stderr, "krino: %s: %v\n", d.Name, rerr) exit = 1 @@ -242,6 +244,9 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int { return true } + // [t] and [d] in review: the file gets the one step chosen + // there instead of the chain its rules planned. + dp.Chains = replaceChains(dp.Chains, replaced) res, aerr := e.Apply(ctx, dp, approved, j, run) if aerr != nil { if errors.Is(aerr, context.Canceled) || errors.Is(aerr, context.DeadlineExceeded) { diff --git a/cmd/krino/undo.go b/cmd/krino/undo.go index d31a5b3..036e3c0 100644 --- a/cmd/krino/undo.go +++ b/cmd/krino/undo.go @@ -325,7 +325,7 @@ func reviewUndoDir(out io.Writer, files []engine.UndoFile, p palette) (map[int]b // // menu, and, for [c], the per-file // -// [y] yes [n] no [a] yes to this and all remaining [d] done, apply chosen so far [q] quit, apply nothing +// [y] yes [n] no [a] yes to this and all remaining [w] write, apply chosen so far [q] quit, apply nothing // // prompt - the same shape as review.go's reviewChains/reviewPerFile, over a // different plan shape (approved is keyed by index into files, not by @@ -386,7 +386,7 @@ func reviewUndoPerFile(in io.Reader, out io.Writer, files []engine.UndoFile, p p continue } - for _, l := range wrapped(" ", perFileKeys, 2, widthPolicy(out), p.keys) { + for _, l := range wrapped(" ", undoPerFileKeys, 2, widthPolicy(out), p.keys) { fmt.Fprintln(out, l) } for { @@ -402,12 +402,12 @@ func reviewUndoPerFile(in io.Reader, out io.Writer, files []engine.UndoFile, p p case 'a': approved[i] = true yesRest = true - case 'd': + case 'w': return approved, false, nil case 'q': return nil, true, nil default: - fmt.Fprintf(out, "%q is not y, n, a, d or q\n", key) + fmt.Fprintf(out, "%q is not y, n, a, w or q\n", key) continue } break @@ -416,6 +416,10 @@ func reviewUndoPerFile(in io.Reader, out io.Writer, files []engine.UndoFile, p p return approved, false, nil } +// undoPerFileKeys is undo's per-file prompt: review's without [t] and [d], +// which make no sense for a reversal. +const undoPerFileKeys = "[y] yes [n] no [a] yes to this and all remaining [w] write, apply chosen so far [q] quit, apply nothing" + // approveAllUndo approves every reversible file in files by index - [a] // apply all, at either the top level or mid per-file review. A refused file // is never marked true: nothing would happen to it anyway (ApplyUndo skips diff --git a/docs/design.md b/docs/design.md index 3130870..271e2a6 100644 --- a/docs/design.md +++ b/docs/design.md @@ -5,7 +5,9 @@ duplicates are never deleted (§5.5), and coloured output with `--no-color` (§8.2, §11); amended again 2026-09-14 for 0.0.3: the plan shown as one block per file, wrapped to the terminal, and `-P` (§8.2, §8.3, §11); amended again 2026-09-14 for 0.0.4: `max-size` (§4.4), `(exclude ...)` -(§4.2, §4.3, §4.6) and `--min-age` (§11). +(§4.2, §4.3, §4.6) and `--min-age` (§11); amended again 2026-09-14 for +0.0.5: the keyword cache (§3, §6.1, §13), and `t`, `d` and `w` in review +(§8.3, §10). krino (from Greek κρίνω, "to separate, to judge, to decide") sorts files in chosen directories by rules. A rule tests a file's type, name, path, size, @@ -59,6 +61,8 @@ $XDG_CONFIG_HOME/krino/ default ~/.config/krino $XDG_STATE_HOME/krino/ default ~/.local/state/krino krino.log the log (§9) <name>.lock per-directory lock while a run is active +$XDG_CACHE_HOME/krino/ default ~/.cache/krino + <name>.cache keyword cache (§6.1), 0600, safe to delete ``` `krino init` creates the config directory with a commented `krino.conf` and @@ -323,6 +327,35 @@ so a keyword split across lines in a PDF still matches. Matching is substring: `"acme"` matches `"acmeco"`. Words hyphenated across lines in a PDF are not rejoined. +### 6.1 Keyword cache + +Extraction is nearly all of a run's time, so krino remembers what it found. +For each file it extracted, `<name>.cache` records which of the +directory's content keywords the text contains, against the full list of +keywords it was checked against. It stores no text and no file names; a +file is known by device, inode, size and modification time, which krino's +own moves and renames keep. The keywords themselves are stored, as they +appear in the config. + +- When a file is extracted, every content keyword of its directory is + answered at once, so one extraction serves every rule and exclude. +- A content test is answered from the cache when the file's entry covers + all of the test's keywords. Otherwise the file is extracted and its entry + replaced: a changed file, or a new keyword, costs one extraction. +- Files above `max-read` are refused before the cache is consulted. + Failures (unreadable, tool missing, timeout) are never cached. +- The cache is discarded whole when the extractor fingerprint changes: a + tool installed, removed or replaced, or krino's extraction code changing + (`extract.Version`). +- Planning a run (`-n` included) reads the cache and writes it back holding + only files still in the directory, under the directory's lock, via a + temporary file renamed into place; the directory is 0700, the file 0600. + `explain` reads it and never writes. A directory with no content tests + has no cache. +- Known gap: a file edited in place with its size and modification time + deliberately preserved keeps its old answers. Deleting + `~/.cache/krino` resets everything. + ## 7. Actions ### 7.1 Chains @@ -525,10 +558,17 @@ list: move → Work/Acme/2026/ rule acme because name \bacme\b - [y] yes [n] no [a] yes to this and all remaining [d] done, apply chosen so far [q] quit, apply nothing + [y] yes [n] no [a] yes to this and all remaining [t] trash [d] delete permanently [w] write, apply chosen so far [q] quit, apply nothing ``` -Approval is per file: the whole chain or none of it. +Approval is per file: the whole chain or none of it. `t` and `d` approve +the file with its chain replaced by a single step on the file itself: to the +Trash, or deleted permanently. They are the user's own decision, logged +under the rule name `(review)`; `(review)` steps are not subject to the +duplicate protection of §5.5, which governs rules. `d` asks +`delete NAME permanently? [y/N]`, and any key but `y` deletes nothing and +asks about the file again. `w` stops asking and applies what was chosen so +far. `q` applies nothing in this directory, choices included. ### 8.4 Modes @@ -569,7 +609,7 @@ not logged; declined files are. is undone by naming it: `krino undo RUN`. Undo runs cannot themselves be undone. - Undo builds a plan like any other, shown and approved the same way - (`-y` and `-n` apply). + (`-y` and `-n` apply). Its per-file prompt has no `t` or `d`. Reversals, last step first within each file: @@ -675,6 +715,11 @@ Rules for the GUI to come: about krino, and would fail or pass for reasons outside its control. Report the measurement; promise nothing. `make bench` runs the Go benchmarks on generated trees. +- Keyword cache (§6.1, 0.0.5): measured on the same kind of folder, 170 + files, a dry run took 12.78 s with an empty cache and 0.13 s with a warm + one, printing the identical plan. Without the cache, 59 `pdftotext` runs + took 21.5 s of wall time between them, one PDF 14.7 s alone; with + extraction answered instantly, the run took 0.25 s. ## 14. Build, dependencies, release diff --git a/internal/cond/compile.go b/internal/cond/compile.go index 590d61a..94118ac 100644 --- a/internal/cond/compile.go +++ b/internal/cond/compile.go @@ -266,6 +266,7 @@ func (c *compiler) compileContent(n *sexp.Node) *node { continue } keywords = append(keywords, keyword{norm: normed, src: a.Text}) + c.cond.Keywords = append(c.cond.Keywords, Keyword{Opt: c.opt, Norm: normed}) } if bad { return nil diff --git a/internal/cond/compile_test.go b/internal/cond/compile_test.go index 2a07ad6..8432365 100644 --- a/internal/cond/compile_test.go +++ b/internal/cond/compile_test.go @@ -165,3 +165,24 @@ func TestGroupsMatchSpec(t *testing.T) { } } } + +// TestCompileCollectsKeywords: every content keyword is recorded as +// compared, normalised under the compile options, and its key names those +// options. +func TestCompileCollectsKeywords(t *testing.T) { + opt := Options{IgnoreCase: true, Fold: true} + c, errs := Compile("d.conf", nodes(t, `(or (content "Spółka" "x") (not (content "NIP")))`), opt) + if len(errs) > 0 { + t.Fatal(errs) + } + want := []Keyword{{Opt: opt, Norm: "spolka"}, {Opt: opt, Norm: "x"}, {Opt: opt, Norm: "nip"}} + if !reflect.DeepEqual(c.Keywords, want) { + t.Errorf("Keywords = %+v, want %+v", c.Keywords, want) + } + if got := want[0].Key(); got != "if:spolka" { + t.Errorf("Key = %q", got) + } + if got := KeywordKey(Options{}, "NIP"); got != "sn:NIP" { + t.Errorf("strict, unfolded key = %q", got) + } +} diff --git a/internal/cond/eval.go b/internal/cond/eval.go index e3093af..3cec110 100644 --- a/internal/cond/eval.go +++ b/internal/cond/eval.go @@ -19,7 +19,10 @@ type Facts interface { Size() int64 ModTime() time.Time Now() time.Time - Content(ignoreCase, fold bool) (string, error) // normalised with norm.Text + // ContentContains reports the index of the first of keywords, each + // normalised under opt with norm.Text, that the file's text contains, or + // -1 when it contains none. + ContentContains(opt Options, keywords []string) (int, error) Duplicate(dirs []string) (original string, ok bool, err error) Matched() bool // an earlier rule matched this file } @@ -167,14 +170,16 @@ func (c *Cond) evalLeaf(n *node, f Facts) (ok bool, reason, warn string, caps [] return false, "", "", nil case kContent: - text, err := f.Content(c.opt.IgnoreCase, c.opt.Fold) + norms := make([]string, len(n.keywords)) + for i, kw := range n.keywords { + norms[i] = kw.norm + } + i, err := f.ContentContains(c.opt, norms) if err != nil { return false, "", "content unreadable: " + err.Error(), nil } - for _, kw := range n.keywords { - if strings.Contains(text, kw.norm) { - return true, `content "` + kw.src + `"`, "", nil - } + if i >= 0 { + return true, `content "` + n.keywords[i].src + `"`, "", nil } return false, "", "", nil diff --git a/internal/cond/eval_test.go b/internal/cond/eval_test.go index 3607978..8891de7 100644 --- a/internal/cond/eval_test.go +++ b/internal/cond/eval_test.go @@ -37,12 +37,18 @@ func (f *fake) Size() int64 { return f.size } func (f *fake) ModTime() time.Time { return now.Add(-f.age) } func (f *fake) Now() time.Time { return now } func (f *fake) Matched() bool { return f.matched } -func (f *fake) Content(ic, fold bool) (string, error) { +func (f *fake) ContentContains(opt Options, keywords []string) (int, error) { f.contentCalls++ if f.rawErr != nil { - return "", f.rawErr + return -1, f.rawErr } - return norm.Text(f.raw, ic, fold), nil + text := norm.Text(f.raw, opt.IgnoreCase, opt.Fold) + for i, kw := range keywords { + if strings.Contains(text, kw) { + return i, nil + } + } + return -1, nil } func (f *fake) Duplicate(dirs []string) (string, bool, error) { return f.dupOrig, f.dupOK, nil } diff --git a/internal/cond/types.go b/internal/cond/types.go index 9455f63..28a4fc9 100644 --- a/internal/cond/types.go +++ b/internal/cond/types.go @@ -24,6 +24,30 @@ type Cond struct { opt Options // the case/fold settings conditions were compiled with; Task 8 needs them again at eval time UsesContent bool // some content test exists DupDirs [][]string // the raw directory arguments of each duplicate test, in order + Keywords []Keyword // every content keyword, as compiled, in order +} + +// Keyword is one content keyword as a content test compares it: normalised +// under the options it was compiled with. +type Keyword struct { + Opt Options + Norm string +} + +// Key is the keyword's identity across runs, for the keyword cache: its +// options and its normalised text. +func (k Keyword) Key() string { return KeywordKey(k.Opt, k.Norm) } + +// KeywordKey is Keyword.Key for opt and an already normalised keyword. +func KeywordKey(opt Options, norm string) string { + b := []byte("sn:") + if opt.IgnoreCase { + b[0] = 'i' + } + if opt.Fold { + b[1] = 'f' + } + return string(b) + norm } // kind is what a compiled node tests, or how it combines its children. diff --git a/internal/engine/cache_test.go b/internal/engine/cache_test.go new file mode 100644 index 0000000..eee2860 --- /dev/null +++ b/internal/engine/cache_test.go @@ -0,0 +1,224 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package engine + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// countingPDFTree makes ~/dl holding the given .pdf files, and a fake +// pdftotext first on PATH that prints the file's own bytes as its text and +// counts its runs in a file; a file whose name contains "fail" makes it +// exit 1. +func countingPDFTree(t *testing.T, files map[string]string) (home, dl string, runs func() int) { + t.Helper() + home, dl = excludeTree(t, files) + bin := t.TempDir() + count := filepath.Join(t.TempDir(), "count") + script := "#!/bin/sh\necho x >> '" + count + "'\ncase \"$4\" in *fail*) exit 1;; esac\nexec /bin/cat \"$4\"\n" + if err := os.WriteFile(filepath.Join(bin, "pdftotext"), []byte(script), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", bin) + runs = func() int { + b, _ := os.ReadFile(count) + return strings.Count(string(b), "x") + } + return home, dl, runs +} + +// cachedMatch loads the configuration afresh, as a new krino process +// would, and matches dl with the cache under ~/.cache/krino. +func cachedMatch(t *testing.T, home, main string) *Result { + t.Helper() + e, errs := Load(main) + if len(errs) > 0 { + t.Fatal(errs) + } + e.CacheDir = filepath.Join(home, ".cache", "krino") + r, err := e.Match(context.Background(), e.Dirs[0]) + if err != nil { + t.Fatal(err) + } + return r +} + +func matchedNames(r *Result) string { + var names []string + for _, fm := range r.Matched { + names = append(names, fm.File.Rel) + } + return strings.Join(names, " ") +} + +const acmeRules = ` +(path "~/dl") +(rule "acme" (when (content "acme")) (move "Acme")) +` + +// TestCacheSkipsExtractionOnRerun: the second run over unchanged files +// extracts nothing and matches the same files. +func TestCacheSkipsExtractionOnRerun(t *testing.T) { + home, _, runs := countingPDFTree(t, map[string]string{"a.pdf": "Invoice ACME", "b.pdf": "nothing here"}) + main := writeConfig(t, home, `(include "dl")`, map[string]string{"dl": acmeRules}) + + first := cachedMatch(t, home, main) + if runs() != 2 || matchedNames(first) != "a.pdf" { + t.Fatalf("first run: %d extractions, matched %q", runs(), matchedNames(first)) + } + second := cachedMatch(t, home, main) + if runs() != 2 { + t.Errorf("second run extracted again: %d runs in all", runs()) + } + if matchedNames(second) != "a.pdf" { + t.Errorf("second run matched %q, want a.pdf", matchedNames(second)) + } +} + +// TestCacheRereadsChangedFile: a file whose modification time changed is +// extracted again; the other is not. +func TestCacheRereadsChangedFile(t *testing.T) { + home, dl, runs := countingPDFTree(t, map[string]string{"a.pdf": "Invoice ACME", "b.pdf": "nothing here"}) + main := writeConfig(t, home, `(include "dl")`, map[string]string{"dl": acmeRules}) + cachedMatch(t, home, main) + + later := time.Date(2026, 2, 1, 0, 0, 0, 0, time.UTC) + if err := os.WriteFile(filepath.Join(dl, "b.pdf"), []byte("now ACME too"), 0o644); err != nil { + t.Fatal(err) + } + os.Chtimes(filepath.Join(dl, "b.pdf"), later, later) + r := cachedMatch(t, home, main) + if runs() != 3 { + t.Errorf("%d extractions in all, want 3: only b.pdf read again", runs()) + } + if matchedNames(r) != "a.pdf b.pdf" { + t.Errorf("matched %q, want both", matchedNames(r)) + } +} + +// TestCacheRereadsForNewKeyword: a keyword no entry was checked against +// makes the files that reach it be read again, once. +func TestCacheRereadsForNewKeyword(t *testing.T) { + home, _, runs := countingPDFTree(t, map[string]string{"a.pdf": "Invoice ACME", "b.pdf": "nothing here"}) + main := writeConfig(t, home, `(include "dl")`, map[string]string{"dl": acmeRules}) + cachedMatch(t, home, main) + + writeConfig(t, home, `(include "dl")`, map[string]string{"dl": acmeRules + `(rule "nothing" (when (content "nothing")) (move "Nothing"))` + "\n"}) + r := cachedMatch(t, home, main) + if runs() != 4 { + t.Errorf("%d extractions in all, want 4: both read again for the new keyword", runs()) + } + if matchedNames(r) != "a.pdf b.pdf" { + t.Errorf("matched %q, want both", matchedNames(r)) + } + cachedMatch(t, home, main) + if runs() != 4 { + t.Errorf("third run extracted again: %d runs in all", runs()) + } +} + +// TestCacheOffWithoutCacheDir: an engine with no CacheDir reads every time +// and writes no cache. +func TestCacheOffWithoutCacheDir(t *testing.T) { + home, _, runs := countingPDFTree(t, map[string]string{"a.pdf": "Invoice ACME"}) + main := writeConfig(t, home, `(include "dl")`, map[string]string{"dl": acmeRules}) + for i := 0; i < 2; i++ { + e, errs := Load(main) + if len(errs) > 0 { + t.Fatal(errs) + } + if _, err := e.Match(context.Background(), e.Dirs[0]); err != nil { + t.Fatal(err) + } + } + if runs() != 2 { + t.Errorf("%d extractions, want one per run", runs()) + } + if _, err := os.Stat(filepath.Join(home, ".cache")); !os.IsNotExist(err) { + t.Errorf("a cache was written with no CacheDir: %v", err) + } +} + +// TestCacheDoesNotStoreFailures: a file the tool fails on is tried again +// on every run, and warned about every time. +func TestCacheDoesNotStoreFailures(t *testing.T) { + home, _, runs := countingPDFTree(t, map[string]string{"fail.pdf": "Invoice ACME"}) + main := writeConfig(t, home, `(include "dl")`, map[string]string{"dl": acmeRules}) + for i := 1; i <= 2; i++ { + r := cachedMatch(t, home, main) + if runs() != i { + t.Errorf("run %d: %d extractions in all, want %d", i, runs(), i) + } + if len(r.Unmatched) != 1 || len(r.Unmatched[0].Warnings) == 0 { + t.Errorf("run %d: want fail.pdf unmatched with a warning: %+v", i, r.Unmatched) + } + } +} + +// TestCacheRespectsMaxRead: a file over max-read is not read, even when an +// earlier run cached its answers. +func TestCacheRespectsMaxRead(t *testing.T) { + home, _, _ := countingPDFTree(t, map[string]string{"a.pdf": "Invoice ACME"}) + main := writeConfig(t, home, `(include "dl")`, map[string]string{"dl": acmeRules}) + cachedMatch(t, home, main) + + writeConfig(t, home, `(include "dl")`, map[string]string{"dl": "(max-read 1)\n" + acmeRules}) + r := cachedMatch(t, home, main) + if len(r.Matched) != 0 || len(r.Unmatched) != 1 || !strings.Contains(strings.Join(r.Unmatched[0].Warnings, " "), "larger than max-read") { + t.Errorf("want a.pdf unmatched as larger than max-read: matched %+v unmatched %+v", r.Matched, r.Unmatched) + } +} + +// TestCacheHoldsNoText: the cache file holds the keywords and answers, not +// the extracted text or the file's name. +func TestCacheHoldsNoText(t *testing.T) { + home, _, _ := countingPDFTree(t, map[string]string{"secret-name.pdf": "Invoice ACME confidential"}) + main := writeConfig(t, home, `(include "dl")`, map[string]string{"dl": acmeRules}) + cachedMatch(t, home, main) + b, err := os.ReadFile(filepath.Join(home, ".cache", "krino", "dl.cache")) + if err != nil { + t.Fatal(err) + } + for _, leak := range []string{"Invoice", "invoice", "confidential", "secret-name"} { + if strings.Contains(string(b), leak) { + t.Errorf("cache holds %q:\n%s", leak, b) + } + } +} + +// TestExplainUsesCacheWithoutWriting: explain answers from the cache a run +// wrote, and never writes one itself. +func TestExplainUsesCacheWithoutWriting(t *testing.T) { + home, dl, runs := countingPDFTree(t, map[string]string{"a.pdf": "Invoice ACME"}) + main := writeConfig(t, home, `(include "dl")`, map[string]string{"dl": acmeRules}) + explain := func() { + t.Helper() + e, errs := Load(main) + if len(errs) > 0 { + t.Fatal(errs) + } + e.CacheDir = filepath.Join(home, ".cache", "krino") + x, err := e.Explain(context.Background(), filepath.Join(dl, "a.pdf")) + if err != nil { + t.Fatal(err) + } + if len(x.Rules) != 1 || !x.Rules[0].Match { + t.Errorf("explain: %+v", x.Rules) + } + } + explain() + if _, err := os.Stat(filepath.Join(home, ".cache")); !os.IsNotExist(err) { + t.Errorf("explain wrote a cache: %v", err) + } + cachedMatch(t, home, main) + before := runs() + explain() + if runs() != before { + t.Errorf("explain extracted although the run cached a.pdf") + } +} diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 8cffbd1..b50728e 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -8,6 +8,7 @@ package engine import ( "fmt" "os" + "sort" "strings" "time" @@ -26,6 +27,11 @@ type Engine struct { Extract *extract.Extractor Now func() time.Time // time.Now; tests replace it MainFile string + + // CacheDir holds each directory's keyword cache (spec §6.1), as + // NAME.cache; "" means no cache is read or written. Load leaves it + // empty: the command line sets it. + CacheDir string } // Dir is one configured directory, with its ignore matcher and rules @@ -38,13 +44,11 @@ type Dir struct { Ignore *ignore.Matcher Rules []*Rule - // ContentVariants is the distinct (IgnoreCase, Fold) pairs any of - // Rules' content tests evaluate under, in first-seen order. B2: when - // this holds exactly one variant, facts.Content releases a file's raw - // extracted text once that variant's normalised copy exists, since no - // other variant will ever be asked for; with more than one, both must - // stay memoised, as before. - ContentVariants []cond.Options + // ContentKeywords is every content keyword the directory's excludes + // and rules test, once each, sorted by Key. When a file's text is + // extracted, every one of them is answered at once, so one extraction + // serves every content test and fills the keyword cache. + ContentKeywords []cond.Keyword // Excludes are the (exclude ...) forms that apply here, compiled with the // directory's settings: krino.conf's first, then the directory's own. A @@ -135,7 +139,7 @@ func Load(mainFile string, names ...string) (*Engine, []*config.Diag) { } } } - dir.ContentVariants = contentVariants(dir.Rules, dir.Excludes, dirOpt) + dir.ContentKeywords = contentKeywords(dir.Rules, dir.Excludes) dir.DupScopes = dupScopes(dir.Rules) dirs = append(dirs, dir) } @@ -216,32 +220,26 @@ func dedupeNames(names []string) []string { return out } -// contentVariants returns the distinct (IgnoreCase, Fold) pairs any content -// test evaluates under - each exclude's (under the directory's settings, -// dirOpt) and each rule's - in first-seen order — B2's per-Dir -// ContentVariants. A rule whose condition has no content test at all -// (Cond.UsesContent false) never calls facts.Content, so its resolved -// case/fold settings contribute no variant here. -func contentVariants(rules []*Rule, excludes []*Exclude, dirOpt cond.Options) []cond.Options { - var out []cond.Options - seen := map[cond.Options]bool{} - for _, x := range excludes { - if x.Cond.UsesContent && !seen[dirOpt] { - seen[dirOpt] = true - out = append(out, dirOpt) +// contentKeywords returns every content keyword excludes and rules test, +// each once, sorted by Key: Dir.ContentKeywords. +func contentKeywords(rules []*Rule, excludes []*Exclude) []cond.Keyword { + seen := map[string]bool{} + var out []cond.Keyword + add := func(c *cond.Cond) { + for _, k := range c.Keywords { + if !seen[k.Key()] { + seen[k.Key()] = true + out = append(out, k) + } } } + for _, x := range excludes { + add(x.Cond) + } for _, r := range rules { - if !r.Cond.UsesContent { - continue - } - opt := cond.Options{IgnoreCase: r.Settings.Case == config.CaseIgnore, Fold: r.Settings.Fold} - if seen[opt] { - continue - } - seen[opt] = true - out = append(out, opt) + add(r.Cond) } + sort.Slice(out, func(i, j int) bool { return out[i].Key() < out[j].Key() }) return out } diff --git a/internal/engine/engine_test.go b/internal/engine/engine_test.go index 18b35e7..c38c1fc 100644 --- a/internal/engine/engine_test.go +++ b/internal/engine/engine_test.go @@ -46,14 +46,14 @@ func writeConfig(t *testing.T, home, main string, dirs map[string]string) string // fakeFacts is a minimal cond.Facts for checking compiled rules. type fakeFacts struct{ name string } -func (f fakeFacts) Name() string { return f.name } -func (f fakeFacts) Rel() string { return f.name } -func (f fakeFacts) Size() int64 { return 1 } -func (f fakeFacts) ModTime() time.Time { return time.Time{} } -func (f fakeFacts) Now() time.Time { return time.Time{} } -func (f fakeFacts) Content(bool, bool) (string, error) { return "", nil } -func (f fakeFacts) Duplicate([]string) (string, bool, error) { return "", false, nil } -func (f fakeFacts) Matched() bool { return false } +func (f fakeFacts) Name() string { return f.name } +func (f fakeFacts) Rel() string { return f.name } +func (f fakeFacts) Size() int64 { return 1 } +func (f fakeFacts) ModTime() time.Time { return time.Time{} } +func (f fakeFacts) Now() time.Time { return time.Time{} } +func (f fakeFacts) ContentContains(cond.Options, []string) (int, error) { return -1, nil } +func (f fakeFacts) Duplicate([]string) (string, bool, error) { return "", false, nil } +func (f fakeFacts) Matched() bool { return false } var _ cond.Facts = fakeFacts{} @@ -187,12 +187,10 @@ func TestLoadAcceptsSuppliedCaptures(t *testing.T) { } } -// 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 -// nothing; two rules sharing a variant fold into one; a rule-level (case -// ignore) override adds a second. -func TestContentVariantsComputedAtLoad(t *testing.T) { +// TestContentKeywordsComputedAtLoad: Load collects every content keyword +// of a directory's rules, once per (options, normalised keyword), sorted by +// key: a rule-level (case ignore) makes the same word a second keyword. +func TestContentKeywordsComputedAtLoad(t *testing.T) { h := sandbox(t) os.Mkdir(filepath.Join(h, "dl"), 0o755) main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": ` @@ -207,10 +205,11 @@ func TestContentVariantsComputedAtLoad(t *testing.T) { if len(errs) > 0 { t.Fatal(errs) } - got := e.Dirs[0].ContentVariants - want := []cond.Options{{IgnoreCase: false, Fold: true}, {IgnoreCase: true, Fold: true}} + got := e.Dirs[0].ContentKeywords + strict, loose := cond.Options{IgnoreCase: false, Fold: true}, cond.Options{IgnoreCase: true, Fold: true} + want := []cond.Keyword{{Opt: loose, Norm: "acme"}, {Opt: strict, Norm: "acme"}, {Opt: strict, Norm: "other"}} if !reflect.DeepEqual(got, want) { - t.Fatalf("ContentVariants = %+v, want %+v", got, want) + t.Fatalf("ContentKeywords = %+v, want %+v", got, want) } } diff --git a/internal/engine/exclude_test.go b/internal/engine/exclude_test.go index 40b3548..8b26450 100644 --- a/internal/engine/exclude_test.go +++ b/internal/engine/exclude_test.go @@ -115,10 +115,10 @@ func TestExcludeNeedsEveryCondition(t *testing.T) { } } -// TestExcludeContentKeepsRuleContentVariants: an exclude reading content -// under the directory's settings must not release the raw text a rule -// with different case/fold settings still needs. -func TestExcludeContentKeepsRuleContentVariants(t *testing.T) { +// TestExcludeContentAndRuleContentKeepTheirOwnSettings: an exclude reading +// content under the directory's settings first must not change how a rule +// with different case/fold settings sees the same text. +func TestExcludeContentAndRuleContentKeepTheirOwnSettings(t *testing.T) { h, _ := excludeTree(t, map[string]string{"a.txt": "Invoice ACME"}) main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": ` (path "~/dl") diff --git a/internal/engine/facts.go b/internal/engine/facts.go index 5790e95..9035245 100644 --- a/internal/engine/facts.go +++ b/internal/engine/facts.go @@ -12,6 +12,8 @@ import ( "krino/internal/cond" "krino/internal/dup" + "krino/internal/extract" + "krino/internal/kwcache" "krino/internal/norm" "krino/internal/plan" "krino/internal/scan" @@ -30,6 +32,7 @@ type matchRun struct { ctx context.Context now time.Time files []scan.File + cache *kwcache.Cache // nil: no keyword cache mu sync.Mutex dupOnce map[string]*sync.Once @@ -103,26 +106,25 @@ func (run *matchRun) dupIndex(key string, dirs []string) *dup.Index { } // facts is one file's cond.Facts. It is used by exactly one goroutine, so -// its own memoised state (content, its normalised variants, and whether an -// earlier rule matched) needs no locking of its own; only the matchRun it -// points at is shared. +// its own memoised state (the keyword answers, and whether an earlier rule +// matched) needs no locking of its own; only the matchRun it points at is +// shared. type facts struct { run *matchRun file scan.File matched bool - contentDone bool - content string - contentErr error - normCache map[[2]bool]string + contentDone bool // extraction was attempted + contentErr error // why it failed + answers map[string]bool // by cond.KeywordKey, once extracted } var _ cond.Facts = (*facts)(nil) // newFacts builds the Facts for one scanned file. func newFacts(run *matchRun, file scan.File) *facts { - return &facts{run: run, file: file, normCache: make(map[[2]bool]string)} + return &facts{run: run, file: file} } func (f *facts) Name() string { return f.file.Name } @@ -132,32 +134,85 @@ func (f *facts) ModTime() time.Time { return f.file.ModTime } func (f *facts) Now() time.Time { return f.run.now } func (f *facts) Matched() bool { return f.matched } -// Content extracts the file's text once, then normalises it per -// (ignoreCase, fold) variant, memoising each. B2: when the directory's -// rules use exactly one variant (Dir.ContentVariants), the raw text is -// released as soon as that variant's normalised copy exists — no other -// variant will ever be asked for, so there is no reason to keep both the -// raw text and its normalised copy in memory at once. A directory using -// more than one variant keeps the raw text for as long as f lives, exactly -// as before. -func (f *facts) Content(ignoreCase, fold bool) (string, error) { +// ContentContains answers a content test (spec §6.1). A file above +// max-read is never read, cached or not. Before the file has been +// extracted this run, the keyword cache answers when it knows every one of +// keywords for this file as it is now; otherwise the text is extracted +// once, every keyword of the directory (and of this test) is answered from +// it and stored in the cache, and the text itself is dropped. A failed +// extraction is not cached: the next run tries again. +func (f *facts) ContentContains(opt cond.Options, keywords []string) (int, error) { + if max := f.run.d.Settings.MaxRead; max > 0 && f.file.Size > max { + return -1, extract.ErrTooLarge + } + keys := make([]string, len(keywords)) + for i, kw := range keywords { + keys[i] = cond.KeywordKey(opt, kw) + } if !f.contentDone { - f.content, f.contentErr = f.run.e.Extract.Text(f.run.ctx, f.file.Path, f.file.Size, f.run.d.Settings.MaxRead) - f.contentDone = true + if id, ok := f.cacheID(); ok { + if hits, ok := f.run.cache.Lookup(id, keys); ok { + for i, hit := range hits { + if hit { + return i, nil + } + } + return -1, nil + } + } + f.extract(opt, keywords) } if f.contentErr != nil { - return "", f.contentErr + return -1, f.contentErr + } + for i, k := range keys { + if f.answers[k] { + return i, nil + } + } + return -1, nil +} + +// extract reads the file's text and answers every keyword of the directory, +// plus the asking test's (opt, keywords), from it. +func (f *facts) extract(opt cond.Options, keywords []string) { + f.contentDone = true + text, err := f.run.e.Extract.Text(f.run.ctx, f.file.Path, f.file.Size, f.run.d.Settings.MaxRead) + if err != nil { + f.contentErr = err + return + } + all := append([]cond.Keyword(nil), f.run.d.ContentKeywords...) + for _, kw := range keywords { + all = append(all, cond.Keyword{Opt: opt, Norm: kw}) + } + normed := map[cond.Options]string{} + f.answers = make(map[string]bool, len(all)) + for _, k := range all { + t, ok := normed[k.Opt] + if !ok { + t = norm.Text(text, k.Opt.IgnoreCase, k.Opt.Fold) + normed[k.Opt] = t + } + f.answers[k.Key()] = strings.Contains(t, k.Norm) } - key := [2]bool{ignoreCase, fold} - if v, ok := f.normCache[key]; ok { - return v, nil + if id, ok := f.cacheID(); ok { + f.run.cache.Store(id, f.answers) } - v := norm.Text(f.content, ignoreCase, fold) - f.normCache[key] = v - if len(f.run.d.ContentVariants) == 1 { - f.content = "" +} + +// cacheID is the file's keyword cache identity; ok is false when there is +// no cache, or the platform gave the file no inode. +func (f *facts) cacheID() (kwcache.ID, bool) { + if f.run.cache == nil || f.file.Ino == 0 { + return kwcache.ID{}, false } - return v, nil + return fileCacheID(f.file), true +} + +// fileCacheID is file's kwcache.ID. +func fileCacheID(file scan.File) kwcache.ID { + return kwcache.ID{Dev: file.Dev, Ino: file.Ino, Size: file.Size, MTime: file.ModTime.UnixNano()} } // Duplicate resolves dirs against the directory's root, builds (or reuses) diff --git a/internal/engine/facts_test.go b/internal/engine/facts_test.go index 7d844a0..9739fa6 100644 --- a/internal/engine/facts_test.go +++ b/internal/engine/facts_test.go @@ -3,79 +3,10 @@ package engine import ( - "context" - "os" "path/filepath" "testing" - "time" - - "krino/internal/cond" - "krino/internal/extract" - "krino/internal/scan" ) -// contentFacts builds a *facts for a real text file, under a Dir whose -// ContentVariants is variants, for exercising B2's raw-release directly. -func contentFacts(t *testing.T, body string, variants []cond.Options) *facts { - t.Helper() - p := filepath.Join(t.TempDir(), "a.txt") - if err := os.WriteFile(p, []byte(body), 0o644); err != nil { - t.Fatal(err) - } - fi, err := os.Stat(p) - if err != nil { - t.Fatal(err) - } - e := &Engine{Extract: extract.New(), Now: time.Now} - d := &Dir{Name: "d", ContentVariants: variants} - file := scan.File{Path: p, Rel: "a.txt", Name: "a.txt", Size: fi.Size(), ModTime: fi.ModTime()} - run := newMatchRun(e, d, context.Background(), time.Now(), []scan.File{file}) - return newFacts(run, file) -} - -// TestContentReleasesRawWithOneVariant: B2. A directory whose rules use -// exactly one (ignoreCase, fold) variant releases the raw extracted text -// once that variant's normalised copy exists. -func TestContentReleasesRawWithOneVariant(t *testing.T) { - f := contentFacts(t, "Hello World", []cond.Options{{IgnoreCase: true, Fold: false}}) - const want = "hello world" - got, err := f.Content(true, false) - if err != nil { - t.Fatal(err) - } - if got != want { - t.Fatalf("got %q, want %q", got, want) - } - if f.content != "" { - t.Errorf("raw text not released with a single content variant: %q", f.content) - } - // A second call for the same (already cached) variant must still work - // from normCache, without needing the released raw text. - if got, err := f.Content(true, false); err != nil || got != want { - t.Errorf("second call for the cached variant: got %q, %v, want %q", got, err, want) - } -} - -// TestContentKeepsRawWithTwoVariants: B2. A directory whose rules use two -// distinct variants must not release the raw text after the first: the -// second variant still needs it, and normalising it correctly (not from an -// emptied string) is the proof the raw text was kept. -func TestContentKeepsRawWithTwoVariants(t *testing.T) { - f := contentFacts(t, "Hello World", []cond.Options{ - {IgnoreCase: true, Fold: false}, - {IgnoreCase: false, Fold: false}, - }) - if got, err := f.Content(true, false); err != nil || got != "hello world" { - t.Fatalf("first variant: got %q, %v", got, err) - } - if f.content == "" { - t.Fatal("raw text released after only the first of two variants") - } - if got, err := f.Content(false, false); err != nil || got != "Hello World" { - t.Fatalf("second variant: got %q, %v, want the unfolded original (raw text must still be available)", got, err) - } -} - // TestDisplayOriginalAbbreviatesHome: a duplicate's original is shown // root-relative inside the root, and home-abbreviated outside it, the way // every other user-visible path is; a path outside $HOME stays absolute. diff --git a/internal/engine/match.go b/internal/engine/match.go index 8daea9d..1a8077a 100644 --- a/internal/engine/match.go +++ b/internal/engine/match.go @@ -16,6 +16,7 @@ import ( "krino/internal/cond" "krino/internal/config" + "krino/internal/kwcache" "krino/internal/plan" "krino/internal/scan" "krino/internal/xdg" @@ -70,6 +71,7 @@ func (e *Engine) Match(ctx context.Context, d *Dir) (*Result, error) { } run := newMatchRun(e, d, ctx, now, wres.Files) + cacheWarn := e.openCache(run) fileMatches := make([]FileMatch, len(wres.Files)) workers := runtime.GOMAXPROCS(0) @@ -103,7 +105,18 @@ func (e *Engine) Match(ctx context.Context, d *Dir) (*Result, error) { } } - warnings := run.warnings() + warnings := append(run.warnings(), cacheWarn...) + if run.cache != nil { + ids := make([]kwcache.ID, 0, len(wres.Files)) + for _, f := range wres.Files { + if f.Ino != 0 { + ids = append(ids, fileCacheID(f)) + } + } + if err := run.cache.Save(e.cacheFile(d), ids); err != nil { + warnings = append(warnings, "cache: "+err.Error()) + } + } sort.Strings(warnings) return &Result{ @@ -242,20 +255,14 @@ func (e *Engine) Explain(ctx context.Context, path string) (*Explanation, error) } rel = filepath.ToSlash(rel) - sf := scan.File{ - Path: abs, - Rel: rel, - Name: filepath.Base(abs), - Size: info.Size(), - ModTime: info.ModTime(), - Mode: info.Mode(), - } + sf := scan.NewFile(abs, rel, info) now := e.Now() excl := e.excludeDirs(d) skip := explainSkip(d, sf, excl, now) run := newMatchRun(e, d, ctx, now, e.filesForExplain(d, sf, excl, now)) + e.openCache(run) // read only: Explain never writes the cache f := newFacts(run, sf) var excludes []ExcludeTrace @@ -289,6 +296,26 @@ func (e *Engine) Explain(ctx context.Context, path string) (*Explanation, error) return &Explanation{Dir: d, File: sf, Skip: skip, Excludes: excludes, Excluded: excluded, Rules: rules}, nil } +// cacheFile is d's keyword cache file. +func (e *Engine) cacheFile(d *Dir) string { + return filepath.Join(e.CacheDir, d.Name+".cache") +} + +// openCache loads run's directory keyword cache into run.cache, when the +// engine has a CacheDir and the directory has content tests at all. A cache +// that cannot be read is replaced by an empty one, reported as a warning. +func (e *Engine) openCache(run *matchRun) []string { + if e.CacheDir == "" || len(run.d.ContentKeywords) == 0 { + return nil + } + c, err := kwcache.Load(e.cacheFile(run.d), e.Extract.Fingerprint()) + run.cache = c + if err != nil { + return []string{"cache: " + err.Error() + " (starting a new one)"} + } + return nil +} + // filesForExplain returns the file set Explain's duplicate checks run // against: the directory's ordinary scan, plus the explained file itself // when that scan would not have reached it (it is busy, ignored, too new, diff --git a/internal/extract/extract.go b/internal/extract/extract.go index aa3ae93..9768db1 100644 --- a/internal/extract/extract.go +++ b/internal/extract/extract.go @@ -7,12 +7,18 @@ package extract import ( "context" "errors" + "fmt" "os" "path/filepath" "strings" "time" ) +// Version is the version of the text this package extracts. Bump it +// whenever a change could make any format's text differ, so every keyword +// cache built from the old text is discarded (Fingerprint). +const Version = 1 + var ( // ErrUnsupported is returned when the format carries no text krino // knows how to extract. @@ -111,6 +117,26 @@ func newWithPath(path string) *Extractor { return &Extractor{tools: tools, Timeout: 30 * time.Second} } +// Fingerprint identifies what text this Extractor would produce: Version, +// and each external tool found with its path, size and modification time. +// A keyword cache built under another fingerprint is discarded, so +// installing, removing or upgrading a tool invalidates it. +func (e *Extractor) Fingerprint() string { + var b strings.Builder + fmt.Fprintf(&b, "v%d", Version) + for _, name := range toolNames { + p := e.tools[name] + if p == "" { + continue + } + fmt.Fprintf(&b, " %s=%s", name, p) + if fi, err := os.Stat(p); err == nil { + fmt.Fprintf(&b, ":%d:%d", fi.Size(), fi.ModTime().UnixNano()) + } + } + return b.String() +} + // Tools reports every external tool Extractor knows about, in a fixed // order, with the path it was found at or "" if it was not found. func (e *Extractor) Tools() []Tool { diff --git a/internal/extract/tools_test.go b/internal/extract/tools_test.go index 4a03f1b..b481e8a 100644 --- a/internal/extract/tools_test.go +++ b/internal/extract/tools_test.go @@ -237,3 +237,27 @@ func TestMaxReadCapsToolOutput(t *testing.T) { t.Fatalf("got %v, want ErrTooLarge (max-read 1024 should have capped a 200000-byte tool output)", err) } } + +// TestFingerprintFollowsTools: the fingerprint changes when a tool is +// installed or replaced, and not otherwise. +func TestFingerprintFollowsTools(t *testing.T) { + bin := t.TempDir() + none := newWithPath(bin).Fingerprint() + if none != newWithPath(bin).Fingerprint() { + t.Fatal("fingerprint not stable") + } + tool := filepath.Join(bin, "pdftotext") + if err := os.WriteFile(tool, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + installed := newWithPath(bin).Fingerprint() + if installed == none { + t.Error("installing pdftotext did not change the fingerprint") + } + if err := os.WriteFile(tool, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatal(err) + } + if newWithPath(bin).Fingerprint() == installed { + t.Error("replacing pdftotext did not change the fingerprint") + } +} diff --git a/internal/kwcache/kwcache.go b/internal/kwcache/kwcache.go new file mode 100644 index 0000000..b61cfb1 --- /dev/null +++ b/internal/kwcache/kwcache.go @@ -0,0 +1,231 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +// Package kwcache remembers which content keywords a file's extracted text +// contains, so a file that has not changed is not extracted again (spec +// §6.1). It stores answers only, never the text, and knows a file by its +// ID, never by its name. +package kwcache + +import ( + "encoding/json" + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "sort" + "strings" + "sync" +) + +// version is the on-disk format; a file of any other version loads empty. +const version = 1 + +// ID is how a file is recognised: the same inode with the same size and +// modification time is taken to hold the same content. A move or rename +// within one filesystem keeps it. +type ID struct { + Dev, Ino uint64 + Size int64 + MTime int64 // Unix nanoseconds +} + +// entry is what is known about one file: every keyword it was checked +// against, and those its text contains. +type entry struct { + checked []string // sorted + hits map[string]bool +} + +// Cache holds one directory's answers: those loaded from disk and those +// stored during this run. It is safe for concurrent use. +type Cache struct { + fingerprint string + existed bool // Load found a file, so Save must rewrite it even when empty + + mu sync.Mutex + old map[ID]entry + cur map[ID]entry +} + +// New returns an empty cache for extractor fingerprint. +func New(fingerprint string) *Cache { + return &Cache{fingerprint: fingerprint, old: map[ID]entry{}, cur: map[ID]entry{}} +} + +type diskFile struct { + Dev uint64 `json:"dev"` + Ino uint64 `json:"ino"` + Size int64 `json:"size"` + MTime int64 `json:"mtime"` + Set int `json:"keywords"` // index into diskCache.Keywords + Hits []int `json:"hits"` // indices into that keyword list +} + +type diskCache struct { + Version int `json:"version"` + Fingerprint string `json:"fingerprint"` + Keywords [][]string `json:"keywords"` + Files []diskFile `json:"files"` +} + +// Load reads the cache at path. A missing file, another format version or +// another fingerprint is an empty cache and no error; an unreadable file +// is an empty cache and an error. The cache returned is never nil. +func Load(path, fingerprint string) (*Cache, error) { + c := New(fingerprint) + data, err := os.ReadFile(path) + if errors.Is(err, fs.ErrNotExist) { + return c, nil + } + c.existed = err == nil + if err != nil { + return c, err + } + var d diskCache + if err := json.Unmarshal(data, &d); err != nil { + return c, fmt.Errorf("%s: %v", path, err) + } + if d.Version != version || d.Fingerprint != fingerprint { + return c, nil + } + for i, set := range d.Keywords { + if !sort.StringsAreSorted(set) { + return New(fingerprint), fmt.Errorf("%s: keyword list %d is not sorted", path, i) + } + } + for _, f := range d.Files { + if f.Set < 0 || f.Set >= len(d.Keywords) { + return New(fingerprint), fmt.Errorf("%s: bad keyword list %d", path, f.Set) + } + set := d.Keywords[f.Set] + hits := make(map[string]bool, len(f.Hits)) + for _, h := range f.Hits { + if h < 0 || h >= len(set) { + return New(fingerprint), fmt.Errorf("%s: bad keyword %d", path, h) + } + hits[set[h]] = true + } + c.old[ID{Dev: f.Dev, Ino: f.Ino, Size: f.Size, MTime: f.MTime}] = entry{checked: set, hits: hits} + } + c.existed = true + return c, nil +} + +// Lookup reports, for each of keys, whether the text of the file id +// contains it. ok is false unless an entry for id exists and was checked +// against every one of keys. +func (c *Cache) Lookup(id ID, keys []string) (hits []bool, ok bool) { + c.mu.Lock() + defer c.mu.Unlock() + e, found := c.cur[id] + if !found { + e, found = c.old[id] + } + if !found { + return nil, false + } + hits = make([]bool, len(keys)) + for i, k := range keys { + j := sort.SearchStrings(e.checked, k) + if j == len(e.checked) || e.checked[j] != k { + return nil, false + } + hits[i] = e.hits[k] + } + return hits, true +} + +// Store records answers, every keyword the file id was checked against +// and whether its text contains it, replacing anything known about id. +func (c *Cache) Store(id ID, answers map[string]bool) { + e := entry{checked: make([]string, 0, len(answers)), hits: map[string]bool{}} + for k, hit := range answers { + e.checked = append(e.checked, k) + if hit { + e.hits[k] = true + } + } + sort.Strings(e.checked) + c.mu.Lock() + c.cur[id] = e + c.mu.Unlock() +} + +// Save writes the entries of the files in present, from this run or loaded, +// to path, and drops every other: the cache only ever describes files +// still in the directory. The directory is created 0700 and the file +// written 0600 under a temporary name, then renamed into place. A cache +// with nothing to write and no file on disk writes nothing. +func (c *Cache) Save(path string, present []ID) error { + c.mu.Lock() + defer c.mu.Unlock() + + d := diskCache{Version: version, Fingerprint: c.fingerprint, Keywords: [][]string{}, Files: []diskFile{}} + sets := map[string]int{} + seen := map[ID]bool{} + for _, id := range present { + if seen[id] { + continue + } + seen[id] = true + e, ok := c.cur[id] + if !ok { + e, ok = c.old[id] + } + if !ok { + continue + } + key := strings.Join(e.checked, "\x00") + set, ok := sets[key] + if !ok { + set = len(d.Keywords) + sets[key] = set + d.Keywords = append(d.Keywords, e.checked) + } + f := diskFile{Dev: id.Dev, Ino: id.Ino, Size: id.Size, MTime: id.MTime, Set: set, Hits: []int{}} + for i, k := range e.checked { + if e.hits[k] { + f.Hits = append(f.Hits, i) + } + } + d.Files = append(d.Files, f) + } + if len(d.Files) == 0 && !c.existed { + return nil + } + sort.Slice(d.Files, func(i, j int) bool { + if d.Files[i].Dev != d.Files[j].Dev { + return d.Files[i].Dev < d.Files[j].Dev + } + return d.Files[i].Ino < d.Files[j].Ino + }) + + data, err := json.Marshal(d) + if err != nil { + return err + } + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o700); err != nil { + return err + } + tmp, err := os.CreateTemp(dir, ".kwcache-*") + if err != nil { + return err + } + if _, err := tmp.Write(data); err != nil { + tmp.Close() + os.Remove(tmp.Name()) + return err + } + if err := tmp.Close(); err != nil { + os.Remove(tmp.Name()) + return err + } + if err := os.Rename(tmp.Name(), path); err != nil { + os.Remove(tmp.Name()) + return err + } + c.existed = true + return nil +} diff --git a/internal/kwcache/kwcache_test.go b/internal/kwcache/kwcache_test.go new file mode 100644 index 0000000..8395b2d --- /dev/null +++ b/internal/kwcache/kwcache_test.go @@ -0,0 +1,180 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package kwcache + +import ( + "os" + "path/filepath" + "reflect" + "testing" +) + +var ( + a = ID{Dev: 1, Ino: 10, Size: 100, MTime: 1000} + b = ID{Dev: 1, Ino: 11, Size: 200, MTime: 2000} +) + +func saved(t *testing.T, c *Cache, present ...ID) string { + t.Helper() + path := filepath.Join(t.TempDir(), "sub", "dl.cache") + if err := c.Save(path, present); err != nil { + t.Fatal(err) + } + return path +} + +// TestLookupAfterReload: answers stored and saved come back after a Load +// with the same fingerprint, for any keywords the entry covers. +func TestLookupAfterReload(t *testing.T) { + c := New("fp") + c.Store(a, map[string]bool{"k:acme": true, "k:faktura": false, "k:nip": true}) + path := saved(t, c, a) + + got, err := Load(path, "fp") + if err != nil { + t.Fatal(err) + } + hits, ok := got.Lookup(a, []string{"k:faktura", "k:nip"}) + if !ok || !reflect.DeepEqual(hits, []bool{false, true}) { + t.Errorf("Lookup = %v, %v; want [false true], true", hits, ok) + } + if _, ok := got.Lookup(a, []string{"k:acme", "k:new"}); ok { + t.Error("a keyword the entry was never checked against must be a miss") + } + if _, ok := got.Lookup(b, []string{"k:acme"}); ok { + t.Error("a file never stored must be a miss") + } +} + +// TestLookupBeforeSave: what was stored in this run answers at once. +func TestLookupBeforeSave(t *testing.T) { + c := New("fp") + c.Store(a, map[string]bool{"k:acme": true}) + if hits, ok := c.Lookup(a, []string{"k:acme"}); !ok || !hits[0] { + t.Errorf("Lookup = %v, %v", hits, ok) + } +} + +// TestChangedFileMisses: a different size or modification time is a +// different ID, so nothing stored for the old one answers. +func TestChangedFileMisses(t *testing.T) { + c := New("fp") + c.Store(a, map[string]bool{"k:acme": true}) + for _, id := range []ID{{Dev: 1, Ino: 10, Size: 101, MTime: 1000}, {Dev: 1, Ino: 10, Size: 100, MTime: 1001}} { + if _, ok := c.Lookup(id, []string{"k:acme"}); ok { + t.Errorf("%+v answered from %+v's entry", id, a) + } + } +} + +// TestFingerprintMismatchDiscardsAll: a cache written under a different +// extractor fingerprint loads empty, without error. +func TestFingerprintMismatchDiscardsAll(t *testing.T) { + c := New("old") + c.Store(a, map[string]bool{"k:acme": true}) + path := saved(t, c, a) + got, err := Load(path, "new") + if err != nil { + t.Fatal(err) + } + if _, ok := got.Lookup(a, []string{"k:acme"}); ok { + t.Error("entry survived a fingerprint change") + } +} + +// TestSaveKeepsOnlyPresentFiles: entries for files not passed to Save, from +// this run or an earlier one, are dropped. +func TestSaveKeepsOnlyPresentFiles(t *testing.T) { + c := New("fp") + c.Store(a, map[string]bool{"k:acme": true}) + c.Store(b, map[string]bool{"k:acme": false}) + path := saved(t, c, a, b) + + next, err := Load(path, "fp") + if err != nil { + t.Fatal(err) + } + if err := next.Save(path, []ID{b}); err != nil { + t.Fatal(err) + } + last, err := Load(path, "fp") + if err != nil { + t.Fatal(err) + } + if _, ok := last.Lookup(a, []string{"k:acme"}); ok { + t.Error("a is gone from the directory but its entry was kept") + } + if hits, ok := last.Lookup(b, []string{"k:acme"}); !ok || hits[0] { + t.Errorf("b's entry lost across a reload: %v, %v", hits, ok) + } +} + +// TestStoreReplacesLoadedEntry: a file read again this run replaces what +// an earlier run stored for it. +func TestStoreReplacesLoadedEntry(t *testing.T) { + c := New("fp") + c.Store(a, map[string]bool{"k:acme": true}) + path := saved(t, c, a) + next, _ := Load(path, "fp") + next.Store(a, map[string]bool{"k:acme": false, "k:new": true}) + if hits, ok := next.Lookup(a, []string{"k:acme", "k:new"}); !ok || hits[0] || !hits[1] { + t.Errorf("Lookup = %v, %v; want the new entry", hits, ok) + } +} + +// TestSavePermissions: the directory is private and the file readable by +// its owner only, with no temporary file left behind. +func TestSavePermissions(t *testing.T) { + c := New("fp") + c.Store(a, map[string]bool{"k:acme": true}) + path := saved(t, c, a) + fi, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if perm := fi.Mode().Perm(); perm != 0o600 { + t.Errorf("file mode %o, want 600", perm) + } + di, err := os.Stat(filepath.Dir(path)) + if err != nil { + t.Fatal(err) + } + if perm := di.Mode().Perm(); perm != 0o700 { + t.Errorf("directory mode %o, want 700", perm) + } + entries, _ := os.ReadDir(filepath.Dir(path)) + if len(entries) != 1 { + t.Errorf("directory holds %d entries, want only the cache", len(entries)) + } +} + +// TestLoadMissingAndCorrupt: no file is an empty cache and no error; an +// unreadable one is an empty cache and an error saying so. +func TestLoadMissingAndCorrupt(t *testing.T) { + dir := t.TempDir() + c, err := Load(filepath.Join(dir, "none.cache"), "fp") + if err != nil || c == nil { + t.Fatalf("missing: %v, %v", c, err) + } + bad := filepath.Join(dir, "bad.cache") + os.WriteFile(bad, []byte("{not json"), 0o600) + c, err = Load(bad, "fp") + if err == nil || c == nil { + t.Fatalf("corrupt: want an empty cache and an error, got %v, %v", c, err) + } + if _, ok := c.Lookup(a, []string{"k:acme"}); ok { + t.Error("corrupt cache answered") + } +} + +// TestSaveWithNothingWritesNothing: a cache that never held an entry, and +// has no file yet, creates none. +func TestSaveWithNothingWritesNothing(t *testing.T) { + path := filepath.Join(t.TempDir(), "sub", "dl.cache") + if err := New("fp").Save(path, []ID{a}); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Dir(path)); !os.IsNotExist(err) { + t.Errorf("Save created %s for an empty cache", filepath.Dir(path)) + } +} diff --git a/internal/scan/fileid_other.go b/internal/scan/fileid_other.go new file mode 100644 index 0000000..503a9b4 --- /dev/null +++ b/internal/scan/fileid_other.go @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +//go:build !unix + +package scan + +import "io/fs" + +// fileID reports no identity where the platform has no inodes. +func fileID(fs.FileInfo) (dev, ino uint64) { return 0, 0 } diff --git a/internal/scan/fileid_unix.go b/internal/scan/fileid_unix.go new file mode 100644 index 0000000..e29066a --- /dev/null +++ b/internal/scan/fileid_unix.go @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +//go:build unix + +package scan + +import ( + "io/fs" + "syscall" +) + +// fileID returns the device and inode info was read from. +func fileID(info fs.FileInfo) (dev, ino uint64) { + st, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return 0, 0 + } + return uint64(st.Dev), uint64(st.Ino) +} diff --git a/internal/scan/scan.go b/internal/scan/scan.go index 7fe9e92..77cca66 100644 --- a/internal/scan/scan.go +++ b/internal/scan/scan.go @@ -24,6 +24,26 @@ type File struct { Size int64 ModTime time.Time Mode fs.FileMode + + // Dev and Ino identify the file on its filesystem; both are 0 where the + // platform reports neither. + Dev, Ino uint64 +} + +// NewFile builds the File for path, at rel under the root, from its Lstat +// info. +func NewFile(path, rel string, info fs.FileInfo) File { + dev, ino := fileID(info) + return File{ + Path: path, + Rel: rel, + Name: filepath.Base(path), + Size: info.Size(), + ModTime: info.ModTime(), + Mode: info.Mode(), + Dev: dev, + Ino: ino, + } } // Reason is why an entry was not returned as a File. @@ -217,14 +237,7 @@ func (w *walker) walk(dir, relDir string, depth int, entries []os.DirEntry) erro w.result.Skipped = append(w.result.Skipped, Skipped{Rel: rel, Reason: TooBig}) continue } - w.result.Files = append(w.result.Files, File{ - Path: path, - Rel: rel, - Name: name, - Size: info.Size(), - ModTime: info.ModTime(), - Mode: info.Mode(), - }) + w.result.Files = append(w.result.Files, NewFile(path, rel, info)) } return nil } diff --git a/internal/scan/scan_test.go b/internal/scan/scan_test.go index a245893..e9aeb73 100644 --- a/internal/scan/scan_test.go +++ b/internal/scan/scan_test.go @@ -293,3 +293,29 @@ func TestTooBig(t *testing.T) { t.Errorf("MaxSize 0 skipped files: %v", skipped(r)) } } + +// TestFilesCarryInode: a walked file carries its device and inode, which a +// rename keeps. +func TestFilesCarryInode(t *testing.T) { + root := tree(t) + p := filepath.Join(root, "a.txt") + if err := os.WriteFile(p, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + old := now.Add(-time.Hour) + os.Chtimes(p, old, old) + first, err := Walk(root, Options{Now: now}) + if err != nil || len(first.Files) != 1 { + t.Fatalf("walk: %v %+v", err, first) + } + if first.Files[0].Ino == 0 { + t.Fatal("no inode on a unix filesystem") + } + if err := os.Rename(p, filepath.Join(root, "b.txt")); err != nil { + t.Fatal(err) + } + second, _ := Walk(root, Options{Now: now}) + if len(second.Files) != 1 || second.Files[0].Ino != first.Files[0].Ino || second.Files[0].Dev != first.Files[0].Dev { + t.Errorf("rename changed the identity: %+v then %+v", first.Files[0], second.Files) + } +} diff --git a/internal/xdg/xdg.go b/internal/xdg/xdg.go index ed34838..be13650 100644 --- a/internal/xdg/xdg.go +++ b/internal/xdg/xdg.go @@ -18,6 +18,9 @@ func StateHome() string { return base("XDG_STATE_HOME", filepath.Join(".local", // DataHome is $XDG_DATA_HOME, or ~/.local/share. func DataHome() string { return base("XDG_DATA_HOME", filepath.Join(".local", "share")) } +// CacheHome is $XDG_CACHE_HOME, or ~/.cache. +func CacheHome() string { return base("XDG_CACHE_HOME", ".cache") } + // base follows the XDG rule that a relative value is invalid and ignored. func base(env, fallback string) string { if v := os.Getenv(env); filepath.IsAbs(v) { diff --git a/man/krino.1 b/man/krino.1 index 458bd56..cc4c2e1 100644 --- a/man/krino.1 +++ b/man/krino.1 @@ -244,11 +244,25 @@ each step, then the rule and the reason it matched: copy \(-> ~/backup/invoices/2026/ rule backup because content "invoice" - [y] yes [n] no [a] yes to this and all remaining [d] done, apply chosen so far [q] quit, apply nothing + [y] yes [n] no [a] yes to this and all remaining [t] trash [d] delete permanently [w] write, apply chosen so far [q] quit, apply nothing .Ed .Pp Approval is per file: a file's whole chain runs, or none of it. +.Ic t +and +.Ic d +set aside what the rules planned for the file and approve one step on the +file itself instead: to the Trash, or deleted permanently, logged under the +rule name +.Sy (review) . .Ic d +first asks +.Dq delete Ar name No permanently? [y/N] ; +any key but +.Ic y +deletes nothing and asks about the file again. +A permanent delete cannot be undone. +.Ic w stops asking and applies whatever was already chosen, declining the rest. .Ic q here aborts the review for this directory entirely, discarding even a file @@ -272,6 +286,13 @@ both apply nothing: [a] apply all [c] choose per file [s] skip [q] quit .Ed .Pp +Its per-file prompt is the one in +.Sx REVIEW +without +.Ic t +and +.Ic d . +.Pp An undo run cannot itself be undone: naming it to .Ic undo is refused. @@ -320,6 +341,11 @@ step uses .Pq Pa $XDG_DATA_HOME/Trash ; default .Pa ~/.local/share . +.It Ev XDG_CACHE_HOME +Base of the keyword cache +.Pq Pa $XDG_CACHE_HOME/krino ; +default +.Pa ~/.cache . .It Ev PAGER Used to show a plan taller than the terminal; default .Dq less -FRX . @@ -355,6 +381,12 @@ against the same directory waits, or fails immediately with The freedesktop.org trash a .Sy (delete) step moves files into. +.It Pa $XDG_CACHE_HOME/krino/ Ns Ar name Ns Pa .cache +Which content keywords each extracted file of directory +.Ar name +contains, so an unchanged file is not extracted again; see +.Xr krino.conf 5 , Sx CONTENT EXTRACTION . +Safe to delete at any time. .El .Sh EXIT STATUS .Bl -tag -width Ds diff --git a/man/krino.conf.5 b/man/krino.conf.5 index 769bd51..c89fbd9 100644 --- a/man/krino.conf.5 +++ b/man/krino.conf.5 @@ -521,6 +521,28 @@ Matching is substring: matches .Ql \&"acmeco\&" . Words hyphenated across lines in a PDF are not rejoined. +.Ss Keyword cache +For each file it extracts, +.Nm krino +records in +.Pa $XDG_CACHE_HOME/krino/ Ns Ar name Ns Pa .cache +which of the directory's content keywords the text contains, and answers +later content tests from it while the file is unchanged. +No text and no file names are stored; a file is known by device, inode, +size and modification time, which a move or rename within one filesystem +keeps. +The keywords are stored as written in the configuration. +.Pp +A file is extracted again when it changes, or when a test asks about a +keyword its entry was never checked against. +Failures are never cached, and a file above +.Ic max-read +is refused before the cache is read. +The whole cache is discarded when an extraction tool is installed, removed +or replaced. +Each run keeps entries only for files still in the directory. +A file edited in place with its size and modification time preserved keeps +its old answers; deleting the cache resets everything. .Sh ACTIONS .Ss Chains A file's chain is the actions of every matching rule, in rule order, and |