Merge gh/main (705c92d9) into collapse-fu/ciw8
#197 changed project::cell_id to take &Model; aggregate_refusal and the two sjdb tests now pass the host model. The text merge was clean there and did not compile (E0308) until they did. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
@@ -28,6 +28,7 @@ them here would let the copy drift:
|
||||
| `context/repo-map.md` | The "where", plus **numbered gotchas** — read the ones your diff touches |
|
||||
| `prolog-sessions/kb.pl` | Executable knowledge: `invariant/2`, `doctrine/2`, `forbidden/2`, `decision/3`, and prior `work_done/3` |
|
||||
| `AGENTS.md` | Working rules: bug-first tests, `Option`/`Result`/`Box` at the edges, merges over rebases |
|
||||
| `.agents/skills/wave/agent-rules.md`, "GitHub" | The reviewer's rule: what to read before your first comment, how to answer each existing review, and what every review you post must state. Read it before reviewing a PR |
|
||||
|
||||
`kb.pl` is the highest-value and least-obvious of these. Grep it before
|
||||
reviewing: `grep -n "invariant(\|doctrine(\|decision(" prolog-sessions/kb.pl`.
|
||||
|
||||
@@ -80,7 +80,8 @@ defect is a test that passes.
|
||||
"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. 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:
|
||||
- Only the user's explicit approval accepts an ADR, and lets its PR merge. The
|
||||
review gates still apply, and none of them is approval:
|
||||
`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.
|
||||
|
||||
@@ -32,9 +32,17 @@ not in a second copy.
|
||||
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).
|
||||
`git push` would be refused (rule 4), or worse. Rules 1–5 and 7 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).
|
||||
7. **Pin `gh/main` to a literal SHA when the task starts.** Run
|
||||
`git rev-parse gh/main` once. Use that SHA, not the ref name, for every
|
||||
reset, merge-base and diff. Remote refs are shared by every worktree of the
|
||||
repository, so `gh/main` moves whenever another session fetches. An agent
|
||||
ran `git reset --soft gh/main` mid-task and staged a reverse diff of #196.
|
||||
It noticed only because it read `git status` (improvise-diho). Re-resolve
|
||||
only on purpose, for example when you mean to merge a newer main. Rule 3's
|
||||
check before a PR is such a time.
|
||||
|
||||
## Commit early: a worktree is not durable storage
|
||||
|
||||
@@ -113,6 +121,12 @@ out on purpose:
|
||||
check that no test *disappeared*. Diff test **names**, not counts: a
|
||||
textually clean merge can delete a test module with no conflict marker, and
|
||||
the other side's new tests mask the loss in a count.
|
||||
- **Build each side's test list in a fresh `CARGO_TARGET_DIR`, every time.** If
|
||||
a parent's tree comes from `git archive`, a reused target dir gives a wrong
|
||||
answer, not just a slow one. Archive stamps every file with its commit time,
|
||||
so cargo judges the sources older than the last build and lists the previous
|
||||
tree's tests. On #188 that made three of the branch's own tests look
|
||||
merge-only.
|
||||
- **The shell guard refusing a compound command means NEITHER part ran.** Split
|
||||
the command. After a refusal, re-check the state it was meant to produce.
|
||||
- **Re-run the full suite after reverting an injected fault.** A revert applied
|
||||
@@ -200,6 +214,8 @@ text for any of these in your hand-back.
|
||||
`./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.
|
||||
- **Answer a review even if the PR has already merged.** Reviews arrive late,
|
||||
and the thread is still where the next reader looks.
|
||||
- **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
|
||||
|
||||
@@ -131,9 +131,10 @@ and watch the check fail.
|
||||
|
||||
**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
|
||||
CI is green and an adversarial review agent has passed the PR's head SHA. An ADR
|
||||
PR also needs the user's explicit approval, and nothing else stands in for it:
|
||||
"ADR pull requests" below. 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.
|
||||
|
||||
@@ -184,8 +185,8 @@ earlier head, and the finding may already be fixed — verify rather than assume
|
||||
in either direction.
|
||||
|
||||
Reviews arrive **as the owner's quota allows**, so absence is not evidence.
|
||||
Wait where a review is plausible, and when one is found, answer it on the
|
||||
thread even if the PR has already merged: that is where the next reader looks.
|
||||
Wait where a review is plausible. How a review is answered once found, by the
|
||||
author and by a reviewer, is `.agents/skills/wave/agent-rules.md`, "GitHub".
|
||||
|
||||
**Copilot is not reviewing this repository, and the request is a silent no-op.**
|
||||
Measured 2026-09-20: PRs #12, #21 and #27 carry 2, 8 and 4 Copilot reviews; #61,
|
||||
@@ -231,8 +232,8 @@ normally.)
|
||||
|
||||
Across four PRs Copilot has found genuine bugs and no false positives, so its
|
||||
findings are worth the loop. **But judge each one against the repo's doctrine
|
||||
rather than applying it reflexively**, and when you decline, reply on the thread
|
||||
with the reason. The distinction that has mattered: a finding can be *correct*
|
||||
rather than applying it reflexively**. How a declined finding is answered is
|
||||
agent-rules, "GitHub". The distinction that has mattered: a finding can be *correct*
|
||||
and still be wrong to fix in that PR. On #12, a non-ASCII `truncate` panic was
|
||||
applied (byte-identical on ASCII, so it cost no evidence) while a hint-line
|
||||
padding bug in the same file was declined — fixing it would have rewritten the
|
||||
@@ -251,21 +252,30 @@ is to branch from `gh/main`, cherry-pick, open a follow-up PR, and delete the
|
||||
stray re-creation. Check `gh pr view <n> --json state` *before* pushing a fix to
|
||||
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** —
|
||||
Merge only once a review of the PR's current head has passed — an adversarial
|
||||
agent's while Copilot is absent. What a review must state is agent-rules,
|
||||
"GitHub". Prefer a **merge commit** —
|
||||
`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):
|
||||
**Only the user's explicit approval accepts an ADR (`context/adr/`), and only
|
||||
it lets an ADR PR merge.** "An ADR isn't approved until I say it is" (the
|
||||
user). Green CI, every review answered and a passing adversarial review
|
||||
("Shipping a wave") are still required. They are gates, never approval: an ADR
|
||||
PR that passes all of them is still unapproved and does not merge. 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"**.
|
||||
4. The PR merges only after the user comments **"Approved"**, and only while
|
||||
the gates above still hold. Nobody else's comment or review stands in for
|
||||
it. "Read the PR's reviews before merging it" still
|
||||
applies after the approval. On #122 the user's own review landed as the PR
|
||||
merged.
|
||||
|
||||
### Never hand-edit `.beads/issues.jsonl`
|
||||
|
||||
@@ -296,7 +306,7 @@ bd show $(cat /tmp/bd-audit-ids.txt) 2>&1 \
|
||||
|
||||
`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:
|
||||
out two 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
|
||||
@@ -304,9 +314,16 @@ out three things a `grep -r` over the same roots reads:
|
||||
- **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`.
|
||||
|
||||
`-d skip` is there for the skills' symlinks, and it changes no answer. The
|
||||
`.claude/skills/*` entries are symlinks to directories under `.agents/skills/`,
|
||||
and `git ls-files` lists each one, so each reaches grep as a command-line
|
||||
operand. The grep has no `-r`, so it never reads a directory operand. It prints
|
||||
`grep: <path>: Is a directory` on stderr instead, and `-d skip` only silences
|
||||
those lines. The output goes through `sort -u`, so a duplicate could not change
|
||||
the answer anyway. Measured with BSD grep 2.6.0 and GNU grep 3.12: 3092 matches
|
||||
with or without the flag, 462 distinct ids. Do not add `-r`: GNU grep then
|
||||
follows those symlinks and reads the skills a second time (3107 matches).
|
||||
|
||||
Plain `git grep` is not a substitute: on macOS its `-E` silently ignores `\b`
|
||||
and finds a fraction of the ids.
|
||||
@@ -346,8 +363,10 @@ real defect through:
|
||||
added to the exclusion list in the same commit**, or it arrives as a dead id
|
||||
that no amount of `bd` archaeology explains.
|
||||
|
||||
Note the recipe's own prose deliberately spells no live or dead id: a document
|
||||
that lists ids as examples makes itself a source of audit noise.
|
||||
The recipe's own prose spells no id as an *example*, because a document that
|
||||
lists example ids makes itself a source of audit noise. The ids it does cite,
|
||||
improvise-8haj and improvise-oujb, are the tickets behind two of the rules
|
||||
above. They resolve, and the audit checks them like any other.
|
||||
|
||||
One environment note: in a worktree-isolated agent the shell guard refuses a
|
||||
loop that passes a runtime-computed argument to `bd`, so the recipe above
|
||||
@@ -376,9 +395,10 @@ 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.
|
||||
SHA ("Shipping a wave"). An ADR PR merges only on the user's explicit
|
||||
approval as well ("ADR pull requests"). 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
|
||||
|
||||
+141
-22
@@ -592,10 +592,23 @@ the client was told nothing at all.
|
||||
key-path batch assumes the failed one happened. `refuse_and_stop` does it for
|
||||
`LoadModel` and every `ImportJsonHeadless` arm, so `load x . q` and
|
||||
`import-json x . q` do not quit having loaded nothing. `AddCategory` keeps its
|
||||
own inline abort (gotcha 21). The item, formula and view effects only refuse:
|
||||
the frame is rolled back by the refusal alone. On the key path, a failed
|
||||
`AddItem`, `AddFormula` or view edit no longer stops the rest of its batch,
|
||||
which is what it did before bok4.
|
||||
own inline abort (gotcha 21). The item and view effects, and `AddFormula`'s
|
||||
missing-category arm, only refuse: the frame is rolled back by the refusal
|
||||
alone. On the key path, a failed `AddItem` or view edit no longer stops the
|
||||
rest of its batch, which is what it did before bok4.
|
||||
- **`AddFormula`'s parse-error arm also stops (improvise-v4hg).** It is the one
|
||||
item-or-formula failure a key-path batch was found to depend on. Enter in
|
||||
`FormulaEdit` is `[SetStatus, AddFormula, ChangeMode(FormulaPanel),
|
||||
ClearBuffer("formula")]`, and the last two assume the commit worked. Without
|
||||
the stop, `Total = (` was reported and then the editor closed and the typed
|
||||
text was cleared. With it, the user stays in `FormulaEdit` with the text, and
|
||||
the error is drawn beside the prompt. `fit-measure` emits `AddFormula` last, so
|
||||
there is nothing after it to stop. The missing-category arm is reachable only
|
||||
from a wire frame or a `:` line naming another category, and nothing in either
|
||||
depends on it, so it does not stop. PR #181's review searched for an
|
||||
`AddItem` case the same way and found none, so `AddItem` does not stop either.
|
||||
An abort is added where a later effect depends on the failed one, never for
|
||||
symmetry.
|
||||
|
||||
In a client's frame, any refusal becomes `Rejected(MutationRefused)`. The frame
|
||||
is undone whole, and the client is told, which is the channel the narrated
|
||||
@@ -615,9 +628,24 @@ Consequences:
|
||||
refusal, not by `status_msg`. The frame's rollback also rolls the host's
|
||||
status line back.
|
||||
- **Every refusal site has a test that fails when it is reverted** to a
|
||||
status-only write (improvise-9fpl). Most are one table,
|
||||
`every_failing_model_command_refuses_its_whole_frame`. That matters because
|
||||
these are exactly the lines a merge conflict could silently drop.
|
||||
status-only write (improvise-9fpl, improvise-0v8l). Two tables carry most of
|
||||
them. `every_failing_model_command_refuses_its_whole_frame` covers the wire.
|
||||
`every_refusal_site_refuses_its_failure_and_only_its_failure` covers the key
|
||||
path and the `:` line, which is what the headless exit code reads. The wire
|
||||
table cannot see the refusal half of a `refuse_and_stop` site, because
|
||||
`Host::mutating_frame` rejects on `refused || aborted`, and the abort alone
|
||||
suffices. So each `refuse_and_stop` site needs a key-path assertion for its
|
||||
refusal as well as one for its abort. The abort is
|
||||
`a_failed_load_or_import_does_not_run_the_quit_after_it` (improvise-514o).
|
||||
Three sites are not in the key-path table (improvise-rtud). The quit test's
|
||||
"setup:" assertion holds the refusal of `load` and `import-json absent.json`.
|
||||
`a_formula_that_does_not_parse_keeps_the_user_in_the_editor_with_their_text`
|
||||
holds `AddFormula`'s parse arm, which has no case in the quit test. The other
|
||||
five import arms are in
|
||||
`every_refusal_site_refuses_its_failure_and_only_its_failure`.
|
||||
Each case in that table has a succeeding twin, so a site that refused
|
||||
unconditionally would fail too. That matters because these are exactly the
|
||||
lines a merge conflict could silently drop.
|
||||
- **Not covered:** a no-op *inside* a view that does resolve. Examples are
|
||||
`SetAxis` on a category the view does not have, `HideItem` of an absent
|
||||
item, and `RemoveItem`/`RemoveCategory`/`RemoveFormula` of something already
|
||||
@@ -3312,9 +3340,77 @@ so `paste Region=West _Measure=Revenue Product=Gadget` from a script still
|
||||
writes where the current view happens to aggregate `Product` away: a key that
|
||||
names every coordinate is a record whatever the view shows. For the cursor's
|
||||
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`.
|
||||
category. The **collapsed-group fold** half of improvise-l4mp cannot be seen by
|
||||
any question about the key alone, because the key carries the group's name as an
|
||||
ordinary coordinate. It is answered at the cursor instead, by the next section.
|
||||
**The first version of that section wrote a false sentence here**: that the
|
||||
checks in `paste_into` were reached "only by a key a script spells out". The
|
||||
cursor path handed `PasteCell` the resolved record key, and `paste_into` ran
|
||||
`aggregates` over it **again**. That is a second derivation beside `cell_id`, and
|
||||
it reads a record lacking a later category (every record, once `+cat` adds one)
|
||||
as an aggregate, so `p` refused where gh/main had pasted and the browser still
|
||||
wrote (improvise-q9eg). The distinction is now a type: `PasteAt::Resolved`
|
||||
carries a key the door answered and is written as it stands, and
|
||||
`PasteAt::Spelled` carries a key a script typed, which only its shape can vet.
|
||||
Nothing is asked twice.
|
||||
|
||||
## The cursor's cell has three doors (improvise-gw18, lx7w, 54g1, yk2s)
|
||||
|
||||
**The lesson of improvise-gw18 is that guarding one command did not close the
|
||||
path.** PR #176 taught `EditOrDrill` to refuse a collapsed group's fold, and `>`
|
||||
still drilled into it. The drill was empty, and `o` inside it wrote
|
||||
`Month=Q1, _Measure=Revenue`, a coordinate that is not an item, into `## Data`.
|
||||
The same shape turned up three more times, each in a command nobody had
|
||||
listed. `p` wrote onto the fold (the fold half of improvise-l4mp). `x` in records
|
||||
mode cleared the synthetic `(_Index, _Dim)` key, a silent no-op, while the
|
||||
browser cleared the real record (improvise-lx7w). `p` onto a formula's cell
|
||||
stored a value under the formula that survived in `## Data`, while the browser
|
||||
declined (improvise-54g1). Each fix was one more guard in one more command. So
|
||||
the answer is the **accessor**, not the commands: a command gets the cursor's
|
||||
cell through a door that matches what it does with it.
|
||||
|
||||
Every command that takes the cursor's cell, and its door:
|
||||
|
||||
| command | door | on a fold | on a formula's cell | in records mode |
|
||||
|---|---|---|---|---|
|
||||
| `yank` | `cell_key` (reads) | yanks the fold's value | yanks the result | `YankCell` resolves the synthetic key |
|
||||
| `paste`, `clear-cell` | `value_cell_key` | refused | refused | resolved to the real record; a coordinate or creating column and a drill are refused |
|
||||
| `commit-cell-edit`, `commit-and-advance-right` | `committable_cell_key` | refused | refused | the synthetic key, which `commit_cell_value` resolves itself (gotcha 19) |
|
||||
| `drill-into-cell` | `unfolded_cell_key` | refused | drills | unchanged |
|
||||
| `edit-or-drill` | its own check, `folds_a_group` then `owning_formula` | refused | refused | unchanged |
|
||||
| `hide-selected-row-item` (`H`) | `GridLayout::row_folds_a_group` | refused | n/a (hides a row item) | n/a |
|
||||
| `add-record-row` (`o`) | none, it takes no cell | n/a | n/a | closed by the drill door: no drill of a fold exists to add a row in |
|
||||
|
||||
**`value_cell_key` asks the browser's own question.** It calls
|
||||
`project::cell_id`, the function that fills `CellDisplay::id` for the browser
|
||||
(ADR 002), and writes only where the answer is `CellId::Record`. The browser's
|
||||
`record_cell_at_cursor` is that same match on the projected id. The two ends
|
||||
therefore agree because they read one answer, not because two derivations
|
||||
happen to match. `tests/lockstep.rs` holds them together for all three tickets.
|
||||
`cell_id` took `&App` only to reach the model, so it now takes `&Model`. That is
|
||||
what let `CmdContext` ask it.
|
||||
|
||||
**Commit has a door of its own**, and not `value_cell_key`, because commit is
|
||||
the one writer that must see the synthetic key: renaming a coordinate column,
|
||||
creating a column and staging a drill edit all resolve the records position
|
||||
inside `commit_cell_value`. Its door refuses the two arms `i` refuses, an
|
||||
aggregate (a fold included) and a formula's result. It needed one because
|
||||
`Enter` and `Tab` advance the editor, and `EnterEditAtCursor` re-opens it at the
|
||||
new cursor with no guard. `i` declines to edit `Profit`, yet `Tab` from `Cost`
|
||||
landed the editor on it, and the next commit stored a value under the formula.
|
||||
That is a fourth door nobody had listed. **The entry half is still open**
|
||||
(improvise-enz6): the editor still opens on a fold or a formula's cell. Only the
|
||||
write is refused, out loud, at commit. Closing the entry belongs in `ui/effect.rs`.
|
||||
|
||||
**Drill keeps the narrowest door** because drilling into an aggregate is its
|
||||
purpose. Only a fold is refused, since no record carries a group's name. The
|
||||
ticket's alternative, expanding the fold into its members' records, would be the
|
||||
more useful drill and is not built.
|
||||
|
||||
**A text-path key is not covered.** `paste Month=Q1 _Measure=Revenue` typed as
|
||||
coordinates still reaches `PasteCell` with a group name. The doors are about the
|
||||
**cursor**, and a key a user spells out is a claim about the document that no
|
||||
layout checked. The same is true of `set-cell`.
|
||||
|
||||
## A command is a function of its context — the determinism law (improvise-hzjo)
|
||||
|
||||
@@ -3357,18 +3453,41 @@ 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.
|
||||
the demo that is four records and eight cells for each of the eight drills. With
|
||||
the store's sort removed, the law fails 400 of 400 runs.
|
||||
|
||||
**The width guard measures what the comparison compared (improvise-b9f2,
|
||||
improvise-btyw).** The test asserts that some `drill-into-cell` the comparison
|
||||
actually built and compared froze at least four records. For each one it applies
|
||||
those very effects to one scratch `App` and counts the records in its
|
||||
`drill_state`. It counts them only when their `Debug` text, which carries their
|
||||
order, appears in the text the comparison compared. The comparison reads `Debug`
|
||||
while the scratch `App` reads the applied state, and those are two
|
||||
representations. The review showed they can disagree: give `StartDrill` a
|
||||
`Debug` that prints only a row count, and the frozen records are still four
|
||||
while the compared text no longer holds their order. With the store's sort
|
||||
removed the law then passed 300 of 300 runs, with the guard green. Requiring
|
||||
the ordered records in the compared text ties the guard to what the comparison
|
||||
could actually see. A first
|
||||
version measured what the *argument lists* match in the store instead. That was
|
||||
a parallel derivation, and the review beat it with a `drill-into-cell` parser
|
||||
that refuses fewer than two arguments. Every one-coordinate drill then went to
|
||||
the one-record cursor fallback, the argument lists still matched four records,
|
||||
and with the store's sort removed the law passed 38 of 300 runs.
|
||||
|
||||
Counting frozen records is not enough by itself. A drill over no coordinates
|
||||
(any candidate with no `Cat/Item` argument, such as `["1", "1"]`) freezes
|
||||
**every** record, and it reads them through `iter_cells` in insertion order,
|
||||
never through the narrowing index where gotcha 18 lives. Under the parser rival
|
||||
that whole-store drill still froze 16 records, and a guard counting it stayed
|
||||
silent. So only a drill that **narrowed** the store counts: one that froze fewer
|
||||
records than the store holds. With the parser rival in place, the guard fires on
|
||||
every run ("froze 1 records"), and it fires on every run under the `Debug` rival
|
||||
too ("froze 0 records").
|
||||
|
||||
What the guard still cannot see is a narrowing drill whose records are frozen in
|
||||
an order that does not come from the store. It checks that the law had a wide
|
||||
drill to compare, not why the drill's order would differ.
|
||||
|
||||
**What it cannot see.** Only commands and arguments the fixture reaches. A
|
||||
command whose nondeterminism needs some other state, such as records mode, a
|
||||
|
||||
@@ -900,7 +900,11 @@ regular on None projected the fold as `CellId::Record`, offered it for editing,
|
||||
and a `SetCell` would have written the coordinate `Month=Q1`, which is not an
|
||||
item. `render.rs::editable` is untouched (ADR 002: the variant decides).
|
||||
`EditOrDrill` asks the same predicate and declines out loud: drilling would filter
|
||||
on `Month=Q1`, which no record carries, and show an empty list.
|
||||
on `Month=Q1`, which no record carries, and show an empty list. Guarding that one
|
||||
command left `>`, `p`, `H` and an `Enter` advanced onto the fold all still writing
|
||||
or drilling there (improvise-gw18, yk2s). Every command that takes the cursor's
|
||||
cell now goes through a door that asks this predicate, or asks `cell_id`, which
|
||||
does: `context/code/command.md`, "The cursor's cell has three doors".
|
||||
|
||||
### The cursor, and why the order is the fix
|
||||
|
||||
|
||||
@@ -4558,6 +4558,27 @@ registers to agree; a TUI-only blank filter (`Text("")` to `None`) fails it on
|
||||
`+`, which is why that filter was removed rather than kept. The evidence entries
|
||||
for the host-side tests are in `command.md`.
|
||||
|
||||
### x_in_records_mode_clears_the_record_the_browser_clears
|
||||
|
||||
improvise-lx7w, both ends. The browser composes `ClearCell` of the projected
|
||||
`CellId::Record`. Before the fix the host's `x` cleared the synthetic key and
|
||||
the documents diverged in `both_ends`. South carries **two** measures. With one,
|
||||
clearing it removes South's whole records row, the host resets its cursor on
|
||||
the reframe, and `both_ends` reports a cursor divergence that belongs to the
|
||||
fixture and not to `x`. The first version of the test did exactly that. The test
|
||||
also asserts that South/Revenue is gone and North is untouched, because two ends
|
||||
that both do nothing also agree.
|
||||
|
||||
### p_onto_a_formula_owned_cell_declines_on_both_ends
|
||||
|
||||
improvise-54g1, both ends, on `yank_host`'s `Profit` column (row 1, column 3).
|
||||
The client must not compose a `Command::Model`, and the host's document must not
|
||||
move and must say why. The control, Cost at column 2, must compose a write, so
|
||||
a client that never writes cannot pass. The fold case,
|
||||
`a_write_onto_a_fold_declines_on_both_ends`, runs the same helper for `p` and `x`
|
||||
at row 2. Rival for all three: the host's paste/clear door restored to
|
||||
`ctx.cell_key()`, which was `gh/main`'s code. All three fail.
|
||||
|
||||
### the worker is not called up until its first frame has arrived
|
||||
|
||||
improvise-i0v9. The worker transport wrote `in-page worker · <doc>` when it
|
||||
@@ -4749,3 +4770,69 @@ claims at commit. The commit was then *accepted*. With the refusal assertion
|
||||
disabled, the host held `Some(Number(42.0))`, so A's commit had re-created the
|
||||
cell B cleared. Under that rival, the 0j0f test above failed as well. Both
|
||||
files were restored.
|
||||
|
||||
### p_onto_a_sparse_record_writes_what_the_browser_writes
|
||||
|
||||
improvise-q9eg, both ends, on a **sparse** record: `Season` is added after
|
||||
entering records mode, so no record carries it. `yy` on North/Revenue, then `p`
|
||||
on South/Revenue (row 1), through `both_ends`, which compares the host's
|
||||
document with the one the client's composed commands build. The host refused
|
||||
while the browser composed `SetCell`, so the documents differed. The final
|
||||
assertion that South holds 1200 is the half that keeps the test from passing
|
||||
when both ends decline. Rival: `PasteAt::Resolved` back through `paste_into`.
|
||||
This test fails.
|
||||
|
||||
### a_locally_resolved_page_scroll_lands_where_the_host_would_put_it
|
||||
|
||||
**What was missing (improvise-5xbq).** `tests/lockstep.rs` compared only `j`
|
||||
and `G` against the host. The browser's page rule (`resolve::page_rows`) and
|
||||
the TUI's (`Move`'s `Page` arm, `visible_rows * 3 / 4`) were two copies, and
|
||||
two faults injected into the browser's copy left the whole workspace green:
|
||||
`page_rows` returning `window_rows / 2`, and `grid_move` paging by
|
||||
`window_cols`. The second was invisible for a further reason: the only window
|
||||
the page-rule test used was square, `(8, 8)`.
|
||||
|
||||
**The fixture.** 20 rows by 3 columns; the host sees 8 rows and the client
|
||||
announces `(8, 3)`. The three candidate rules then give three different
|
||||
steps: 6 (three quarters of the rows), 4 (half the rows) and 2 (three quarters
|
||||
of the columns). The walk starts off the edge (two `j`), then presses `PgDn`,
|
||||
`PgUp`, `Ctrl-d` and `Ctrl-u`, including a `PgDn` that clamps at row 19.
|
||||
|
||||
**Why it pins the rows as well as comparing the ends.** `page_rows` is now the
|
||||
one home (the TUI's `Move` calls it), so a wrong rule moves host and client
|
||||
together, and a comparison alone passes. Each press therefore also asserts the
|
||||
row the TUI's rule names.
|
||||
|
||||
**Injected, before and after the move to one home.**
|
||||
|
||||
| Fault | Two copies | One home |
|
||||
|---|---|---|
|
||||
| `page_rows` returns `window_rows / 2` | fails at press 2 (`PgDn`), "client and host disagree" | fails at press 2, "not the row the TUI's page rule names"; `app.rs`'s `page_down_scrolls_by_three_quarters_visible` and `page_up_scrolls_backward` fail too |
|
||||
| `grid_move` pages by `window_cols` | fails at press 2, "client and host disagree" | fails at press 2, "client and host disagree" |
|
||||
|
||||
`Ctrl-d`/`Ctrl-u` are `scroll-rows ±5`, not the page rule. They are in the walk
|
||||
because the ticket named them, and they move the start point of the next page
|
||||
turn to a row the fixed rules alone would not reach.
|
||||
|
||||
### a narrow window still shows how to page through help — `harness/tests/help.spec.js`
|
||||
|
||||
**What broke (improvise-1c5m).** Below the width at which the app bar wraps,
|
||||
`#help` (fixed, `inset: 2rem`, `z-index: 30`) covers `#hint`, and since PR #180
|
||||
Welcome carries no paging instruction of its own. So a new user on Welcome at
|
||||
phone width could not see how to reach page two.
|
||||
|
||||
**The fix is option (b) from the ticket.** The overlay draws `model.hint` in a
|
||||
footer. In Help mode that is already `hint_line(Help)`, the same string `#hint`
|
||||
carries, so it adds no derivation, and it sits outside `.help-body`, so
|
||||
"Welcome is prose" still holds. The footer is `position: sticky` because the
|
||||
overlay itself scrolls and Welcome is longer than a phone-height overlay.
|
||||
|
||||
**What the case asserts.** At 600x800 on Welcome, it reads the hint string from
|
||||
`#hint`, so the spec spells no keys. It then requires some element showing
|
||||
exactly that string to be the topmost element at its own centre, which is the
|
||||
`elementFromPoint` measurement the ticket made by hand.
|
||||
|
||||
**Red, then a rival.** At base it failed with "the paging hint is under the
|
||||
help overlay". With the fix and `position: sticky` changed to `static`, it
|
||||
fails the same way: the footer exists but sits below the fold of a scrolling
|
||||
overlay, so presence alone cannot satisfy the case.
|
||||
|
||||
@@ -5269,6 +5269,97 @@ the first message does not mention drilling. At `0ed2e41` it failed with the
|
||||
old text. The rival is restoring that text. Real drill paste, which means
|
||||
staging a typed `CellValue`, is still open as improvise-48jd.
|
||||
|
||||
The drilled paste now declines at the cursor door (`value_cell_key`) before
|
||||
`PasteCell` runs, so the status carries the registry's `paste:` prefix and a
|
||||
wording shared with `clear-cell`. The assertion was re-aimed from equality with
|
||||
`PASTE_INSIDE_DRILL` to `ends_with(INSIDE_A_DRILL_REFUSAL)` (improvise-gw18's
|
||||
doors). The advice half is unchanged.
|
||||
|
||||
### drilling_into_a_fold_is_refused_and_o_after_it_writes_no_group_coordinate
|
||||
|
||||
improvise-gw18. On `gh/main` at `800348a`, `>` on the Q2 fold (row 2, column 1)
|
||||
opened a `_Drill` with no rows, and `o` in it stored `Month=Q2` in `## Data`. The
|
||||
test presses both keys through `App::handle_key`, then saves and reloads, and
|
||||
asserts that no coordinate names a non-item (`non_item_coordinates`, which asks
|
||||
the category, not the spelling). It failed on the first assertion: the drill
|
||||
opened. Rival: the registry's cursor fallback on the reading accessor
|
||||
(`cell_key`) in place of `unfolded_cell_key`. This test and
|
||||
`no_normal_mode_key_pressed_on_a_fold_stores_a_group_coordinate` fail. The
|
||||
control, `drilling_into_the_row_above_a_fold_still_opens_a_drill`, holds the
|
||||
refusal to the fold alone.
|
||||
|
||||
### no_normal_mode_key_pressed_on_a_fold_stores_a_group_coordinate
|
||||
|
||||
improvise-gw18's enumeration, made executable. Every key bound in Normal mode,
|
||||
prefix sub-keys included, is pressed on a fresh app with the cursor on a fold
|
||||
and a value yanked, and no stored coordinate may name a non-item. The sweep is
|
||||
**enumerated** rather than hand-picked, which is how it found `p` (the fold half
|
||||
of improvise-l4mp) without being told. The first run failed on
|
||||
`[(Char('p'), NONE)] on a fold stored [("Month", "Q2")]`. It asserts that the
|
||||
enumeration reaches `p`, so an empty sweep cannot pass. What it cannot see is a
|
||||
write two keys deep (`>` then `o`). That is why the drill test above exists. A
|
||||
two-key sweep was **estimated**, not measured, at several seconds and not built.
|
||||
|
||||
Widened after PR #197's review, which showed that `H` without its fold check
|
||||
passed the sweep: a ghost `hidden:` entry is not a stored coordinate. Every
|
||||
stroke must now also leave the `## Data` section and the view's hidden set
|
||||
exactly as they were. With the `H` guard disabled, this test fails as well as
|
||||
the yk2s one. What no sweep can see is `clear-cell` back on the reading
|
||||
accessor, because clearing a key nothing is stored at changes nothing. The
|
||||
lx7w tests and the fold lockstep case catch that.
|
||||
|
||||
### hiding_a_folds_row_is_refused_and_persists_no_hidden_group
|
||||
|
||||
improvise-yk2s. `H` on the fold emitted `HideItem { Month, "Q2" }`. `View`
|
||||
accepts an unknown name, so a ghost `hidden:` line was saved, with no visible
|
||||
change and no message. The test asserts no hidden entry, a message, and no
|
||||
`hidden:` line in the saved text. The control hides `Feb` one row up. Rival: the
|
||||
`row_folds_a_group` guard disabled (`if false && …`). Only this test fails. The
|
||||
guard asks the row half of `folds_a_group`, so an inner-level fold
|
||||
(`North/Q1`) is refused too, although `North` is a real item. That is a
|
||||
deliberate over-refusal, taken to keep one predicate.
|
||||
|
||||
### enter_advancing_onto_a_fold_commits_nothing_there
|
||||
|
||||
The write half of improvise-enz6, closed by the commit door. `i` on Feb, then
|
||||
`Enter`, which commits and advances onto the Q2 fold and re-opens the editor
|
||||
there. That entry is still enz6's open half. Then `7` and `Enter`. At the
|
||||
previous commit door (`cell_key`) the second commit stored `Month=Q2`. Rival:
|
||||
`commit_at_cursor` on `cell_key` again. This test and
|
||||
`tab_advancing_onto_a_formula_cell_commits_nothing_under_it` fail.
|
||||
|
||||
### x_in_records_mode_clears_the_real_record_under_the_cursor
|
||||
|
||||
improvise-lx7w. In records mode `x` built `ClearCell` from the synthetic
|
||||
`(_Index, _Dim)` key, which names nothing stored, so it was a silent no-op. The
|
||||
cursor is on West × Cost (row 1, column 3). The test asserts that West/Cost is
|
||||
gone, that East/Cost is untouched (so a clear of the wrong row cannot pass), and
|
||||
that nothing synthetic reaches the reloaded file. Rival: `clear-cell`'s cursor
|
||||
fallback on the old door. This test and the coordinate-column one fail.
|
||||
|
||||
### x_on_a_records_coordinate_column_changes_nothing_and_says_so
|
||||
|
||||
improvise-lx7w's decline arm, the answer #184 gave paste: a coordinate column
|
||||
stores no value, so `x` there changes nothing and says so. Before the fix it
|
||||
changed nothing *silently*, and it fails on the message.
|
||||
|
||||
### p_onto_a_formula_owned_cell_writes_nothing_and_names_the_formula
|
||||
|
||||
improvise-54g1. `p` on West × Profit (`Profit = Revenue - Cost`, row 1,
|
||||
column 2) stored `_Measure=Profit` under the formula. The recompute hid it, but
|
||||
it survived in `## Data`. The test asserts that the saved text has no Profit
|
||||
line, that the reload has no such cell, and that the status names the formula.
|
||||
The control, one column left, must write, so a door that refused everything
|
||||
cannot pass. Rival: the old door. It fails on the saved text.
|
||||
|
||||
### tab_advancing_onto_a_formula_cell_commits_nothing_under_it
|
||||
|
||||
A door the tickets did not list, found by working the enumeration. `i` declines
|
||||
to edit a formula's cell, but `Tab` from West/Cost commits and advances onto
|
||||
Profit, `EnterEditAtCursor` re-opens the editor there, and `Enter` stored
|
||||
`7` under the formula. Found by this test failing at `90b93d4`, before
|
||||
`committable_cell_key` existed. The rival is the one above.
|
||||
|
||||
### ui::host::tests::every_failing_model_command_refuses_its_whole_frame
|
||||
|
||||
improvise-9fpl, found by PR #181's review. Eight of bok4's refusal sites had no
|
||||
@@ -5316,6 +5407,81 @@ 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".
|
||||
|
||||
#### improvise-514o: every `refuse_and_stop` arm
|
||||
|
||||
The case list above covered `load` and the JSON cannot-read arm only. PR #181's
|
||||
review reverted each `refuse_and_stop` to `app.refuse` one at a time, which
|
||||
keeps the refusal and drops the abort. Five `ImportJsonHeadless` arms failed
|
||||
nothing: CSV cannot-read, CSV parse, JSON parse, array path and build. The wire
|
||||
table `every_failing_model_command_refuses_its_whole_frame` cannot see the
|
||||
abort, because `Host::mutating_frame` rejects on `refused || aborted`, and the
|
||||
refusal alone suffices. The list now has one case per arm, using the fixtures
|
||||
the wire table uses, which now come from `ui::host::import_fixture_storage`.
|
||||
Each case also asserts the arm's own status text, so a fixture that failed in a
|
||||
different arm cannot stand in for the one it names. The same scripted sweep,
|
||||
rerun after the change, fails this test at all seven sites. Before, it failed
|
||||
at two.
|
||||
|
||||
### ui::app::tests::a_formula_that_does_not_parse_keeps_the_user_in_the_editor_with_their_text
|
||||
|
||||
improvise-v4hg. In `FormulaEdit`, Enter on `Total = (` reported the parse error
|
||||
and then did what a successful commit does: `CommitBuffer`'s
|
||||
`ChangeMode(FormulaPanel)` closed the editor and the keymap's welded
|
||||
`ClearBuffer("formula")` cleared the text. The user saw the error with their
|
||||
formula gone. Driven through `App::handle_key` from `FormulaPanel`: `a`, the
|
||||
characters, then Enter. The test failed first with "got FormulaPanel". The fix is
|
||||
`refuse_and_stop` on the parse-error arm, and a `Binding::Sequence` is one
|
||||
effect vec, so the stop also covers the `ClearBuffer` step.
|
||||
|
||||
The mode assertion alone would be vacuous against a fix that stops the mode
|
||||
change but still loses the text. That rival (clear the `formula` buffer, then
|
||||
`refuse_and_stop`) was run and fails "and must keep what they typed" with
|
||||
`left: None`. The control, `Total = 1`, asserts the ordinary path still stores
|
||||
the formula and closes the editor, so a stop on every arm would fail it. After
|
||||
the fix, the abort sweep (`refuse_and_stop` back to `app.refuse`) fails this
|
||||
test at that site.
|
||||
|
||||
### ui::app::tests::every_refusal_site_refuses_its_failure_and_only_its_failure
|
||||
|
||||
improvise-0v8l. PR #185 split refusals (`App::refuse`, which the headless exit
|
||||
code reads) from reports (`set_status`), and most of the refusal sites were
|
||||
unpinned. The ticket counted 23 of 27. At gh/main `608ba6a` (after #181's wire
|
||||
table), a script (`/tmp/refuse-fu/sweep.py`, not committed) reverted each of
|
||||
the **32** production refusal sites in `ui/effect.rs` and `command/cmd/*.rs`
|
||||
alone to a status-only write, and ran `cargo test --workspace --no-fail-fast`.
|
||||
For `effect::refuse(x)` the revert is `effect::set_status(x)`. For
|
||||
`app.refuse(x)` it is `app.view_state.status_msg = x.into()`. For
|
||||
`refuse_and_stop` it is the status write with the abort kept. **13 of 32 failed
|
||||
a test. 19 survived:** `AddCategory` at the limit, `apply_coordinate_rename`,
|
||||
both `StartImportWizard` arms, the five `ImportJsonHeadless` arms the wire
|
||||
table cannot see (the abort alone rejects the frame), `rename_record_coordinate`,
|
||||
`CommitExport`, both `ViewNavigate` arms, both `AddRecordRow` arms, both
|
||||
`EditOrDrill` arms, `SearchNavigate` and `ExecuteCommand`.
|
||||
|
||||
This table has one case per survivor. Each case drives the failure through a
|
||||
`:` line (`run_line`, which does what `improvise cmd` does, one batch per
|
||||
segment) or through Enter in the site's own minibuffer mode. It asserts that the
|
||||
batch refused and that the status names the site's own reason. Each case also
|
||||
has a **succeeding twin** that must not refuse, so a site that refused
|
||||
unconditionally fails too. That rival (`AddCategory` refusing on `Ok` as well)
|
||||
was run and fails "its succeeding twin was refused". Rerun after the change,
|
||||
the sweep catches **32 of 32**. The abort half, 10 sites before v4hg's eleventh,
|
||||
went from 5 caught to 11 of 11.
|
||||
|
||||
Three fixture choices. The second was found when the first run failed: the input meant to be refused was accepted.
|
||||
|
||||
- **`AddCategory` past the limit** needs eleven categories and a two-segment
|
||||
line. The first `add-category` fills the twelfth slot and the second is
|
||||
refused, so the succeeding twin is the same fixture with one segment.
|
||||
- **`add-record-row` onto an existing row** must page the category on the
|
||||
view the client is actually on. Records mode switches to `_Records`, so
|
||||
writing `Workbook::active_view_mut` (the document's default) changed a view
|
||||
nobody was looking at, and the case "succeeded". It goes through
|
||||
`App::view_mut_of(&ViewTarget::Active)` (gotcha 29).
|
||||
- **The drill rename** edits record 1, not record 0, following the
|
||||
cursor-at-index-0 rule in the wave agent rules. Nothing failed here: this is
|
||||
a precaution, not a finding.
|
||||
|
||||
### ui::effect::tests::a_drill_rows_measure_edits_follow_its_rename
|
||||
|
||||
improvise-uo2p. `ApplyAndClearDrill` walked `pending_edits`, a `HashMap`, and
|
||||
@@ -5434,14 +5600,41 @@ rivals it catches:
|
||||
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.
|
||||
fails with "the widest narrowing drill compared froze 1 records".
|
||||
- **`drill-into-cell`'s parser changed to refuse fewer than two arguments**
|
||||
(improvise-b9f2): every one-coordinate drill then goes to the one-record cursor
|
||||
fallback. The first width guard measured what the argument lists match, so it
|
||||
stayed silent. With gotcha 18 re-injected the law passed 38 of 300 runs in
|
||||
review, and 31 of 300 here at `cfcc9bff`. The guard now applies the compared
|
||||
drills' own effects to a scratch `App` and counts the records they froze.
|
||||
Against this rival it fails on every run with "froze 1 records".
|
||||
- A first version of the fix counted all frozen records and **still passed**.
|
||||
A candidate with no `Cat/Item` argument drills the whole store (16 records),
|
||||
and it reads them in insertion order through `iter_cells`, which the
|
||||
injection cannot reach. So the guard counts only drills that froze fewer
|
||||
records than the store holds.
|
||||
- Measured at `7dfa90f1`, test binary in a loop:
|
||||
|
||||
| Faults | Law passes | Guard fired |
|
||||
|---|---|---|
|
||||
| gotcha 18 + parser rival | **0 of 300** | all 300 |
|
||||
| gotcha 18 alone | **0 of 300** | 0 (every failure a real divergence) |
|
||||
| neither | 300 of 300 | 0 |
|
||||
- **`StartDrill`'s `Debug` changed to print only its row count**
|
||||
(`StartDrill(4 rows)`, improvise-btyw): the law compares effects by their
|
||||
`Debug` text, and this rival takes the record order out of that text, while
|
||||
the frozen `drill_state` still holds four records. At `7dfa90f1`, with gotcha
|
||||
18 re-injected, the law **passed 200 of 200** runs and the guard stayed green.
|
||||
The guard now counts a drill only when its frozen records, as `Debug` prints
|
||||
them, appear in the text the comparison compared. The same measurements after
|
||||
the repair, 200 runs each:
|
||||
|
||||
| Faults | Law passes | Guard fired |
|
||||
|---|---|---|
|
||||
| gotcha 18 + `Debug` rival | **0 of 200** | all 200 ("froze 0 records") |
|
||||
| gotcha 18 + parser rival | **0 of 200** | all 200 |
|
||||
| gotcha 18 alone | **0 of 200** | 0 (every failure a real divergence) |
|
||||
| neither | 200 of 200 | 0 |
|
||||
- **A stale exemption**: `("quit", "stale")` listed in `OUTSIDE_THE_CONTEXT`.
|
||||
It fails as "listed … but did not".
|
||||
|
||||
@@ -5519,6 +5712,20 @@ 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`).
|
||||
|
||||
### p_onto_a_sparse_record_writes_it
|
||||
|
||||
improvise-q9eg, a regression introduced by PR #197 and found in its review. In
|
||||
records mode a category is added (as `+cat` does), so every existing record
|
||||
lacks it and it sits on `Axis::None`. `yy` on East/Cost, then `p` on West/Cost
|
||||
(row 1): gh/main pasted 700, and #197 refused with the aggregate message.
|
||||
`value_cell_key` handed `PasteCell` the real record key, and `paste_into` then
|
||||
re-ran `aggregates(ctx, key)`. That key-shape check is a second derivation
|
||||
beside `cell_id`, and it reads a record missing a None-axis category as an
|
||||
aggregate. Every records fixture before this one was fully populated, which is
|
||||
why nothing saw it. The fix is `PasteAt::{Resolved, Spelled}`: a resolved key is
|
||||
written as it stands. Rival: routing `Resolved` back through `paste_into`. This
|
||||
test and `p_onto_a_sparse_record_writes_what_the_browser_writes` fail.
|
||||
|
||||
## `ui/effect.rs` — `RecordRewrite`'s untested branches (improvise-phg1)
|
||||
|
||||
### renaming_a_drill_row_held_by_a_lone_measureless_cell_moves_that_cell
|
||||
@@ -5637,3 +5844,51 @@ another view, may write it. The setup asserts the host really does project
|
||||
Rival: stored records sent through `cell_id` like unstored ones. That rival
|
||||
fails this test and nothing else in `improvise-command`. Before this test,
|
||||
nothing in the suite saw it.
|
||||
|
||||
### a_fresh_sessions_slice_is_the_hosts_startup_slice
|
||||
|
||||
improvise-v8ab. A fresh session's slice is built twice: `App::with_storage`
|
||||
builds the host's own, and `project.rs::fresh_view` builds a client's. Nothing
|
||||
asserted they agree. `tests/lockstep.rs` seeds most of its clients from
|
||||
`app.view_state.clone()`, and those could not see improvise-lxg0's phantom
|
||||
frame or the next drift between the two. There are three exceptions.
|
||||
`switching_between_a_records_view_and_a_pivot_view_settles_both_ways` and
|
||||
`a_locally_resolved_page_turn_lands_where_the_host_would_put_it` go through
|
||||
`Host::connect`, and the `published_basis` helper uses `Sessions::connect`.
|
||||
None of them compares the connected slice with the host's. The test connects to two documents, both opening on `Second`, one with a `Default` view
|
||||
and one without. It asserts the whole `ViewState` equals the host's, not only
|
||||
the back stack, so any field the two constructors disagree on fails it. The
|
||||
rival is the old fold, restored in `fresh_view`. It failed on the
|
||||
`["Default", "Second"]` case: the only difference was `view_back_stack:
|
||||
[ViewFrame { view_name: "Default", mode: Normal }]`.
|
||||
|
||||
The scope is **connect only**. A rebase deliberately resets the client's whole
|
||||
slice, while `install_workbook` keeps the host's mode and panels, so equality
|
||||
is not the contract there. `lockstep.rs` still copies `app.view_state`, and
|
||||
that is deliberate. Its helpers seed a client *after* mutating the host (mode,
|
||||
buffers, cursor), so a `Snapshot` would drop exactly the state those tests
|
||||
stage. This test is what makes the copy faithful at startup.
|
||||
|
||||
### a_document_opening_on_a_records_view_opens_the_host_in_records_normal
|
||||
|
||||
improvise-y5eo. `App::with_storage` set `view_state.mode` to `Normal` and never
|
||||
settled it against the layout. The host settles only at the end of a batch
|
||||
(`settle_normal_mode` in `apply_effects`). A session's mirror settles in
|
||||
`project_diff`, so `Sessions::connect` handed a client `RecordsNormal` while
|
||||
the host sat in `Normal` over the same records grid. The host's first `i` then
|
||||
resolved through `Normal`'s keymap and entered plain `Editing`. The fixture's
|
||||
active view puts `_Index` on rows and `_Dim` on columns, which the `.improv`
|
||||
writer does persist. The test asserts the starting mode, then presses `i` and
|
||||
asserts the host is still in a records mode. The fix calls
|
||||
`settle_normal_mode` once at the end of construction, so the startup mode
|
||||
comes from the same rule every batch ends with, and so from the same
|
||||
`improvise_protocol::settled_mode` a client uses.
|
||||
|
||||
Rival injected: that one call removed. The whole workspace had exactly two
|
||||
failures. This test failed at the mode assertion (`left: Normal, right:
|
||||
RecordsNormal`), and `a_fresh_sessions_slice_is_the_hosts_startup_slice`
|
||||
failed on its new records case: the client's `RecordsNormal` against the
|
||||
host's `Normal`, and nothing else different. `install_workbook` needs no
|
||||
change of its own, because both of its callers (`LoadModel`,
|
||||
`AcceptWizardModel`) run inside `apply_effects`, which settles after the batch.
|
||||
|
||||
|
||||
@@ -763,3 +763,50 @@ to spare a harness directory. `opfs.spec.js` instead asserts `/opfs/index.html`
|
||||
is served, naming the fix. Injected by running `web/assemble.sh harness/serve`
|
||||
alone: 4 chromium failures, each carrying that message, where it used to fail
|
||||
with no hint of the cause.
|
||||
|
||||
### Polling past the second poll — `scripts/pr-reviews-test.sh` (improvise-7epn)
|
||||
|
||||
**What was unguarded.** Every multi-poll fixture resolved on poll 2, and the
|
||||
timeout case expects exit 1 whether the loop polls twice or twenty times. So
|
||||
replacing the post-poll deadline check with an unconditional `break` left the
|
||||
suite at 23 passed, 0 failed, and `--wait N` would have polled at most twice.
|
||||
|
||||
**Two cases, both on the one-tick-per-read clock.** "a review arriving on the
|
||||
third poll is seen" empties polls 1 and 2 and puts an inline comment at the head
|
||||
in `last`, with `--wait 10`, and asserts exit 0 *and* exactly 3 polls. "--wait
|
||||
keeps polling until its deadline" runs the empty fixture with `--wait 5` and
|
||||
asserts exit 1 after at least 5 polls. The second one is the timeout path. Its
|
||||
exit code alone cannot tell a loop that stopped early from one that reached the
|
||||
deadline, so it counts polls.
|
||||
|
||||
**Rival, injected and watched.** `break` in place of
|
||||
`[ "$(date +%s)" -lt "$deadline" ] || break`: 23 passed, 2 failed. The failures
|
||||
were exactly the two new cases, each "exit 1 after 2 polls". The first attempt
|
||||
to inject used `sed -i ''`, which the GNU sed on this PATH read as a filename.
|
||||
Nothing changed and the suite passed 25/25, which looks like a vacuous rival.
|
||||
The second attempt asserted the replacement landed before running.
|
||||
|
||||
**Still not covered.** Deleting the check outright makes the timeout case loop
|
||||
forever rather than fail. The suite has no wall-clock guard, so that fault shows
|
||||
as a hang, not a FAIL.
|
||||
|
||||
### A stale harness lock names its remedy — `harness/build.sh` (improvise-lnsh)
|
||||
|
||||
**What broke.** With a stale `harness/opfs-contract/Cargo.lock`, `build.sh`
|
||||
exited 101 and printed only cargo's text, whose `help:` line says to remove
|
||||
`--locked`. That is the one change that brings improvise-mkbp back. The real
|
||||
remedy was only in a source comment.
|
||||
|
||||
**Reproduced, then fixed, by hand.** Changing `pest`'s locked version from
|
||||
`2.9.2` to `2.9.1` in the lock reproduces the stale state. Before the fix:
|
||||
exit 101 and cargo's hint only. After: exit 1, and the script's own two lines
|
||||
follow cargo's, telling the reader not to remove `--locked` and naming
|
||||
`cargo update -w --manifest-path harness/opfs-contract/Cargo.toml`. The lock
|
||||
was untouched both times. Running that command on the stale lock restored it
|
||||
byte for byte (`cmp` against the committed file). With the committed lock,
|
||||
`build.sh` still runs to "harness/serve ready".
|
||||
|
||||
**Not automated.** There is no suite for `harness/build.sh`. A test would have
|
||||
to corrupt a tracked file and call cargo, which needs the crates.io index, from
|
||||
`checkers.sh`, whose environment is scrubbed. The by-hand run above is the
|
||||
evidence.
|
||||
|
||||
@@ -836,3 +836,37 @@ tests pass — including both of the ones *aimed* at the suppression — and onl
|
||||
this one fails. That is the shape design-principles §7 names: the assertion next
|
||||
door is satisfied by the wrong fix, and the discriminating input is the mode the
|
||||
change was not supposed to touch.
|
||||
|
||||
### the_help_footer_names_the_keys_the_keymap_binds
|
||||
|
||||
**What was wrong (improvise-17ak).** `src/ui/help.rs::render_footer`
|
||||
hand-wrote the popup's paging keys: `l/Tab: next`, `h/S-Tab: prev`,
|
||||
`q/Esc: close`. No check read them, since `every_hint_key_resolves_in_its_mode`
|
||||
cannot see a string literal in a widget (the blind spot improvise-rr8n and
|
||||
improvise-wk18 name). The literal was also already incomplete: the keymap binds
|
||||
`help-page-next` to `n` and `→` as well, and `?` closes help, and the footer
|
||||
advertised none of them.
|
||||
|
||||
**Derive rather than delete.** rr8n's rule is to keep whichever table is
|
||||
derived from the thing it describes, and the derived one here is the status
|
||||
row's `hint_line(Help)`. The popup's margins always leave that row uncovered,
|
||||
which argues for deleting the footer's keys. But `draw_status` prints
|
||||
`status_msg` *instead of* `hint_text()` whenever the message is non-empty, and
|
||||
nothing clears the message on entering Help. So after any message ("Saved to
|
||||
…", an error), a user who presses `?` would see no paging keys at all. The
|
||||
footer therefore now draws the page indicator followed by `hint_line(Help)`
|
||||
itself, truncated in display cells (gotcha 50).
|
||||
|
||||
**The test.** It renders the popup at 140×30, wide enough that the whole hint
|
||||
fits (the rr8n lesson: an 80-column render can hide the very thing asserted),
|
||||
on every page. It requires the footer row to contain `hint_line(&AppMode::Help)`
|
||||
exactly, and first asserts that hint is non-empty and pages.
|
||||
|
||||
**Red, and the rival that matters.**
|
||||
- At base it fails on page 0: `"page 1 of 5 l/Tab: next q/Esc: close"` is
|
||||
not `"Tab/l/n/→:next-page S-Tab/h/p/←:prev-page ?/Esc/q:close"`.
|
||||
- A footer holding a literal copy of today's derived string passes today, so it
|
||||
was injected together with a rebinding (`n` removed from `help-page-next` in
|
||||
the protocol keymap). The test then fails: the copy still advertises `n`,
|
||||
which the keymap no longer binds. That rebinding case is exactly the one the
|
||||
ticket said the old footer would lie through.
|
||||
|
||||
@@ -3,7 +3,7 @@ use crate::model::cell::{CellKey, CellValue};
|
||||
use crate::ui::effect::{self, Effect, ViewTarget};
|
||||
use crate::view::synthetic_record_info;
|
||||
|
||||
use super::core::{Cmd, CmdContext};
|
||||
use super::core::{AGGREGATE_REFUSAL, Cmd, CmdContext, INSIDE_A_DRILL_REFUSAL, NO_VALUE_REFUSAL};
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
@@ -66,7 +66,9 @@ mod tests {
|
||||
("Type".into(), "Clothing".into()),
|
||||
("Month".into(), "Feb".into()),
|
||||
]);
|
||||
let cmd = PasteCell { key: key.clone() };
|
||||
let cmd = PasteCell {
|
||||
at: PasteAt::Spelled(key.clone()),
|
||||
};
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
let mut app = crate::ui::app::App::new(two_cat_model(), None);
|
||||
@@ -88,7 +90,9 @@ mod tests {
|
||||
("Type".into(), "Food".into()),
|
||||
("Month".into(), "Jan".into()),
|
||||
]);
|
||||
let cmd = PasteCell { key };
|
||||
let cmd = PasteCell {
|
||||
at: PasteAt::Spelled(key),
|
||||
};
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert!(effects.is_empty());
|
||||
}
|
||||
@@ -332,9 +336,9 @@ mod tests {
|
||||
app.cursor_mut().selected = (0, col_of(&app, "Revenue"));
|
||||
press(&mut app, 'p');
|
||||
let in_drill = app.view_state.status_msg.clone();
|
||||
assert_eq!(
|
||||
in_drill, PASTE_INSIDE_DRILL,
|
||||
"setup: the drilled paste declines"
|
||||
assert!(
|
||||
in_drill.ends_with(INSIDE_A_DRILL_REFUSAL),
|
||||
"setup: the drilled paste declines, got {in_drill:?}"
|
||||
);
|
||||
assert!(
|
||||
!advice.to_lowercase().contains("drill"),
|
||||
@@ -353,7 +357,10 @@ mod tests {
|
||||
ctx.yanked = &yanked;
|
||||
assert!(!ctx.none_cats().is_empty(), "setup: the view aggregates");
|
||||
let key = record("West", "Revenue").with("Product", "Gadget");
|
||||
let effects = PasteCell { key: key.clone() }.execute(&ctx);
|
||||
let effects = PasteCell {
|
||||
at: PasteAt::Spelled(key.clone()),
|
||||
}
|
||||
.execute(&ctx);
|
||||
let mut app = aggregated_app(crate::view::Axis::None);
|
||||
app.apply_effects(effects);
|
||||
assert_eq!(
|
||||
@@ -389,6 +396,163 @@ mod tests {
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(dbg.contains("Save"), "Expected Save, got: {dbg}");
|
||||
}
|
||||
|
||||
/// improvise-q9eg: a record that lacks a later category is still a record —
|
||||
/// see context/test-evidence/command.md.
|
||||
#[test]
|
||||
fn p_onto_a_sparse_record_writes_it() {
|
||||
let mut app = records_app();
|
||||
app.model_state.workbook.add_category("Season").unwrap();
|
||||
app.rebuild_layout();
|
||||
assert!(
|
||||
app.layout.none_cats.iter().any(|c| c == "Season"),
|
||||
"setup: the new category is aggregated away, so every record lacks it"
|
||||
);
|
||||
app.cursor_mut().selected = (0, col_of(&app, "Cost"));
|
||||
press(&mut app, 'y');
|
||||
press(&mut app, 'y');
|
||||
app.cursor_mut().selected = (1, col_of(&app, "Cost"));
|
||||
press(&mut app, 'p');
|
||||
assert_eq!(
|
||||
app.model_state
|
||||
.workbook
|
||||
.model
|
||||
.get_cell(&record("West", "Cost")),
|
||||
Some(&CellValue::Number(700.0)),
|
||||
"status: {:?}",
|
||||
app.view_state.status_msg
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-lx7w — see context/test-evidence/command.md.
|
||||
#[test]
|
||||
fn x_in_records_mode_clears_the_real_record_under_the_cursor() {
|
||||
let mut app = records_app();
|
||||
app.cursor_mut().selected = (1, col_of(&app, "Cost"));
|
||||
press(&mut app, 'x');
|
||||
let wb = &app.model_state.workbook;
|
||||
assert_eq!(
|
||||
wb.model.get_cell(&record("West", "Cost")),
|
||||
None,
|
||||
"West/Cost"
|
||||
);
|
||||
assert_eq!(
|
||||
wb.model.get_cell(&record("East", "Cost")),
|
||||
Some(&CellValue::Number(700.0)),
|
||||
"the neighbouring record is untouched"
|
||||
);
|
||||
let (text, back) = saved_and_reloaded(wb);
|
||||
assert!(!text.contains("West, _Measure=Cost"), "saved:\n{text}");
|
||||
assert_eq!(back.model.get_cell(&record("West", "Cost")), None);
|
||||
assert_eq!(synthetic_cells(&back), Vec::<CellKey>::new());
|
||||
}
|
||||
|
||||
/// improvise-lx7w: a coordinate column stores no value, so `x` declines there.
|
||||
#[test]
|
||||
fn x_on_a_records_coordinate_column_changes_nothing_and_says_so() {
|
||||
let mut app = records_app();
|
||||
let before = crate::persistence::format_md(&app.model_state.workbook);
|
||||
app.cursor_mut().selected = (1, col_of(&app, "Region"));
|
||||
press(&mut app, 'x');
|
||||
assert_eq!(
|
||||
crate::persistence::format_md(&app.model_state.workbook),
|
||||
before
|
||||
);
|
||||
assert!(
|
||||
app.view_state.status_msg.ends_with(NO_VALUE_REFUSAL),
|
||||
"declined out loud, got {:?}",
|
||||
app.view_state.status_msg
|
||||
);
|
||||
}
|
||||
|
||||
/// Two regions by two raw measures and `Profit = Revenue - Cost`, the formula
|
||||
/// in the third column, so the cursor on it is off row 0 and column 0.
|
||||
fn formula_app() -> App {
|
||||
let mut wb = Workbook::new("T");
|
||||
wb.add_category("Region").unwrap();
|
||||
wb.add_item("Region", "East").unwrap();
|
||||
wb.add_item("Region", "West").unwrap();
|
||||
wb.add_item("_Measure", "Revenue").unwrap();
|
||||
wb.add_item("_Measure", "Cost").unwrap();
|
||||
wb.model.add_formula(
|
||||
crate::formula::parse_formula("Profit = Revenue - Cost", "_Measure").unwrap(),
|
||||
);
|
||||
wb.active_view_mut()
|
||||
.set_axis("Region", crate::view::Axis::Row);
|
||||
wb.active_view_mut()
|
||||
.set_axis("_Measure", crate::view::Axis::Column);
|
||||
for (region, measure, n) in [
|
||||
("East", "Revenue", 10.0),
|
||||
("East", "Cost", 4.0),
|
||||
("West", "Revenue", 9.0),
|
||||
("West", "Cost", 2.0),
|
||||
] {
|
||||
wb.model
|
||||
.set_cell(record(region, measure), CellValue::Number(n));
|
||||
}
|
||||
let mut app = App::new(wb, None);
|
||||
app.view_state.yanked = Some(CellValue::Number(99.0));
|
||||
app
|
||||
}
|
||||
|
||||
/// improvise-54g1 — see context/test-evidence/command.md.
|
||||
#[test]
|
||||
fn p_onto_a_formula_owned_cell_writes_nothing_and_names_the_formula() {
|
||||
let mut app = formula_app();
|
||||
app.cursor_mut().selected = (1, 2);
|
||||
assert_eq!(app.layout.col_label(2), "Profit", "setup: on the formula");
|
||||
press(&mut app, 'p');
|
||||
let (text, back) = saved_and_reloaded(&app.model_state.workbook);
|
||||
assert!(
|
||||
!text.contains("_Measure=Profit"),
|
||||
"saved under the formula:\n{text}"
|
||||
);
|
||||
assert_eq!(back.model.get_cell(&record("West", "Profit")), None);
|
||||
assert!(
|
||||
app.view_state
|
||||
.status_msg
|
||||
.contains("Profit is computed by a formula"),
|
||||
"declined out loud, got {:?}",
|
||||
app.view_state.status_msg
|
||||
);
|
||||
|
||||
// The control: one column left is a stored cell, and p writes it.
|
||||
app.cursor_mut().selected = (1, 1);
|
||||
press(&mut app, 'p');
|
||||
let wb = &app.model_state.workbook;
|
||||
assert_eq!(
|
||||
wb.model.get_cell(&record("West", "Cost")),
|
||||
Some(&CellValue::Number(99.0))
|
||||
);
|
||||
}
|
||||
|
||||
/// The second door onto a formula cell: Tab advances the editor onto it, where
|
||||
/// `i` would have declined. See context/test-evidence/command.md.
|
||||
#[test]
|
||||
fn tab_advancing_onto_a_formula_cell_commits_nothing_under_it() {
|
||||
let mut app = formula_app();
|
||||
app.cursor_mut().selected = (1, 1);
|
||||
press(&mut app, 'i');
|
||||
assert!(app.view_state.mode.is_editing(), "setup: editing West/Cost");
|
||||
app.handle_key(KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE))
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
app.cursor().selected,
|
||||
(1, 2),
|
||||
"setup: Tab moved onto Profit"
|
||||
);
|
||||
for c in "7".chars() {
|
||||
press(&mut app, c);
|
||||
}
|
||||
app.handle_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE))
|
||||
.unwrap();
|
||||
let (text, back) = saved_and_reloaded(&app.model_state.workbook);
|
||||
assert!(
|
||||
!text.contains("_Measure=Profit"),
|
||||
"saved under the formula:\n{text}"
|
||||
);
|
||||
assert_eq!(back.model.get_cell(&record("West", "Profit")), None);
|
||||
}
|
||||
}
|
||||
|
||||
// ── Cell operations ──────────────────────────────────────────────────────────
|
||||
@@ -432,20 +596,31 @@ impl Cmd for YankCell {
|
||||
}
|
||||
}
|
||||
|
||||
/// Where a paste lands: a key the cursor's door already resolved, or one a
|
||||
/// script spelled out, which only the key's own shape can vet (improvise-q9eg).
|
||||
#[derive(Debug)]
|
||||
pub enum PasteAt {
|
||||
Resolved(CellKey),
|
||||
Spelled(CellKey),
|
||||
}
|
||||
|
||||
/// Paste the yanked value into a cell.
|
||||
#[derive(Debug)]
|
||||
pub struct PasteCell {
|
||||
pub key: crate::model::cell::CellKey,
|
||||
pub at: PasteAt,
|
||||
}
|
||||
impl Cmd for PasteCell {
|
||||
fn name(&self) -> &'static str {
|
||||
"paste"
|
||||
}
|
||||
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
match ctx.yanked.clone() {
|
||||
Some(value) => vec![paste_into(ctx, &self.key, value)],
|
||||
None => vec![],
|
||||
}
|
||||
let Some(value) = ctx.yanked.clone() else {
|
||||
return vec![];
|
||||
};
|
||||
vec![match &self.at {
|
||||
PasteAt::Resolved(key) => Box::new(effect::SetCell(key.clone(), value)),
|
||||
PasteAt::Spelled(key) => paste_into(ctx, key, value),
|
||||
}]
|
||||
}
|
||||
}
|
||||
|
||||
@@ -454,7 +629,7 @@ impl Cmd for PasteCell {
|
||||
fn paste_into(ctx: &CmdContext, key: &CellKey, value: CellValue) -> Box<dyn Effect> {
|
||||
match synthetic_record_info(key) {
|
||||
Some((row, col)) => paste_into_record(ctx, row, col, value),
|
||||
None if aggregates(ctx, key) => effect::set_status(PASTE_ONTO_AGGREGATE),
|
||||
None if aggregates(ctx, key) => effect::set_status(AGGREGATE_REFUSAL),
|
||||
None => Box::new(effect::SetCell(key.clone(), value)),
|
||||
}
|
||||
}
|
||||
@@ -466,9 +641,9 @@ fn paste_into_record(
|
||||
value: CellValue,
|
||||
) -> Box<dyn Effect> {
|
||||
match (ctx.layout.record_cell_key(row, col), ctx.has_drill_state) {
|
||||
(Some(_), true) => effect::set_status(PASTE_INSIDE_DRILL),
|
||||
(Some(_), true) => effect::set_status(INSIDE_A_DRILL_REFUSAL),
|
||||
(Some(real), false) => Box::new(effect::SetCell(real, value)),
|
||||
(None, _) => effect::set_status(PASTE_ONTO_NO_VALUE),
|
||||
(None, _) => effect::set_status(NO_VALUE_REFUSAL),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -479,10 +654,6 @@ fn aggregates(ctx: &CmdContext, key: &CellKey) -> bool {
|
||||
})
|
||||
}
|
||||
|
||||
const PASTE_ONTO_AGGREGATE: &str = "Cannot paste onto an aggregate — it combines several records, so there is no one cell to write";
|
||||
const PASTE_ONTO_NO_VALUE: &str = "Paste lands only on a measure column";
|
||||
const PASTE_INSIDE_DRILL: &str = "Paste is not staged inside a drill — edit the cell instead";
|
||||
|
||||
// ── View commands ────────────────────────────────────────────────────────────
|
||||
|
||||
#[derive(Debug)]
|
||||
|
||||
@@ -11,6 +11,21 @@ use crate::ui::app::AppMode;
|
||||
use crate::ui::effect::{Effect, Panel};
|
||||
use crate::view::{Axis, GridLayout, View};
|
||||
use crate::workbook::Workbook;
|
||||
use improvise_protocol::CellId;
|
||||
|
||||
/// What a command says when the cursor sits on a collapsed group's row (§14).
|
||||
pub const FOLD_REFUSAL: &str = "Collapsed group — press z to expand it first";
|
||||
const NO_CELL: &str = "no cell at cursor";
|
||||
pub const AGGREGATE_REFUSAL: &str =
|
||||
"Cannot write to an aggregate — it combines several records, so there is no one cell to write";
|
||||
pub const NO_VALUE_REFUSAL: &str = "Only a measure column holds a value";
|
||||
pub const INSIDE_A_DRILL_REFUSAL: &str =
|
||||
"A value is not staged inside a drill — edit the cell instead";
|
||||
|
||||
/// What a command says when the cell it would write is a formula's result.
|
||||
pub fn derived_refusal(target: &str) -> String {
|
||||
format!("{target} is computed by a formula — edit the formula, not the cell")
|
||||
}
|
||||
|
||||
/// Read-only context available to commands for decision-making: a `&Model`, the
|
||||
/// active `&View`, and the `&Workbook` for the rare command that enumerates views.
|
||||
@@ -64,10 +79,75 @@ impl<'a> CmdContext<'a> {
|
||||
self.layout.is_records_mode()
|
||||
}
|
||||
|
||||
/// The cell under the cursor, for a command that only READS it. The writers'
|
||||
/// doors: `context/code/command.md`, "The cursor's cell has three doors".
|
||||
pub fn cell_key(&self) -> Option<CellKey> {
|
||||
self.layout.cell_key(self.selected.0, self.selected.1)
|
||||
}
|
||||
|
||||
/// The cursor's key for a command that resolves it itself (commit, drill),
|
||||
/// refusing a fold: its key names a group, which no record carries (§14).
|
||||
pub fn unfolded_cell_key(&self) -> Result<CellKey, String> {
|
||||
match self.layout.folds_a_group(self.selected.0, self.selected.1) {
|
||||
true => Err(FOLD_REFUSAL.to_string()),
|
||||
false => self.cell_key().ok_or_else(|| NO_CELL.to_string()),
|
||||
}
|
||||
}
|
||||
|
||||
/// The cursor's key for a commit, which resolves a records position itself
|
||||
/// (gotcha 19). An aggregate or a formula's result is refused, as `i` refuses it.
|
||||
pub fn committable_cell_key(&self) -> Result<CellKey, String> {
|
||||
match self.cursor_cell_id() {
|
||||
Some(CellId::Aggregate(_)) => Err(self.aggregate_refusal()),
|
||||
Some(CellId::Derived { target }) => Err(derived_refusal(&target)),
|
||||
Some(CellId::Record(_) | CellId::Create(_) | CellId::Coordinate) | None => {
|
||||
self.cell_key().ok_or_else(|| NO_CELL.to_string())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The stored cell a VALUE written at the cursor lands on, answered by the
|
||||
/// `CellId` the browser is given: `Record` is the one settable arm (ADR 002).
|
||||
pub fn value_cell_key(&self) -> Result<CellKey, String> {
|
||||
match self.cursor_cell_id() {
|
||||
Some(CellId::Record(_)) => self.stored_cell_key(),
|
||||
Some(CellId::Aggregate(_)) => Err(self.aggregate_refusal()),
|
||||
Some(CellId::Derived { target }) => Err(derived_refusal(&target)),
|
||||
Some(CellId::Create(_) | CellId::Coordinate) => Err(NO_VALUE_REFUSAL.to_string()),
|
||||
None => Err(match self.cell_key() {
|
||||
Some(_) => NO_VALUE_REFUSAL,
|
||||
None => NO_CELL,
|
||||
}
|
||||
.to_string()),
|
||||
}
|
||||
}
|
||||
|
||||
/// What the cursor's cell is, asked exactly as the projection asks it.
|
||||
fn cursor_cell_id(&self) -> Option<CellId> {
|
||||
crate::ui::project::cell_id(self.model, self.layout, self.selected.0, self.selected.1)
|
||||
}
|
||||
|
||||
/// An aggregate's refusal, naming the fold where the aggregate is one (§14).
|
||||
fn aggregate_refusal(&self) -> String {
|
||||
match self.layout.folds_a_group(self.selected.0, self.selected.1) {
|
||||
true => FOLD_REFUSAL,
|
||||
false => AGGREGATE_REFUSAL,
|
||||
}
|
||||
.to_string()
|
||||
}
|
||||
|
||||
/// A `Record`'s real key: a records position resolves through its row, and a
|
||||
/// drill's records are staged, never written directly (gotcha 1, 19).
|
||||
fn stored_cell_key(&self) -> Result<CellKey, String> {
|
||||
let (row, col) = self.selected;
|
||||
let stored = match (self.is_records_mode(), self.has_drill_state) {
|
||||
(true, true) => return Err(INSIDE_A_DRILL_REFUSAL.to_string()),
|
||||
(true, false) => self.layout.record_cell_key(row, col),
|
||||
(false, _) => self.cell_key(),
|
||||
};
|
||||
stored.ok_or_else(|| NO_CELL.to_string())
|
||||
}
|
||||
|
||||
/// The formula that owns the cell under the cursor, if one does. In records
|
||||
/// mode the synthetic key names no measure, so the record's key is resolved.
|
||||
pub fn owning_formula(&self) -> Option<&'a crate::formula::Formula> {
|
||||
|
||||
@@ -1,14 +1,14 @@
|
||||
//! 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 std::collections::BTreeSet;
|
||||
|
||||
use super::core::{Cmd, CmdContext};
|
||||
use super::grid::DrillIntoCell;
|
||||
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};
|
||||
|
||||
@@ -61,27 +61,23 @@ fn one_coordinate_drills(model: &Model) -> Vec<Vec<String>> {
|
||||
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)
|
||||
/// Records a compared drill froze, counted only if it narrowed the store and its
|
||||
/// ordered records are in `compared`, the text the law compared (improvise-btyw).
|
||||
fn records_narrowed(scratch: &mut App, effects: Vec<Box<dyn Effect>>, compared: &str) -> usize {
|
||||
let store = scratch
|
||||
.model_state
|
||||
.workbook
|
||||
.model
|
||||
.data
|
||||
.live_records()
|
||||
.count();
|
||||
scratch.apply_effects(effects);
|
||||
let frozen = scratch
|
||||
.drill_state
|
||||
.as_ref()
|
||||
.filter(|drill| compared.contains(&format!("{:?}", drill.records)))
|
||||
.map_or(0, |drill| drill.records.len());
|
||||
if frozen < store { frozen } else { 0 }
|
||||
}
|
||||
|
||||
/// The registry's candidates, plus names the demo really has: a drill or a fit
|
||||
@@ -108,9 +104,9 @@ fn args_the_document_names(model: &Model) -> Vec<Vec<String>> {
|
||||
all
|
||||
}
|
||||
|
||||
fn effects_of(name: &str, args: &[String], ctx: &CmdContext) -> Option<String> {
|
||||
fn effects_of(name: &str, args: &[String], ctx: &CmdContext) -> Option<Vec<Box<dyn Effect>>> {
|
||||
let cmd = ctx.registry.interactive(name, args, ctx).ok()?;
|
||||
Some(format!("{:?}", cmd.execute(ctx)))
|
||||
Some(cmd.execute(ctx))
|
||||
}
|
||||
|
||||
/// What running every registered command under `a` and under each of `others` found.
|
||||
@@ -120,20 +116,34 @@ struct Comparison {
|
||||
shown: Vec<String>,
|
||||
never_built: Vec<&'static str>,
|
||||
compared: usize,
|
||||
widest_drill: usize,
|
||||
}
|
||||
|
||||
fn compare(a: &CmdContext, others: &[CmdContext]) -> Comparison {
|
||||
let mut found = Comparison::default();
|
||||
let candidates = args_the_document_names(a.model);
|
||||
let drill = DrillIntoCell {
|
||||
key: CellKey::new(vec![]),
|
||||
};
|
||||
let mut scratch = fixture();
|
||||
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 {
|
||||
let Some(effects) = effects_of(name, args, a) else {
|
||||
continue;
|
||||
};
|
||||
let left = format!("{effects:?}");
|
||||
if name == drill.name() {
|
||||
found.widest_drill =
|
||||
found
|
||||
.widest_drill
|
||||
.max(records_narrowed(&mut scratch, effects, &left));
|
||||
}
|
||||
found.compared += 1;
|
||||
for b in others {
|
||||
let right = effects_of(name, args, b).unwrap_or_default();
|
||||
let right = effects_of(name, args, b)
|
||||
.map(|e| format!("{e:?}"))
|
||||
.unwrap_or_default();
|
||||
if left != right {
|
||||
found.diverged.insert(name);
|
||||
found
|
||||
@@ -157,13 +167,12 @@ fn every_command_gives_equal_effects_in_independently_built_equal_contexts() {
|
||||
.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.widest_drill >= 4,
|
||||
"the widest narrowing drill compared froze {} records, so the store's order is nearly a coin toss",
|
||||
found.widest_drill
|
||||
);
|
||||
assert!(
|
||||
found.never_built.is_empty(),
|
||||
"no candidate args build these in the fixture, so the law never ran them: {:?}",
|
||||
|
||||
@@ -11,6 +11,96 @@ mod tests {
|
||||
use super::*;
|
||||
use crate::command::cmd::test_helpers::*;
|
||||
|
||||
fn press(app: &mut crate::ui::app::App, c: char) {
|
||||
use crate::command::key::{KeyCode, KeyEvent, KeyModifiers};
|
||||
app.handle_key(KeyEvent::new(KeyCode::Char(c), KeyModifiers::NONE))
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
/// improvise-gw18 — see context/test-evidence/command.md.
|
||||
#[test]
|
||||
fn drilling_into_a_fold_is_refused_and_o_after_it_writes_no_group_coordinate() {
|
||||
let mut app = app_on_a_fold();
|
||||
let origin = app.view_state.active_view.clone();
|
||||
press(&mut app, '>');
|
||||
assert_eq!(app.view_state.active_view, origin, "no drill was opened");
|
||||
assert!(
|
||||
app.view_state.status_msg.contains("ollapsed group"),
|
||||
"the refusal is out loud, got: {:?}",
|
||||
app.view_state.status_msg
|
||||
);
|
||||
press(&mut app, 'o');
|
||||
|
||||
let (text, back) = saved_and_reloaded(&app.model_state.workbook);
|
||||
let data = text.split("## Data").nth(1).expect("a data section");
|
||||
assert!(
|
||||
!data.contains("Q2"),
|
||||
"a group coordinate was saved:\n{text}"
|
||||
);
|
||||
assert_eq!(non_item_coordinates(&back), Vec::<(String, String)>::new());
|
||||
}
|
||||
|
||||
/// The commit door onto a fold: Enter advances the editor onto it, and the next
|
||||
/// commit is what must refuse. See context/test-evidence/command.md.
|
||||
#[test]
|
||||
fn enter_advancing_onto_a_fold_commits_nothing_there() {
|
||||
use crate::command::key::{KeyCode, KeyEvent, KeyModifiers};
|
||||
let enter = KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE);
|
||||
let mut app = app_on_a_fold();
|
||||
app.cursor_mut().selected = (1, 1);
|
||||
press(&mut app, 'i');
|
||||
app.handle_key(enter).unwrap();
|
||||
assert_eq!(
|
||||
app.cursor().selected,
|
||||
(2, 1),
|
||||
"setup: Enter moved onto the fold"
|
||||
);
|
||||
press(&mut app, '7');
|
||||
app.handle_key(enter).unwrap();
|
||||
let (text, back) = saved_and_reloaded(&app.model_state.workbook);
|
||||
let data = text.split("## Data").nth(1).expect("a data section");
|
||||
assert!(
|
||||
!data.contains("Q2"),
|
||||
"a group coordinate was saved:\n{text}"
|
||||
);
|
||||
assert_eq!(non_item_coordinates(&back), Vec::<(String, String)>::new());
|
||||
}
|
||||
|
||||
/// The control for the refusal above: the same key one row up drills.
|
||||
#[test]
|
||||
fn drilling_into_the_row_above_a_fold_still_opens_a_drill() {
|
||||
let mut app = app_on_a_fold();
|
||||
app.cursor_mut().selected = (1, 1);
|
||||
let origin = app.view_state.active_view.clone();
|
||||
press(&mut app, '>');
|
||||
assert_ne!(app.view_state.active_view, origin, "Feb × Cost drills");
|
||||
}
|
||||
|
||||
/// improvise-yk2s — see context/test-evidence/command.md.
|
||||
#[test]
|
||||
fn hiding_a_folds_row_is_refused_and_persists_no_hidden_group() {
|
||||
let mut app = app_on_a_fold();
|
||||
press(&mut app, 'H');
|
||||
let view = app.model_state.workbook.active_view();
|
||||
assert!(!view.is_hidden("Month", "Q2"), "no ghost hidden entry");
|
||||
assert!(
|
||||
app.view_state.status_msg.contains("ollapsed group"),
|
||||
"the refusal is out loud, got: {:?}",
|
||||
app.view_state.status_msg
|
||||
);
|
||||
let (text, _) = saved_and_reloaded(&app.model_state.workbook);
|
||||
assert!(
|
||||
!text.contains("hidden:"),
|
||||
"a hidden entry was saved:\n{text}"
|
||||
);
|
||||
|
||||
// The control: the row above is an item, and H hides it.
|
||||
app.cursor_mut().selected = (1, 1);
|
||||
press(&mut app, 'H');
|
||||
let view = app.model_state.workbook.active_view();
|
||||
assert!(view.is_hidden("Month", "Feb"), "H on Feb hides Feb");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn toggle_group_under_cursor_returns_empty_without_groups() {
|
||||
let m = two_cat_model();
|
||||
@@ -505,6 +595,10 @@ impl Cmd for HideSelectedRowItem {
|
||||
let Some(cat_name) = ctx.layout.row_cats.first().cloned() else {
|
||||
return vec![];
|
||||
};
|
||||
// A fold's row names a group, not an item, so there is nothing to hide (§14).
|
||||
if ctx.layout.row_folds_a_group(ctx.selected.0) {
|
||||
return vec![effect::refuse(super::core::FOLD_REFUSAL)];
|
||||
}
|
||||
let sel_row = ctx.selected.0;
|
||||
let Some(items) = ctx
|
||||
.layout
|
||||
|
||||
@@ -143,4 +143,62 @@ pub(super) mod test_helpers {
|
||||
pub fn make_registry() -> CmdRegistry {
|
||||
default_registry()
|
||||
}
|
||||
|
||||
/// Two quarters of months on Row, two measures on Column, every cell funded,
|
||||
/// Q2 collapsed: rows `[Jan, Feb, Q2]`, and the cursor on the fold at `(2, 1)`.
|
||||
pub fn app_on_a_fold() -> crate::ui::app::App {
|
||||
use crate::model::cell::{CellKey, CellValue};
|
||||
use crate::view::Axis;
|
||||
let mut wb = Workbook::new("T");
|
||||
wb.add_category("Month").unwrap();
|
||||
for (month, quarter) in [("Jan", "Q1"), ("Feb", "Q1"), ("Apr", "Q2"), ("May", "Q2")] {
|
||||
wb.add_item_in_group("Month", month, quarter).unwrap();
|
||||
}
|
||||
wb.add_item("_Measure", "Revenue").unwrap();
|
||||
wb.add_item("_Measure", "Cost").unwrap();
|
||||
for (i, month) in ["Jan", "Feb", "Apr", "May"].into_iter().enumerate() {
|
||||
for measure in ["Revenue", "Cost"] {
|
||||
let key = CellKey::new(vec![
|
||||
("Month".into(), month.into()),
|
||||
("_Measure".into(), measure.into()),
|
||||
]);
|
||||
wb.model.set_cell(key, CellValue::Number(10.0 + i as f64));
|
||||
}
|
||||
}
|
||||
wb.active_view_mut().set_axis("Month", Axis::Row);
|
||||
wb.active_view_mut().set_axis("_Measure", Axis::Column);
|
||||
wb.active_view_mut().toggle_group_collapse("Month", "Q2");
|
||||
let mut app = crate::ui::app::App::new(wb, None);
|
||||
app.rebuild_layout();
|
||||
app.cursor_mut().selected = (2, 1);
|
||||
assert!(
|
||||
app.layout.folds_a_group(2, 1),
|
||||
"setup: the cursor is on the fold"
|
||||
);
|
||||
assert_eq!(app.layout.row_label(2), "Q2", "setup: the fold is Q2's row");
|
||||
app
|
||||
}
|
||||
|
||||
/// Every stored coordinate naming no item of its category — what a write to a
|
||||
/// fold leaves behind (core.md §14).
|
||||
pub fn non_item_coordinates(wb: &Workbook) -> Vec<(String, String)> {
|
||||
let model = &wb.model;
|
||||
model
|
||||
.data
|
||||
.iter_cells()
|
||||
.flat_map(|(key, _)| key.0.clone())
|
||||
.filter(|(cat, item)| {
|
||||
!model
|
||||
.category(cat)
|
||||
.is_some_and(|c| c.items.contains_key(item.as_str()))
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// The document saved and read back: what reaches the file, not the store.
|
||||
pub fn saved_and_reloaded(wb: &Workbook) -> (String, Workbook) {
|
||||
let text = crate::persistence::format_md(wb);
|
||||
let back = crate::persistence::parse_md(&text).expect("the saved text reloads");
|
||||
(text, back)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -380,9 +380,8 @@ impl Cmd for EditOrDrill {
|
||||
// The same question `cell_id` asks, off the same predicate. In records mode
|
||||
// the synthetic key names no measure, so the record's key is resolved.
|
||||
if let Some(formula) = ctx.owning_formula() {
|
||||
return vec![effect::refuse(format!(
|
||||
"{} is computed by a formula — edit the formula, not the cell",
|
||||
formula.target
|
||||
return vec![effect::refuse(super::core::derived_refusal(
|
||||
&formula.target,
|
||||
))];
|
||||
}
|
||||
vec![
|
||||
|
||||
@@ -118,7 +118,7 @@ impl Cmd for Move {
|
||||
}
|
||||
}
|
||||
MoveKind::Page(dir) => {
|
||||
let delta = (self.cursor.visible_rows as i32 * 3 / 4).max(1) * dir;
|
||||
let delta = improvise_protocol::resolve::page_rows(self.cursor.visible_rows) * dir;
|
||||
let nr = (self.cursor.row as i32 + delta).clamp(0, row_max) as usize;
|
||||
(nr, self.cursor.col)
|
||||
}
|
||||
|
||||
@@ -22,9 +22,20 @@ use super::text_buffer::*;
|
||||
use super::tile::*;
|
||||
use super::wizard::*;
|
||||
|
||||
/// The cell under the cursor: what a coordinate command falls back to when given none.
|
||||
/// The cell under the cursor for `drill-into-cell`, which opens aggregates but
|
||||
/// not a fold. Which command takes which door: `context/code/command.md`.
|
||||
fn cursor_key(ctx: &CmdContext) -> Result<CellKey, String> {
|
||||
ctx.cell_key().clone().ok_or("no cell at cursor".into())
|
||||
ctx.unfolded_cell_key()
|
||||
}
|
||||
|
||||
/// The stored cell a value lands on, for `clear-cell` and `paste`.
|
||||
fn cursor_value_key(ctx: &CmdContext) -> Result<CellKey, String> {
|
||||
ctx.value_cell_key()
|
||||
}
|
||||
|
||||
/// [`cursor_key`] for the one command that only READS the cell: `yank`.
|
||||
fn cursor_key_to_read(ctx: &CmdContext) -> Result<CellKey, String> {
|
||||
ctx.cell_key().ok_or("no cell at cursor".into())
|
||||
}
|
||||
|
||||
impl AtCursor for Move {
|
||||
@@ -57,7 +68,7 @@ fn commit_at_cursor(
|
||||
Ok(CommitAndAdvance {
|
||||
edit_mode: parse_mode_name(mode)?,
|
||||
value: read_buffer(ctx, "edit"),
|
||||
key: cursor_key(ctx)?,
|
||||
key: ctx.committable_cell_key()?,
|
||||
advance,
|
||||
cursor: CursorState::from_ctx(ctx),
|
||||
})
|
||||
@@ -89,7 +100,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
},
|
||||
Some(|_, ctx| {
|
||||
Ok(ClearCellCommand {
|
||||
key: cursor_key(ctx)?,
|
||||
key: cursor_value_key(ctx)?,
|
||||
})
|
||||
}),
|
||||
);
|
||||
@@ -237,25 +248,25 @@ pub fn default_registry() -> CmdRegistry {
|
||||
},
|
||||
Some(|_, ctx| {
|
||||
Ok(YankCell {
|
||||
key: cursor_key(ctx)?,
|
||||
key: cursor_key_to_read(ctx)?,
|
||||
})
|
||||
}),
|
||||
);
|
||||
r.register(
|
||||
&PasteCell {
|
||||
key: CellKey::new(vec![]),
|
||||
at: PasteAt::Spelled(CellKey::new(vec![])),
|
||||
},
|
||||
|args| {
|
||||
if args.is_empty() {
|
||||
return Err("paste requires at least one Cat/Item coordinate".into());
|
||||
}
|
||||
Ok(PasteCell {
|
||||
key: parse_cell_key_from_args(args),
|
||||
at: PasteAt::Spelled(parse_cell_key_from_args(args)),
|
||||
})
|
||||
},
|
||||
Some(|_, ctx| {
|
||||
Ok(PasteCell {
|
||||
key: cursor_key(ctx)?,
|
||||
at: PasteAt::Resolved(cursor_value_key(ctx)?),
|
||||
})
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -477,6 +477,69 @@ mod tests {
|
||||
out
|
||||
}
|
||||
|
||||
/// improvise-gw18: every Normal key, prefixes walked, pressed on a fold with a
|
||||
/// value yanked — see context/test-evidence/command.md.
|
||||
#[test]
|
||||
fn no_normal_mode_key_pressed_on_a_fold_stores_a_group_coordinate() {
|
||||
use improvise_protocol::key::KeyEvent;
|
||||
let ks = KeymapSet::default_keymaps();
|
||||
let normal = ks
|
||||
.maps()
|
||||
.get(&ModeKey::Normal)
|
||||
.expect("a Normal keymap")
|
||||
.clone();
|
||||
let mods = [
|
||||
KeyModifiers::NONE,
|
||||
KeyModifiers::SHIFT,
|
||||
KeyModifiers::CONTROL,
|
||||
KeyModifiers::ALT,
|
||||
];
|
||||
let mut strokes: Vec<Vec<(KeyCode, KeyModifiers)>> = Vec::new();
|
||||
for key in probe_keys() {
|
||||
for m in mods {
|
||||
match normal.lookup(key, m) {
|
||||
Some(Binding::Prefix(sub)) => strokes.extend(
|
||||
probe_keys()
|
||||
.into_iter()
|
||||
.flat_map(|k2| mods.map(|m2| (k2, m2)))
|
||||
.filter(|&(k2, m2)| sub.lookup(k2, m2).is_some())
|
||||
.map(|second| vec![(key, m), second]),
|
||||
),
|
||||
Some(_) => strokes.push(vec![(key, m)]),
|
||||
None => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
strokes
|
||||
.iter()
|
||||
.any(|s| s == &[(KeyCode::Char('p'), KeyModifiers::NONE)]),
|
||||
"the sweep reaches paste"
|
||||
);
|
||||
// No key on a fold may touch the stored data or the hidden set at all.
|
||||
let touched = |app: &crate::ui::app::App| {
|
||||
let wb = &app.model_state.workbook;
|
||||
let text = crate::persistence::format_md(wb);
|
||||
let data = text.split("## Data").nth(1).unwrap_or_default().to_string();
|
||||
(data, wb.active_view().hidden_items.clone())
|
||||
};
|
||||
let before = touched(&test_helpers::app_on_a_fold());
|
||||
for stroke in &strokes {
|
||||
let mut app = test_helpers::app_on_a_fold();
|
||||
app.view_state.yanked = Some(crate::model::cell::CellValue::Number(5.0));
|
||||
for &(key, m) in stroke {
|
||||
let _ = app.handle_key(KeyEvent::new(key, m));
|
||||
}
|
||||
let stray = test_helpers::non_item_coordinates(&app.model_state.workbook);
|
||||
assert!(stray.is_empty(), "{stroke:?} on a fold stored {stray:?}");
|
||||
assert_eq!(
|
||||
touched(&app),
|
||||
before,
|
||||
"{stroke:?} on a fold changed the document"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// A representative `AppMode` for each keymap, so a test can put a
|
||||
/// `CmdContext` in the mode whose bindings it is walking.
|
||||
fn app_mode_for(mode: &ModeKey) -> AppMode {
|
||||
|
||||
@@ -386,7 +386,9 @@ impl App {
|
||||
let view = workbook.active_view();
|
||||
GridLayout::with_frozen_records(&workbook.model, view, None)
|
||||
};
|
||||
Self {
|
||||
// A document may open on a records grid, so the first mode is settled
|
||||
// by the rule every batch ends with (improvise-y5eo).
|
||||
let mut app = Self {
|
||||
model_state: ModelState {
|
||||
workbook,
|
||||
file_path,
|
||||
@@ -416,7 +418,9 @@ impl App {
|
||||
last_reframed_elsewhere: std::collections::BTreeSet::new(),
|
||||
keymap_set: KeymapSet::default_keymaps(),
|
||||
storage,
|
||||
}
|
||||
};
|
||||
app.settle_normal_mode();
|
||||
app
|
||||
}
|
||||
|
||||
/// This client's cursor in the active view. Absent means the origin, so
|
||||
@@ -591,7 +595,8 @@ impl App {
|
||||
fn aggregate_refusal(&self, hash: RecordHash, key: &CellKey) -> Option<String> {
|
||||
let stored = self.model_state.workbook.model.data.resolve_record(hash);
|
||||
let (row, col) = stored.map_or_else(|| self.cell_position(key), |_| None)?;
|
||||
let projected = crate::ui::project::cell_id(self, &self.layout, row, col);
|
||||
let projected =
|
||||
crate::ui::project::cell_id(&self.model_state.workbook.model, &self.layout, row, col);
|
||||
matches!(projected, Some(CellId::Aggregate(_))).then(|| {
|
||||
"an aggregate combines several records, so there is no one cell to write".into()
|
||||
})
|
||||
@@ -2797,6 +2802,29 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-y5eo: a document that opens on a records grid opens the host in
|
||||
/// `RecordsNormal`. See `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn a_document_opening_on_a_records_view_opens_the_host_in_records_normal() {
|
||||
use crate::view::Axis;
|
||||
let mut wb = Workbook::new("T");
|
||||
wb.add_category("Region").unwrap();
|
||||
wb.add_item("Region", "East").unwrap();
|
||||
wb.active_view_mut().set_axis("_Index", Axis::Row);
|
||||
wb.active_view_mut().set_axis("_Dim", Axis::Column);
|
||||
let mut app = App::new(wb, None);
|
||||
assert!(app.layout.is_records_mode(), "setup: the grid is records");
|
||||
assert_eq!(app.view_state.mode, AppMode::RecordsNormal);
|
||||
|
||||
app.handle_key(KeyEvent::new(KeyCode::Char('i'), KeyModifiers::NONE))
|
||||
.unwrap();
|
||||
assert!(
|
||||
app.view_state.mode.is_records(),
|
||||
"the host's first `i` left records mode: {:?}",
|
||||
app.view_state.mode
|
||||
);
|
||||
}
|
||||
|
||||
/// The settle's `RecordsNormal` arm, driven from the mode it names. The
|
||||
/// `:` crossing above enters the settle in `Normal` both ways — the evidence.
|
||||
#[test]
|
||||
@@ -3040,16 +3068,34 @@ mod tests {
|
||||
fn a_failed_load_or_import_does_not_run_the_quit_after_it() {
|
||||
use improvise_protocol::keymap::Keymap;
|
||||
|
||||
for (command, path) in [("load", "absent.improv"), ("import-json", "absent.json")] {
|
||||
// One case per `refuse_and_stop` site (improvise-514o).
|
||||
let cases: [(&str, &[&str], &str); 7] = [
|
||||
("load", &["absent.improv"], "Load error"),
|
||||
("import-json", &["absent.json"], "Cannot read 'absent.json'"),
|
||||
("import-json", &["absent.csv"], "Cannot read 'absent.csv'"),
|
||||
("import-json", &["bad.csv"], "CSV error"),
|
||||
("import-json", &["bad.json"], "JSON parse error"),
|
||||
(
|
||||
"import-json",
|
||||
&["nested.json", "M", "nope"],
|
||||
"No array at path",
|
||||
),
|
||||
("import-json", &["measures.json"], "Import error"),
|
||||
];
|
||||
for (command, args, arm) in cases {
|
||||
let path = args.join(" ");
|
||||
let mut app = App::new(Workbook::new("T"), None);
|
||||
app.storage = Box::new(crate::storage::MemoryFs::new());
|
||||
app.storage = Box::new(crate::ui::host::import_fixture_storage());
|
||||
let key = KeyCode::Char('x');
|
||||
let mods = KeyModifiers::NONE;
|
||||
let mut km = Keymap::new();
|
||||
km.bind_seq(
|
||||
key,
|
||||
mods,
|
||||
vec![(command, vec![path.to_string()]), ("force-quit", vec![])],
|
||||
vec![
|
||||
(command, args.iter().map(|a| a.to_string()).collect()),
|
||||
("force-quit", vec![]),
|
||||
],
|
||||
);
|
||||
let effects = {
|
||||
let ctx = app.cmd_context(key, mods);
|
||||
@@ -3059,8 +3105,8 @@ mod tests {
|
||||
app.apply_effects(effects);
|
||||
|
||||
assert!(
|
||||
app.last_batch_refused(),
|
||||
"setup: `{command}` must actually have failed, got {:?}",
|
||||
app.last_batch_refused() && app.view_state.status_msg.contains(arm),
|
||||
"setup: `{command} {path}` must fail in the {arm:?} arm, got {:?}",
|
||||
app.view_state.status_msg
|
||||
);
|
||||
assert_ne!(
|
||||
@@ -3071,6 +3117,392 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-v4hg: Enter on a formula that does not parse threw the typed text
|
||||
/// away. Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn a_formula_that_does_not_parse_keeps_the_user_in_the_editor_with_their_text() {
|
||||
let type_and_commit = |text: &str| {
|
||||
let mut app = two_col_model();
|
||||
app.view_state.mode = AppMode::FormulaPanel;
|
||||
let press = |app: &mut App, code| {
|
||||
app.handle_key(KeyEvent::new(code, KeyModifiers::NONE))
|
||||
.unwrap()
|
||||
};
|
||||
press(&mut app, KeyCode::Char('a'));
|
||||
assert!(
|
||||
matches!(app.view_state.mode, AppMode::FormulaEdit { .. }),
|
||||
"setup: `a` in the formula panel opens the editor"
|
||||
);
|
||||
for c in text.chars() {
|
||||
press(&mut app, KeyCode::Char(c));
|
||||
}
|
||||
press(&mut app, KeyCode::Enter);
|
||||
app
|
||||
};
|
||||
|
||||
// The control: a formula that parses is stored and the editor closes.
|
||||
let ok = type_and_commit("Total = 1");
|
||||
assert_eq!(ok.view_state.mode, AppMode::FormulaPanel);
|
||||
assert!(
|
||||
ok.model_state
|
||||
.workbook
|
||||
.model
|
||||
.formulas()
|
||||
.iter()
|
||||
.any(|f| f.target == "Total"),
|
||||
"the control formula must land"
|
||||
);
|
||||
|
||||
let bad = type_and_commit("Total = (");
|
||||
assert!(
|
||||
bad.last_batch_refused() && bad.view_state.status_msg.contains("Formula error"),
|
||||
"the parse error must be reported, got {:?}",
|
||||
bad.view_state.status_msg
|
||||
);
|
||||
assert!(
|
||||
matches!(bad.view_state.mode, AppMode::FormulaEdit { .. }),
|
||||
"a refused formula must leave the user in the editor, got {:?}",
|
||||
bad.view_state.mode
|
||||
);
|
||||
assert_eq!(
|
||||
bad.view_state.buffers.get("formula").map(String::as_str),
|
||||
Some("Total = ("),
|
||||
"and must keep what they typed"
|
||||
);
|
||||
}
|
||||
|
||||
/// Run a `:` line the way `improvise cmd` does, one batch per segment; true
|
||||
/// when any segment refused, which is what its exit code reads.
|
||||
fn run_line(app: &mut App, line: &str) -> bool {
|
||||
let mut refused = false;
|
||||
for segment in crate::command::parse::segments(line) {
|
||||
let effects = {
|
||||
let ctx = app.cmd_context(KeyCode::Null, KeyModifiers::NONE);
|
||||
crate::command::parse::parse_segment_interactive(segment, &ctx)
|
||||
.unwrap_or_else(|e| panic!("`{segment}` must parse: {e}"))
|
||||
.map(|cmd| cmd.execute(&ctx))
|
||||
.unwrap_or_default()
|
||||
};
|
||||
app.apply_effects(effects);
|
||||
refused |= app.last_batch_refused();
|
||||
}
|
||||
refused
|
||||
}
|
||||
|
||||
/// Commit `text` in a minibuffer mode with Enter; true when that batch refused.
|
||||
fn commit_in(app: &mut App, mode: AppMode, buffer: &str, text: &str) -> bool {
|
||||
app.view_state.mode = mode;
|
||||
app.view_state
|
||||
.buffers
|
||||
.insert(buffer.to_string(), text.to_string());
|
||||
app.handle_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE))
|
||||
.unwrap();
|
||||
app.last_batch_refused()
|
||||
}
|
||||
|
||||
fn with_import_fixtures(mut app: App) -> App {
|
||||
app.storage = Box::new(crate::ui::host::import_fixture_storage());
|
||||
app
|
||||
}
|
||||
|
||||
/// Eleven regular categories: one short of the limit.
|
||||
fn one_category_short_of_the_limit() -> App {
|
||||
let mut wb = Workbook::new("T");
|
||||
for n in 0..11 {
|
||||
wb.add_category(format!("C{n}")).unwrap();
|
||||
}
|
||||
App::new(wb, None)
|
||||
}
|
||||
|
||||
/// Stage one coordinate rename in a drill over the records fixture, then commit it.
|
||||
fn commit_drill_rename(app: &mut App, to: &str) -> bool {
|
||||
let records = app.layout.records.clone().expect("records mode");
|
||||
let mut pending_edits = HashMap::new();
|
||||
pending_edits.insert(
|
||||
(1, crate::view::RecordCol::Category("Region".into())),
|
||||
to.into(),
|
||||
);
|
||||
app.drill_state = Some(DrillState {
|
||||
records,
|
||||
pending_edits,
|
||||
});
|
||||
app.apply_effects(vec![Box::new(crate::ui::effect::ApplyAndClearDrill)]);
|
||||
app.last_batch_refused()
|
||||
}
|
||||
|
||||
/// Records mode with `Region` paged to `page`: `add-record-row` then names
|
||||
/// the row whose coordinates are exactly that page.
|
||||
fn records_paged_to(app: &mut App, page: &str) {
|
||||
app.model_state.workbook.add_item("Region", "West").unwrap();
|
||||
let view = app
|
||||
.view_mut_of(&ViewTarget::Active)
|
||||
.expect("the active view");
|
||||
view.set_axis("Region", crate::view::Axis::Page);
|
||||
view.set_page_selection("Region", page);
|
||||
app.rebuild_layout();
|
||||
}
|
||||
|
||||
/// A grid with a regular category on None, so every cell is an aggregate.
|
||||
fn aggregated_grid(with_items: bool) -> App {
|
||||
let mut wb = Workbook::new("T");
|
||||
for cat in ["Row", "Col", "Hidden"] {
|
||||
wb.add_category(cat).unwrap();
|
||||
if with_items {
|
||||
wb.add_item(cat, "A").unwrap();
|
||||
}
|
||||
}
|
||||
wb.active_view_mut()
|
||||
.set_axis("Hidden", crate::view::Axis::None);
|
||||
let mut app = App::new(wb, None);
|
||||
app.rebuild_layout();
|
||||
app
|
||||
}
|
||||
|
||||
/// `Region` down, `_Measure` across: `Revenue` at column 0, then `Total`,
|
||||
/// which a formula owns, at column 1.
|
||||
fn a_formula_column() -> App {
|
||||
let mut wb = Workbook::new("T");
|
||||
wb.add_category("Region").unwrap();
|
||||
wb.add_item("Region", "North").unwrap();
|
||||
wb.add_item("_Measure", "Revenue").unwrap();
|
||||
wb.active_view_mut()
|
||||
.set_axis("_Measure", crate::view::Axis::Column);
|
||||
let mut app = App::new(wb, None);
|
||||
app.apply_effects(vec![Box::new(crate::ui::effect::AddFormula {
|
||||
raw: "Total = Revenue * 2".into(),
|
||||
target_category: "_Measure".into(),
|
||||
})]);
|
||||
app
|
||||
}
|
||||
|
||||
fn with_a_cell(mut app: App) -> App {
|
||||
app.model_state.workbook.model.set_cell(
|
||||
CellKey::new(vec![("Row".into(), "A".into()), ("Col".into(), "X".into())]),
|
||||
CellValue::Number(42.0),
|
||||
);
|
||||
app.rebuild_layout();
|
||||
app
|
||||
}
|
||||
|
||||
fn with_a_second_view(mut app: App) -> App {
|
||||
app.model_state.workbook.create_view("Second");
|
||||
app
|
||||
}
|
||||
|
||||
fn switch_to_second(app: &mut App) {
|
||||
app.apply_effects(vec![Box::new(crate::ui::effect::SwitchView(
|
||||
"Second".into(),
|
||||
))]);
|
||||
}
|
||||
|
||||
/// improvise-0v8l: one case per refusal site no other test pinned, each with a
|
||||
/// succeeding twin. Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn every_refusal_site_refuses_its_failure_and_only_its_failure() {
|
||||
type Setup = fn() -> App;
|
||||
type Drive = fn(&mut App) -> bool;
|
||||
let cases: Vec<(&str, Setup, Drive, &str, Setup, Drive)> = vec![
|
||||
(
|
||||
"add-category past the limit",
|
||||
one_category_short_of_the_limit,
|
||||
|app| run_line(app, "add-category A . add-category B"),
|
||||
"Maximum of 12 categories",
|
||||
one_category_short_of_the_limit,
|
||||
|app| run_line(app, "add-category A"),
|
||||
),
|
||||
(
|
||||
"a drill commit that empties a coordinate",
|
||||
records_app_for_drill,
|
||||
|app| commit_drill_rename(app, ""),
|
||||
crate::ui::effect::RECORD_COORDS_CANNOT_BE_EMPTY,
|
||||
records_app_for_drill,
|
||||
|app| commit_drill_rename(app, "West"),
|
||||
),
|
||||
(
|
||||
"import of a JSON that does not parse",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import bad.json"),
|
||||
"JSON parse error",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import good.json"),
|
||||
),
|
||||
(
|
||||
"import of an absent file",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import absent.json"),
|
||||
"Cannot read file",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import good.json"),
|
||||
),
|
||||
(
|
||||
"import-json of an absent CSV",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json absent.csv"),
|
||||
"Cannot read 'absent.csv'",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json good.json"),
|
||||
),
|
||||
(
|
||||
"import-json of a CSV that is not text",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json bad.csv"),
|
||||
"CSV error",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json good.json"),
|
||||
),
|
||||
(
|
||||
"import-json of JSON that does not parse",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json bad.json"),
|
||||
"JSON parse error",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json good.json"),
|
||||
),
|
||||
(
|
||||
"import-json at a path with no array",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json nested.json M nope"),
|
||||
"No array at path",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json good.json"),
|
||||
),
|
||||
(
|
||||
"import-json that builds no model",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json measures.json"),
|
||||
"Import error",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json good.json"),
|
||||
),
|
||||
(
|
||||
"a records coordinate edited to nothing",
|
||||
records_model_with_two_rows,
|
||||
|app| {
|
||||
app.cursor_mut().selected = (1, 0);
|
||||
commit_in(app, AppMode::records_editing(), "edit", "")
|
||||
},
|
||||
crate::ui::effect::RECORD_COORDS_CANNOT_BE_EMPTY,
|
||||
records_model_with_two_rows,
|
||||
|app| {
|
||||
app.cursor_mut().selected = (1, 0);
|
||||
commit_in(app, AppMode::records_editing(), "edit", "West")
|
||||
},
|
||||
),
|
||||
(
|
||||
"an export with no path",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| commit_in(app, AppMode::export_prompt(), "export", ""),
|
||||
"not exported",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| commit_in(app, AppMode::export_prompt(), "export", "out.csv"),
|
||||
),
|
||||
(
|
||||
"view-forward with nowhere to go",
|
||||
|| with_a_second_view(two_col_model()),
|
||||
|app| run_line(app, "view-forward"),
|
||||
"No forward view",
|
||||
|| with_a_second_view(two_col_model()),
|
||||
|app| {
|
||||
switch_to_second(app);
|
||||
run_line(app, "view-back");
|
||||
run_line(app, "view-forward")
|
||||
},
|
||||
),
|
||||
(
|
||||
"view-back with nowhere to go",
|
||||
|| with_a_second_view(two_col_model()),
|
||||
|app| run_line(app, "view-back"),
|
||||
"No previous view",
|
||||
|| with_a_second_view(two_col_model()),
|
||||
|app| {
|
||||
switch_to_second(app);
|
||||
run_line(app, "view-back")
|
||||
},
|
||||
),
|
||||
(
|
||||
"add-record-row outside records mode",
|
||||
two_col_model,
|
||||
|app| run_line(app, "add-record-row"),
|
||||
"only works in records mode",
|
||||
records_model_with_two_rows,
|
||||
|app| run_line(app, "add-record-row"),
|
||||
),
|
||||
(
|
||||
"add-record-row onto a row that is already there",
|
||||
records_model_with_two_rows,
|
||||
|app| {
|
||||
records_paged_to(app, "South");
|
||||
run_line(app, "add-record-row")
|
||||
},
|
||||
"already here",
|
||||
records_model_with_two_rows,
|
||||
|app| {
|
||||
records_paged_to(app, "West");
|
||||
run_line(app, "add-record-row")
|
||||
},
|
||||
),
|
||||
(
|
||||
"edit-or-drill on an aggregate grid with no cell",
|
||||
|| aggregated_grid(false),
|
||||
|app| run_line(app, "edit-or-drill editing"),
|
||||
"No cell at cursor",
|
||||
|| aggregated_grid(true),
|
||||
|app| run_line(app, "edit-or-drill editing"),
|
||||
),
|
||||
(
|
||||
"edit-or-drill on a cell a formula owns",
|
||||
a_formula_column,
|
||||
|app| {
|
||||
app.cursor_mut().selected = (0, 1);
|
||||
run_line(app, "edit-or-drill editing")
|
||||
},
|
||||
"computed by a formula",
|
||||
a_formula_column,
|
||||
|app| {
|
||||
app.cursor_mut().selected = (0, 0);
|
||||
run_line(app, "edit-or-drill editing")
|
||||
},
|
||||
),
|
||||
(
|
||||
"a search with no matches",
|
||||
|| with_a_cell(two_col_model()),
|
||||
|app| {
|
||||
app.view_state.search_query = "zzz".into();
|
||||
run_line(app, "search-navigate forward")
|
||||
},
|
||||
"No matches",
|
||||
|| with_a_cell(two_col_model()),
|
||||
|app| {
|
||||
app.view_state.search_query = "42".into();
|
||||
run_line(app, "search-navigate forward")
|
||||
},
|
||||
),
|
||||
(
|
||||
"a : line that does not parse",
|
||||
two_col_model,
|
||||
|app| commit_in(app, AppMode::command_mode(), "command", "no-such-command"),
|
||||
":no-such-command",
|
||||
two_col_model,
|
||||
|app| commit_in(app, AppMode::command_mode(), "command", "add-category Z"),
|
||||
),
|
||||
];
|
||||
for (what, fail_setup, fails, says, ok_setup, succeeds) in cases {
|
||||
let mut app = fail_setup();
|
||||
let refused = fails(&mut app);
|
||||
assert!(
|
||||
refused && app.view_state.status_msg.contains(says),
|
||||
"{what}: expected a refusal saying {says:?}, got refused={refused} {:?}",
|
||||
app.view_state.status_msg
|
||||
);
|
||||
let mut app = ok_setup();
|
||||
assert!(
|
||||
!succeeds(&mut app),
|
||||
"{what}: its succeeding twin was refused: {:?}",
|
||||
app.view_state.status_msg
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// A prefix is consumed whether or not the next key matches, so a stray key
|
||||
/// after `g` does nothing rather than acting as a fresh Normal binding.
|
||||
#[test]
|
||||
|
||||
@@ -45,8 +45,8 @@ pub enum ViewTarget {
|
||||
Named(String),
|
||||
}
|
||||
|
||||
/// Refuse AND stop the batch, for a failed document swap: what follows it in
|
||||
/// a `:` line assumes it happened, so `load x . q` must not quit (cz8d).
|
||||
/// Refuse AND stop the batch, where what follows assumes the change happened:
|
||||
/// `load x . q` must not quit, a bad formula must not close its editor (cz8d).
|
||||
fn refuse_and_stop(app: &mut App, why: impl Into<String>) {
|
||||
app.refuse(why);
|
||||
app.abort_effects = true;
|
||||
@@ -219,9 +219,9 @@ impl Effect for AddFormula {
|
||||
}
|
||||
app.model_state.workbook.model.add_formula(formula);
|
||||
}
|
||||
Err(e) => {
|
||||
app.refuse(format!("Formula error: {e}"));
|
||||
}
|
||||
// Stop too: what follows it on the key path closes the editor and
|
||||
// clears the buffer, which would throw the user's text away (v4hg).
|
||||
Err(e) => refuse_and_stop(app, format!("Formula error: {e}")),
|
||||
}
|
||||
}
|
||||
fn kind(&self) -> EffectKind {
|
||||
|
||||
@@ -266,6 +266,27 @@ impl Host {
|
||||
}
|
||||
}
|
||||
|
||||
/// One file per way `ImportJsonHeadless` can fail, plus one that imports; the
|
||||
/// `absent.*` paths are left unwritten. Shared by the frame and key-path tests.
|
||||
#[cfg(test)]
|
||||
pub(crate) fn import_fixture_storage() -> crate::storage::MemoryFs {
|
||||
let storage = crate::storage::MemoryFs::new();
|
||||
let files: [(&str, &[u8]); 5] = [
|
||||
// Every field numeric, so no category is accepted and the build fails.
|
||||
("measures.json", br#"[{"x": 1}, {"x": 2}]"#),
|
||||
("bad.json", b"{not json"),
|
||||
("nested.json", br#"{"data": [{"Region": "North"}]}"#),
|
||||
// A record that is not UTF-8 is a CSV read error, not a warning.
|
||||
("bad.csv", b"Region,Revenue\n\xff\xfe,1\n"),
|
||||
("good.json", br#"[{"Region": "North", "Revenue": 1}]"#),
|
||||
];
|
||||
for (name, bytes) in files {
|
||||
crate::storage::Storage::write(&storage, std::path::Path::new(name), bytes)
|
||||
.expect("fixture writes");
|
||||
}
|
||||
storage
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::path::{Path, PathBuf};
|
||||
@@ -1541,7 +1562,8 @@ mod tests {
|
||||
.find(|&c| app.layout.folds_a_group(1, c))
|
||||
.expect("setup: a folded measure column");
|
||||
assert!(fold < units, "setup: the fold comes first on Feb's row");
|
||||
let id = crate::ui::project::cell_id(&app, &app.layout, 1, units);
|
||||
let id =
|
||||
crate::ui::project::cell_id(&app.model_state.workbook.model, &app.layout, 1, units);
|
||||
let projected = matches!(id, Some(improvise_protocol::CellId::Record(_)));
|
||||
assert!(projected, "setup: Feb x Units projects as {id:?}");
|
||||
let mut host = Host::new(app);
|
||||
@@ -1569,7 +1591,12 @@ mod tests {
|
||||
.flat_map(|r| (0..app.layout.col_count()).map(move |c| (r, c)))
|
||||
.find(|&(r, c)| app.layout.cell_key(r, c).as_ref() == Some(&cell("South")))
|
||||
.expect("setup: South's cell is drawn");
|
||||
let id = crate::ui::project::cell_id(&app, &app.layout, south.0, south.1);
|
||||
let id = crate::ui::project::cell_id(
|
||||
&app.model_state.workbook.model,
|
||||
&app.layout,
|
||||
south.0,
|
||||
south.1,
|
||||
);
|
||||
let aggregate = matches!(id, Some(improvise_protocol::CellId::Aggregate(_)));
|
||||
assert!(aggregate, "setup: the host's view aggregates South: {id:?}");
|
||||
let mut host = Host::new(app);
|
||||
@@ -2194,19 +2221,7 @@ mod tests {
|
||||
/// A host whose storage holds one file per way `ImportJsonHeadless` can
|
||||
/// fail, plus nothing at the two paths that must fail to read.
|
||||
fn host_with_import_fixtures() -> Host {
|
||||
let storage = MemoryFs::new();
|
||||
let files: [(&str, &[u8]); 4] = [
|
||||
// Every field numeric, so no category is accepted and the build fails.
|
||||
("measures.json", br#"[{"x": 1}, {"x": 2}]"#),
|
||||
("bad.json", b"{not json"),
|
||||
("nested.json", br#"{"data": [{"Region": "North"}]}"#),
|
||||
// A record that is not UTF-8 is a CSV read error, not a warning.
|
||||
("bad.csv", b"Region,Revenue\n\xff\xfe,1\n"),
|
||||
];
|
||||
for (name, bytes) in files {
|
||||
crate::storage::Storage::write(&storage, Path::new(name), bytes)
|
||||
.expect("fixture writes");
|
||||
}
|
||||
let storage = super::import_fixture_storage();
|
||||
// A second view, so deleting an absent one is not refused as "the last".
|
||||
let mut wb = workbook();
|
||||
wb.create_view("Scratch");
|
||||
|
||||
@@ -280,7 +280,7 @@ fn cell_display(
|
||||
text,
|
||||
numeric: matches!(value, Some(CellValue::Number(_))),
|
||||
error: matches!(value, Some(CellValue::Error(_))),
|
||||
id: cell_id(app, layout, row, col),
|
||||
id: cell_id(&app.model_state.workbook.model, layout, row, col),
|
||||
value: value.clone(),
|
||||
};
|
||||
}
|
||||
@@ -289,7 +289,7 @@ fn cell_display(
|
||||
text: plan.at(key.as_ref()).value(value.as_ref()),
|
||||
numeric: matches!(value, Some(CellValue::Number(_))),
|
||||
error: matches!(value, Some(CellValue::Error(_))),
|
||||
id: cell_id(app, layout, row, col),
|
||||
id: cell_id(&app.model_state.workbook.model, layout, row, col),
|
||||
// The *unformatted* value the TUI seeds its edit buffer from. Projecting it
|
||||
// beats a client un-formatting `text` (gotcha 47).
|
||||
edit_text: key.map(|key| app.display_value_in(layout, &key)),
|
||||
@@ -299,8 +299,12 @@ fn cell_display(
|
||||
|
||||
/// What the cell at `(row, col)` *is*, as the surrogate the client echoes back. The
|
||||
/// fork is `EditOrDrill`'s, asked the same way — see `context/code/command.md`.
|
||||
pub(crate) fn cell_id(app: &App, layout: &GridLayout, row: usize, col: usize) -> Option<CellId> {
|
||||
let model = &app.model_state.workbook.model;
|
||||
pub(crate) fn cell_id(
|
||||
model: &Model,
|
||||
layout: &GridLayout,
|
||||
row: usize,
|
||||
col: usize,
|
||||
) -> Option<CellId> {
|
||||
if layout.is_records_mode() {
|
||||
// A measure can be BOTH an explicit `_Measure` item and a formula target,
|
||||
// so the document is asked about the record's real key (improvise-ir7l).
|
||||
@@ -1602,6 +1606,32 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-v8ab: a fresh client starts from the host's own startup slice.
|
||||
/// See `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn a_fresh_sessions_slice_is_the_hosts_startup_slice() {
|
||||
for (names, records) in [
|
||||
(&["Default", "Second"][..], false),
|
||||
(&["Main", "Second"][..], false),
|
||||
(&["Main", "Second"][..], true),
|
||||
] {
|
||||
let mut wb = opening_on(names, "Second");
|
||||
if records {
|
||||
// `Second` opens on a records grid (improvise-y5eo).
|
||||
wb.active_view_mut().set_axis("_Index", Axis::Row);
|
||||
wb.active_view_mut().set_axis("_Dim", Axis::Column);
|
||||
}
|
||||
let app = App::new(wb, None);
|
||||
assert_eq!(app.view_state.active_view, "Second", "setup: {names:?}");
|
||||
assert_eq!(app.layout.is_records_mode(), records, "setup: {names:?}");
|
||||
let (_, snapshot) = Sessions::default().connect(&app, 10, 10);
|
||||
assert_eq!(
|
||||
snapshot.view, app.view_state,
|
||||
"{names:?}, records {records}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-lxg0: a document swap reseeds through the same door.
|
||||
#[test]
|
||||
fn a_rebased_session_has_no_history_to_step_back_through() {
|
||||
|
||||
@@ -736,7 +736,12 @@ impl GridLayout {
|
||||
/// Whether the cell at `(row, col)` folds a collapsed group: an aggregate
|
||||
/// with no record under it. The one home of that answer (§14).
|
||||
pub fn folds_a_group(&self, row: usize, col: usize) -> bool {
|
||||
folds(&self.row_items, row) || folds(&self.col_items, col)
|
||||
self.row_folds_a_group(row) || folds(&self.col_items, col)
|
||||
}
|
||||
|
||||
/// The row half of [`Self::folds_a_group`], for a command about the row alone.
|
||||
pub fn row_folds_a_group(&self, row: usize) -> bool {
|
||||
folds(&self.row_items, row)
|
||||
}
|
||||
|
||||
/// The first data row of `cat`/`group`'s run under `prefix` — where a toggle
|
||||
|
||||
@@ -620,7 +620,8 @@ enum GridMove {
|
||||
}
|
||||
|
||||
/// Rows one `page-scroll` step moves: three quarters of the window, at least one.
|
||||
fn page_rows(window_rows: usize) -> i32 {
|
||||
/// The one home: the TUI's `Move` calls it too (improvise-5xbq).
|
||||
pub fn page_rows(window_rows: usize) -> i32 {
|
||||
(window_rows as i32 * 3 / 4).max(1)
|
||||
}
|
||||
|
||||
|
||||
@@ -266,6 +266,70 @@ fn a_locally_resolved_cursor_move_lands_where_the_host_would_put_it() {
|
||||
);
|
||||
}
|
||||
|
||||
/// Twenty rows by three columns, the host's window eight rows by three: not
|
||||
/// square, so a rule keyed off the wrong axis lands somewhere else.
|
||||
fn paging_host() -> App {
|
||||
let mut wb = Workbook::new("Test");
|
||||
wb.add_category("Region").unwrap();
|
||||
for i in 0..20 {
|
||||
wb.add_item("Region", format!("R{i:02}")).unwrap();
|
||||
}
|
||||
for m in ["A", "B", "C"] {
|
||||
wb.add_item("_Measure", m).unwrap();
|
||||
}
|
||||
wb.active_view_mut().set_axis("Region", Axis::Row);
|
||||
wb.active_view_mut().set_axis("_Measure", Axis::Column);
|
||||
let mut app = App::new(wb, None);
|
||||
app.term_height = 8 + 8;
|
||||
app.rebuild_layout();
|
||||
app
|
||||
}
|
||||
|
||||
/// Page and half-page scrolls land in the same place on both ends, and in the
|
||||
/// place the TUI's rule names. Why the fixture and the pinned rows: the evidence file.
|
||||
#[test]
|
||||
fn a_locally_resolved_page_scroll_lands_where_the_host_would_put_it() {
|
||||
let window = (8, 3);
|
||||
let mut app = paging_host();
|
||||
let mut client = client_of(&app, window);
|
||||
let key = |code, mods| KeyEvent::new(code, mods);
|
||||
let none = KeyModifiers::NONE;
|
||||
let ctrl = KeyModifiers::CONTROL;
|
||||
let presses = [
|
||||
(key(KeyCode::Char('j'), none), 1),
|
||||
(key(KeyCode::Char('j'), none), 2),
|
||||
(key(KeyCode::PageDown, none), 8),
|
||||
(key(KeyCode::PageDown, none), 14),
|
||||
(key(KeyCode::PageUp, none), 8),
|
||||
(key(KeyCode::Char('d'), ctrl), 13),
|
||||
(key(KeyCode::PageDown, none), 19),
|
||||
(key(KeyCode::Char('u'), ctrl), 14),
|
||||
(key(KeyCode::PageUp, none), 8),
|
||||
(key(KeyCode::PageUp, none), 2),
|
||||
];
|
||||
for (press, (event, want_row)) in presses.into_iter().enumerate() {
|
||||
match resolve(&client, event) {
|
||||
KeyOutcome::Local(cmds) => {
|
||||
for cmd in &cmds {
|
||||
client.fold(cmd);
|
||||
}
|
||||
}
|
||||
other => panic!("press {press}: the client must answer {event:?}, got {other:?}"),
|
||||
}
|
||||
reduce_key(&mut app, event);
|
||||
assert_eq!(
|
||||
client.view.cursor(),
|
||||
app.view_state.cursor(),
|
||||
"press {press} ({event:?}): client and host disagree"
|
||||
);
|
||||
assert_eq!(
|
||||
client.view.cursor().selected,
|
||||
(want_row, 0),
|
||||
"press {press} ({event:?}): not the row the TUI's page rule names"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The same for `G`, the key `GridExtent` exists for: it is *defined* as "the
|
||||
/// last row", so a window-only client would have nothing to jump to.
|
||||
#[test]
|
||||
@@ -1831,6 +1895,185 @@ fn yy_over_an_aggregate_copies_the_sum_the_host_copies() {
|
||||
);
|
||||
}
|
||||
|
||||
// The cursor's cell as a write target (improvise-lx7w, improvise-54g1, improvise-gw18).
|
||||
|
||||
/// improvise-lx7w: `x` on a records measure clears the real record on both ends.
|
||||
/// Evidence: `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn x_in_records_mode_clears_the_record_the_browser_clears() {
|
||||
let r = KeyEvent::new(KeyCode::Char('R'), KeyModifiers::NONE);
|
||||
let two_records = || {
|
||||
let mut app = furnished_host();
|
||||
let wb = &mut app.model_state.workbook;
|
||||
wb.add_item("_Measure", "Cost").unwrap();
|
||||
// Two measures, so clearing one leaves South's row where the cursor is.
|
||||
for (measure, n) in [("Revenue", 900.0), ("Cost", 40.0)] {
|
||||
let key = north_revenue()
|
||||
.with("Region", "South")
|
||||
.with("_Measure", measure);
|
||||
wb.model
|
||||
.set_cell(key, improvise_protocol::CellValue::Number(n));
|
||||
}
|
||||
app.rebuild_layout();
|
||||
reduce_key(&mut app, r);
|
||||
app
|
||||
};
|
||||
let (mut app, mut twin) = (two_records(), two_records());
|
||||
let revenue = (0..app.layout.col_count())
|
||||
.find(|&c| app.layout.col_label(c) == "Revenue")
|
||||
.expect("a Revenue column");
|
||||
assert_eq!(
|
||||
app.layout.record_row(1).unwrap().coords.get("Region"),
|
||||
Some("South"),
|
||||
"setup: row 1 is South"
|
||||
);
|
||||
for host in [&mut app, &mut twin] {
|
||||
host.cursor_mut().selected = (1, revenue);
|
||||
}
|
||||
let mut client = records_client(&app);
|
||||
client.view.cursor_mut().selected = (1, revenue);
|
||||
|
||||
both_ends(&mut app, &mut twin, &mut client, "x");
|
||||
let model = &app.model_state.workbook.model;
|
||||
assert_eq!(
|
||||
model.get_cell(&north_revenue().with("Region", "South")),
|
||||
None
|
||||
);
|
||||
assert!(
|
||||
model.get_cell(&north_revenue()).is_some(),
|
||||
"North is untouched"
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-q9eg: `yy` then `p` onto a record that lacks a later category,
|
||||
/// on both ends. Evidence: `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn p_onto_a_sparse_record_writes_what_the_browser_writes() {
|
||||
let r = KeyEvent::new(KeyCode::Char('R'), KeyModifiers::NONE);
|
||||
let sparse = || {
|
||||
let mut app = furnished_host();
|
||||
let south = north_revenue().with("Region", "South");
|
||||
app.model_state
|
||||
.workbook
|
||||
.model
|
||||
.set_cell(south, improvise_protocol::CellValue::Number(900.0));
|
||||
app.rebuild_layout();
|
||||
reduce_key(&mut app, r);
|
||||
// As `+cat` does: every existing record now lacks `Season`.
|
||||
app.model_state.workbook.add_category("Season").unwrap();
|
||||
app.rebuild_layout();
|
||||
app
|
||||
};
|
||||
let (mut app, mut twin) = (sparse(), sparse());
|
||||
assert!(
|
||||
app.layout.none_cats.iter().any(|c| c == "Season"),
|
||||
"setup: the new category is aggregated away"
|
||||
);
|
||||
let revenue = (0..app.layout.col_count())
|
||||
.find(|&c| app.layout.col_label(c) == "Revenue")
|
||||
.expect("a Revenue column");
|
||||
for host in [&mut app, &mut twin] {
|
||||
host.cursor_mut().selected = (0, revenue);
|
||||
}
|
||||
let mut client = records_client(&app);
|
||||
client.view.cursor_mut().selected = (0, revenue);
|
||||
both_ends(&mut app, &mut twin, &mut client, "y");
|
||||
both_ends(&mut app, &mut twin, &mut client, "y");
|
||||
|
||||
for host in [&mut app, &mut twin] {
|
||||
host.cursor_mut().selected = (1, revenue);
|
||||
}
|
||||
client.view.cursor_mut().selected = (1, revenue);
|
||||
both_ends(&mut app, &mut twin, &mut client, "p");
|
||||
assert_eq!(
|
||||
app.model_state
|
||||
.workbook
|
||||
.model
|
||||
.get_cell(&north_revenue().with("Region", "South")),
|
||||
Some(&improvise_protocol::CellValue::Number(1200.0)),
|
||||
"the paste must land, or both ends agreeing is both declining"
|
||||
);
|
||||
}
|
||||
|
||||
/// `key` at `at` on a host and on a client seeded from it: neither writes.
|
||||
fn declines_on_both_ends(mut app: App, at: (usize, usize), key: char) {
|
||||
let event = KeyEvent::new(KeyCode::Char(key), KeyModifiers::NONE);
|
||||
app.view_state.yanked = Some(improvise_protocol::CellValue::Number(99.0));
|
||||
app.cursor_mut().selected = at;
|
||||
let client = client_ready_to_edit(&app, (6, 6));
|
||||
assert!(
|
||||
!composes_a_write(&client, event),
|
||||
"the browser composed a write"
|
||||
);
|
||||
let before = document_text(&app);
|
||||
reduce_key(&mut app, event);
|
||||
assert_eq!(
|
||||
document_text(&app),
|
||||
before,
|
||||
"the TUI wrote where the browser declines"
|
||||
);
|
||||
assert!(
|
||||
!app.view_state.status_msg.is_empty(),
|
||||
"the TUI declined silently"
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-54g1: `p` on a formula-owned cell. The control is the stored cell
|
||||
/// beside it. Evidence: `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn p_onto_a_formula_owned_cell_declines_on_both_ends() {
|
||||
let app = yank_host();
|
||||
assert_eq!(
|
||||
app.layout.col_label(3),
|
||||
"Profit",
|
||||
"setup: column 3 is the formula"
|
||||
);
|
||||
declines_on_both_ends(app, (1, 3), 'p');
|
||||
|
||||
let mut app = yank_host();
|
||||
app.view_state.yanked = Some(improvise_protocol::CellValue::Number(99.0));
|
||||
app.cursor_mut().selected = (1, 2);
|
||||
let client = client_ready_to_edit(&app, (6, 6));
|
||||
let p = KeyEvent::new(KeyCode::Char('p'), KeyModifiers::NONE);
|
||||
assert!(
|
||||
composes_a_write(&client, p),
|
||||
"the control: Cost is settable"
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-gw18/l4mp: `p` and `x` on a collapsed group's fold, off the origin.
|
||||
#[test]
|
||||
fn a_write_onto_a_fold_declines_on_both_ends() {
|
||||
let fold_host = || {
|
||||
let mut wb = Workbook::new("Test");
|
||||
wb.add_category("Month").unwrap();
|
||||
for (month, quarter) in [("Jan", "Q1"), ("Feb", "Q1"), ("Apr", "Q2"), ("May", "Q2")] {
|
||||
wb.add_item_in_group("Month", month, quarter).unwrap();
|
||||
}
|
||||
wb.add_item("_Measure", "Revenue").unwrap();
|
||||
wb.add_item("_Measure", "Cost").unwrap();
|
||||
for month in ["Jan", "Feb", "Apr", "May"] {
|
||||
for measure in ["Revenue", "Cost"] {
|
||||
let key = improvise_command::model::cell::CellKey::new(vec![
|
||||
("Month".to_string(), month.to_string()),
|
||||
("_Measure".to_string(), measure.to_string()),
|
||||
]);
|
||||
wb.model
|
||||
.set_cell(key, improvise_protocol::CellValue::Number(3.0));
|
||||
}
|
||||
}
|
||||
wb.active_view_mut().set_axis("Month", Axis::Row);
|
||||
wb.active_view_mut().set_axis("_Measure", Axis::Column);
|
||||
wb.active_view_mut().toggle_group_collapse("Month", "Q2");
|
||||
let mut app = App::new(wb, None);
|
||||
app.rebuild_layout();
|
||||
assert_eq!(app.layout.row_label(2), "Q2", "setup: the fold is row 2");
|
||||
app
|
||||
};
|
||||
declines_on_both_ends(fold_host(), (2, 1), 'p');
|
||||
declines_on_both_ends(fold_host(), (2, 1), 'x');
|
||||
}
|
||||
|
||||
/// Two regions by two measures, `value` at (South, Revenue): row 1, column 1,
|
||||
/// so "the cell under the cursor" is not "the first cell".
|
||||
fn precise_host(value: improvise_protocol::CellValue) -> App {
|
||||
|
||||
+5
-1
@@ -22,7 +22,11 @@ serve="$root/harness/serve"
|
||||
# re-resolves the lock itself before any `-- --locked` reaches cargo build.
|
||||
# Fix a failure with `cargo update -w` in harness/opfs-contract, and commit it.
|
||||
cargo metadata --locked --format-version 1 \
|
||||
--manifest-path "$root/harness/opfs-contract/Cargo.toml" >/dev/null
|
||||
--manifest-path "$root/harness/opfs-contract/Cargo.toml" >/dev/null || {
|
||||
echo "harness/build.sh: harness/opfs-contract/Cargo.lock is stale. Do not remove --locked." >&2
|
||||
echo " Run: cargo update -w --manifest-path harness/opfs-contract/Cargo.toml, and commit the lock." >&2
|
||||
exit 1
|
||||
}
|
||||
|
||||
"$root/web/build.sh" >/dev/null
|
||||
"$root/web/assemble.sh" "$serve" harness >/dev/null
|
||||
|
||||
@@ -132,4 +132,30 @@ test.describe("the help overlay draws the shared content", () => {
|
||||
await expect(rows_.last().locator("td.selected")).toHaveCount(1);
|
||||
await expect(page.locator("#grid tbody td.selected")).toHaveCount(1);
|
||||
});
|
||||
|
||||
// improvise-1c5m. Below the width at which the app bar wraps, `#help` covers
|
||||
// `#hint`, and Welcome itself says nothing about paging. The contract: the
|
||||
// mode's own hint is UNCOVERED somewhere while help is open.
|
||||
test("a narrow window still shows how to page through help", async ({ page }) => {
|
||||
await page.setViewportSize({ width: 600, height: 800 });
|
||||
await openHelp(page);
|
||||
|
||||
// The string is the page's own `#hint`, i.e. `hint_line(Help)`; this spec
|
||||
// spells no keys. Asserting it names paging keeps an empty hint out.
|
||||
const hint = (await page.locator("#hint").textContent()).trim();
|
||||
expect(hint).toContain("next-page");
|
||||
|
||||
// An element showing exactly that string, topmost at its own centre: the
|
||||
// overlay covering `#hint` is precisely what `elementFromPoint` sees.
|
||||
const uncovered = await page.evaluate((text) =>
|
||||
[...document.querySelectorAll("body *")].some((node) => {
|
||||
if (node.children.length > 0 || node.textContent.trim() !== text) return false;
|
||||
const r = node.getBoundingClientRect();
|
||||
if (r.width === 0 || r.height === 0) return false;
|
||||
const top = document.elementFromPoint(r.left + r.width / 2, r.top + r.height / 2);
|
||||
return top !== null && (top === node || node.contains(top));
|
||||
}),
|
||||
hint);
|
||||
expect(uncovered, "the paging hint is under the help overlay").toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -217,6 +217,39 @@ got=$?
|
||||
[ "$got" = 0 ] && ok "a second passing before the first re-poll does not skip it" \
|
||||
|| bad "a second passing before the first re-poll does not skip it" "exit $got, wanted 0"
|
||||
|
||||
# Every fixture above resolves by poll 2, so a loop that stops after one
|
||||
# re-poll passed them all (improvise-7epn). This one resolves on poll 3.
|
||||
d3="$work/arrives-third"
|
||||
for n in 1 2; do
|
||||
mk "$d3" "$n.pr.json" <<EOF
|
||||
{"headRefOid":"$HEAD","reviews":[]}
|
||||
EOF
|
||||
mk "$d3" "$n.inline.json" <<'EOF'
|
||||
[]
|
||||
EOF
|
||||
mk "$d3" "$n.issue.json" <<'EOF'
|
||||
[]
|
||||
EOF
|
||||
done
|
||||
cp -f "$d/last.pr.json" "$d/last.inline.json" "$d/last.issue.json" "$d3/"
|
||||
rm -f "$d3/.poll" "$work/ticks"
|
||||
CLOCK_TICKS="$work/ticks" PATH="$clock:$PATH" PR_REVIEWS_FIXTURE_DIR="$d3" \
|
||||
"$REVIEWS" 1 --wait 10 --interval 1 > "$work/out" 2>&1
|
||||
got=$?
|
||||
polls=$(cat "$d3/.poll" 2>/dev/null || echo 0)
|
||||
[ "$got" = 0 ] && [ "$polls" = 3 ] && ok "a review arriving on the third poll is seen" \
|
||||
|| bad "a review arriving on the third poll is seen" "exit $got after $polls polls, wanted 0 after 3"
|
||||
|
||||
# The timeout path: nothing ever arrives, so --wait 5 on a clock that ticks
|
||||
# once per read must keep polling until the deadline, then report exit 1.
|
||||
rm -f "$work/empty/.poll" "$work/ticks"
|
||||
CLOCK_TICKS="$work/ticks" PATH="$clock:$PATH" PR_REVIEWS_FIXTURE_DIR="$work/empty" \
|
||||
"$REVIEWS" 1 --wait 5 --interval 1 > "$work/out" 2>&1
|
||||
got=$?
|
||||
polls=$(cat "$work/empty/.poll" 2>/dev/null || echo 0)
|
||||
[ "$got" = 1 ] && [ "$polls" -ge 5 ] && ok "--wait keeps polling until its deadline" \
|
||||
|| bad "--wait keeps polling until its deadline" "exit $got after $polls polls, wanted 1 after 5 or more"
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# `gh api --paginate` emits one array PER PAGE, so a thread past the page size
|
||||
# arrives as several concatenated documents. Passing that straight to
|
||||
|
||||
+38
-54
@@ -6,6 +6,7 @@ use ratatui::{
|
||||
};
|
||||
|
||||
use crate::protocol::help::{self, HelpLine};
|
||||
use crate::ui::text::truncate;
|
||||
|
||||
/// Number of help pages available (defined alongside the help-page state).
|
||||
pub use crate::ui::app::HELP_PAGE_COUNT;
|
||||
@@ -228,66 +229,49 @@ fn render_tab_bar(buf: &mut Buffer, x: u16, y: u16, width: u16, active_page: usi
|
||||
}
|
||||
}
|
||||
|
||||
/// Renders the footer with page navigation hints.
|
||||
/// The page indicator, then Help mode's own hint off the keymap: a footer that
|
||||
/// spelled its keys was a second table nothing checked (improvise-17ak).
|
||||
fn render_footer(buf: &mut Buffer, x: u16, y: u16, width: u16, page: usize) {
|
||||
let dim = Style::default().fg(Color::DarkGray);
|
||||
let key_style = Style::default().fg(Color::Cyan);
|
||||
let hint = crate::protocol::hint_line(&crate::ui::app::AppMode::Help);
|
||||
let line = format!(" page {} of {} {hint}", page + 1, HELP_PAGE_COUNT);
|
||||
buf.set_string(x, y, truncate(&line, width as usize), dim);
|
||||
}
|
||||
|
||||
let page_indicator = format!(" page {} of {} ", page + 1, HELP_PAGE_COUNT);
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::ui::app::AppMode;
|
||||
|
||||
let nav_parts: Vec<(&str, Style)> = if page == 0 && HELP_PAGE_COUNT > 1 {
|
||||
vec![
|
||||
(" ", dim),
|
||||
("l", key_style),
|
||||
("/", dim),
|
||||
("Tab", key_style),
|
||||
(": next", dim),
|
||||
]
|
||||
} else if page >= HELP_PAGE_COUNT - 1 {
|
||||
vec![
|
||||
(" ", dim),
|
||||
("h", key_style),
|
||||
("/", dim),
|
||||
("S-Tab", key_style),
|
||||
(": prev", dim),
|
||||
]
|
||||
} else {
|
||||
vec![
|
||||
(" ", dim),
|
||||
("h", key_style),
|
||||
(": prev ", dim),
|
||||
("l", key_style),
|
||||
(": next", dim),
|
||||
]
|
||||
};
|
||||
|
||||
let close_parts: Vec<(&str, Style)> = vec![
|
||||
(" ", dim),
|
||||
("q", key_style),
|
||||
("/", dim),
|
||||
("Esc", key_style),
|
||||
(": close", dim),
|
||||
];
|
||||
|
||||
// Render page indicator on the left
|
||||
buf.set_string(x, y, &page_indicator, dim);
|
||||
|
||||
// Render navigation hints after the page indicator
|
||||
let mut col = x + page_indicator.len() as u16;
|
||||
for (text, style) in &nav_parts {
|
||||
if col >= x + width {
|
||||
break;
|
||||
}
|
||||
buf.set_string(col, y, text, *style);
|
||||
col += text.len() as u16;
|
||||
fn rows(buf: &Buffer) -> Vec<String> {
|
||||
let w = buf.area().width as usize;
|
||||
buf.content()
|
||||
.chunks(w)
|
||||
.map(|row| row.iter().map(|c| c.symbol()).collect::<String>())
|
||||
.collect()
|
||||
}
|
||||
|
||||
// Render close hint
|
||||
for (text, style) in &close_parts {
|
||||
if col >= x + width {
|
||||
break;
|
||||
/// The footer's keys are `hint_line(Help)`'s, not a table of its own
|
||||
/// (improvise-17ak). Why the width and every page: `context/test-evidence/tui.md`.
|
||||
#[test]
|
||||
fn the_help_footer_names_the_keys_the_keymap_binds() {
|
||||
let hint = crate::protocol::hint_line(&AppMode::Help);
|
||||
assert!(
|
||||
hint.contains("next-page"),
|
||||
"the derived hint pages: {hint:?}"
|
||||
);
|
||||
let area = Rect::new(0, 0, 140, 30);
|
||||
for page in 0..HELP_PAGE_COUNT {
|
||||
let mut buf = Buffer::empty(area);
|
||||
HelpWidget::new(page).render(area, &mut buf);
|
||||
let rows = rows(&buf);
|
||||
let marker = format!("page {} of", page + 1);
|
||||
let footer = rows.iter().find(|row| row.contains(&marker));
|
||||
let footer = footer.unwrap_or_else(|| panic!("page {page}: no footer row"));
|
||||
assert!(
|
||||
footer.contains(&hint),
|
||||
"page {page}: the footer is not the keymap's hint {hint:?}: {footer:?}"
|
||||
);
|
||||
}
|
||||
buf.set_string(col, y, text, *style);
|
||||
col += text.len() as u16;
|
||||
}
|
||||
}
|
||||
|
||||
+7
-1
@@ -348,7 +348,13 @@ function paintHelp(model) {
|
||||
}
|
||||
}
|
||||
|
||||
el.help.replaceChildren(tabs, body);
|
||||
// The mode's own hint again, as the TUI popup's footer: once the app bar
|
||||
// wraps, the overlay covers `#hint` (improvise-1c5m).
|
||||
const footer = document.createElement("p");
|
||||
footer.className = "help-footer";
|
||||
footer.textContent = model.hint;
|
||||
|
||||
el.help.replaceChildren(tabs, body, footer);
|
||||
}
|
||||
|
||||
/** The page bar: which item each fixed category is fixed to. Without it a
|
||||
|
||||
@@ -736,6 +736,17 @@ td.total {
|
||||
font-weight: 700;
|
||||
}
|
||||
|
||||
/* Sticky, so the paging keys stay in view however long the page scrolls. */
|
||||
.help-footer {
|
||||
position: sticky;
|
||||
bottom: -1.25rem;
|
||||
margin: 0.75rem 0 0;
|
||||
padding: 0.5rem 0;
|
||||
border-top: 1px solid var(--line);
|
||||
background: var(--bg);
|
||||
color: var(--dim);
|
||||
}
|
||||
|
||||
.help-body {
|
||||
display: grid;
|
||||
grid-template-columns: 16rem 1fr;
|
||||
|
||||
Reference in New Issue
Block a user