diff options
| -rw-r--r-- | docs/gui-design.md | 202 | ||||
| -rw-r--r-- | internal/engine/exclude_test.go | 9 | ||||
| -rw-r--r-- | internal/extract/plain.go | 2 | ||||
| -rw-r--r-- | internal/extract/plain_test.go | 2 |
4 files changed, 208 insertions, 7 deletions
diff --git a/docs/gui-design.md b/docs/gui-design.md new file mode 100644 index 0000000..d377bb2 --- /dev/null +++ b/docs/gui-design.md @@ -0,0 +1,202 @@ +# krino-gui design + +Status: draft, 2026-09-15. The window layout (B), the rules editor layout +(A) and sections 1–4 were agreed from mockups; sections 5–6 are the +author's defaults, to be corrected before implementation. Companion to +`docs/design.md` (the engine spec, "the spec" below), whose §12 anticipated +this GUI. + +`krino-gui` is a GTK4 window for what `krino` does in a terminal: review a +directory's plan and apply the chosen files, look back over runs and undo +them, and write rules. It adds no sorting behaviour of its own: every +decision is the engine's, and a run made in the GUI is the same run, in the +same log, that `krino log` and `krino undo` see. + +## 1. Architecture + +### 1.1 Where the code lives + +- A nested Go module `krino/gui` in `gui/`, with its own `go.mod` + (`github.com/diamondburned/gotk4/pkg` v0.4.1, and `replace krino => ../`), + building one binary, `krino-gui`. It imports `krino/internal/...`, which + Go allows because the import path is under `krino` (verified with a probe + module, 2026-09-15). +- krino's own module, `make ci` and its dependency gate are unchanged: + `go list ./...` at the root does not include `gui/`. +- `make gui` builds `krino-gui` with `CGO_ENABLED=1`; `make gui-ci` runs + gofmt, vet and the GUI module's tests; `make install-gui` installs the + binary beside `krino`. GTK 4 development files and a C compiler are + needed (present on the development laptop: GTK 4.18.6). +- Platform: Linux with X11 on the development laptop. Other platforms are not built or + tested in version 1. + +### 1.2 Layers + +1. `gui/internal/model` — plain Go, no GTK. The state of each tab and the + operations on it (scan, select, apply, runs, undo, load and edit rules, + check, test on file, save). It calls the engine and is where all logic + and all tests live. +2. `gui/internal/ui` — GTK4 widgets. Renders model state, forwards user + actions, runs model operations off the main loop and delivers results + with `glib.IdleAdd`. No decisions are made here. +3. `gui/cmd/krino-gui` — flags (`-c FILE`, `--version`) and startup. + +### 1.3 Changes to krino itself + +Made first, each tested through the CLI's existing tests as well as new ones: + +- **Session.** The directory orchestration now in `cmd/krino/sort.go` — + lock per directory, opening the log and choosing the run id, planning with + run-wide claims, applying the approved set, keeping only where files ended + up as claims afterwards (spec §7.4) — moves into `internal/engine` as a + `Session` used by both `cmd/krino` and the GUI. The CLI's behaviour and + output do not change. Undo gets the same treatment (locks of the + directories involved, log, run id). +- **Load with overrides.** `engine.LoadWith(mainFile, overrides map[string][]byte, names...)` + loads the configuration with some files' text replaced in memory, so + unsaved editor text is checked exactly as a run would read it. +- **Explain with outcomes.** `Explain` also returns, per matching rule, the + capture values, and the chain the file would get (each step's action, + destination after placeholders and conflicts, skip reason), so the test + pane shows `{1}=2026` and `→ Pictures/Screenshots/2026-09/`. +- **Printer and splice.** `internal/config` gains a printer for rule and + exclude forms (two-space indentation as in `examples/`) and a splice that + replaces one form's byte range (`sexp.Node.Pos`/`End`) and nothing else. + `config.Rule` and `config.Exclude` record their form's end position. + +## 2. Window + +One window, three tabs across the top: **Plan**, **History & undo**, +**Rules** (layout B of the mockups). A status bar at the bottom shows the +current operation, its progress and Cancel. Text from files, logs and tools +is shown with plain-text setters only (never Pango markup), and control, +bidirectional and line-separator characters are escaped as the CLI's +`display` does (spec §15.1), so a hostile name cannot reorder or restyle +what is shown. + +## 3. Plan tab + +- A directory picker in the toolbar lists `include`; **Scan** plans it. + Scanning takes the directory's lock, as a terminal review does; the lock + is held while the plan is shown and released after Apply, on choosing + another directory, or on closing the window. A held lock is reported in + the tab, not waited for. While a plan is open, `krino -y` of that + directory fails immediately, as with a terminal review left open. +- The table (a virtualised `GtkColumnView`): checkbox, file, action(s), + destination, rule. Rows start checked (as `[a]`); Select all / None. + A file whose steps are all skipped shows its skip reason and has no + checkbox. Files with a "content unreadable" or duplicate-check warning + that no rule could decide are listed under "needs attention", never + checkable. +- A row's context menu: **Trash instead** and **Delete permanently + instead…** (confirmation dialog naming the file), replacing that file's + steps with the one step, as `t` and `d`/`y` do in review. +- The right pane: the selected file's explanation — every exclude and rule + with each test's yes/no/?, captures, and the planned chain. +- **Apply N selected** runs in the background. Checked files are applied, + unchecked ones logged as declined (as choosing per file in the terminal). + **Cancel** is Ctrl-C: the step in flight finishes and is logged, then it + stops. Rows then show done / failed (with the reason) / declined, and a + summary line like the CLI's. +- Not in version 1: watching the directory, a `--min-age` override, several + directories in one Apply. + +## 4. History & undo tab + +- Left: runs newest first (last 50, **Show more**), with time, directories, + counts, `(undone)` / `(partly undone)`; undo runs labelled "undo of …". + Selecting an undo run offers to finish the run it undid, as plain + `krino undo` does. +- Main: that run's undo plan — checkbox, file, reversal, target; refused + files in red with the reason, not checkable; the header + "N files · N to reverse · N refused". +- **Undo N selected** takes the locks of the directories involved (a held + one is reported), reverses checked files, logs unchecked ones as + declined, runs in the background with Cancel (the file in flight is + finished), then shows outcomes and refreshes the run list. +- Not in version 1: raw log entries, searching runs. + +## 5. Rules tab + +Layout A of the mockups: the directory's exclusions and rules on the left, a +form in the middle, **Test on file** on the right; a **Text** sub-tab edits +the whole directory file. Only directory files (`dirs/NAME.conf`) are +edited; `krino.conf` is not, in version 1. + +### 5.1 Forms + +- **Rule form:** name; conditions as a tree (all top-level conditions must + hold, as `when`), with **+ test** (type, name, path, content, size, age, + duplicate with optional directories, matched) and **+ and / or / not**; + actions in order (copy, move, rename, delete, delete permanent, stop) with + their argument; rule settings case, fold, on-conflict ("default" leaves + them out). Placeholder help next to destination and name fields. +- **Exclude form:** its conditions, as for a rule. +- **Directory settings form:** path, recursive, max-depth, min-age, + max-read, max-size, busy, case, fold, on-conflict, ignore patterns. +- **Rule list:** add (after the selected rule), delete (confirmation), move + up / move down. A rule's block for moving or deleting is its form plus the + comment lines directly above it with no blank line between. +- A form edit replaces only that form's text. If the form has comments + inside it, saving a form edit warns that they will be dropped and offers + the Text tab instead. + +### 5.2 Text + +The whole file in a `GtkTextView` (monospace, line numbers), edited freely. +Switching from Text to Forms re-reads the text; a text that does not parse +keeps the Text tab until fixed. + +### 5.3 Live check, test, save + +- After each change (300 ms debounce) the model loads the configuration with + the unsaved text (`LoadWith`) and shows every diagnostic with line:col; + in Forms a diagnostic inside a form marks that field when its position + falls in it, otherwise it is listed above the form. A bar reads + "✓ check: no errors" or the count. +- **Test on file:** pick one of the directory's scanned files or any file + under its path; runs `Explain` against the unsaved text, in the + background (content extraction can take seconds), and shows the trace, + captures and chain. +- **Save** is refused while there are errors. It writes atomically (temp + file, fsync, rename, mode kept, symlinks resolved as `krino new` does), + after keeping the previous text as `NAME.conf.bak` beside it. If the file + changed on disk since it was loaded, a dialog offers Reload, Overwrite or + Cancel. **Revert** discards unsaved changes. + +## 6. Safety, errors, tests + +- **Nothing changes on disk without a click on Apply, Undo, Save, or a + confirmed delete-instead.** The GUI uses the engine's lock, log, Trash, + exclusions and duplicate protection unchanged. +- Engine errors appear in an info bar in the tab they belong to. A panic in + a background operation is recovered, reported in a dialog, and leaves the + tab as it was before the operation. +- **Tests:** + - `internal/model` against sandboxed homes (HOME and XDG_* under a temp + directory), as the engine and CLI tests do: scan, select, apply, + decline, trash/delete-instead, cancel; runs, undo plans, undo; load, + edit, check with overrides, test on file, save (atomic, backup, + external change), rule add/delete/move with attached comments. + - Printer and splice: `parse(print(form))` gives the same form for + generated forms (a fuzz target), and a splice leaves every byte outside + the form's range unchanged. + - Session refactor: every existing `cmd/krino` test stays green, plus a + test that the GUI's session and the CLI produce the same log entries + for the same plan. + - GTK layer: no automated tests (no Xvfb on the development laptop); a written manual + checklist, run on the real display with screenshots (`import`), before + each GUI release. + +## 7. Milestones + +1. krino core: Session (sort and undo), LoadWith, Explain outcomes; CLI + unchanged. Released as a krino version. +2. GUI module skeleton and the Plan tab. +3. History & undo tab. +4. Rules tab: Text sub-tab, live check, Test on file, Save. +5. Printer and splice; Forms (rules, excludes, directory settings; list add, + delete, move). + +Each milestone ends working and tested; the GUI is reviewed and corrected +after milestones 2, 4 and 5, from screenshots and use. diff --git a/internal/engine/exclude_test.go b/internal/engine/exclude_test.go index 3398b07..6381a04 100644 --- a/internal/engine/exclude_test.go +++ b/internal/engine/exclude_test.go @@ -212,8 +212,7 @@ func TestExplainReportsExclusionAndSize(t *testing.T) { // TestExcludeFailsClosedOnUnreadableContent: an exclude meant to protect // files holds when its content test cannot read a file (over max-read), so -// no rule acts on a file krino could not check (review M11, Łukasz's -// decision). +// no rule acts on a file krino could not check (review M11). func TestExcludeFailsClosedOnUnreadableContent(t *testing.T) { big := "confidential " + strings.Repeat("x", 2048) h, dl := excludeTree(t, map[string]string{"big.txt": big, "small.txt": "nothing to hide"}) @@ -273,7 +272,7 @@ func TestLoadChecksMainExcludesWithoutDirectories(t *testing.T) { // TestNoTextFormatIsNoMatch: a file whose format has no text cannot contain // a keyword, so a content exclude without a type does not set it aside, and // no "content unreadable" warning is raised - while a real read failure (over -// max-read) still fails closed (re-review N2, Łukasz's decision). +// max-read) still fails closed (re-review N2). func TestNoTextFormatIsNoMatch(t *testing.T) { big := "confidential " + strings.Repeat("x", 2048) h, dl := excludeTree(t, map[string]string{"photo.jpg": "\xff\xd8\xff\x00\x01binary", "big.txt": big}) @@ -315,8 +314,8 @@ func TestNoTextFormatIsNoMatch(t *testing.T) { // TestTextTurningBinaryIsNoText: a file with no known extension whose first // 8 KiB read as text but which holds a NUL further on - a self-extracting // installer - counts as having no text, like an image: a content exclude -// does not set it aside and there is no warning (Łukasz, after the plan 10 -// re-check: "ignore such ones"). +// does not set it aside and there is no warning (decided after the plan 10 +// re-check). func TestTextTurningBinaryIsNoText(t *testing.T) { mixed := "confidential " + strings.Repeat("x", 9000) + "\x00tail" h, _ := excludeTree(t, map[string]string{"mixed": mixed}) diff --git a/internal/extract/plain.go b/internal/extract/plain.go index ce9dedc..beffae1 100644 --- a/internal/extract/plain.go +++ b/internal/extract/plain.go @@ -39,7 +39,7 @@ func readDecoded(path string) (string, error) { // sample that merely looks like UTF-8 must hold for the WHOLE file — no // NUL byte anywhere, and no invalid UTF-8 anywhere past the sample — or // the file is ErrUnsupported after all (a self-extracting installer has no -// text in krino's sense, Łukasz's decision after the plan 10 re-check); the +// text in krino's sense, decided after the plan 10 re-check); the // Latin-1 fallback in decode // never applies to a sniffed file, only to a file whose extension already // names it as text. D2: when the file continues past the sample (n == diff --git a/internal/extract/plain_test.go b/internal/extract/plain_test.go index c8e1008..e53ac73 100644 --- a/internal/extract/plain_test.go +++ b/internal/extract/plain_test.go @@ -156,7 +156,7 @@ func TestToolsListedInOrder(t *testing.T) { // sample — sniffText must reject the whole file, not just decode what the // sample alone promised (it must not fall back to Latin-1 the way a known // text extension would). Such a file - a self-extracting installer, say - -// has no text in krino's sense (Łukasz, after the plan 10 re-check). +// has no text in krino's sense (decided after the plan 10 re-check). func TestSniffWholeFileMustBeValid(t *testing.T) { e := newWithPath("") data := append([]byte(strings.Repeat("x", 8192)), 0xFF, 0x00) |