From 6ef83d6bdfb6120f9e1fbd145e0bc463196103d1 Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Wed, 16 Sep 2026 09:39:59 +0200 Subject: gui: History and undo tab; each plan and undo is its own run --- gui/internal/model/history.go | 280 +++++++++++++++++++++++++++++++++++++ gui/internal/model/history_test.go | 207 +++++++++++++++++++++++++++ gui/internal/model/plan.go | 41 +++--- gui/internal/model/plan_test.go | 35 ++--- 4 files changed, 525 insertions(+), 38 deletions(-) create mode 100644 gui/internal/model/history.go create mode 100644 gui/internal/model/history_test.go (limited to 'gui/internal/model') diff --git a/gui/internal/model/history.go b/gui/internal/model/history.go new file mode 100644 index 0000000..9ce854c --- /dev/null +++ b/gui/internal/model/history.go @@ -0,0 +1,280 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package model + +import ( + "context" + "fmt" + "sort" + "strings" + "time" + + "krino/internal/engine" + "krino/internal/journal" + "krino/internal/lock" +) + +// Run is one line of the History tab's run list: what a run did, and +// whether it has since been reversed (GUI design §4). +type Run struct { + ID string + Start time.Time + Dirs []string + Summary string // "3 moved · 1 trashed", or "nothing applied" + Note string // "(undone)", "(partly undone)", or "" + UndoOf string // for an undo run, the run it reverses +} + +// Runs lists the most recent runs, newest first. It reads the log only. +func Runs(e *engine.Engine, n int) ([]Run, error) { + rs, err := journal.Runs(e.Config.LogFile(), n) + if err != nil { + return nil, err + } + out := make([]Run, 0, len(rs)) + for _, r := range rs { + out = append(out, Run{ + ID: r.ID, + Start: r.Start, + Dirs: r.Dirs, + Summary: countsText(r.Counts), + Note: note(r), + UndoOf: r.UndoOf, + }) + } + return out, nil +} + +// note is what the list says about a run that has been reversed. +func note(r journal.Run) string { + switch { + case r.PartlyUndone: + return "(partly undone)" + case r.Undone: + return "(undone)" + } + return "" +} + +// pastTense and countOrder are krino log's own display words and order +// (cmd/krino/log.go): the log's action names are the wire format and are +// never renamed, so both front ends translate them the same way. +var pastTense = map[string]string{ + "copy": "copied", "move": "moved", "rename": "renamed", + "trash": "trashed", "delete": "deleted", "displace": "displaced", + "undo-copy": "undo-copied", "undo-move": "undo-moved", + "undo-rename": "undo-renamed", "undo-trash": "undo-trashed", + "undo-displace": "undo-displaced", +} + +var countOrder = []string{ + "copy", "move", "rename", "trash", "delete", "displace", + "undo-copy", "undo-move", "undo-rename", "undo-trash", "undo-displace", +} + +// countsText renders a run's counts in a fixed order, skipping actions with +// no successful entries. +func countsText(counts map[string]int) string { + var parts []string + for _, action := range countOrder { + if n := counts[action]; n > 0 { + parts = append(parts, fmt.Sprintf("%d %s", n, pastTense[action])) + } + } + if len(parts) == 0 { + return "nothing applied" + } + return strings.Join(parts, " · ") +} + +// UndoRow is one file of a run's reversal. +type UndoRow struct { + File string // the Rel the original run logged + Dir string + Steps []engine.UndoStep + Refused string // non-empty: nothing here is reversed, and why + // Selected is the checkbox. A refused file has none. + Selected bool + Actable bool + Outcome string // "", "done", "failed: ...", "declined" +} + +// UndoCounts is the header "N files · N to reverse · N refused". +type UndoCounts struct { + Files, ToReverse, Refused int +} + +// UndoTab is one run's reversal, ready to show and approve. +type UndoTab struct { + Run string + Rows []UndoRow + Counts UndoCounts + Applied bool + + sess *engine.Session + up *engine.UndoPlan + locks []*lock.Lock +} + +// PlanUndo builds the reversal of runID and locks every directory it +// touches, in name order, so nothing moves under the user while they +// choose; a directory another krino holds stops the whole undo, named +// (spec §10, GUI design §4). Nothing is touched until Apply. +// +// Like a plan, an undo gets its own session: reversing a run is a run of +// its own, logged under a new id that says which run it undoes. +func PlanUndo(ctx context.Context, e *engine.Engine, runID string) (*UndoTab, error) { + sess, err := e.NewSession(false) + if err != nil { + return nil, err + } + up, err := sess.PlanUndo(runID) + if err != nil { + sess.Close() + return nil, err + } + locks, err := sess.LockDirs(ctx, undoDirNames(up.Files), false) + if err != nil { + sess.Close() + return nil, err + } + t := &UndoTab{Run: up.Run, sess: sess, up: up, locks: locks} + t.fill() + return t, nil +} + +// fill turns the engine's undo plan into rows and counts. +func (t *UndoTab) fill() { + t.Rows = nil + t.Counts = UndoCounts{Files: len(t.up.Files)} + for _, f := range t.up.Files { + row := UndoRow{File: f.File, Dir: f.Dir, Steps: f.Steps, Refused: f.Refused} + row.Actable = f.Refused == "" + row.Selected = row.Actable + if row.Actable { + t.Counts.ToReverse++ + } else { + t.Counts.Refused++ + } + t.Rows = append(t.Rows, row) + } +} + +// undoDirNames is every directory the plan touches, once, in name order. +func undoDirNames(files []engine.UndoFile) []string { + seen := map[string]bool{} + var out []string + for _, f := range files { + if f.Dir != "" && !seen[f.Dir] { + seen[f.Dir] = true + out = append(out, f.Dir) + } + } + sort.Strings(out) + return out +} + +// SelectAll checks every file that can be reversed; SelectNone clears them. +func (t *UndoTab) SelectAll() { t.setAll(true) } +func (t *UndoTab) SelectNone() { t.setAll(false) } + +func (t *UndoTab) setAll(on bool) { + for i := range t.Rows { + t.Rows[i].Selected = on && t.Rows[i].Actable + } +} + +// Toggle flips row i's checkbox; a refused row stays off. +func (t *UndoTab) Toggle(i int) { + if i < 0 || i >= len(t.Rows) || !t.Rows[i].Actable { + return + } + t.Rows[i].Selected = !t.Rows[i].Selected +} + +// SelectedCount is how many files Undo would reverse. +func (t *UndoTab) SelectedCount() int { + n := 0 + for _, r := range t.Rows { + if r.Selected { + n++ + } + } + return n +} + +// Apply reverses the checked files. An unchecked one is not dropped: it is +// logged as declined, exactly as the terminal review logs a file the user +// said no to (spec §9). Refused files ride along unchanged, as they do on +// the command line. The locks are released afterwards: the plan is history. +func (t *UndoTab) Apply(ctx context.Context) (*engine.ApplyResult, error) { + toApply := &engine.UndoPlan{Run: t.up.Run, Cleanup: t.up.Cleanup} + for i, f := range t.up.Files { + if f.Refused == "" && !t.Rows[i].Selected { + f.Declined = true + } + toApply.Files = append(toApply.Files, f) + } + res, err := t.sess.ApplyUndo(ctx, toApply) + t.Applied = true + t.Close() + if res != nil { + t.record(res) + } + return res, err +} + +// record writes each file's outcome onto its row, matching on the file name +// the undo result carries. +func (t *UndoTab) record(res *engine.ApplyResult) { + rows := map[string]int{} + for i, r := range t.Rows { + rows[r.File] = i + } + for _, fr := range res.Files { + if i, ok := rows[fr.File.Rel]; ok { + t.Rows[i].Outcome = undoOutcome(t.Rows[i], fr) + } + } +} + +// undoOutcome is what happened to one file. Removing a directory the +// original run made is tidiness, not a restoration - it fails whenever +// something else still lives there, a declined file of this very undo +// included - so a failed undo-mkdir does not make the file itself a +// failure, exactly as the engine's own counts treat it. +func undoOutcome(row UndoRow, fr engine.FileResult) string { + out := "done" + for i, sr := range fr.Steps { + action := "" + if i < len(row.Steps) { + action = row.Steps[i].Action + } + switch sr.Status { + case "failed": + if action != "undo-mkdir" { + return "failed: " + sr.Detail + } + case "declined": + out = "declined" + } + } + return out +} + +// Close releases the directories' locks and ends the run, which Apply also +// does. One lock's failure never stops the rest from being released, or the +// session from being closed. Closing twice is not an error. +func (t *UndoTab) Close() error { + var first error + for _, l := range t.locks { + if err := l.Release(); err != nil && first == nil { + first = err + } + } + t.locks = nil + if err := t.sess.Close(); err != nil && first == nil { + first = err + } + return first +} diff --git a/gui/internal/model/history_test.go b/gui/internal/model/history_test.go new file mode 100644 index 0000000..a486c2e --- /dev/null +++ b/gui/internal/model/history_test.go @@ -0,0 +1,207 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package model + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "krino/internal/engine" + "krino/internal/lock" +) + +// applyOneRun sorts the sandbox directory and returns the run id it wrote, +// so the history tests have something to look back at. +func applyOneRun(t *testing.T, e *engine.Engine) string { + t.Helper() + tab, err := Plan(context.Background(), e, e.Dirs[0]) + if err != nil { + t.Fatal(err) + } + if _, err := tab.Apply(context.Background()); err != nil { + t.Fatal(err) + } + return tab.Run() +} + +// TestRunsListsWhatHappened: the run list shows the run just applied, with +// its directory and its counts in krino log's words, and says so once the +// run has been undone. +func TestRunsListsWhatHappened(t *testing.T) { + conf := "(path \"~/dl\")\n(rule \"all\" (move \"Out\"))\n" + e, _ := sandboxDir(t, conf, map[string]string{"a.pdf": "one", "b.pdf": "two"}) + id := applyOneRun(t, e) + + runs, err := Runs(e, 50) + if err != nil { + t.Fatal(err) + } + if len(runs) != 1 { + t.Fatalf("runs = %+v, want the one run", runs) + } + r := runs[0] + if r.ID != id || len(r.Dirs) != 1 || r.Dirs[0] != "dl" { + t.Errorf("run = %+v, want %s in dl", r, id) + } + if r.Summary != "2 moved" { + t.Errorf("summary = %q, want %q", r.Summary, "2 moved") + } + if r.Note != "" || r.UndoOf != "" { + t.Errorf("a fresh run is not marked: %+v", r) + } + + // Undo it, and the list says so - and carries the undo run itself. + tab, err := PlanUndo(context.Background(), e, id) + if err != nil { + t.Fatal(err) + } + if _, err := tab.Apply(context.Background()); err != nil { + t.Fatal(err) + } + tab.Close() + runs, err = Runs(e, 50) + if err != nil { + t.Fatal(err) + } + if len(runs) != 2 { + t.Fatalf("runs = %+v, want the run and its undo", runs) + } + if runs[0].UndoOf != id { + t.Errorf("newest run = %+v, want the undo of %s", runs[0], id) + } + if runs[1].Note != "(undone)" { + t.Errorf("undone run = %+v, want (undone)", runs[1]) + } +} + +// TestUndoRowsAndSelection: every file of the run is a row, unchecked files +// are logged as declined rather than reversed, and the counts read as the +// terminal's header does. +func TestUndoRowsAndSelection(t *testing.T) { + conf := "(path \"~/dl\")\n(rule \"all\" (move \"Out\"))\n" + e, h := sandboxDir(t, conf, map[string]string{"a.pdf": "one", "b.pdf": "two"}) + id := applyOneRun(t, e) + + tab, err := PlanUndo(context.Background(), e, id) + if err != nil { + t.Fatal(err) + } + defer tab.Close() + if tab.Counts.Files != 2 || tab.Counts.ToReverse != 2 || tab.Counts.Refused != 0 { + t.Fatalf("counts = %+v", tab.Counts) + } + if tab.SelectedCount() != 2 { + t.Fatalf("rows do not start selected: %+v", tab.Rows) + } + for i, r := range tab.Rows { + if r.File == "b.pdf" { + tab.Toggle(i) + } + } + res, err := tab.Apply(context.Background()) + if err != nil { + t.Fatal(err) + } + if res.Applied != 1 || res.Declined != 1 { + t.Errorf("result = %+v", res) + } + if _, err := os.Stat(filepath.Join(h, "dl", "a.pdf")); err != nil { + t.Errorf("the checked file was not put back: %v", err) + } + if _, err := os.Stat(filepath.Join(h, "dl", "Out", "b.pdf")); err != nil { + t.Errorf("the unchecked file was put back anyway: %v", err) + } + for _, r := range tab.Rows { + want := "done" + if r.File == "b.pdf" { + want = "declined" + } + if r.Outcome != want { + t.Errorf("%s: outcome %q, want %q", r.File, r.Outcome, want) + } + } +} + +// TestUndoRefusedRowIsNotSelectable: a file undo will not touch - here its +// destination is gone - is shown with the reason and can never be checked +// (GUI design §4). +func TestUndoRefusedRowIsNotSelectable(t *testing.T) { + conf := "(path \"~/dl\")\n(rule \"all\" (move \"Out\"))\n" + e, h := sandboxDir(t, conf, map[string]string{"a.pdf": "one", "b.pdf": "two"}) + id := applyOneRun(t, e) + if err := os.Remove(filepath.Join(h, "dl", "Out", "a.pdf")); err != nil { + t.Fatal(err) + } + + tab, err := PlanUndo(context.Background(), e, id) + if err != nil { + t.Fatal(err) + } + defer tab.Close() + if tab.Counts.Refused != 1 || tab.Counts.ToReverse != 1 { + t.Fatalf("counts = %+v", tab.Counts) + } + tab.SelectAll() + for i, r := range tab.Rows { + if r.File != "a.pdf" { + continue + } + if r.Refused == "" { + t.Errorf("the missing file carries no reason: %+v", r) + } + if r.Selected || r.Actable { + t.Errorf("a refused row is selectable: %+v", r) + } + tab.Toggle(i) + if tab.Rows[i].Selected { + t.Error("Toggle checked a refused row") + } + } +} + +// TestUndoHoldsTheDirectoryLocks: while an undo plan is open nothing else +// may work in the directories it covers, and Close - like Apply - frees +// them (GUI design §4). +func TestUndoHoldsTheDirectoryLocks(t *testing.T) { + conf := "(path \"~/dl\")\n(rule \"all\" (move \"Out\"))\n" + e, _ := sandboxDir(t, conf, map[string]string{"a.pdf": "one"}) + id := applyOneRun(t, e) + + tab, err := PlanUndo(context.Background(), e, id) + if err != nil { + t.Fatal(err) + } + if _, err := lock.Acquire(context.Background(), e.Config.LockFile("dl"), false); !errors.Is(err, lock.ErrHeld) { + t.Fatalf("an open undo plan does not hold the lock: %v", err) + } + if err := tab.Close(); err != nil { + t.Fatal(err) + } + l, err := lock.Acquire(context.Background(), e.Config.LockFile("dl"), false) + if err != nil { + t.Fatalf("closing the tab did not release the lock: %v", err) + } + l.Release() +} + +// TestPlanUndoRefusesAHeldDirectory: a directory another krino is working +// in stops the undo before anything is shown, and the message names it. +func TestPlanUndoRefusesAHeldDirectory(t *testing.T) { + conf := "(path \"~/dl\")\n(rule \"all\" (move \"Out\"))\n" + e, _ := sandboxDir(t, conf, map[string]string{"a.pdf": "one"}) + id := applyOneRun(t, e) + held, err := lock.Acquire(context.Background(), e.Config.LockFile("dl"), false) + if err != nil { + t.Fatal(err) + } + defer held.Release() + if _, err := PlanUndo(context.Background(), e, id); !errors.Is(err, lock.ErrHeld) { + t.Fatalf("PlanUndo = %v, want lock.ErrHeld", err) + } else if !strings.Contains(err.Error(), "dl") { + t.Errorf("the message does not name the directory: %v", err) + } +} diff --git a/gui/internal/model/plan.go b/gui/internal/model/plan.go index 4066eae..86acab7 100644 --- a/gui/internal/model/plan.go +++ b/gui/internal/model/plan.go @@ -54,18 +54,26 @@ type Counts struct { // chooses, and no other krino moves the files under them (GUI design §3). // A directory another run is already working in is not planned at all: the // error is lock.ErrHeld, naming the directory. -func Plan(ctx context.Context, sess *engine.Session, d *engine.Dir) (*PlanTab, error) { +// +// Each plan gets its own session, so applying it is one run of krino with +// its own run id in the log - the same shape a command line invocation +// writes. A window is not a run: one that sorted a directory and later +// undid something must not log both under one id, which would make the run +// the undo of itself. +func Plan(ctx context.Context, e *engine.Engine, d *engine.Dir) (*PlanTab, error) { + sess, err := e.NewSession(false) + if err != nil { + return nil, err + } l, err := sess.Lock(ctx, d, false) if err != nil { + sess.Close() return nil, fmt.Errorf("%s: %w", d.Name, err) } - // Start from what this run has actually landed: a name a plan the user - // closed had reserved is free again, so looking twice never creeps up - // through name-1, name-2 (spec §7.4). - sess.FinishDirectory() dp, err := sess.Plan(ctx, d) if err != nil { l.Release() + sess.Close() return nil, err } t := &PlanTab{Dir: d, sess: sess, dp: dp, lock: l} @@ -73,17 +81,22 @@ func Plan(ctx context.Context, sess *engine.Session, d *engine.Dir) (*PlanTab, e return t, nil } -// Close releases the directory's lock, which Apply also does once the plan -// is history. The session outlives the tab, so closing one plan to open -// another keeps the run - and its claims - going. Closing twice is not an -// error. +// Run is the run id this plan was applied under, "" until Apply. +func (t *PlanTab) Run() string { return t.sess.Run() } + +// Close releases the directory's lock and ends the run, which Apply also +// does once the plan is history. Closing twice is not an error. func (t *PlanTab) Close() error { if t.lock == nil { return nil } l := t.lock t.lock = nil - return l.Release() + err := l.Release() + if cerr := t.sess.Close(); err == nil { + err = cerr + } + return err } // fill turns the engine's plan into rows and counts. @@ -218,12 +231,8 @@ func (t *PlanTab) Apply(ctx context.Context) (*engine.ApplyResult, error) { } res, err := t.sess.Apply(ctx, t.dp, approved) t.Applied = true - // The disk is now the truth: a destination this plan reserved but never - // used is free again, while one it did use stays protected for the rest - // of the run (spec §7.4). - t.sess.FinishDirectory() - // An applied plan is history, so the directory is free again without - // closing the window (GUI design §3). + // An applied plan is history: the run is over and the directory free + // again, without closing the window (GUI design §3). t.Close() if res != nil { t.record(res) diff --git a/gui/internal/model/plan_test.go b/gui/internal/model/plan_test.go index bf8f19f..746ddfc 100644 --- a/gui/internal/model/plan_test.go +++ b/gui/internal/model/plan_test.go @@ -52,18 +52,14 @@ func sandboxDir(t *testing.T, conf string, files map[string]string) (*engine.Eng return e, h } -func planTab(t *testing.T, e *engine.Engine) (*PlanTab, *engine.Session) { +func planTab(t *testing.T, e *engine.Engine) *PlanTab { t.Helper() - s, err := e.NewSession(false) + tab, err := Plan(context.Background(), e, e.Dirs[0]) if err != nil { t.Fatal(err) } - t.Cleanup(func() { s.Close() }) - tab, err := Plan(context.Background(), s, e.Dirs[0]) - if err != nil { - t.Fatal(err) - } - return tab, s + t.Cleanup(func() { tab.Close() }) + return tab } // TestPlanRowsAndCounts: the tab shows one row per file with steps, a row @@ -79,7 +75,7 @@ func TestPlanRowsAndCounts(t *testing.T) { "c.txt": strings.Repeat("x", 2048), // over max-read: content unknown "plain.txt": "nothing", }) - tab, _ := planTab(t, e) + tab := planTab(t, e) if tab.Counts.Scanned != 4 || tab.Counts.Acting != 1 || tab.Counts.Excluded != 1 { t.Errorf("counts = %+v", tab.Counts) } @@ -111,7 +107,7 @@ func TestPlanRowsAndCounts(t *testing.T) { func TestSelectionAndApply(t *testing.T) { conf := "(path \"~/dl\")\n(rule \"all\" (move \"Out\"))\n" e, h := sandboxDir(t, conf, map[string]string{"a.pdf": "one", "b.pdf": "two"}) - tab, _ := planTab(t, e) + tab := planTab(t, e) if tab.SelectedCount() != 2 { t.Fatalf("rows do not start selected: %+v", tab.Rows) } @@ -154,7 +150,7 @@ func TestSelectionAndApply(t *testing.T) { func TestReplaceWithTrash(t *testing.T) { conf := "(path \"~/dl\")\n(rule \"all\" (move \"Out\"))\n" e, h := sandboxDir(t, conf, map[string]string{"a.pdf": "one"}) - tab, _ := planTab(t, e) + tab := planTab(t, e) if err := tab.Replace(0, plan.Trash); err != nil { t.Fatal(err) } @@ -195,7 +191,7 @@ func TestPlanLeavesTheDirectoryAlone(t *testing.T) { func TestPlanHoldsTheLock(t *testing.T) { conf := "(path \"~/dl\")\n(rule \"all\" (move \"Out\"))\n" e, _ := sandboxDir(t, conf, map[string]string{"a.pdf": "one"}) - tab, _ := planTab(t, e) + tab := planTab(t, e) if _, err := lock.Acquire(context.Background(), e.Config.LockFile("dl"), false); !errors.Is(err, lock.ErrHeld) { t.Fatalf("an open plan does not hold the lock: %v", err) } @@ -217,7 +213,7 @@ func TestPlanHoldsTheLock(t *testing.T) { func TestApplyReleasesTheLock(t *testing.T) { conf := "(path \"~/dl\")\n(rule \"all\" (move \"Out\"))\n" e, _ := sandboxDir(t, conf, map[string]string{"a.pdf": "one"}) - tab, _ := planTab(t, e) + tab := planTab(t, e) if _, err := tab.Apply(context.Background()); err != nil { t.Fatal(err) } @@ -241,12 +237,7 @@ func TestPlanRefusesAHeldDirectory(t *testing.T) { t.Fatal(err) } defer held.Release() - s, err := e.NewSession(false) - if err != nil { - t.Fatal(err) - } - defer s.Close() - if _, err := Plan(context.Background(), s, e.Dirs[0]); !errors.Is(err, lock.ErrHeld) { + if _, err := Plan(context.Background(), e, e.Dirs[0]); !errors.Is(err, lock.ErrHeld) { t.Fatalf("Plan = %v, want lock.ErrHeld", err) } else if !strings.Contains(err.Error(), "dl") { t.Errorf("the message does not name the directory: %v", err) @@ -259,7 +250,7 @@ func TestPlanRefusesAHeldDirectory(t *testing.T) { func TestRescanForgetsUnusedNames(t *testing.T) { conf := "(path \"~/dl\")\n(rule \"all\" (move \"Out\") (rename \"same.pdf\"))\n" e, _ := sandboxDir(t, conf, map[string]string{"a.pdf": "one", "b.pdf": "two"}) - tab, s := planTab(t, e) + tab := planTab(t, e) // Each file moves into Out and is then renamed; the second file's name // is taken, so the plan reserves a suffixed one for it. want := destinations(t, tab) @@ -271,7 +262,7 @@ func TestRescanForgetsUnusedNames(t *testing.T) { if err := tab.Close(); err != nil { t.Fatal(err) } - again, err := Plan(context.Background(), s, e.Dirs[0]) + again, err := Plan(context.Background(), e, e.Dirs[0]) if err != nil { t.Fatal(err) } @@ -286,7 +277,7 @@ func TestRescanForgetsUnusedNames(t *testing.T) { if err := again.Close(); err != nil { t.Fatal(err) } - third, err := Plan(context.Background(), s, e.Dirs[0]) + third, err := Plan(context.Background(), e, e.Dirs[0]) if err != nil { t.Fatal(err) } -- cgit v1.3