aboutsummaryrefslogtreecommitdiff
path: root/cmd
diff options
context:
space:
mode:
Diffstat (limited to 'cmd')
-rw-r--r--cmd/krino/check.go3
-rw-r--r--cmd/krino/common.go8
-rw-r--r--cmd/krino/history_test.go56
-rw-r--r--cmd/krino/init.go3
-rw-r--r--cmd/krino/log.go3
-rw-r--r--cmd/krino/main.go21
-rw-r--r--cmd/krino/new.go3
-rw-r--r--cmd/krino/sort.go4
-rw-r--r--cmd/krino/undo.go55
9 files changed, 132 insertions, 24 deletions
diff --git a/cmd/krino/check.go b/cmd/krino/check.go
index 4463c73..97c1f16 100644
--- a/cmd/krino/check.go
+++ b/cmd/krino/check.go
@@ -19,6 +19,9 @@ func cmdCheck(g *globals, args []string, stdout, stderr io.Writer) int {
if code, ok := parse(fs, args, stdout, stderr); !ok {
return code
}
+ if g.minAgeSet {
+ return usageError(stderr, "--min-age applies only to sorting and explain")
+ }
e, errs := engine.Load(mainFile(g), fs.Args()...)
if len(errs) > 0 {
printDiags(stderr, errs)
diff --git a/cmd/krino/common.go b/cmd/krino/common.go
index 11e73c5..a9c92f3 100644
--- a/cmd/krino/common.go
+++ b/cmd/krino/common.go
@@ -18,10 +18,12 @@ import (
// 2m, 1h, 1d, 1w), or a bare 0. set is false when the flag was not given.
// Checked before any config is read, as every flag is.
func minAgeOverride(g *globals) (d time.Duration, set bool, err error) {
- switch g.minAge {
- case "":
+ switch {
+ case !g.minAgeSet:
return 0, false, nil
- case "0":
+ case g.minAge == "":
+ return 0, false, fmt.Errorf("--min-age: needs a duration, like 0, 30m or 1d")
+ case g.minAge == "0":
return 0, true, nil
}
d, err = config.ParseDuration(g.minAge)
diff --git a/cmd/krino/history_test.go b/cmd/krino/history_test.go
index 1a9ea2b..fce38ad 100644
--- a/cmd/krino/history_test.go
+++ b/cmd/krino/history_test.go
@@ -143,7 +143,7 @@ func TestFinalizeUndoPlanMarksUnapprovedAsDeclined(t *testing.T) {
{File: "b"}, // index 1: not approved -> declined
{File: "c", Refused: "gone"}, // index 2: refused, never declined
}}
- out := finalizeUndoPlan(up, map[int]bool{0: true})
+ out, _ := finalizeUndoPlan(up, map[int]bool{0: true}, 'c')
if len(out.Files) != 3 {
t.Fatalf("files = %+v, want all three carried through", out.Files)
}
@@ -203,8 +203,8 @@ func TestReviewUndoWriteStopsAsking(t *testing.T) {
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 action != 'w' || !approved[0] || len(approved) != 1 {
+ t.Errorf("approved = %v action = %q; want 'w' with only index 0 decided", 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)
@@ -466,3 +466,53 @@ func TestUndoWithoutRunContinuesTheLastUndo(t *testing.T) {
t.Fatalf("undo -n after a failed undo: exit %d\n%s\n%s", code, out, errOut)
}
}
+
+// TestReviewUndoMatchesReview: undo's per-file review behaves as review's
+// does (review cli F2): n is recorded, each choice is echoed in red, and w
+// leaves the files it never reached out of the plan, counted as not
+// reviewed rather than logged as declined.
+func TestReviewUndoMatchesReview(t *testing.T) {
+ out := new(strings.Builder)
+ files := undoFiles("a", "b", "c")
+ approved, action, err := reviewUndoFiles(strings.NewReader("cynw"), out, files, palette{on: true})
+ if err != nil {
+ t.Fatal(err)
+ }
+ if action != 'w' || !approved[0] || approved[1] {
+ t.Fatalf("approved = %v action = %q", approved, action)
+ }
+ if v, ok := approved[1]; !ok || v {
+ t.Errorf("b should be decided as no: %v", approved)
+ }
+ for _, want := range []string{"\x1b[31m→ yes\x1b[0m", "\x1b[31m→ no\x1b[0m"} {
+ if !strings.Contains(out.String(), want) {
+ t.Errorf("no %q echo in:\n%q", want, out)
+ }
+ }
+ up := &engine.UndoPlan{Run: "r", Files: files}
+ plan, notReviewed := finalizeUndoPlan(up, approved, action)
+ if notReviewed != 1 || len(plan.Files) != 2 || plan.Files[0].Declined || !plan.Files[1].Declined {
+ t.Errorf("finalize: notReviewed %d, files %+v; want a, declined b, c left out", notReviewed, plan.Files)
+ }
+}
+
+// TestMinAgeRejectedOutsideSortAndExplain: --min-age only changes sorting
+// and explain; any other command refuses it rather than silently ignoring
+// a mistyped value, and an empty value is an error (review cli F4).
+func TestMinAgeRejectedOutsideSortAndExplain(t *testing.T) {
+ matchingFixture(t)
+ for _, args := range [][]string{
+ {"undo", "-n", "--min-age", "1d"},
+ {"--min-age", "garbage", "undo", "-n"},
+ {"check", "--min-age", "1d"},
+ {"log", "--min-age", "1d"},
+ {"-n", "--min-age="},
+ } {
+ if code, _, errOut := runCLI(t, args...); code != 2 || !strings.Contains(errOut, "--min-age") {
+ t.Errorf("krino %q: exit %d, stderr %q; want 2 naming --min-age", args, code, errOut)
+ }
+ }
+ if code, _, errOut := runCLI(t, "-n", "--min-age", "0"); code != 0 {
+ t.Errorf("-n --min-age 0: exit %d %s", code, errOut)
+ }
+}
diff --git a/cmd/krino/init.go b/cmd/krino/init.go
index 2122ff8..4383a14 100644
--- a/cmd/krino/init.go
+++ b/cmd/krino/init.go
@@ -19,6 +19,9 @@ func cmdInit(g *globals, args []string, stdout, stderr io.Writer) int {
if code, ok := parse(fs, args, stdout, stderr); !ok {
return code
}
+ if g.minAgeSet {
+ return usageError(stderr, "--min-age applies only to sorting and explain")
+ }
if fs.NArg() != 0 {
return usageError(stderr, "init takes no arguments")
}
diff --git a/cmd/krino/log.go b/cmd/krino/log.go
index 2f6c521..2394ec2 100644
--- a/cmd/krino/log.go
+++ b/cmd/krino/log.go
@@ -54,6 +54,9 @@ func cmdLog(g *globals, args []string, stdout, stderr io.Writer) int {
if code, ok := parse(fs, args, stdout, stderr); !ok {
return code
}
+ if g.minAgeSet {
+ return usageError(stderr, "--min-age applies only to sorting and explain")
+ }
if fs.NArg() > 0 {
return usageError(stderr, "usage: krino log [-n N]")
}
diff --git a/cmd/krino/main.go b/cmd/krino/main.go
index 7a6a5f7..29da31a 100644
--- a/cmd/krino/main.go
+++ b/cmd/krino/main.go
@@ -61,7 +61,8 @@ Sort the files in the directories listed in krino.conf by their rules.
type globals struct {
yes, dry, verbose, json bool
noColor, noPager bool
- minAge string // --min-age as given; "" when not
+ minAge string // --min-age as given
+ minAgeSet bool // --min-age was given, even as an empty value
conf string
}
@@ -127,10 +128,26 @@ func flagSet(name string, g *globals) *flag.FlagSet {
fs.BoolVar(&g.noColor, "no-color", g.noColor, "")
fs.BoolVar(&g.noPager, "no-pager", g.noPager, "")
fs.BoolVar(&g.noPager, "P", g.noPager, "")
- fs.StringVar(&g.minAge, "min-age", g.minAge, "")
+ fs.Var(minAgeValue{g}, "min-age", "")
return fs
}
+// minAgeValue is --min-age as a flag.Value, so that a flag given with an
+// empty value (--min-age=) is told apart from one not given at all.
+type minAgeValue struct{ g *globals }
+
+func (v minAgeValue) String() string {
+ if v.g == nil {
+ return ""
+ }
+ return v.g.minAge
+}
+
+func (v minAgeValue) Set(s string) error {
+ v.g.minAge, v.g.minAgeSet = s, true
+ return nil
+}
+
// parse parses args into fs. On -h it prints the usage and returns (0, false);
// on a bad flag it reports it and returns (2, false).
func parse(fs *flag.FlagSet, args []string, stdout, stderr io.Writer) (int, bool) {
diff --git a/cmd/krino/new.go b/cmd/krino/new.go
index 1cd89a6..37f2418 100644
--- a/cmd/krino/new.go
+++ b/cmd/krino/new.go
@@ -18,6 +18,9 @@ func cmdNew(g *globals, args []string, stdout, stderr io.Writer) int {
if code, ok := parse(fs, args, stdout, stderr); !ok {
return code
}
+ if g.minAgeSet {
+ return usageError(stderr, "--min-age applies only to sorting and explain")
+ }
if fs.NArg() != 2 {
return usageError(stderr, "usage: krino new NAME PATH")
}
diff --git a/cmd/krino/sort.go b/cmd/krino/sort.go
index 5e52f84..901b19e 100644
--- a/cmd/krino/sort.go
+++ b/cmd/krino/sort.go
@@ -270,7 +270,9 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
}
fmt.Fprintf(stderr, "krino: %s: %v\n", d.Name, aerr)
exit = 1
- return false
+ // [w] stops krino whether or not its apply succeeded (review
+ // cli F3): no later directory is planned or asked about.
+ return action == 'w'
}
fmt.Fprintln(stdout, withNotReviewed(outcome(p, res.Applied, res.Failed, res.Declined), notReviewed))
// Ruling 1: only an actual step failure makes the run exit 1
diff --git a/cmd/krino/undo.go b/cmd/krino/undo.go
index 4c85185..4cefc82 100644
--- a/cmd/krino/undo.go
+++ b/cmd/krino/undo.go
@@ -47,6 +47,9 @@ func cmdUndo(g *globals, args []string, stdout, stderr io.Writer) int {
if code, ok := parse(fs, args, stdout, stderr); !ok {
return code
}
+ if g.minAgeSet {
+ return usageError(stderr, "--min-age applies only to sorting and explain")
+ }
if g.yes && g.dry {
return usageError(stderr, "-y and -n cannot be used together")
}
@@ -187,7 +190,7 @@ func cmdUndo(g *globals, args []string, stdout, stderr io.Writer) int {
return 0
}
- toApply := finalizeUndoPlan(up, approved)
+ toApply, notReviewed := finalizeUndoPlan(up, approved, action)
// Ruling 2 (Task 7), carried over: journal.Open creates the state
// directory and the log file as a side effect of merely being called,
@@ -220,7 +223,7 @@ func cmdUndo(g *globals, args []string, stdout, stderr io.Writer) int {
fmt.Fprintf(stderr, "krino: %v\n", aerr)
return 1
}
- fmt.Fprintln(stdout, outcome(p, res.Applied, res.Failed, res.Declined))
+ fmt.Fprintln(stdout, withNotReviewed(outcome(p, res.Applied, res.Failed, res.Declined), notReviewed))
if ctx.Err() != nil {
return 130
@@ -306,15 +309,26 @@ func releaseUndoLocks(locks []*lock.Lock) []error {
// though nothing happens to it, the same as the forward path already does,
// so it is kept with Declined set, which tells ApplyUndo to log its steps
// as declined rather than reverse them.
-func finalizeUndoPlan(up *engine.UndoPlan, approved map[int]bool) *engine.UndoPlan {
+//
+// After [w] (action 'w'), a reversible file the review never reached is
+// left out of the plan entirely, as review's [w] leaves a forward file
+// unlogged, and counted in notReviewed (review cli F2).
+func finalizeUndoPlan(up *engine.UndoPlan, approved map[int]bool, action rune) (plan *engine.UndoPlan, notReviewed int) {
out := &engine.UndoPlan{Run: up.Run}
for i, f := range up.Files {
- if f.Refused == "" && !approved[i] {
- f.Declined = true
+ if f.Refused == "" {
+ yes, decided := approved[i]
+ switch {
+ case !decided && action == 'w':
+ notReviewed++
+ continue
+ case !yes:
+ f.Declined = true
+ }
}
out.Files = append(out.Files, f)
}
- return out
+ return out, notReviewed
}
// reviewUndoDir drives the interactive review over the real terminal,
@@ -358,12 +372,15 @@ func reviewUndoFiles(in io.Reader, out io.Writer, files []engine.UndoFile, p pal
case 'q':
return map[int]bool{}, 'q', nil
case 'c':
- approved, quit, err := reviewUndoPerFile(in, out, files, p)
+ approved, end, err := reviewUndoPerFile(in, out, files, p)
if err != nil {
return nil, 0, err
}
- if quit {
+ switch end {
+ case 'q':
return map[int]bool{}, 'q', nil
+ case 'w':
+ return approved, 'w', nil
}
return approved, 'c', nil
default:
@@ -376,8 +393,11 @@ func reviewUndoFiles(in io.Reader, out io.Writer, files []engine.UndoFile, p pal
// refused at planning time is never asked about - spec §10 shows it with
// its reason and reverses nothing of it regardless of anything chosen here
// - but it still gets its own [i/N] line, so the numbering accounts for
-// every file in the plan, not just the reversible ones.
-func reviewUndoPerFile(in io.Reader, out io.Writer, files []engine.UndoFile, p palette) (approved map[int]bool, quit bool, err error) {
+// every file in the plan, not just the reversible ones. It behaves as
+// review.go's reviewPerFile does (review cli F2): a no is recorded as false,
+// each choice is echoed in red, [w] ends with 'w' leaving unreached files
+// out of approved, and [q] ends with 'q'.
+func reviewUndoPerFile(in io.Reader, out io.Writer, files []engine.UndoFile, p palette) (approved map[int]bool, end rune, err error) {
approved = map[int]bool{}
yesRest := false
for i, f := range files {
@@ -400,30 +420,35 @@ func reviewUndoPerFile(in io.Reader, out io.Writer, files []engine.UndoFile, p p
for {
key, kerr := readKey(in)
if kerr != nil {
- return nil, false, kerr
+ return nil, 0, kerr
}
+ var choice string
switch key {
case '\r', '\n':
continue
case 'y':
approved[i] = true
+ choice = "yes"
case 'n':
- // leave unapproved
+ approved[i] = false
+ choice = "no"
case 'a':
approved[i] = true
yesRest = true
+ choice = "yes, and all remaining"
case 'w':
- return approved, false, nil
+ return approved, 'w', nil
case 'q':
- return nil, true, nil
+ return nil, 'q', nil
default:
fmt.Fprintf(out, "%q is not y, n, a, w or q\n", key)
continue
}
+ fmt.Fprintln(out, " "+p.bad("→ "+choice))
break
}
}
- return approved, false, nil
+ return approved, 0, nil
}
// undoPerFileKeys is undo's per-file prompt: review's without [t] and [d],
generated by cgit v1.3 (git 2.53.0) at 2026-09-19 13:16:44 +0000