aboutsummaryrefslogtreecommitdiff
path: root/internal
diff options
context:
space:
mode:
Diffstat (limited to 'internal')
-rw-r--r--internal/config/dir.go4
-rw-r--r--internal/config/dir_test.go1
-rw-r--r--internal/engine/match.go2
-rw-r--r--internal/extract/extract.go5
-rw-r--r--internal/extract/tools_test.go15
-rw-r--r--internal/plan/conflict.go5
-rw-r--r--internal/plan/enum_test.go27
-rw-r--r--internal/scan/scan.go12
-rw-r--r--internal/scan/scan_test.go21
9 files changed, 88 insertions, 4 deletions
diff --git a/internal/config/dir.go b/internal/config/dir.go
index 461d912..54ee35e 100644
--- a/internal/config/dir.go
+++ b/internal/config/dir.go
@@ -184,6 +184,10 @@ func parseRule(n *sexp.Node, d *diags) *Rule {
d.at(n, `rule needs a name in quotes first, like (rule "invoices" ...)`)
return nil
}
+ if strings.HasPrefix(args[0].Text, "(") {
+ d.at(n, `rule names cannot start with "(": (review) marks choices made in review`)
+ return nil
+ }
r := &Rule{Name: args[0].Text, Pos: n.Pos}
seen := map[string]*sexp.Node{}
var whenNode *sexp.Node
diff --git a/internal/config/dir_test.go b/internal/config/dir_test.go
index 080905c..cae5f5f 100644
--- a/internal/config/dir_test.go
+++ b/internal/config/dir_test.go
@@ -112,6 +112,7 @@ func TestParseDirErrors(t *testing.T) {
{`(path "/a") (rule "x" (delete) (move "y"))`, `d.conf:1:32: rule "x": (move "y") after delete would never run`},
{`(path "/a") (rule "x" (stop now))`, `d.conf:1:23: rule "x": stop takes nothing: write (stop)`},
{`(path "/a") (rule "x" (fly "y"))`, `d.conf:1:23: rule "x": unknown form (fly ...); a rule has when, copy, move, rename, delete, stop, case, fold and on-conflict`},
+ {`(path "/a") (rule "(review)" (delete))`, `d.conf:1:13: rule names cannot start with "(": (review) marks choices made in review`},
{`(path "/a") (sort "x")`, `d.conf:1:13: unknown form (sort ...); a directory file has path, ignore, exclude, rule and settings like (recursive yes)`},
}
for _, tt := range tests {
diff --git a/internal/engine/match.go b/internal/engine/match.go
index fedc053..162730b 100644
--- a/internal/engine/match.go
+++ b/internal/engine/match.go
@@ -422,7 +422,7 @@ func explainSkip(d *Dir, sf scan.File, excl []string, now time.Time) string {
if isBusy(sf.Path, d.Settings.Busy) {
return "busy"
}
- if now.Sub(sf.ModTime) < d.Settings.MinAge {
+ if scan.Age(now, sf.ModTime) < d.Settings.MinAge {
return "too new"
}
if d.Settings.MaxSize > 0 && sf.Size > d.Settings.MaxSize {
diff --git a/internal/extract/extract.go b/internal/extract/extract.go
index 9768db1..ad51c00 100644
--- a/internal/extract/extract.go
+++ b/internal/extract/extract.go
@@ -99,7 +99,10 @@ func newWithPath(path string) *Extractor {
tools := make(map[string]string, len(toolNames))
for _, name := range toolNames {
for _, dir := range dirs {
- if dir == "" {
+ // A relative entry would find a tool relative to the working
+ // directory - a bin/pdftotext an unpacked download left behind
+ // (review planapply F7) - so only absolute entries count.
+ if dir == "" || !filepath.IsAbs(dir) {
continue
}
p := filepath.Join(dir, name)
diff --git a/internal/extract/tools_test.go b/internal/extract/tools_test.go
index b481e8a..756903d 100644
--- a/internal/extract/tools_test.go
+++ b/internal/extract/tools_test.go
@@ -261,3 +261,18 @@ func TestFingerprintFollowsTools(t *testing.T) {
t.Error("replacing pdftotext did not change the fingerprint")
}
}
+
+// TestToolLookupSkipsRelativePathEntries: a relative PATH entry would make
+// a tool's path relative to the working directory - a bin/pdftotext left by
+// an unpacked download - so it is ignored (review planapply F7).
+func TestToolLookupSkipsRelativePathEntries(t *testing.T) {
+ wd := t.TempDir()
+ if err := os.Mkdir(filepath.Join(wd, "bin"), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ fakeTool(t, filepath.Join(wd, "bin"), "pdftotext", "echo injected")
+ t.Chdir(wd)
+ if e := newWithPath("bin"); e.tools["pdftotext"] != "" {
+ t.Errorf("tool found through a relative PATH entry: %q", e.tools["pdftotext"])
+ }
+}
diff --git a/internal/plan/conflict.go b/internal/plan/conflict.go
index c4c6b64..c660a8b 100644
--- a/internal/plan/conflict.go
+++ b/internal/plan/conflict.go
@@ -120,10 +120,13 @@ func resolveConflict(kind Kind, policy config.Conflict, src, dst string, d Disk,
// displaces nothing.
resolved, skip := suffixed(dst, d, c)
return resolved, skip, ""
- default: // config.ConflictSuffix
+ case config.ConflictSuffix:
resolved, skip := suffixed(dst, d, c)
return resolved, skip, ""
}
+ // Every policy has its own branch (review cli F13): a new one must not
+ // quietly plan as another.
+ panic(fmt.Sprintf("plan: unknown config.Conflict %d", int(policy)))
}
// maxSuffixAttempts bounds suffixed(): it is unbounded by design and
diff --git a/internal/plan/enum_test.go b/internal/plan/enum_test.go
index 6458f5f..5e4f645 100644
--- a/internal/plan/enum_test.go
+++ b/internal/plan/enum_test.go
@@ -45,3 +45,30 @@ func TestEveryKindIsNamed(t *testing.T) {
}()
}
}
+
+// TestEveryConflictPolicyIsPlanned: every config.Conflict value is resolved
+// by its own branch; an unknown value panics instead of quietly planning as
+// suffix (review cli F13). Conflict is an iota block from 0.
+func TestEveryConflictPolicyIsPlanned(t *testing.T) {
+ policies, err := enumtest.Names("../config/settings.go", "Conflict")
+ if err != nil {
+ t.Fatal(err)
+ }
+ d := fakeDisk{exists: map[string]bool{"/r/b.pdf": true}}
+ for i, name := range policies {
+ func() {
+ defer func() {
+ if r := recover(); r != nil {
+ t.Errorf("config.%s is not planned: %v", name, r)
+ }
+ }()
+ resolveConflict(Move, config.Conflict(i), "/r/a.pdf", "/r/b.pdf", d, claimed{})
+ }()
+ }
+ defer func() {
+ if recover() == nil {
+ t.Error("an unknown conflict policy did not panic")
+ }
+ }()
+ resolveConflict(Move, config.Conflict(len(policies)), "/r/a.pdf", "/r/b.pdf", d, claimed{})
+}
diff --git a/internal/scan/scan.go b/internal/scan/scan.go
index 77cca66..bd717e0 100644
--- a/internal/scan/scan.go
+++ b/internal/scan/scan.go
@@ -229,7 +229,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: Busy})
continue
}
- if w.opt.Now.Sub(info.ModTime()) < w.opt.MinAge {
+ if Age(w.opt.Now, info.ModTime()) < w.opt.MinAge {
w.result.Skipped = append(w.result.Skipped, Skipped{Rel: rel, Reason: TooNew})
continue
}
@@ -252,3 +252,13 @@ func (w *walker) isBusy(name string, names map[string]bool) bool {
}
return false
}
+
+// Age is how long ago mtime was, at now. A modification time ahead of the
+// clock (a skewed server, an archive's timestamps) counts as brand new: age
+// 0, so min-age 0 still considers the file (review cli F5).
+func Age(now, mtime time.Time) time.Duration {
+ if a := now.Sub(mtime); a > 0 {
+ return a
+ }
+ return 0
+}
diff --git a/internal/scan/scan_test.go b/internal/scan/scan_test.go
index e9aeb73..35e7a0d 100644
--- a/internal/scan/scan_test.go
+++ b/internal/scan/scan_test.go
@@ -319,3 +319,24 @@ func TestFilesCarryInode(t *testing.T) {
t.Errorf("rename changed the identity: %+v then %+v", first.Files[0], second.Files)
}
}
+
+// TestFutureFileIsNotTooNewAtMinAgeZero: a file whose modification time is
+// ahead of the clock counts as brand new - skipped while min-age is above
+// zero, considered at min-age 0 (review cli F5).
+func TestFutureFileIsNotTooNewAtMinAgeZero(t *testing.T) {
+ root := tree(t)
+ p := filepath.Join(root, "future.txt")
+ if err := os.WriteFile(p, []byte("x"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ ahead := now.Add(time.Hour)
+ if err := os.Chtimes(p, ahead, ahead); err != nil {
+ t.Fatal(err)
+ }
+ if r, _ := Walk(root, Options{Now: now}); len(rels(r)) != 1 {
+ t.Errorf("min-age 0: files %v, skipped %v; want future.txt considered", rels(r), skipped(r))
+ }
+ if r, _ := Walk(root, Options{Now: now, MinAge: time.Minute}); skipped(r)["future.txt"] != TooNew {
+ t.Errorf("min-age 1m: skipped %v; want future.txt too new", skipped(r))
+ }
+}
generated by cgit v1.3 (git 2.53.0) at 2026-09-19 12:23:35 +0000