Merge remote-tracking branch 'gh/main' into wire2/j4qt
# Conflicts: # context/test-evidence/clients.md
This commit is contained in:
Edward Langley
@@ -7,6 +7,8 @@ description: Dispatch parallel agents to work through open beads and shepherd ea
|
||||
|
||||
An orchestrator chunks the open backlog, fans agents out, reviews what comes
|
||||
back, and merges. `AGENTS.md` has the history; this is the operating procedure.
|
||||
This file is the brief checklist's one home, and `agent-rules.md` beside it
|
||||
holds the rules each agent follows.
|
||||
|
||||
## Every agent gets its own worktree. No exceptions.
|
||||
|
||||
@@ -17,9 +19,8 @@ here. If you discover it mid-wave, stop every agent, have each create a worktree
|
||||
and copy across **only files it can prove it touched**, then `git checkout --`
|
||||
those files in the primary tree.
|
||||
|
||||
In the worktree: `direnv allow`, or `bd` reports a version *mismatch* rather than
|
||||
failing cleanly. `direnv status` prints `Found RC allowed 0`, and `0` means
|
||||
allowed — it reads like the opposite.
|
||||
In the worktree `.envrc` must be allowed, or `bd` fails confusingly:
|
||||
`agent-rules.md`, "Discipline per ticket".
|
||||
|
||||
## Partitioning
|
||||
|
||||
@@ -33,32 +34,41 @@ it must cross. Two agents adding a workspace member will still collide in
|
||||
|
||||
## What every brief must carry
|
||||
|
||||
**Point the agent at [`agent-rules.md`](agent-rules.md) and tell it to read it in
|
||||
full first.** That file is the agent's standing rules: base and push mechanics,
|
||||
the gate list, the claim, the vacuity shapes, evidence, the files it must not
|
||||
write, GitHub conventions, the hand-back and disk cleanup. Do not restate them
|
||||
in the brief. A restated rule drifts from its source, and the brief is the copy
|
||||
nobody corrects. The brief adds only what varies per wave:
|
||||
|
||||
1. **The expected base commit, by name.** Worktrees do not reliably branch from
|
||||
HEAD — one was created three merges behind and only noticed because it was
|
||||
told to check.
|
||||
2. **The baseline**, so "N passing" is checkable rather than asserted: Rust test
|
||||
count, harness counts, node counts.
|
||||
3. **Its ground, and the ground that is not.**
|
||||
4. **Never write** `.beads/**`, `prolog-sessions/kb.pl`, `context/repo-map.md`.
|
||||
Agents read them freely and report proposed text; the orchestrator applies it.
|
||||
5. **The commit trailers and PR footer**, spelled out.
|
||||
6. **Do not push to `main`, do not merge.** The orchestrator merges.
|
||||
7. **What to report**: branch and head SHA, PR number, what changed per ticket,
|
||||
*what could not be verified and why*, proposed repo-map/kb text, and **what it
|
||||
learned that the ticket did not predict** — consistently the most valuable
|
||||
part, and omitted unless asked for explicitly.
|
||||
HEAD. One was created three merges behind and was only noticed because it
|
||||
was told to check. The agent checks against it: `agent-rules.md` rule 1.
|
||||
2. **A reference baseline**, so "N passing" is checkable and a drop is
|
||||
visible: Rust test count, harness counts, node counts, **and the machine**.
|
||||
The same commit once measured `improvise-io`'s binary at 1.4s on one machine
|
||||
and 3.7–5.2s on another, so a figure with no machine is a claim about the
|
||||
suite, which it is not. The agent still measures its own.
|
||||
3. **Its ground, and the ground that is not.** Name any orchestrator-only file
|
||||
it is exceptionally allowed to edit.
|
||||
4. **Its `BEADS_ACTOR` and its tickets.**
|
||||
5. **The attribution line, commit trailers and PR footer**, spelled out.
|
||||
They name the agent (Claude's line opens `_Posted by Claude (<model>)…_`)
|
||||
and this session's URL, which is why the shared `agent-rules.md` cannot
|
||||
hold them.
|
||||
6. **An explicit ask for what the ticket did not predict.** `agent-rules.md`
|
||||
lists it in the hand-back. Say it in the brief anyway: it is consistently
|
||||
the most valuable part of a hand-back, and agents omit it unless asked.
|
||||
|
||||
## Method to require, because this repo's defect is a test that passes
|
||||
When a hand-back teaches a new rule, add it to `agent-rules.md` in the same
|
||||
wave, not to the next brief.
|
||||
|
||||
- Failing test first, then the fix.
|
||||
- Every guard proved by **injecting the fault it names** and watching it fail
|
||||
**by name**. An injection that does not compile reads as a clean run, so
|
||||
require a positive signal the build ran.
|
||||
- `cargo test --workspace` is **fail-fast**; `--no-fail-fast` for any claim about
|
||||
blast radius.
|
||||
- A textually clean merge can **delete tests** with no conflict marker, and
|
||||
`cargo check` will not notice. After any merge that moves a test module, diff
|
||||
test **names** — a count is masked by the other side's additions.
|
||||
## Method to require
|
||||
|
||||
`agent-rules.md`, "Discipline per ticket" and "Gates": failing test first, a
|
||||
rival injected and watched failing by name, `--no-fail-fast`, and test *names*
|
||||
diffed after any merge. Hold the hand-back to it. This repo's characteristic
|
||||
defect is a test that passes.
|
||||
|
||||
## Shepherding
|
||||
|
||||
@@ -72,7 +82,9 @@ it must cross. Two agents adding a workspace member will still collide in
|
||||
commit reads as a pass.
|
||||
- Merge on GitHub, then `git fetch gh && git merge --ff-only gh/main`.
|
||||
- Anything posted to GitHub goes out under the owner's account: open every
|
||||
comment with a line saying a Codex agent wrote it.
|
||||
comment with a line naming the model and saying an agent wrote it. That
|
||||
wording is agent-neutral because this one file serves Claude and Codex
|
||||
(`.claude/skills/wave` is a symlink to `.agents/skills/wave`).
|
||||
|
||||
## Between waves
|
||||
|
||||
|
||||
@@ -0,0 +1,198 @@
|
||||
# Standing rules for a wave agent
|
||||
|
||||
Every work-unit agent reads this **in full before its first command**. A brief
|
||||
points here and adds only what is specific to its wave: the expected base
|
||||
commit, the agent's ground, its `BEADS_ACTOR`, and its tickets. Every rule below
|
||||
was bought by a failure.
|
||||
|
||||
This file holds each agent-side rule **once**. The brief checklist is
|
||||
`SKILL.md` beside it. Where `AGENTS.md` ("The Wave Workflow") carries a longer
|
||||
history, the rule here points there. Change a rule here, not in a brief, and
|
||||
not in a second copy.
|
||||
|
||||
## Base, branch, push
|
||||
|
||||
1. **Verify your base against the commit your brief names, first.** Run
|
||||
`git log --oneline -1` and compare. Worktrees do not reliably branch from
|
||||
the orchestrator's HEAD: one was created three merges behind, and it was
|
||||
only noticed because the agent was told to look.
|
||||
2. **Branch from `gh/main`, never `main` or `origin/main`.** `git fetch gh &&
|
||||
git checkout -b <branch> gh/main`. A worktree handed to you may already sit
|
||||
on unpushed commits. Why: `AGENTS.md`, "Shipping a wave" (the two remotes
|
||||
both spelled `main`, and the worktree-base vector).
|
||||
3. **Before opening a PR, check `git diff --numstat gh/main...<branch>`.** Do
|
||||
not use `gh pr view --json files`: that is the union of your commits' files,
|
||||
not the effective diff.
|
||||
4. **Push with an explicit refspec:** `git push gh HEAD:refs/heads/<branch>`. A
|
||||
branch cut from `gh/main` tracks `gh/main`, so a bare push targets `main` and
|
||||
is refused.
|
||||
5. **Before pushing a fix to an existing PR, check `gh pr view <n> --json
|
||||
state`.** GitHub deletes a merged PR's branch. A push to it then prints
|
||||
`* [new branch]` and lands nowhere. Why: `AGENTS.md`, "Shipping a wave".
|
||||
6. **The "Session Completion" block near the end of `AGENTS.md` does not apply
|
||||
to you.** It says `git merge origin/main` and then a bare `git push`. On a
|
||||
wave branch the first is the #145 trap and the second is refused (rule 4).
|
||||
Rules 1–5 replace it. The block sits between bd's generated
|
||||
`BEGIN/END BEADS INTEGRATION` markers, so it is not edited by hand
|
||||
(improvise-x1uk).
|
||||
|
||||
## Commit early: a worktree is not durable storage
|
||||
|
||||
Commit a WIP as soon as your first test run is green, and again after every
|
||||
meaningful step. Push your branch (rule 4) once it is worth keeping. An agent
|
||||
finished a whole ticket with every gate green and lost all of it, uncommitted,
|
||||
when its worktree was deleted in a disk cleanup. A local commit survives a
|
||||
deleted worktree. Tidying history before the PR is fine; losing it is not.
|
||||
|
||||
## Gates
|
||||
|
||||
```
|
||||
direnv exec . cargo test --workspace --no-fail-fast
|
||||
direnv exec . cargo clippy --workspace --all-targets -- -D warnings
|
||||
direnv exec . ./scripts/fmt-workspaces.sh # NOT cargo fmt --all; exit 2 = the gate did not run
|
||||
direnv exec . python3 scripts/check-comment-runs.py --check-evidence
|
||||
direnv exec . ./scripts/checkers.sh
|
||||
direnv exec . ./scripts/node-test.sh 'web/tests/*.test.js'
|
||||
```
|
||||
|
||||
- **Never `cargo fmt --all`.** It reaches only **one** of this repo's two
|
||||
workspace roots, because `harness/opfs-contract` declares its own
|
||||
`[workspace]`. The obvious repair is worse: `cargo fmt -p opfs-contract` from
|
||||
the repo root exits **1**, *"not a member of the workspace"*, which looks
|
||||
exactly like a real formatting failure (improvise-b4ks). Both spellings fail
|
||||
in the direction that looks like success.
|
||||
- **Run every gate under `direnv exec .`.** `./scripts/checkers.sh` scrubs the
|
||||
environment to an allowlist, so a bare run drops `tpl` off `PATH` and reports
|
||||
spurious failures.
|
||||
- **Never pipe a gate.** `$?` must be the gate's own. And
|
||||
`check-comment-runs.py --check-evidence` prints two lines with the comment
|
||||
cap **first**, so `| tail -1` silently drops half of it.
|
||||
- **`--no-fail-fast`**, because plain `cargo test --workspace` stops at the
|
||||
first failing binary. Use it for any claim about blast radius.
|
||||
- If you touch a browser-bundle crate (core, formula, io, protocol, command,
|
||||
wasm-client, worker-server), also run
|
||||
`cargo check --target wasm32-unknown-unknown -p <crate>` **for each crate**.
|
||||
CI checks each one separately, so feature unification cannot hide a leak.
|
||||
- If you touch `web/` or `harness/`, run the Playwright suite in `harness/`.
|
||||
- **Measure your own baseline** before changing anything, and report it with
|
||||
the machine it was measured on. A figure from the brief is a reference, not
|
||||
your baseline.
|
||||
- **Merge `gh/main` and re-run every gate before calling a PR green.** Then
|
||||
check that no test *disappeared*. Diff test **names**, not counts: a
|
||||
textually clean merge can delete a test module with no conflict marker, and
|
||||
the other side's new tests mask the loss in a count.
|
||||
- **The shell guard refusing a compound command means NEITHER part ran.** Split
|
||||
the command. After a refusal, re-check the state it was meant to produce.
|
||||
- **Re-run the full suite after reverting an injected fault.** A revert applied
|
||||
to the wrong occurrence once shipped two faults past every other gate.
|
||||
|
||||
## Discipline per ticket
|
||||
|
||||
- **Claim first:** `export BEADS_ACTOR=<your actor>` in every shell, then
|
||||
`bd update <id> --claim`. Exit 13 means another actor holds it. Pick another;
|
||||
do not retry.
|
||||
- **`bd` needs `.envrc` allowed in your worktree.** Without it you get a
|
||||
**version mismatch**, not a clean failure. Run `direnv allow` in the worktree,
|
||||
or rely on the `[whitelist] prefix` in `~/.config/direnv/direnv.toml`.
|
||||
`direnv status` reports `Found RC allowed 0`, and `0` means **allowed** (the
|
||||
enum is 0=Allowed, 1=NotAllowed). It reads like the opposite.
|
||||
- **Reproduce at HEAD before fixing.** A dozen tickets were stale-open in one
|
||||
campaign, one fixed two days before it was filed. If it does not reproduce,
|
||||
find the fixing commit, confirm with `git merge-base --is-ancestor <sha> HEAD`,
|
||||
report the SHA, and close nothing.
|
||||
- **Write a failing test first, then inject a rival and watch it fail.** Run
|
||||
it; do not reason about it. Report *where* it failed, by case name. An
|
||||
injection that does not compile reads as a clean run, so get a positive
|
||||
signal that the build ran.
|
||||
- **Check your test against every vacuity shape seen here:**
|
||||
- an assertion the default state already satisfies;
|
||||
- `f(x) == f(x)`: calling a pure `Cmd` twice against one immutable context;
|
||||
- a disjunctive assertion whose other branch is the bug's own symptom;
|
||||
- a rival that fails the *setup*, which proves the neighbouring question
|
||||
(e.g. setup and subject share an `EffectKind`);
|
||||
- a fixture that is right by accident (expected value 1, group at row 0,
|
||||
bystander already on the relayed value);
|
||||
- a fixture or stub that lacks the structure the hazard needs, or fails in a
|
||||
wider shape than the real fault;
|
||||
- hand-picked fixture cases where the set could be **enumerated**: four
|
||||
vacuities in `fmt-workspaces-test.sh` were each a case nobody thought of
|
||||
(improvise-90ea);
|
||||
- a rendered-output test at a width where the thing asserted is never drawn;
|
||||
- a fold test that batches differently from the real producer (a
|
||||
`Binding::Sequence` is ONE effect vec);
|
||||
- **the cursor at index 0**: "the item under the cursor" and "the first item"
|
||||
are indistinguishable there, and that was four of five findings in one
|
||||
review. Seat the cursor at a nonzero index of a list with at least two
|
||||
elements;
|
||||
- a determinism check against ONE context instance: every `HashMap` iterates
|
||||
the same way twice. Build two equal contexts independently
|
||||
(improvise-hzjo);
|
||||
- two derivations of one fact where the fixture makes them agree (drawn vs
|
||||
committed, edit text vs value, `1200` vs `1.23456789`). Use a value where
|
||||
they disagree.
|
||||
- **Record each bug in `context/test-evidence/<area>.md`**, keyed by test name
|
||||
(module-qualify the name when the leaf is not unique). It is append-at-EOF, so
|
||||
**expect a conflict**. Keep both sides and verify line by line against both
|
||||
parents: `--check-evidence` cannot see a dropped entry.
|
||||
- **Comment runs are at most two lines.** Reasoning goes in `context/`, not in
|
||||
comments (`context/design-principles.md` §6).
|
||||
- **If the acceptance criteria look wrong, argue it** rather than following
|
||||
them silently. Three agents inverted theirs in one campaign, and all three
|
||||
were right.
|
||||
|
||||
## Files you must not write
|
||||
|
||||
`.beads/**` (never commit `issues.jsonl`, never `git checkout` it),
|
||||
`prolog-sessions/kb.pl`, `context/repo-map.md`: `AGENTS.md`, "Who owns what".
|
||||
Also `AGENTS.md`, unless your brief grants you a named section. Report proposed
|
||||
text for any of these in your hand-back.
|
||||
|
||||
## GitHub
|
||||
|
||||
- `gh` is shell-aliased to a different tool. Use
|
||||
`nix shell nixpkgs#gh --command gh …`, and pass bodies with `-F body=@file`.
|
||||
- **Everything posted goes out under the repository owner's account.** Open
|
||||
every PR body and comment with the attribution line your brief gives you,
|
||||
verbatim. It names your agent and model, which is how our posts are told
|
||||
apart from an automated Codex review (`AGENTS.md`, "Shipping a wave").
|
||||
- The attribution line, commit trailers and PR footer all come from your brief.
|
||||
They name the agent and the session, so this shared file cannot hold them.
|
||||
- Read reviews with `./scripts/pr-reviews <n>`, and do not pipe it. How to tell
|
||||
a ChatGPT Codex review from ours, and why Copilot is not coming: `AGENTS.md`,
|
||||
"Shipping a wave". Answer each Codex review on its thread.
|
||||
- **Do not merge, and do not close beads.** Avoid stacked PRs. If you must
|
||||
stack, never let the base branch be deleted.
|
||||
|
||||
## Hand-back, per ticket
|
||||
|
||||
Report:
|
||||
|
||||
- your branch, head SHA and PR number. Review is dispatched against the SHA;
|
||||
- what changed, per ticket;
|
||||
- the claim result;
|
||||
- your measured baseline;
|
||||
- whether the ticket reproduced (the SHA if it did not);
|
||||
- the rival and where it failed;
|
||||
- what you could not verify, and why;
|
||||
- proposed repo-map and `kb.pl` text;
|
||||
- **what the ticket did not predict**, consistently the most valuable part.
|
||||
|
||||
## Disk
|
||||
|
||||
Each worktree's `target/` grows to 3–6G, and a full disk (ENOSPC) once stopped
|
||||
every agent. **When you hand back, delete your own build output, and only
|
||||
yours.**
|
||||
|
||||
Run `pwd` and `git rev-parse --show-toplevel` first, and read the answer. Then
|
||||
delete by **literal absolute path**, typed out:
|
||||
`rm -rf /Users/.../.claude/worktrees/agent-<you>/target`.
|
||||
|
||||
**Never interpolate a computed path into `rm -rf`.** One agent's worktree had
|
||||
been removed, so `$(git rev-parse --show-toplevel)` printed nothing and the
|
||||
command became `rm -rf "/target"`. A guarded one-liner was tried, but the
|
||||
worktree shell guard refuses it as too complex, which pushed agents back to an
|
||||
unguarded form. A literal path is the version that is both allowed and safe.
|
||||
|
||||
Keep your branch and any uncommitted work. Delete only `target/` (and
|
||||
`harness/opfs-contract/target` if you built it). If you see ENOSPC, stop and
|
||||
report rather than deleting anything outside your worktree.
|
||||
Symlink
+1
@@ -0,0 +1 @@
|
||||
../../.agents/skills/wave
|
||||
@@ -1,83 +0,0 @@
|
||||
---
|
||||
name: wave
|
||||
description: Dispatch parallel agents to work through open beads and shepherd each to a merged PR. Use when an orchestrator is running a wave, partitioning the backlog, or writing a brief for a work-unit agent.
|
||||
---
|
||||
|
||||
# Running a wave
|
||||
|
||||
An orchestrator chunks the open backlog, fans agents out, reviews what comes
|
||||
back, and merges. `AGENTS.md` has the history; this is the operating procedure.
|
||||
|
||||
## Every agent gets its own worktree. No exceptions.
|
||||
|
||||
Dispatch with `isolation: "worktree"`. Without it every agent edits the **same**
|
||||
working tree: one `git status` shows four agents' half-finished files, any commit
|
||||
sweeps up the others' work, and any `git checkout` destroys it. This has happened
|
||||
here. If you discover it mid-wave, stop every agent, have each create a worktree
|
||||
and copy across **only files it can prove it touched**, then `git checkout --`
|
||||
those files in the primary tree.
|
||||
|
||||
In the worktree: `direnv allow`, or `bd` reports a version *mismatch* rather than
|
||||
failing cleanly. `direnv status` prints `Found RC allowed 0`, and `0` means
|
||||
allowed — it reads like the opposite.
|
||||
|
||||
## Partitioning
|
||||
|
||||
By **crate or directory**, never by ticket count. **Tickets that share a file go
|
||||
to one agent**, to be done sequentially — four widget migrations all touching
|
||||
`src/draw.rs` are a guaranteed conflict split four ways. Group first, then count.
|
||||
|
||||
Tell each agent what ground is *not* theirs, and to report rather than guess when
|
||||
it must cross. Two agents adding a workspace member will still collide in
|
||||
`Cargo.toml`/`Cargo.lock`; keep both sides.
|
||||
|
||||
## What every brief must carry
|
||||
|
||||
1. **The expected base commit, by name.** Worktrees do not reliably branch from
|
||||
HEAD — one was created three merges behind and only noticed because it was
|
||||
told to check.
|
||||
2. **The baseline**, so "N passing" is checkable rather than asserted: Rust test
|
||||
count, harness counts, node counts.
|
||||
3. **Its ground, and the ground that is not.**
|
||||
4. **Never write** `.beads/**`, `prolog-sessions/kb.pl`, `context/repo-map.md`.
|
||||
Agents read them freely and report proposed text; the orchestrator applies it.
|
||||
5. **The commit trailers and PR footer**, spelled out.
|
||||
6. **Do not push to `main`, do not merge.** The orchestrator merges.
|
||||
7. **What to report**: branch and head SHA, PR number, what changed per ticket,
|
||||
*what could not be verified and why*, proposed repo-map/kb text, and **what it
|
||||
learned that the ticket did not predict** — consistently the most valuable
|
||||
part, and omitted unless asked for explicitly.
|
||||
|
||||
## Method to require, because this repo's defect is a test that passes
|
||||
|
||||
- Failing test first, then the fix.
|
||||
- Every guard proved by **injecting the fault it names** and watching it fail
|
||||
**by name**. An injection that does not compile reads as a clean run, so
|
||||
require a positive signal the build ran.
|
||||
- `cargo test --workspace` is **fail-fast**; `--no-fail-fast` for any claim about
|
||||
blast radius.
|
||||
- A textually clean merge can **delete tests** with no conflict marker, and
|
||||
`cargo check` will not notice. After any merge that moves a test module, diff
|
||||
test **names** — a count is masked by the other side's additions.
|
||||
|
||||
## Shepherding
|
||||
|
||||
- Wait on checks with `scripts/pr-watch <pr> --expect <sha>`. Never hand-roll a
|
||||
poll; see the `pr-watch` skill for the three ways they report green while wrong.
|
||||
- Review before merging. Copilot's quota may be exhausted and its request is a
|
||||
**silent no-op**; an absent review and an unrequested one both read as
|
||||
"pending". Dispatch an adversarial reviewer and give it the head SHA, the base,
|
||||
and a test-merge, so it reviews the tree that will exist.
|
||||
- A re-review is of the **delta**, at a named SHA. A review of a superseded
|
||||
commit reads as a pass.
|
||||
- Merge on GitHub, then `git fetch gh && git merge --ff-only gh/main`.
|
||||
- Anything posted to GitHub goes out under the owner's account: open every
|
||||
comment with a line saying a Claude agent wrote it.
|
||||
|
||||
## Between waves
|
||||
|
||||
Run the code-review skill on the merged range. The characteristic find here is
|
||||
not a wrong line, it is **a test that passes while the thing it names is
|
||||
broken** — a vacuous fold test, a one-category fixture, a helper that fabricates
|
||||
what the code under test needs. Ask the reviewer to inject the fault and watch
|
||||
the check fail.
|
||||
@@ -100,52 +100,19 @@ Two agents each adding a workspace member will still collide in
|
||||
|
||||
### Every wave agent's brief must include
|
||||
|
||||
1. **Verify your base commit first.** Worktrees do **not** reliably branch from
|
||||
the orchestrator's HEAD — one was created three merges behind and only
|
||||
noticed because it was told to look. `git log --oneline -1` against a named
|
||||
commit, reset onto `main` if behind. Name the expected commit in the brief.
|
||||
2. **The baseline test count**, so "1090 passing" is checkable rather than
|
||||
asserted, and a drop is visible. Tell the agent to **measure its own** and
|
||||
cite the machine: the same commit measured `improvise-io`'s binary at 1.4s on
|
||||
one machine and 3.7–5.2s on another, so a figure with no machine is a claim
|
||||
about the suite, which it is not.
|
||||
3. **The gate list, and it must not say `cargo fmt --all`.** That reaches only
|
||||
**one** of this repo's two workspace roots — `harness/opfs-contract` declares
|
||||
its own `[workspace]` — and the obvious repair is worse: `cargo fmt
|
||||
-p opfs-contract` from the repo root exits **1**, *"not a member of the
|
||||
workspace"*, which is indistinguishable from a real formatting failure
|
||||
(improvise-b4ks). Both spellings fail in the direction that looks like
|
||||
success. The gate is:
|
||||
The wave skill holds both halves, each in one place (improvise-1f8e):
|
||||
|
||||
```
|
||||
direnv exec . ./scripts/fmt-workspaces.sh # exit 2 = the gate did not run
|
||||
```
|
||||
- **The brief checklist** — what an orchestrator must put in a brief — is
|
||||
`.agents/skills/wave/SKILL.md`, "What every brief must carry".
|
||||
- **The agent's standing rules** — base and push, gates, claim, vacuity shapes,
|
||||
evidence, forbidden files, GitHub, hand-back, disk, commit early — are
|
||||
`.agents/skills/wave/agent-rules.md`. A brief points there rather than
|
||||
restating them.
|
||||
|
||||
Every gate goes through `direnv exec .` — `./scripts/checkers.sh` scrubs the
|
||||
environment to an allowlist, so a bare invocation drops `tpl` off `PATH` and
|
||||
reports spurious failures. And **never pipe a gate**: `$?` must be the gate's
|
||||
own, and `check-comment-runs.py --check-evidence` prints two lines with the
|
||||
comment cap **first**, so `| tail -1` silently drops half of it.
|
||||
4. **Commit frequently, so the work is safe in git.** A WIP commit after the
|
||||
first green test, and again after every meaningful step. A worktree is not
|
||||
durable storage: an agent finished a whole ticket with every gate green and
|
||||
lost all of it, uncommitted, when its worktree was deleted during a disk
|
||||
cleanup. Tidying the history before the PR is fine; losing it is not.
|
||||
5. **Do not merge to `main`.** The orchestrator merges. Pushing the agent's own
|
||||
branch (`git push gh HEAD:refs/heads/<branch>`) is encouraged once it is
|
||||
worth keeping.
|
||||
6. The commit-message trailers, spelled out.
|
||||
7. **What to report**: branch and commits, what changed per ticket, *what could
|
||||
not be verified and why*, and proposed repo-map/`kb.pl` text. Ask explicitly
|
||||
for the thing the agent learned that the ticket did not predict — that is
|
||||
consistently the most valuable part of a hand-back, and agents omit it unless
|
||||
asked.
|
||||
|
||||
`bd` works in any worktree, but `.envrc` must be allowed there or you get a
|
||||
**version mismatch** rather than a clean failure — `direnv allow` in the
|
||||
worktree, or rely on the `[whitelist] prefix` in `~/.config/direnv/direnv.toml`.
|
||||
Note `direnv status` reports `Found RC allowed 0`, and `0` means **allowed**
|
||||
(the enum is 0=Allowed, 1=NotAllowed). It reads like the opposite.
|
||||
`.claude/skills/wave` is a symlink to `.agents/skills/wave`, so Claude and Codex
|
||||
read the same files. A new rule goes into one of those two files, not here and
|
||||
not into the next brief: a restated rule is the copy nobody corrects, and this
|
||||
list was once a third copy that drifted from the skill's.
|
||||
|
||||
### Between waves
|
||||
|
||||
|
||||
@@ -151,7 +151,75 @@ atomicity is preserved by keeping the existing whole-line `parse_line` as a
|
||||
still parses, and a line that did not still does not, so the only behavioural
|
||||
change is which cursor the commands see.
|
||||
|
||||
**That atomicity claim was false as written** (improvise-2bac), and §8 is what
|
||||
makes it true. The gate runs the pure arm; the loop runs the interactive arm. A
|
||||
gate is only a gate if *every argument list the pure arm accepts, the
|
||||
interactive arm accepts too* — and `commit-cell-edit` broke it:
|
||||
`add-category Foo . commit-cell-edit editing 5 Foo/x` on an empty document
|
||||
passed the gate, applied the first segment, and then refused the second with
|
||||
`no cell at cursor`. Half the line landed.
|
||||
|
||||
The TUI's `:` line had the same defect and is fixed the same way, with one
|
||||
limitation that is not new: `ExecuteCommand` returns one effect batch, so the
|
||||
segments of `:a . b` all see the pre-batch cursor, exactly as a
|
||||
`Binding::Sequence` does (gotchas 21 and 23).
|
||||
|
||||
## 8 — the interactive arm is `parse`, then context; it is never a second parser
|
||||
|
||||
#151 fixed three instances of *a registry with two construction arms demotes
|
||||
whichever caller reaches the weaker one*, one patch each. A post-merge review
|
||||
found the fourth (improvise-2bac): `commit-cell-edit <mode> <value> <coords>`,
|
||||
whose interactive arm read only `<mode>`, took the value from the `edit` buffer
|
||||
and the key from the cursor. So a script that wrote 5 to `Region/East`
|
||||
**deleted the cell under the cursor** — the empty buffer, committed — with exit
|
||||
0 and nothing on stderr. The fifth was already in the tree:
|
||||
`toggle-panel-and-focus <panel> false false` ignored both flags and toggled.
|
||||
|
||||
The shape rather than the instances is what changed. `CmdRegistry::register`
|
||||
used to take two free functions, `parse(args)` and `interactive(args, ctx)`,
|
||||
and nothing related them; each command re-derived its arguments twice and every
|
||||
disagreement was silent. It now takes three pieces, and the interactive arm is
|
||||
**built from `parse`**, not beside it:
|
||||
|
||||
- `parse(args) -> C` — the only function that reads arguments a caller supplied.
|
||||
- `contextualize(C, &CmdContext) -> C` — fills the fields that are context and
|
||||
never argument (a `Move`'s origin cursor, a `MovePanelCursor`'s bound). It is
|
||||
handed the built command and **not the args**, so it cannot discard them.
|
||||
- `fallback: Option<(args, ctx) -> C>` — reached **only** when `parse` refused.
|
||||
This is where "no coordinates means the cursor" lives: `clear-cell` bare, and
|
||||
`commit-cell-edit <mode>` bare, which is what the keymap binds.
|
||||
|
||||
Two consequences hold by construction rather than by test. **Whatever `parse`
|
||||
accepts, the interactive arm accepts** — `contextualize` is infallible — so the
|
||||
§7 gate cannot pass a segment the loop then refuses. And **an accepted argument
|
||||
list cannot be overridden by the cursor**, because the only function that sees
|
||||
both args and context never runs on an accepted list. `register_pure` and
|
||||
`register_nullary` already had one arm; they are the `contextualize = identity,
|
||||
fallback = None` case.
|
||||
|
||||
What is still representable, stated rather than hidden:
|
||||
|
||||
- `contextualize` can overwrite a field `parse` set. It would have to name the
|
||||
field, which is a visible line in review, where the old `|_args, ctx|` was an
|
||||
invisible omission. `commit_cell_edit_honours_an_explicit_value_and_cell_over_the_cursor`
|
||||
fails if `commit-cell-edit`'s does.
|
||||
- A `fallback` sees the args `parse` refused and could ignore some of them.
|
||||
`commit-cell-edit`'s refuses anything but a lone mode, so the malformed
|
||||
`commit-cell-edit editing 5` is an error on the `:` line — which has no pure
|
||||
gate — rather than a commit of the edit buffer to the cursor cell.
|
||||
- The gate is **stricter** than the loop wherever a fallback exists: a bare
|
||||
`clear-cell` is refused in a script. That is the safe direction — the whole
|
||||
line is refused — and it is unchanged from before #151.
|
||||
|
||||
`toggle-panel-and-focus` did not fit `contextualize`, because its pure arm
|
||||
defaulted the omitted flags to `true` and a contextualizer cannot tell an
|
||||
omitted flag from an explicit one. So the omission is now in the type —
|
||||
`target: Option<(bool, bool)>`, `None` meaning *toggle* — and the toggle is
|
||||
resolved in `execute` against the same context. That is the honest home: the
|
||||
decision depends on context, and `execute` is where context is.
|
||||
|
||||
Guards: `every_argument_list_the_gate_accepts_the_run_accepts_too` enumerates
|
||||
**every registration** against every candidate argument list, in a context whose
|
||||
cursor sits on no cell — the context in which the old `commit-cell-edit` arm
|
||||
refused. It lists no commands, so a new registration is covered the day it
|
||||
lands. It is backed by the process-level tests in `tests/headless_cli.rs`.
|
||||
|
||||
+30
-2
@@ -932,8 +932,23 @@ gotcha 47 asks for — closing e24v has to come to that line and delete it.
|
||||
broadcasts nothing, so it reaches this user and no one else. This argument
|
||||
arrived as `edit_or_drill`'s, i.e. about `i`/`a` on an aggregate, and for a
|
||||
while it read as doctrine for `>` while `>` was still silent. Both doors now
|
||||
refuse, and **they refuse with different sentences on purpose**: "this key is
|
||||
not built yet" and "this cell is not yours to write" are different facts.
|
||||
refuse, and **they name the same missing feature in the same words** —
|
||||
`not_yet("drill")` — because they reach the same one. In the TUI,
|
||||
`EditOrDrill` hands an aggregate straight to `DrillIntoCell`, so an aggregate
|
||||
*is* writable there, by drilling; `i` on it is drill by another door. `i`'s
|
||||
sentence adds only *why* it drilled rather than edited ("an aggregate is
|
||||
edited by drilling in — …"), and `>` still says nothing about aggregates,
|
||||
because it is refused whatever the cell is.
|
||||
|
||||
This **reverses** what this bullet said until improvise-s8h7: that the two
|
||||
refused with different sentences on purpose, "this key is not built yet"
|
||||
against "this cell is not yours to write". The second half was false about
|
||||
the application — nothing refuses a write to an aggregate; the TUI drills —
|
||||
and the separate wording hid from a browser user that `i` and `>` were one
|
||||
missing feature. The help page's `edit-or-drill` caption carried the same
|
||||
error, calling `i` only "Edit current cell"; it now says it drills into an
|
||||
aggregate, which is true in both front ends because the browser's refusal
|
||||
names drill. Delivering drill is `improvise-obtu`'s, not this bullet's.
|
||||
- **The commit's write is not spelled again here.** It is
|
||||
`Client::typed_cell`'s, the same one click-to-edit uses, so a browser keyboard
|
||||
commit and a browser mouse commit cannot disagree about what `""` or `"42"`
|
||||
@@ -1517,6 +1532,19 @@ what says it is closed**. That is the shape `col_groups` already uses for "this
|
||||
grid has no column groups" — an absent list rather than a flag the page has to
|
||||
interpret alongside a mode it would then have to understand.
|
||||
|
||||
**That shape is only sound where "open" implies "non-empty"**, and help is the
|
||||
one place it does — its content is static. `views` and `formulas` borrowed it
|
||||
and were wrong: a document with no formulas opened a formula panel that built
|
||||
the same `RenderModel` as a closed one, so the page hid it under a mode reading
|
||||
FORMULAS (improvise-df2w). Both are now `Option<Vec<_>>` — `None` closed,
|
||||
`Some(vec![])` open and empty — one field, so the two facts cannot disagree,
|
||||
where a `formula_panel_open` flag beside the rows (the `category_panel_open`
|
||||
shape) would have been two. The same mistake as improvise-xg1z: an absence
|
||||
spent on encoding a second fact. What the page does **not** yet draw is the
|
||||
TUI's empty-state line (`(no formulas — press 'n' to add)`); that string lives
|
||||
in `src/ui/formula_panel.rs`, and copying it into `render.rs` would be a second
|
||||
home for it.
|
||||
|
||||
The rows themselves are `improvise_protocol::help::pages()`, the same ones
|
||||
`src/ui/help.rs` draws. `RenderHelpRow` carries a `kind` string
|
||||
(`heading`, `keys`, `accent`, …) which is a *semantic* name the page maps to a
|
||||
|
||||
@@ -2489,12 +2489,13 @@ line separated by `.`, coordinate pairs written `Category/Item`, and quoted stri
|
||||
available for the rare command that needs to enumerate all views, such as the view
|
||||
panel.
|
||||
|
||||
`InteractiveFn` constructs a `Cmd` from the interactive context (keymap dispatch).
|
||||
It receives both the keymap args and the interactive context, so commands can
|
||||
combine text arguments (a panel name) with runtime state (whether that panel is
|
||||
currently open). `register_pure` registers a command that does not need interactive
|
||||
context: called interactively with args it delegates to `parse`, and without args it
|
||||
errors.
|
||||
The interactive arm (keymap dispatch, scripts, the `:` line) is built **from**
|
||||
`parse`: `register` takes `parse`, a `contextualize` that fills context-only
|
||||
fields of the command `parse` built without seeing the args, and an optional
|
||||
`fallback` reached only for argument lists `parse` refused. So an accepted
|
||||
argument list is never overridden by context (improvise-2bac; the argument is
|
||||
`context/code/cli.md` §8). `register_pure` is the case with nothing to
|
||||
contextualize and no fallback: both arms are `parse`.
|
||||
|
||||
`write_buffer` is the effect that writes a named buffer — the mirror of
|
||||
`read_buffer`, and the only place that knows "search" is not an entry in the buffers
|
||||
|
||||
@@ -552,8 +552,39 @@ client over the same grid, and they disagreed by a comma.
|
||||
|
||||
`numeric` and `error` are decided by the server for the same reason: it is
|
||||
the side that knows which it was, and re-deriving client-side is how the two
|
||||
ends drift apart. `CellDisplay` is deliberately not a `CellValue` — by the
|
||||
time a cell reaches a client the number formatting has already happened.
|
||||
ends drift apart. `CellDisplay` is deliberately not *drawn from* a `CellValue`
|
||||
— by the time a cell reaches a client the number formatting has already
|
||||
happened, and `text` is what a renderer paints.
|
||||
|
||||
### `CellDisplay::value` — what `yank` copies (improvise-dsxq)
|
||||
|
||||
The value the host evaluated at this position, the same `Option<CellValue>`
|
||||
that `text` was formatted from: `evaluate_aggregated` over the view's
|
||||
`none_cats` in a pivot grid, `records_value` in a records grid. It exists
|
||||
because the browser's `yy` has to copy what the TUI's `YankCell` copies, and
|
||||
neither string on the cell can be turned back into it:
|
||||
|
||||
- `edit_text` is `CellValue::to_string()`, which writes a non-whole number
|
||||
`{:.4}` — `1.23456789` becomes `1.2346`.
|
||||
- Re-parsing any text with `typed()` turns number-like text into a number —
|
||||
`Text("007")` becomes `Number(7.0)`.
|
||||
- In a pivot grid `edit_text` is the *stored* cell (`get_cell`), so an
|
||||
aggregate or a formula cell has none and yanked nothing at all.
|
||||
|
||||
So the value is **projected**, not re-derived (gotcha 47): a client that
|
||||
wants the value asks for the value. It is kept separate from `edit_text`
|
||||
rather than replacing it, because the two answer different questions — what
|
||||
an editor starts with is a *string*, deliberately the one the TUI's buffer
|
||||
holds, and changing it would change what the TUI's own editor shows.
|
||||
|
||||
It cost `Eq`: `CellValue` holds an `f64`, so `CellDisplay`, `GridCache` and
|
||||
`GridUpdate` are `PartialEq` only. Nothing hashed or `Eq`-bounded them.
|
||||
|
||||
In a records grid the TUI does **not** yank this value: `YankCell` and
|
||||
`PasteCell` resolve the synthetic `(_Index, _Dim)` key, so the TUI yanks
|
||||
`None` and pastes a junk cell into the document (improvise-6p8q). The browser
|
||||
is right there and the TUI is wrong, which is why the lockstep yank test stays
|
||||
on a pivot grid.
|
||||
|
||||
### `AxisFrame` — the grid's frame, and why it is *one* type
|
||||
|
||||
|
||||
@@ -229,12 +229,94 @@ the syntax gate and then cleared whatever the cursor was sitting on — the orig
|
||||
So improvise-6uva's own lesson had a **third** instance in its own fix: two
|
||||
construction arms that mean different things demote whichever caller reaches the
|
||||
weaker one, and here the "weaker" one was the arm that had the context. The four
|
||||
arms now go through `key_from_args_or_cursor`: explicit coordinates win, the
|
||||
arms went through `key_from_args_or_cursor`: explicit coordinates win, the
|
||||
cursor is the fallback, which is also what the keymap path wants since a binding
|
||||
carries no arguments.
|
||||
carries no arguments. That helper is gone since improvise-2bac: the cursor is
|
||||
now the registry's `fallback`, reached only when `parse` refused, so explicit
|
||||
coordinates win by construction (`context/code/cli.md` §8).
|
||||
|
||||
The fixture writes three cells and clears the one the cursor is **not** on, so
|
||||
the assertion separates "cleared the named cell" from "cleared the cursor cell"
|
||||
— clearing `North` by name would have passed against the broken code. Restoring
|
||||
the arg-discarding arm fails it: `the cursor cell was cleared instead of the
|
||||
named one: ["Region=East = 3", "Region=South = 2"]`.
|
||||
|
||||
### commit_cell_edit_writes_the_named_cell_and_spares_the_cursors
|
||||
|
||||
improvise-2bac, found by the post-merge adversarial review of PR #151: the
|
||||
fourth instance of that PR's own lesson. `commit-cell-edit` was registered with
|
||||
two free arms. The pure arm read `<mode> <value> <coords>`; the interactive arm
|
||||
read only `<mode>`, took the value from the `edit` buffer and the key from the
|
||||
cursor. Once #151 made scripts construct through the interactive arm,
|
||||
`commit-cell-edit editing 5 Region/East` passed the syntax gate and then
|
||||
committed an **empty** buffer to the cursor cell — deleting it — with exit 0
|
||||
and nothing on stderr.
|
||||
|
||||
Reproduced at `40b8913` before the fix, through the real binary: the document
|
||||
came back as `["Region=East = 3", "Region=North = 1"]` — `South`, the cell the
|
||||
cursor had been moved onto, was gone and `East` was unchanged.
|
||||
|
||||
The fixture writes three cells and **moves the cursor to `South`** in the same
|
||||
line before committing to `East`, so the cursor sits on neither the target nor
|
||||
the origin: a test with the cursor on the target is right by accident, and one
|
||||
that only asserts the target misses the destruction. It asserts all three.
|
||||
|
||||
Injected rivals, run: (a) `contextualize` for `CommitAndAdvance` overwriting
|
||||
`key` and `value` from the cursor and the buffer — fails this test and
|
||||
`a_line_whose_gate_passes_runs_whole`; (b) the registry dispatching to the
|
||||
fallback first whenever one exists (the pre-fix shape) — fails this test,
|
||||
`a_line_whose_gate_passes_runs_whole`, `an_explicit_coordinate_still_beats_the_cursor`
|
||||
and both registry unit tests below.
|
||||
|
||||
### a_line_whose_gate_passes_runs_whole
|
||||
|
||||
improvise-2bac's second symptom, reported on the ticket: `context/code/cli.md`
|
||||
§7 said the whole-line `parse_line` gate preserved line atomicity. It did not,
|
||||
wherever the interactive arm refused an argument list the pure arm accepted.
|
||||
`add-category Foo . commit-cell-edit editing 5 Foo/x` on an empty document
|
||||
printed `Parse error: commit-cell-edit: no cell at cursor` after `Foo` had
|
||||
already been added and saved: half the line landed.
|
||||
|
||||
The assertion is on `Parse error` and on the committed cell, **not** on the exit
|
||||
code. The line exits 1 both before #151 and after this fix, for a different and
|
||||
pre-existing reason: the grid has no rows, so `commit-cell-edit`'s advance
|
||||
emits `AbortChain`, and the CLI maps an aborted batch to exit 1
|
||||
(`context/code/cli.md` §3). Asserting success would have made this test fail on
|
||||
the correct behaviour.
|
||||
|
||||
### a_malformed_commit_line_is_refused_whole
|
||||
|
||||
The other half of atomicity, which must survive the fix: a segment the gate
|
||||
refuses (`commit-cell-edit editing 5` — a value and no coordinates) refuses the
|
||||
whole line, so the preceding `add-category Foo` never lands. It passed before
|
||||
the fix and must keep passing; its job is to catch a "fix" that loosens the
|
||||
pure arm to make the gate agree.
|
||||
|
||||
### commit_cell_edit_honours_an_explicit_value_and_cell_over_the_cursor
|
||||
|
||||
The unit-level form of the headless test, against `CmdRegistry::interactive`
|
||||
directly: cursor on `Type=Food, Month=Jan`, a decoy `edit` buffer, and an
|
||||
explicit `5` for `Type/Clothing Month/Feb`. It asserts the built command's key
|
||||
and value. Rival (a) above — a `contextualize` that overwrites the parsed
|
||||
fields — fails it; this is the one hole the new registry shape leaves
|
||||
representable, so it is the one that needs a test.
|
||||
|
||||
### every_argument_list_the_gate_accepts_the_run_accepts_too
|
||||
|
||||
Enumerates **every** registration (`CmdRegistry::names`, no hand-kept list)
|
||||
against every candidate argument list, and requires the interactive arm to
|
||||
accept whatever the pure arm accepts, in a context whose cursor sits on **no
|
||||
cell** — a category with no items. That context is what made the old
|
||||
`commit-cell-edit` arm refuse, so a context with a cell under the cursor would
|
||||
have passed the bug. The assertion that the cursor has no cell is checked
|
||||
first; a plain `Workbook::new` does not satisfy it, which is how the fixture
|
||||
came to add the empty category. Rival (b) fails it.
|
||||
|
||||
### explicit_open_and_focus_arguments_beat_the_toggle
|
||||
|
||||
The fifth instance, named on improvise-2bac as plausible and not yet run:
|
||||
`toggle-panel-and-focus <panel> false false` ignored both flags and toggled, so
|
||||
on a **closed** panel it opened it. The fixture closes every panel first,
|
||||
because on an open-and-focused panel the toggle also closes it and the test
|
||||
would pass against the bug. Rival: `execute` ignoring `target` and always
|
||||
toggling — fails this test for every panel.
|
||||
|
||||
@@ -1130,8 +1130,8 @@ Normal and never closes the panel, exactly as `src/draw.rs` keeps drawing it
|
||||
off `view_panel_open` alone. Asserting it hidden would have pinned the browser
|
||||
to behaviour the TUI does not have.
|
||||
|
||||
The gating rule itself — an empty list is what says a panel is closed, the same
|
||||
rule `help` follows — is asserted in Rust as well, by
|
||||
The gating rule itself — `None` is what says a panel is closed (it was an empty
|
||||
list, the rule `help` follows, until improvise-df2w) — is asserted in Rust as well, by
|
||||
`a_closed_panel_projects_no_rows_and_an_open_one_does` and
|
||||
`the_tile_bar_is_always_present_and_only_its_cursor_is_modal`, because a browser
|
||||
test cannot distinguish "closed" from "the cache is empty" and those two tests
|
||||
@@ -1389,28 +1389,74 @@ happened* passes against a page that has stopped listening, so it presses `i`
|
||||
afterwards and requires `#status` to change. Without that, this file would be
|
||||
folklore with a green tick.
|
||||
|
||||
### the_drill_key_and_an_aggregate_edit_refuse_for_different_reasons
|
||||
### an_aggregate_edit_names_the_same_missing_drill_as_the_drill_key
|
||||
|
||||
The Rust half of the interim, and the assertion that carries the whole weight.
|
||||
Two keys on the same aggregate cell must both refuse and must **not** refuse
|
||||
with the same sentence: `>` says drill is unavailable, `i` says drilling into an
|
||||
aggregate needs the host. Collapsing them into one message would be the
|
||||
tempting tidy-up and would lose the distinction between "this key does nothing
|
||||
here yet" and "this cell is not one you may write".
|
||||
improvise-s8h7. Until it, this test was
|
||||
`the_drill_key_and_an_aggregate_edit_refuse_for_different_reasons` and asserted
|
||||
the opposite: that `i` and `>` on an aggregate must refuse with *different*
|
||||
sentences, "this key does nothing here yet" against "this cell is not one you
|
||||
may write". The second half was false about the application. `EditOrDrill`
|
||||
hands an aggregated cell to `DrillIntoCell`, so in the TUI `i` on an aggregate
|
||||
**is** drill — nothing refuses the write, it drills — and the browser's
|
||||
separate wording ("drilling into an aggregate needs the host") hid that `i` and
|
||||
`>` are one missing feature. The reasoning is `context/code/clients.md`.
|
||||
|
||||
**The injection, and why it is not the obvious one.** Replacing `>`'s sentence
|
||||
with the *aggregate* sentence looks like it should fail on the first assertion —
|
||||
and does not: that sentence contains both "drill" and "browser", so it clears
|
||||
the substring check. What fires is the anti-aggregate assertion lower down. The
|
||||
claim that the second assertion is the load-bearing one was doubted on review,
|
||||
tested, and confirmed; record it here because the first assertion reads like the
|
||||
discriminating one and is not.
|
||||
So the test now asserts four things on one aggregate fixture: `>` names drill
|
||||
and the browser; `>` does **not** mention aggregates (it is refused whatever the
|
||||
cell is); `i`'s sentence **contains** `>`'s sentence verbatim; and `i`'s
|
||||
sentence mentions "aggregate", so it still says why it drilled rather than
|
||||
edited. The fixture is `client_on(CellId::Aggregate(..))` rather than the old
|
||||
empty `in_mode` state, because `i` needs a cell under the cursor to reach the
|
||||
`Aggregate` arm at all — on an empty grid it answers nothing and only `>` was
|
||||
ever exercised.
|
||||
|
||||
The harness sibling (`drill.spec.js`) asserts the same distinction in a browser,
|
||||
but its `not.toContainText("aggregate")` is **decoration under the only
|
||||
injection that exists** — Playwright throws at the positive assertion above it
|
||||
first, so that line is never reached. The Rust assertion is the guard; the
|
||||
harness one is end-to-end confirmation that the string survives the wire.
|
||||
**Three injections, each failing at a different assertion.** Run, not reasoned:
|
||||
|
||||
- `i` says only `not_yet("drill")` — the "why" dropped. Fails at the last
|
||||
assertion, "and `i` must still say why". This is the tempting over-tidy fix.
|
||||
- `>`'s `declined` subject becomes "drilling into an aggregate". Fails at the
|
||||
anti-aggregate assertion on `>`. Note that `i` still contains `>`'s *old*
|
||||
sentence here, because `edit_or_drill` calls `not_yet("drill")` itself rather
|
||||
than going through `declined` — so the containment assertion alone would not
|
||||
have caught a `>` that started blaming the cell.
|
||||
- `i` paraphrases ("… drill needs the host"). Fails at the containment
|
||||
assertion: the same feature named in different words is exactly what the
|
||||
ticket reported.
|
||||
|
||||
The harness sibling (`drill.spec.js`) asserts the same wording in a browser.
|
||||
Its `not.toContainText("aggregate")` on `>` is still **decoration under the
|
||||
only injection that exists** — Playwright throws at the positive assertion
|
||||
above it first — so the Rust assertion is the guard and the harness is
|
||||
end-to-end confirmation that the string survives the wire.
|
||||
|
||||
### the_edit_key_row_says_it_drills_into_an_aggregate
|
||||
|
||||
improvise-s8h7, the help-page half. The `edit-or-drill` row on the Editing page
|
||||
read "Edit current cell", while `i`/`a` on an aggregate drill in the TUI and
|
||||
name drill in the browser. The row is found by its **keys** — the labels
|
||||
`keys_for(Normal, "edit-or-drill", ["editing"])` returns — not by its caption,
|
||||
and exactly one row must carry them, so a caption fix that also moved or
|
||||
duplicated the row fails on the `let [..]` pattern rather than passing on
|
||||
whichever copy was found first. Failed at HEAD on the caption; the injection
|
||||
"Edit current cell, or drill" (drill named, aggregate not) fails it too, since
|
||||
`>` is also drill and the row must say *when* `i` drills.
|
||||
|
||||
### only_the_edit_key_is_captioned_as_editing_the_cell
|
||||
|
||||
Found while fixing improvise-s8h7, not predicted by it. The row below the edit
|
||||
key read `Enter Edit current cell (same as i)`, but Normal-mode Enter is bound
|
||||
to `enter-advance` alone — not a `Binding::Sequence` — and `EnterAdvance` only
|
||||
moves the cursor down, wrapping to the next column. The test takes every help
|
||||
row captioned "Edit current cell" and requires its keys to be a subset of
|
||||
`edit-or-drill`'s; at HEAD it failed naming `["Enter"]`. The fix moved the row
|
||||
to the Navigation page's "Cursor movement" section with a caption that says
|
||||
what it does.
|
||||
|
||||
Its reach is stated rather than implied: it checks the one phrase that was
|
||||
false. A caption that misdescribes Enter in *other* words passes it: a caption
|
||||
is prose, and nothing checks it against the command it documents. That is a
|
||||
neighbour of improvise-wk18, not the same gap — wk18 is about `literal()` rows
|
||||
whose *keys* go unchecked; here the keys were right and the words were wrong.
|
||||
|
||||
### a_view_row_click_focuses_moves_and_switches_to_the_row_clicked
|
||||
|
||||
@@ -4041,17 +4087,18 @@ Demonstrated at `e7de19d`: the first test below reports `Some("")`.
|
||||
|
||||
### `an_editing_mode_with_no_edit_buffer_shows_the_cell_rather_than_a_blank`
|
||||
|
||||
The fallback is `improvise_protocol::edit_seed`, which is the **existing** home
|
||||
of "what an editor opened here starts with" — `seed_at` at the cursor, the
|
||||
function `enter-advance` already seeds from. A second reconciliation of the
|
||||
scroll offset inside `render.rs` would have been gotcha 42's own trap, so the
|
||||
one-line fallback cost one `pub` rather than a copy.
|
||||
**Superseded in part by improvise-ktlk** (entry at the end of this file). As
|
||||
first landed, the fallback lived in `render.rs`: `editing` fell back to
|
||||
`edit_seed` while `commit` and `append-char` still read the absent buffer as
|
||||
`""`, so the overlay showed `1200` and Enter cleared it. The fallback now lives
|
||||
in the fold (`ClientState::settle_editor`), which seeds the buffer itself, and
|
||||
`render.rs` reads the same buffer `commit` does. The test's first assertion
|
||||
changed with it: it used to check that the buffer was still absent after the
|
||||
fold, and now checks that the fold seeded it.
|
||||
|
||||
The test asserts three things in order, and the middle one is why it is not
|
||||
vacuous: that the buffer really is absent, that the cell really holds `1200`,
|
||||
and only then what `editing` says. Without the first two a fixture that happened
|
||||
to carry an `edit` buffer, or a grid whose cell was blank anyway, would pass
|
||||
against the bug.
|
||||
The test asserts three things in order: what the fold put in the buffer, that
|
||||
the cell really holds `1200`, and only then what `editing` says. Without the
|
||||
second, a grid whose cell was blank anyway would pass against the bug.
|
||||
|
||||
**Fault injected:** `unwrap_or_default()`, the original expression. Fails with
|
||||
`Some("")`.
|
||||
@@ -4101,6 +4148,12 @@ so the paste is evidence about the yank rather than about a hand-seeded field.
|
||||
|
||||
**Fault injected:** the `"yank"` arm of `composed` renamed. Fails on the yank.
|
||||
|
||||
**Superseded in part by improvise-dsxq** (entry at the end of this file). The
|
||||
fixture's `1200` is the one kind of value on which the edit text and the value
|
||||
agree, so this test could not see that the arm yanked the edit text re-parsed.
|
||||
The arm now yanks `CellDisplay::value`, and the test sets that field on its
|
||||
hand-built cell. The parity claim lives in the lockstep tests, not here.
|
||||
|
||||
### `a_second_key_that_matches_nothing_still_spends_the_prefix`
|
||||
|
||||
`App::dispatch_key` `take()`s the prefix before looking the second key up, so
|
||||
@@ -4214,3 +4267,126 @@ Fault injected: `bindings.rs` reverted to `Worker::new(app)` for the read-only
|
||||
boot, then `harness/build.sh` rebuilt. The spec then failed on the toast, which
|
||||
received `""` because no refusal arrived. Reverted and rebuilt, and the whole
|
||||
of `shell.spec.js` passed (18).
|
||||
|
||||
### `an_open_empty_panel_is_not_a_closed_one`
|
||||
|
||||
improvise-df2w. `views` and `formulas` said "closed" by being empty, so an open
|
||||
formula panel over a document with no formulas built a `RenderModel` identical
|
||||
to a closed one, and the page hid it while the mode read FORMULAS. The test
|
||||
compares the two models whole rather than asserting a field, so it does not
|
||||
depend on how the difference is spelled.
|
||||
|
||||
**Reproduced:** fails at HEAD before the fix with `left == right` on the formula
|
||||
half — the two models were byte-identical. The existing
|
||||
`a_closed_panel_projects_no_rows_and_an_open_one_does` could not see it, because
|
||||
it fills both caches before opening the panels.
|
||||
|
||||
### the formula panel opens over a document with no formulas
|
||||
|
||||
The browser half of improvise-df2w, in `harness/tests/contract.spec.js`, over
|
||||
`harness/fixtures/tall.improv`, which has no `## Formulas` section. It asserts
|
||||
**both** directions, because each rival fails a different one.
|
||||
|
||||
**Reproduced:** against a bundle built from the unfixed `render.rs` and
|
||||
`app.js`, fails at `#formulas` `toBeVisible()`, immediately after `#mode` reads
|
||||
FORMULAS — the exact disagreement the ticket describes.
|
||||
|
||||
**Fault injected:** `paintPanel` writing `host.hidden = false` (a page that
|
||||
stops inferring closed and simply never hides). Fails at the launch-time
|
||||
`toBeHidden()` before the panel is opened.
|
||||
|
||||
## improvise-ktlk — the overlay and the commit read one buffer
|
||||
|
||||
PR #162 (improvise-xg1z) changed what the overlay **draws** for an `Editing`
|
||||
mode that arrived without its `edit` buffer, and not what Enter **commits**.
|
||||
`render.rs` painted `edit_seed(state)`, while `resolve.rs::commit` and
|
||||
`append-char` read `read_buffer(&state.view, "edit")`, which is `""` for an
|
||||
absent key. So the overlay showed the value, Enter composed `ClearCell`, and one
|
||||
typed character replaced the shown text rather than extending it. #162's tests
|
||||
only looked at the drawing, and none of them pressed a key.
|
||||
|
||||
The fix gives the fact one home. The fold seeds the missing buffer
|
||||
(`ClientState::settle_editor`, called after every fold, bootstrap and refusal,
|
||||
beside `settle_mode`), and `render.rs` reads the buffer through `read_buffer`,
|
||||
as `commit` does. `edit_seed` is `pub(crate)` again and no renderer calls it.
|
||||
Seeding happens only when there is a cell under the cursor that has
|
||||
`edit_text`. With no such cell the buffer stays absent, which reads as `""`
|
||||
on both paths, and `commit` declines anyway for want of a cell.
|
||||
|
||||
### `an_editor_entered_without_its_buffer_commits_and_extends_the_cell`
|
||||
|
||||
The fixture's display text (`1,200.50`) and edit text (`1200.5`) **differ**.
|
||||
With `CellDisplay::number("1200")`, as #162 used, a fallback that seeds from the
|
||||
formatted `text` passes too (gotcha 8). The test folds `ChangeMode(editing)`
|
||||
with no `SetBuffer`, then presses the keys: Enter must compose
|
||||
`SetCell(Number(1200.5))`, and after a typed `5`, Enter must compose
|
||||
`SetCell(Number(1200.55))`.
|
||||
|
||||
**Faults injected, each run:**
|
||||
- The tree before this fix (render-only fallback). The buffer assertion fails
|
||||
with `""`. With that assertion removed, the Enter assertion fails on its own
|
||||
with `None`, because the first command is `ClearCell`.
|
||||
- `edit_seed` returning the drawn `text` instead of `edit_text`. Fails with
|
||||
`"1,200.50"`.
|
||||
- The over-broad seed, "absent **or empty**, refill". This test passes, and
|
||||
`an_edit_buffer_the_user_has_emptied_is_not_refilled_from_the_cell` fails,
|
||||
as it did against the same rival in `render.rs`.
|
||||
|
||||
## improvise-dsxq — the browser's `yy` copies the value, not a string
|
||||
|
||||
PR #161 (improvise-lut7) made the browser's `yank` arm compose
|
||||
`SetYanked(typed(edit_text))`. The TUI's `YankCell` yanks
|
||||
`evaluate_aggregated(key)`, the value itself. Those differ in three ways, and
|
||||
#161's only fixture, `1200`, hits none of them:
|
||||
|
||||
- `edit_text` is `CellValue::to_string()`, which rounds a non-whole number to
|
||||
four places.
|
||||
- `typed()` turns number-like text into a number.
|
||||
- In a pivot grid `edit_text` is the stored cell only, so an aggregate or a
|
||||
formula cell has none and the browser yanked `None`.
|
||||
|
||||
The fix projects the evaluated value as `CellDisplay::value`, and the arm yanks
|
||||
that (`context/code/protocol.md` §3). Reproduced at `40b8913` by both tests
|
||||
below before the fix: the browser yanked `Number(1.2346)` for `1.23456789`, and
|
||||
`None` for the aggregate.
|
||||
|
||||
### `yy_then_p_copies_the_value_the_host_copies`
|
||||
|
||||
Three cases, one per disagreement, each on a real `App` and a `ClientState`
|
||||
seeded by `grid_of`. The host presses `y y p` through `reduce_key`; a twin
|
||||
host applies exactly what the client composed. Each case asserts the host
|
||||
yanked the expected value (so a fixture that yanks nothing cannot pass), that
|
||||
the client yanked the same, that the host's paste landed, and that the two
|
||||
documents match. Sources sit at row 2 or 3 and targets at row 1, never row 0,
|
||||
and the first column is an unused `Units` measure, so no cursor is at index 0.
|
||||
|
||||
- `1.23456789`, a non-whole number.
|
||||
- `Text("007")`, number-like text.
|
||||
- `Profit = Revenue - Cost` over `10.123456` and `0.5`, a formula's value,
|
||||
pasted into a stored `Revenue` cell.
|
||||
|
||||
**Faults injected, each run:**
|
||||
- The pre-fix arm, `typed(edit_text)`. Fails on the first case with
|
||||
`Number(1.2346)`.
|
||||
- The arm yanking `typed(&value.to_string())`, the projected value re-rendered.
|
||||
Fails on the first case with `Number(1.2346)`.
|
||||
- The producer coercing number-like `Text` through `typed()`. The first case
|
||||
passes, and the second fails with `Number(7.0)`.
|
||||
- The producer projecting the raw `get_cell` instead of the evaluated value.
|
||||
The first two cases pass, and the third fails with `None`.
|
||||
|
||||
### `yy_over_an_aggregate_copies_the_sum_the_host_copies`
|
||||
|
||||
`Product` on the None axis with two stored records, `1.23456789` and `1.0`, so
|
||||
the cell at row 1 is an `Aggregate` with no stored value. Yank only, since an
|
||||
all-aggregate grid has no record cell to paste into. The host must yank
|
||||
`Number(2.23456789)`, which rules out a fixture that is not an aggregate.
|
||||
|
||||
**Faults injected, each run:** the pre-fix arm (fails with `None`), the
|
||||
re-rendered value (fails with `Number(2.2346)`), and the raw `get_cell`
|
||||
producer (fails with `None`).
|
||||
|
||||
Records mode is deliberately not covered. There the TUI yanks `None` and its
|
||||
`p` writes a junk `_Dim`/`_Index` cell, because it resolves the synthetic key
|
||||
(improvise-6p8q). The browser is right and the TUI is wrong, so a lockstep
|
||||
case there would assert the bug.
|
||||
|
||||
@@ -2382,6 +2382,31 @@ keystroke to land in.
|
||||
Injected fault: dropping `self.view_state = snapshot.view_state` from
|
||||
`App::restore` fails this test and only this test.
|
||||
|
||||
**Addendum (improvise-gho5).** Improvise-gho5 found that this
|
||||
test and `a_rolled_back_load_tells_nobody_and_puts_every_mirror_back` passed
|
||||
against a host that **never ran the load**. The review's fault, reproduced
|
||||
here, was placed at the top of `Host::commands`: refuse any multi-command frame
|
||||
that contains a `LoadModel` with `Rejected(DocumentReplaced)`, before
|
||||
`CheckedBatch` runs. Both tests stayed green. A frame that never got that far
|
||||
leaves the generation, the view, the status line and every mirror untouched,
|
||||
which is exactly what the tests asserted.
|
||||
|
||||
Both tests now assert that the asker's whole reply is
|
||||
`the_load_ran_then_the_write_broke()`, which is `Rejected(ChainBroken(North))`.
|
||||
They also assert, before the frame, that `North` resolves. Together these make
|
||||
the rejection a witness that the load ran. `ChainBroken` comes only from
|
||||
`reduce::broken_chain`, which runs inside `mutating_frame` just before each
|
||||
mutation. It names `North` only once the document no longer holds that
|
||||
record, and only an applied `LoadModel` in this frame can have removed it.
|
||||
|
||||
With the same fault re-injected, both tests fail. The left side is
|
||||
`[Rejected(DocumentReplaced)]` and the right is
|
||||
`[Rejected(ChainBroken(<North>))]`. The fault was reverted.
|
||||
|
||||
The residual: a rogue up-front check that forged `ChainBroken` specifically
|
||||
would still pass. That is the same forgery a hand-rolled client could commit
|
||||
against any basis check, and a test cannot guard against it.
|
||||
|
||||
### a_refused_mutation_rolls_back_its_predecessor_and_the_abort_flag
|
||||
|
||||
The `Rejection::MutationRefused` arm — the one the first hand-back wrongly called
|
||||
@@ -4885,9 +4910,14 @@ Three things make it non-vacuous:
|
||||
`LoadModel` silently failed, `Applied::Rebased` would never run and every
|
||||
assertion here would pass over nothing. The control asserts the generation
|
||||
moves and that *both* clients are owed a `Snapshot`.
|
||||
- **The rollback is asserted before anything is concluded from it** — the
|
||||
`document_generation` equality is checked first, so "the frame was rejected"
|
||||
cannot be confused with "the frame never got that far".
|
||||
- **The load is witnessed to have RUN before anything is concluded from the
|
||||
rollback** (improvise-gho5). The asker's whole reply must be
|
||||
`Rejected(ChainBroken(North))`. `North` is asserted to resolve before the
|
||||
frame, so only `broken_chain` can give that answer, and only after the
|
||||
`LoadModel` has replaced the document. `CheckedBatch` never produces
|
||||
`ChainBroken`. This bullet used to say the `document_generation` equality
|
||||
did the job, and that was false: generation equality is exactly what holds
|
||||
when the frame never got that far.
|
||||
- **A bystander is seated where a wrong relay could move it.** A single-client
|
||||
fixture cannot tell "the mirrors were restored" from "there was one mirror and
|
||||
it happened to be the asker's".
|
||||
|
||||
@@ -391,6 +391,8 @@ green suite over a vacuous assertion.
|
||||
|
||||
**What is not covered.** The script asks `cargo metadata` for members, so a root
|
||||
whose metadata succeeds but whose member list is wrong is not detectable here.
|
||||
*Narrowed by improvise-90ea, below:* a list that **omits** a package in the tree
|
||||
is now refused; one that names a package it should not is still not detected.
|
||||
And `harness/opfs-contract`'s own `cargo fmt -p opfs-contract` step in the
|
||||
`harness-lint` job is left as it is: it resolves correctly because that step
|
||||
sets `working-directory`, and the ticket's own note confirms CI as written was
|
||||
@@ -415,9 +417,11 @@ getting wrong. The line the split actually needs is **"did the gate run"**:
|
||||
- exit 1 — rustfmt did not certify a root clean. A diff or a refusal to parse;
|
||||
either way **a file is named** and the tree is what is wrong.
|
||||
- exit 2 — the gate did not run. Usage, no cargo, no rustfmt, `cargo metadata`
|
||||
failed, no `[workspace]` root. **No file is named.** Every such cause is
|
||||
checked *above* the loop, which is what makes the narrowing true rather than
|
||||
hopeful.
|
||||
failed, no `[workspace]` root. **No source file is named.** Every such cause
|
||||
is checked *above* the loop, which is what makes the narrowing true rather
|
||||
than hopeful. (Since improvise-90ea, a package no root reaches is also exit 2,
|
||||
and it names that package's `Cargo.toml`, which is the reason no verdict
|
||||
exists. The line is "no verdict", not "no output"; improvise-ximf.)
|
||||
|
||||
Both directions are now pinned, and two further rivals were injected:
|
||||
|
||||
@@ -439,6 +443,67 @@ stub must fail in the **shape** of the real fault, not merely fail — a stub th
|
||||
breaks more than the real thing breaks satisfies the test through the wrong arm,
|
||||
and reads exactly like coverage.
|
||||
|
||||
#### The root package, and why the fourth fix is structural — `scripts/fmt-workspaces-test.sh`
|
||||
|
||||
A post-merge review of PR #159 found a **fourth** fixture vacuity
|
||||
(improvise-90ea). The fixture's outer root is a package (`root-pkg`) as well as
|
||||
a workspace, which is right, but no case ever dirtied **the root package's own
|
||||
`src/lib.rs`**. Only the member `a/` and `nested/` were dirtied. So a script
|
||||
that formats every member *except* the root package passed 17/17. In this repo
|
||||
the root package is `improvise`, the whole TUI. Reproduced at `40b8913` with
|
||||
the review's rival in `members_of` (drop the package whose manifest sits at
|
||||
`workspace_root`): the suite passed 17/17 and `--list` left `improvise` out.
|
||||
|
||||
Adding one more hand-picked case would have fixed the fourth and not the fifth.
|
||||
Four vacuities in one checker all have the same cause: **the cases were chosen
|
||||
by hand, one per hazard somebody had thought of.** So the fix is structural,
|
||||
and it has two halves because it has to cover two sets of packages:
|
||||
|
||||
- **Every package the fixture has.** The two hand-picked fault cases are
|
||||
replaced by a loop over every `[package]` manifest in the fixture. Each one
|
||||
gets its own misformatted `src/lib.rs`, must exit 1, and must be named
|
||||
**exactly once**, so the `--all` boundary check now covers every package
|
||||
rather than one. The enumeration is a `grep` for `[package]`, deliberately
|
||||
*not* the script's `[workspace]` + `cargo metadata` discovery, since that
|
||||
discovery is what is under test. A count assertion (three packages) stops
|
||||
the loop going vacuous if the fixture loses a package. A package added to
|
||||
the fixture gets its case automatically.
|
||||
- **Every package the tree has.** The loop proves only that the script handles
|
||||
the fixture's *shapes*. A real tree can hold a package no root lists (an
|
||||
`exclude`d crate, say), and nothing in the fixture would say so. So the gate
|
||||
itself now checks an invariant **before it formats anything**: every
|
||||
`[package]` manifest the find reaches must be a member of some root, or it
|
||||
exits **2** and names the package. That is exit 2 and not 1 because no
|
||||
verdict was produced for that package, which is what exit 2 means here.
|
||||
Paths are compared through `realpath`, because on macOS `mktemp` lives under
|
||||
`/var` while cargo reports `/private/var`.
|
||||
|
||||
**Rivals injected, and where each failed** (the suite went from 17 to 21 cases):
|
||||
|
||||
- **The review's skip-the-root rival.** It now fails the fixture at `a fault in
|
||||
src/lib.rs is reported` (exit 0). With the invariant in place it fails
|
||||
**six** cases (exit 2, "no workspace root reaches"). It also fails the
|
||||
**real repo** with no fixture involved: `fmt-workspaces.sh` exits 2 naming
|
||||
`Cargo.toml`. That last one is the property worth having. The gate refuses
|
||||
its own blindness in CI, not only in its test suite.
|
||||
- **Disabling the invariant** (`if false`). It fails `a package no workspace
|
||||
root reaches is exit 2, not a pass` with **exit 0**, even though that
|
||||
package's `src/lib.rs` is misformatted. That exit 0 *is* the hazard: an
|
||||
unreached package is silently clean.
|
||||
- **The two earlier rivals, re-run against the loop.** `--all` for `-p` fails
|
||||
both `a/src/lib.rs` and `src/lib.rs` "named exactly once". The second of
|
||||
those was not predicted: the nested root's `--all` also reaches the root
|
||||
package. The bare manifest-path form fails `a fault in a/src/lib.rs is
|
||||
reported`.
|
||||
|
||||
The real-repo control from the ticket still holds: a misformatted fn appended
|
||||
to `src/ui/text.rs` is reported (`Diff in …/src/ui/text.rs:72`).
|
||||
|
||||
**What is still not covered.** A root whose member list *includes* something it
|
||||
should not is not detected, and a package in a directory the find excludes
|
||||
(`target/`, `.claude/`, `node_modules/`) is outside the invariant by
|
||||
construction.
|
||||
|
||||
### The kb tool's completeness guard and atomic write — `scripts/kb-test.sh` cases 18–22
|
||||
|
||||
improvise-usus: `scripts/kb add` wrote onto a working-tree `kb.pl` that had lost
|
||||
|
||||
@@ -139,9 +139,12 @@ pub trait Cmd: Debug + Send + Sync {
|
||||
/// Factory that constructs a Cmd from text arguments (headless/script).
|
||||
pub type ParseFn = fn(&[String]) -> Result<Box<dyn Cmd>, String>;
|
||||
|
||||
/// Factory constructing a Cmd from the interactive context. It takes both the keymap
|
||||
/// args and the context, so a command can combine text args with runtime state.
|
||||
pub type InteractiveFn = fn(&[String], &CmdContext) -> Result<Box<dyn Cmd>, String>;
|
||||
/// Fills the context-only fields of a command `parse` already built. It never
|
||||
/// sees the args, so it cannot discard them — `context/code/cli.md` §8.
|
||||
pub type ContextualizeFn<C> = fn(C, &CmdContext) -> C;
|
||||
|
||||
/// Builds a command from context, reached ONLY for args `parse` refused.
|
||||
pub type FallbackFn<C> = fn(&[String], &CmdContext) -> Result<C, String>;
|
||||
|
||||
type BoxParseFn = Box<dyn Fn(&[String]) -> Result<Box<dyn Cmd>, String>>;
|
||||
type BoxInteractiveFn = Box<dyn Fn(&[String], &CmdContext) -> Result<Box<dyn Cmd>, String>>;
|
||||
@@ -183,13 +186,26 @@ impl CmdRegistry {
|
||||
name
|
||||
}
|
||||
|
||||
/// Register a command with both a text parser and an interactive constructor.
|
||||
/// The name is derived from a prototype command instance.
|
||||
pub fn register(&mut self, prototype: &dyn Cmd, parse: ParseFn, interactive: InteractiveFn) {
|
||||
/// Register a command whose interactive arm is `parse` then `contextualize`, so
|
||||
/// args `parse` accepts are honoured whole — `context/code/cli.md` §8.
|
||||
pub fn register<C: Cmd + 'static>(
|
||||
&mut self,
|
||||
prototype: &C,
|
||||
parse: fn(&[String]) -> Result<C, String>,
|
||||
contextualize: ContextualizeFn<C>,
|
||||
fallback: Option<FallbackFn<C>>,
|
||||
) {
|
||||
self.entries.push(CmdEntry {
|
||||
name: prototype.name(),
|
||||
parse: Box::new(parse),
|
||||
interactive: Box::new(interactive),
|
||||
parse: Box::new(move |args| parse(args).map(|c| Box::new(c) as Box<dyn Cmd>)),
|
||||
interactive: Box::new(move |args, ctx| {
|
||||
let built = match (parse(args), fallback) {
|
||||
(Ok(cmd), _) => Ok(contextualize(cmd, ctx)),
|
||||
(Err(_), Some(fallback)) => fallback(args, ctx),
|
||||
(Err(err), None) => Err(err),
|
||||
};
|
||||
built.map(|c| Box::new(c) as Box<dyn Cmd>)
|
||||
}),
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -20,8 +20,7 @@ mod tests {
|
||||
let ctx = make_ctx(&m, &layout, ®);
|
||||
let cmd = TogglePanelAndFocus {
|
||||
panel: effect::Panel::Formula,
|
||||
open: true,
|
||||
focused: true,
|
||||
target: Some((true, true)),
|
||||
};
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 2);
|
||||
@@ -40,8 +39,7 @@ mod tests {
|
||||
let ctx = make_ctx(&m, &layout, ®);
|
||||
let cmd = TogglePanelAndFocus {
|
||||
panel: effect::Panel::Formula,
|
||||
open: false,
|
||||
focused: false,
|
||||
target: Some((false, false)),
|
||||
};
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 2);
|
||||
@@ -531,20 +529,24 @@ mod tests {
|
||||
#[derive(Debug)]
|
||||
pub struct TogglePanelAndFocus {
|
||||
pub panel: Panel,
|
||||
pub open: bool,
|
||||
pub focused: bool,
|
||||
/// Explicit `(open, focused)`; `None` toggles: close if open and focused, else open and focus.
|
||||
pub target: Option<(bool, bool)>,
|
||||
}
|
||||
impl Cmd for TogglePanelAndFocus {
|
||||
fn name(&self) -> &'static str {
|
||||
"toggle-panel-and-focus"
|
||||
}
|
||||
fn execute(&self, _ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
let (open, focused) = self.target.unwrap_or_else(|| {
|
||||
let shown = ctx.panel_open(self.panel) && ctx.mode.panel() == Some(self.panel);
|
||||
(!shown, !shown)
|
||||
});
|
||||
let mut effects: Vec<Box<dyn Effect>> = Vec::new();
|
||||
effects.push(Box::new(effect::SetPanelOpen {
|
||||
panel: self.panel,
|
||||
open: self.open,
|
||||
open,
|
||||
}));
|
||||
if self.focused {
|
||||
if focused {
|
||||
effects.push(effect::change_mode(self.panel.mode()));
|
||||
} else {
|
||||
effects.push(effect::change_mode(AppMode::Normal));
|
||||
|
||||
@@ -22,15 +22,50 @@ use super::text_buffer::*;
|
||||
use super::tile::*;
|
||||
use super::wizard::*;
|
||||
|
||||
/// Explicit coordinates win; the cursor is the fallback — `context/code/cli.md`.
|
||||
fn key_from_args_or_cursor(args: &[String], ctx: &CmdContext) -> Result<CellKey, String> {
|
||||
if args.is_empty() {
|
||||
ctx.cell_key().clone().ok_or("no cell at cursor".into())
|
||||
} else {
|
||||
Ok(parse_cell_key_from_args(args))
|
||||
/// The cell under the cursor: what a coordinate command falls back to when given none.
|
||||
fn cursor_key(ctx: &CmdContext) -> Result<CellKey, String> {
|
||||
ctx.cell_key().clone().ok_or("no cell at cursor".into())
|
||||
}
|
||||
|
||||
/// A `Move` built from args takes the live cursor as its origin.
|
||||
fn move_from_cursor(cmd: Move, ctx: &CmdContext) -> Move {
|
||||
Move {
|
||||
cursor: CursorState::from_ctx(ctx),
|
||||
..cmd
|
||||
}
|
||||
}
|
||||
|
||||
/// A commit built from args still advances from the live cursor.
|
||||
fn commit_from_cursor(cmd: CommitAndAdvance, ctx: &CmdContext) -> CommitAndAdvance {
|
||||
CommitAndAdvance {
|
||||
cursor: CursorState::from_ctx(ctx),
|
||||
..cmd
|
||||
}
|
||||
}
|
||||
|
||||
/// `<mode>` alone: commit the edit buffer to the cell under the cursor.
|
||||
fn commit_at_cursor(
|
||||
args: &[String],
|
||||
ctx: &CmdContext,
|
||||
advance: AdvanceDir,
|
||||
) -> Result<CommitAndAdvance, String> {
|
||||
let [mode] = args else {
|
||||
return Err("takes a mode alone, or a mode, a value and coordinates".into());
|
||||
};
|
||||
Ok(CommitAndAdvance {
|
||||
edit_mode: parse_mode_name(mode)?,
|
||||
value: read_buffer(ctx, "edit"),
|
||||
key: cursor_key(ctx)?,
|
||||
advance,
|
||||
cursor: CursorState::from_ctx(ctx),
|
||||
})
|
||||
}
|
||||
|
||||
/// For a command with no context-only fields: `parse`'s result is used as built.
|
||||
fn as_parsed<C>(cmd: C, _ctx: &CmdContext) -> C {
|
||||
cmd
|
||||
}
|
||||
|
||||
/// Build the default command registry with all commands.
|
||||
/// Registry names MUST match the `Cmd::name()` return value.
|
||||
pub fn default_registry() -> CmdRegistry {
|
||||
@@ -51,14 +86,16 @@ pub fn default_registry() -> CmdRegistry {
|
||||
if args.is_empty() {
|
||||
return Err("clear-cell requires at least one Cat/Item coordinate".into());
|
||||
}
|
||||
Ok(Box::new(ClearCellCommand {
|
||||
Ok(ClearCellCommand {
|
||||
key: parse_cell_key_from_args(args),
|
||||
}))
|
||||
},
|
||||
|args, ctx| {
|
||||
let key = key_from_args_or_cursor(args, ctx)?;
|
||||
Ok(Box::new(ClearCellCommand { key }))
|
||||
})
|
||||
},
|
||||
as_parsed,
|
||||
Some(|_, ctx| {
|
||||
Ok(ClearCellCommand {
|
||||
key: cursor_key(ctx)?,
|
||||
})
|
||||
}),
|
||||
);
|
||||
r.register_pure(&AddFormulaCmd(vec![]), AddFormulaCmd::parse);
|
||||
r.register_pure(&FitMeasure::prototype(), FitMeasure::parse);
|
||||
@@ -80,16 +117,8 @@ pub fn default_registry() -> CmdRegistry {
|
||||
r.register_pure(&ExportCmd(vec![]), ExportCmd::parse);
|
||||
r.register_pure(&WriteCmd(vec![]), WriteCmd::parse);
|
||||
r.register_pure(&HelpCmd(vec![]), HelpCmd::parse);
|
||||
r.register(
|
||||
&HelpPageNextCmd(vec![]),
|
||||
HelpPageNextCmd::parse,
|
||||
|_args, _ctx| Ok(Box::new(HelpPageNextCmd(vec![]))),
|
||||
);
|
||||
r.register(
|
||||
&HelpPagePrevCmd(vec![]),
|
||||
HelpPagePrevCmd::parse,
|
||||
|_args, _ctx| Ok(Box::new(HelpPagePrevCmd(vec![]))),
|
||||
);
|
||||
r.register_pure(&HelpPageNextCmd(vec![]), HelpPageNextCmd::parse);
|
||||
r.register_pure(&HelpPagePrevCmd(vec![]), HelpPagePrevCmd::parse);
|
||||
|
||||
// ── Navigation (unified Move) ──────────────────────────────────────
|
||||
r.register(
|
||||
@@ -102,22 +131,14 @@ pub fn default_registry() -> CmdRegistry {
|
||||
require_args(args, 2)?;
|
||||
let dr = args[0].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
let dc = args[1].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
Ok(Box::new(Move {
|
||||
Ok(Move {
|
||||
kind: MoveKind::Relative(dr, dc),
|
||||
cursor: CursorState::default(),
|
||||
cmd_name: "move-selection",
|
||||
}))
|
||||
},
|
||||
|args, ctx| {
|
||||
require_args(args, 2)?;
|
||||
let dr = args[0].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
let dc = args[1].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
Ok(Box::new(Move {
|
||||
kind: MoveKind::Relative(dr, dc),
|
||||
cursor: CursorState::from_ctx(ctx),
|
||||
cmd_name: "move-selection",
|
||||
}))
|
||||
})
|
||||
},
|
||||
move_from_cursor,
|
||||
None,
|
||||
);
|
||||
// Jump-to-edge commands: first/last row/col
|
||||
macro_rules! reg_jump {
|
||||
@@ -133,7 +154,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
cmd_name: $name,
|
||||
},
|
||||
|_| {
|
||||
Ok(Box::new(Move {
|
||||
Ok(Move {
|
||||
kind: if $to_end {
|
||||
MoveKind::ToEnd($is_row)
|
||||
} else {
|
||||
@@ -141,19 +162,10 @@ pub fn default_registry() -> CmdRegistry {
|
||||
},
|
||||
cursor: CursorState::default(),
|
||||
cmd_name: $name,
|
||||
}))
|
||||
},
|
||||
|_, ctx| {
|
||||
Ok(Box::new(Move {
|
||||
kind: if $to_end {
|
||||
MoveKind::ToEnd($is_row)
|
||||
} else {
|
||||
MoveKind::ToStart($is_row)
|
||||
},
|
||||
cursor: CursorState::from_ctx(ctx),
|
||||
cmd_name: $name,
|
||||
}))
|
||||
})
|
||||
},
|
||||
move_from_cursor,
|
||||
None,
|
||||
);
|
||||
};
|
||||
}
|
||||
@@ -170,21 +182,14 @@ pub fn default_registry() -> CmdRegistry {
|
||||
|args| {
|
||||
require_args(args, 1)?;
|
||||
let n = args[0].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
Ok(Box::new(Move {
|
||||
Ok(Move {
|
||||
kind: MoveKind::Relative(n, 0),
|
||||
cursor: CursorState::default(),
|
||||
cmd_name: "scroll-rows",
|
||||
}))
|
||||
},
|
||||
|args, ctx| {
|
||||
require_args(args, 1)?;
|
||||
let n = args[0].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
Ok(Box::new(Move {
|
||||
kind: MoveKind::Relative(n, 0),
|
||||
cursor: CursorState::from_ctx(ctx),
|
||||
cmd_name: "scroll-rows",
|
||||
}))
|
||||
})
|
||||
},
|
||||
move_from_cursor,
|
||||
None,
|
||||
);
|
||||
r.register(
|
||||
&Move {
|
||||
@@ -195,28 +200,21 @@ pub fn default_registry() -> CmdRegistry {
|
||||
|args| {
|
||||
require_args(args, 1)?;
|
||||
let dir = args[0].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
Ok(Box::new(Move {
|
||||
Ok(Move {
|
||||
kind: MoveKind::Page(dir),
|
||||
cursor: CursorState::default(),
|
||||
cmd_name: "page-scroll",
|
||||
}))
|
||||
},
|
||||
|args, ctx| {
|
||||
require_args(args, 1)?;
|
||||
let dir = args[0].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
Ok(Box::new(Move {
|
||||
kind: MoveKind::Page(dir),
|
||||
cursor: CursorState::from_ctx(ctx),
|
||||
cmd_name: "page-scroll",
|
||||
}))
|
||||
})
|
||||
},
|
||||
move_from_cursor,
|
||||
None,
|
||||
);
|
||||
r.register(
|
||||
&EnterAdvance {
|
||||
cursor: CursorState::default(),
|
||||
},
|
||||
|_| {
|
||||
Ok(Box::new(EnterAdvance {
|
||||
Ok(EnterAdvance {
|
||||
cursor: CursorState {
|
||||
row: 0,
|
||||
col: 0,
|
||||
@@ -227,13 +225,12 @@ pub fn default_registry() -> CmdRegistry {
|
||||
visible_rows: 20,
|
||||
visible_cols: 8,
|
||||
},
|
||||
}))
|
||||
})
|
||||
},
|
||||
|_, ctx| {
|
||||
Ok(Box::new(EnterAdvance {
|
||||
cursor: CursorState::from_ctx(ctx),
|
||||
}))
|
||||
|_, ctx| EnterAdvance {
|
||||
cursor: CursorState::from_ctx(ctx),
|
||||
},
|
||||
None,
|
||||
);
|
||||
|
||||
// ── Cell operations ──────────────────────────────────────────────────
|
||||
@@ -245,14 +242,16 @@ pub fn default_registry() -> CmdRegistry {
|
||||
if args.is_empty() {
|
||||
return Err("yank requires at least one Cat/Item coordinate".into());
|
||||
}
|
||||
Ok(Box::new(YankCell {
|
||||
Ok(YankCell {
|
||||
key: parse_cell_key_from_args(args),
|
||||
}))
|
||||
},
|
||||
|args, ctx| {
|
||||
let key = key_from_args_or_cursor(args, ctx)?;
|
||||
Ok(Box::new(YankCell { key }))
|
||||
})
|
||||
},
|
||||
as_parsed,
|
||||
Some(|_, ctx| {
|
||||
Ok(YankCell {
|
||||
key: cursor_key(ctx)?,
|
||||
})
|
||||
}),
|
||||
);
|
||||
r.register(
|
||||
&PasteCell {
|
||||
@@ -262,14 +261,16 @@ pub fn default_registry() -> CmdRegistry {
|
||||
if args.is_empty() {
|
||||
return Err("paste requires at least one Cat/Item coordinate".into());
|
||||
}
|
||||
Ok(Box::new(PasteCell {
|
||||
Ok(PasteCell {
|
||||
key: parse_cell_key_from_args(args),
|
||||
}))
|
||||
},
|
||||
|args, ctx| {
|
||||
let key = key_from_args_or_cursor(args, ctx)?;
|
||||
Ok(Box::new(PasteCell { key }))
|
||||
})
|
||||
},
|
||||
as_parsed,
|
||||
Some(|_, ctx| {
|
||||
Ok(PasteCell {
|
||||
key: cursor_key(ctx)?,
|
||||
})
|
||||
}),
|
||||
);
|
||||
// clear-cell is registered above (unified: ctx.cell_key() or explicit coords)
|
||||
|
||||
@@ -313,21 +314,19 @@ pub fn default_registry() -> CmdRegistry {
|
||||
if args.is_empty() {
|
||||
return Err("drill-into-cell requires Cat/Item coordinates".into());
|
||||
}
|
||||
Ok(Box::new(DrillIntoCell {
|
||||
Ok(DrillIntoCell {
|
||||
key: parse_cell_key_from_args(args),
|
||||
}))
|
||||
},
|
||||
|args, ctx| {
|
||||
let key = key_from_args_or_cursor(args, ctx)?;
|
||||
Ok(Box::new(DrillIntoCell { key }))
|
||||
})
|
||||
},
|
||||
as_parsed,
|
||||
Some(|_, ctx| {
|
||||
Ok(DrillIntoCell {
|
||||
key: cursor_key(ctx)?,
|
||||
})
|
||||
}),
|
||||
);
|
||||
r.register_nullary(|| Box::new(ViewNavigate { forward: false }));
|
||||
r.register(
|
||||
&ViewNavigate { forward: true },
|
||||
|_| Ok(Box::new(ViewNavigate { forward: true })),
|
||||
|_, _| Ok(Box::new(ViewNavigate { forward: true })),
|
||||
);
|
||||
r.register_nullary(|| Box::new(ViewNavigate { forward: true }));
|
||||
r.register_pure(&EnterMode(AppMode::Normal), |args| {
|
||||
require_args(args, 1)?;
|
||||
Ok(Box::new(EnterMode(parse_mode_name(&args[0])?)))
|
||||
@@ -345,38 +344,20 @@ pub fn default_registry() -> CmdRegistry {
|
||||
r.register(
|
||||
&TogglePanelAndFocus {
|
||||
panel: Panel::Formula,
|
||||
open: true,
|
||||
focused: true,
|
||||
target: None,
|
||||
},
|
||||
|args| {
|
||||
// Parse: toggle-panel-and-focus <panel> [open] [focused]
|
||||
require_args(args, 1)?;
|
||||
let panel = parse_panel(&args[0])?;
|
||||
let open = args.get(1).map(|s| s == "true").unwrap_or(true);
|
||||
let focused = args.get(2).map(|s| s == "true").unwrap_or(open);
|
||||
Ok(Box::new(TogglePanelAndFocus {
|
||||
panel,
|
||||
open,
|
||||
focused,
|
||||
}))
|
||||
},
|
||||
|args, ctx| {
|
||||
require_args(args, 1)?;
|
||||
let panel = parse_panel(&args[0])?;
|
||||
// Default interactive: if already open+focused -> close, else open+focus
|
||||
let currently_open = ctx.panel_open(panel);
|
||||
let currently_focused = ctx.mode.panel() == Some(panel);
|
||||
let (open, focused) = if currently_open && currently_focused {
|
||||
(false, false) // close
|
||||
} else {
|
||||
(true, true) // open + focus
|
||||
};
|
||||
Ok(Box::new(TogglePanelAndFocus {
|
||||
panel,
|
||||
open,
|
||||
focused,
|
||||
}))
|
||||
let target = args.get(1).map(|s| {
|
||||
let open = s == "true";
|
||||
(open, args.get(2).map(|s| s == "true").unwrap_or(open))
|
||||
});
|
||||
Ok(TogglePanelAndFocus { panel, target })
|
||||
},
|
||||
as_parsed,
|
||||
None,
|
||||
);
|
||||
r.register(
|
||||
&TogglePanelVisibility {
|
||||
@@ -386,20 +367,16 @@ pub fn default_registry() -> CmdRegistry {
|
||||
|args| {
|
||||
require_args(args, 1)?;
|
||||
let panel = parse_panel(&args[0])?;
|
||||
Ok(Box::new(TogglePanelVisibility {
|
||||
Ok(TogglePanelVisibility {
|
||||
panel,
|
||||
currently_open: false,
|
||||
}))
|
||||
})
|
||||
},
|
||||
|args, ctx| {
|
||||
require_args(args, 1)?;
|
||||
let panel = parse_panel(&args[0])?;
|
||||
let currently_open = ctx.panel_open(panel);
|
||||
Ok(Box::new(TogglePanelVisibility {
|
||||
panel,
|
||||
currently_open,
|
||||
}))
|
||||
|cmd, ctx| TogglePanelVisibility {
|
||||
currently_open: ctx.panel_open(cmd.panel),
|
||||
..cmd
|
||||
},
|
||||
None,
|
||||
);
|
||||
r.register(
|
||||
&CyclePanelFocus {
|
||||
@@ -408,19 +385,18 @@ pub fn default_registry() -> CmdRegistry {
|
||||
view_open: false,
|
||||
},
|
||||
|_| {
|
||||
Ok(Box::new(CyclePanelFocus {
|
||||
Ok(CyclePanelFocus {
|
||||
formula_open: false,
|
||||
category_open: false,
|
||||
view_open: false,
|
||||
}))
|
||||
})
|
||||
},
|
||||
|_, ctx| {
|
||||
Ok(Box::new(CyclePanelFocus {
|
||||
formula_open: ctx.formula_panel_open,
|
||||
category_open: ctx.category_panel_open,
|
||||
view_open: ctx.view_panel_open,
|
||||
}))
|
||||
|_, ctx| CyclePanelFocus {
|
||||
formula_open: ctx.formula_panel_open,
|
||||
category_open: ctx.category_panel_open,
|
||||
view_open: ctx.view_panel_open,
|
||||
},
|
||||
None,
|
||||
);
|
||||
r.register(
|
||||
&MovePanelCursor {
|
||||
@@ -433,29 +409,26 @@ pub fn default_registry() -> CmdRegistry {
|
||||
require_args(args, 2)?;
|
||||
let panel = parse_panel(&args[0])?;
|
||||
let delta = args[1].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
Ok(Box::new(MovePanelCursor {
|
||||
Ok(MovePanelCursor {
|
||||
panel,
|
||||
delta,
|
||||
current: 0,
|
||||
max: 0,
|
||||
}))
|
||||
})
|
||||
},
|
||||
|args, ctx| {
|
||||
require_args(args, 2)?;
|
||||
let panel = parse_panel(&args[0])?;
|
||||
let delta = args[1].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
let (current, max) = match panel {
|
||||
|cmd, ctx| {
|
||||
let (current, max) = match cmd.panel {
|
||||
Panel::Formula => (ctx.formula_cursor, ctx.model.formulas().len()),
|
||||
Panel::Category => (ctx.cat_panel_cursor, ctx.cat_tree_len()),
|
||||
Panel::View => (ctx.view_panel_cursor, ctx.workbook.views.len()),
|
||||
};
|
||||
Ok(Box::new(MovePanelCursor {
|
||||
panel,
|
||||
delta,
|
||||
MovePanelCursor {
|
||||
current,
|
||||
max,
|
||||
}))
|
||||
..cmd
|
||||
}
|
||||
},
|
||||
None,
|
||||
);
|
||||
r.register_nullary(|| Box::new(DeleteFormulaAtCursor));
|
||||
r.register_nullary(|| Box::new(AddRecordRow));
|
||||
@@ -490,11 +463,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
|
||||
// ── Grid operations ──────────────────────────────────────────────────
|
||||
r.register_nullary(|| Box::new(ToggleGroupAtCursor { is_row: true }));
|
||||
r.register(
|
||||
&ToggleGroupAtCursor { is_row: false },
|
||||
|_| Ok(Box::new(ToggleGroupAtCursor { is_row: false })),
|
||||
|_, _| Ok(Box::new(ToggleGroupAtCursor { is_row: false })),
|
||||
);
|
||||
r.register_nullary(|| Box::new(ToggleGroupAtCursor { is_row: false }));
|
||||
r.register_nullary(|| Box::new(HideSelectedRowItem));
|
||||
|
||||
// ── Text buffer ──────────────────────────────────────────────────────
|
||||
@@ -537,27 +506,16 @@ pub fn default_registry() -> CmdRegistry {
|
||||
return Err("commit-cell-edit requires a mode, value, and coords".into());
|
||||
}
|
||||
let edit_mode = parse_mode_name(&args[0])?;
|
||||
Ok(Box::new(CommitAndAdvance {
|
||||
Ok(CommitAndAdvance {
|
||||
key: parse_cell_key_from_args(&args[2..]),
|
||||
value: args[1].clone(),
|
||||
advance: AdvanceDir::Down,
|
||||
cursor: CursorState::default(),
|
||||
edit_mode,
|
||||
}))
|
||||
},
|
||||
|args, ctx| {
|
||||
require_args(args, 1)?;
|
||||
let edit_mode = parse_mode_name(&args[0])?;
|
||||
let value = read_buffer(ctx, "edit");
|
||||
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
|
||||
Ok(Box::new(CommitAndAdvance {
|
||||
key,
|
||||
value,
|
||||
advance: AdvanceDir::Down,
|
||||
cursor: CursorState::from_ctx(ctx),
|
||||
edit_mode,
|
||||
}))
|
||||
})
|
||||
},
|
||||
commit_from_cursor,
|
||||
Some(|args, ctx| commit_at_cursor(args, ctx, AdvanceDir::Down)),
|
||||
);
|
||||
r.register(
|
||||
&CommitAndAdvance {
|
||||
@@ -568,19 +526,8 @@ pub fn default_registry() -> CmdRegistry {
|
||||
edit_mode: AppMode::editing(),
|
||||
},
|
||||
|_| Err("commit-and-advance-right requires context".into()),
|
||||
|args, ctx| {
|
||||
require_args(args, 1)?;
|
||||
let edit_mode = parse_mode_name(&args[0])?;
|
||||
let value = read_buffer(ctx, "edit");
|
||||
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
|
||||
Ok(Box::new(CommitAndAdvance {
|
||||
key,
|
||||
value,
|
||||
advance: AdvanceDir::Right,
|
||||
cursor: CursorState::from_ctx(ctx),
|
||||
edit_mode,
|
||||
}))
|
||||
},
|
||||
commit_from_cursor,
|
||||
Some(|args, ctx| commit_at_cursor(args, ctx, AdvanceDir::Right)),
|
||||
);
|
||||
r.register(
|
||||
&CommitBuffer {
|
||||
@@ -589,13 +536,14 @@ pub fn default_registry() -> CmdRegistry {
|
||||
|args| {
|
||||
require_args(args, 1)?;
|
||||
let kind = parse_commit_kind(&args[0], &AppMode::Normal)?;
|
||||
Ok(Box::new(CommitBuffer { kind }))
|
||||
Ok(CommitBuffer { kind })
|
||||
},
|
||||
|args, ctx| {
|
||||
as_parsed,
|
||||
Some(|args, ctx| {
|
||||
require_args(args, 1)?;
|
||||
let kind = parse_commit_kind(&args[0], ctx.mode)?;
|
||||
Ok(Box::new(CommitBuffer { kind }))
|
||||
},
|
||||
Ok(CommitBuffer { kind })
|
||||
}),
|
||||
);
|
||||
r.register_nullary(|| Box::new(CommitExport));
|
||||
r.register_nullary(|| Box::new(CommitSave));
|
||||
@@ -641,6 +589,9 @@ mod tests {
|
||||
vec!["1", "1"],
|
||||
vec!["formula", "1"],
|
||||
vec!["editing", "5", "Region", "East"],
|
||||
vec!["editing", "5", "Region/East"],
|
||||
vec!["formula", "false", "false"],
|
||||
vec!["Region/East"],
|
||||
]
|
||||
.into_iter()
|
||||
.map(|v| v.into_iter().map(String::from).collect())
|
||||
@@ -802,6 +753,67 @@ mod tests {
|
||||
"only {checked} ex-command examples were found; the scan has stopped seeing them"
|
||||
);
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/cli.md`.
|
||||
#[test]
|
||||
fn commit_cell_edit_honours_an_explicit_value_and_cell_over_the_cursor() {
|
||||
let mut app = crate::ui::app::App::new(two_cat_model(), None);
|
||||
app.view_state.buffers.insert("edit".into(), "decoy".into());
|
||||
let ctx = app.cmd_context(
|
||||
crate::command::key::KeyCode::Null,
|
||||
crate::command::key::KeyModifiers::NONE,
|
||||
);
|
||||
let target = CellKey::new(vec![
|
||||
("Month".into(), "Feb".into()),
|
||||
("Type".into(), "Clothing".into()),
|
||||
]);
|
||||
assert_ne!(
|
||||
ctx.cell_key().as_ref(),
|
||||
Some(&target),
|
||||
"the cursor is elsewhere"
|
||||
);
|
||||
let args: Vec<String> = ["editing", "5", "Type/Clothing", "Month/Feb"]
|
||||
.map(String::from)
|
||||
.into();
|
||||
let dbg = format!(
|
||||
"{:?}",
|
||||
ctx.registry
|
||||
.interactive("commit-cell-edit", &args, &ctx)
|
||||
.expect("commit-cell-edit resolves")
|
||||
);
|
||||
assert!(
|
||||
dbg.contains(&format!("key: {target:?}")),
|
||||
"wrong cell: {dbg}"
|
||||
);
|
||||
assert!(dbg.contains("value: \"5\""), "wrong value: {dbg}");
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/cli.md`.
|
||||
#[test]
|
||||
fn every_argument_list_the_gate_accepts_the_run_accepts_too() {
|
||||
let r = default_registry();
|
||||
let mut wb = crate::workbook::Workbook::new("Empty");
|
||||
wb.add_category("Foo").expect("a category with no items");
|
||||
let app = crate::ui::app::App::new(wb, None);
|
||||
let ctx = app.cmd_context(
|
||||
crate::command::key::KeyCode::Null,
|
||||
crate::command::key::KeyModifiers::NONE,
|
||||
);
|
||||
assert!(ctx.cell_key().is_none(), "the cursor must sit on no cell");
|
||||
let mut accepted = 0;
|
||||
for name in r.names().collect::<Vec<_>>() {
|
||||
for args in candidate_args() {
|
||||
if r.parse(name, &args).is_err() {
|
||||
continue;
|
||||
}
|
||||
accepted += 1;
|
||||
if let Err(e) = r.interactive(name, &args, &ctx) {
|
||||
panic!("the gate accepts `{name} {args:?}` but the run refuses it: {e}");
|
||||
}
|
||||
}
|
||||
}
|
||||
assert!(accepted > 50, "only {accepted} accepted pairs were tried");
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -880,6 +892,7 @@ mod panel_open_tests {
|
||||
ctx.registry
|
||||
.interactive("toggle-panel-and-focus", &args, &ctx)
|
||||
.expect("toggle-panel-and-focus resolves")
|
||||
.execute(&ctx)
|
||||
);
|
||||
assert!(
|
||||
dbg.contains("open: false"),
|
||||
@@ -897,6 +910,7 @@ mod panel_open_tests {
|
||||
ctx.registry
|
||||
.interactive("toggle-panel-and-focus", &args, &ctx)
|
||||
.expect("toggle-panel-and-focus resolves")
|
||||
.execute(&ctx)
|
||||
);
|
||||
assert!(
|
||||
dbg.contains("open: true"),
|
||||
@@ -905,4 +919,29 @@ mod panel_open_tests {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/cli.md`.
|
||||
#[test]
|
||||
fn explicit_open_and_focus_arguments_beat_the_toggle() {
|
||||
for panel in Panel::ALL {
|
||||
let mut app = App::new(two_cat_model(), None);
|
||||
app.view_state.formula_panel_open = false;
|
||||
app.view_state.category_panel_open = false;
|
||||
app.view_state.view_panel_open = false;
|
||||
let ctx = app.cmd_context(KeyCode::Null, KeyModifiers::NONE);
|
||||
let args = [panel.name().to_string(), "false".into(), "false".into()];
|
||||
let dbg = format!(
|
||||
"{:?}",
|
||||
ctx.registry
|
||||
.interactive("toggle-panel-and-focus", &args, &ctx)
|
||||
.expect("toggle-panel-and-focus resolves")
|
||||
.execute(&ctx)
|
||||
);
|
||||
assert!(
|
||||
dbg.contains("open: false") && !dbg.contains("open: true"),
|
||||
"`{} false false` on a closed panel toggled it open: {dbg}",
|
||||
panel.name()
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1049,6 +1049,12 @@ mod tests {
|
||||
let was_view = host.app().view_state.active_view.clone();
|
||||
let was_status = host.app().view_state.status_msg.clone();
|
||||
let basis = frame_basis(&host, &["North"]);
|
||||
assert!(
|
||||
host.app()
|
||||
.resolve_record(addressed("North").record)
|
||||
.is_some(),
|
||||
"North must resolve BEFORE the load, or a refusal naming it proves nothing"
|
||||
);
|
||||
|
||||
// The load lands, then the write names a record the incoming document
|
||||
// has never had — so the frame fails at its second command.
|
||||
@@ -1069,11 +1075,10 @@ mod tests {
|
||||
]),
|
||||
);
|
||||
|
||||
assert!(
|
||||
answer
|
||||
.iter()
|
||||
.any(|a| matches!(a.msg, ServerFrame::Rejected(_))),
|
||||
"the fixture must actually refuse the frame: {answer:?}"
|
||||
assert_eq!(
|
||||
to(&answer, asker),
|
||||
vec![the_load_ran_then_the_write_broke()],
|
||||
"only a load that RAN can refuse the write this way: {answer:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
host.app().view_state.active_view,
|
||||
@@ -1097,6 +1102,12 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// `North` resolves in the outgoing document, so only `broken_chain` AFTER the
|
||||
/// load applied can answer this. Evidence: `context/test-evidence/command.md`.
|
||||
fn the_load_ran_then_the_write_broke() -> ServerFrame {
|
||||
ServerFrame::Rejected(Rejection::ChainBroken(addressed("North").record))
|
||||
}
|
||||
|
||||
/// Everything one client was told, flattened to the projections it carries.
|
||||
fn projections(frames: &[Addressed], id: SessionId) -> Vec<Projection> {
|
||||
to(frames, id)
|
||||
@@ -1147,6 +1158,12 @@ mod tests {
|
||||
let (bystander, _) = host.connect(10, 10);
|
||||
let was_generation = host.app().document_generation();
|
||||
let basis = frame_basis(&host, &["North"]);
|
||||
assert!(
|
||||
host.app()
|
||||
.resolve_record(addressed("North").record)
|
||||
.is_some(),
|
||||
"North must resolve BEFORE the load, or a refusal naming it proves nothing"
|
||||
);
|
||||
|
||||
// The load lands and rebases every mirror; the write then names a
|
||||
// record the incoming document has never had, so the frame fails.
|
||||
@@ -1167,6 +1184,11 @@ mod tests {
|
||||
]),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
to(&answer, asker),
|
||||
vec![the_load_ran_then_the_write_broke()],
|
||||
"only a load that RAN can refuse the write this way: {answer:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
host.app().document_generation(),
|
||||
was_generation,
|
||||
|
||||
@@ -282,6 +282,7 @@ fn cell_display(
|
||||
numeric: matches!(value, Some(CellValue::Number(_))),
|
||||
error: matches!(value, Some(CellValue::Error(_))),
|
||||
id: cell_id(app, layout, row, col),
|
||||
value: value.clone(),
|
||||
};
|
||||
}
|
||||
let key = layout.cell_key(row, col);
|
||||
@@ -293,6 +294,7 @@ fn cell_display(
|
||||
// The *unformatted* value the TUI seeds its edit buffer from. Projecting it
|
||||
// beats a client un-formatting `text` (gotcha 47).
|
||||
edit_text: key.map(|key| app.display_value_in(layout, &key)),
|
||||
value: value.clone(),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -74,6 +74,7 @@ impl ClientState {
|
||||
self.apply_projection(projection);
|
||||
}
|
||||
self.settle_mode();
|
||||
self.settle_editor();
|
||||
self.revision += 1;
|
||||
}
|
||||
|
||||
@@ -85,6 +86,7 @@ impl ClientState {
|
||||
other => reduce_view(&mut self.view, other),
|
||||
}
|
||||
self.settle_mode();
|
||||
self.settle_editor();
|
||||
self.revision += 1;
|
||||
}
|
||||
|
||||
@@ -124,6 +126,7 @@ impl ClientState {
|
||||
}
|
||||
self.last_rejection = Some(rejection);
|
||||
self.settle_mode();
|
||||
self.settle_editor();
|
||||
self.revision += 1;
|
||||
}
|
||||
|
||||
@@ -158,6 +161,21 @@ impl ClientState {
|
||||
self.view.mode = settled_mode(&self.view.mode, self.grid.records);
|
||||
}
|
||||
|
||||
/// An editor that arrived without its buffer is seeded here, so what is
|
||||
/// drawn and what Enter commits read one buffer (improvise-ktlk).
|
||||
fn settle_editor(&mut self) {
|
||||
let absent = self
|
||||
.view
|
||||
.mode
|
||||
.minibuffer()
|
||||
.filter(|_| self.view.mode.is_editing())
|
||||
.map(|mb| mb.buffer_key.clone())
|
||||
.filter(|key| !self.view.buffers.contains_key(key));
|
||||
if let (Some(key), Some(seed)) = (absent, crate::resolve::edit_seed(self)) {
|
||||
self.view.buffers.insert(key, seed);
|
||||
}
|
||||
}
|
||||
|
||||
/// The widget dispatch: a projection names the widget it is addressed to.
|
||||
/// The `CategoryTreeCache`/`TileBarCache` overlap is deliberate (gotcha 32).
|
||||
fn apply_projection(&mut self, projection: &Projection) {
|
||||
|
||||
@@ -439,6 +439,12 @@ fn navigation() -> Vec<HelpLine> {
|
||||
Some(HelpLine::Heading("Cursor movement")),
|
||||
Some(HelpLine::Blank),
|
||||
keys(n, "move-selection", None, "Move one cell"),
|
||||
keys(
|
||||
n,
|
||||
"enter-advance",
|
||||
None,
|
||||
"Move down, wrapping to the next column",
|
||||
),
|
||||
Some(literal("gg", "Jump to first row")),
|
||||
keys(n, "jump-last-row", None, "Jump to last row"),
|
||||
keys(n, "jump-first-col", None, "Jump to first column"),
|
||||
@@ -513,8 +519,12 @@ fn editing() -> Vec<HelpLine> {
|
||||
Some(HelpLine::Blank),
|
||||
Some(HelpLine::Heading("Entering edit mode")),
|
||||
Some(HelpLine::Blank),
|
||||
keys(n, "edit-or-drill", Some(&["editing"]), "Edit current cell"),
|
||||
keys(n, "enter-advance", None, "Edit current cell (same as i)"),
|
||||
keys(
|
||||
n,
|
||||
"edit-or-drill",
|
||||
Some(&["editing"]),
|
||||
"Edit current cell (drill into an aggregate)",
|
||||
),
|
||||
keys(
|
||||
e,
|
||||
"clear-buffer",
|
||||
@@ -950,6 +960,58 @@ mod tests {
|
||||
"an undescribed name falls back to itself rather than to silence"
|
||||
);
|
||||
}
|
||||
|
||||
fn rows() -> Vec<(Vec<String>, &'static str)> {
|
||||
pages()
|
||||
.into_iter()
|
||||
.flat_map(|page| page.lines)
|
||||
.filter_map(|line| match line {
|
||||
HelpLine::Keys { keys, description } => Some((keys, description)),
|
||||
_ => None,
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn labels_of(command: &str, args: Option<&[String]>) -> Vec<String> {
|
||||
keys_for(&ModeKey::Normal, command, args)
|
||||
.iter()
|
||||
.map(|(c, m)| key_label(c, *m))
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// improvise-s8h7: `i` on an aggregate drills in the TUI, so the row
|
||||
/// that documents it must say so. See `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn the_edit_key_row_says_it_drills_into_an_aggregate() {
|
||||
let edit = labels_of("edit-or-drill", Some(&["editing".to_string()]));
|
||||
let found: Vec<_> = rows().into_iter().filter(|(k, _)| *k == edit).collect();
|
||||
let [(_, description)] = &found[..] else {
|
||||
panic!("expected one help row keyed {edit:?}, got {found:?}");
|
||||
};
|
||||
assert!(
|
||||
description.contains("drill") && description.contains("aggregate"),
|
||||
"{edit:?} drills into an aggregate and the row must say so: {description}"
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-s8h7: Normal's Enter is `enter-advance`, which only moves.
|
||||
/// See `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn only_the_edit_key_is_captioned_as_editing_the_cell() {
|
||||
let edit = labels_of("edit-or-drill", Some(&["editing".to_string()]));
|
||||
let claims: Vec<_> = rows()
|
||||
.into_iter()
|
||||
.filter(|(_, d)| d.contains("Edit current cell"))
|
||||
.collect();
|
||||
assert!(!claims.is_empty(), "the check needs a row to check");
|
||||
for (keys, description) in claims {
|
||||
assert!(
|
||||
keys.iter().all(|k| edit.contains(k)),
|
||||
"{keys:?} is captioned {description:?} but only {edit:?} edit"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Not an assertion: a printout for a human to read once. `cargo test -p
|
||||
/// improvise-protocol eyeball -- --nocapture --ignored`.
|
||||
#[test]
|
||||
|
||||
@@ -34,7 +34,7 @@ pub use render::{
|
||||
WizardProposal, WizardStepKind, header_level_widths,
|
||||
};
|
||||
pub use resolve::{
|
||||
Click, KeyOutcome, Request, click, edit_seed, read_buffer, resolve, typed, typed_cell_command,
|
||||
Click, KeyOutcome, Request, click, read_buffer, resolve, typed, typed_cell_command,
|
||||
};
|
||||
pub use viewmodel::{CatTreeEntry, flatten_cat_tree, view_axis_summary};
|
||||
// `improvise-core`'s, because the index that resolves one lives in
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
use serde::{Deserialize, Serialize};
|
||||
use unicode_width::UnicodeWidthStr;
|
||||
|
||||
use improvise_core::model::cell::CellValue;
|
||||
use improvise_core::model::record::{AggregateHash, RecordHash};
|
||||
use improvise_core::view::Axis;
|
||||
use improvise_core::view::layout::PageAxis;
|
||||
@@ -64,9 +65,9 @@ pub enum CreateKind {
|
||||
Category,
|
||||
}
|
||||
|
||||
/// One cell as the client draws it. Deliberately not a `CellValue` — the
|
||||
/// One cell as the client draws it. Drawn from `text`, never from `value` — the
|
||||
/// formatting has already happened, and re-deriving it is how ends drift.
|
||||
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)]
|
||||
pub struct CellDisplay {
|
||||
pub text: String,
|
||||
/// Right-align numerics, left-align text — decided by the server, which
|
||||
@@ -80,6 +81,9 @@ pub struct CellDisplay {
|
||||
/// What an editor should start with — the value as the user last
|
||||
/// **typed** it, not as it is drawn. The difference corrupts data (§3).
|
||||
pub edit_text: Option<String>,
|
||||
/// The value the host evaluated here, unrounded and untyped-by-text: what
|
||||
/// `yank` copies. Neither `text` nor `edit_text` can be re-parsed into it (§3).
|
||||
pub value: Option<CellValue>,
|
||||
}
|
||||
|
||||
impl CellDisplay {
|
||||
@@ -98,6 +102,7 @@ impl CellDisplay {
|
||||
numeric: true,
|
||||
error: false,
|
||||
id: None,
|
||||
value: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -110,6 +115,7 @@ impl CellDisplay {
|
||||
error: true,
|
||||
id: None,
|
||||
edit_text: None,
|
||||
value: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -416,7 +422,7 @@ fn shows(items: &[&Vec<String>], idx: usize, first_rendered: usize, level: usize
|
||||
|
||||
/// The grid widget's copy of the visible cells (improvise-n10), indexed by
|
||||
/// position within the **viewport** and never by `CellKey`.
|
||||
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)]
|
||||
pub struct GridCache {
|
||||
/// The row axis's frame: levels, label tuples and groups. **Not** a list
|
||||
/// of strings — see [`AxisFrame`].
|
||||
@@ -563,7 +569,7 @@ enum LabelAxis {
|
||||
|
||||
/// What can change about the grid: cell values, the frames that name them,
|
||||
/// the widths, the totals, the page bar and the extent.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||
pub enum GridUpdate {
|
||||
/// Display values for cells, by viewport position.
|
||||
Cells(Vec<((usize, usize), CellDisplay)>),
|
||||
|
||||
@@ -530,12 +530,10 @@ fn composed(
|
||||
state,
|
||||
ModelCommand::ClearCell(record_cell_at_cursor(state)?),
|
||||
)?]),
|
||||
// `YankCell::execute`: the value, then the status. `edit_text` is
|
||||
// what the host evaluated, which is what the TUI yanks.
|
||||
// `YankCell::execute`: the value, then the status. The projected
|
||||
// value, never text re-parsed: improvise-dsxq.
|
||||
"yank" => Some(vec![
|
||||
Command::SetYanked(typed(
|
||||
cell_at_cursor(state)?.edit_text.as_deref().unwrap_or(""),
|
||||
)),
|
||||
Command::SetYanked(cell_at_cursor(state)?.value.clone()),
|
||||
Command::SetStatus("Yanked".to_string()),
|
||||
]),
|
||||
"paste" => Some(match state.view.yanked.clone() {
|
||||
@@ -748,9 +746,10 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
|
||||
},
|
||||
Command::ChangeMode(mode),
|
||||
]),
|
||||
CellId::Aggregate(_) => Some(vec![Command::SetStatus(
|
||||
"drilling into an aggregate needs the host — not in the browser yet".to_string(),
|
||||
)]),
|
||||
CellId::Aggregate(_) => Some(vec![Command::SetStatus(format!(
|
||||
"an aggregate is edited by drilling in — {}",
|
||||
not_yet("drill")
|
||||
))]),
|
||||
CellId::Derived { target } => Some(vec![Command::SetStatus(format!(
|
||||
"{target} is computed by a formula — edit the formula, not the cell"
|
||||
))]),
|
||||
@@ -954,11 +953,10 @@ fn record_at_cursor(state: &ClientState) -> Option<crate::RecordHash> {
|
||||
}
|
||||
}
|
||||
|
||||
/// What an editor opened on the cursor starts with. The one home, so a
|
||||
/// renderer does not reconcile the scroll offset again (gotcha 42).
|
||||
pub fn edit_seed(state: &ClientState) -> String {
|
||||
let (row, col) = state.view.cursor().selected;
|
||||
seed_at(state, row, col)
|
||||
/// What an editor opened on the cursor starts with, or `None` with no cell
|
||||
/// there to seed from. Read by the fold, never a renderer (improvise-ktlk).
|
||||
pub(crate) fn edit_seed(state: &ClientState) -> Option<String> {
|
||||
cell_at_cursor(state)?.edit_text.clone()
|
||||
}
|
||||
|
||||
/// The display text at an absolute grid position, as the cache holds it.
|
||||
@@ -2435,28 +2433,38 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// The advertised key must say what is actually true. Two refusals, two
|
||||
/// subjects — see `context/test-evidence/clients.md`.
|
||||
/// `i` on an aggregate IS drill in the TUI, so both doors name one missing
|
||||
/// feature; only `i` says why. See `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn the_drill_key_and_an_aggregate_edit_refuse_for_different_reasons() {
|
||||
let mut state = in_mode(AppMode::Normal);
|
||||
state.grid.extent = GridExtent { rows: 2, cols: 2 };
|
||||
state.window = (2, 2);
|
||||
|
||||
let KeyOutcome::Local(drill) = resolve(&state, ev(KeyCode::Char('>'))) else {
|
||||
panic!("`>` is advertised in the hint line; it must answer");
|
||||
};
|
||||
let [Command::SetStatus(said)] = &drill[..] else {
|
||||
panic!("expected one status, got {drill:?}");
|
||||
fn an_aggregate_edit_names_the_same_missing_drill_as_the_drill_key() {
|
||||
let aggregate = CellId::Aggregate(
|
||||
serde_json::from_str("\"aa4c8e5e9cbd578e0d388f4b2b5a7879\"").unwrap(),
|
||||
);
|
||||
let state = client_on(aggregate);
|
||||
let said = |key| match resolve(&state, ev(key)) {
|
||||
KeyOutcome::Local(commands) => match &commands[..] {
|
||||
[Command::SetStatus(said)] => said.clone(),
|
||||
other => panic!("expected one status, got {other:?}"),
|
||||
},
|
||||
other => panic!("{key:?} is advertised; it must answer, got {other:?}"),
|
||||
};
|
||||
let (drill, edit) = (said(KeyCode::Char('>')), said(KeyCode::Char('i')));
|
||||
assert!(
|
||||
said.contains("drill") && said.contains("browser"),
|
||||
"the status must name drill and say it is the browser that lacks it: {said}"
|
||||
drill.contains("drill") && drill.contains("browser"),
|
||||
"the status must name drill and say it is the browser that lacks it: {drill}"
|
||||
);
|
||||
assert!(
|
||||
!said.contains("aggregate"),
|
||||
"a sentence about aggregates is TRUE-SOUNDING about the wrong thing — \
|
||||
`>` is refused whatever the cell is: {said}"
|
||||
!drill.contains("aggregate"),
|
||||
"`>` is refused whatever the cell is, so it must not blame the cell: {drill}"
|
||||
);
|
||||
assert!(
|
||||
edit.contains(&drill),
|
||||
"`i` reached the same missing feature and must name it the same way: \
|
||||
{edit:?} does not contain {drill:?}"
|
||||
);
|
||||
assert!(
|
||||
edit.contains("aggregate"),
|
||||
"and `i` must still say why it drilled rather than edited: {edit}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -3017,6 +3025,55 @@ mod tests {
|
||||
));
|
||||
}
|
||||
|
||||
/// improvise-ktlk: what an editor shows and what Enter commits are one
|
||||
/// fact. See `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn an_editor_entered_without_its_buffer_commits_and_extends_the_cell() {
|
||||
let mut client = client_on(record_cell());
|
||||
let mut cell = CellDisplay::number("1,200.50");
|
||||
cell.edit_text = Some("1200.5".into());
|
||||
cell.id = Some(record_cell());
|
||||
client.fold(&Command::Project(Projection::Grid(GridUpdate::Cells(
|
||||
vec![((0, 0), cell)],
|
||||
))));
|
||||
client.fold(&Command::ChangeMode(AppMode::editing()));
|
||||
assert_eq!(
|
||||
read_buffer(&client.view, "edit"),
|
||||
"1200.5",
|
||||
"the buffer is what the overlay paints and what a commit reads"
|
||||
);
|
||||
|
||||
let committed = |client: &ClientState| {
|
||||
let KeyOutcome::Local(commands) = resolve(client, ev(KeyCode::Enter)) else {
|
||||
panic!("Enter in Editing must resolve here");
|
||||
};
|
||||
match &commands[0] {
|
||||
Command::Model {
|
||||
cmd: ModelCommand::SetCell { value, .. },
|
||||
..
|
||||
} => Some(value.clone()),
|
||||
_ => None,
|
||||
}
|
||||
};
|
||||
assert_eq!(
|
||||
committed(&client),
|
||||
Some(CellValue::Number(1200.5)),
|
||||
"Enter on an editor showing the cell must not clear it"
|
||||
);
|
||||
|
||||
let KeyOutcome::Local(typed) = resolve(&client, ev(KeyCode::Char('5'))) else {
|
||||
panic!("a character in Editing is append-char");
|
||||
};
|
||||
for command in &typed {
|
||||
client.fold(command);
|
||||
}
|
||||
assert_eq!(
|
||||
committed(&client),
|
||||
Some(CellValue::Number(1200.55)),
|
||||
"a typed character extends what the editor showed, not replaces it"
|
||||
);
|
||||
}
|
||||
|
||||
/// At the bottom-right corner `EnterAdvance` returns `AbortChain`, so the
|
||||
/// commit leaves the user in normal mode. See the evidence file.
|
||||
#[test]
|
||||
@@ -3139,6 +3196,7 @@ mod tests {
|
||||
#[test]
|
||||
fn yy_yanks_the_cell_under_the_cursor_and_p_pastes_what_it_yanked() {
|
||||
let mut state = client_on(record_cell());
|
||||
state.grid.cells[0][0].value = Some(CellValue::Number(1200.0));
|
||||
arm(&mut state, ev(KeyCode::Char('y')));
|
||||
let KeyOutcome::Local(yank) = resolve(&state, ev(KeyCode::Char('y'))) else {
|
||||
panic!("`yy` is bound to yank");
|
||||
|
||||
@@ -42,11 +42,11 @@ pub struct RenderModel {
|
||||
pub categories: Vec<RenderCatRow>,
|
||||
/// Whether the panel is open, so the page does not decide.
|
||||
pub category_panel_open: bool,
|
||||
/// The view panel's rows, **empty unless it is open** — the same rule the
|
||||
/// help overlay follows, so the page asks no question about the mode.
|
||||
pub views: Vec<RenderViewRow>,
|
||||
/// The formula panel's rows, empty unless it is open.
|
||||
pub formulas: Vec<RenderFormulaRow>,
|
||||
/// The view panel's rows, **`None` when it is closed** and possibly empty
|
||||
/// when open: an absence is not an empty list (improvise-df2w).
|
||||
pub views: Option<Vec<RenderViewRow>>,
|
||||
/// The formula panel's rows, on the same terms as `views`.
|
||||
pub formulas: Option<Vec<RenderFormulaRow>>,
|
||||
/// The tile bar, drawn on every frame as the TUI draws it. Which tile is
|
||||
/// `selected` is the mode's answer, not the page's.
|
||||
pub tiles: Vec<RenderTile>,
|
||||
@@ -294,16 +294,13 @@ pub fn render_model(state: &ClientState) -> RenderModel {
|
||||
help: help_rows(&state.view),
|
||||
help_page: state.view.help_page,
|
||||
help_titles: help_titles(&state.view),
|
||||
// An ABSENT buffer is not an empty one: blanking the overlay over a
|
||||
// real value is one Enter from clearing it (improvise-xg1z).
|
||||
editing: state.view.mode.is_editing().then(|| {
|
||||
state
|
||||
.view
|
||||
.buffers
|
||||
.get("edit")
|
||||
.cloned()
|
||||
.unwrap_or_else(|| improvise_protocol::edit_seed(state))
|
||||
}),
|
||||
// The buffer Enter commits, never a second derivation of it: the fold
|
||||
// seeds an absent one (improvise-xg1z, improvise-ktlk).
|
||||
editing: state
|
||||
.view
|
||||
.mode
|
||||
.is_editing()
|
||||
.then(|| improvise_protocol::read_buffer(&state.view, "edit")),
|
||||
minibuffer: state.view.mode.minibuffer().map(|mb| RenderMinibuffer {
|
||||
prompt: mb.prompt.clone(),
|
||||
value: improvise_protocol::read_buffer(&state.view, &mb.buffer_key),
|
||||
@@ -344,41 +341,39 @@ fn page_axes(state: &ClientState) -> Vec<RenderPageAxis> {
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// The view panel's rows while it is open. The axis line is
|
||||
/// The view panel's rows, or `None` while it is closed. The axis line is
|
||||
/// `viewmodel::view_axis_summary`, which the TUI's own panel calls too.
|
||||
fn view_rows(state: &ClientState) -> Vec<RenderViewRow> {
|
||||
if !state.view.view_panel_open {
|
||||
return Vec::new();
|
||||
}
|
||||
state
|
||||
.views
|
||||
.views
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, summary)| RenderViewRow {
|
||||
name: summary.name.clone(),
|
||||
axes: improvise_protocol::viewmodel::view_axis_summary(summary),
|
||||
active: summary.name == state.view.active_view,
|
||||
selected: i == state.view.view_panel_cursor,
|
||||
})
|
||||
.collect()
|
||||
fn view_rows(state: &ClientState) -> Option<Vec<RenderViewRow>> {
|
||||
state.view.view_panel_open.then(|| {
|
||||
state
|
||||
.views
|
||||
.views
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, summary)| RenderViewRow {
|
||||
name: summary.name.clone(),
|
||||
axes: improvise_protocol::viewmodel::view_axis_summary(summary),
|
||||
active: summary.name == state.view.active_view,
|
||||
selected: i == state.view.view_panel_cursor,
|
||||
})
|
||||
.collect()
|
||||
})
|
||||
}
|
||||
|
||||
/// The formula panel's rows while it is open.
|
||||
fn formula_rows(state: &ClientState) -> Vec<RenderFormulaRow> {
|
||||
if !state.view.formula_panel_open {
|
||||
return Vec::new();
|
||||
}
|
||||
state
|
||||
.formulas
|
||||
.formulas
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, text)| RenderFormulaRow {
|
||||
text: text.clone(),
|
||||
selected: i == state.view.formula_cursor,
|
||||
})
|
||||
.collect()
|
||||
/// The formula panel's rows, or `None` while it is closed.
|
||||
fn formula_rows(state: &ClientState) -> Option<Vec<RenderFormulaRow>> {
|
||||
state.view.formula_panel_open.then(|| {
|
||||
state
|
||||
.formulas
|
||||
.formulas
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, text)| RenderFormulaRow {
|
||||
text: text.clone(),
|
||||
selected: i == state.view.formula_cursor,
|
||||
})
|
||||
.collect()
|
||||
})
|
||||
}
|
||||
|
||||
/// The tile bar, always. Only the *selection* is conditional, exactly as
|
||||
@@ -800,34 +795,48 @@ mod tests {
|
||||
client
|
||||
}
|
||||
|
||||
/// An empty list is what says a panel is closed — the same rule the help
|
||||
/// overlay follows, so the page asks no question about the mode.
|
||||
/// `None` is what says a panel is closed, so the page asks no question
|
||||
/// about the mode.
|
||||
#[test]
|
||||
fn a_closed_panel_projects_no_rows_and_an_open_one_does() {
|
||||
let mut client = client_with_panels();
|
||||
let closed = client.model();
|
||||
assert!(
|
||||
closed.views.is_empty(),
|
||||
closed.views.is_none(),
|
||||
"the cache is full but the panel is shut"
|
||||
);
|
||||
assert!(closed.formulas.is_empty());
|
||||
assert!(closed.formulas.is_none());
|
||||
|
||||
client.state_mut().view.view_panel_open = true;
|
||||
client.state_mut().view.formula_panel_open = true;
|
||||
client.state_mut().view.view_panel_cursor = 1;
|
||||
client.state_mut().view.active_view = "Default".into();
|
||||
let open = client.model();
|
||||
assert_eq!(open.views.len(), 2);
|
||||
let (views, formulas) = (open.views.unwrap(), open.formulas.unwrap());
|
||||
assert_eq!(views.len(), 2);
|
||||
assert_eq!(
|
||||
open.views[0].axes, "R:Region",
|
||||
views[0].axes, "R:Region",
|
||||
"the axis line is the shared derivation, and _Dim is not part of it"
|
||||
);
|
||||
assert!(
|
||||
open.views[0].active,
|
||||
"active is ViewState's, not the cache's"
|
||||
);
|
||||
assert!(open.views[1].selected, "and selected is the panel cursor's");
|
||||
assert_eq!(open.formulas[0].text, "Profit = Revenue - Cost");
|
||||
assert!(views[0].active, "active is ViewState's, not the cache's");
|
||||
assert!(views[1].selected, "and selected is the panel cursor's");
|
||||
assert_eq!(formulas[0].text, "Profit = Revenue - Cost");
|
||||
}
|
||||
|
||||
/// Open with nothing in it is not closed: the page must be able to tell
|
||||
/// them apart from the model alone (improvise-df2w).
|
||||
#[test]
|
||||
fn an_open_empty_panel_is_not_a_closed_one() {
|
||||
let mut client = init();
|
||||
let closed = client.model();
|
||||
client.state_mut().view.formula_panel_open = true;
|
||||
let formulas_open = client.model();
|
||||
assert_ne!(formulas_open, closed, "an open, empty formula panel");
|
||||
|
||||
client.state_mut().view.formula_panel_open = false;
|
||||
client.state_mut().view.view_panel_open = true;
|
||||
let views_open = client.model();
|
||||
assert_ne!(views_open, closed, "an open, empty view panel");
|
||||
}
|
||||
|
||||
/// The tile bar is drawn always; only the *selection* waits for the mode,
|
||||
@@ -912,10 +921,11 @@ mod tests {
|
||||
client
|
||||
.state_mut()
|
||||
.fold(&Command::ChangeMode(improvise_protocol::AppMode::editing()));
|
||||
assert!(
|
||||
!client.state().view.buffers.contains_key("edit"),
|
||||
"the fixture must really be missing the buffer, or this proves \
|
||||
nothing about the fallback"
|
||||
assert_eq!(
|
||||
client.state().view.buffers.get("edit").map(String::as_str),
|
||||
Some("1200"),
|
||||
"the fold seeds the missing buffer, so the overlay and a commit \
|
||||
read one thing (improvise-ktlk)"
|
||||
);
|
||||
assert_eq!(
|
||||
client.model().rows[0].cells()[0].text,
|
||||
|
||||
@@ -1581,3 +1581,177 @@ fn opens_an_editor(client: &ClientState, key: KeyEvent) -> bool {
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
// Yank and paste (improvise-dsxq).
|
||||
|
||||
fn region_measure(region: &str, measure: &str) -> improvise_command::model::cell::CellKey {
|
||||
improvise_command::model::cell::CellKey::new(vec![
|
||||
("Region".to_string(), region.to_string()),
|
||||
("_Measure".to_string(), measure.to_string()),
|
||||
])
|
||||
}
|
||||
|
||||
/// Four rows and four measures, the last a formula, holding values on which the
|
||||
/// edit text and the value disagree. Evidence file.
|
||||
fn yank_host() -> App {
|
||||
use improvise_protocol::CellValue::{Number, Text};
|
||||
let mut wb = Workbook::new("Test");
|
||||
wb.add_category("Region").unwrap();
|
||||
for item in ["West", "North", "South", "East"] {
|
||||
wb.add_item("Region", item).unwrap();
|
||||
}
|
||||
for item in ["Units", "Revenue", "Cost"] {
|
||||
wb.add_item("_Measure", item).unwrap();
|
||||
}
|
||||
wb.model.add_formula(
|
||||
improvise_command::formula::parse_formula("Profit = Revenue - Cost", "_Measure").unwrap(),
|
||||
);
|
||||
wb.active_view_mut().set_axis("Region", Axis::Row);
|
||||
wb.active_view_mut().set_axis("_Measure", Axis::Column);
|
||||
for (region, measure, value) in [
|
||||
("North", "Revenue", Number(1.0)),
|
||||
("North", "Cost", Number(2.0)),
|
||||
("South", "Revenue", Number(1.23456789)),
|
||||
("South", "Cost", Text("007".into())),
|
||||
("East", "Revenue", Number(10.123456)),
|
||||
("East", "Cost", Number(0.5)),
|
||||
] {
|
||||
wb.model.set_cell(region_measure(region, measure), value);
|
||||
}
|
||||
let mut app = App::new(wb, None);
|
||||
app.term_height = 4 + 8;
|
||||
app.rebuild_layout();
|
||||
app
|
||||
}
|
||||
|
||||
/// One unmodified character on both ends: the host through its keymap, a twin
|
||||
/// through exactly what the client composed.
|
||||
fn press(app: &mut App, twin: &mut App, client: &mut ClientState, key: char) {
|
||||
let event = KeyEvent::new(KeyCode::Char(key), KeyModifiers::NONE);
|
||||
let KeyOutcome::Local(cmds) = resolve(client, event) else {
|
||||
panic!("{key}: the client must answer this key locally");
|
||||
};
|
||||
for cmd in &cmds {
|
||||
client.fold(cmd);
|
||||
}
|
||||
apply_on(twin, &cmds);
|
||||
reduce_key(app, event);
|
||||
}
|
||||
|
||||
fn seat(app: &mut App, client: &mut ClientState, at: (usize, usize)) {
|
||||
app.cursor_mut().selected = at;
|
||||
client.view.cursor_mut().selected = at;
|
||||
}
|
||||
|
||||
/// `yy` then `p` copies the value the TUI copies, not its edit text re-parsed.
|
||||
/// Never an integer: the one kind on which the two agree. Evidence file.
|
||||
#[test]
|
||||
fn yy_then_p_copies_the_value_the_host_copies() {
|
||||
use improvise_protocol::CellValue::{Number, Text};
|
||||
let cases = [
|
||||
(
|
||||
"a non-whole number",
|
||||
(2, 1),
|
||||
(1, 1),
|
||||
"Revenue",
|
||||
Number(1.23456789),
|
||||
),
|
||||
(
|
||||
"number-like text",
|
||||
(2, 2),
|
||||
(1, 2),
|
||||
"Cost",
|
||||
Text("007".into()),
|
||||
),
|
||||
(
|
||||
"a formula's value",
|
||||
(3, 3),
|
||||
(1, 1),
|
||||
"Revenue",
|
||||
Number(10.123456 - 0.5),
|
||||
),
|
||||
];
|
||||
for (what, from, to, measure, expected) in cases {
|
||||
let mut app = yank_host();
|
||||
let mut twin = yank_host();
|
||||
let mut client = client_ready_to_edit(&app, (4, 4));
|
||||
|
||||
seat(&mut app, &mut client, from);
|
||||
press(&mut app, &mut twin, &mut client, 'y');
|
||||
press(&mut app, &mut twin, &mut client, 'y');
|
||||
assert_eq!(
|
||||
app.view_state.yanked,
|
||||
Some(expected.clone()),
|
||||
"{what}: the host must yank the value itself, or the fixture proves nothing"
|
||||
);
|
||||
assert_eq!(
|
||||
client.view.yanked, app.view_state.yanked,
|
||||
"{what}: client and host yanked different values"
|
||||
);
|
||||
|
||||
seat(&mut app, &mut client, to);
|
||||
press(&mut app, &mut twin, &mut client, 'p');
|
||||
assert_eq!(
|
||||
app.model_state
|
||||
.workbook
|
||||
.model
|
||||
.get_cell(®ion_measure("North", measure)),
|
||||
Some(&expected),
|
||||
"{what}: the host's paste must have landed"
|
||||
);
|
||||
assert_eq!(
|
||||
document_text(&twin),
|
||||
document_text(&app),
|
||||
"{what}: the client's paste wrote a different document"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// A true aggregate: `Product` on no axis, so the cell sums two records and has
|
||||
/// no stored value. Yank only: an all-aggregate grid has nowhere to paste.
|
||||
#[test]
|
||||
fn yy_over_an_aggregate_copies_the_sum_the_host_copies() {
|
||||
let mut wb = Workbook::new("Test");
|
||||
wb.add_category("Region").unwrap();
|
||||
wb.add_item("Region", "North").unwrap();
|
||||
wb.add_item("Region", "South").unwrap();
|
||||
wb.add_category("Product").unwrap();
|
||||
wb.add_item("Product", "Widget").unwrap();
|
||||
wb.add_item("Product", "Gadget").unwrap();
|
||||
wb.add_item("_Measure", "Revenue").unwrap();
|
||||
wb.active_view_mut().set_axis("Region", Axis::Row);
|
||||
wb.active_view_mut().set_axis("_Measure", Axis::Column);
|
||||
wb.active_view_mut().set_axis("Product", Axis::None);
|
||||
for (product, value) in [("Widget", 1.23456789), ("Gadget", 1.0)] {
|
||||
wb.model.set_cell(
|
||||
region_measure("South", "Revenue").with("Product", product),
|
||||
improvise_protocol::CellValue::Number(value),
|
||||
);
|
||||
}
|
||||
let mut app = App::new(wb, None);
|
||||
app.term_height = 2 + 8;
|
||||
app.rebuild_layout();
|
||||
let mut client = client_of(&app, (2, 1));
|
||||
|
||||
app.cursor_mut().selected = (1, 0);
|
||||
client.view.cursor_mut().selected = (1, 0);
|
||||
for _ in 0..2 {
|
||||
let event = KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE);
|
||||
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
|
||||
panic!("y must resolve locally over an aggregate too");
|
||||
};
|
||||
for cmd in &cmds {
|
||||
client.fold(cmd);
|
||||
}
|
||||
reduce_key(&mut app, event);
|
||||
}
|
||||
assert_eq!(
|
||||
app.view_state.yanked,
|
||||
Some(improvise_protocol::CellValue::Number(1.23456789 + 1.0)),
|
||||
"the host yanks the sum, or the fixture is not an aggregate"
|
||||
);
|
||||
assert_eq!(
|
||||
client.view.yanked, app.view_state.yanked,
|
||||
"client and host yanked different values over an aggregate"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -70,7 +70,7 @@ test.describe("every field the render model projects reaches the screen", () =>
|
||||
await expect(page.locator("#categories")).toBeVisible();
|
||||
await page.keyboard.press("Escape");
|
||||
|
||||
// `views`: empty unless open, so the page asks no question about the mode.
|
||||
// `views`: `null` unless open, so the page asks no question about the mode.
|
||||
await expect(page.locator("#views")).toBeHidden();
|
||||
await page.keyboard.press("Shift+V");
|
||||
await expect(page.locator("#views li")).toHaveCount(2);
|
||||
@@ -356,6 +356,27 @@ test.describe("a pointer event reaches the client and its answer is painted", ()
|
||||
});
|
||||
});
|
||||
|
||||
test.describe("an open panel is drawn even when it has nothing in it", () => {
|
||||
// improvise-df2w: `formulas` said "closed" by being empty, so a document with
|
||||
// no formulas could open the panel and never see it. `tall.improv` has none.
|
||||
test("the formula panel opens over a document with no formulas", async ({ page }) => {
|
||||
await page.goto("/app/index.html?doc=df2w-empty.improv&seed=tall.improv");
|
||||
await expect(page.locator("#grid table")).toBeVisible({ timeout: 20_000 });
|
||||
await page.locator("#grid").focus();
|
||||
await expect(page.locator("#formulas")).toBeHidden();
|
||||
|
||||
await page.keyboard.press("Shift+F");
|
||||
await expect(page.locator("#mode")).toHaveText("FORMULAS");
|
||||
await expect(page.locator("#formulas")).toBeVisible();
|
||||
await expect(page.locator("#formulas li")).toHaveCount(0);
|
||||
|
||||
// And closed is still closed: the other direction, which a page that
|
||||
// simply never hid the panel would fail.
|
||||
await page.keyboard.press("Shift+F");
|
||||
await expect(page.locator("#formulas")).toBeHidden();
|
||||
});
|
||||
});
|
||||
|
||||
test.describe("the selection is on screen", () => {
|
||||
// improvise-jq8l: `web/app.js` called `scrollIntoView` NOWHERE, so a selection
|
||||
// moved past the bottom of `#grid` simply stayed there, invisible, with no way
|
||||
|
||||
@@ -36,12 +36,14 @@ async function anAggregateGrid(page) {
|
||||
test.describe("drill is refused in the browser, not broken", () => {
|
||||
// The honest half: `i`/`a` on an aggregate declines OUT LOUD, because
|
||||
// `edit_or_drill` answers the `CellId::Aggregate` arm with a status.
|
||||
test("editing an aggregate says drill needs the host", async ({ page }) => {
|
||||
test("editing an aggregate says it is edited by drilling, and drill is missing", async ({
|
||||
page,
|
||||
}) => {
|
||||
await anAggregateGrid(page);
|
||||
|
||||
await page.keyboard.press("i");
|
||||
await expect(page.locator("#toast")).toContainText(
|
||||
"drilling into an aggregate needs the host",
|
||||
"an aggregate is edited by drilling in — drill is not available in the browser yet",
|
||||
);
|
||||
// And it really is a refusal: no editor opened, no mode change.
|
||||
await expect(page.locator("#mode")).toHaveText("NORMAL");
|
||||
@@ -63,16 +65,15 @@ test.describe("drill is refused in the browser, not broken", () => {
|
||||
);
|
||||
await expect(page.locator("#mode")).toHaveText("NORMAL");
|
||||
|
||||
// The two refusals have two SUBJECTS, and conflating them would be a
|
||||
// true-sounding sentence about the wrong thing — worse than silence,
|
||||
// because it stops the reader looking. `>` must not talk about aggregates.
|
||||
// `>` is refused whatever the cell is, so it must not blame the cell.
|
||||
await expect(page.locator("#toast")).not.toContainText("aggregate");
|
||||
|
||||
// And `i` still gives the OTHER refusal, which is what proves the two are
|
||||
// distinct rather than one message reached by two keys.
|
||||
// `i` reaches the SAME missing feature (in the TUI it drills), so it names
|
||||
// it the same way and adds only why — improvise-s8h7, clients.md.
|
||||
await page.keyboard.press("i");
|
||||
await expect(page.locator("#toast")).toContainText("an aggregate is edited by drilling in");
|
||||
await expect(page.locator("#toast")).toContainText(
|
||||
"drilling into an aggregate needs the host",
|
||||
"drill is not available in the browser yet",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -76,36 +76,37 @@ run_case() {
|
||||
scaffold
|
||||
run_case "both workspaces clean is a pass" 0 "$work/repo"
|
||||
|
||||
# The load-bearing case. `cargo fmt --all` at the outer root cannot reach this
|
||||
# file, and `cargo fmt -p inner-pkg` from the outer root exits 1 saying `not a
|
||||
# member of the workspace` -- the usage error this whole script exists to split
|
||||
# from a real verdict.
|
||||
# One fault case PER PACKAGE, enumerated from the fixture rather than
|
||||
# hand-picked: three hand-picked shapes each missed a rival, and the fourth
|
||||
# missed the root package itself (improvise-90ea). Reasoning:
|
||||
# context/test-evidence/tooling.md. The enumeration is a find over
|
||||
# `[package]` manifests, deliberately NOT the script's own `[workspace]` +
|
||||
# `cargo metadata` discovery, which is the thing under test.
|
||||
scaffold
|
||||
printf '%s' "$DIRTY" > "$work/repo/nested/src/lib.rs"
|
||||
if run_case "a fault in the NESTED workspace is reported" 1 "$work/repo"; then
|
||||
case "$OUT" in
|
||||
*nested/src/lib.rs*) ok " and the nested file is named" ;;
|
||||
*) bad " and the nested file is named" "output did not name it: ${OUT:0:300}" ;;
|
||||
esac
|
||||
pkg_manifests=()
|
||||
while IFS= read -r m; do pkg_manifests+=("$m"); done < <(
|
||||
cd "$work/repo" && grep -rl --include=Cargo.toml '^\[package\]' . | sort)
|
||||
# An empty or shrunken enumeration would make the loop below vacuous.
|
||||
if [ "${#pkg_manifests[@]}" = 3 ]; then
|
||||
ok "the fixture enumerates all three packages (root, member, nested)"
|
||||
else
|
||||
bad "the fixture enumerates all three packages" "found: ${pkg_manifests[*]:-none}"
|
||||
fi
|
||||
|
||||
scaffold
|
||||
printf '%s' "$DIRTY" > "$work/repo/a/src/lib.rs"
|
||||
if run_case "a fault in the OUTER workspace is reported" 1 "$work/repo"; then
|
||||
case "$OUT" in
|
||||
*a/src/lib.rs*) ok " and the outer file is named" ;;
|
||||
*) bad " and the outer file is named" "output did not name it: ${OUT:0:300}" ;;
|
||||
esac
|
||||
# The rival: spelling the per-root check `--all` instead of `-p <members>`
|
||||
# walks PAST the nested workspace boundary (measured), so the OUTER file is
|
||||
# reported a second time under the inner root. Counting is what sees that.
|
||||
n="$(printf '%s\n' "$OUT" | grep -c 'a/src/lib.rs')"
|
||||
if [ "$n" = 1 ]; then
|
||||
ok " and it is named ONCE -- no root reaches past its own boundary"
|
||||
else
|
||||
bad " and it is named ONCE" "named $n times; a root is walking past its boundary"
|
||||
for m in "${pkg_manifests[@]}"; do
|
||||
rel="$(dirname "${m#./}")/src/lib.rs"; rel="${rel#./}"
|
||||
scaffold
|
||||
printf '%s' "$DIRTY" > "$work/repo/$rel"
|
||||
if run_case "a fault in $rel is reported" 1 "$work/repo"; then
|
||||
# Counting, not matching: `--all` for `-p` walks PAST the nested boundary
|
||||
# (measured) and names an outer file a second time under the inner root.
|
||||
n="$(printf '%s\n' "$OUT" | grep -cF "$work/repo/$rel")"
|
||||
if [ "$n" = 1 ]; then
|
||||
ok " and $rel is named exactly once"
|
||||
else
|
||||
bad " and $rel is named exactly once" "named $n times; output: ${OUT:0:300}"
|
||||
fi
|
||||
fi
|
||||
fi
|
||||
done
|
||||
|
||||
# A usage error must NOT wear a formatting failure's exit code. This is the
|
||||
# whole ticket: `cargo fmt -p opfs-contract` from the repo root exits 1.
|
||||
@@ -121,6 +122,32 @@ run_case "a tree with no workspace root is a usage error, not a pass" 2 "$work/e
|
||||
|
||||
run_case "--help is a usage error, so a typo cannot pass" 2 "$work/repo" --help
|
||||
|
||||
# The loop above covers every package the FIXTURE has; this covers every
|
||||
# package the TREE has. A package no root lists is checked by nothing, so the
|
||||
# gate must refuse a verdict -- and name the package -- rather than pass.
|
||||
scaffold
|
||||
mkdir -p "$work/repo/orphan/src"
|
||||
printf '[package]\nname = "orphan-pkg"\nversion = "0.1.0"\nedition = "2021"\n' \
|
||||
> "$work/repo/orphan/Cargo.toml"
|
||||
printf '%s' "$DIRTY" > "$work/repo/orphan/src/lib.rs"
|
||||
cat > "$work/repo/Cargo.toml" <<'EOF'
|
||||
[workspace]
|
||||
members = ["a"]
|
||||
exclude = ["orphan"]
|
||||
resolver = "2"
|
||||
|
||||
[package]
|
||||
name = "root-pkg"
|
||||
version = "0.1.0"
|
||||
edition = "2021"
|
||||
EOF
|
||||
if run_case "a package no workspace root reaches is exit 2, not a pass" 2 "$work/repo"; then
|
||||
case "$OUT" in
|
||||
*"no workspace root reaches"*orphan/Cargo.toml*) ok " and the unreached package is named" ;;
|
||||
*) bad " and the unreached package is named" "output: ${OUT:0:300}" ;;
|
||||
esac
|
||||
fi
|
||||
|
||||
# The narrowed contract, pinned so it is not merely prose (ChatGPT Codex on
|
||||
# PR #159). rustfmt exits 1 for a source it cannot parse, exactly as for a
|
||||
# diff -- so exit 1 is "rustfmt did not certify this root clean", and what
|
||||
|
||||
+43
-17
@@ -1,7 +1,7 @@
|
||||
#!/usr/bin/env bash
|
||||
# Format-checks every Cargo workspace root in the repo, from any directory.
|
||||
# `cargo fmt --all` reaches only ONE of them. Reasoning: context/code/tooling.md
|
||||
# section 4 -- including why `-p` scoped by `--manifest-path` is the only
|
||||
# `cargo fmt --all` reaches only ONE of them. Reasoning: context/test-evidence/
|
||||
# tooling.md, "The two-workspace fmt gate" -- including why `-p` scoped by `--manifest-path` is the only
|
||||
# spelling that is neither blind nor boundary-crossing (improvise-b4ks).
|
||||
set -uo pipefail
|
||||
|
||||
@@ -18,8 +18,9 @@ exit 1 rustfmt did not certify a root clean: a file differs from rustfmt's
|
||||
output, or rustfmt refused to parse one. Either way a FILE IS NAMED
|
||||
and the tree is what is wrong.
|
||||
exit 2 the gate did not run: usage error, no cargo, no rustfmt, cargo
|
||||
metadata failed, or no [workspace] root found. NO verdict was
|
||||
produced and NO file is named.
|
||||
metadata failed, no [workspace] root found, or a [package] that no
|
||||
root has as a member. NO verdict was produced. No SOURCE file is
|
||||
named; the unreached-package case names its Cargo.toml.
|
||||
|
||||
The split is "did the gate run", not "is every exit-1 cause a diff". That is
|
||||
narrower than it sounds and it is deliberate -- see the note on rc=1 below.
|
||||
@@ -38,8 +39,8 @@ EOF
|
||||
# The find is RELATIVE to the tree being scanned: an absolute `-path` filter
|
||||
# matches the ancestors too, and this repo is itself checked out under
|
||||
# `.claude/worktrees/` in every agent worktree, where that excluded everything.
|
||||
workspace_roots() {
|
||||
local root="$1"
|
||||
manifests_with() {
|
||||
local root="$1" section="$2"
|
||||
( cd "$root" && find . -name Cargo.toml \
|
||||
-not -path './target/*' -not -path '*/target/*' \
|
||||
-not -path './.git/*' \
|
||||
@@ -47,11 +48,12 @@ workspace_roots() {
|
||||
-not -path './.claude/*' ) \
|
||||
| sort \
|
||||
| while IFS= read -r rel; do
|
||||
if grep -q '^\[workspace\]' "$root/${rel#./}"; then
|
||||
if grep -q "^\\[$section\\]" "$root/${rel#./}"; then
|
||||
printf '%s\n' "$root/${rel#./}"
|
||||
fi
|
||||
done
|
||||
}
|
||||
workspace_roots() { manifests_with "$1" workspace; }
|
||||
|
||||
# The members of ONE workspace, named by cargo rather than parsed out of the
|
||||
# manifest: a `members = [...]` list may carry globs and path dependencies.
|
||||
@@ -61,10 +63,10 @@ members_of() {
|
||||
printf '%s' "$meta" | python3 -c '
|
||||
import json, sys
|
||||
doc = json.load(sys.stdin)
|
||||
names = [p["name"] for p in doc["packages"]]
|
||||
if not names:
|
||||
rows = [p["name"] + "\t" + p["manifest_path"] for p in doc["packages"]]
|
||||
if not rows:
|
||||
sys.exit(1)
|
||||
print("\n".join(names))
|
||||
print("\n".join(rows))
|
||||
' || return 1
|
||||
}
|
||||
|
||||
@@ -106,15 +108,39 @@ main() {
|
||||
return 2
|
||||
fi
|
||||
|
||||
local rc=0 manifest member
|
||||
for manifest in "${roots[@]}"; do
|
||||
local -a pkgs=()
|
||||
while IFS= read -r member; do [ -n "$member" ] && pkgs+=("$member"); done \
|
||||
< <(members_of "$manifest")
|
||||
if [ ${#pkgs[@]} -eq 0 ]; then
|
||||
printf 'fmt-workspaces: cargo metadata failed for %s\n' "$manifest" >&2
|
||||
# Pass 1: every root's members, before anything is formatted, so that a
|
||||
# package NO root reaches is exit 2 rather than silently never checked.
|
||||
local rc=0 manifest member i m
|
||||
local -a members=() covered=()
|
||||
for i in "${!roots[@]}"; do
|
||||
local rows
|
||||
rows="$(members_of "${roots[$i]}")" || rows=""
|
||||
if [ -z "$rows" ]; then
|
||||
printf 'fmt-workspaces: cargo metadata failed for %s\n' "${roots[$i]}" >&2
|
||||
return 2
|
||||
fi
|
||||
members[$i]="$(printf '%s\n' "$rows" | cut -f1 | tr '\n' ' ')"
|
||||
while IFS= read -r m; do covered+=("$m"); done < <(printf '%s\n' "$rows" | cut -f2)
|
||||
done
|
||||
local orphans
|
||||
orphans="$(manifests_with "$root" package | python3 -c '
|
||||
import os, sys
|
||||
covered = {os.path.realpath(p) for p in sys.argv[1:]}
|
||||
for line in sys.stdin:
|
||||
m = line.rstrip("\n")
|
||||
if m and os.path.realpath(m) not in covered:
|
||||
print(m)
|
||||
' "${covered[@]}")"
|
||||
if [ -n "$orphans" ]; then
|
||||
printf 'fmt-workspaces: no workspace root reaches these packages, so nothing checks them:\n%s\n' \
|
||||
"$orphans" >&2
|
||||
return 2
|
||||
fi
|
||||
|
||||
for i in "${!roots[@]}"; do
|
||||
manifest="${roots[$i]}"
|
||||
local -a pkgs=()
|
||||
read -r -a pkgs <<< "${members[$i]}"
|
||||
|
||||
if [ "$list" = 1 ]; then
|
||||
printf '%s: %s\n' "$manifest" "${pkgs[*]}"
|
||||
|
||||
@@ -357,3 +357,86 @@ fn an_explicit_coordinate_still_beats_the_cursor() {
|
||||
"the named cell survived: {left:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// North=1, South=2, East=3 on the row axis; the cursor starts on North.
|
||||
fn three_cells(scratch: &Scratch, name: &str) -> PathBuf {
|
||||
let doc = three_rows(scratch, name);
|
||||
run_cmd(
|
||||
&[
|
||||
"set-cell 1 Region/North",
|
||||
"set-cell 2 Region/South",
|
||||
"set-cell 3 Region/East",
|
||||
],
|
||||
&doc,
|
||||
);
|
||||
assert_eq!(cell_lines(&doc).len(), 3, "the fixture wrote three cells");
|
||||
doc
|
||||
}
|
||||
|
||||
fn value_of(doc: &Path, item: &str) -> Option<String> {
|
||||
cell_lines(doc)
|
||||
.into_iter()
|
||||
.find(|l| l.contains(&format!("Region={item}")))
|
||||
.and_then(|l| l.rsplit(" = ").next().map(str::to_string))
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/cli.md`.
|
||||
#[test]
|
||||
fn commit_cell_edit_writes_the_named_cell_and_spares_the_cursors() {
|
||||
let scratch = Scratch::new("commit-explicit");
|
||||
let doc = three_cells(&scratch, "commit.improv");
|
||||
|
||||
let out = run_cmd(
|
||||
&["move-selection 1 0 . commit-cell-edit editing 5 Region/East"],
|
||||
&doc,
|
||||
);
|
||||
assert!(out.status.success(), "the line failed: {}", stderr_of(&out));
|
||||
let cells = cell_lines(&doc);
|
||||
assert_eq!(value_of(&doc, "East").as_deref(), Some("5"), "{cells:?}");
|
||||
assert_eq!(
|
||||
value_of(&doc, "South").as_deref(),
|
||||
Some("2"),
|
||||
"the cell under the cursor was touched: {cells:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
value_of(&doc, "North").as_deref(),
|
||||
Some("1"),
|
||||
"the origin cell was touched: {cells:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/cli.md`.
|
||||
#[test]
|
||||
fn a_line_whose_gate_passes_runs_whole() {
|
||||
let scratch = Scratch::new("commit-atomic");
|
||||
let doc = scratch.path("atomic.improv");
|
||||
let out = run_cmd(
|
||||
&["add-category Foo . commit-cell-edit editing 5 Foo/x"],
|
||||
&doc,
|
||||
);
|
||||
assert!(
|
||||
!stderr_of(&out).contains("Parse error"),
|
||||
"a line the gate accepted was refused part-way: {}",
|
||||
stderr_of(&out)
|
||||
);
|
||||
let text = std::fs::read_to_string(&doc).expect("the document");
|
||||
assert!(
|
||||
text.lines()
|
||||
.any(|l| l.contains("Foo=x") && l.ends_with("= 5")),
|
||||
"the commit did not land: {text}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/cli.md`.
|
||||
#[test]
|
||||
fn a_malformed_commit_line_is_refused_whole() {
|
||||
let scratch = Scratch::new("commit-malformed");
|
||||
let doc = seeded(&scratch, "malformed.improv");
|
||||
let out = run_cmd(&["add-category Foo . commit-cell-edit editing 5"], &doc);
|
||||
assert!(!out.status.success(), "a malformed line exited 0");
|
||||
let text = std::fs::read_to_string(&doc).expect("the document");
|
||||
assert!(
|
||||
!text.contains("Foo"),
|
||||
"the first half of a refused line landed: {text}"
|
||||
);
|
||||
}
|
||||
|
||||
+4
-4
@@ -594,12 +594,12 @@ function edit(td, row, col, seed) {
|
||||
});
|
||||
}
|
||||
|
||||
/** A side panel. An empty row list is what says it is closed — the page asks
|
||||
* no question about the mode, exactly as `paintHelp` does not. */
|
||||
/** A side panel. `null` is what says it is closed, and `[]` is open and empty:
|
||||
* two facts, never inferred from a count (improvise-df2w). */
|
||||
function paintPanel(host, rows, build) {
|
||||
host.hidden = rows.length === 0;
|
||||
host.hidden = rows === null;
|
||||
const list = document.createElement("ul");
|
||||
rows.forEach((row, index) => {
|
||||
(rows ?? []).forEach((row, index) => {
|
||||
const li = build(row);
|
||||
li.dataset.index = String(index);
|
||||
list.appendChild(li);
|
||||
|
||||
Reference in New Issue
Block a user