Merge remote-tracking branch 'gh/main' into collapse-fu/eccl
# Conflicts: # context/test-evidence/command.md # context/test-evidence/core.md # crates/improvise-core/src/view/layout.rs
This commit is contained in:
Edward Langley
@@ -37,7 +37,8 @@ it must cross. Two agents adding a workspace member will still collide in
|
||||
**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
|
||||
write, GitHub conventions (including how authors and reviewers answer reviews
|
||||
on the PR thread), 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:
|
||||
|
||||
@@ -77,10 +78,15 @@ defect is a test that passes.
|
||||
- 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.
|
||||
and a test-merge, so it reviews the tree that will exist. Point it at
|
||||
`agent-rules.md`, "GitHub", for what a reviewer reads and answers first.
|
||||
- An ADR PR merges on the user's approval, not on a review:
|
||||
`AGENTS.md`, "ADR pull requests".
|
||||
- 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`.
|
||||
- Merge on GitHub, then `git fetch gh && git merge --ff-only gh/main`. Close
|
||||
the bead only once the merge has landed. That is when the work is done:
|
||||
`AGENTS.md`, "Repository policy over the Beads blocks".
|
||||
- Anything posted to GitHub goes out under the owner's account: open every
|
||||
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
|
||||
|
||||
@@ -29,12 +29,12 @@ not in a second copy.
|
||||
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).
|
||||
6. **The Beads "Session Completion" block near the end of `AGENTS.md` does not
|
||||
set your git policy.** This repository does not opt in to the block's
|
||||
"team-maintainer" profile. On a wave branch that profile's pull and bare
|
||||
`git push` would be refused (rule 4), or worse. Rules 1–5 are your git
|
||||
policy. The block is bd's and bd regenerates it whole, so it is not edited
|
||||
by hand: `AGENTS.md`, "Repository policy over the Beads blocks" (improvise-x1uk).
|
||||
|
||||
## Commit early: a worktree is not durable storage
|
||||
|
||||
@@ -47,14 +47,33 @@ 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 . cargo clippy --workspace --all-targets -- -D warnings
|
||||
direnv exec . cargo test --workspace --no-fail-fast
|
||||
direnv exec . python3 scripts/check-comment-runs.py --self-test
|
||||
direnv exec . python3 scripts/check-comment-runs.py --check-evidence # also answers the comment cap
|
||||
direnv exec . python3 scripts/check-ci-graph.py --self-test
|
||||
direnv exec . python3 scripts/check-ci-graph.py
|
||||
direnv exec . ./scripts/checkers.sh # every suite in scripts/, its own self-test included
|
||||
direnv exec . tpl prolog-sessions/kb.pl -g "(kb_consistent -> halt(0) ; (kb_report, halt(1)))"
|
||||
direnv exec . ./scripts/node-test.sh 'web/tests/*.test.js'
|
||||
```
|
||||
|
||||
This list is the `test`, `js` and `kb` jobs of `.github/workflows/ci.yml`,
|
||||
step for step, and this file is the one place it is written down. Change it in
|
||||
the same PR that changes CI. The other CI jobs are conditional, below, or left
|
||||
out on purpose:
|
||||
|
||||
- **`wasm`**: when you touch a crate it checks (next bullet).
|
||||
- **`harness-lint`**: when you touch `harness/opfs-contract`, run
|
||||
`cargo clippy --target wasm32-unknown-unknown --all-targets -- -D warnings`
|
||||
from that directory. Its format half is already `fmt-workspaces.sh`.
|
||||
- **`build` and `browser`**: when you touch `web/`, `harness/`, or the surface
|
||||
the page calls in `improvise-wasm-client` or `improvise-worker-server`
|
||||
(third bullet).
|
||||
- **`pages-deploy`** is left out. It deploys, and it runs only on a push to
|
||||
`main`. What guards it locally is `check-ci-graph.py`, which is in the list.
|
||||
|
||||
- **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
|
||||
@@ -70,10 +89,17 @@ direnv exec . ./scripts/node-test.sh 'web/tests/*.test.js'
|
||||
- **`--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
|
||||
wasm-client, worker-server) or `improvise-gp`, 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/`.
|
||||
- If you touch `web/`, `harness/` or the page's wasm surface, run
|
||||
`./harness/build.sh`, then
|
||||
`./scripts/node-test.sh 'web/tests/bundle/*.test.js'`, then `npm ci` and
|
||||
`npx playwright test` in `harness/`. The bundle tests sit outside the
|
||||
`web/tests/*.test.js` glob and read the built `web/pkg`, so the standard list
|
||||
never runs them. They failed only in CI once. A fresh worktree has no
|
||||
`harness/node_modules`, and Playwright then reports a missing
|
||||
`@playwright/test`, which reads like a broken checkout.
|
||||
- **If you change the grid's row or column count, also grep
|
||||
`harness/fixtures/*.improv` and `harness/tests/*.spec.js`.** On PR #176 a
|
||||
Playwright spec pinned the old collapsed-group row count. `cargo test` cannot
|
||||
@@ -165,9 +191,19 @@ text for any of these in your hand-back.
|
||||
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.
|
||||
"Shipping a wave".
|
||||
- **As a PR's author, reply on the PR to every review finding you address.**
|
||||
Name the commit that fixes it, or say why you declined it. Putting it only in
|
||||
your hand-back to the orchestrator is not enough: the PR thread is where the
|
||||
next reader looks.
|
||||
- **As a reviewer, read the existing reviews first**, with
|
||||
`./scripts/pr-reviews <n>`, unpiped. Reply on its thread to every Codex
|
||||
review: fixed (name the commit), declined (say why), or already one of our
|
||||
tickets (name it). Every review you post states the head SHA it reviewed.
|
||||
- **Do not merge, and do not close beads.** Your part ends at an open PR plus
|
||||
your hand-back. What "done" means is in `AGENTS.md`, "Repository policy over
|
||||
the Beads blocks". Avoid stacked PRs. If you must stack, never let the base
|
||||
branch be deleted.
|
||||
|
||||
## Hand-back, per ticket
|
||||
|
||||
|
||||
@@ -57,9 +57,9 @@ cp -rf source dest # NOT: cp -r source dest
|
||||
## The Wave Workflow
|
||||
|
||||
Most work here is done by **waves**: an orchestrator chunks the open backlog,
|
||||
fans several agents out into their own git worktrees, merges what comes back,
|
||||
reviews it, and pushes the wave as one PR. This section is what that costs to
|
||||
get right. Everything in it was learned by getting it wrong first.
|
||||
fans several agents out into their own git worktrees, and shepherds each
|
||||
agent's pull request through review to a merge on GitHub. This section is what
|
||||
that costs to get right. Everything in it was learned by getting it wrong first.
|
||||
|
||||
### Who owns what
|
||||
|
||||
@@ -75,11 +75,13 @@ badly:
|
||||
|
||||
Wave agents **read** all three freely (`bd show`, `bd ready`, grep `kb.pl`) and
|
||||
**write** none of them. They report proposed repo-map lines and `kb.pl` facts as
|
||||
*text* in their hand-back, and the orchestrator applies them — batched, at the
|
||||
end of the wave, after the review. **`kb.pl` is written by `scripts/kb add|replace|rm` and by nothing else** — not
|
||||
*text* in their hand-back, and the orchestrator applies them. Repo-map lines
|
||||
are batched at the end of the wave, after the review, because they conflict.
|
||||
`kb.pl` facts are not batched: the orchestrator writes each one when it arrives.
|
||||
**`kb.pl` is written by `scripts/kb add|replace|rm` and by nothing else** — not
|
||||
the Prolog MCP, not an editor. Every write is gated on `kb_consistent` against
|
||||
the candidate, so a bad delta leaves the file untouched. Facts do not need
|
||||
batching: write one when you learn it. There is a `kb` skill; read it before
|
||||
the candidate, so a bad delta leaves the file untouched, which is why a fact
|
||||
can be written the moment it is learned. There is a `kb` skill; read it before
|
||||
your first write. Reading is unaffected — `loadSession` at session start is
|
||||
still how a session gets the model in hand.
|
||||
|
||||
@@ -127,18 +129,21 @@ and watch the check fail.
|
||||
|
||||
### Shipping a wave
|
||||
|
||||
**Do not push `main` to `gh/main`.** Work reaches GitHub as a branch and a pull
|
||||
request, and lands on `main` by being merged there. The orchestrator still
|
||||
merges agent branches into its *local* `main` to integrate and run the gates —
|
||||
that is how conflicts get found early — but the push is a branch push, not a
|
||||
`main` push. `origin` and `gitea` are mirrors and take `main` directly.
|
||||
**Do not push `main` to `gh/main`.** Each agent's work reaches GitHub as its own
|
||||
branch and its own pull request, and lands on `main` by being merged there, once
|
||||
CI is green and an adversarial review agent has passed the PR's head SHA. The
|
||||
orchestrator does not integrate agent branches locally: its `main` only
|
||||
fast-forwards to `gh/main` after a merge. The procedure is
|
||||
`.agents/skills/wave/SKILL.md`, "Shepherding". `origin` and `gitea` are mirrors
|
||||
and take `main` directly.
|
||||
|
||||
**So `origin/main` runs ahead of `gh/main`, and a branch destined for a GitHub
|
||||
PR must merge `gh/main` — never `origin/main`.** This is a trap in this repo's
|
||||
own topology rather than a slip: the two remotes are both spelled `main`, and
|
||||
nothing about `git merge origin/main` looks wrong. The session-completion
|
||||
protocol below literally says `git merge origin/main`, which is right for
|
||||
`main` itself and wrong for a PR branch. It cost a review round on PR #145,
|
||||
**Because the mirrors take `main` directly, `origin/main` can run ahead of
|
||||
`gh/main`, and a branch destined for a GitHub PR must merge `gh/main` — never
|
||||
`origin/main`.** This is a trap in this repo's own topology rather than a slip:
|
||||
the two remotes are both spelled `main`, and nothing about
|
||||
`git merge origin/main` looks wrong. bd's generated Session Completion block
|
||||
once prescribed exactly that command (see "Repository policy over the Beads blocks" below).
|
||||
It cost a review round on PR #145,
|
||||
where four unmerged commits — including another PR's whole diff — were pulled
|
||||
into a branch based on `gh/main` and had to be rebuilt by cherry-picking.
|
||||
ChatGPT Codex caught it; nothing in CI could, because the result compiles and
|
||||
@@ -153,15 +158,13 @@ finishing. It happened in this repo: 27 commits across six agent branches —
|
||||
including a new crate, a new protocol type, and a type moving between crates —
|
||||
went straight to `main` unreviewed, while `AGENTS.md` already said not to. If
|
||||
you notice a batch has bypassed the loop, the cheap repair is a **review-only
|
||||
PR**: base a branch at the last PR merge, open a PR with `main` as the head, let
|
||||
Copilot review the diff, and land the findings as an ordinary PR afterwards. It
|
||||
PR**: base a branch at the last PR merge, open a PR with `main` as the head,
|
||||
have an adversarial review agent review the diff, and land the findings as an
|
||||
ordinary PR afterwards. It
|
||||
cannot be merged — the commits are already in — so close it once its findings
|
||||
are filed or fixed. Rewinding the default branch to fake a mergeable PR is not
|
||||
worth it on a repo with other collaborators.
|
||||
|
||||
One PR per wave, built by a **dedicated PR agent in its own worktree**, so waves
|
||||
pipeline against it: the next wave runs while the PR absorbs review fixes.
|
||||
|
||||
**Read the PR's reviews before merging it.** This is separate from CI and from
|
||||
the adversarial agent, and it has been missed: two ChatGPT Codex reviews landed
|
||||
on merged PRs and were not read until afterwards, one of them a live P2
|
||||
@@ -250,9 +253,20 @@ a PR you have not looked at in a few minutes, rather than after.
|
||||
|
||||
Merge only once a review has passed — an adversarial agent's while Copilot is
|
||||
absent, and it must name the head SHA it reviewed. Prefer a **merge commit** —
|
||||
`AGENTS.md` says merges over rebases, and a wave's commits are individually
|
||||
`AGENTS.md` says merges over rebases, and a PR's commits are individually
|
||||
meaningful.
|
||||
|
||||
### ADR pull requests
|
||||
|
||||
An ADR (`context/adr/`) merges on the user's approval, not on a review. The
|
||||
steps, modelled on PR #122 (ADR 004):
|
||||
|
||||
1. The PR proposes the ADR with its **open questions numbered**.
|
||||
2. The user answers them on the PR.
|
||||
3. The answers are folded into the ADR, in a section headed **"What the user
|
||||
decided"**.
|
||||
4. The PR merges only after the user comments **"Approved"**.
|
||||
|
||||
### Never hand-edit `.beads/issues.jsonl`
|
||||
|
||||
bd manipulates that file and auto-exports to it. Editing it — or even
|
||||
@@ -269,15 +283,37 @@ session before an agent tried one and found nothing. Read the ID back after
|
||||
filing, and before pushing docs run the audit:
|
||||
|
||||
```bash
|
||||
grep -rohE 'improvise-[a-z0-9]{3,8}\b' \
|
||||
git ls-files -z --cached --others --exclude-standard -- \
|
||||
context README.md AGENTS.md CLAUDE.md crates src tests examples web harness docs \
|
||||
.agents .claude \
|
||||
| xargs -0 grep -d skip -ohE 'improvise-[a-z0-9]{3,8}\b' \
|
||||
| sort -u \
|
||||
| grep -vE '^improvise-(core|formula|io|protocol|command|gp|ws|worker|wasm|harness|registry|side|web)$' \
|
||||
| grep -vE '^improvise-(core|formula|io|protocol|command|gp|ws|worker|wasm|harness|registry|report|side|web)$' \
|
||||
> /tmp/bd-audit-ids.txt
|
||||
bd show $(cat /tmp/bd-audit-ids.txt) 2>&1 \
|
||||
| grep -iE 'not found|no such issue' || echo "all resolve"
|
||||
| grep -E '^(Issue [^ ]+ not found|[Nn]o such issue)' || echo "all resolve"
|
||||
```
|
||||
|
||||
`git ls-files` picks the files, and grep only reads them. The file list is the
|
||||
tracked files plus the untracked ones, minus anything git ignores. That leaves
|
||||
out three things a `grep -r` over the same roots reads:
|
||||
|
||||
- **Build output.** After `./harness/build.sh`, `harness/serve/` holds
|
||||
rendered copies of `context/`, and `grep -r` reported three dead ids from them
|
||||
that no tracked file contains (improvise-8haj).
|
||||
- **Other agents' worktrees.** In the orchestrator's checkout
|
||||
`.claude/worktrees/` holds every agent's worktree, dozens of copies of the
|
||||
repository with their unpushed work.
|
||||
- **The skills twice.** `.claude/skills/*` are symlinks to directories under
|
||||
`.agents/skills/`. `-d skip` passes over them, so the skills are read once,
|
||||
through `.agents`.
|
||||
|
||||
Plain `git grep` is not a substitute: on macOS its `-E` silently ignores `\b`
|
||||
and finds a fraction of the ids.
|
||||
|
||||
The last line matches at the start of a line because `bd show` prints each
|
||||
ticket's body indented, and a body can itself contain the words "not found".
|
||||
|
||||
Three things the earlier version of this recipe got wrong, each of which let a
|
||||
real defect through:
|
||||
|
||||
@@ -293,7 +329,8 @@ real defect through:
|
||||
- **The exclusion list named only `core` and `wasm`**, so every other crate
|
||||
name would have been reported missing. The exclusions above also cover the
|
||||
non-ticket tokens this repo really contains: a package name, a temp file
|
||||
name, and the prose word "improvise-side". **They are anchored `^…$` for the
|
||||
name, the prose word "improvise-side", and the example crate name in the
|
||||
`kb` skill's `crate(...)` fact. **They are anchored `^…$` for the
|
||||
same reason the recipe is read as code**: unanchored, the `gp` alternative is
|
||||
a *prefix* match, so a future real ticket whose id happens to begin `gp` is
|
||||
silently excluded and the audit reports nothing while skipping it. An
|
||||
@@ -327,8 +364,32 @@ in progress. Two duplicates were filed in this repo within minutes of the
|
||||
hand-backs that prompted them. `bd search` the core concept *and* scan
|
||||
in-progress issues first.
|
||||
|
||||
## Repository policy over the Beads blocks
|
||||
|
||||
<!-- BEGIN BEADS INTEGRATION v:1 profile:minimal hash:ca08a54f -->
|
||||
Everything between a `BEGIN BEADS …` marker and its `END` is generated by bd,
|
||||
and bd replaces it whole. `bd init` and `bd setup factory|opencode|mux` rewrite
|
||||
the integration block, and the last three call it "stale" and invite the
|
||||
rewrite whenever bd's template has moved on. `bd setup codex` rewrites the
|
||||
Codex block. So a hand edit inside the markers is silently reverted by the next
|
||||
regeneration. Repository policy lives here, outside them:
|
||||
|
||||
- **Work is not done until it is in a pull request and that pull request has
|
||||
merged.** A pushed branch is not done, and neither is a green one. In a wave,
|
||||
the merge is the orchestrator's, after an adversarial review passes the head
|
||||
SHA ("Shipping a wave"). So a wave agent's part ends at an open PR plus its
|
||||
hand-back, and the ticket is done, and its bead closed, only when the merge
|
||||
lands.
|
||||
- **This repository does not opt in to the Beads "team-maintainer" profile.**
|
||||
So the pull (`git pull`, or `git pull --rebase` in bd's "full" profile) and
|
||||
the bare `git push` that the block lists for that profile are not
|
||||
instructions here. Work reaches `main` as "Shipping a wave" describes,
|
||||
and a wave agent pushes with the explicit refspec in
|
||||
`.agents/skills/wave/agent-rules.md`, "Base, branch, push".
|
||||
- **The no-markdown-TODO and no-`MEMORY.md` rules have one exception:**
|
||||
`context/design-principles.md` and `context/repo-map.md` are kept by hand
|
||||
(the repo map by the orchestrator only; see "Who owns what").
|
||||
|
||||
<!-- BEGIN BEADS INTEGRATION v:1 profile:minimal hash:46cd31e7 -->
|
||||
## Beads Issue Tracker
|
||||
|
||||
This project uses **bd (beads)** for issue tracking. Run `bd prime` to see full workflow context and commands.
|
||||
@@ -344,35 +405,44 @@ bd close <id> # Complete work
|
||||
|
||||
### Rules
|
||||
|
||||
- Use `bd` for ALL task tracking — do NOT use TodoWrite, TaskCreate, or markdown TODO lists (the exception being that you must maintain context/design-principles.md and context/repo-map.md)
|
||||
- Use `bd` for ALL task tracking — do NOT use TodoWrite, TaskCreate, or markdown TODO lists
|
||||
- Run `bd prime` for detailed command reference and session close protocol
|
||||
- Use `bd remember` for persistent knowledge — do NOT use MEMORY.md files (the exception being that you must maintain context/design-principles.md and context/repo-map.md)
|
||||
- Use `bd remember` for persistent knowledge — do NOT use MEMORY.md files
|
||||
|
||||
**Architecture in one line:** issues live in a local Dolt DB; sync uses `refs/dolt/data` on your git remote; `.beads/issues.jsonl` is a passive export. See https://github.com/gastownhall/beads/blob/main/docs/core-concepts/sync-concepts.md for details and anti-patterns.
|
||||
|
||||
## Agent Context Profiles
|
||||
|
||||
The managed Beads block is task-tracking guidance, not permission to override repository, user, or orchestrator instructions.
|
||||
|
||||
- **Conservative (default)**: Use `bd` for task tracking. Do not run git commits, git pushes, or Dolt remote sync unless explicitly asked. At handoff, report changed files, validation, and suggested next commands.
|
||||
- **Minimal**: Keep tool instruction files as pointers to `bd prime`; use the same conservative git policy unless active instructions say otherwise.
|
||||
- **Team-maintainer**: Only when the repository explicitly opts in, agents may close beads, run quality gates, commit, and push as part of session close. A current "do not commit" or "do not push" instruction still wins.
|
||||
|
||||
## Session Completion
|
||||
|
||||
**When ending a work session**, you MUST complete ALL steps below. Work is NOT complete until `git push` succeeds.
|
||||
This protocol applies when ending a Beads implementation workflow. It is subordinate to explicit user, repository, and orchestrator instructions.
|
||||
|
||||
**MANDATORY WORKFLOW:**
|
||||
|
||||
1. **File issues for remaining work** - Create issues for anything that needs follow-up
|
||||
1. **File issues for remaining work** - Create beads for anything that needs follow-up
|
||||
2. **Run quality gates** (if code changed) - Tests, linters, builds
|
||||
3. **Update issue status** - Close finished work, update in-progress items
|
||||
4. **PUSH TO REMOTE** - This is MANDATORY:
|
||||
4. **Handle git/sync by active profile**:
|
||||
```bash
|
||||
git merge origin/main
|
||||
# Conservative/minimal/default: report status and proposed commands; wait for approval.
|
||||
git status
|
||||
|
||||
# Team-maintainer opt-in only, unless current instructions forbid it:
|
||||
git pull
|
||||
bd dolt push
|
||||
git push
|
||||
git status # MUST show "up to date with origin"
|
||||
git status
|
||||
```
|
||||
5. **Clean up** - Clear stashes, prune remote branches
|
||||
6. **Verify** - All changes committed AND pushed
|
||||
7. **Hand off** - Provide context for next session
|
||||
5. **Hand off** - Summarize changes, validation, issue status, and any blocked sync/commit/push step
|
||||
|
||||
**CRITICAL RULES:**
|
||||
- Work is NOT complete until `git push` succeeds
|
||||
- NEVER stop before pushing - that leaves work stranded locally
|
||||
- NEVER say "ready to push when you are" - YOU must push
|
||||
- If push fails, resolve and retry until it succeeds
|
||||
**Critical rules:**
|
||||
- Explicit user or orchestrator instructions override this Beads block.
|
||||
- Do not commit or push without clear authority from the active profile or the current user request.
|
||||
- If a required sync or push is blocked, stop and report the exact command and error.
|
||||
<!-- END BEADS INTEGRATION -->
|
||||
|
||||
<!-- BEGIN BEADS CODEX SETUP: generated by bd setup codex -->
|
||||
|
||||
+3
-1
@@ -40,7 +40,9 @@ improvise-command = { path = "crates/improvise-command" }
|
||||
ratatui = "0.30"
|
||||
crossterm = "0.29"
|
||||
serde = { version = "1", features = ["derive"] }
|
||||
serde_json = "1"
|
||||
# Parses JSON a user's numbers cross, so it must read back the f64 written: per crate,
|
||||
# because each build stands alone (improvise-6yvg, context/code/protocol.md).
|
||||
serde_json = { version = "1", features = ["float_roundtrip"] }
|
||||
anyhow = "1"
|
||||
thiserror = "2"
|
||||
indexmap = { version = "2", features = ["serde"] }
|
||||
|
||||
+2
-1
@@ -98,7 +98,8 @@ The message states the **shape** rather than the arity, because
|
||||
## 6 — the help's ex-commands were the unguarded surface
|
||||
|
||||
`help.rs` derives `HelpLine::Keys` rows from the keymap and drops any row whose
|
||||
command does not resolve, so a key row cannot go stale. `HelpLine::Literal` and
|
||||
command does not resolve, so a key row cannot go stale. `HelpLine::Literal` (an
|
||||
ex-command only, since improvise-wk18: built by `ex!`) and
|
||||
`HelpLine::Accent` rows are hand-written prose and had nothing checking them —
|
||||
which is how all three documented spellings of `:formula` came to be ones the
|
||||
parser refuses.
|
||||
|
||||
+170
-4
@@ -935,6 +935,52 @@ There is deliberately **no `dirty = true`** in it: it declares
|
||||
of `MarkDirty` — harmless, and a second home for the one thing that commit set
|
||||
out to give exactly one.
|
||||
|
||||
### `RecordRewrite` — a row's edits composed, not replayed (improvise-uo2p)
|
||||
|
||||
A staged drill edit is a statement about the record **as the snapshot showed
|
||||
it**. It is not a statement about the model as the previous edit left it. The
|
||||
edits in one row do not commute when they are replayed one at a time. A rename
|
||||
moves every cell in the row (gotcha 1). A measure edit replayed after it writes
|
||||
the old key, which leaves a ghost record there and a stale value in the moved
|
||||
record. A second rename replayed after the first finds nothing at the old keys,
|
||||
so it is dropped. `ApplyAndClearDrill` replayed them in `HashMap` order, so the
|
||||
outcome was different from one App to the next. That is gotcha 18's class of
|
||||
bug, found in an effect rather than an index.
|
||||
|
||||
**Sorting is not the fix.** "Measures before renames" repairs the ticket's own
|
||||
case and still drops a second rename every time (rival 1 in the evidence). Any
|
||||
fixed order makes the corruption reproducible instead of removing it. So the
|
||||
rule is **build each row's final record once, then write it**:
|
||||
|
||||
- `RecordRewrite::of(model, row, edits)` collects the row's renames and measure
|
||||
edits into maps keyed by column, and reads each unedited value off the model
|
||||
**before anything is written**. It then states the rewrite: clear every old
|
||||
key, and write every cell at the key with *all* renames applied. A measure
|
||||
edit that is staged replaces the carried value. The result does not depend on
|
||||
the order of the edits. A drill stages at most one edit per `(row, column)`,
|
||||
so no two edits in a row name the same column. A blank rename is refused, and
|
||||
the row's other edits still land at the unrenamed coordinates, as before.
|
||||
- `RecordRewrite::merged` concatenates the rows' rewrites **by phase**: every
|
||||
refusal, then every item added, then every clear, then every write. The clears
|
||||
come first so that a swap is a swap. Rows are taken in snapshot row order
|
||||
(`BTreeMap`), so where two final records land on one cell, **the later row
|
||||
wins**. That is the answer plain records mode gives for the same renames made
|
||||
top to bottom.
|
||||
|
||||
The collision rule is a **choice**, and it still loses the earlier row's values
|
||||
silently. Plain records mode loses them the same way when a rename lands on an
|
||||
existing record. Refusing the colliding rows instead would be a change to both
|
||||
paths.
|
||||
|
||||
**One copy.** The plain records path (`commit.rs::commit_plain_records_edit`)
|
||||
calls the same `RecordRewrite::of` with its one edit, so `rename_record_coordinate`
|
||||
and the drill's `apply_coordinate_rename`/`apply_measure_edit` are gone. This is
|
||||
the consolidation improvise-fdk1 asked for, "regardless" of its wire half. For a
|
||||
single edit the output is the effects the plain path used to emit, reordered from
|
||||
`clear, set` per cell to all clears then all sets, which touches distinct keys.
|
||||
One behaviour moved on the drill side: it used to discard `add_item`'s error, and
|
||||
now goes through `effect::AddItem`, which refuses when the category has gone.
|
||||
|
||||
### `Save` / `SaveAs` abort the batch (improvise-8tk / improvise-lae)
|
||||
|
||||
A failed save aborts the rest of the batch, so the `Quit` that `wq`/`ZZ` chains
|
||||
@@ -2629,8 +2675,10 @@ At bottom-right `EnterAdvance` has no place to go, so it emits a single `AbortCh
|
||||
stages in drill pending edits while in drill mode, or applies directly in plain
|
||||
records mode; for real keys it writes to the model.
|
||||
|
||||
`rename_record_coordinate` renames one coordinate of a records row. In wide format a
|
||||
row is a group of cells — one per measure — so the rename has to move **every** one
|
||||
`commit_plain_records_edit` hands a coordinate or measure edit to
|
||||
`effect::RecordRewrite::of`, the one home it shares with the drill (improvise-uo2p;
|
||||
see `RecordRewrite` above). It replaced `rename_record_coordinate`. In wide format a
|
||||
row is a group of cells — one per measure — so a rename has to move **every** one
|
||||
of them, not just the cell under the cursor (improvise-c5v, gotcha 1). Every cell in
|
||||
the row moves, and a row seeded by `add-record-row` has no measure cells yet: its
|
||||
coordinates live on a lone measure-less cell, which must move too or the row loses
|
||||
@@ -2874,6 +2922,43 @@ flag: tidy-up goes where an abort cannot reach it.
|
||||
stays in `TileSelect` mode so the user can adjust several tiles. `axis: None` cycles;
|
||||
`axis: Some(a)` sets.
|
||||
|
||||
**The tile bar offers `_Index` and `_Dim`, and that is deliberate (improvise-jgzw).**
|
||||
Every tile call site passes `Model::category_names()`, virtual categories included,
|
||||
so tile cursor 0 is `_Index`. The ticket asked whether that was a leak. It is not,
|
||||
for three reasons, each checked rather than argued:
|
||||
|
||||
- **It is a decision the user already made.** improvise-kos: "`_Dim` and `_Index`
|
||||
should work as pivot axes outside Records/Drill view". A category that works as a
|
||||
pivot axis is one the axis picker offers.
|
||||
- **The tile bar is the only way out of a records layout that was saved.**
|
||||
improvise-jgzw's "check what it does" found that hand-assigning `_Index` → Row and
|
||||
`_Dim` → Column in a real view (tile bar or `:set-axis`) is **written** to the
|
||||
`.improv`, because a real view persists every axis, and it reopens as records mode.
|
||||
(`VIRTUAL_VIEW_NAMES`' "never-persisted `_Index`/`_Dim` on axes" is true only of
|
||||
`_Records`/`_Drill`.) In that document, `R` (`toggle-records-mode`) emits
|
||||
`ViewBack` on an empty back stack, so it leaves the layout as it was and sets the
|
||||
status to "Pivot mode". That is filed as improvise-gj5p. Moving the two axes back through
|
||||
the tile bar is what gets out. In the browser, which declines
|
||||
`toggle-records-mode`, the tile bar is also the only way in, and improvise-bd9o
|
||||
already made that route safe instead of closing it
|
||||
(`the_tile_bar_route_into_records_mode_does_not_strand_the_user`).
|
||||
- **What filtering would cost was measured.** With `View::tileable_cats` filtering
|
||||
on `CategoryKind::is_index_or_dim`, that bd9o test and the TUI's
|
||||
`renders_each_tile_with_its_axis_label` fail, as well as the two characterization
|
||||
tests here.
|
||||
|
||||
What the default cursor does cost, stated: one cycle key on tile 0 puts `_Index`
|
||||
on Row alone, and in a pivot view that empties the grid (0 rows on
|
||||
`web/demo.improv`). The status line says `_Index → Row`, and three more presses
|
||||
undo it. If that turns out to matter, the fix is the order the tile bar lists
|
||||
categories in, not taking entries out of it.
|
||||
|
||||
**`_Measure` must stay tileable.** It is on an axis in every shipped document, so
|
||||
`regular_category_names()`, the advice for *prompts*, is the wrong filter here.
|
||||
`the_tile_bar_offers_every_category_by_name_measure_and_records_axes_included`
|
||||
names the whole offered list with `_Measure` on a column, and fails if either
|
||||
decision changes.
|
||||
|
||||
### `cmd/effect_cmds.rs`
|
||||
|
||||
The parseable model-mutation commands are thin `Cmd` wrappers around effects,
|
||||
@@ -3150,8 +3235,8 @@ the cursor**. The result was silent corruption of an unrelated record: appending
|
||||
a row changes `view_frame`'s hash, the frame observation inside `apply_effects`
|
||||
invalidates the cursor to the origin *mid-batch* (gotcha 24), and
|
||||
`EnterEditAtCursor` then pre-fills the editor with row 0's category coordinate.
|
||||
Enter commits through `rename_record_coordinate`, which by gotcha 1 moves every
|
||||
measure cell of that row.
|
||||
Enter commits through `rename_record_coordinate` (since improvise-uo2p,
|
||||
`RecordRewrite::of`), which by gotcha 1 moves every measure cell of that row.
|
||||
|
||||
Two things are worth carrying past this fix.
|
||||
|
||||
@@ -3225,3 +3310,84 @@ key the two questions agree, because a cursor key never carries a None-axis
|
||||
category. The **collapsed-group fold** half of improvise-l4mp is not covered:
|
||||
its key carries the group's name as an ordinary coordinate, so no question about
|
||||
the key alone can see it — it needs `GridLayout::folds_a_group`.
|
||||
|
||||
## A command is a function of its context — the determinism law (improvise-hzjo)
|
||||
|
||||
`cmd/determinism.rs` holds the law the old `law_toggle_group_involution` claimed
|
||||
and could not state. For **every** registered command and every argument list
|
||||
that builds it, the effects `execute` returns must be equal, by `Debug` text,
|
||||
across contexts built **independently from equal data**: four `App`s, each
|
||||
parsed from `web/demo.improv` and given the same buffers, expanded categories,
|
||||
yank, panels and cursor.
|
||||
|
||||
**Why the instances must be distinct.** The old test called `execute` twice
|
||||
against one `CmdContext`. Every `HashMap` it reached was the same instance with
|
||||
the same `RandomState`, so it iterated the same way both times: `f(x) == f(x)`,
|
||||
which nothing can fail. Separately built `App`s give every map its own seed. So a
|
||||
command whose effects follow hash iteration order disagrees with itself. Hash
|
||||
order is the one kind of nondeterminism this repo has actually shipped (gotcha
|
||||
18). This law can see it where a command's `execute` builds it, as the drill
|
||||
snapshot's record order was. It cannot see the other shipped instance, the
|
||||
analyzer's `HashSet`: `analyze_records` is reached only from inside
|
||||
`Effect::apply`, which is outside this law (see below). The companion test
|
||||
`only_distinct_instances_can_see_a_command_that_follows_hash_order` keeps that
|
||||
property checked: a test-only command that emits `ctx.buffers` in iteration
|
||||
order must agree with itself on one context and disagree across two.
|
||||
|
||||
**Four instances, not two.** A rival that takes *the first* entry of a map, not
|
||||
all of them, agrees across two seeds whenever both happen to put the same entry
|
||||
first. With 21 buffers that is roughly one chance in 21, and three comparisons
|
||||
make it negligible. The fixture's values are also chosen to stay distinct under
|
||||
the edits commands make. A first version used `v0`…`v15`, which `pop()` collapses
|
||||
to `v`, and so it let `command-mode-backspace` reading an arbitrary buffer pass.
|
||||
|
||||
**The store needs several records to order, so the drills come from the
|
||||
document (improvise-e1sm).** The registry's `candidate_args` are extended with
|
||||
names the demo really has, because coordinates that match nothing never reach
|
||||
the records a drill or a fit would order. A hand-picked name is not enough. The
|
||||
first version's only real drill was `Product/Latte, Quarter/Q2`: one record, two
|
||||
cells. With the store's sort removed, two contexts put those two cells in the
|
||||
same order about half the time, and with three comparisons the law passed about
|
||||
one run in eight (measured: 46 of 400). So the arguments now include **every
|
||||
item of every regular category as a one-coordinate drill**, read off the parsed
|
||||
document. A one-coordinate drill spans every other category whole, so it matches
|
||||
the whole slice of records under that item, not one record picked by name. In
|
||||
the demo that is four records and eight cells for each of the eight drills. The
|
||||
test
|
||||
also asserts that the widest candidate it actually passes matches at least four
|
||||
records. That guard catches a demo with too little data, and it catches the
|
||||
derived drills being dropped from the argument list. It does **not** catch the
|
||||
comparison losing its wide drills some other way, because it measures what the
|
||||
argument lists match, not what the built commands ordered (improvise-b9f2). The
|
||||
review showed this with a rival: a `drill-into-cell` parser that refuses fewer
|
||||
than two arguments sends every one-coordinate drill to the one-record cursor
|
||||
fallback. The guard still reports four records, and with the store's sort
|
||||
removed the law passed 38 of 300 runs. With the store's sort removed and the
|
||||
parser intact, the law fails 400 of 400 runs.
|
||||
|
||||
**What it cannot see.** Only commands and arguments the fixture reaches. A
|
||||
command whose nondeterminism needs some other state, such as records mode, a
|
||||
wizard or a drill in progress, is outside this fixture. And `Debug` equality is
|
||||
text equality: two effects that differ only in something their `Debug` omits are
|
||||
equal here.
|
||||
|
||||
**Its scope is what `execute` returns, not what `Effect::apply` does.** The law
|
||||
compares the effects a command hands back. Many effects are unit structs that
|
||||
decide all their work inside `apply`: `ApplyAndClearDrill`,
|
||||
`SortData`, `CleanEmptyRecords`, `Save`, `RebuildLayout`, `AcceptWizardModel`.
|
||||
Hash-order iteration inside an `apply` is therefore invisible here, however
|
||||
often the law runs. improvise-uo2p is the worked example.
|
||||
`ApplyAndClearDrill::apply` walks `drill.pending_edits`, a `HashMap`, in hash
|
||||
order. A measure edit and a category rename staged on one row can then write a
|
||||
ghost record. The command that emits it passes this law, because the effect's
|
||||
`Debug` is just its name. So "the one kind of nondeterminism this repo has
|
||||
shipped" is guarded here only on the `execute` side. The `apply` side needs its
|
||||
own test per effect, or a law over `App` state after `apply_effects`.
|
||||
|
||||
**The exemption list.** `OUTSIDE_THE_CONTEXT` names commands that legitimately
|
||||
depend on something outside the context, such as a clock or an entropy source,
|
||||
each with its reason. It is empty: `fit-measure`'s seed is an argument with a
|
||||
fixed default, not a clock read. The list is checked in both directions. An
|
||||
unlisted command that diverges fails the test, and so does a listed one that
|
||||
does not diverge. A stale entry therefore cannot linger, and a new registration
|
||||
is covered the day it lands.
|
||||
|
||||
+77
-27
@@ -756,28 +756,54 @@ coming back one ULP off, both in `CellDisplay::value` (host to client, so `yy`
|
||||
copied a neighbour) and in a `SetCell` (client to host, so a typed or pasted
|
||||
number landed as a neighbour).
|
||||
|
||||
The feature is on for **every crate that parses a frame**: `improvise-ws-server`
|
||||
(`transport.rs`), `improvise-worker-server` (`worker.rs`) and
|
||||
`improvise-wasm-client` (`socket.rs`). Each crate's own manifest names it,
|
||||
because each browser bundle is built from one crate alone. The `wasm` CI job
|
||||
and `wasm-pack` both invoke per crate, and feature unification cannot reach
|
||||
across separate invocations (`context/repo-map.md`, `## Wasm`). Enabling it in
|
||||
one crate and relying on the others to inherit it works in `cargo test` and
|
||||
nowhere else.
|
||||
The feature is on for **every workspace crate with a normal dependency on
|
||||
serde_json** (improvise-dz8c). Each crate's own manifest names it, because each
|
||||
build is made from one crate alone: the `wasm` CI job and `wasm-pack` invoke
|
||||
per crate, and feature unification cannot reach across separate invocations
|
||||
(`context/repo-map.md`, `## Wasm`). Enabling it in one crate and relying on the
|
||||
others to inherit it works in `cargo test` and nowhere else.
|
||||
|
||||
That is also why the end-to-end tests cannot guard this alone. They run in one
|
||||
unified build with `improvise-ws-server` as a dev-dependency, so they stay
|
||||
green with the feature removed from the browser bundle.
|
||||
`every_build_that_parses_a_frame_enables_float_roundtrip` asks `cargo tree`
|
||||
per crate over normal edges instead. A new crate that parses frames belongs in
|
||||
its list.
|
||||
**Why "can parse" and not "parses a frame".** The first version of the rule
|
||||
covered only the three crates that parse frames and listed them by hand.
|
||||
Frames are not the only place a user's number crosses a JSON parser.
|
||||
`improvise-io`'s legacy-JSON `.improv` load, `improvise-command`'s JSON import
|
||||
(`StartImportWizard`, `ImportJsonHeadless`) and the root binary's
|
||||
`improvise import` all parsed with the lossy default, and in the TUI binary
|
||||
nothing enabled the feature (improvise-6yvg, reproduced: `0.24815714285714285`
|
||||
stored as `…283`). The worker bundle happened to be exact through unification,
|
||||
so one import gave different numbers in the two front ends. "Parses JSON input"
|
||||
cannot be read off a manifest, but "has a normal serde_json dependency" can, and
|
||||
it over-approximates in the cheap direction: the feature changes only parsing,
|
||||
so a crate that only serializes pays nothing for it. Dev-dependencies are out,
|
||||
because a test build ships nowhere. That also leaves out `gp-tree`, whose only
|
||||
serde_json is a dev-dependency (its example CLI reads a saved model). Every
|
||||
number it persists is an `f32`, and 3M random `f32`s all read back exactly
|
||||
through the lossy parser, against about 30% of random `f64`s.
|
||||
|
||||
Not covered, and filed separately rather than widened into this change
|
||||
(improvise-6yvg): `improvise-io`'s JSON import and `improvise-command`'s JSON
|
||||
reads also parse numbers with the default parser. They are file formats, not
|
||||
frames. In the TUI binary nothing enables the feature. In the worker bundle
|
||||
unification happens to reach them, so one import can give different numbers
|
||||
in the two front ends.
|
||||
**Why a declaration and not the build graph.** The first check asked
|
||||
`cargo tree` whether each listed crate's *build* had the feature. That answer
|
||||
can be yes by accident. `improvise-command` depends on `improvise-io`, so with
|
||||
the feature removed from command's manifest, command's own test build still
|
||||
has it, and its import test stays green. The rule requires the crate's own
|
||||
manifest to say it, so a later change to a dependency cannot take it away.
|
||||
|
||||
**Why not enable it once, in `improvise-protocol`.** That removes the list, but
|
||||
it would give the protocol a normal serde_json dependency it has no use for
|
||||
(its repo-map entry says core and serde only). It would also reach only builds
|
||||
that include the protocol, and `improvise-io` does not depend on it. Under this
|
||||
rule the protocol's one serde_json dependency, the optional `test-support` one,
|
||||
carries the feature too. That is off in every shipped build, so it costs
|
||||
nothing.
|
||||
|
||||
`every_crate_that_can_parse_json_enables_float_roundtrip` (`tests/json_numbers.rs`)
|
||||
reads `cargo metadata --no-deps` and names every package, and every
|
||||
target-specific entry, whose normal serde_json dependency lacks the feature. A
|
||||
new crate is covered the day it declares the dependency. It does not reach
|
||||
`harness/opfs-contract`, which is its own workspace and only serializes.
|
||||
|
||||
The end-to-end tests cannot guard this alone. They run in one unified build
|
||||
with `improvise-ws-server` as a dev-dependency, so they stay green with the
|
||||
feature removed from the browser bundle.
|
||||
|
||||
### `AxisFrame` — the grid's frame, and why it is *one* type
|
||||
|
||||
@@ -2064,14 +2090,38 @@ nothing in the table to look up, and its hint is a hand-written tail. That is a
|
||||
real hole and it is where it is visible, not papered over: a keymap cannot
|
||||
describe keys a command resolves privately.
|
||||
|
||||
**`gg`, `yy`, `ZZ` and the ex-commands.** A prefix sequence is two addresses and
|
||||
not one binding (gotcha 13), and `:q`/`:w` are parsed text rather than keys.
|
||||
Both are `HelpLine::Literal`, which says out loud that the row is spelled rather
|
||||
than derived.
|
||||
**The ex-commands.** `:q`/`:w` are parsed text rather than keys, so they are
|
||||
`HelpLine::Literal` — built only by the `ex!` macro, whose leading colon is its
|
||||
own — and the parser, not the keymap, is what checks them
|
||||
(`every_ex_command_the_help_pages_show_is_one_the_parser_accepts`).
|
||||
|
||||
**The captions.** `i:edit` where `i` entered records mode would still pass every
|
||||
test here. A caption is English beside a command name and only a reader judges
|
||||
it — the same limit gotcha 14 recorded for the old hint test.
|
||||
This paragraph used to say the same of `gg`, `yy` and `ZZ`: "a prefix sequence
|
||||
is two addresses and not one binding". Two addresses is true and was no reason
|
||||
to spell them by hand — `Keymap::prefixed_keys_running` looks the pair up the
|
||||
way `keys_running` looks up one key, and `keys()` asks both (improvise-wk18). A
|
||||
key string with no command name was also the one row no guard could reach.
|
||||
|
||||
**Every row the pages ask for is visible to a guard.** The page builders take
|
||||
the `keys` lookup as a parameter; `pages()` passes the real one, and
|
||||
`help_bindings()` (tests only) passes one that records `(mode, command, args,
|
||||
description)`. So a check over the help overlay reads the *same builders* the
|
||||
overlay is drawn from, never a list kept beside them. That premise is a type,
|
||||
not a convention (improvise-v29m): `HelpLine::Keys` carries a `KeyRow` whose
|
||||
only constructor does the lookup, so a builder cannot type a key row the
|
||||
recorder would never see; and `ex!` refuses at compile time any usage with a
|
||||
bare word after the command name, so an ex row cannot smuggle a key either.
|
||||
`resolve::tests::every_advertised_command_answers_in_its_own_mode` walks it
|
||||
beside `advertised`, which is how the browser's black holes on the help page
|
||||
became visible (improvise-uln5).
|
||||
|
||||
**The captions.** `i:edit` where `i` entered records mode would still pass most
|
||||
tests here: a caption is English beside a command name and only a reader judges
|
||||
it — the same limit gotcha 14 recorded for the old hint test. Two checks now
|
||||
bite at its edges. The browser guard requires an advertised key to *act*, so a
|
||||
key whose caption promises an action and which only refuses must be named
|
||||
(improvise-3t5x); and `only_the_edit_key_is_captioned_as_editing_the_cell` asks
|
||||
by command that no other Normal row claims to edit (improvise-jhdf). A false
|
||||
caption in words neither looks for still passes.
|
||||
|
||||
### The browser half
|
||||
|
||||
|
||||
@@ -311,10 +311,9 @@ candidate for the store.
|
||||
|
||||
- **And an argument must not live only in a commit message.** A message is
|
||||
write-once and unauditable: it cannot be corrected without rewriting merged
|
||||
history, and no check in this repo can read one — the ID audit greps
|
||||
`context`, `README.md`, `AGENTS.md`, `CLAUDE.md`, `crates`, `src`, `tests`,
|
||||
`examples`, `web`, `harness` and `docs`, and a commit message is in none of
|
||||
them. So a reason recorded only there is a reason nothing can ever find wrong.
|
||||
history, and no check in this repo can read one — the ID audit
|
||||
(`AGENTS.md`, "Cite a ticket ID you have read back") greps files in the
|
||||
tree, and a commit message is in none of them. So a reason recorded only there is a reason nothing can ever find wrong.
|
||||
When one turns out to be wrong, correct `context/` and say in `context/` that
|
||||
the message is superseded; that is the fix rather than a compromise. PR #51's
|
||||
`7d25762` is the worked example, corrected by #56.
|
||||
|
||||
@@ -1458,6 +1458,17 @@ 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.
|
||||
|
||||
**improvise-jhdf closed the paraphrase, and the paragraph above is what it
|
||||
found.** Rewording Enter's caption to "Edit the cell (same as i)" — the same lie
|
||||
in new words — passed both help tests. The test now asks by **command**, which
|
||||
wk18's `help::help_bindings()` made possible: every Normal-mode help row whose
|
||||
caption says "edit" in any case must run `edit-or-drill`. It fails on that
|
||||
rival naming `(Normal, "enter-advance", None, "Edit the cell (same as i)")`.
|
||||
Two controls stop it passing by looking away: `edit-or-drill` and
|
||||
`enter-advance` must each still have a Normal row, or the filter has nothing to
|
||||
catch. What it still cannot see is a false caption that avoids the word — "Opens
|
||||
the cell" — which is the honest limit of any check over prose.
|
||||
|
||||
### a_view_row_click_focuses_moves_and_switches_to_the_row_clicked
|
||||
|
||||
improvise-iioi, and it is the **same trap** as the category click one level
|
||||
@@ -2071,6 +2082,50 @@ mode `declined` refuses at the door, so a browser never stands in front of the
|
||||
hint; the other two are improvise-gcvb's records-cell commit, which needs
|
||||
`record_col`.
|
||||
|
||||
**improvise-3t5x and improvise-wk18 widened it again, in two directions, and the
|
||||
list above is superseded by the one in the test.**
|
||||
|
||||
*What "answers" means.* It counted any `Local` as answered, so a lone
|
||||
`SetStatus` refusal passed — which is why improvise-s8h7's false "Edit current
|
||||
cell" stayed green. Injected to show it: make `edit_or_drill`'s `Record` arm
|
||||
return only `SetStatus("RIVAL")`, so `i` refuses on every cell — the old guard
|
||||
**passed**. The line drawn now: a caption is a promise, so an advertised key
|
||||
must *act*. A refusal is legitimate — a feature a browser honestly lacks should
|
||||
say so rather than go silent — but it is not what the caption says, so it has to
|
||||
be **named** in the list with its reason, like a black hole. The test therefore
|
||||
sorts every key into three outcomes, `Acts`, `Declines` (said something, did
|
||||
nothing) and `Silent`, and lists the last two. The same injection now fails
|
||||
naming `NORMAL: a/i edit-or-drill Declines`.
|
||||
|
||||
*Judged by what moved, not by the variant.* The first version of the new
|
||||
classifier called anything but an all-status `Local` an action, and a rival that
|
||||
unhooked `jump-first-row` from `composed` **passed** — because a chord's second
|
||||
key always returns `Local([SetTransientPrefix(None), ..])`, the disarm, even
|
||||
when nothing else happens. That is the laundering this entry already warns
|
||||
about, one level down: `KeyOutcome::of` launders an empty vec into `Ignored`,
|
||||
and `chord` launders a no-op into a `Local`. The disarm is now ignored when
|
||||
deciding whether a key did anything, and the same rival fails naming `NORMAL:
|
||||
gg jump-first-row Silent`.
|
||||
|
||||
*Which keys.* It read `help::advertised`, the status-bar table, and the help
|
||||
overlay is a second body of advertising the browser also paints
|
||||
(improvise-uln5). It now also reads `help::help_bindings()`, which records every
|
||||
keys row the page builders ask for — the same builders `pages()` renders, so the
|
||||
two cannot drift — including prefix sequences (`gg`, `yy`, `ZZ`), which it
|
||||
presses by arming the prefix first. That is what added the five host-only
|
||||
black holes (`/`, `n`, `H`, `z`, Normal's `Enter`) to the list: each was already
|
||||
in `the_command_names_that_still_need_the_host_are_these`, and the help page
|
||||
advertised it anyway.
|
||||
|
||||
*Which cells.* `furnished` stood on a record cell only, so `i` on an aggregate
|
||||
(s8h7's whole subject) was outside its reach. Normal mode is now pressed on a
|
||||
record, an aggregate and a derived cell, and a non-record cell is reported only
|
||||
where it changes the answer, so the list does not triple. `furnished_on` also
|
||||
carries a history in both directions and a yanked value, so `<`, `,` and `p`
|
||||
are not refused by an empty stack — state refusals are correct behaviour and
|
||||
not this test's subject, the same argument that gave `furnished` three of
|
||||
everything.
|
||||
|
||||
### a_locally_committed_creating_column_agrees_with_the_host
|
||||
|
||||
improvise-gcvb. `resolve`'s commit returned `None` for the whole of records
|
||||
@@ -3341,9 +3396,22 @@ over `Ctrl-R` is in `context/code/tui.md`), the test is renamed, and the second
|
||||
half now asserts the binding **exists** — with `view-back` kept as the control,
|
||||
which now guards the opposite failure: a table that binds everything, or a
|
||||
sweep that reports names it never read. A third assertion pins the key itself,
|
||||
`composed(&state, ev(Char(',')), "view-forward", &[]) == [Command::ViewForward]`,
|
||||
so the binding cannot drift onto some other key while the presence check stays
|
||||
happy.
|
||||
`resolve(&state, ev(Char(','))) == Local([Command::ViewForward])`, so the
|
||||
binding cannot drift onto some other key while the presence check stays happy.
|
||||
|
||||
**That third assertion was vacuous as first written, and the paragraph above
|
||||
said it was not** (improvise-3ij0). It called `composed(&state, ev(Char(',')),
|
||||
"view-forward", &[])`, and `composed` dispatches on the command **name**: the
|
||||
`view-forward` arm never reads the event, so the `,` was decoration and the
|
||||
assertion restated the first half of the test. Rebinding `view-forward` from `,`
|
||||
to `~` in `keymap.rs` left it green; the key was pinned only by
|
||||
`ui::app::tests::the_view_history_walks_both_ways_from_the_keyboard`, in another
|
||||
crate. It now goes through `resolve`, which looks the key up in the shared
|
||||
keymap before anything reads a name — so the rival fails **here**, at this
|
||||
assertion (resolve.rs, "and the key is `,`"), and the pre-fix code under the
|
||||
same rival was run and passes. The general shape: a helper that takes a key
|
||||
*and* a name, and dispatches on the name, cannot pin the key; only the lookup
|
||||
can.
|
||||
|
||||
**The rival: bind `,` to `view-back` instead** — the misspelling the original
|
||||
control was written against, and the one a copy-paste actually produces. It
|
||||
@@ -4522,21 +4590,78 @@ before #178 too, as the review noted.
|
||||
**Fault injected:** the same pre-fix tree. Fails: the host stored
|
||||
`4598108823619341467` in South/Cost.
|
||||
|
||||
### `every_build_that_parses_a_frame_enables_float_roundtrip`
|
||||
**Retired: `every_build_that_parses_a_frame_enables_float_roundtrip`.** It asked
|
||||
`cargo tree` about three crates listed by hand. improvise-dz8c replaced it with
|
||||
`every_crate_that_can_parse_json_enables_float_roundtrip`, whose entry is in
|
||||
`tooling.md`. The injections recorded for it still hold: with the feature
|
||||
removed from `improvise-wasm-client` alone, the two tests above stay green, so
|
||||
they cannot guard the bundle alone.
|
||||
|
||||
The two tests above run in **one** feature-unified test build, which includes
|
||||
`improvise-ws-server` and `improvise-command` as dev-dependencies. So they pass
|
||||
whenever **any** crate in that build enables the feature. The shipped browser
|
||||
bundle is built from one crate alone, so it would still be lossy. That was
|
||||
demonstrated: with the feature removed from `improvise-wasm-client` only, both
|
||||
tests stayed green while `cargo tree` showed no `float_roundtrip` in the wasm
|
||||
bundle. This test asks cargo instead, once per crate that parses a frame:
|
||||
`cargo tree --locked --edges normal,features --invert serde_json --package
|
||||
<crate>`, with `--target wasm32-unknown-unknown` for the two bundles. It uses
|
||||
normal edges only, because a dev edge would unify the feature back in
|
||||
(`context/repo-map.md`, `## Wasm`).
|
||||
### resolve::tests::every_grid_move_lands_where_its_binding_says
|
||||
|
||||
**Faults injected:** the feature removed from `improvise-wasm-client` alone,
|
||||
which fails naming that crate while the two tests above pass. Then removed from
|
||||
`improvise-ws-server` alone, which fails naming it. The native path runs
|
||||
without `--target`, so the two arms are different invocations.
|
||||
improvise-6562, found reviewing PR #180 at 949810c, and it is the same
|
||||
laundering as the disarm, one level over. `heard()` said it judged by what moved
|
||||
and in fact judged by command **type**: any `Local` command besides a status or a
|
||||
disarm counted as `Acts`. The fixture sat at every edge — a 2x1 grid with the
|
||||
cursor at (0,0), one tile, help page 0 — so about twenty movement keys that
|
||||
clamp to nothing were counted as acting. Rebinding `jump-last-col` to
|
||||
`GridMove::ToStart(false)`, so `$` and `End` go to the *first* column, left the
|
||||
whole workspace green.
|
||||
|
||||
Two changes, because either alone is not enough. `heard()` now **folds** the
|
||||
outcome and compares the view slice before and after (status and disarm aside),
|
||||
counting a `Command::Model` as sent and a `Request` as an act; and `furnished_on`
|
||||
stands on a 3x3 grid with the cursor at (1,1), three tiles with the cursor on
|
||||
the middle one, a page axis on its middle item and help page 1, so no move is
|
||||
clamped by an edge. Injecting `jump-last-col` as a `SetSelected` to where the
|
||||
cursor already is now fails `every_advertised_command_answers_in_its_own_mode`
|
||||
naming `NORMAL: $/End jump-last-col Silent`.
|
||||
|
||||
**The reviewer's own rival still passes that guard, and should.** From (1,1),
|
||||
`ToStart(false)` moves to (1,0) — the view moved, so the key "acts". No
|
||||
did-anything-happen check can see *direction*; only an expected destination can.
|
||||
This test supplies one without a second table: a `move-selection` binding's
|
||||
expected landing is read off its own args (the deltas), and the four jumps are
|
||||
named by what they are called. Every key that runs one is pressed from the
|
||||
centre — fourteen of them, with a floor so a blind walk fails. Under the rival
|
||||
it fails naming `End` and `$`. The general point: the fixture that made the
|
||||
laundering invisible (the edge) is also the one that makes a wrong-direction
|
||||
jump visible, so moving off the edge fixed one blind spot and opened the other,
|
||||
and each needs its own question.
|
||||
|
||||
**improvise-w7qd, and the floor was the hole.** As first written the test
|
||||
covered `move-selection` and a four-entry jump table, behind `pressed >= 14`.
|
||||
Two things passed the whole workspace, both found in re-review of #180 at
|
||||
544744c: negating the `scroll-rows` and `page-scroll` arms, so `Ctrl-d`/`Ctrl-u`
|
||||
and `PgDn`/`PgUp` all scroll the wrong way (advertised, and still "acting" from
|
||||
the centre); and dropping the jump table *with* `$` sent to the first column,
|
||||
because the sixteen `move-selection` sequences met the floor on their own. The
|
||||
test now walks **every** command Normal binds. `scroll-rows` and `page-scroll`
|
||||
state their landing from their own argument exactly as `move-selection` does —
|
||||
from the centre of a 3x3 grid any step of at least one lands at `1 + d`
|
||||
clamped, so a sign is visible whatever the page size — and a command that moves
|
||||
the cursor (in the same view) without a stated landing **fails** rather than
|
||||
being skipped. So a dropped table entry is caught by derivation, not by a count:
|
||||
under the second rival it fails naming every jump as moving with nothing stating
|
||||
where. The first rival fails on `PgUp` (`page-scroll ["-1"]`) and, alone, on
|
||||
`Ctrl-u` (`scroll-rows ["-5"]`). A last assertion that each kind was pressed at
|
||||
least once guards the other direction — a binding vanishing — and is a named
|
||||
list on purpose, since that is the one thing no walk over bindings can see.
|
||||
|
||||
**improvise-5kdr: the 3x3 fixture saw only a sign.** From the centre of a 3x3
|
||||
grid every step of one or more clamps to an edge, so the w7qd version stated
|
||||
`page-scroll` as `1 + d` — one row, which is not what it does — and passed
|
||||
because the true landing clamped to the same edge. Measured on #198 at 8772db2:
|
||||
`page-scroll` ignoring the page size together with `scroll-rows` moving one row,
|
||||
or `move-selection` moving `2d`, left the test green. The walk now stands at
|
||||
(10,10) of a 21x21 grid with an 8x8 window, far from every edge, so a landing
|
||||
shows magnitude: `10+d` for `move-selection` and `scroll-rows`, `10 + page·d`
|
||||
for `page-scroll`, 0/20 for the jumps. The page size is not written into the
|
||||
test: `grid_move`'s page rule moved into `page_rows(window_rows)` and the test
|
||||
asks it, so the rule has one home. Rivals, each run: `move-selection` doubled
|
||||
fails on `Up` (`move-selection ["-1","0"]`); the page-size-and-one-row pair
|
||||
fails on `PgUp` (`page-scroll ["-1"]`); the one-row `scroll-rows` alone fails on
|
||||
`Ctrl-u` (`scroll-rows ["-5"]`); the jump-table rival still fails naming every
|
||||
jump. The residual, stated: because the test reads `page_rows`, a change to the
|
||||
page rule itself moves the test with it — whether that rule is the TUI's is
|
||||
`tests/lockstep.rs`'s question, not this one's.
|
||||
|
||||
@@ -5314,6 +5314,167 @@ before `refuse_and_stop` was applied to its arms. For `load`, the injected fault
|
||||
is removing its abort (`refuse_and_stop` back to `app.refuse`), and the test
|
||||
then fails with "`load absent.improv . q` quit having loaded nothing".
|
||||
|
||||
### ui::effect::tests::a_drill_rows_measure_edits_follow_its_rename
|
||||
|
||||
improvise-uo2p. `ApplyAndClearDrill` walked `pending_edits`, a `HashMap`, and
|
||||
applied each edit alone against the model as the previous one left it. A rename
|
||||
moves the row's cells. A measure edit then wrote `row.cell_key(measure)`, the
|
||||
snapshot's old key. So when the rename came first, a ghost record appeared at the
|
||||
old coordinates carrying the new value, and the moved record kept the stale one.
|
||||
The reviewer saw this in 16 of 40 fresh Apps. The fixture stages two measure edits
|
||||
and a rename on drill row 1, so a wrong answer needs only one of the three orders
|
||||
that put the rename last. Each assertion runs over 32 fresh Apps and compares the
|
||||
**whole** store, so the ghost and the stale value are both visible. Before the
|
||||
fix it failed on five of five runs. Rival 3 (the rewrite addresses measure edits
|
||||
at the unrenamed key) fails it.
|
||||
|
||||
### ui::effect::tests::a_drill_row_renamed_twice_and_edited_lands_as_one_record
|
||||
|
||||
improvise-uo2p. This is the case that fails **every** time, not with some
|
||||
probability. Each rename read the snapshot's old keys. The second rename found
|
||||
nothing there, because the first had moved it, so one rename was always dropped
|
||||
and the row split into a `Clothing/Feb` record and a `Drink/Jan` record. Rival 1
|
||||
is the ticket's proposed fix direction: measures before renames, each edit
|
||||
applied alone. It passes the ticket's own case and fails here. So "a fixed
|
||||
order" is not a fix; the renames have to be composed.
|
||||
|
||||
### ui::effect::tests::a_drill_that_swaps_two_records_coordinates_swaps_them
|
||||
|
||||
improvise-uo2p. Row 0 is renamed onto row 1's coordinates and row 1 onto row 0's.
|
||||
If the rows are applied in sequence, the first move overwrites the other record
|
||||
and the second move carries the overwritten values back, so one record's data
|
||||
is lost in either order. Rival 2 composes each row correctly but writes it
|
||||
before reading the next row, and it fails here and nowhere else. That is why
|
||||
`RecordRewrite::merged` reads every row off the model first, then clears every
|
||||
old key, then writes.
|
||||
|
||||
### ui::effect::tests::a_drill_edit_addresses_its_own_record_not_the_one_renamed_onto_its_coordinates
|
||||
|
||||
improvise-uo2p. Row 1's staged `Cost` belongs to row 1's record, even though row
|
||||
0 is being renamed away from coordinates and row 1 onto them. Before the fix,
|
||||
some hash orders left `Clothing/Jan/Cost = 999` as a ghost. Rival 3 fails it.
|
||||
|
||||
### ui::effect::tests::two_drill_rows_renamed_onto_one_record_resolve_to_the_later_row
|
||||
|
||||
improvise-uo2p. Two rows renamed onto the same coordinates collide per cell. The
|
||||
rule chosen is that the later snapshot row wins, which is what plain records
|
||||
mode gives when the same renames are made top to bottom. Before the fix the
|
||||
winner was the hash order's choice. Rival 4 reverses the row order in `merged`
|
||||
and fails here. The rule is a choice, and it still loses data silently. See
|
||||
`context/code/command.md`, `RecordRewrite`.
|
||||
|
||||
### ui::effect::tests::a_record_rewrite_is_the_same_under_every_order_of_its_edits
|
||||
|
||||
improvise-uo2p. The composition is order-independent **by construction**, so it
|
||||
is asserted over all 24 orders of four edits on one row: two renames, a measure
|
||||
write and a measure clear. The test pins one value as well as the equality, so
|
||||
a rewrite that is the same wrong answer in every order still fails. Rival 3
|
||||
fails it.
|
||||
|
||||
## `command/cmd/tile.rs` — the tile bar's offered set, by name (improvise-jgzw)
|
||||
|
||||
### the_tile_bar_offers_every_category_by_name_measure_and_records_axes_included
|
||||
|
||||
This test pins a decision, not a bug. The argument is in `context/code/command.md`
|
||||
under `cmd/tile.rs`. Before this test, no tile assertion named the whole offered
|
||||
set. The two older ones named only the category at cursor 0, so a filter that
|
||||
changed what the bar offers past the cursor was invisible. The fixture puts
|
||||
`_Measure` on **Column**, as every shipped document does. The test then walks the
|
||||
cursor past the end and asserts the exact list
|
||||
`[_Index, _Dim, _Measure, Type, Month]`.
|
||||
|
||||
Rivals, injected, full workspace run:
|
||||
|
||||
- **`regular_category_names()` in `TileAxisOp`**, the obvious "no virtual
|
||||
categories in pickers" fix, which drops `_Measure` too. It fails this test
|
||||
(`["Type", "Month"]`), the two characterization tests above it, and
|
||||
`lockstep.rs::a_locally_resolved_tile_axis_change_agrees_with_the_host`.
|
||||
- **`View::tileable_cats` filtering `is_index_or_dim`**, the ticket's option (b).
|
||||
With the list asserted this test fails by construction, so the point is what
|
||||
*else* fails: `renders_each_tile_with_its_axis_label` (TUI) and
|
||||
`the_tile_bar_route_into_records_mode_does_not_strand_the_user`. The second one
|
||||
is improvise-bd9o's own test of the route this decision keeps.
|
||||
|
||||
## `command/cmd/determinism.rs` — the determinism law over every command (improvise-hzjo)
|
||||
|
||||
The reasoning is in `context/code/command.md`, "A command is a function of its
|
||||
context". This entry records what each test can see.
|
||||
|
||||
### every_command_gives_equal_effects_in_independently_built_equal_contexts
|
||||
|
||||
It runs all 89 registrations, 1500+ (command, args) pairs, in four contexts built
|
||||
from four separately parsed copies of `web/demo.improv`, and requires equal
|
||||
effect `Debug` text. It passed at `ae96b8c` with no exemptions. It is not a
|
||||
regression test for a live bug. It restores a law, and the evidence is which
|
||||
rivals it catches:
|
||||
|
||||
- **`command-mode-backspace` reading `ctx.buffers.values().next()`** instead of
|
||||
the `command` buffer: a real command following hash order. Injected, full
|
||||
workspace run: this test is the **only** failure, because every
|
||||
`text_buffer.rs` test builds a one-entry buffer map. With the comparison
|
||||
switched to one reused context (`compare(&a, from_ref(&a))`) the same rival
|
||||
**passes**. That is the half the ticket required: the distinct construction
|
||||
is what does the work.
|
||||
- **`DataStore::matching_positions` without its sort**: gotcha 18 re-shipped.
|
||||
This entry first said the test catches it through `drill-into-cell` and
|
||||
`fit-measure`'s record order. That was wrong (improvise-e1sm). `fit-measure`
|
||||
never diverged, in 60 failing runs in review and in 354 here. And the catch
|
||||
was a matter of luck. The only real drill was `Product/Latte, Quarter/Q2`,
|
||||
which is one record with two cells, so each other context agrees with
|
||||
probability 1/2. With the sort removed and the test binary run in a loop, the
|
||||
law **passed 46 of 400 runs** at `fc0b13c`, and `drill-into-cell
|
||||
["Product/Latte", "Quarter/Q2"]` was the only pair that ever diverged. The
|
||||
arguments now include every item of every regular category as a
|
||||
one-coordinate drill, read off the parsed document (four records, eight cells
|
||||
each in the demo). With the same injection the law **passed 0 of 400**, and
|
||||
all eight new drills diverged in about 1195 of 1200 pairwise comparisons each.
|
||||
Without the injection it passed 400 of 400. The three `cell.rs` order tests
|
||||
still pin the store directly.
|
||||
- **The derived drills dropped from the argument list** (the
|
||||
`all.extend(one_coordinate_drills(model))` line commented out): the test
|
||||
fails before comparing anything, with "the widest drill orders 1 records".
|
||||
That is the only way of losing the wide drills the guard can see. It measures
|
||||
what the argument lists match, not what the built commands ordered, so it can
|
||||
pass while the comparison has lost its wide drills (improvise-b9f2). Review
|
||||
rival: `drill-into-cell`'s parser changed to refuse fewer than two arguments,
|
||||
which sends every one-coordinate drill to the one-record cursor fallback. The
|
||||
guard stays silent and, with gotcha 18 re-injected, the law passed 38 of 300
|
||||
runs.
|
||||
- **A stale exemption**: `("quit", "stale")` listed in `OUTSIDE_THE_CONTEXT`.
|
||||
It fails as "listed … but did not".
|
||||
|
||||
**A vacuity this test itself had on its first version.** The fixture's filler
|
||||
buffers held `v0`…`v15`. The `command-mode-backspace` rival pops a character,
|
||||
so every value collapsed to `v` (or `v1`), and the rival passed across two
|
||||
distinct contexts. This is the "two derivations the fixture makes agree" shape.
|
||||
The values are now `value {i} of 16`. The comparison was also widened from one
|
||||
other instance to three, because a take-the-first rival agrees across two seeds
|
||||
about one time in 21.
|
||||
|
||||
### only_distinct_instances_can_see_a_command_that_follows_hash_order
|
||||
|
||||
The check-the-checker behind the law. A test-only `EmitsInHashOrder` emits one
|
||||
`SetBuffer` per `ctx.buffers` entry in iteration order. It must equal itself on
|
||||
one context, which is why the old single-context law was vacuous, and differ
|
||||
across two independently built ones. If a future refactor made the fixture
|
||||
share one map, or share one `RandomState`, the second assertion fails. Without
|
||||
this test the law would then pass everything and look healthy.
|
||||
|
||||
## JSON import numbers (improvise-6yvg)
|
||||
|
||||
### `a_json_import_stores_its_numbers_bit_for_bit`
|
||||
|
||||
`ImportJsonHeadless` imports `[{"Region":"East","Revenue":0.24815714285714285}]`
|
||||
and the test reads Region=East, _Measure=Revenue back by `f64::to_bits`.
|
||||
`StartImportWizard` parses through the same `serde_json::from_slice` call, so
|
||||
one test covers the crate's feature.
|
||||
|
||||
**Red at `1158c23`** in `cargo test -p improvise-command`: stored
|
||||
`0.24815714285714283`. That reproduces improvise-6yvg. **Limitation:** this test
|
||||
cannot see the feature removed from `improvise-command` alone, because
|
||||
`improvise-io`, a normal dependency, enables it in the same build. That case is
|
||||
held by `every_crate_that_can_parse_json_enables_float_roundtrip` (`tooling.md`).
|
||||
|
||||
### the_grid_and_a_csv_export_agree_on_a_fold_row
|
||||
|
||||
The projection-level pin PR #203's review asked to keep. The review's own
|
||||
|
||||
@@ -1362,6 +1362,29 @@ fold is the row `z` names the group from. Note the narrowing this implies: an
|
||||
its header goes with it. lhi4 kept that header because it was the only way back to
|
||||
the rows; the fold is now that way back.
|
||||
|
||||
### records_steps_are_the_spine_then_every_non_index_dim_category_on_none
|
||||
|
||||
improvise-t0ma. Before this, `records_steps` was guarded by one incidental test: the
|
||||
`is_index_or_dim() → is_measure()` rival failed only
|
||||
`save_with_unfinished_record_is_readable`, a persistence test that happens to enter
|
||||
records mode. This asserts the **steps**, not the effects `cmd/grid.rs` translates
|
||||
them into, because the steps are the model-side answer a browser will also consume.
|
||||
The expected list filters `_Index`/`_Dim` by **name**, not by `CategoryKind`, so it
|
||||
is not a second copy of the predicate under test, and the setup asserts `_Measure`
|
||||
and a regular category are both present, since a model with neither could not tell
|
||||
the two predicates apart. Rival: `is_measure()` in place of `is_index_or_dim()`.
|
||||
This test fails, because `_Measure` is dropped and `_Index`/`_Dim` are put on `None`
|
||||
after the spine that just placed them.
|
||||
|
||||
### drill_steps_filter_each_coordinate_and_free_the_rest
|
||||
|
||||
improvise-t0ma's companion for `drill_steps`. The fixture includes a `none_cat` that
|
||||
the key also fixes (`Region`) and one that is virtual (`_Index`), so both exclusions
|
||||
are exercised. It also lists the key's coordinates out of sorted order, which checks
|
||||
that the steps follow `CellKey`'s canonical order. Rival: `Axis::Page` in place of
|
||||
`Axis::Filter` for a fixed coordinate, which reopens improvise-dz8 (a drilled
|
||||
coordinate the user could page away from). This test fails.
|
||||
|
||||
### a_folds_ratio_formula_is_the_ratio_of_its_folded_inputs
|
||||
|
||||
improvise-eccl. `Margin = (Revenue - Cost) / Revenue` with Jan `10/5` and Feb `30/3`
|
||||
|
||||
@@ -1434,3 +1434,17 @@ rather than defaulted: the alternative — expanding the group in the export —
|
||||
reintroduce the iot6 divergence between what the user sees and what they export.
|
||||
On the pre-6qet tree the body is empty (the group had no row, and pruning left
|
||||
nothing), so the test fails there.
|
||||
|
||||
## Legacy JSON numbers (improvise-6yvg)
|
||||
|
||||
### `a_legacy_json_workbook_loads_its_numbers_bit_for_bit`
|
||||
|
||||
A workbook with Food/Jan holding `0.24815714285714285` is serialized with
|
||||
`serde_json` and loaded through `load_from`. A leading `{` sends it down the
|
||||
legacy JSON path. The test first asserts that the text holds the exact decimal,
|
||||
so what it measures is the parse. It compares `f64::to_bits`. Found by the
|
||||
review of PR #190, not by the ticket.
|
||||
|
||||
**Red at `1158c23`** in `cargo test -p improvise-io`: loaded
|
||||
`0.24815714285714283`. Green once `improvise-io`'s own manifest enables
|
||||
`float_roundtrip`, and red again with only that line reverted.
|
||||
|
||||
@@ -1100,3 +1100,94 @@ and the text between the inner quotes leaked. It failed as
|
||||
`InRawC survived`. `c"…"` and `cr#"…"#` are now literals. A first version of
|
||||
this test used a C string with no inner quotes and passed against the bug,
|
||||
because the ordinary lexing happened to blank it too.
|
||||
|
||||
### help::tests::no_help_row_spells_a_key_by_hand
|
||||
|
||||
improvise-wk18. The help pages had one kind of row whose keys the keymap never
|
||||
saw: `literal("gg", …)`, `literal("yy", …)`, `literal("ZZ", …)`, and a dim tip
|
||||
reading "press Tab or l/n". Every other key row comes off `keys_for`, so a
|
||||
rebinding reaches it; these would have gone on saying `gg` after `g g` stopped
|
||||
meaning anything, and no guard could see them because a string carries no
|
||||
command name. At HEAD the test failed naming all four.
|
||||
|
||||
The fix is structural rather than a second scanner. The three prefix rows are
|
||||
ordinary `keys(..)` rows now — `keys` also asks `Keymap::prefixed_keys_running`
|
||||
for two-key sequences. The tip is **gone** rather than derived: made a `keys` row
|
||||
in `ModeKey::Help`, it gave the Welcome page its one row with a description,
|
||||
and `harness/tests/help.spec.js` ("? opens help, and its rows carry the keys and
|
||||
the words") failed in CI on `welcome.every(row => row.description === null)` —
|
||||
Welcome is prose by contract. The Help mode's own status hint already derives
|
||||
`next-page` from the keymap, so the literal was a second table (improvise-rr8n's
|
||||
rule: keep the one derived from the thing it describes).
|
||||
`literal()` and `dim()` are gone; the only way left to build a
|
||||
`HelpLine::Literal` is the `ex!` macro, which writes the leading colon itself, so
|
||||
what remains hand-written is ex-command text — checked against the parser by
|
||||
`every_ex_command_the_help_pages_show_is_one_the_parser_accepts`, which reads
|
||||
exactly the text a user would type. This test is what stops a direct
|
||||
`HelpLine::Literal { keys: "gg", .. }` coming back past the macro.
|
||||
|
||||
### help::tests::every_keys_row_the_pages_ask_for_finds_its_keys
|
||||
|
||||
A `keys` row whose lookup finds nothing is **dropped**, by design — a page never
|
||||
advertises a key that does not exist. The cost of that design is that a broken
|
||||
lookup looks like a shorter page: `every_help_page_key_row_names_keys_that_exist`
|
||||
only checks the rows that survived, plus a floor of twenty. So this asks the
|
||||
question from the other end, over `help_bindings()` — every row the builders
|
||||
*asked for* — and requires each to find a key.
|
||||
|
||||
The injection: comment out the `prefixed_labels` extension in `keys`. The three
|
||||
prefix rows vanish from the page, every other help test stays green, and this
|
||||
fails naming `Normal jump-first-row`, `Normal yank` and `Normal wq`.
|
||||
|
||||
### help::tests::an_ex_row_can_name_placeholders_and_nothing_else
|
||||
|
||||
improvise-v29m, found reviewing PR #180 at 949810c. `no_help_row_spells_a_key_by_hand`
|
||||
read only `HelpLine::Literal`, and its premise — that `keys()` is the only way to
|
||||
make a key row — was a convention: `HelpLine::Keys { keys: Vec<String>, .. }`
|
||||
was a public variant with public fields, and `help_bindings()` records only what
|
||||
the builders *ask* for, so `HelpLine::Keys { keys: vec!["Q"], description: "Quit
|
||||
instantly" }` typed straight into `panels()`, with `Q` bound to nothing, passed
|
||||
the whole workspace. So did `ex!("q or press Q", ..)`: the parser test splits on
|
||||
colons and `parse_line` accepts trailing words.
|
||||
|
||||
Both are now **compile errors**, which is why this test is not where the guard
|
||||
lives. `HelpLine::Keys` carries a `KeyRow` whose fields are private to a module
|
||||
whose one constructor, `KeyRow::looked_up`, does the keymap lookup; the injected
|
||||
row fails with E0559/E0451. `ex!` evaluates `ex_usage_is_placeholders` in a
|
||||
`const` item, and the injected usage fails with `evaluation panicked: assertion
|
||||
failed: ex_usage_is_placeholders("q or press Q")`. The rule cost one row:
|
||||
`:add-items <cat> a b c ...` had three bare words and is now
|
||||
`:add-items <cat> <item> ...`. This test pins the rule the macro asks, in both
|
||||
directions, including that shipped usage and the rival.
|
||||
|
||||
### help::tests::every_keys_row_drawn_is_one_the_builders_asked_for
|
||||
|
||||
improvise-ssoe, found in re-review of #180 at 544744c. `KeyRow` closed
|
||||
hand-typed keys, but `KeyRow::looked_up` was `pub(super)`, so a page builder
|
||||
could call it directly instead of through its `keys` parameter — and
|
||||
`help_bindings()` records only what goes through the parameter. Injected into
|
||||
`editing()`: `KeyRow::looked_up(n, "clear-cell", None, "Edit the current
|
||||
cell")`, which captions `x` as editing the cell — the exact lie `jhdf`'s test
|
||||
exists for — and the whole workspace stayed green, because both guards that
|
||||
would object read `help_bindings()`.
|
||||
|
||||
Two changes. `looked_up` is now private to the `key_row` module, whose one
|
||||
caller is the module's own `keys` — the lookup `pages()` hands every builder —
|
||||
so the reviewer's injection is `E0624: associated function looked_up is
|
||||
private`. That narrows the door without closing it: a builder can still reach
|
||||
the module-level `keys` by path (`self::keys(..)`) rather than through its
|
||||
shadowing parameter. So this test states the premise directly — the number of
|
||||
keys rows drawn equals the number of rows the builders asked for (every asked
|
||||
row finds its keys, by `every_keys_row_the_pages_ask_for_finds_its_keys`) —
|
||||
and the `self::keys` bypass fails it `51 != 50`.
|
||||
|
||||
**improvise-aszt: a count is not the premise.** The test above compared
|
||||
*counts*, so a bypass that also drops one real row keeps them equal. Injected in
|
||||
`editing()`: call the parameter `keys(e, "clear-buffer", …)` and discard it with
|
||||
`let _ =`, then return `self::keys(n, "clear-cell", None, "Edit the current
|
||||
cell")` in its place. The asked row is recorded and never drawn, the drawn row
|
||||
is never recorded, 50 = 50, and the whole workspace stayed green. The test now
|
||||
compares **content in order**: the drawn keys rows must equal
|
||||
`help_bindings()` looked up through the same `keys`, row for row. The same
|
||||
injection now fails. What still passes is a bypass that draws exactly what the
|
||||
builders asked for, in the order they asked — which is harmless by definition.
|
||||
|
||||
@@ -681,3 +681,41 @@ reproduction.
|
||||
alone fails, exiting 0. All 44 others pass. That is the review's point: the
|
||||
older `a head that moves mid-wait is not a pass` starts from MERGEABLE, so it
|
||||
cannot see this rival.
|
||||
|
||||
## Every JSON parser reads numbers exactly (improvise-dz8c)
|
||||
|
||||
### `every_crate_that_can_parse_json_enables_float_roundtrip`
|
||||
|
||||
It replaces a check over three crates listed by hand. It reads
|
||||
`cargo metadata --no-deps` and fails naming every workspace package, and
|
||||
every target-specific entry, whose **normal** serde_json dependency lacks
|
||||
`float_roundtrip`. The rule and why it was chosen over the alternatives are in
|
||||
`context/code/protocol.md`. It is red at `1158c23`, naming `improvise`,
|
||||
`improvise-command`, `improvise-io` and `improvise-protocol`.
|
||||
|
||||
**Faults injected, each run:**
|
||||
|
||||
- **The feature removed from `improvise-io` alone.** The check fails naming
|
||||
`improvise-io (all targets)`, and so does
|
||||
`a_legacy_json_workbook_loads_its_numbers_bit_for_bit`.
|
||||
`a_json_import_stores_its_numbers_bit_for_bit` stays green, because
|
||||
`improvise-command` enables the feature and unification reaches io in its
|
||||
build.
|
||||
- **The feature removed from `improvise-command` alone.** The check fails naming
|
||||
`improvise-command (all targets)`. The command crate's own import test
|
||||
**stays green**: `cargo tree -p improvise-command` shows io's declaration
|
||||
switching the feature on. So the manifest check is the only guard that
|
||||
sees this case. It is also the case the retired build-graph check would
|
||||
have passed.
|
||||
- **A throwaway workspace member**, `zz-throwaway`, with normal dependencies on
|
||||
`improvise-protocol` and `serde_json` and no feature (the lockfile updated,
|
||||
as a real new crate would be). The check fails naming
|
||||
`zz-throwaway (all targets)`. With the unconditional dependency carrying the
|
||||
feature and a `cfg(target_arch = "wasm32")` one lacking it, the check fails
|
||||
naming `zz-throwaway (cfg(target_arch = "wasm32"))`. That is stricter than
|
||||
the build needs, since a wasm build unifies the two entries, and cheap to
|
||||
satisfy. The crate was then removed.
|
||||
|
||||
The `checked.len() >= 2` assertion is the positive signal. If a cargo change
|
||||
renamed the metadata fields, the check would find no serde_json dependency and
|
||||
pass vacuously without it.
|
||||
|
||||
@@ -14,7 +14,9 @@ improvise-io = { path = "../improvise-io" }
|
||||
improvise-gp = { path = "../improvise-gp" }
|
||||
anyhow = "1"
|
||||
serde = { version = "1", features = ["derive"] }
|
||||
serde_json = "1"
|
||||
# Parses JSON a user's numbers cross, so it must read back the f64 written: per crate,
|
||||
# because each build stands alone (improvise-6yvg, context/code/protocol.md).
|
||||
serde_json = { version = "1", features = ["float_roundtrip"] }
|
||||
unicode-width = "^0.2"
|
||||
# Instant::now() panics on wasm32-unknown-unknown; web_time re-exports
|
||||
# std::time off-wasm and uses performance.now() on it (improvise-ywd).
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
use crate::model::category::MEASURE_CATEGORY;
|
||||
use crate::model::cell::{CellKey, CellValue};
|
||||
use crate::model::cell::CellKey;
|
||||
use crate::ui::app::AppMode;
|
||||
use crate::ui::effect::{self, Effect};
|
||||
use crate::view::{RecordCol, RecordRow};
|
||||
use crate::view::RecordCol;
|
||||
|
||||
use super::core::{Cmd, CmdContext, read_buffer};
|
||||
use super::grid::AddRecordRow;
|
||||
@@ -14,6 +14,7 @@ mod tests {
|
||||
|
||||
use super::*;
|
||||
use crate::command::cmd::test_helpers::*;
|
||||
use crate::model::cell::CellValue;
|
||||
use crate::workbook::Workbook;
|
||||
|
||||
fn commit(kind: CommitKind) -> CommitBuffer {
|
||||
@@ -622,47 +623,8 @@ fn stage_drill_edit(record_idx: usize, col: RecordCol, value: &str) -> Box<dyn E
|
||||
})
|
||||
}
|
||||
|
||||
/// Rename one coordinate of a records row. In wide format a row is a group of cells,
|
||||
/// so the rename moves **every** one of them (improvise-c5v, gotcha 1).
|
||||
fn rename_record_coordinate(
|
||||
model: &crate::model::Model,
|
||||
row: &RecordRow,
|
||||
category: &str,
|
||||
value: &str,
|
||||
effects: &mut Vec<Box<dyn Effect>>,
|
||||
) {
|
||||
if value.is_empty() {
|
||||
effects.push(effect::refuse(effect::RECORD_COORDS_CANNOT_BE_EMPTY));
|
||||
return;
|
||||
}
|
||||
effects.push(Box::new(effect::AddItem {
|
||||
category: category.to_string(),
|
||||
item: value.to_string(),
|
||||
}));
|
||||
|
||||
// Every cell in the row moves. A row seeded by `add-record-row` carries its
|
||||
// coordinates on a lone measure-less cell, which must move too.
|
||||
let cells: Vec<(CellKey, CellValue)> = if row.values.is_empty() {
|
||||
model
|
||||
.get_cell(&row.coords)
|
||||
.map(|v| vec![(row.coords.clone(), v.clone())])
|
||||
.unwrap_or_default()
|
||||
} else {
|
||||
row.values
|
||||
.iter()
|
||||
.map(|(measure, v)| (row.cell_key(measure), v.clone()))
|
||||
.collect()
|
||||
};
|
||||
for (old_key, cell_value) in cells {
|
||||
effects.push(Box::new(effect::ClearCell(old_key.clone())));
|
||||
effects.push(Box::new(effect::SetCell(
|
||||
old_key.with(category, value),
|
||||
cell_value,
|
||||
)));
|
||||
}
|
||||
}
|
||||
|
||||
/// Apply a records-mode edit directly to the model (no drill state).
|
||||
/// Apply a records-mode edit directly to the model (no drill state). A drill's staged
|
||||
/// edits go through the same `RecordRewrite`, all of a row's at once (gotcha 19).
|
||||
fn commit_plain_records_edit(
|
||||
ctx: &CmdContext,
|
||||
record_idx: usize,
|
||||
@@ -674,11 +636,8 @@ fn commit_plain_records_edit(
|
||||
return;
|
||||
};
|
||||
match col {
|
||||
RecordCol::Category(category) => {
|
||||
rename_record_coordinate(ctx.model, row, category, value, effects);
|
||||
}
|
||||
RecordCol::Measure(measure) => {
|
||||
commit_regular_cell_value(&row.cell_key(measure), value, effects);
|
||||
RecordCol::Category(_) | RecordCol::Measure(_) => {
|
||||
effects.extend(effect::RecordRewrite::of(ctx.model, row, [(col, value)]).effects());
|
||||
}
|
||||
RecordCol::NewMeasure => add_measure(value, effects),
|
||||
RecordCol::NewCategory => add_category(value, effects),
|
||||
|
||||
@@ -0,0 +1,233 @@
|
||||
//! A command is a function of its context: two contexts built independently from
|
||||
//! equal data give equal effects — `context/code/command.md`, improvise-hzjo.
|
||||
|
||||
use std::collections::{BTreeSet, HashSet};
|
||||
|
||||
use super::core::{Cmd, CmdContext};
|
||||
use super::registry::tests::candidate_args;
|
||||
use crate::command::key::{KeyCode, KeyModifiers};
|
||||
use crate::model::Model;
|
||||
use crate::model::cell::{CellKey, CellValue};
|
||||
use crate::model::record::record_of;
|
||||
use crate::ui::app::App;
|
||||
use crate::ui::effect::{self, Effect};
|
||||
|
||||
/// Commands whose effects depend on something outside the context, with why.
|
||||
const OUTSIDE_THE_CONTEXT: &[(&str, &str)] = &[];
|
||||
|
||||
const DOCUMENT: &str = include_str!("../../../../../web/demo.improv");
|
||||
|
||||
/// One `App` over the shipped demo. Every context map holds several entries that
|
||||
/// stay distinct under an edit like `pop()` — see the evidence.
|
||||
fn fixture() -> App {
|
||||
let workbook = crate::persistence::parse_md(DOCUMENT).expect("the demo parses");
|
||||
let mut app = App::new(workbook, None);
|
||||
let vs = &mut app.view_state;
|
||||
for i in 0..16 {
|
||||
vs.buffers
|
||||
.insert(format!("b{i}"), format!("value {i} of 16"));
|
||||
}
|
||||
for (name, text) in [
|
||||
("edit", "42"),
|
||||
("formula", "Margin = Profit / Revenue"),
|
||||
("category", "Region"),
|
||||
("command", "add-item Product Mocha"),
|
||||
("export", "/tmp/improvise-hzjo.csv"),
|
||||
] {
|
||||
vs.buffers.insert(name.into(), text.into());
|
||||
}
|
||||
for cat in ["Product", "Quarter", "_Measure", "_Index", "_Dim"] {
|
||||
vs.expanded_cats.insert(cat.into());
|
||||
}
|
||||
vs.yanked = Some(CellValue::Number(7.0));
|
||||
vs.formula_panel_open = true;
|
||||
vs.category_panel_open = true;
|
||||
vs.view_panel_open = true;
|
||||
vs.cat_panel_cursor = 2;
|
||||
vs.view_panel_cursor = 1;
|
||||
app.cursor_mut().selected = (1, 1);
|
||||
app
|
||||
}
|
||||
|
||||
/// Every item as a one-coordinate drill: each spans the other categories whole,
|
||||
/// so the store has several records to order — see the evidence.
|
||||
fn one_coordinate_drills(model: &Model) -> Vec<Vec<String>> {
|
||||
let mut drills = Vec::new();
|
||||
for cat in model.regular_category_names() {
|
||||
for item in model.category(cat).expect("listed").ordered_item_names() {
|
||||
drills.push(vec![format!("{cat}/{item}")]);
|
||||
}
|
||||
}
|
||||
drills
|
||||
}
|
||||
|
||||
/// How many distinct records the widest all-`Cat/Item` candidate matches.
|
||||
fn widest_drill(model: &Model, candidates: &[Vec<String>]) -> usize {
|
||||
let coordinates = |args: &Vec<String>| -> Option<Vec<(String, String)>> {
|
||||
args.iter()
|
||||
.map(|a| {
|
||||
a.split_once('/')
|
||||
.map(|(c, i)| (c.to_string(), i.to_string()))
|
||||
})
|
||||
.collect()
|
||||
};
|
||||
candidates
|
||||
.iter()
|
||||
.filter(|args| !args.is_empty())
|
||||
.filter_map(coordinates)
|
||||
.map(|partial| {
|
||||
let cells = model.data.matching_cells(&partial);
|
||||
let records: HashSet<CellKey> = cells.iter().map(|(k, _)| record_of(k)).collect();
|
||||
records.len()
|
||||
})
|
||||
.max()
|
||||
.unwrap_or(0)
|
||||
}
|
||||
|
||||
/// The registry's candidates, plus names the demo really has: a drill or a fit
|
||||
/// over coordinates that match nothing never reaches the records it would order.
|
||||
fn args_the_document_names(model: &Model) -> Vec<Vec<String>> {
|
||||
let mut all = candidate_args();
|
||||
all.extend(one_coordinate_drills(model));
|
||||
all.extend(
|
||||
[
|
||||
vec!["Revenue"],
|
||||
vec!["Product/Latte", "Quarter/Q2"],
|
||||
vec!["Product/Latte", "Quarter/Q2", "_Measure/Cost"],
|
||||
vec![
|
||||
"editing",
|
||||
"5",
|
||||
"Product/Latte",
|
||||
"Quarter/Q2",
|
||||
"_Measure/Cost",
|
||||
],
|
||||
]
|
||||
.into_iter()
|
||||
.map(|v| v.into_iter().map(String::from).collect()),
|
||||
);
|
||||
all
|
||||
}
|
||||
|
||||
fn effects_of(name: &str, args: &[String], ctx: &CmdContext) -> Option<String> {
|
||||
let cmd = ctx.registry.interactive(name, args, ctx).ok()?;
|
||||
Some(format!("{:?}", cmd.execute(ctx)))
|
||||
}
|
||||
|
||||
/// What running every registered command under `a` and under each of `others` found.
|
||||
#[derive(Default)]
|
||||
struct Comparison {
|
||||
diverged: BTreeSet<&'static str>,
|
||||
shown: Vec<String>,
|
||||
never_built: Vec<&'static str>,
|
||||
compared: usize,
|
||||
}
|
||||
|
||||
fn compare(a: &CmdContext, others: &[CmdContext]) -> Comparison {
|
||||
let mut found = Comparison::default();
|
||||
let candidates = args_the_document_names(a.model);
|
||||
for name in a.registry.names().collect::<Vec<_>>() {
|
||||
let before = found.compared;
|
||||
for args in &candidates {
|
||||
let Some(left) = effects_of(name, args, a) else {
|
||||
continue;
|
||||
};
|
||||
found.compared += 1;
|
||||
for b in others {
|
||||
let right = effects_of(name, args, b).unwrap_or_default();
|
||||
if left != right {
|
||||
found.diverged.insert(name);
|
||||
found
|
||||
.shown
|
||||
.push(format!("{name} {args:?}:\n {left}\n {right}"));
|
||||
}
|
||||
}
|
||||
}
|
||||
if found.compared == before {
|
||||
found.never_built.push(name);
|
||||
}
|
||||
}
|
||||
found
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn every_command_gives_equal_effects_in_independently_built_equal_contexts() {
|
||||
let apps: Vec<App> = (0..4).map(|_| fixture()).collect();
|
||||
let contexts: Vec<CmdContext> = apps
|
||||
.iter()
|
||||
.map(|app| app.cmd_context(KeyCode::Null, KeyModifiers::NONE))
|
||||
.collect();
|
||||
let model = contexts[0].model;
|
||||
let widest = widest_drill(model, &args_the_document_names(model));
|
||||
assert!(
|
||||
widest >= 4,
|
||||
"the widest drill orders {widest} records, so the store's order is nearly a coin toss"
|
||||
);
|
||||
let found = compare(&contexts[0], &contexts[1..]);
|
||||
assert!(
|
||||
found.never_built.is_empty(),
|
||||
"no candidate args build these in the fixture, so the law never ran them: {:?}",
|
||||
found.never_built
|
||||
);
|
||||
let listed: BTreeSet<&str> = OUTSIDE_THE_CONTEXT.iter().map(|(n, _)| *n).collect();
|
||||
let unlisted: Vec<&str> = found
|
||||
.shown
|
||||
.iter()
|
||||
.filter(|s| !listed.iter().any(|n| s.starts_with(&format!("{n} "))))
|
||||
.map(String::as_str)
|
||||
.collect();
|
||||
assert!(
|
||||
unlisted.is_empty(),
|
||||
"effects depend on more than the context ({} pairs compared):\n{}",
|
||||
found.compared,
|
||||
unlisted.join("\n")
|
||||
);
|
||||
let stale: Vec<&&str> = listed
|
||||
.iter()
|
||||
.filter(|n| !found.diverged.contains(**n))
|
||||
.collect();
|
||||
assert!(
|
||||
stale.is_empty(),
|
||||
"listed as depending on something outside the context, but did not: {stale:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Iterates a context `HashMap` and emits in that order: the one nondeterminism
|
||||
/// this repo has shipped (gotcha 18). The law must see it; a reused context cannot.
|
||||
#[derive(Debug)]
|
||||
struct EmitsInHashOrder;
|
||||
impl Cmd for EmitsInHashOrder {
|
||||
fn name(&self) -> &'static str {
|
||||
"emits-in-hash-order"
|
||||
}
|
||||
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
ctx.buffers
|
||||
.iter()
|
||||
.map(|(name, value)| {
|
||||
Box::new(effect::SetBuffer {
|
||||
name: name.clone(),
|
||||
value: value.clone(),
|
||||
}) as Box<dyn Effect>
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn only_distinct_instances_can_see_a_command_that_follows_hash_order() {
|
||||
let (first, second) = (fixture(), fixture());
|
||||
let a = first.cmd_context(KeyCode::Null, KeyModifiers::NONE);
|
||||
let b = second.cmd_context(KeyCode::Null, KeyModifiers::NONE);
|
||||
let run = |ctx: &CmdContext| format!("{:?}", EmitsInHashOrder.execute(ctx));
|
||||
assert_eq!(
|
||||
run(&a),
|
||||
run(&a),
|
||||
"one reused context iterates its maps the same way twice"
|
||||
);
|
||||
assert_ne!(
|
||||
run(&a),
|
||||
run(&b),
|
||||
"two independently built contexts must iterate differently, or the law is vacuous"
|
||||
);
|
||||
}
|
||||
@@ -13,6 +13,9 @@ pub mod text_buffer;
|
||||
pub mod tile;
|
||||
pub mod wizard;
|
||||
|
||||
#[cfg(test)]
|
||||
mod determinism;
|
||||
|
||||
// Re-export items used by external code
|
||||
pub use self::core::{Cmd, CmdContext, CmdRegistry};
|
||||
pub use registry::default_registry;
|
||||
|
||||
@@ -498,13 +498,13 @@ pub fn default_registry() -> CmdRegistry {
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
pub(super) mod tests {
|
||||
use super::*;
|
||||
use crate::command::cmd::test_helpers::*;
|
||||
|
||||
/// Argument vectors to try when constructing a command, most specific
|
||||
/// first. A command is exercised by the first one its parser accepts.
|
||||
fn candidate_args() -> Vec<Vec<String>> {
|
||||
pub(in crate::command::cmd) fn candidate_args() -> Vec<Vec<String>> {
|
||||
[
|
||||
vec![],
|
||||
vec!["1"],
|
||||
|
||||
@@ -59,6 +59,26 @@ mod tests {
|
||||
assert!(effects.is_empty());
|
||||
}
|
||||
|
||||
/// improvise-jgzw: `_Index`/`_Dim` stay tileable on purpose, and `_Measure` on
|
||||
/// a column must too — `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn the_tile_bar_offers_every_category_by_name_measure_and_records_axes_included() {
|
||||
let mut m = two_cat_model();
|
||||
m.active_view_mut().set_axis("_Measure", Axis::Column);
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let mut ctx = make_ctx(&m, &layout, ®);
|
||||
let mut offered = Vec::new();
|
||||
for idx in 0..8 {
|
||||
ctx.tile_cat_idx = idx;
|
||||
let dbg = effects_debug(&TileAxisOp { axis: None }.execute(&ctx));
|
||||
if let Some(rest) = dbg.split("CycleAxis(Active, \"").nth(1) {
|
||||
offered.push(rest.split('"').next().unwrap_or_default().to_string());
|
||||
}
|
||||
}
|
||||
assert_eq!(offered, ["_Index", "_Dim", "_Measure", "Type", "Month"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn move_tile_cursor_right() {
|
||||
let m = two_cat_model();
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use std::collections::BTreeMap;
|
||||
use std::fmt::Debug;
|
||||
use std::path::PathBuf;
|
||||
|
||||
@@ -663,24 +664,19 @@ impl Effect for ApplyAndClearDrill {
|
||||
let Some(drill) = app.drill_state.take() else {
|
||||
return;
|
||||
};
|
||||
if drill.pending_edits.is_empty() {
|
||||
return;
|
||||
}
|
||||
let mut rows: BTreeMap<usize, Vec<(&RecordCol, &str)>> = BTreeMap::new();
|
||||
for ((record_idx, col), new_value) in &drill.pending_edits {
|
||||
let Some(row) = drill.records.get(*record_idx) else {
|
||||
continue;
|
||||
};
|
||||
match col {
|
||||
RecordCol::Measure(measure) => {
|
||||
apply_measure_edit(app, &row.cell_key(measure), new_value);
|
||||
}
|
||||
RecordCol::Category(category) => {
|
||||
apply_coordinate_rename(app, row, category, new_value);
|
||||
}
|
||||
// Never staged: `commit_cell_value` applies a new measure or
|
||||
// category immediately — a schema change is not a cell edit.
|
||||
RecordCol::NewMeasure | RecordCol::NewCategory => {}
|
||||
}
|
||||
rows.entry(*record_idx)
|
||||
.or_default()
|
||||
.push((col, new_value.as_str()));
|
||||
}
|
||||
let model = &app.model_state.workbook.model;
|
||||
let rewrite = RecordRewrite::merged(rows.iter().filter_map(|(record_idx, edits)| {
|
||||
let row = drill.records.get(*record_idx)?;
|
||||
Some(RecordRewrite::of(model, row, edits.iter().copied()))
|
||||
}));
|
||||
for effect in rewrite.effects() {
|
||||
effect.apply(app);
|
||||
}
|
||||
// No `dirty = true` here: this declares `Model(CellData)` and
|
||||
// `App::apply_effects` raises the flag off that (improvise-gv9r).
|
||||
@@ -690,51 +686,114 @@ impl Effect for ApplyAndClearDrill {
|
||||
}
|
||||
}
|
||||
|
||||
/// Write one measure cell, or clear it when the edit is blank.
|
||||
fn apply_measure_edit(app: &mut App, key: &CellKey, new_value: &str) {
|
||||
let model = &mut app.model_state.workbook.model;
|
||||
match new_value {
|
||||
"" => {
|
||||
model.clear_cell(key);
|
||||
}
|
||||
v => {
|
||||
let value = v
|
||||
.parse::<f64>()
|
||||
.map(CellValue::Number)
|
||||
.unwrap_or_else(|_| CellValue::Text(v.to_string()));
|
||||
model.set_cell(key.clone(), value);
|
||||
}
|
||||
}
|
||||
/// A records row's edits composed into the one record they mean, read off the model
|
||||
/// before anything is written. Why composed: `context/code/command.md`, `RecordRewrite`.
|
||||
#[derive(Debug, Default, PartialEq)]
|
||||
pub(crate) struct RecordRewrite {
|
||||
refused: bool,
|
||||
items: Vec<(String, String)>,
|
||||
clears: Vec<CellKey>,
|
||||
sets: Vec<(CellKey, CellValue)>,
|
||||
}
|
||||
|
||||
/// Rename one coordinate of a records row. The row is a group of cells —
|
||||
/// one per measure — so every one of them moves (improvise-c5v).
|
||||
fn apply_coordinate_rename(app: &mut App, row: &RecordRow, category: &str, new_value: &str) {
|
||||
if new_value.is_empty() {
|
||||
app.refuse(RECORD_COORDS_CANNOT_BE_EMPTY);
|
||||
return;
|
||||
}
|
||||
let keys: Vec<CellKey> = if row.values.is_empty() {
|
||||
vec![row.coords.clone()]
|
||||
} else {
|
||||
row.values
|
||||
.iter()
|
||||
.map(|(measure, _)| row.cell_key(measure))
|
||||
.collect()
|
||||
};
|
||||
let _ = app
|
||||
.model_state
|
||||
.workbook
|
||||
.add_item(category, new_value.to_string());
|
||||
for old_key in keys {
|
||||
let Some(value) = app.model_state.workbook.model.get_cell(&old_key).cloned() else {
|
||||
continue;
|
||||
impl RecordRewrite {
|
||||
/// `row`'s edits, each naming a distinct column. The result does not depend on
|
||||
/// their order: every coordinate is renamed before any cell is placed.
|
||||
pub(crate) fn of<'a>(
|
||||
model: &crate::model::Model,
|
||||
row: &RecordRow,
|
||||
edits: impl IntoIterator<Item = (&'a RecordCol, &'a str)>,
|
||||
) -> Self {
|
||||
let mut refused = false;
|
||||
let mut renames: BTreeMap<&str, &str> = BTreeMap::new();
|
||||
let mut measures: BTreeMap<&str, Option<CellValue>> = BTreeMap::new();
|
||||
for (col, value) in edits {
|
||||
match col {
|
||||
RecordCol::Category(_) if value.is_empty() => refused = true,
|
||||
RecordCol::Category(category) => {
|
||||
renames.insert(category.as_str(), value);
|
||||
}
|
||||
RecordCol::Measure(measure) => {
|
||||
measures.insert(measure.as_str(), improvise_protocol::typed(value));
|
||||
}
|
||||
// Never staged: a new measure or category is a schema change,
|
||||
// applied at once by `commit_cell_value`.
|
||||
RecordCol::NewMeasure | RecordCol::NewCategory => {}
|
||||
}
|
||||
}
|
||||
let renamed = |key: &CellKey| {
|
||||
renames.iter().fold(key.clone(), |key, (category, item)| {
|
||||
key.with(*category, *item)
|
||||
})
|
||||
};
|
||||
app.model_state.workbook.model.clear_cell(&old_key);
|
||||
app.model_state
|
||||
.workbook
|
||||
.model
|
||||
.set_cell(old_key.with(category, new_value), value);
|
||||
let moved: Vec<(Option<&str>, CellKey)> = match (renames.is_empty(), row.values.is_empty())
|
||||
{
|
||||
(true, _) => Vec::new(),
|
||||
(false, true) => vec![(None, row.coords.clone())],
|
||||
(false, false) => row
|
||||
.values
|
||||
.iter()
|
||||
.map(|(measure, _)| (Some(measure.as_str()), row.cell_key(measure)))
|
||||
.collect(),
|
||||
};
|
||||
let mut rewrite = RecordRewrite {
|
||||
refused,
|
||||
items: renames
|
||||
.iter()
|
||||
.map(|(category, item)| (category.to_string(), item.to_string()))
|
||||
.collect(),
|
||||
..Self::default()
|
||||
};
|
||||
for (measure, old) in &moved {
|
||||
let value = match measure.and_then(|m| measures.remove(m)) {
|
||||
Some(edit) => edit,
|
||||
None => model.get_cell(old).cloned(),
|
||||
};
|
||||
rewrite.clears.push(old.clone());
|
||||
rewrite.sets.extend(value.map(|v| (renamed(old), v)));
|
||||
}
|
||||
for (measure, edit) in measures {
|
||||
let key = renamed(&row.cell_key(measure));
|
||||
match edit {
|
||||
Some(value) => rewrite.sets.push((key, value)),
|
||||
None => rewrite.clears.push(key),
|
||||
}
|
||||
}
|
||||
rewrite
|
||||
}
|
||||
|
||||
/// Several rows' rewrites as one: every clear lands before any write, so a
|
||||
/// swap is a swap, and where two rows land on one cell the later row wins.
|
||||
pub(crate) fn merged(rewrites: impl IntoIterator<Item = Self>) -> Self {
|
||||
rewrites.into_iter().fold(Self::default(), |mut all, one| {
|
||||
all.refused |= one.refused;
|
||||
all.items.extend(one.items);
|
||||
all.clears.extend(one.clears);
|
||||
all.sets.extend(one.sets);
|
||||
all
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn effects(self) -> Vec<Box<dyn Effect>> {
|
||||
let refusal = self.refused.then(|| refuse(RECORD_COORDS_CANNOT_BE_EMPTY));
|
||||
let items = self
|
||||
.items
|
||||
.into_iter()
|
||||
.map(|(category, item)| Box::new(AddItem { category, item }) as Box<dyn Effect>);
|
||||
let clears = self
|
||||
.clears
|
||||
.into_iter()
|
||||
.map(|key| Box::new(ClearCell(key)) as Box<dyn Effect>);
|
||||
let sets = self
|
||||
.sets
|
||||
.into_iter()
|
||||
.map(|(key, value)| Box::new(SetCell(key, value)) as Box<dyn Effect>);
|
||||
refusal
|
||||
.into_iter()
|
||||
.chain(items)
|
||||
.chain(clears)
|
||||
.chain(sets)
|
||||
.collect()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3071,6 +3130,209 @@ mod tests {
|
||||
assert_eq!(app.model_state.workbook.model.get_cell(&key), None);
|
||||
}
|
||||
|
||||
// ── Composing a drill's staged edits (improvise-uo2p) ───────────────
|
||||
|
||||
fn drill_cell(ty: &str, month: &str, measure: &str) -> CellKey {
|
||||
CellKey::new(vec![
|
||||
("Type".into(), ty.into()),
|
||||
("Month".into(), month.into()),
|
||||
("_Measure".into(), measure.into()),
|
||||
])
|
||||
}
|
||||
|
||||
/// Two records, each carrying Revenue and Cost: row 0 is Food/Jan (1, 2),
|
||||
/// row 1 is Clothing/Jan (42, 17). Every edit below is staged on row 1 at least.
|
||||
fn two_record_drill() -> App {
|
||||
let mut app = test_app();
|
||||
let cells = [
|
||||
(drill_cell("Food", "Jan", "Revenue"), CellValue::Number(1.0)),
|
||||
(drill_cell("Food", "Jan", "Cost"), CellValue::Number(2.0)),
|
||||
(
|
||||
drill_cell("Clothing", "Jan", "Revenue"),
|
||||
CellValue::Number(42.0),
|
||||
),
|
||||
(
|
||||
drill_cell("Clothing", "Jan", "Cost"),
|
||||
CellValue::Number(17.0),
|
||||
),
|
||||
];
|
||||
for (key, value) in cells.clone() {
|
||||
app.model_state.workbook.model.set_cell(key, value);
|
||||
}
|
||||
let records = RecordRow::group(cells);
|
||||
assert_eq!(records.len(), 2, "one row per coordinate tuple");
|
||||
assert_eq!(records[1].coords.get("Type"), Some("Clothing"));
|
||||
StartDrill(records).apply(&mut app);
|
||||
app
|
||||
}
|
||||
|
||||
fn stage(app: &mut App, record_idx: usize, col: RecordCol, value: &str) {
|
||||
SetDrillPendingEdit {
|
||||
record_idx,
|
||||
col,
|
||||
new_value: value.to_string(),
|
||||
}
|
||||
.apply(app);
|
||||
}
|
||||
|
||||
/// The whole store, as `(Type, Month, measure) = n`, so a ghost cannot hide.
|
||||
fn drill_store(app: &App) -> std::collections::BTreeSet<String> {
|
||||
app.model_state
|
||||
.workbook
|
||||
.model
|
||||
.data
|
||||
.iter_cells()
|
||||
.map(|(key, value)| {
|
||||
let coord = |c: &str| key.get(c).unwrap_or("-").to_string();
|
||||
let n = value.as_f64().expect("every drill cell here is a number");
|
||||
format!(
|
||||
"{}/{}/{} = {n}",
|
||||
coord("Type"),
|
||||
coord("Month"),
|
||||
coord("_Measure")
|
||||
)
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn category(name: &str) -> RecordCol {
|
||||
RecordCol::Category(name.into())
|
||||
}
|
||||
|
||||
fn measure(name: &str) -> RecordCol {
|
||||
RecordCol::Measure(name.into())
|
||||
}
|
||||
|
||||
/// Stage `edits` and leave the drill, in 32 fresh Apps: each `HashMap` gets its
|
||||
/// own `RandomState`, and one map iterates the same way twice.
|
||||
fn assert_drill_commits_to(edits: &[(usize, RecordCol, &str)], expected: &[&str]) {
|
||||
let expected: std::collections::BTreeSet<String> =
|
||||
expected.iter().map(|s| s.to_string()).collect();
|
||||
for attempt in 0..32 {
|
||||
let mut app = two_record_drill();
|
||||
for (row, col, value) in edits {
|
||||
stage(&mut app, *row, col.clone(), value);
|
||||
}
|
||||
app.apply_effects(vec![Box::new(ApplyAndClearDrill)]);
|
||||
assert_eq!(drill_store(&app), expected, "fresh App #{attempt}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_drill_row_renamed_twice_and_edited_lands_as_one_record() {
|
||||
assert_drill_commits_to(
|
||||
&[
|
||||
(1, measure("Cost"), "999"),
|
||||
(1, category("Type"), "Drink"),
|
||||
(1, category("Month"), "Feb"),
|
||||
],
|
||||
&[
|
||||
"Food/Jan/Revenue = 1",
|
||||
"Food/Jan/Cost = 2",
|
||||
"Drink/Feb/Revenue = 42",
|
||||
"Drink/Feb/Cost = 999",
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_drill_rows_measure_edits_follow_its_rename() {
|
||||
assert_drill_commits_to(
|
||||
&[
|
||||
(1, measure("Revenue"), "100"),
|
||||
(1, measure("Cost"), "999"),
|
||||
(1, category("Type"), "Drink"),
|
||||
],
|
||||
&[
|
||||
"Food/Jan/Revenue = 1",
|
||||
"Food/Jan/Cost = 2",
|
||||
"Drink/Jan/Revenue = 100",
|
||||
"Drink/Jan/Cost = 999",
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_drill_that_swaps_two_records_coordinates_swaps_them() {
|
||||
assert_drill_commits_to(
|
||||
&[
|
||||
(0, category("Type"), "Clothing"),
|
||||
(1, category("Type"), "Food"),
|
||||
],
|
||||
&[
|
||||
"Clothing/Jan/Revenue = 1",
|
||||
"Clothing/Jan/Cost = 2",
|
||||
"Food/Jan/Revenue = 42",
|
||||
"Food/Jan/Cost = 17",
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_drill_edit_addresses_its_own_record_not_the_one_renamed_onto_its_coordinates() {
|
||||
assert_drill_commits_to(
|
||||
&[
|
||||
(0, category("Type"), "Drink"),
|
||||
(1, category("Type"), "Food"),
|
||||
(1, measure("Cost"), "999"),
|
||||
],
|
||||
&[
|
||||
"Drink/Jan/Revenue = 1",
|
||||
"Drink/Jan/Cost = 2",
|
||||
"Food/Jan/Revenue = 42",
|
||||
"Food/Jan/Cost = 999",
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_record_rewrite_is_the_same_under_every_order_of_its_edits() {
|
||||
let app = two_record_drill();
|
||||
let row = app.drill_state.as_ref().unwrap().records[1].clone();
|
||||
let edits = [
|
||||
(measure("Cost"), "999"),
|
||||
(category("Type"), "Drink"),
|
||||
(measure("Revenue"), ""),
|
||||
(category("Month"), "Feb"),
|
||||
];
|
||||
let model = &app.model_state.workbook.model;
|
||||
let mut seen = Vec::new();
|
||||
for a in 0..4 {
|
||||
for b in (0..4).filter(|&b| b != a) {
|
||||
for c in (0..4).filter(|&c| c != a && c != b) {
|
||||
let d = 6 - a - b - c;
|
||||
let order = [a, b, c, d].map(|i| (&edits[i].0, edits[i].1));
|
||||
seen.push(RecordRewrite::of(model, &row, order));
|
||||
}
|
||||
}
|
||||
}
|
||||
assert_eq!(seen.len(), 24);
|
||||
let first = &seen[0];
|
||||
assert_eq!(
|
||||
first.clears,
|
||||
vec![
|
||||
drill_cell("Clothing", "Jan", "Revenue"),
|
||||
drill_cell("Clothing", "Jan", "Cost")
|
||||
]
|
||||
);
|
||||
assert_eq!(
|
||||
first.sets,
|
||||
vec![(drill_cell("Drink", "Feb", "Cost"), CellValue::Number(999.0))]
|
||||
);
|
||||
assert!(seen.iter().all(|rewrite| rewrite == first));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn two_drill_rows_renamed_onto_one_record_resolve_to_the_later_row() {
|
||||
assert_drill_commits_to(
|
||||
&[
|
||||
(0, category("Type"), "Drink"),
|
||||
(1, category("Type"), "Drink"),
|
||||
],
|
||||
&["Drink/Jan/Revenue = 42", "Drink/Jan/Cost = 17"],
|
||||
);
|
||||
}
|
||||
|
||||
// ── Toggle effects ──────────────────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
@@ -3283,6 +3545,38 @@ mod tests {
|
||||
assert_eq!(app.view_state.status_msg, "Imported successfully");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_json_import_stores_its_numbers_bit_for_bit() {
|
||||
const LOSSY: f64 = 0.248_157_142_857_142_85;
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("data.json");
|
||||
std::fs::write(
|
||||
&path,
|
||||
r#"[{"Region":"East","Revenue":0.24815714285714285}]"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let mut app = test_app();
|
||||
ImportJsonHeadless {
|
||||
path,
|
||||
model_name: None,
|
||||
array_path: None,
|
||||
}
|
||||
.apply(&mut app);
|
||||
|
||||
let key = CellKey::new(vec![
|
||||
("Region".to_string(), "East".to_string()),
|
||||
("_Measure".to_string(), "Revenue".to_string()),
|
||||
]);
|
||||
let Some(CellValue::Number(n)) = app.model_state.workbook.model.get_cell(&key) else {
|
||||
panic!(
|
||||
"the import must store the number; status {:?}",
|
||||
app.view_state.status_msg
|
||||
);
|
||||
};
|
||||
assert_eq!(n.to_bits(), LOSSY.to_bits(), "stored {n:?}, not {LOSSY:?}");
|
||||
}
|
||||
|
||||
// ── json_import_records (improvise-oaq helper) ──────────────────────
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -1927,4 +1927,66 @@ mod tests {
|
||||
assert_eq!(grid_value(&mut wb, "Q1", "Food"), None, "no Q1 row");
|
||||
assert_eq!(column_total(&wb), 100.0);
|
||||
}
|
||||
|
||||
/// improvise-t0ma: the steps themselves, not their translation into effects —
|
||||
/// see context/test-evidence/core.md.
|
||||
#[test]
|
||||
fn records_steps_are_the_spine_then_every_non_index_dim_category_on_none() {
|
||||
use super::{ViewStep, records_spine, records_steps};
|
||||
let mut m = crate::model::Model::new("T");
|
||||
m.add_category("Region").unwrap();
|
||||
m.add_category("Product").unwrap();
|
||||
let none = |c: &str| ViewStep::Axis {
|
||||
category: c.to_string(),
|
||||
axis: Axis::None,
|
||||
};
|
||||
let mut expected = records_spine();
|
||||
expected.extend(
|
||||
m.categories
|
||||
.keys()
|
||||
.filter(|c| !matches!(c.as_str(), "_Index" | "_Dim"))
|
||||
.map(|c| none(c)),
|
||||
);
|
||||
assert!(
|
||||
expected.contains(&none("_Measure")) && expected.contains(&none("Region")),
|
||||
"setup: the model carries _Measure and a regular category"
|
||||
);
|
||||
assert_eq!(records_steps(&m), expected);
|
||||
}
|
||||
|
||||
/// improvise-t0ma: a drill FIXES its coordinates on Filter and frees only the
|
||||
/// unfixed, non-virtual aggregated categories.
|
||||
#[test]
|
||||
fn drill_steps_filter_each_coordinate_and_free_the_rest() {
|
||||
use super::{ViewStep, drill_steps, records_spine};
|
||||
let key = CellKey::new(vec![
|
||||
("Region".into(), "North".into()),
|
||||
("Month".into(), "Feb".into()),
|
||||
("_Measure".into(), "Revenue".into()),
|
||||
]);
|
||||
let none_cats: Vec<String> = ["Product", "Region", "_Index", "Customer"]
|
||||
.map(String::from)
|
||||
.to_vec();
|
||||
let fixed = |c: &str, i: &str| {
|
||||
[
|
||||
ViewStep::Axis {
|
||||
category: c.into(),
|
||||
axis: Axis::Filter,
|
||||
},
|
||||
ViewStep::PageSelection {
|
||||
category: c.into(),
|
||||
item: i.into(),
|
||||
},
|
||||
]
|
||||
};
|
||||
let mut expected = records_spine();
|
||||
expected.extend(fixed("Month", "Feb"));
|
||||
expected.extend(fixed("Region", "North"));
|
||||
expected.extend(fixed("_Measure", "Revenue"));
|
||||
expected.extend(["Product", "Customer"].map(|c| ViewStep::Axis {
|
||||
category: c.into(),
|
||||
axis: Axis::None,
|
||||
}));
|
||||
assert_eq!(drill_steps(&key, &none_cats), expected);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -17,7 +17,9 @@ indexmap = { version = "2", features = ["serde"] }
|
||||
pest = "2.9.2"
|
||||
pest_derive = "2.9.2"
|
||||
serde = { version = "1", features = ["derive"] }
|
||||
serde_json = "1"
|
||||
# Parses JSON a user's numbers cross, so it must read back the f64 written: per crate,
|
||||
# because each build stands alone (improvise-6yvg, context/code/protocol.md).
|
||||
serde_json = { version = "1", features = ["float_roundtrip"] }
|
||||
|
||||
# The browser backend. Only the wasm build pulls these in; the native
|
||||
# binary never sees them (improvise-i34).
|
||||
|
||||
@@ -205,6 +205,28 @@ mod tests {
|
||||
assert_eq!(m2.model.name, "Budget");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_legacy_json_workbook_loads_its_numbers_bit_for_bit() {
|
||||
use crate::storage::{MemoryFs, Storage};
|
||||
const LOSSY: f64 = 0.248_157_142_857_142_85;
|
||||
let key = coord(&[("Type", "Food"), ("Month", "Jan")]);
|
||||
let mut wb = two_cat_model();
|
||||
wb.model.set_cell(key.clone(), CellValue::Number(LOSSY));
|
||||
let json = serde_json::to_string(&wb).unwrap();
|
||||
assert!(
|
||||
json.contains("0.24815714285714285"),
|
||||
"the file holds the exact text"
|
||||
);
|
||||
let fs = MemoryFs::new();
|
||||
let path = std::path::Path::new("legacy.improv");
|
||||
fs.write(path, json.as_bytes()).unwrap();
|
||||
let loaded = super::load_from(&fs, path).unwrap();
|
||||
let Some(CellValue::Number(n)) = loaded.model.get_cell(&key) else {
|
||||
panic!("the cell must load as a number");
|
||||
};
|
||||
assert_eq!(n.to_bits(), LOSSY.to_bits(), "loaded {n:?}, not {LOSSY:?}");
|
||||
}
|
||||
|
||||
// ── save/load roundtrip via file ────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -14,7 +14,8 @@ serde = { version = "1", features = ["derive"] }
|
||||
# `char`, and the TUI has always measured it this way.
|
||||
unicode-width = "^0.2"
|
||||
# Only for `variants`, which is off unless a test build asks for it.
|
||||
serde_json = { version = "1", optional = true }
|
||||
# float_roundtrip because every normal serde_json dep carries it (improvise-dz8c).
|
||||
serde_json = { version = "1", optional = true, features = ["float_roundtrip"] }
|
||||
|
||||
[features]
|
||||
# `variants` reaches OUT of this crate: improvise-command bounds its own
|
||||
|
||||
@@ -304,14 +304,11 @@ pub enum HelpLine {
|
||||
Heading(&'static str),
|
||||
Text(&'static str),
|
||||
Accent(&'static str),
|
||||
/// A keys-and-description row whose keys came off the keymap, so it
|
||||
/// cannot go stale against the bindings it documents.
|
||||
Keys {
|
||||
keys: Vec<String>,
|
||||
description: &'static str,
|
||||
},
|
||||
/// A row naming something the keymap does not own: an ex-command, or a
|
||||
/// prefix sequence, which is two addresses and not one binding.
|
||||
/// A keys-and-description row whose keys came off the keymap. A [`KeyRow`]
|
||||
/// has no public constructor, so no builder can type one (improvise-v29m).
|
||||
Keys(KeyRow),
|
||||
/// An ex-command, which the keymap does not own. Built only by `ex!`, whose
|
||||
/// colon is what stops a key being typed here (improvise-wk18).
|
||||
Literal {
|
||||
keys: &'static str,
|
||||
description: &'static str,
|
||||
@@ -326,38 +323,127 @@ pub struct HelpPage {
|
||||
pub lines: Vec<HelpLine>,
|
||||
}
|
||||
|
||||
/// A keys row, looked up in `mode`. An unbound command yields no keys and the
|
||||
/// row is dropped, so the page never advertises a key that does not exist.
|
||||
fn keys(
|
||||
mode: &ModeKey,
|
||||
command: &str,
|
||||
args: Option<&[&str]>,
|
||||
description: &'static str,
|
||||
) -> Option<HelpLine> {
|
||||
let args: Option<Vec<String>> = args.map(|a| a.iter().map(|s| (*s).to_string()).collect());
|
||||
let found = keys_for(mode, command, args.as_deref());
|
||||
match found.is_empty() {
|
||||
true => None,
|
||||
false => Some(HelpLine::Keys {
|
||||
keys: found.iter().map(|(c, m)| key_label(c, *m)).collect(),
|
||||
description,
|
||||
}),
|
||||
pub use key_row::KeyRow;
|
||||
use key_row::keys;
|
||||
|
||||
/// The one door to a keys row: its fields are private to this module, and its
|
||||
/// only constructor looks the keys up.
|
||||
mod key_row {
|
||||
use super::{HelpLine, ModeKey, key_label, keys_for, prefixed_labels};
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct KeyRow {
|
||||
keys: Vec<String>,
|
||||
description: &'static str,
|
||||
}
|
||||
|
||||
impl KeyRow {
|
||||
pub fn keys(&self) -> &[String] {
|
||||
&self.keys
|
||||
}
|
||||
|
||||
pub fn description(&self) -> &'static str {
|
||||
self.description
|
||||
}
|
||||
|
||||
/// An unbound command yields no keys and no row, so the page never
|
||||
/// advertises a key that does not exist.
|
||||
fn looked_up(
|
||||
mode: &ModeKey,
|
||||
command: &str,
|
||||
args: Option<&[String]>,
|
||||
description: &'static str,
|
||||
) -> Option<Self> {
|
||||
let mut keys: Vec<String> = keys_for(mode, command, args)
|
||||
.iter()
|
||||
.map(|(c, m)| key_label(c, *m))
|
||||
.collect();
|
||||
keys.extend(prefixed_labels(mode, command, args));
|
||||
(!keys.is_empty()).then_some(Self { keys, description })
|
||||
}
|
||||
}
|
||||
|
||||
/// A keys row, looked up in `mode`: the one caller of [`KeyRow::looked_up`],
|
||||
/// and the lookup `pages` hands every builder (improvise-ssoe).
|
||||
pub(super) fn keys(
|
||||
mode: &ModeKey,
|
||||
command: &'static str,
|
||||
args: Option<&[&str]>,
|
||||
description: &'static str,
|
||||
) -> Option<HelpLine> {
|
||||
let args: Option<Vec<String>> = args.map(|a| a.iter().map(|s| (*s).to_string()).collect());
|
||||
KeyRow::looked_up(mode, command, args.as_deref(), description).map(HelpLine::Keys)
|
||||
}
|
||||
}
|
||||
|
||||
fn literal(keys: &'static str, description: &'static str) -> HelpLine {
|
||||
HelpLine::Literal {
|
||||
keys,
|
||||
description,
|
||||
dim: false,
|
||||
/// `gg`, `yy`, `ZZ`: a prefix key and the key after it, both off the keymap.
|
||||
fn prefixed_labels(mode: &ModeKey, command: &str, args: Option<&[String]>) -> Vec<String> {
|
||||
let mut labels: Vec<String> = match Keymaps::shared().get(mode) {
|
||||
Some(keymap) => keymap.prefixed_keys_running(command, args),
|
||||
None => Vec::new(),
|
||||
}
|
||||
.iter()
|
||||
.map(|[(c1, m1), (c2, m2)]| format!("{}{}", key_label(c1, *m1), key_label(c2, *m2)))
|
||||
.collect();
|
||||
labels.sort();
|
||||
labels.dedup();
|
||||
labels
|
||||
}
|
||||
|
||||
fn dim(keys: &'static str, description: &'static str) -> HelpLine {
|
||||
HelpLine::Literal {
|
||||
keys,
|
||||
description,
|
||||
dim: true,
|
||||
/// How a page asks for a keys row. `pages` answers with [`keys`]; a test
|
||||
/// answers by recording the question, so a guard reads commands, not text.
|
||||
type KeysRow<'a> =
|
||||
&'a dyn Fn(&ModeKey, &'static str, Option<&[&str]>, &'static str) -> Option<HelpLine>;
|
||||
|
||||
/// An ex-command row. The colon is the macro's, and every word after the
|
||||
/// command's name must be a placeholder, checked when the page compiles.
|
||||
macro_rules! ex {
|
||||
($text:literal, $description:expr $(,)?) => {{
|
||||
const _: () = assert!(ex_usage_is_placeholders($text));
|
||||
HelpLine::Literal {
|
||||
keys: concat!(":", $text),
|
||||
description: $description,
|
||||
dim: false,
|
||||
}
|
||||
}};
|
||||
(dim $text:literal, $description:expr $(,)?) => {{
|
||||
const _: () = assert!(ex_usage_is_placeholders($text));
|
||||
HelpLine::Literal {
|
||||
keys: concat!(" :", $text),
|
||||
description: $description,
|
||||
dim: true,
|
||||
}
|
||||
}};
|
||||
}
|
||||
|
||||
/// After the name, every word opens with `<`, `[` or `"`, or is `...`: so an
|
||||
/// ex row cannot also advertise a key ("q or press Q"). Quotes span spaces.
|
||||
const fn ex_usage_is_placeholders(text: &str) -> bool {
|
||||
let b = text.as_bytes();
|
||||
let mut i = 0;
|
||||
while i < b.len() && b[i] != b' ' {
|
||||
i += 1;
|
||||
}
|
||||
loop {
|
||||
while i < b.len() && b[i] == b' ' {
|
||||
i += 1;
|
||||
}
|
||||
if i == b.len() {
|
||||
return true;
|
||||
}
|
||||
let start = i;
|
||||
let mut quoted = false;
|
||||
while i < b.len() && (quoted || b[i] != b' ') {
|
||||
if b[i] == b'"' {
|
||||
quoted = !quoted;
|
||||
}
|
||||
i += 1;
|
||||
}
|
||||
let ellipsis =
|
||||
i - start == 3 && b[start] == b'.' && b[start + 1] == b'.' && b[start + 2] == b'.';
|
||||
if !(ellipsis || b[start] == b'<' || b[start] == b'[' || b[start] == b'"') {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -367,30 +453,48 @@ pub fn pages() -> Vec<HelpPage> {
|
||||
let pages = vec![
|
||||
HelpPage {
|
||||
title: "Welcome",
|
||||
lines: welcome(),
|
||||
lines: welcome(&keys),
|
||||
},
|
||||
HelpPage {
|
||||
title: "Navigation",
|
||||
lines: navigation(),
|
||||
lines: navigation(&keys),
|
||||
},
|
||||
HelpPage {
|
||||
title: "Editing",
|
||||
lines: editing(),
|
||||
lines: editing(&keys),
|
||||
},
|
||||
HelpPage {
|
||||
title: "Panels & Views",
|
||||
lines: panels(),
|
||||
lines: panels(&keys),
|
||||
},
|
||||
HelpPage {
|
||||
title: "Commands",
|
||||
lines: commands(),
|
||||
lines: commands(&keys),
|
||||
},
|
||||
];
|
||||
debug_assert_eq!(pages.len(), HELP_PAGE_COUNT);
|
||||
pages
|
||||
}
|
||||
|
||||
fn welcome() -> Vec<HelpLine> {
|
||||
/// Every keys row the pages ask for, as `(mode, command, args, description)`
|
||||
/// — the same builders, so a guard over it cannot drift from what is drawn.
|
||||
#[cfg(test)]
|
||||
pub(crate) fn help_bindings() -> Vec<(ModeKey, &'static str, Option<Vec<String>>, &'static str)> {
|
||||
let asked = std::cell::RefCell::new(Vec::new());
|
||||
let record: KeysRow = &|mode, command, args, description| {
|
||||
let args = args.map(|a| a.iter().map(|s| (*s).to_string()).collect());
|
||||
asked
|
||||
.borrow_mut()
|
||||
.push((mode.clone(), command, args, description));
|
||||
None
|
||||
};
|
||||
for build in [welcome, navigation, editing, panels, commands] {
|
||||
build(record);
|
||||
}
|
||||
asked.into_inner()
|
||||
}
|
||||
|
||||
fn welcome(_: KeysRow) -> Vec<HelpLine> {
|
||||
vec![
|
||||
HelpLine::Blank,
|
||||
HelpLine::Banner(" improvise"),
|
||||
@@ -427,12 +531,10 @@ fn welcome() -> Vec<HelpLine> {
|
||||
),
|
||||
HelpLine::Text(" Tile The row/column/page assignment of a category."),
|
||||
HelpLine::Text(" Formula A computed item: derives its value from other items."),
|
||||
HelpLine::Blank,
|
||||
dim("Tip: press Tab or l/n to go to the next page.", ""),
|
||||
]
|
||||
}
|
||||
|
||||
fn navigation() -> Vec<HelpLine> {
|
||||
fn navigation(keys: KeysRow) -> Vec<HelpLine> {
|
||||
let n = &ModeKey::Normal;
|
||||
[
|
||||
Some(HelpLine::Blank),
|
||||
@@ -445,7 +547,7 @@ fn navigation() -> Vec<HelpLine> {
|
||||
None,
|
||||
"Move down, wrapping to the next column",
|
||||
),
|
||||
Some(literal("gg", "Jump to first row")),
|
||||
keys(n, "jump-first-row", None, "Jump to first row"),
|
||||
keys(n, "jump-last-row", None, "Jump to last row"),
|
||||
keys(n, "jump-first-col", None, "Jump to first column"),
|
||||
keys(n, "jump-last-col", None, "Jump to last column"),
|
||||
@@ -512,7 +614,7 @@ fn navigation() -> Vec<HelpLine> {
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn editing() -> Vec<HelpLine> {
|
||||
fn editing(keys: KeysRow) -> Vec<HelpLine> {
|
||||
let n = &ModeKey::Normal;
|
||||
let e = &ModeKey::Editing;
|
||||
[
|
||||
@@ -555,7 +657,7 @@ fn editing() -> Vec<HelpLine> {
|
||||
Some(HelpLine::Blank),
|
||||
Some(HelpLine::Heading("Copy and paste")),
|
||||
Some(HelpLine::Blank),
|
||||
Some(literal("yy", "Yank (copy) the current cell value")),
|
||||
keys(n, "yank", None, "Yank (copy) the current cell value"),
|
||||
keys(
|
||||
n,
|
||||
"paste",
|
||||
@@ -593,7 +695,7 @@ fn editing() -> Vec<HelpLine> {
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn panels() -> Vec<HelpLine> {
|
||||
fn panels(keys: KeysRow) -> Vec<HelpLine> {
|
||||
let n = &ModeKey::Normal;
|
||||
[
|
||||
Some(HelpLine::Blank),
|
||||
@@ -741,14 +843,14 @@ fn panels() -> Vec<HelpLine> {
|
||||
"Toggle collapse of nearest group above cursor",
|
||||
),
|
||||
keys(n, "hide-selected-row-item", None, "Hide current row item"),
|
||||
Some(dim(" :show-item <cat> <item>", "Restore a hidden item")),
|
||||
Some(ex!(dim "show-item <cat> <item>", "Restore a hidden item")),
|
||||
]
|
||||
.into_iter()
|
||||
.flatten()
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn commands() -> Vec<HelpLine> {
|
||||
fn commands(keys: KeysRow) -> Vec<HelpLine> {
|
||||
let n = &ModeKey::Normal;
|
||||
[
|
||||
Some(HelpLine::Blank),
|
||||
@@ -763,55 +865,40 @@ fn commands() -> Vec<HelpLine> {
|
||||
Some(HelpLine::Blank),
|
||||
Some(HelpLine::Heading("File operations")),
|
||||
Some(HelpLine::Blank),
|
||||
Some(literal(
|
||||
":w [path]",
|
||||
"Save (path optional after first save)",
|
||||
)),
|
||||
Some(literal(":wq", "Save and quit")),
|
||||
Some(literal(":q", "Quit (warns if unsaved changes)")),
|
||||
Some(literal(":q!", "Force quit without saving")),
|
||||
Some(literal("ZZ", "Save and quit (same as :wq)")),
|
||||
Some(ex!("w [path]", "Save (path optional after first save)",)),
|
||||
Some(ex!("wq", "Save and quit")),
|
||||
Some(ex!("q", "Quit (warns if unsaved changes)")),
|
||||
Some(ex!("q!", "Force quit without saving")),
|
||||
keys(n, "wq", None, "Save and quit (same as :wq)"),
|
||||
keys(n, "save", None, "Save (same as :w)"),
|
||||
Some(HelpLine::Blank),
|
||||
Some(HelpLine::Heading("Import and export")),
|
||||
Some(HelpLine::Blank),
|
||||
Some(literal(":import <path>", "Open the JSON/CSV import wizard")),
|
||||
Some(literal(
|
||||
":export [path.csv]",
|
||||
"Export the active view to CSV",
|
||||
)),
|
||||
Some(ex!("import <path>", "Open the JSON/CSV import wizard")),
|
||||
Some(ex!("export [path.csv]", "Export the active view to CSV",)),
|
||||
Some(HelpLine::Blank),
|
||||
Some(HelpLine::Heading("Model building")),
|
||||
Some(HelpLine::Blank),
|
||||
Some(literal(":add-cat <name>", "Add a new category")),
|
||||
Some(literal(
|
||||
":add-item <cat> <item>",
|
||||
"Add one item to a category",
|
||||
)),
|
||||
Some(literal(
|
||||
":add-items <cat> a b c ...",
|
||||
Some(ex!("add-cat <name>", "Add a new category")),
|
||||
Some(ex!("add-item <cat> <item>", "Add one item to a category",)),
|
||||
Some(ex!(
|
||||
"add-items <cat> <item> ...",
|
||||
"Add multiple items at once",
|
||||
)),
|
||||
Some(literal(
|
||||
":formula <cat> \"<Name> = <expr>\"",
|
||||
"Add a formula",
|
||||
)),
|
||||
Some(ex!("formula <cat> \"<Name> = <expr>\"", "Add a formula",)),
|
||||
Some(HelpLine::Blank),
|
||||
Some(HelpLine::Heading("Views")),
|
||||
Some(HelpLine::Blank),
|
||||
Some(literal(":add-view [name]", "Create a new view")),
|
||||
Some(ex!("add-view [name]", "Create a new view")),
|
||||
Some(HelpLine::Blank),
|
||||
Some(HelpLine::Heading("Display")),
|
||||
Some(HelpLine::Blank),
|
||||
Some(literal(
|
||||
":set-format <fmt>",
|
||||
"Set number format (e.g. ',.2')",
|
||||
)),
|
||||
Some(ex!("set-format <fmt>", "Set number format (e.g. ',.2')",)),
|
||||
Some(HelpLine::Blank),
|
||||
Some(HelpLine::Heading("Other keys")),
|
||||
Some(HelpLine::Blank),
|
||||
keys(n, "enter-mode", Some(&["help"]), "Open this help screen"),
|
||||
Some(literal(":help", "Open this help screen")),
|
||||
Some(ex!("help", "Open this help screen")),
|
||||
]
|
||||
.into_iter()
|
||||
.flatten()
|
||||
@@ -884,8 +971,12 @@ mod tests {
|
||||
for page in &pages {
|
||||
assert!(!page.title.is_empty());
|
||||
for line in &page.lines {
|
||||
if let HelpLine::Keys { keys, description } = line {
|
||||
assert!(!keys.is_empty(), "{description} resolved to no keys");
|
||||
if let HelpLine::Keys(row) = line {
|
||||
assert!(
|
||||
!row.keys().is_empty(),
|
||||
"{} resolved to no keys",
|
||||
row.description()
|
||||
);
|
||||
rows += 1;
|
||||
}
|
||||
}
|
||||
@@ -896,6 +987,84 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-wk18: a key the pages name is looked up, never typed. The only
|
||||
/// hand-written row left is an ex-command. See `context/test-evidence/protocol.md`.
|
||||
#[test]
|
||||
fn no_help_row_spells_a_key_by_hand() {
|
||||
let spelled: Vec<&str> = pages()
|
||||
.iter()
|
||||
.flat_map(|page| page.lines.iter())
|
||||
.filter_map(|line| match line {
|
||||
HelpLine::Literal { keys, .. } => Some(*keys),
|
||||
_ => None,
|
||||
})
|
||||
.filter(|keys| !keys.trim_start().starts_with(':'))
|
||||
.collect();
|
||||
assert!(
|
||||
spelled.is_empty(),
|
||||
"these rows name keys the keymap never saw: {spelled:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-v29m: `ex!` refuses a usage with a bare word in it at compile
|
||||
/// time; this pins the rule it asks. See `context/test-evidence/protocol.md`.
|
||||
#[test]
|
||||
fn an_ex_row_can_name_placeholders_and_nothing_else() {
|
||||
for usage in [
|
||||
"w [path]",
|
||||
"q!",
|
||||
"formula <cat> \"<Name> = <expr>\"",
|
||||
"add-items <cat> <item> ...",
|
||||
] {
|
||||
assert!(ex_usage_is_placeholders(usage), "{usage} is refused");
|
||||
}
|
||||
for usage in ["q or press Q", "add-items <cat> a b c", "help ?"] {
|
||||
assert!(!ex_usage_is_placeholders(usage), "{usage} is accepted");
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-ssoe: a keys row drawn but never asked for through the builders'
|
||||
/// parameter is invisible to every guard. See `context/test-evidence/protocol.md`.
|
||||
#[test]
|
||||
fn every_keys_row_drawn_is_one_the_builders_asked_for() {
|
||||
let drawn: Vec<HelpLine> = pages()
|
||||
.into_iter()
|
||||
.flat_map(|page| page.lines)
|
||||
.filter(|line| matches!(line, HelpLine::Keys(_)))
|
||||
.collect();
|
||||
// Content, in order, not a count: a bypass that also drops a real row
|
||||
// keeps the count and changes this (improvise-aszt).
|
||||
let asked: Vec<HelpLine> = help_bindings()
|
||||
.iter()
|
||||
.filter_map(|(mode, command, args, description)| {
|
||||
let args: Option<Vec<&str>> = args
|
||||
.as_ref()
|
||||
.map(|a| a.iter().map(String::as_str).collect());
|
||||
keys(mode, command, args.as_deref(), description)
|
||||
})
|
||||
.collect();
|
||||
assert_eq!(drawn, asked);
|
||||
}
|
||||
|
||||
/// A row whose lookup finds no key is dropped silently, so a broken lookup
|
||||
/// looks like a shorter page. See `context/test-evidence/protocol.md`.
|
||||
#[test]
|
||||
fn every_keys_row_the_pages_ask_for_finds_its_keys() {
|
||||
let asked = help_bindings();
|
||||
assert!(asked.len() > 20, "only {} rows asked", asked.len());
|
||||
let dropped: Vec<String> = asked
|
||||
.iter()
|
||||
.filter(|(mode, command, args, description)| {
|
||||
let args: Option<Vec<&str>> = args
|
||||
.as_ref()
|
||||
.map(|a| a.iter().map(String::as_str).collect());
|
||||
keys(mode, command, args.as_deref(), description).is_none()
|
||||
})
|
||||
.map(|(mode, command, _, _)| format!("{mode:?} {command}"))
|
||||
.collect();
|
||||
assert!(dropped.is_empty(), "rows with no key: {dropped:?}");
|
||||
}
|
||||
|
||||
/// `compact`'s three shapes, including the one no shipped mode reaches:
|
||||
/// keys that are all chords leave nothing to draw.
|
||||
#[test]
|
||||
@@ -966,7 +1135,7 @@ mod tests {
|
||||
.into_iter()
|
||||
.flat_map(|page| page.lines)
|
||||
.filter_map(|line| match line {
|
||||
HelpLine::Keys { keys, description } => Some((keys, description)),
|
||||
HelpLine::Keys(row) => Some((row.keys().to_vec(), row.description())),
|
||||
_ => None,
|
||||
})
|
||||
.collect()
|
||||
@@ -998,16 +1167,26 @@ mod tests {
|
||||
/// 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()
|
||||
let normal: Vec<_> = help_bindings()
|
||||
.into_iter()
|
||||
.filter(|(_, d)| d.contains("Edit current cell"))
|
||||
.filter(|(mode, ..)| *mode == ModeKey::Normal)
|
||||
.collect();
|
||||
assert!(!claims.is_empty(), "the check needs a row to check");
|
||||
for (keys, description) in claims {
|
||||
// By COMMAND, not phrase (improvise-jhdf): any wording of "edit" on a
|
||||
// Normal row that is not edit-or-drill's is the lie s8h7 removed.
|
||||
let liars: Vec<_> = normal
|
||||
.iter()
|
||||
.filter(|(_, command, _, d)| {
|
||||
d.to_lowercase().contains("edit") && *command != "edit-or-drill"
|
||||
})
|
||||
.collect();
|
||||
assert!(
|
||||
liars.is_empty(),
|
||||
"captioned as editing but do not: {liars:?}"
|
||||
);
|
||||
for command in ["edit-or-drill", "enter-advance"] {
|
||||
assert!(
|
||||
keys.iter().all(|k| edit.contains(k)),
|
||||
"{keys:?} is captioned {description:?} but only {edit:?} edit"
|
||||
normal.iter().any(|(_, c, ..)| *c == command),
|
||||
"{command} has no Normal help row, so the check above cannot see it"
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -1024,8 +1203,8 @@ mod tests {
|
||||
println!("\n=== {} ===", page.title);
|
||||
for line in &page.lines {
|
||||
match line {
|
||||
HelpLine::Keys { keys, description } => {
|
||||
println!(" {:<28}{description}", keys.join(" "))
|
||||
HelpLine::Keys(row) => {
|
||||
println!(" {:<28}{}", row.keys().join(" "), row.description())
|
||||
}
|
||||
HelpLine::Literal {
|
||||
keys, description, ..
|
||||
|
||||
@@ -235,6 +235,32 @@ impl Keymap {
|
||||
keys
|
||||
}
|
||||
|
||||
/// Every two-key sequence here or in a parent that runs `command`: a
|
||||
/// prefix key, then a key of the sub-keymap it opens (flat, gotcha 13).
|
||||
pub fn prefixed_keys_running(
|
||||
&self,
|
||||
command: &str,
|
||||
args: Option<&[String]>,
|
||||
) -> Vec<[(KeyCode, KeyModifiers); 2]> {
|
||||
let mut keys: Vec<[(KeyCode, KeyModifiers); 2]> = self
|
||||
.bindings
|
||||
.iter()
|
||||
.filter_map(|(pattern, binding)| match (pattern, binding) {
|
||||
(KeyPattern::Key(code, mods), Binding::Prefix(sub)) => Some((*code, *mods, sub)),
|
||||
_ => None,
|
||||
})
|
||||
.flat_map(|(code, mods, sub)| {
|
||||
sub.keys_running(command, args)
|
||||
.into_iter()
|
||||
.map(move |second| [(code, mods), second])
|
||||
})
|
||||
.collect();
|
||||
if let Some(parent) = &self.parent {
|
||||
keys.extend(parent.prefixed_keys_running(command, args));
|
||||
}
|
||||
keys
|
||||
}
|
||||
|
||||
/// Look up the binding for a key in this keymap's own bindings.
|
||||
fn lookup_local(&self, key: KeyCode, mods: KeyModifiers) -> Option<&Binding> {
|
||||
self.bindings
|
||||
|
||||
@@ -619,6 +619,11 @@ enum GridMove {
|
||||
Page(i32),
|
||||
}
|
||||
|
||||
/// Rows one `page-scroll` step moves: three quarters of the window, at least one.
|
||||
fn page_rows(window_rows: usize) -> i32 {
|
||||
(window_rows as i32 * 3 / 4).max(1)
|
||||
}
|
||||
|
||||
fn grid_move(state: &ClientState, kind: GridMove) -> Option<Vec<Command>> {
|
||||
let GridExtent { rows, cols } = state.grid.extent;
|
||||
let (window_rows, window_cols) = state.window;
|
||||
@@ -641,7 +646,7 @@ fn grid_move(state: &ClientState, kind: GridMove) -> Option<Vec<Command>> {
|
||||
GridMove::ToEnd(true) => (row_max as usize, cursor.selected.1),
|
||||
GridMove::ToEnd(false) => (cursor.selected.0, col_max as usize),
|
||||
GridMove::Page(dir) => {
|
||||
let delta = (window_rows as i32 * 3 / 4).max(1) * dir;
|
||||
let delta = page_rows(window_rows) * dir;
|
||||
((row + delta).clamp(0, row_max) as usize, cursor.selected.1)
|
||||
}
|
||||
};
|
||||
@@ -2044,18 +2049,60 @@ mod tests {
|
||||
|
||||
// ── what is bound and what is not ───────────────────────────────
|
||||
|
||||
/// A three-by-three grid with `id` at (1,1) and the cursor on it: no move
|
||||
/// from there is clamped by an edge (improvise-6562).
|
||||
fn centred_on(id: CellId) -> ClientState {
|
||||
let mut centre = CellDisplay::number("1200");
|
||||
centre.id = Some(id);
|
||||
let mut cells: Vec<_> = (0..3)
|
||||
.flat_map(|r| (0..3).map(move |c| ((r, c), CellDisplay::number("7"))))
|
||||
.collect();
|
||||
cells[4].1 = centre;
|
||||
let mut client = ClientState::default();
|
||||
client.bootstrap(Snapshot {
|
||||
view: ViewState::default(),
|
||||
seed: vec![
|
||||
Projection::Grid(GridUpdate::RowFrame(AxisFrame::flat([
|
||||
"North", "South", "East",
|
||||
]))),
|
||||
Projection::Grid(GridUpdate::ColFrame(AxisFrame::flat([
|
||||
"Revenue", "Cost", "Profit",
|
||||
]))),
|
||||
Projection::Grid(GridUpdate::Cells(cells)),
|
||||
Projection::Grid(GridUpdate::Extent(GridExtent { rows: 3, cols: 3 })),
|
||||
Projection::Basis(published()),
|
||||
],
|
||||
});
|
||||
client.window = (3, 3);
|
||||
client.view.cursor_mut().selected = (1, 1);
|
||||
client
|
||||
}
|
||||
|
||||
/// A client whose every widget has something in it, so an `Upstream` names
|
||||
/// an **unhandled command** and never a state that said no (improvise-rmvx).
|
||||
fn furnished(mode: AppMode) -> ClientState {
|
||||
let mut state = client_on(record_cell());
|
||||
furnished_on(mode, record_cell())
|
||||
}
|
||||
|
||||
/// `furnished`, with `cell` under the cursor. A history in both directions
|
||||
/// and a yanked value, so no `Cmd` is refused by an empty stack (3t5x).
|
||||
fn furnished_on(mode: AppMode, cell: CellId) -> ClientState {
|
||||
let mut state = centred_on(cell);
|
||||
let frame = |name: &str| crate::ViewFrame {
|
||||
view_name: name.into(),
|
||||
mode: AppMode::Normal,
|
||||
};
|
||||
state.view.view_back_stack = vec![frame("Second")];
|
||||
state.view.view_forward_stack = vec![frame("Third")];
|
||||
state.view.yanked = Some(CellValue::Number(5.0));
|
||||
// A records MODE over a non-records GRID is a state no host can
|
||||
// produce, so the fixture projects both (improvise-e5pf).
|
||||
crate::WidgetCache::apply(&mut state.grid, &GridUpdate::Records(mode.is_records()));
|
||||
state.view.mode = mode;
|
||||
state.grid.page_coords = vec![crate::PageAxis {
|
||||
cat: "Quarter".into(),
|
||||
selected: "Q1".into(),
|
||||
items: Some(vec!["Q1".into(), "Q2".into()]),
|
||||
selected: "Q2".into(),
|
||||
items: Some(vec!["Q1".into(), "Q2".into(), "Q3".into()]),
|
||||
}];
|
||||
// THREE of everything, every panel cursor seated in the MIDDLE: an edge
|
||||
// makes `move-panel-cursor` answer `Ignored` for the wrong reason.
|
||||
@@ -2087,10 +2134,15 @@ mod tests {
|
||||
axes: Vec::new(),
|
||||
})
|
||||
.collect();
|
||||
state.tiles.tiles = vec![crate::TileEntry {
|
||||
category: "Region".into(),
|
||||
axis: Axis::Row,
|
||||
}];
|
||||
state.tiles.tiles = ["Region", "Product", "Quarter"]
|
||||
.into_iter()
|
||||
.map(|category| crate::TileEntry {
|
||||
category: category.into(),
|
||||
axis: Axis::Row,
|
||||
})
|
||||
.collect();
|
||||
state.view.tile_cat_idx = 1;
|
||||
state.view.help_page = 1;
|
||||
state.view.cat_panel_cursor = 1;
|
||||
state.view.formula_cursor = 1;
|
||||
state.view.view_panel_cursor = 1;
|
||||
@@ -2107,59 +2159,298 @@ mod tests {
|
||||
state
|
||||
}
|
||||
|
||||
/// improvise-rmvx. An advertised key that resolves upstream is a black hole
|
||||
/// (gotcha 47). See `context/test-evidence/clients.md`.
|
||||
/// What a user sees an advertised key do. A status alone is `Declines`:
|
||||
/// honest, but not what the caption promised (improvise-3t5x).
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
|
||||
enum Heard {
|
||||
Acts,
|
||||
Declines,
|
||||
Silent,
|
||||
}
|
||||
|
||||
/// Judged by what the fold MOVED, never by a command's type: a clamped move
|
||||
/// or a lone disarm changes nothing (improvise-6562). `Model` is sent.
|
||||
fn heard(state: &mut ClientState, outcome: KeyOutcome) -> Heard {
|
||||
let commands = match outcome {
|
||||
KeyOutcome::Upstream(_) | KeyOutcome::Ignored => return Heard::Silent,
|
||||
KeyOutcome::Request(_) => return Heard::Acts,
|
||||
KeyOutcome::Local(commands) => commands,
|
||||
};
|
||||
let quiet = |view: &ViewState| ViewState {
|
||||
status_msg: String::new(),
|
||||
transient_prefix: None,
|
||||
..view.clone()
|
||||
};
|
||||
let before = quiet(&state.view);
|
||||
commands.iter().for_each(|c| state.fold(c));
|
||||
let sent = commands.iter().any(|c| matches!(c, Command::Model { .. }));
|
||||
let said = commands.iter().any(|c| matches!(c, Command::SetStatus(_)));
|
||||
match (sent || quiet(&state.view) != before, said) {
|
||||
(true, _) => Heard::Acts,
|
||||
(false, true) => Heard::Declines,
|
||||
(false, false) => Heard::Silent,
|
||||
}
|
||||
}
|
||||
|
||||
/// Every `(mode, search, command, args)` either table advertises: the
|
||||
/// status-bar hint and the help pages (improvise-wk18).
|
||||
fn advertised_everywhere() -> Vec<(AppMode, bool, &'static str, Option<Vec<String>>)> {
|
||||
let mut rows = Vec::new();
|
||||
for mode in AppMode::all() {
|
||||
for (command, args, _) in crate::help::advertised(&mode) {
|
||||
rows.push((mode.clone(), false, command, args));
|
||||
}
|
||||
}
|
||||
for (key, command, args, _) in crate::help::help_bindings() {
|
||||
let (mode, search) = match key {
|
||||
ModeKey::SearchMode => (AppMode::Normal, true),
|
||||
key => match AppMode::all()
|
||||
.into_iter()
|
||||
.find(|m| ModeKey::from_app_mode(m, false).as_ref() == Some(&key))
|
||||
{
|
||||
Some(mode) => (mode, false),
|
||||
None => panic!("a help row names {key:?}, which no mode reaches"),
|
||||
},
|
||||
};
|
||||
rows.push((mode, search, command, args));
|
||||
}
|
||||
rows.sort_by_key(|(m, s, c, a)| (crate::mode_label(m), *s, *c, a.clone()));
|
||||
rows.dedup();
|
||||
rows
|
||||
}
|
||||
|
||||
/// Every key sequence that runs `command` in `keymap`: one key, or a
|
||||
/// prefix and the key after it.
|
||||
fn sequences_running(
|
||||
keymap: &Keymap,
|
||||
command: &str,
|
||||
args: Option<&[String]>,
|
||||
) -> Vec<Vec<KeyEvent>> {
|
||||
let one = keymap
|
||||
.keys_running(command, args)
|
||||
.into_iter()
|
||||
.map(|(code, mods)| vec![KeyEvent::new(code, mods)]);
|
||||
let two = keymap
|
||||
.prefixed_keys_running(command, args)
|
||||
.into_iter()
|
||||
.map(|pair| pair.map(|(code, mods)| KeyEvent::new(code, mods)).to_vec());
|
||||
one.chain(two).collect()
|
||||
}
|
||||
|
||||
/// What pressing `sequence` in `state` does, arming any prefix first.
|
||||
fn press(mut state: ClientState, sequence: &[KeyEvent]) -> Heard {
|
||||
let (last, prefix) = sequence.split_last().expect("a sequence has a key");
|
||||
for key in prefix {
|
||||
if let KeyOutcome::Local(commands) = resolve(&state, *key) {
|
||||
commands.iter().for_each(|c| state.fold(c));
|
||||
}
|
||||
}
|
||||
let outcome = resolve(&state, *last);
|
||||
heard(&mut state, outcome)
|
||||
}
|
||||
|
||||
/// improvise-rmvx, wk18, 3t5x. Every advertised key does what it says, or
|
||||
/// is named below saying why not. See `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn every_advertised_command_answers_in_its_own_mode() {
|
||||
let mut unanswered = Vec::new();
|
||||
for mode in AppMode::all() {
|
||||
let Some(mode_key) = ModeKey::from_app_mode(&mode, false) else {
|
||||
continue;
|
||||
let aggregate = CellId::Aggregate(
|
||||
serde_json::from_str("\"aa4c8e5e9cbd578e0d388f4b2b5a7879\"").unwrap(),
|
||||
);
|
||||
let derived = CellId::Derived {
|
||||
target: "Profit".into(),
|
||||
};
|
||||
// (mode, cell, command, heard) -> the keys that heard it.
|
||||
let mut short: std::collections::BTreeMap<(String, &str, Heard), Vec<String>> =
|
||||
Default::default();
|
||||
for (mode, search, command, args) in advertised_everywhere() {
|
||||
let key = ModeKey::from_app_mode(&mode, search).expect("a keyed mode");
|
||||
let keymap = Keymaps::shared().get(&key).expect("a keymap");
|
||||
let at = |cell: &CellId| {
|
||||
let mut state = furnished_on(mode.clone(), cell.clone());
|
||||
state.view.search_mode = search;
|
||||
state
|
||||
};
|
||||
let Some(keymap) = Keymaps::shared().get(&mode_key) else {
|
||||
continue;
|
||||
};
|
||||
let state = furnished(mode.clone());
|
||||
for (command, args, caption) in crate::help::advertised(&mode) {
|
||||
for (code, mods) in keymap.keys_running(command, args.as_deref()) {
|
||||
let event = KeyEvent::new(code, mods);
|
||||
// BOTH silent variants. To a user `Ignored` is the same
|
||||
// black hole as `Upstream` — nothing is on the screen.
|
||||
let answered = !matches!(
|
||||
resolve(&state, event),
|
||||
KeyOutcome::Upstream(_) | KeyOutcome::Ignored
|
||||
);
|
||||
if !answered {
|
||||
unanswered.push(format!(
|
||||
"{}: {:?} → `{command} {args:?}` ({caption})",
|
||||
crate::mode_label(&mode),
|
||||
code
|
||||
));
|
||||
let label = crate::mode_label(&mode).to_string() + if search { " SEARCH" } else { "" };
|
||||
for sequence in sequences_running(keymap, command, args.as_deref()) {
|
||||
let keys: String = sequence
|
||||
.iter()
|
||||
.map(|k| crate::keymap::key_label(&k.code, k.modifiers))
|
||||
.collect();
|
||||
let on_record = press(at(&record_cell()), &sequence);
|
||||
let mut report = vec![(label.clone(), on_record)];
|
||||
// Only a grid mode stands on another kind of cell, and a cell
|
||||
// matters only where it changes the answer.
|
||||
if mode == AppMode::Normal {
|
||||
for (on, cell) in [
|
||||
(" on an aggregate", &aggregate),
|
||||
(" on a derived cell", &derived),
|
||||
] {
|
||||
let got = press(at(cell), &sequence);
|
||||
if got != on_record {
|
||||
report.push((label.clone() + on, got));
|
||||
}
|
||||
}
|
||||
}
|
||||
for (where_, got) in report.into_iter().filter(|(_, got)| *got != Heard::Acts) {
|
||||
short
|
||||
.entry((where_, command, got))
|
||||
.or_default()
|
||||
.push(keys.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
unanswered.sort();
|
||||
unanswered.dedup();
|
||||
// The five that remain, each with the reason it is not a black hole.
|
||||
// Closing one has to come here and say so; opening a sixth fails.
|
||||
let expected = [
|
||||
let short: Vec<String> = short
|
||||
.into_iter()
|
||||
.map(|((at, command, got), mut keys)| {
|
||||
keys.sort();
|
||||
keys.dedup();
|
||||
format!("{at}: {} {command} {got:?}", keys.join("/"))
|
||||
})
|
||||
.collect();
|
||||
// Closing one has to come here and say so; adding one fails. A caption
|
||||
// is a promise, and each line here is a promise a browser does not keep.
|
||||
let expected: &[&str] = &[
|
||||
// Reached only from a mode `declined` refuses at the door, so a
|
||||
// browser is never standing in front of this hint.
|
||||
"COMMAND: Enter → `execute-command None` (run)",
|
||||
"EXPORT: Enter → `commit-export None` (export)",
|
||||
"SAVE AS: Enter → `commit-save None` (save)",
|
||||
// browser is never standing in front of these.
|
||||
"COMMAND: Enter execute-command Silent",
|
||||
"EXPORT: Enter commit-export Silent",
|
||||
"SAVE AS: Enter commit-save Silent",
|
||||
"NORMAL SEARCH: Enter/Esc exit-search-mode Silent",
|
||||
// Commands the browser has no arm for: upstream, so a black hole
|
||||
// (improvise-uln5). Each is also in the host-only list below.
|
||||
"NORMAL: Enter enter-advance Silent",
|
||||
"NORMAL: H hide-selected-row-item Silent",
|
||||
"NORMAL: / search Silent",
|
||||
"NORMAL: n search-navigate Silent",
|
||||
"NORMAL: z toggle-group-under-cursor Silent",
|
||||
// Honestly unavailable, and saying so in `declined`'s words.
|
||||
"FORMULAS: Del/d delete-formula-at-cursor Declines",
|
||||
"NORMAL: > drill-into-cell Declines",
|
||||
"NORMAL: : enter-mode Declines",
|
||||
"NORMAL: P toggle-prune-empty Declines",
|
||||
"NORMAL: R toggle-records-mode Declines",
|
||||
"NORMAL: ZZ wq Declines",
|
||||
"RECORDS: o add-record-row Declines",
|
||||
"RECORDS: a/i edit-or-drill Declines",
|
||||
"RECORDS: : enter-mode Declines",
|
||||
"RECORDS: P toggle-prune-empty Declines",
|
||||
"RECORDS: R toggle-records-mode Declines",
|
||||
// `i` on an aggregate IS drill, which the browser lacks (s8h7); on a
|
||||
// derived cell the browser names the formula instead (ir7l).
|
||||
"NORMAL on an aggregate: a/i edit-or-drill Declines",
|
||||
"NORMAL on a derived cell: a/i edit-or-drill Declines",
|
||||
// No record under the cursor to write. The TUI writes onto the
|
||||
// aggregate's partial key, which is its own bug (improvise-l4mp).
|
||||
"NORMAL on an aggregate: p paste Silent",
|
||||
"NORMAL on an aggregate: x clear-cell Silent",
|
||||
"NORMAL on a derived cell: p paste Silent",
|
||||
"NORMAL on a derived cell: x clear-cell Silent",
|
||||
// A records cell commit needs `record_col` (gotcha 1); only the two
|
||||
// creating columns resolve, and this fixture is on a record cell.
|
||||
"RECORDS INSERT: Enter → `commit-cell-edit None` (commit)",
|
||||
"RECORDS INSERT: Tab → `commit-and-advance-right None` (commit+right)",
|
||||
"RECORDS INSERT: Tab commit-and-advance-right Silent",
|
||||
"RECORDS INSERT: Enter commit-cell-edit Silent",
|
||||
];
|
||||
let mut expected: Vec<String> = expected.iter().map(|s| s.to_string()).collect();
|
||||
expected.sort();
|
||||
let mut short = short;
|
||||
short.sort();
|
||||
assert_eq!(
|
||||
unanswered, expected,
|
||||
"advertised and unanswerable: anything here that is not in the \
|
||||
named list above reads as broken rather than as unavailable"
|
||||
short,
|
||||
expected,
|
||||
"advertised keys that do not do what they say, in full:\n{}",
|
||||
short.join("\n")
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-6562: "it moved" cannot see a move the WRONG way, so every grid
|
||||
/// key is pressed from the centre and must land where it says. Evidence file.
|
||||
#[test]
|
||||
fn every_grid_move_lands_where_its_binding_says() {
|
||||
let keymap = Keymaps::shared().get(&ModeKey::Normal).expect("Normal");
|
||||
let mut bound = Vec::new();
|
||||
bound_commands(keymap, &mut bound);
|
||||
bound.sort();
|
||||
bound.dedup();
|
||||
// Far from every edge of a grid far larger than any step, so a landing
|
||||
// shows magnitude as well as sign (improvise-5kdr); a jump's is its name.
|
||||
const START: (usize, usize) = (10, 10);
|
||||
const WINDOW: (usize, usize) = (8, 8);
|
||||
let d = |a: &String| a.parse::<isize>().expect("a delta");
|
||||
let at = |base: usize, by: isize| base.checked_add_signed(by).expect("inside the grid");
|
||||
let page = page_rows(WINDOW.0) as isize;
|
||||
let destination = |name: &str, args: &[String]| match name {
|
||||
"move-selection" => Some((at(START.0, d(&args[0])), at(START.1, d(&args[1])))),
|
||||
"scroll-rows" => Some((at(START.0, d(&args[0])), START.1)),
|
||||
"page-scroll" => Some((at(START.0, page * d(&args[0])), START.1)),
|
||||
"jump-first-row" => Some((0, START.1)),
|
||||
"jump-last-row" => Some((20, START.1)),
|
||||
"jump-first-col" => Some((START.0, 0)),
|
||||
"jump-last-col" => Some((START.0, 20)),
|
||||
_ => None,
|
||||
};
|
||||
let mut unstated = Vec::new();
|
||||
let mut unpressed = Vec::new();
|
||||
let mut pressed = std::collections::BTreeSet::new();
|
||||
for (name, args) in &bound {
|
||||
let to = destination(name, args);
|
||||
let sequences = sequences_running(keymap, name, Some(args.as_slice()));
|
||||
if to.is_some() && sequences.is_empty() {
|
||||
unpressed.push(format!("{name} {args:?}"));
|
||||
}
|
||||
for sequence in sequences {
|
||||
let mut state = furnished(AppMode::Normal);
|
||||
state.grid.extent = GridExtent { rows: 21, cols: 21 };
|
||||
state.window = WINDOW;
|
||||
*state.view.cursor_mut() = crate::ViewCursor {
|
||||
selected: START,
|
||||
row_offset: 5,
|
||||
col_offset: 5,
|
||||
};
|
||||
let view = state.view.active_view.clone();
|
||||
for key in &sequence {
|
||||
if let KeyOutcome::Local(commands) = resolve(&state, *key) {
|
||||
commands.iter().for_each(|c| state.fold(c));
|
||||
}
|
||||
}
|
||||
let landed = state.view.cursor().selected;
|
||||
if to.is_some() {
|
||||
pressed.insert(name.as_str());
|
||||
}
|
||||
match to {
|
||||
Some(to) => {
|
||||
assert_eq!(
|
||||
landed, to,
|
||||
"{sequence:?} runs `{name} {args:?}` from {START:?}"
|
||||
)
|
||||
}
|
||||
None if state.view.active_view == view && landed != START => {
|
||||
unstated.push(format!("{name} {args:?} -> {landed:?}"))
|
||||
}
|
||||
None => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
unpressed.is_empty(),
|
||||
"stated and never pressed: {unpressed:?}"
|
||||
);
|
||||
assert!(
|
||||
unstated.is_empty(),
|
||||
"these move the cursor and nothing states where to: {unstated:?}"
|
||||
);
|
||||
let every = [
|
||||
"jump-first-col",
|
||||
"jump-first-row",
|
||||
"jump-last-col",
|
||||
"jump-last-row",
|
||||
"move-selection",
|
||||
"page-scroll",
|
||||
"scroll-rows",
|
||||
];
|
||||
assert_eq!(
|
||||
pressed.into_iter().collect::<Vec<_>>(),
|
||||
every,
|
||||
"each kind is pressed"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -2431,8 +2722,8 @@ mod tests {
|
||||
not vacuous over a table that binds everything or nothing"
|
||||
);
|
||||
assert_eq!(
|
||||
composed(&state, ev(KeyCode::Char(',')), "view-forward", &[]),
|
||||
Some(vec![Command::ViewForward]),
|
||||
resolve(&state, ev(KeyCode::Char(','))),
|
||||
KeyOutcome::Local(vec![Command::ViewForward]),
|
||||
"and the key is `,` — the unshifted twin of `<`, so both halves of \
|
||||
the history sit on one physical key"
|
||||
);
|
||||
|
||||
@@ -427,9 +427,9 @@ impl RenderHelpRow {
|
||||
L::Heading(t) => prose("heading", t),
|
||||
L::Text(t) => prose("text", t),
|
||||
L::Accent(t) => prose("accent", t),
|
||||
L::Keys { keys, description } => Self {
|
||||
keys: keys.join(" "),
|
||||
description: description.to_string(),
|
||||
L::Keys(row) => Self {
|
||||
keys: row.keys().join(" "),
|
||||
description: row.description().to_string(),
|
||||
kind: "keys",
|
||||
},
|
||||
L::Literal {
|
||||
|
||||
@@ -707,44 +707,6 @@ fn a_yanked_number_crosses_the_serialized_wire_exactly() {
|
||||
);
|
||||
}
|
||||
|
||||
/// The two tests around this one share a feature-unified build, so neither can
|
||||
/// see ONE frame-parsing crate lacking `float_roundtrip`. Evidence file.
|
||||
#[test]
|
||||
fn every_build_that_parses_a_frame_enables_float_roundtrip() {
|
||||
for (krate, target) in [
|
||||
("improvise-wasm-client", Some("wasm32-unknown-unknown")),
|
||||
("improvise-worker-server", Some("wasm32-unknown-unknown")),
|
||||
("improvise-ws-server", None),
|
||||
] {
|
||||
let mut cargo = std::process::Command::new(env!("CARGO"));
|
||||
cargo.current_dir(env!("CARGO_MANIFEST_DIR")).args([
|
||||
"tree",
|
||||
"--locked",
|
||||
"--edges",
|
||||
"normal,features",
|
||||
"--invert",
|
||||
"serde_json",
|
||||
"--package",
|
||||
krate,
|
||||
]);
|
||||
if let Some(target) = target {
|
||||
cargo.args(["--target", target]);
|
||||
}
|
||||
let out = cargo.output().expect("cargo tree must run");
|
||||
assert!(
|
||||
out.status.success(),
|
||||
"{krate}: cargo tree failed: {}",
|
||||
String::from_utf8_lossy(&out.stderr)
|
||||
);
|
||||
let tree = String::from_utf8(out.stdout).expect("cargo tree prints UTF-8");
|
||||
assert!(
|
||||
tree.contains(r#"serde_json feature "float_roundtrip""#),
|
||||
"{krate} parses frames without float_roundtrip, so its numbers land \
|
||||
one ULP off:\n{tree}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// A typed number is parsed exactly in the client and re-parsed from JSON by
|
||||
/// the host. Evidence file.
|
||||
#[test]
|
||||
|
||||
+3
-3
@@ -69,9 +69,9 @@ fn page_content(page: usize) -> Vec<Row> {
|
||||
HelpLine::Heading(text) => plain(text, s.heading),
|
||||
HelpLine::Text(text) => plain(text, s.normal),
|
||||
HelpLine::Accent(text) => plain(text, s.accent),
|
||||
HelpLine::Keys { keys, description } => Row {
|
||||
key: format!(" {}", keys.join(" ")),
|
||||
desc: description,
|
||||
HelpLine::Keys(row) => Row {
|
||||
key: format!(" {}", row.keys().join(" ")),
|
||||
desc: row.description(),
|
||||
style: s.key,
|
||||
},
|
||||
HelpLine::Literal {
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
//! A number read through serde_json must be the number written (improvise-6yvg,
|
||||
//! improvise-dz8c). The rule and its derivation: `context/code/protocol.md`.
|
||||
|
||||
use serde_json::Value;
|
||||
|
||||
/// Every NORMAL serde_json dependency in the workspace, and those lacking
|
||||
/// `float_roundtrip`, each spelled `package (target)`.
|
||||
fn lossy_json_dependencies(metadata: &Value) -> (Vec<String>, Vec<String>) {
|
||||
let mut checked = vec![];
|
||||
let mut lossy = vec![];
|
||||
let packages = metadata["packages"]
|
||||
.as_array()
|
||||
.expect("metadata lists packages");
|
||||
for package in packages {
|
||||
let name = package["name"].as_str().expect("a package has a name");
|
||||
let deps = package["dependencies"]
|
||||
.as_array()
|
||||
.expect("a package lists deps");
|
||||
for dep in deps {
|
||||
if dep["name"] != "serde_json" || !dep["kind"].is_null() {
|
||||
continue;
|
||||
}
|
||||
let target = dep["target"].as_str().unwrap_or("all targets");
|
||||
let spelled = format!("{name} ({target})");
|
||||
let features = dep["features"].as_array().expect("a dep lists features");
|
||||
if !features.iter().any(|f| f == "float_roundtrip") {
|
||||
lossy.push(spelled.clone());
|
||||
}
|
||||
checked.push(spelled);
|
||||
}
|
||||
}
|
||||
(checked, lossy)
|
||||
}
|
||||
|
||||
/// Derived from `cargo metadata`, not listed: a new crate that can parse JSON
|
||||
/// is covered the day it declares the dependency. Evidence file.
|
||||
#[test]
|
||||
fn every_crate_that_can_parse_json_enables_float_roundtrip() {
|
||||
let out = std::process::Command::new(env!("CARGO"))
|
||||
.current_dir(env!("CARGO_MANIFEST_DIR"))
|
||||
.args(["metadata", "--no-deps", "--format-version", "1"])
|
||||
.output()
|
||||
.expect("cargo metadata must run");
|
||||
assert!(
|
||||
out.status.success(),
|
||||
"cargo metadata failed: {}",
|
||||
String::from_utf8_lossy(&out.stderr)
|
||||
);
|
||||
let metadata: Value = serde_json::from_slice(&out.stdout).expect("metadata is JSON");
|
||||
let (checked, lossy) = lossy_json_dependencies(&metadata);
|
||||
assert!(
|
||||
checked.len() >= 2,
|
||||
"the derivation found {checked:?}: too few to be reading the metadata right"
|
||||
);
|
||||
assert!(
|
||||
lossy.is_empty(),
|
||||
"these parse JSON numbers with serde_json's lossy default, one ULP off \
|
||||
for roughly one decimal in seven; add features = [\"float_roundtrip\"] \
|
||||
to each crate's own manifest: {lossy:?}"
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user