Merge collapse-fu/h6z1 (12f206fb) into fold-memo/pntu-0jw5
Carries #258 onto #247's re-merged head, which now has #239's spanned_values/spanned_positions and the &Expansion fold signatures. Code auto-merged: FoldMemo::new(agg, covers) still owns both, and fold_stored reads through spanned_values. The only conflicts were the append-at-EOF evidence files (command.md, core.md); both sides kept. 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:
Edward Langley
Generated
+1
@@ -897,6 +897,7 @@ dependencies = [
|
||||
"pest_derive",
|
||||
"pest_meta",
|
||||
"proptest",
|
||||
"serde",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
@@ -105,6 +105,51 @@ fn chain_app(regions: usize, products: usize) -> App {
|
||||
app
|
||||
}
|
||||
|
||||
/// Days in collapsed groups of `per_group` on Row, with Product: every row is a
|
||||
/// fold over a few days, never the whole category (improvise-jq0m).
|
||||
fn grouped_app(days: usize, per_group: usize, products: usize, measures: usize) -> App {
|
||||
let mut wb = Workbook::new("Grouped");
|
||||
wb.add_category("Day").unwrap();
|
||||
wb.add_category("Product").unwrap();
|
||||
for d in 0..days {
|
||||
let group = format!("G{}", d / per_group);
|
||||
wb.add_item_in_group("Day", format!("Day{d}"), group)
|
||||
.unwrap();
|
||||
}
|
||||
for p in 0..products {
|
||||
wb.add_item("Product", format!("Product{p}")).unwrap();
|
||||
}
|
||||
for m in 0..measures {
|
||||
wb.add_item("_Measure", format!("Measure{m}")).unwrap();
|
||||
}
|
||||
{
|
||||
let v = wb.active_view_mut();
|
||||
v.set_axis("Day", Axis::Row);
|
||||
v.set_axis("Product", Axis::Row);
|
||||
v.set_axis("_Measure", Axis::Column);
|
||||
for g in 0..days.div_ceil(per_group) {
|
||||
v.toggle_group_collapse("Day", &format!("G{g}"));
|
||||
}
|
||||
}
|
||||
let mut n = 0.0f64;
|
||||
for d in 0..days {
|
||||
for p in 0..products {
|
||||
for m in 0..measures {
|
||||
n += 1.0;
|
||||
let key = CellKey::new(vec![
|
||||
("Day".to_string(), format!("Day{d}")),
|
||||
("Product".to_string(), format!("Product{p}")),
|
||||
("_Measure".to_string(), format!("Measure{m}")),
|
||||
]);
|
||||
wb.model.set_cell(key, CellValue::Number(n));
|
||||
}
|
||||
}
|
||||
}
|
||||
let mut app = App::new(wb, None);
|
||||
app.rebuild_layout();
|
||||
app
|
||||
}
|
||||
|
||||
fn whole_grid(app: &App) -> Viewport {
|
||||
Viewport {
|
||||
row_offset: 0,
|
||||
@@ -174,9 +219,12 @@ fn report(label: &str, app: App, iters: u32) {
|
||||
true => "UNSTABLE".to_string(),
|
||||
false => format!("{share:.0}%"),
|
||||
};
|
||||
// The TUI pays both on every keypress: widths in `cmd_context`, the grid in draw.
|
||||
let keypress = full + widths;
|
||||
println!(
|
||||
"{label:<22} {rows:>5} x {cols:>3} = {cells:>7} cells whole-grid {full:>11.3?} \
|
||||
windowed {windowed:>11.3?} ratio {ratio:>4.1}x col_widths {widths:>11.3?} ({share} of the frame)"
|
||||
windowed {windowed:>11.3?} ratio {ratio:>4.1}x col_widths {widths:>11.3?} ({share} of the frame) \
|
||||
keypress {keypress:>11.3?}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -184,10 +232,17 @@ fn main() {
|
||||
println!("project::grid_of, one call = one TUI frame's grid cache");
|
||||
println!("a 60fps frame budget is 16.7ms; the TUI redraws on each keypress\n");
|
||||
case("typical", 20, 5, 4, 200);
|
||||
case("1.2k cells", 60, 5, 4, 100);
|
||||
case("2.4k cells", 60, 10, 4, 100);
|
||||
case("4.8k cells", 60, 20, 4, 50);
|
||||
case("large", 60, 20, 8, 50);
|
||||
case("18k cells (gotcha 41)", 150, 20, 6, 20);
|
||||
case("pathological", 300, 30, 10, 5);
|
||||
println!("\nfolded rows and a five-formula chain (improvise-h6z1)\n");
|
||||
report("folded chain ~18k", chain_app(1000, 30), 5);
|
||||
report("folded chain ~90k", chain_app(5000, 30), 3);
|
||||
println!("\ncollapsed small groups on the row axis (improvise-jq0m)\n");
|
||||
report("364 days in weeks", grouped_app(364, 7, 20, 6), 5);
|
||||
report("364 days in pairs", grouped_app(364, 2, 20, 6), 5);
|
||||
report("1000 days in pairs", grouped_app(1000, 2, 10, 4), 3);
|
||||
}
|
||||
|
||||
+69
-31
@@ -996,20 +996,39 @@ folded when the resolver runs — the trap that reads as "the cursor lands on
|
||||
`Plan` and the document switches to `Default`", silent and wrong in the half a
|
||||
user does not watch.
|
||||
|
||||
The second clause of rule 1 is what makes the formula panel look inconsistent
|
||||
and is in fact the rule holding. `ModeKey::ViewPanel` binds `Enter` to
|
||||
`switch-view-at-cursor`; `ModeKey::FormulaPanel` binds `a`/`n`/`o` and
|
||||
`d`/`Delete` and **no `Enter` at all**. So a view-row click switches and a
|
||||
formula-row click only navigates. Opening the editor on a formula click would
|
||||
be an activation *invented for the pointer that the keyboard does not have* —
|
||||
a second derivation of what a formula row means, which is the disease §5 names.
|
||||
**The guard for this must be aimed at the keymap, not at the click**: asserting
|
||||
that the click emits no `FormulaEdit` only restates the author's reading, and a
|
||||
reviewer demonstrated the gap by adding `Enter -> enter-mode formula-edit` to
|
||||
`FormulaPanel` and watching the whole workspace stay green while the click and
|
||||
the keyboard disagreed. `the_formula_panel_has_no_enter_to_mirror_and_the_view_panel_does`
|
||||
asserts the binding's absence and names `click_formula_row` as what must change
|
||||
if it ever appears.
|
||||
The second clause of rule 1 is why a formula-row click opens the editor. Until
|
||||
improvise-km71, `ModeKey::FormulaPanel` bound `a`/`n`/`o` and `d`/`Delete` and
|
||||
**no `Enter` at all**, so a formula-row click only navigated, and opening the
|
||||
editor on a click would have been an activation *invented for the pointer that
|
||||
the keyboard did not have*. km71 bound `Enter` (and `e`) to
|
||||
`edit-formula-at-cursor`, so the rule now yields the editor on the clicked row,
|
||||
and `click_formula_row` ends with `edit_formula_at(index)` exactly as
|
||||
`click_view_row` ends with `switch_view_at(index)`. **The guard is aimed at what
|
||||
`Enter` resolves to, not at the keymap and not at the click**:
|
||||
`a_formula_row_click_ends_with_what_enter_does_on_that_row` resolves `Enter` at
|
||||
cursor 1 and asserts the click on row 1, made with the cursor at 0, *ends with*
|
||||
those commands. The earlier guard asserted only that `Enter` was unbound, which
|
||||
tied the two by a comment; this one fails if either side changes without the
|
||||
other, and it separates "reads the clicked index" from "reads the cursor",
|
||||
because the two point at different rows. The superseded test was
|
||||
`the_formula_panel_has_no_enter_to_mirror_and_the_view_panel_does`.
|
||||
|
||||
Editing needs the client to name *which* formula, and `FormulaCache` used to
|
||||
carry only the raw text, from which neither the target nor its category is
|
||||
recoverable without reimplementing `parse_formula` on the client — the gap
|
||||
`decision(improvise-e24v)` recorded for `delete-formula-at-cursor`. km71 closed
|
||||
it the gotcha 47 way, by **enriching the projection**: `FormulaCache::formulas`
|
||||
is `Vec<FormulaEntry { id: FormulaId, raw }>`. The id is opaque on purpose
|
||||
(`context/code/core.md` §14). `delete-formula-at-cursor` is still declined in
|
||||
the browser; it now needs only its resolver.
|
||||
|
||||
One divergence is pinned rather than closed: a rename. The host's
|
||||
`ReplaceFormula` rewrites the status to say what was renamed and which formulas
|
||||
still name the old target; the client cannot know the old target (the id is
|
||||
opaque) and folds the optimistic "Formula updated".
|
||||
`renaming_a_formula_from_the_panel_lands_the_same_document_as_the_host` asserts
|
||||
both sides of that, as `a_formula_the_client_cannot_parse_diverges_on_the_status_line`
|
||||
does for a parse error.
|
||||
|
||||
**The browser assertion that PR #60 dropped stays dropped, and improvise-qf57
|
||||
is the decision rather than the oversight.** `#grid input` `toHaveCount(0)` was
|
||||
@@ -1026,6 +1045,9 @@ running the spec: `#grid input` `toHaveCount(0)` **passes**, and the run fails
|
||||
on the `#mode` assertion below it. Restoring it would add an assertion that
|
||||
cannot fail — the same defect this file already records one line down, where an
|
||||
explanatory `assert!` was removed for never being able to be the failing one.
|
||||
Since improvise-km71 the activation is real rather than invented, so the spec's
|
||||
`#mode` assertion reads `"FORMULA"` and it also asserts that `#minibuffer` holds
|
||||
the clicked formula's text; the argument about `#grid input` is unchanged.
|
||||
|
||||
**Rule 2 — a click is all of its intents or none of them** (improvise-hkr8).
|
||||
Each `click_*` returns `Option<Vec<Command>>` and `click` does the single
|
||||
@@ -1450,14 +1472,24 @@ so another client's write to *that same record* bumped the counter in the
|
||||
client's `BasisCache`. The commit then claimed the fresh counter and was
|
||||
accepted, and the other write was silently overwritten. If the other client
|
||||
cleared the cell instead, the commit re-created it. An `EditTarget` is
|
||||
`{ cell, version }`: the record's counter as the client had it when the editor
|
||||
opened. `BasisCache::claim_as_read` states that version beside the *current*
|
||||
document tiers.
|
||||
`{ document_generation, cell, version, opened }`: the document and the record's
|
||||
counter as the client had them when the editor opened, and the seed it opened
|
||||
on (improvise-ixxx, above). `BasisCache::claim_as_read`
|
||||
states both beside the *current* `layout_version`.
|
||||
|
||||
The document tiers stay current on purpose. The rows renumbering under the
|
||||
editor, which was 2wu3's case, moves `layout_version`. Claiming that at open
|
||||
would refuse every such commit as `LayoutMoved`, although the identity already
|
||||
makes the commit safe. So the result is:
|
||||
**The two document tiers are not alike, and only the layout stays current.**
|
||||
The rows renumbering under the editor, which was 2wu3's case, moves
|
||||
`layout_version`. Claiming that at open would refuse every such commit as
|
||||
`LayoutMoved`, although the identity already makes the commit safe. The
|
||||
generation is the opposite case (improvise-vndi). A `RecordHash` is derived
|
||||
from coordinates, so after another client's `LoadModel` the same record can
|
||||
resolve in the incoming document, and a fresh `DataStore`'s counter can equal
|
||||
the one the editor read. Claiming the current generation then got the commit
|
||||
**accepted**: the loaded document's value, which this user never saw, was
|
||||
overwritten. Nothing composed against the outgoing document names anything in
|
||||
the incoming one, which is `Rejection::DocumentReplaced`'s own contract, so the
|
||||
generation is captured at open and the swap refuses the commit. So the result is:
|
||||
- the document was replaced: `DocumentReplaced`;
|
||||
- the record moved: `RecordMoved`;
|
||||
- the record is gone: `RecordGone`;
|
||||
- only the rows moved: the commit is accepted.
|
||||
@@ -1469,10 +1501,11 @@ row. So the record resolves, and its counter has moved. `RecordGone` needs a
|
||||
record that neither the store nor the layout resolves.
|
||||
|
||||
**`EditTarget` is the one object an editor captures at open, and it is a
|
||||
struct so it can grow.** improvise-ixxx needs the seed captured at open too (a
|
||||
commit whose buffer equals the seed writes nothing). That is a `seed: String`
|
||||
field beside `cell` and `version`, and the page does not change, because the
|
||||
target is opaque to it.
|
||||
struct so it can grow.** Two fields have joined `cell` and `version` since:
|
||||
the seed as `opened` (improvise-ixxx: a commit whose buffer equals the seed
|
||||
writes nothing) and the generation (improvise-vndi). The page did not change
|
||||
for either, because the target is opaque to it. `typed_target_frame` asks the
|
||||
two in that order: an untouched editor writes nothing, so it is never refused.
|
||||
|
||||
`typed_cell_command` is the one composition shared by the mouse path
|
||||
(`typed_target_frame`) and the keyboard path (`keys::commit`). Writing it twice is
|
||||
@@ -1573,8 +1606,10 @@ Four smaller decisions that would be re-derived wrongly:
|
||||
Since improvise-n6t8 it shares `#bottom-dock` with the toast, which stacks
|
||||
**above** it, so the prompt is neither covered nor displaced.
|
||||
|
||||
`RenderCell::edit_text` is the stored value, not the formatted one: seeding an
|
||||
editor from `text` and committing turns `1200` into the string `"1,200"`. The
|
||||
An editor is seeded from `edit_seed(target)`, the stored value the target
|
||||
captured, never from `text`: seeding from the formatted text and committing
|
||||
turns `1200` into the string `"1,200"`. `RenderCell` carries no `edit_text`
|
||||
since improvise-ugj4, because a paint can be stale while an editor is open. The
|
||||
`numeric`/`error` flags are the server's answers, echoed — the client must not
|
||||
re-derive them from the text, which is how the two ends drift.
|
||||
|
||||
@@ -1617,7 +1652,7 @@ The browser half of improvise-cr3, kept as thin as it can be made, now talking
|
||||
to either of two peers (improvise-djm). **The boundary is JSON strings in and
|
||||
out, so this file holds no protocol knowledge at all**: it never inspects a
|
||||
`Command`, never builds a `CellId`, never decides whether a cell is editable or
|
||||
whether a typed string is a number. Five calls are the whole harness:
|
||||
whether a typed string is a number. These calls are the whole harness:
|
||||
|
||||
| call | what it is |
|
||||
|---|---|
|
||||
@@ -1625,6 +1660,8 @@ whether a typed string is a number. Five calls are the whole harness:
|
||||
| `client.on_frame(json)` | fold one frame, in arrival order |
|
||||
| `client.render_model()` | everything to paint, already decided |
|
||||
| `client.edit_target(r, c)` | what an editor opened here edits (the record, the version seen, and the seed it opened on): an opaque string the page holds, or `null` |
|
||||
| `client.edit_seed(target)` | the text the target's editor opens on: the seed it captured, never the paint (improvise-ugj4) |
|
||||
| `client.edited_target(target)` | the same target marked edited, so retyping the seed still writes (improvise-jibp) |
|
||||
| `client.typed_cell(target, s)` | the frame for "the user typed s into that cell", by identity (improvise-2wu3), or none if `s` is still the target's seed (improvise-ixxx) |
|
||||
| `client.on_key_event(k,c,a,s,m)` | the frame for a keypress, folded locally first where this client may answer it |
|
||||
|
||||
@@ -1832,9 +1869,10 @@ Neither arm leaves one armed.
|
||||
- **`editable` is the server's answer relayed through Rust**: a stored cell. An
|
||||
aggregate is a drill target and drill is not on the wire yet, so it is simply
|
||||
not offered.
|
||||
- The inline editor seeds from `RenderCell::edit_text` — the STORED value, not
|
||||
the drawn one: `td.textContent` has been through the number format, so a stored
|
||||
1200 reads `"1,200"` and committing that stores the *string* `"1,200"`. Enter
|
||||
- The inline editor seeds from `client.edit_seed(target)` — the STORED value the
|
||||
target captured, not the drawn one: `td.textContent` has been through the number
|
||||
format, so a stored 1200 reads `"1,200"` and committing that stores the *string*
|
||||
`"1,200"`. Enter
|
||||
removes the focused `<input>`, which fires `blur`, which calls `close` again:
|
||||
harmless, but only by accident.
|
||||
- **The title bar is three fields and one joining.** `doc_name`, `doc_dirty` and
|
||||
|
||||
+187
-16
@@ -1766,9 +1766,12 @@ its records under its own aggregator. The user ruled on three cases:
|
||||
- **Text** follows the unanimity rule (gotcha 7): the text if every row agrees,
|
||||
otherwise the `mixed` error.
|
||||
- **An error** in any row makes the total an error, which is what that rule
|
||||
answers for an error among numbers. A formula column is evaluated over the
|
||||
pooled inputs, so a row's `div/0` is not an input and does not propagate.
|
||||
Only the fold's own error does.
|
||||
answers for an error among numbers. That holds for a **stored** error. A
|
||||
formula column is evaluated over the pooled inputs, so a row's `div/0` is not
|
||||
an input and does not propagate. Only the fold's own error does. Rulings (1)
|
||||
and (2) disagree here: the column shows an error in one row and a clean
|
||||
number under it. The user chose the **pooled ratio** (improvise-b5dp). Margin
|
||||
over rows 25/30, 27/30 and -4/0 totals 48/60 = 0.80, not `div/0`.
|
||||
- **With `_Measure` on the row axis** the strip is empty, because a column adds
|
||||
different measures together. An empty strip already means "no totals"
|
||||
(gotcha 33).
|
||||
@@ -1789,16 +1792,100 @@ before and after:
|
||||
| pathological (90,000) | 1.54 s | 3.49 s | 1.07 s | 1.96 s |
|
||||
|
||||
`col_widths` is the keypress path, and it pays too, because a total's text sizes
|
||||
its column. The fold rebuilds a key per drawn row and reads it again rather than
|
||||
reusing the walk's value, since a pooled Avg or a formula needs records, not the
|
||||
rows' answers. A grid-wide fold memo (improvise-h6z1) now exists, and it
|
||||
removes the repeated **formula** chains a fold row and a total re-derive: about
|
||||
2x on a folded five-formula chain. It does **not** move this table. Those grids
|
||||
hold only stored measures, a stored fold is never memoised, and each total
|
||||
reads its column's records once regardless. What is left of this cost is that
|
||||
one read per column. The walk
|
||||
no longer carries a sum, so the window-into-totals fault below cannot happen any
|
||||
more. The test still guards the widths half.
|
||||
its column. The event loop draws once per key, so every key pays the fold twice:
|
||||
once in `App::cmd_context` for `visible_cols`, and once in the draw's `grid_of`.
|
||||
At 4.8k cells that took a held arrow key from about 31 ms to about 52 ms, past
|
||||
the ~33 ms key-repeat interval (improvise-yb4b). The walk no longer carries a
|
||||
sum, so the window-into-totals fault below cannot happen any more. The test
|
||||
still guards the widths half.
|
||||
|
||||
**The fold scans instead of enumerating (improvise-yb4b).** The cost was not the
|
||||
fold's arithmetic. It was *how the fold found its records*: `fold_stored` built
|
||||
one `CellKey` per member, the cross product of every spanned category's covered
|
||||
items (3,000 keys for an 18k column), and looked each one up. Now
|
||||
`group_expansion` returns the expansion as data: a `base` key and the covered
|
||||
items of each expanded category. `fold_stored` hands that to
|
||||
`DataStore::spanned_values`, which narrows through the index and filters each
|
||||
candidate by membership. In `Exact` mode a candidate must also carry no
|
||||
coordinate beyond those, which is what `get(member)` answered. Only a formula
|
||||
still enumerates members (`contexts_of`), since its references are evaluated
|
||||
per context.
|
||||
|
||||
**How it narrows is a choice, and the first version got it wrong
|
||||
(improvise-jq0m).** Narrowing by `base` alone is right for a column total, which
|
||||
spans *every* item of each row category. It is wrong for a collapsed group,
|
||||
which spans two or seven items of a category with hundreds: `base` drops the
|
||||
grouped category entirely, so every fold cell scanned every record of the
|
||||
column, and a grid of small groups went quadratic. The review measured 1000
|
||||
days in pairs at 11–15.7 s a keypress, against 0.44–0.51 s before #239. So
|
||||
`spanned_positions` now prices both narrowings off the index's own set sizes
|
||||
and takes the cheaper: once by `base`, or once per covered item of the span
|
||||
with the fewest items, each intersected with `base`. The per-item positions are
|
||||
disjoint (a record has one item per category) and are sorted afterwards, so
|
||||
both paths return the store's order. A `base` naming a name the store never
|
||||
interned matches nothing on both paths.
|
||||
|
||||
The bench now carries the grouped cases, which it could not see before: Day in
|
||||
collapsed groups on Row, with Product, and `_Measure` on Column. The machine is
|
||||
an Apple M4 Max (16 cores, 64 GB) at load average 15–63. The figures are
|
||||
keypress (`whole-grid + col_widths`), min-of-N, from two alternating rounds,
|
||||
the second reversed. Each cell gives both rounds:
|
||||
|
||||
| case | before #239 (`5932ad74`) | `f668ddc9` | jq0m | jq0m ÷ before |
|
||||
|---|---|---|---|---|
|
||||
| 4.8k dense | 33.9 / 39.1 ms | 40.7 / 40.2 ms | 36.3 / 33.3 ms | 1.07 / 0.85 |
|
||||
| 18k dense | 237 / 239 ms | 238 / 232 ms | 196 / 211 ms | 0.83 / 0.88 |
|
||||
| 90k dense | 3.04 / 3.46 s | 3.24 / 2.70 s | 2.68 / 2.84 s | 0.88 / 0.82 |
|
||||
| 364 days in weeks | 483 / 415 ms | 1.42 / 1.18 s | 356 / 399 ms | 0.74 / 0.96 |
|
||||
| 364 days in pairs | 715 / 539 ms | 5.02 / 4.62 s | 471 / 518 ms | 0.66 / 0.96 |
|
||||
| 1000 days in pairs | 406 / 399 ms | 10.8 / 20.2 s | 352 / 364 ms | 0.87 / 0.91 |
|
||||
|
||||
The grouped cases now come in **below** the pre-#239 figures. The old
|
||||
per-member path looked up each member through a full `CellKey`, and the
|
||||
per-item narrowing intersects index sets directly.
|
||||
|
||||
Two levers were **not** taken. Combining the walk's row values for decomposable
|
||||
aggregators was rejected under §5 as a second derivation of the total. It is
|
||||
also wrong wherever a formula owns a row-axis item: the row reads the formula,
|
||||
while the spanning fold reads the store. Sharing one total between
|
||||
`cmd_context` and the draw was also rejected: a key's effects run between the
|
||||
two, so sharing needs an invalidation story. Dropping the totals from the widths
|
||||
would make the two width computations disagree about `visible_cols`.
|
||||
|
||||
**A grid-wide fold memo (improvise-h6z1)** is the other half, and it answers a
|
||||
different cost. It removes the repeated **formula** chains that a fold row and a
|
||||
total re-derive, about 2x on a folded five-formula chain. It does **not** move
|
||||
the table below. Those grids hold only stored measures, a stored fold is never
|
||||
memoised, and each total reads its column's records once. That one read per
|
||||
column is what the scan above made cheap.
|
||||
|
||||
**What it changes besides speed:** a stored fold's values now arrive in the
|
||||
store's insertion order, not member order. That is the order gotcha 18 names as
|
||||
the store's answer. Min, Max, Count and text unanimity are order-free. A Sum or
|
||||
Avg over non-integral values may differ in the last bits.
|
||||
|
||||
Measured by alternating three builds of `cargo bench --bench grid_projection` on
|
||||
an Apple M4 Max (16 cores, 64 GB) under heavy shared load (load average 30–90).
|
||||
This is min-of-N, three rounds, with the order reversed in the third. The
|
||||
keypress figure is `whole-grid + col_widths`. The millisecond columns are
|
||||
ranges over the rounds. The ratio columns are each round's own pair, in round
|
||||
order:
|
||||
|
||||
| cells | before #239 (`5932ad74`) | #239 (`9948aaf5`) | yb4b | #239 ÷ before | yb4b ÷ before |
|
||||
|---|---|---|---|---|---|
|
||||
| 400 | 1.36–1.77 ms | 2.36–2.76 ms | 1.69–1.82 ms | 2.03, 1.34, 1.78 | 1.24, 0.98, 1.25 |
|
||||
| 1.2k | 4.1–5.2 ms | 7.2–8.2 ms | 5.0–5.3 ms | 2.00, 1.39, 1.85 | 1.23, 0.96, 1.25 |
|
||||
| 2.4k | 10.8–13.6 ms | 18.4–21.5 ms | 12.0–13.9 ms | 1.98, 1.36, 1.91 | 1.18, 0.88, 1.29 |
|
||||
| 4.8k | 32.8–41.8 ms | 55.4–64.1 ms | 34.5–42.0 ms | 1.77, 1.32, 1.92 | 1.15, 0.82, 1.26 |
|
||||
| 9.6k | 102.8–121.2 ms | 190.7–209.6 ms | 107.2–140.0 ms | 2.04, 1.57, 1.92 | 1.36, 0.88, 1.20 |
|
||||
| 18k | 192.9–208.3 ms | 338.6–448.0 ms | 230.4–273.3 ms | 2.32, 1.63, 1.72 | 1.42, 1.11, 1.23 |
|
||||
| 90k | 2.68–3.26 s | 4.92–6.49 s | 3.18–4.00 s | 1.99, 1.84, 1.79 | 1.23, 1.47, 1.14 |
|
||||
|
||||
Read the ratio columns, not the milliseconds. Round 2's `before` ran at load
|
||||
86 against about 31 for its `yb4b`, which is why its ratios fall below 1. The
|
||||
two clean rounds put #239 at about 1.8–2.3× the pre-#239 keypress, and put
|
||||
yb4b at about 1.15–1.4×. At 4.8k a held key is back under the ~33 ms repeat
|
||||
interval in the quietest round (34.5 ms against 32.8 ms before).
|
||||
|
||||
The guard is `a_window_narrows_the_cells_and_neither_the_totals_nor_the_widths`:
|
||||
with one walk serving three consumers, a window that leaked into the other two
|
||||
@@ -2327,9 +2414,31 @@ would be sent and every client would stay stale *permanently* rather than until
|
||||
next scroll: a partial fix strictly worse than none. Broadcasting only would leave
|
||||
the server projecting each client a window it is no longer looking at.
|
||||
|
||||
The audience is `Broadcast::ToAll`, and the asker is not an exception. Its mirror
|
||||
was reset here like everyone else's, and it folded nothing optimistically, because
|
||||
a `Command::Model` is not something a client can apply to itself.
|
||||
The audience is every session on a reframed view, each addressed with
|
||||
`Broadcast::To` (improvise-nynk), and the asker is not an exception: a transpose
|
||||
invalidates its own position too. What an earlier version of this paragraph got
|
||||
wrong is the claim that the asker "folded nothing optimistically". A
|
||||
`Command::Model` is not something a client can apply to itself, but the **rest of
|
||||
its frame** is, and a browser frame routinely places the cursor *after* the
|
||||
mutation. The `+cat` commit is `[ChangeMode, AddCategory, SetSelected]`. The
|
||||
mirror folds that in order: reset, then placed, which is exactly the order the TUI
|
||||
applies the same batch in. The client folded all of it **before** hearing the
|
||||
reset, so the relayed `ResetCursor` arrived last and threw the placement away.
|
||||
The client sat at the origin while its mirror, and every projection built from
|
||||
it, sat at the placement (improvise-wdmp).
|
||||
|
||||
So the host answers the asker in the order the mirror saw. Once the asker has been
|
||||
told `ResetCursor`, `Host::mutating_frame` relays back to it every **later**
|
||||
cursor-placing command of the same frame (`SetSelected`, `SetRowOffset`,
|
||||
`SetColOffset`), ahead of that command's projections. All three are absolute, so
|
||||
folding them twice is idempotent. Excluding the asker from the reset, which the
|
||||
ticket proposed, was rejected. For a frame that places nothing after the reframe
|
||||
it is gotcha 24's own "strictly worse than nothing": the mirror is reset and the
|
||||
client never hears it. `a_frame_that_reframes_and_then_places_the_cursor_leaves_the_asker_placed`
|
||||
runs both shapes. One residue is stated rather than handled: a frame that
|
||||
reframes and then *switches views* before placing would reset the view the
|
||||
client ends on rather than the one it reframed. No resolver composes that frame
|
||||
today.
|
||||
|
||||
What is *not* broadcast is a position: another client's transpose must not move
|
||||
where I am looking — that is UI and it is mine (improvise-g0f). It may only declare
|
||||
@@ -3563,6 +3672,56 @@ unlisted command that diverges fails the test, and so does a listed one that
|
||||
does not diverge. A stale entry therefore cannot linger, and a new registration
|
||||
is covered the day it lands.
|
||||
|
||||
## Editing a formula: the capture is on the mode, read in `execute` (improvise-km71)
|
||||
|
||||
`edit-formula-at-cursor` (Enter or `e` in the formula panel) opens
|
||||
`AppMode::FormulaEdit` with `slot: FormulaSlot::Existing { id, was }` and seeds
|
||||
the `formula` buffer with the formula's `raw`. That slot is the capture: which
|
||||
formula, and what it read when the editor opened. Nothing at commit time looks
|
||||
at the cursor again.
|
||||
|
||||
**Where `commit-buffer formula` reads the slot matters, and the registry is the
|
||||
wrong place.** The obvious move was to resolve it in `parse_commit_kind`, the
|
||||
way `Item` takes its category from `AppMode::ItemAdd`. That does not work, and
|
||||
it fails silently: the interactive arm is built **from `parse`**
|
||||
(`context/code/cli.md` §8), and `parse` accepts `formula` under
|
||||
`AppMode::Normal`, so the mode-reading fallback is never reached. `Item` only
|
||||
works that way because `parse` *refuses* `item`. The first build of km71 did
|
||||
exactly this, and every edit became an add. `add_formula` upserts by target, so
|
||||
an edit that kept its target still landed in place, and only the rename tests
|
||||
and the concurrency tests showed it. Making the interactive arm read the mode
|
||||
first would break improvise-yf7p's guarantee that an accepted argument list is
|
||||
never overridden by context. So `CommitBuffer::execute` asks
|
||||
`CommitKind::in_mode(ctx.mode)`. That is a read of context in `execute`, which
|
||||
is the answer yf7p gave to `toggle-panel-visibility`. A script line stays an
|
||||
add, and cannot name an edit, because the slot does not exist outside the
|
||||
editor.
|
||||
|
||||
**`effect::ReplaceFormula` refuses with `refuse_and_stop`, not `refuse`.** The
|
||||
Enter binding is `[commit-buffer formula, clear-buffer formula]`, and
|
||||
`CommitBuffer` puts the change back to `FormulaPanel` *after* the mutation. A
|
||||
refusal that stops the batch therefore leaves the editor open with the user's
|
||||
text and the reason in the minibuffer line (`draw_bottom_bar` draws the status
|
||||
beside the minibuffer). That covers all four refusals: the formula is gone, it
|
||||
was rewritten since the editor opened (`was`), the new text does not parse, or
|
||||
the new target belongs to another formula, or the rename is of a formula
|
||||
outside `_Measure` (improvise-kb1d, below). `AddFormula`'s parse error stops the
|
||||
batch the same way since improvise-v4hg (#213), so a bad *new* formula also
|
||||
leaves the editor open with its text; adding and editing now agree.
|
||||
|
||||
**A target change outside `_Measure` is refused (improvise-kb1d).** A regular
|
||||
category's formula target is a real item of that category, not a derived one.
|
||||
`replace_formula` added the new target item and left the old one, so renaming
|
||||
`Total = East + West [Region]` to `Sum` left `Total` as an empty item and
|
||||
appended `Sum` at the end. Renaming the item in place was the alternative and
|
||||
was not taken: an item's name is its identity in cells, views, hidden lists and
|
||||
groups, so that rename is the whole of improvise-sboz, not a small fix. Editing
|
||||
such a formula without changing its target still works.
|
||||
|
||||
On success the command's optimistic "Formula updated" stands, except on a
|
||||
rename, where the effect rewrites the status to name the old and new targets
|
||||
and any formula that still mentions the old one (`context/code/core.md` §18).
|
||||
|
||||
## A wire write reaches only a cell the host projects as a Record (improvise-ciw8)
|
||||
|
||||
`App::resolve_record` answers "which record does this surrogate name". Its
|
||||
@@ -3582,6 +3741,18 @@ the refusal marks the batch, `Host::mutating_frame` undoes the frame, the asker
|
||||
gets `Rejected(MutationRefused)`, and the reason is on the host's status line.
|
||||
The question it asks is the projection's own, never a second derivation of it:
|
||||
|
||||
- A coordinate that names a **group** rather than an item is refused, in any
|
||||
view (improvise-b114). A group has no cell of its own, so writing one stores a
|
||||
non-item coordinate. This rule asks the document, not the layout, and runs
|
||||
first, because a fold in the measure half hides from both rules below. Take
|
||||
`{Month=Feb, _Measure=Money}`, where `Money` groups Revenue and Cost. Its
|
||||
record `{Month=Feb}` is stored, so the second rule below accepts it and never
|
||||
asks the projection. And with `Money` expanded the host draws no such cell at
|
||||
all, so the third rule has no position to ask. A fold on the row axis is
|
||||
refused here too, so the `Aggregate` arm below now guards only the None-axis
|
||||
aggregate. It also closes improvise-xur5: a fold's record `{Month=Q2}` written
|
||||
with a measure the host draws nowhere (hidden, unknown or absent) has no
|
||||
position for the third rule to ask, and was accepted.
|
||||
- A cell a formula owns is refused. That is `cell_id`'s own `Derived` rule,
|
||||
`formula_for(&key)`. It is asked directly because a stored record may sit on
|
||||
no position in the host's current view.
|
||||
|
||||
@@ -1316,3 +1316,57 @@ What this does **not** fix: a text cell whose text reads as a number. `"007"`
|
||||
seeds `"007"`, and `typed()` classifies that as `Number(7.0)`. The seed is
|
||||
exact, but the classifier has no way to know the cell was text. That is
|
||||
improvise-ixxx.
|
||||
|
||||
## 18. Which formula an edit means (improvise-km71)
|
||||
|
||||
A `Formula` has no stored id. It lives in `Model::formulas: Vec<Formula>` and is
|
||||
identified, in practice and by `add_formula`'s upsert, by `(target,
|
||||
target_category)`. **`FormulaId` is that pair behind one opaque type**: its
|
||||
fields are private, `Formula::id()` is its only constructor, and nothing
|
||||
outside `improvise-formula` can read a name out of it. That is deliberate. The
|
||||
stable-identity work (improvise-sboz, ADR 005) will make a formula's identity
|
||||
something other than its names, and every signature that already says
|
||||
`FormulaId` — `Model::formula`, `Model::replace_formula`,
|
||||
`ModelCommand::ReplaceFormula`, `FormulaSlot::Existing`, `FormulaEntry` — then
|
||||
changes by changing this one struct. A name pair spread across those signatures
|
||||
would have made the swap a sweep.
|
||||
|
||||
**The index is not a candidate.** A position is what `formula_cursor` is, and
|
||||
the editor must not recompute its target from the cursor at commit
|
||||
(improvise-ixxx/2wu3's lesson): another client deleting an earlier formula
|
||||
shifts every index after it, and the commit would land on a neighbour. It would
|
||||
also put a position on the wire, which `every_model_command_field_is_position_free`
|
||||
makes a compile error.
|
||||
|
||||
**An edit is its own operation, never remove-then-add and never
|
||||
`add_formula`.** `add_formula` upserts on `(target, target_category)`: fed an
|
||||
edit that changes the target, it appends a second formula and leaves the old
|
||||
one; fed an edit onto another formula's target, it silently replaces *that*
|
||||
formula. `Model::replace_formula(id, formula)` writes into the slot `id` names,
|
||||
so the position (and with it `_Measure`'s item order, since
|
||||
`measure_item_names` appends formula targets in formula order) is kept, and it
|
||||
**refuses** when the new target belongs to a different formula.
|
||||
|
||||
**A target change is a rename of the same formula.** The id travels with the
|
||||
edit, so `Profit = …` edited to `Gross = …` is the formula that was `Profit`,
|
||||
in the same place. That is the least surprising reading of "edit this row", and
|
||||
it is what ADR 005 proposes. What a rename cannot carry under name identity is
|
||||
everything else that names the old target: other formulas' references, a
|
||||
view's hidden list, a `measure_agg` entry. Those cannot follow without rewriting
|
||||
`raw` text the parser does not print back, so the edit says so out loud:
|
||||
`ReplaceFormula` reports which formulas still mention the old name
|
||||
(`Formula::mentions`, over the expression and its `WHERE`), and their cells
|
||||
show the `ref` error rather than a silent wrong number. Under a stable id those
|
||||
references are what sboz makes follow. **This holds for `_Measure` targets only.**
|
||||
Outside `_Measure` a target is a real item, and a rename is refused until sboz
|
||||
(improvise-kb1d, `context/code/command.md`). A rename also leaves stored cells under
|
||||
the old `_Measure` name in `## Data`, invisible because the formula shadowed them.
|
||||
|
||||
**Concurrency is checked twice.** `FormulaSlot::Existing { id, was }` is
|
||||
captured when the editor opens: the id must still resolve, and the formula must
|
||||
still read `was`. Either failing means another client removed, renamed or
|
||||
rewrote it meanwhile, and the edit is refused with the batch stopped, so the
|
||||
editor stays open over the user's text (gotcha 21). `was` is not redundant
|
||||
with a stable id: a concurrent *replace* keeps the id and changes the text,
|
||||
which only `was` can see.
|
||||
|
||||
|
||||
@@ -488,8 +488,12 @@ The serde derives that existed only for that path went with it: `Workbook`,
|
||||
`DataStore`'s manual impls and the formula AST. So did indexmap's `serde` feature
|
||||
in `improvise-core`, `improvise-io` and the root crate, and `improvise-formula`'s
|
||||
serde dependency. Each was removed and the whole workspace rebuilt, so the
|
||||
compiler named every other consumer. Two remain, and both are the wire:
|
||||
compiler named every other consumer. Two remained, and both are the wire:
|
||||
`Axis` (whose `rename_all = "lowercase"` is its wire spelling) and `PageAxis`.
|
||||
A third arrived with improvise-km71 by the same rule: `FormulaId`, which crosses
|
||||
the wire in `ModelCommand::ReplaceFormula`, `FormulaSlot` and `FormulaEntry`.
|
||||
So `improvise-formula` depends on serde again, for that one type. No derive went
|
||||
back on the formula AST.
|
||||
`CellValue` was never a candidate, since it is carried on the wire too. JSON
|
||||
*import* (`StartImportWizard`, `ImportJsonHeadless`) is a separate feature and is
|
||||
untouched.
|
||||
@@ -512,6 +516,30 @@ change. `recomputing_formulas_leaves_the_model_fingerprint_alone` pins this.
|
||||
`view_only_effects_leave_shared_state_alone` now runs over a formula-bearing
|
||||
fixture, for the same reason.
|
||||
|
||||
**That fix was canonical within one instance, not across two (improvise-djcn).**
|
||||
It fixed `Model`'s own maps but not the maps inside its fields. `DataStore` has
|
||||
`index`, `records` and the symbol table, and `View` has `page_selections`,
|
||||
`hidden_items` and `collapsed_groups`. All still printed through a derived
|
||||
`Debug`, in hash order. Each `HashMap` gets its own `RandomState`, so two equal
|
||||
documents built separately printed differently in 32 of 32 runs.
|
||||
- The effect-classification tests compare one instance with itself before and
|
||||
after, so they were unaffected.
|
||||
- `a_model_command_lands_where_its_keypress_lands` compares **two** `App`s. It
|
||||
passed only because its fixture had no cells and no multi-entry view maps.
|
||||
|
||||
Now:
|
||||
- `DataStore`, `SymbolTable` and `View` have hand-written `Debug` impls. Each
|
||||
sorts wherever its storage is a hash map.
|
||||
- `DataStore` resolves symbols to their strings. A `Symbol`'s number is interning
|
||||
order, not content, so printing it would make two equal stores differ again.
|
||||
- `SymbolTable` prints its interned strings, sorted.
|
||||
- `cells` keeps its `IndexMap` order, because that order is part of the document
|
||||
(gotcha 18).
|
||||
- Every hand-written `Debug`, `Model`'s included, **destructures** `self`. A field
|
||||
added later is then a compile error there, where a `debug_struct` that names
|
||||
fields one by one would skip it silently. The pc3b version of `Model`'s impl
|
||||
had exactly that hole.
|
||||
|
||||
`parse_number_cell` returns `CellValue::Error` naming the offending text rather
|
||||
than a fabricated `0.0`. No token the current grammar admits actually fails
|
||||
`f64::from_str` (overflow saturates to ±inf), but if the two ever drift the
|
||||
|
||||
+33
-12
@@ -84,6 +84,20 @@ naming it would produce a mode with nothing to add to. `Quit` and
|
||||
`start-import-wizard` — which do work beyond changing the mode, so
|
||||
`enter-mode` would be a way to reach half of each.
|
||||
|
||||
### `AppMode::FormulaEdit` carries what it will commit to (improvise-km71)
|
||||
|
||||
`FormulaEdit { minibuf, slot }` is one mode for two editors. `slot` is
|
||||
`FormulaSlot::New` for `a`/`n`/`o` (`enter-mode formula-edit`, which
|
||||
`from_name` still builds) or `FormulaSlot::Existing { id, was }` for
|
||||
`edit-formula-at-cursor`. It is one variant and not a second `AppMode` because
|
||||
the keys, the buffer and the panel are identical: a second variant would add a
|
||||
`ModeKey`, a keymap and a hint for no difference in what the keys do. The slot
|
||||
is `ItemAdd`'s `category` again, a capture made when the mode is entered. Like
|
||||
`ItemAdd` it is not nameable from `from_name`, since a script line has no
|
||||
formula to open. It must be portable and snapshot-safe (§1 above). `FormulaId`
|
||||
is plain serde data, and a fresh client that inherits an open edit inherits
|
||||
exactly the claim the editor made. The commit re-checks that claim.
|
||||
|
||||
### `Panel`
|
||||
|
||||
`Panel::ALL`/`name()`/`key()` drive the toggle keys for Normal *and* all
|
||||
@@ -164,9 +178,12 @@ attempt in a fourth place is the disease.
|
||||
- The **model half** is `ModelCommand`: every `EffectKind::Model(..)` effect,
|
||||
one variant apiece, carrying that effect's own fields (improvise-kmgs).
|
||||
|
||||
The count is 29 model effects, minus three: `SwitchView`, which `Command`
|
||||
already names and routes, and `ApplyAndClearDrill`/`AcceptWizardModel`, which
|
||||
are **not addressable** (improvise-3x3u). That leaves **26**. A variant with
|
||||
The count is 27 model effects, minus two: `ApplyAndClearDrill` and
|
||||
`AcceptWizardModel`, which are **not addressable** (improvise-3x3u). That
|
||||
leaves **25**. `SwitchView`, `ViewBack` and `ViewForward` are no longer model
|
||||
effects at all (improvise-jytm), so they are not subtracted. This paragraph
|
||||
said "29 minus three, 26" until improvise-km71 added `ReplaceFormula`, and
|
||||
that figure was already stale: the enum had 24. A variant with
|
||||
no data mirrors a nullary effect whose operand is document state the
|
||||
authoritative `App` already holds.
|
||||
|
||||
@@ -699,7 +716,9 @@ for that, beside `typed()`. Every commit path reaches it:
|
||||
- **Browser mouse.** The seed is a field of #188's `EditTarget` (`opened`),
|
||||
captured by `edit_target_at` from the cache's `edit_text` beside the record
|
||||
and its version. `typed_target_frame` starts with the `untouched` check, and
|
||||
the page passes no seed (`typed_cell(target, text)`).
|
||||
the page passes no seed (`typed_cell(target, text)`). The `<input>` shows
|
||||
`edit_seed(target)`, so what the page shows and what it compares against are
|
||||
one capture (improvise-ugj4).
|
||||
|
||||
**The seed is captured at open, never recomputed at commit.** Where the editor
|
||||
opened is where `OpenedEditor` is written. For the TUI that is `EditOrDrill`
|
||||
@@ -742,14 +761,16 @@ deliberately do **not** mark it:
|
||||
|
||||
The comparison stays exact: whitespace is content, and nothing trims.
|
||||
|
||||
**The mouse editor tracks no `touched`.** Its edits happen in a DOM `<input>`
|
||||
that Rust never sees, so on that path `untouched` compares content alone, and
|
||||
the 2054 case survives there (improvise-jibp). The fix belongs with
|
||||
`EditTarget`, for example an `input` listener that reports the first edit. A
|
||||
second gap sits on the same path. The `<input>` is still seeded from the
|
||||
**painted** `editText`, while `EditTarget` captures the **cache**. So a paint
|
||||
that went stale while another editor was open shows old text against a newer
|
||||
seed and version (improvise-ugj4).
|
||||
**The mouse editor reports its first edit.** Its edits happen in a DOM
|
||||
`<input>` that Rust never sees, so the page's `input` listener, registered
|
||||
`{ once: true }`, swaps its target for `edited_target(target)`, which is
|
||||
`EditTarget::edited`: the same capture with `touched` set (improvise-jibp).
|
||||
Before that, the mouse path compared content alone, so the 2054 case survived
|
||||
there. The `<input>` used to be seeded from the **painted** `editText` while
|
||||
`EditTarget` captured the **cache**. `repaint` returns early while an editor is
|
||||
open, so a paint that went stale then showed old text against a newer seed and
|
||||
version (improvise-ugj4). The page now seeds from `edit_seed(target)`, and
|
||||
`RenderCell` no longer carries `edit_text`, so the paint cannot be asked.
|
||||
|
||||
This does **not** change classification. `007` typed fresh is still
|
||||
`Number(7)`, and `a_touched_browser_commit_still_writes` pins that.
|
||||
|
||||
@@ -379,3 +379,48 @@ again.
|
||||
hint is width-constrained and the history lives on help page 5 under "View
|
||||
history" with both halves. That is the #149 lesson — a hint that does not fit is
|
||||
never drawn, so the test pinning the widget cannot see it.
|
||||
|
||||
## A status message lasts until the next key (improvise-cq3d)
|
||||
|
||||
`draw_status` draws `status_msg` *instead of* the mode's derived hint, and
|
||||
nothing in production ever cleared `status_msg`: it was only replaced by the next
|
||||
message. So after the first "Saved", "Loaded" or error of a session, the derived
|
||||
key hint (the one key table on screen, improvise-rr8n) was never drawn again, in
|
||||
any mode.
|
||||
|
||||
**The rule: a message expires at the start of the next keypress.** `draw.rs::press`
|
||||
applies `SetStatus("")` and then dispatches the key, so:
|
||||
|
||||
- a message a key produces is drawn after that key, whatever it is, errors
|
||||
included. It stays until the user presses something, which is the moment they
|
||||
have acted on it. An error is never wiped before it has been drawn.
|
||||
- a message that arrives outside a key, such as one from startup or an import,
|
||||
also stands until the next key.
|
||||
- the next key clears it *before* its own effects run. So a key that writes a
|
||||
new message shows that message, and a key that writes none shows the hint.
|
||||
|
||||
**Rejected alternatives.**
|
||||
|
||||
- *Expire on mode change.* This fails the ticket's own case: a message written in
|
||||
Normal would stand in Normal indefinitely.
|
||||
- *Expire on the next successful command.* Effects carry no notion of success,
|
||||
and a failed command would leave the stale message in place. That is exactly
|
||||
the case where the user most needs the hint back.
|
||||
- *Clear after dispatch.* Every message would be wiped before it is drawn. The
|
||||
test's middle step exists to catch that.
|
||||
- *Compose message and hint on one line.* A long message still eats the hint, and
|
||||
the stale message still stands forever.
|
||||
|
||||
**Why here and not in `App::dispatch_key`.** `dispatch_key` is shared with the
|
||||
lockstep harness (`reduce_key`), which compares `status_msg` on the host and the
|
||||
browser after every key. Expiring there without changing the browser's resolver
|
||||
too would make the two ends disagree. The expiry is also a presentation decision
|
||||
about the TUI's one-line status row, so it belongs to the TUI front end.
|
||||
|
||||
**The browser already behaves this way.** Its `status_msg` is state that is never
|
||||
cleared either, but `web/app.js` shows it in a toast that hides after
|
||||
`TOAST_MS` (`web/shell.js::createToasts`), and `#hint` is a separate element that
|
||||
is always drawn. So a browser user sees a message briefly and never loses the
|
||||
hint. The TUI now does the same, on a keypress rather than a timer, because a
|
||||
terminal status row has one line to share and a keypress is the event the TUI
|
||||
already has.
|
||||
|
||||
@@ -1485,39 +1485,29 @@ view, or with a click on the row already under the cursor, the bug is invisible
|
||||
— the same reason `a_click_focuses_the_panel_moves_its_cursor_and_activates_the_row`
|
||||
clicks Product rather than `_Index`.
|
||||
|
||||
### a_formula_row_click_navigates_and_activates_nothing
|
||||
### a_formula_row_click_navigates_and_opens_the_editor_on_that_row
|
||||
|
||||
The negative arm, and it is a statement about the *rule* rather than about the
|
||||
formula panel. The rule is "go there, then do what `Enter` does there";
|
||||
`ModeKey::FormulaPanel`'s keymap binds `a`/`n`/`o` to `enter-mode formula-edit`
|
||||
and `d`/`Delete` to delete, and **no `Enter` at all**. So the rule yields
|
||||
navigation and nothing else, and that is the rule applying rather than the fix
|
||||
stopping half way.
|
||||
Formerly `a_formula_row_click_navigates_and_activates_nothing`. That was the
|
||||
negative arm of the rule "go there, then do what `Enter` does there", true while
|
||||
`ModeKey::FormulaPanel` bound no `Enter`. improvise-km71 bound `Enter` (and
|
||||
`e`) to `edit-formula-at-cursor`, so the same rule now yields the editor on the
|
||||
clicked row. The test asserts the exact command vector. The clicked row is
|
||||
`Margin` at index 1 and the cursor stays at 0, so a click that read the cursor
|
||||
would open `Profit`.
|
||||
|
||||
This test asserts the *exact* command vector, which is the whole of what it can
|
||||
say. It once carried a second, explanatory `assert!` that no `FormulaEdit` mode
|
||||
change appears — **which could never have been the failing assertion**, because
|
||||
the `assert_eq!` above it pins the same three commands and aborts first. A
|
||||
reviewer demonstrated it by injecting a `ChangeMode(FormulaEdit)` and watching
|
||||
the panic land on the `assert_eq!` with the explanatory message never printed.
|
||||
It was a comment wearing an `assert!`, and the argument now lives in
|
||||
`context/code/clients.md` where a comment run cannot hold it.
|
||||
The old version once carried an explanatory `assert!` that no `FormulaEdit`
|
||||
mode change appeared, which could never have been the failing assertion,
|
||||
because the `assert_eq!` above it pinned the same commands and aborted first.
|
||||
That lesson stands: an `assert!` after an exact `assert_eq!` on the same vector
|
||||
is a comment.
|
||||
|
||||
### the_formula_panel_has_no_enter_to_mirror_and_the_view_panel_does
|
||||
|
||||
The guard that the removed assertion was pretending to be. The asymmetry — a
|
||||
view-row click activates, a formula-row click does not — is a fact about the
|
||||
**keymap**, and nothing tied the click to it. The reviewer proved the gap by
|
||||
adding `Enter -> enter-mode formula-edit` to `ModeKey::FormulaPanel` and running
|
||||
the whole workspace: **1381 passing, nothing failed**, while `click_formula_row`
|
||||
still activated nothing. The click and the keyboard then disagree about what a
|
||||
formula row does, silently.
|
||||
|
||||
Injecting that same binding now fails this test and only this test. The failure
|
||||
message names `click_formula_row` as what must change, because the person who
|
||||
adds an `Enter` binding is not the person reading `resolve.rs`'s click arms. The
|
||||
positive half (`ViewPanel` *does* bind `Enter`) is there so the test cannot pass
|
||||
by asserting that no panel binds anything.
|
||||
The guard that tied the click to the keymap was
|
||||
`the_formula_panel_has_no_enter_to_mirror_and_the_view_panel_does`. A reviewer
|
||||
showed that nothing else tied them: adding an `Enter` binding to `FormulaPanel`
|
||||
left 1381 tests passing while `click_formula_row` still activated nothing. That
|
||||
test is replaced by `a_formula_row_click_ends_with_what_enter_does_on_that_row`,
|
||||
below, which compares the click with what `Enter` actually resolves to rather
|
||||
than asserting a binding's absence.
|
||||
|
||||
#### The view and formula row clicks in the browser (improvise-iioi)
|
||||
|
||||
@@ -4695,6 +4685,29 @@ laundering invisible (the edge) is also the one that makes a wrong-direction
|
||||
jump visible, so moving off the edge fixed one blind spot and opened the other,
|
||||
and each needs its own question.
|
||||
|
||||
### a_formula_row_click_ends_with_what_enter_does_on_that_row
|
||||
|
||||
improvise-km71, replacing `the_formula_panel_has_no_enter_to_mirror_and_the_view_panel_does`.
|
||||
The old guard asserted that `Enter` was unbound, which tied the click to the
|
||||
keyboard only by the message in its `assert!`. This one resolves `Enter` at
|
||||
cursor 1 and asserts that the click on row 1, made with the cursor at 0, ends
|
||||
with the same commands. Rival: `click_formula_row` activates at
|
||||
`state.view.formula_cursor` instead of the clicked index. This test and
|
||||
`a_formula_row_click_navigates_and_opens_the_editor_on_that_row` both fail. The
|
||||
cursor and the index differ in the fixture, which is what lets them fail.
|
||||
|
||||
### renaming_a_formula_from_the_panel_lands_the_same_document_as_the_host
|
||||
|
||||
improvise-km71. `editing_a_formula_from_the_panel_agrees_with_the_host` alone
|
||||
cannot separate a browser that composes `ReplaceFormula` from one that composes
|
||||
`AddFormula`: the upsert lands a same-target edit in place on the twin too.
|
||||
Rival: `CommitKind::of` ignores the slot. Both lockstep tests fail. The edit
|
||||
test fails only on the status line ("Formula added" against "Formula
|
||||
updated"). This test fails on the **document**, which is the property. It also
|
||||
pins a real divergence: on a rename the host says what was renamed and the
|
||||
client cannot, because `FormulaId` is opaque and it does not know the old
|
||||
target.
|
||||
|
||||
**improvise-w7qd, and the floor was the hole.** As first written the test
|
||||
covered `move-selection` and a four-entry jump table, behind `pressed >= 14`.
|
||||
Two things passed the whole workspace, both found in re-review of #180 at
|
||||
@@ -4976,11 +4989,50 @@ sends no write, and a touched one does". It wraps `Worker.prototype.postMessage`
|
||||
opens the editor on cell 4, and presses `Enter`. No `SetCell` may be posted.
|
||||
Then it types `1` and presses `Enter`, and exactly one must be.
|
||||
|
||||
**Known gap, filed as improvise-ugj4.** The `<input>` is still seeded from the
|
||||
painted `editText`, while `EditTarget` captures the cache. A paint made stale
|
||||
while another editor was open (`repaint` returns early while `editing` is set)
|
||||
therefore shows old text against a newer seed and version.
|
||||
**improvise-jibp** is the other residue: the mouse editor has no `touched` flag.
|
||||
Both gaps this entry used to list are closed: the stale-paint seed
|
||||
(improvise-ugj4) and the missing `touched` flag (improvise-jibp). See the
|
||||
entries that follow.
|
||||
|
||||
### `a_mouse_target_marked_edited_writes_its_own_seed`
|
||||
|
||||
**improvise-jibp**, the Rust half. A target on the seed `"1200"` commits
|
||||
`"1200"` and composes nothing: that is the control. `edited()` must leave the
|
||||
seed where it was, and the same text committed through the edited target must
|
||||
compose a write. **Rival:** `edited()` leaving `touched` false fails this test
|
||||
and `the_page_opens_its_editor_on_what_the_target_captured`, and nothing else.
|
||||
|
||||
### `the_page_opens_its_editor_on_what_the_target_captured`
|
||||
|
||||
**improvise-ugj4 and improvise-jibp**, through the wasm-bindgen surface the
|
||||
page calls. The cell is drawn `"1,200.50"` and stored `"1200.5"`, and
|
||||
`edit_seed` must return `"1200.5"`. A projected write then moves the cell to
|
||||
`"777"`. A target taken afterwards seeds `"777"`, and the first target still
|
||||
seeds `"1200.5"`, because a target is one capture. `typed_cell(later, "777")`
|
||||
composes nothing, and the same text through `edited_target(later)` composes a
|
||||
write. **Rival:** as above, `edited()` leaving `touched` false fails here. I
|
||||
wrote no rival for an `edit_seed` that re-reads the cache. `EditTarget` names a
|
||||
record, not a position, so that rival would need a lookup the client does not
|
||||
have.
|
||||
|
||||
### a mouse editor typed back to its own seed still writes — `harness/tests/contract.spec.js`
|
||||
|
||||
**improvise-jibp**, the page half. The editor opens on cell 4, and the test
|
||||
types the seed over the selection and presses Enter. Exactly one `SetCell` must
|
||||
be posted. Before the fix the page sent `typed_cell(target, seed)` with an
|
||||
untouched target, which composes nothing. **Rival:** delete the page's `input`
|
||||
listener. This test fails, and the rest of `contract.spec.js` passes (16).
|
||||
|
||||
### an editor opened after an unpainted write shows and commits what it captured — `harness/tests/contract.spec.js`
|
||||
|
||||
**improvise-ugj4.** The test opens an editor on cell 0, so `repaint` stops.
|
||||
While it is open, it delivers a synthetic frame that moves cell 4 to `777`. It
|
||||
builds that frame from the last `Cells` entry the page actually received for
|
||||
cell 4, changing only `text`, `edit_text` and `value`, so the record and the
|
||||
version are real. Then it opens cell 4. The `<input>` must show `777`, and
|
||||
Enter with no edit must post no `SetCell`. **Rival:** seed the `<input>` from the
|
||||
paint (`drawn.replaceAll(",", "")`). This test fails on the value, and so does
|
||||
"an untouched mouse editor sends no write, and a touched one does", because the
|
||||
drawn text is not the seed. The other 15 pass.
|
||||
|
||||
### `a_browser_editor_typed_back_to_its_seed_still_writes`
|
||||
|
||||
@@ -5046,3 +5098,22 @@ on row 1. The test presses `i`, types `9`, and asserts the first editor really
|
||||
is touched. Enter must then land on (2, 0), and the folded `opened_editor` must
|
||||
be exactly `OpenedEditor::on("800")`. **Injection:** `resolve::advanced`
|
||||
carrying the previous editor's `touched` flag. This test fails alone.
|
||||
|
||||
### an_edit_opened_before_a_document_swap_is_refused_in_the_new_document
|
||||
|
||||
improvise-vndi, through a real `Host` over `MemoryFs`. A opens an inline editor
|
||||
on North/Revenue, which shows 1200. B sends `LoadModel` for a document that
|
||||
differs only in that cell, which holds 5555. Every name, the view included, is
|
||||
kept, per gotcha 39's fixture warning: a rename would relay a `SwitchView` and
|
||||
hide the bug. A folds the rebase `Snapshot` with the editor still open, as
|
||||
`web/app.js` does, and then commits "42". The `EditTarget` held `{cell,
|
||||
version}` and `claim_as_read` paired that version with the *current*
|
||||
generation. `RecordHash` is coordinate-derived, so the same record resolved
|
||||
in the new document at a matching counter, and the host **accepted** the
|
||||
commit. It failed at the `DocumentReplaced` assertion: the answer was
|
||||
`Accepted`, and 5555 had become 42.
|
||||
|
||||
Rival injected: `claim_as_read` ignoring the captured generation and claiming
|
||||
the current one. That is the pre-fix behaviour, restated after the fix. Only
|
||||
this test failed in the whole workspace. The layout tier staying current is
|
||||
still pinned by `an_edit_writes_the_cell_the_user_clicked_after_the_rows_renumber`.
|
||||
|
||||
@@ -205,6 +205,28 @@ Row, swapping the axes and rotating Row→Column land on the same view, and an
|
||||
equivalence test that cannot distinguish the wrong effect from the right one
|
||||
proves nothing (`doctrine(check_the_checker)`).
|
||||
|
||||
**The fixture needs cells and multi-entry view maps too (improvise-djcn).** The
|
||||
test compares `format!("{:?}", workbook)` across **two** `App`s. It passed only
|
||||
because the fixture had no cells and every view map had at most one entry, and a
|
||||
`HashMap` of one entry has only one order. The fixture now carries:
|
||||
- three cells;
|
||||
- two page selections;
|
||||
- two categories with hidden items, one of them with two;
|
||||
- two categories with collapsed groups.
|
||||
|
||||
That fixture was red in 3 of 3 runs on the old `Debug` impls: the by-key and
|
||||
by-command fingerprints differed only in hash order. It was green after the fix.
|
||||
|
||||
Each half of the fix was checked by restoring the derived `Debug` on its own:
|
||||
- `View`'s derived `Debug` alone: 3 of 3 runs red.
|
||||
- `DataStore` and `SymbolTable`'s derived `Debug` alone: 3 of 3 runs red.
|
||||
|
||||
So neither half is carried by the other. The exhaustive destructure in `Model`'s
|
||||
`Debug` has no test of its own, because it is a compile-time guard. It was
|
||||
checked by adding a `probe_field` to `Model` and to `Model::new`. The build then
|
||||
failed with `E0027: pattern does not mention field probe_field` at the `Debug`
|
||||
impl, and nowhere else.
|
||||
|
||||
### a_stale_wire_model_command_cannot_be_applied
|
||||
|
||||
A wire `Command::Model` cannot reach the document without its basis being checked
|
||||
@@ -5657,6 +5679,46 @@ across two independently built ones. If a future refactor made the fixture
|
||||
share one map, or share one `RandomState`, the second assertion fails. Without
|
||||
this test the law would then pass everything and look healthy.
|
||||
|
||||
### changing_the_target_renames_the_formula_in_its_own_place
|
||||
|
||||
improvise-km71. This is the test that separates "edit" from "add". The rival
|
||||
was the first build: `commit-buffer formula` was resolved in `parse_commit_kind`,
|
||||
where the registry's parse-first rule never reaches the mode, so every edit
|
||||
became an `AddFormula`. `committing_an_edit_replaces_the_formula_in_place`
|
||||
**passed** under that rival, because `add_formula` upserts by
|
||||
`(target, target_category)` and an edit that keeps its target lands in the same
|
||||
slot. Injected on purpose (the commit emits `AddFormula` for the replace kind):
|
||||
this test, the rename-status test, the collision test, the parse-error test and
|
||||
both concurrency-refusal tests fail, and the in-place test passes. A rename is the input the upsert cannot fake. The fixture has three
|
||||
formulas with the cursor on index 1, so "the one under the cursor" is not also
|
||||
"the first" or "the last".
|
||||
|
||||
### an_edit_lands_on_its_formula_after_an_earlier_one_is_deleted_meanwhile
|
||||
|
||||
improvise-km71. The capture is made when the editor opens and never re-read
|
||||
from the cursor. Rival: `ReplaceFormula` resolves its formula from
|
||||
`formula_cursor` at commit. The fixture deletes `Profit` (index 0) while the
|
||||
editor is open on `Tax` (index 1), so cursor 1 now names `Margin`. The rival
|
||||
rewrote `Margin`, and this test fails, along with
|
||||
`an_edit_whose_formula_was_deleted_meanwhile_refuses_out_loud` and
|
||||
`an_edit_whose_formula_was_changed_meanwhile_refuses_out_loud`. The other nine
|
||||
formula-edit tests pass under it, because none of them moves the list.
|
||||
|
||||
### an_edit_whose_formula_was_changed_meanwhile_refuses_out_loud
|
||||
|
||||
improvise-km71. Another client rewrote `Tax`'s text but kept its target, so the
|
||||
id still resolves. Only the `was` check can see this. Rival: the `was` filter
|
||||
deleted. This test is the only one that fails. That is the case for keeping
|
||||
`was` after improvise-sboz gives formulas a stable id: a concurrent replace
|
||||
keeps the id.
|
||||
|
||||
### a_rename_onto_another_formulas_target_refuses_and_keeps_both
|
||||
|
||||
improvise-km71. Rival: `Model::replace_formula`'s collision check disabled.
|
||||
This test and `replace_formula_refuses_to_displace_another_formula` fail.
|
||||
Without the check, renaming `Tax` to `Margin` leaves two formulas with one
|
||||
identity, and `formula(&id)` then finds the first.
|
||||
|
||||
### a_fresh_session_has_no_history_to_step_back_through
|
||||
|
||||
improvise-lxg0. `project.rs::fresh_view` built a new session's slice by folding
|
||||
@@ -5802,8 +5864,10 @@ client sends `SetCell` (then, separately, `ClearCell`) for `{Month=Q2, _Measure=
|
||||
with a caught-up basis. At `ea119bd9` both were answered `Accepted`, and the
|
||||
`SetCell` stored `Month=Q2, _Measure=Cost = 7`, the non-item coordinate
|
||||
improvise-gw18 exists to prevent. The test asserts `Rejected(MutationRefused)`
|
||||
and an unchanged document for both. Rival: the aggregate refusal disabled. This
|
||||
test and the None-axis test fail. The control is
|
||||
and an unchanged document for both. Rival: the aggregate refusal disabled. At
|
||||
`46e75c41` that failed this test and the None-axis test. Since improvise-b114 a
|
||||
fold is refused first by the group rule, so this rival fails only the None-axis
|
||||
test. Disabling both rules fails this test. The control is
|
||||
`a_wire_write_to_a_stored_cell_beside_a_fold_is_accepted`, so a host that refused
|
||||
every wire write cannot pass.
|
||||
|
||||
@@ -5846,6 +5910,39 @@ that the target projects as `Record`, so a fixture where the first column is
|
||||
harmless cannot pass. The fix looks the cell up by its whole key. The rival is
|
||||
the old lookup by record, which is this test's red commit, `6e021699`.
|
||||
|
||||
### a_wire_write_naming_a_measure_group_is_rejected_in_any_view
|
||||
|
||||
improvise-b114, found in the review of PR #217 at `bc060f67` (the reviewer's r3).
|
||||
`_Measure` is on Column, Revenue and Cost are grouped as `Money`, and `Units` is
|
||||
ungrouped. Every month is funded. A caught-up `SetCell` on
|
||||
`{Month=Feb, _Measure=Money}` was answered `Accepted` and stored
|
||||
`Month=Feb, _Measure=Money = 7`, a non-item coordinate. Its record `{Month=Feb}`
|
||||
is stored, so the stored arm accepted it without asking the projection. The test
|
||||
runs with `Money` collapsed, where the host draws that cell as the fold, and
|
||||
expanded, where the host draws no such cell. It asserts `MutationRefused` and an
|
||||
unchanged document in both, so a rule that asks the host's layout cannot pass
|
||||
the expanded half. Red at `5eaafbae`. The fix is `App::group_refusal`, which asks
|
||||
the document whether any coordinate names a group rather than an item. Rival:
|
||||
the group rule removed. Injected into a full workspace run, it fails this test
|
||||
and nothing else.
|
||||
|
||||
### a_wire_write_to_a_fold_record_with_an_undrawn_measure_is_rejected
|
||||
|
||||
improvise-xur5, found in the delta re-review of PR #217 at `6026768c`. It is the
|
||||
reviewer's `review_217b::probe_fold_hidden_measure`, widened to all three rows
|
||||
of the reviewer's table. Q2 is collapsed and every month is stored, so the
|
||||
record `{Month=Q2}` is unstored and resolves only through the layout, at the
|
||||
fold row. A caught-up write names that record with a measure the host draws
|
||||
nowhere: `Cost` hidden in the host's view, `Bogus`, which is no measure at all,
|
||||
or no measure. At `6026768c` all three were **accepted** and stored `Month=Q2`,
|
||||
a group name, as a coordinate. The sjdb fix finds the position by the whole key,
|
||||
and "not drawn" read as "not refused". Falling back to the by-record lookup for
|
||||
an undrawn key would close this, but the reviewer showed it wrongly refuses
|
||||
Feb × Revenue beside a collapsed `Money` group, so it is not the fix. The
|
||||
improvise-b114 rule is: no coordinate names a group, asked of the document.
|
||||
Rival: the group rule removed. Injected into a full workspace run, it fails this
|
||||
test and the b114 test, and nothing else.
|
||||
|
||||
### a_stored_record_the_hosts_view_aggregates_is_writable
|
||||
|
||||
improvise-sjdb, from the #217 review. South/Revenue is stored, then `Product` is
|
||||
@@ -5932,6 +6029,16 @@ 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.
|
||||
|
||||
### renaming_a_formula_outside_measure_refuses_and_keeps_the_editor
|
||||
|
||||
improvise-kb1d, found by the pre-merge review of #214 and by ChatGPT Codex (P1).
|
||||
The fixture has a Region-targeted formula at index 1, between two `_Measure`
|
||||
formulas, because every other km71 test used `_Measure`, whose items are
|
||||
derived from the formula list and cannot show a stranded item. Rival: the guard
|
||||
in `Model::replace_formula` disabled. This test fails on the formula list, and
|
||||
`replace_formula_refuses_to_rename_a_target_outside_measure` in core fails too.
|
||||
|
||||
|
||||
### `an_untouched_pivot_edit_of_a_misread_value_writes_nothing`
|
||||
### `an_untouched_records_edit_of_a_misread_value_writes_nothing`
|
||||
|
||||
@@ -6119,6 +6226,53 @@ runs `cargo test --workspace --no-fail-fast`. Four of four fail this test, and
|
||||
nothing else fails. None of the four needs an abort: each is the only effect its
|
||||
command emits, and `p`/`i` are single bindings, not sequences.
|
||||
|
||||
### a_formula_rows_error_does_not_poison_the_pooled_total
|
||||
|
||||
improvise-b5dp, the user's ruling on the conflict between wm20's rulings (1) and
|
||||
(2): a formula column's total is the **pooled ratio**. A row's formula error
|
||||
does not poison it, and a stored error still does
|
||||
(`an_error_in_any_row_makes_the_total_an_error`). Rows North 25/30, South 27/30
|
||||
and East -4/0 draw 0.83, 0.90 and `div/0`. The total must read **0.80**
|
||||
(48/60), not an error. The test asserts that East's row really draws the error,
|
||||
so a fixture where no row errors cannot pass it.
|
||||
|
||||
It was **green at `9948aaf5`**, because #239 already implemented the choice the
|
||||
user then ratified. It is a pin, not a reproduction. Rival: in `grid_body`'s
|
||||
walk, a drawn cell's `Error` overwrote its column's total. That is ruling (2)
|
||||
applied to every row. This test and its Share sibling fail on the total
|
||||
assertion. The other seven `column_totals` tests pass.
|
||||
|
||||
### a_share_columns_total_pools_past_a_rows_div_zero
|
||||
|
||||
improvise-b5dp, the second case the bead named. `Share = Revenue /
|
||||
SUM(Revenue)`: East's own `SUM` is 0, so its row draws `div/0`. The pooled
|
||||
total is 60/60 = **1.00**. It was green at `9948aaf5` and fails under the same
|
||||
rival as the test above.
|
||||
|
||||
### a_frame_that_reframes_and_then_places_the_cursor_leaves_the_asker_placed
|
||||
|
||||
improvise-wdmp. A browser frame that reframes the grid and then places the cursor,
|
||||
for example the records `+cat` commit `[ChangeMode, AddCategory, SetSelected]`, is
|
||||
folded by the client all at once and before any answer arrives. The host's mirror
|
||||
folds the same frame in order. `adopt_cursor_epoch` resets the mirror at the
|
||||
mutation, and the later `SetSelected` places it. The relayed `ResetCursor`
|
||||
reached the client **after** its own placement and undid it. So the client sat at
|
||||
the origin while its mirror, and every projection built from it, sat at the
|
||||
placement. That is a split, not merely the jump the ticket described. The test
|
||||
drives a real `Host` and a real `ClientState` through `fold_outgoing`, then folds
|
||||
every frame addressed to the asker. It seats the cursor off the origin first, so
|
||||
a reset that never arrived is visible. It runs two shapes: with a placement
|
||||
after the reframe (expect `(1, 0)`), and without one (expect `(0, 0)`, gvnb's
|
||||
reset). In both, the client's cursor must equal its mirror's. At the base the
|
||||
placed case failed: the client was at `(0, 0)` and the mirror at `(1, 0)`.
|
||||
|
||||
Two rivals. First, the echo disabled, which is the pre-fix behaviour: the placed
|
||||
case fails. Second, the ticket's suggested fix, which drops the asker's
|
||||
`ResetCursor` from the relay: the unplaced case fails, because the mirror was
|
||||
reset and the client never heard. That rival also fails
|
||||
`a_geometry_command_relays_the_cursor_reset_to_the_asker_too` and the
|
||||
ws-server's `the_other_document_would_also_have_a_cursor_to_invalidate`.
|
||||
|
||||
### one_grid_build_evaluates_each_fold_formula_once_for_cells_and_totals
|
||||
|
||||
**What was unguarded (improvise-0jw5).** PR #247's whole effect is
|
||||
|
||||
@@ -1526,6 +1526,59 @@ had collapsed into one column could not pass the 9 by accident. Rival:
|
||||
Injected into a full workspace run, it fails this test (45 against 9) and
|
||||
nothing else.
|
||||
|
||||
### a_spanned_scan_finds_what_a_lookup_per_member_finds
|
||||
|
||||
improvise-yb4b. `fold_stored` used to enumerate one `CellKey` per member of a
|
||||
fold and look each one up. Since #239 a column total spans every row
|
||||
category, which is 3,000 lookups for one 18k column, paid twice per keypress.
|
||||
`DataStore::spanned_values` now answers with one index-narrowed scan. This test
|
||||
compares the scan with the per-member definition it replaces, in both modes, on
|
||||
a store built to separate them. It has a record with an **extra coordinate**:
|
||||
`get` misses it and `matching_values` finds it. It has an **uncovered item**, a
|
||||
record **lacking** the spanned category, **another measure**, and another
|
||||
value of an unspanned coordinate. It also has a covered item that was never
|
||||
interned. The values are distinct powers of two, so a sum identifies the
|
||||
admitted set. The order differs on purpose: the store's order, not member
|
||||
order, as gotcha 18 names it.
|
||||
|
||||
Three faults were injected, each run against the full `improvise-core` and
|
||||
`improvise-command` suites:
|
||||
|
||||
- Exact mode loses its length check (`==` → `>=`). **Only this test fails.**
|
||||
Nothing else in either suite reaches an `Exact` fold over a record with an
|
||||
extra coordinate.
|
||||
- The span ignores coverage (`|| true`). This test fails, and so do five others
|
||||
(collapsed-group, hidden-row and CSV-agreement tests).
|
||||
- A category the store never interned is skipped instead of matching nothing.
|
||||
**Only this test fails.**
|
||||
|
||||
### a_span_finds_what_its_members_find_in_store_order
|
||||
|
||||
improvise-jq0m. `spanned_values` now chooses between two narrowings, and a
|
||||
fixed example can only ever take one of them. This property builds random
|
||||
stores over `R`, `P` and `M`. Some records lack `R`, some carry an extra `X`,
|
||||
and `r6` is a covered item the store never interned. It spans `R` and sometimes
|
||||
`P`, in both modes, and compares the result with the per-member definition,
|
||||
**in store order**: the values the members' own lookups find, read back in
|
||||
`iter_cells` order. Store and span sizes vary across cases, so both narrowings
|
||||
are taken.
|
||||
|
||||
Three faults were injected, each run against the full `improvise-core` and
|
||||
`improvise-command` suites:
|
||||
|
||||
- The per-item narrowing drops the base, so it keeps each item's pair alone.
|
||||
This test and `a_spanned_scan_finds_what_a_lookup_per_member_finds` fail.
|
||||
**Nothing in either crate's grid or command tests does.** Those fixtures never
|
||||
hold a second value of the base's coordinate within a small span.
|
||||
- The base narrowing drops the base. This test fails, along with eight
|
||||
fold and totals tests.
|
||||
- The per-item positions are not sorted after they are merged. **Only this
|
||||
test fails.** Values stay correct and only their order moves, which changes
|
||||
a Sum or Avg in the last bits.
|
||||
|
||||
The regression itself is a cost, which no test here can see. The evidence for
|
||||
it is the `grid_projection` bench's grouped cases, added for this ticket.
|
||||
|
||||
### a_memo_answers_for_the_covers_it_was_built_with
|
||||
|
||||
**What broke (improvise-pntu).** Found reviewing PR #247, and it was also
|
||||
|
||||
@@ -426,38 +426,69 @@ generator-driven proptests in `improvise-io`. What *those* cannot see is a
|
||||
checked-in document drifting out of the format, because they generate their
|
||||
inputs. This test is aimed at exactly that gap.
|
||||
|
||||
**It is deliberately a directory walk rather than a list of names**: a list is a
|
||||
**It walks the files the repository tracks, not a list of names**: a list is a
|
||||
second place to remember, and the file that gets forgotten is the one nobody
|
||||
added to it. The scope is the directories `.gitignore` un-ignores `*.improv`
|
||||
in. Everything else matching `*.improv` is somebody's working file.
|
||||
added to it. "Shipped" means "tracked", and `shipped_documents` reads it off
|
||||
the index: every `*.improv` and `*.improv.gz` path that `ls-files` lists. It does
|
||||
not include untracked files, so a working `.improv.gz` in someone's tree cannot
|
||||
fail their run. A document not yet added is walked once it is, which is the
|
||||
state CI tests.
|
||||
|
||||
**The directory list is read from `.gitignore`, not written in the test
|
||||
(improvise-qlrh).** The test used to hard-code `["examples", "web"]`, a list of
|
||||
exactly the kind the paragraph above warns against. When improvise-plcx
|
||||
un-ignored `harness/fixtures/*.improv`, nothing added that directory to the
|
||||
walk. Its two fixtures were then loaded only by the Playwright job, where a
|
||||
missing seed degrades to an empty model and surfaces as a missing-label
|
||||
failure, not a load error. `shipped_directories` now takes every
|
||||
`!<dir>/*.improv` line, so un-ignoring a directory is what enlists it.
|
||||
**How the walk got here: two steps, each closing the previous one's holes.**
|
||||
|
||||
The precondition is per directory: each derived directory must yield at least
|
||||
one document. The old whole-walk `>= 2` could not see a directory silently lost.
|
||||
- **improvise-qlrh.** The test hard-coded `["examples", "web"]`. When
|
||||
improvise-plcx un-ignored `harness/fixtures/*.improv`, nothing added that
|
||||
directory. Its two fixtures were then loaded only by the Playwright job,
|
||||
where a missing seed degrades to an empty model and fails as a missing label,
|
||||
not a load error. The fix read `!<dir>/*.improv` lines from the root
|
||||
`.gitignore`, with a per-directory precondition. Red at `705c92d`:
|
||||
`harness/fixtures/tall.improv` replaced with `{"legacy": true}` passed the old
|
||||
test.
|
||||
- **improvise-c0aj.** That `.gitignore` reading was a parser for one shape of
|
||||
one file, so four other ways of shipping a document were missed silently:
|
||||
- a single-file un-ignore (`!web/demo.improv`);
|
||||
- a `.improv.gz` anywhere, since `*.improv` never ignores it and so no
|
||||
un-ignore line names it;
|
||||
- a `!` line in a nested `.gitignore`;
|
||||
- a force-added (`add -f`) document.
|
||||
|
||||
**Red, measured at `705c92d`:** `harness/fixtures/tall.improv` was replaced with
|
||||
`{"legacy": true}`, and the old test still passed. **Rivals, each run against
|
||||
the new test:**
|
||||
Each now has its own test, which builds the shape in a scratch repository
|
||||
and asserts the walk lists the document. **All four were red** against the
|
||||
`.gitignore` walk, each failing at its `walked` assertion with its setup
|
||||
commands succeeding. Walking the index closes all four by construction and
|
||||
deletes the parser, because git has already answered what the parser
|
||||
approximated.
|
||||
|
||||
- **That same broken fixture.** Fails with "…/harness/fixtures/tall.improv does
|
||||
not load".
|
||||
- **A walk that drops every `harness` path.** Fails the per-directory
|
||||
precondition, naming `harness/fixtures`.
|
||||
- **A derivation that matches no `.gitignore` line.** Fails "…must un-ignore at
|
||||
least one *.improv directory".
|
||||
**The one new failure mode, answered: no index.** `ls-files` needs a checkout.
|
||||
In CI (`actions/checkout`) and every worktree it has one. No nix build or
|
||||
packaging step runs this test; the flake is a dev shell only. Where there is no
|
||||
index, the walk must not answer "nothing", because an empty walk passes every
|
||||
per-document assertion vacuously. So it asserts `ls-files` succeeded, and
|
||||
`a_directory_that_is_not_a_checkout_fails_loudly` pins that. The whole-walk
|
||||
precondition "at least one document" stays as the second line of defence.
|
||||
|
||||
**It matches on the full name, not on `Path::extension()`**: `.improv.gz` is the
|
||||
same document gzipped and `extension()` calls it `gz`, so matching on the
|
||||
extension alone would skip it and put back the very hole this test exists to
|
||||
close.
|
||||
**Rivals, each run against the new walk:**
|
||||
|
||||
- **The `.improv.gz` pathspec removed.** Fails `a_gzipped_document_is_walked`,
|
||||
and nothing else.
|
||||
- **A failed `ls-files` answered with an empty list instead of a panic.** Fails
|
||||
`a_directory_that_is_not_a_checkout_fails_loudly`.
|
||||
- **`harness/fixtures/tall.improv` replaced with `{"legacy": true}`.** Fails
|
||||
`every_shipped_document_loads`, naming the file.
|
||||
|
||||
**It matches on the full name, not on `Path::extension()`.** `.improv.gz` is the
|
||||
same document gzipped, and `extension()` calls it `gz`. The pathspec lists both
|
||||
suffixes for the same reason.
|
||||
|
||||
### a_single_file_un_ignore_is_walked
|
||||
|
||||
improvise-c0aj. This test is the first of four shape tests. Its siblings are
|
||||
`a_gzipped_document_is_walked`,
|
||||
`a_document_un_ignored_by_a_nested_gitignore_is_walked` and
|
||||
`a_force_added_document_is_walked`. Each builds one way of shipping a document
|
||||
in a scratch repository and asserts the walk lists it. All four were red
|
||||
against the `.gitignore` walk. The account, including the rivals, is under
|
||||
`every_shipped_document_loads` above.
|
||||
|
||||
---
|
||||
|
||||
@@ -892,3 +923,26 @@ exactly, and first asserts that hint is non-empty and pages.
|
||||
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.
|
||||
|
||||
### a_status_message_gives_way_to_the_hint_on_the_next_key
|
||||
|
||||
**What broke (improvise-cq3d).** Nothing in production ever cleared
|
||||
`status_msg`, and `draw_status` prints it instead of the derived hint. After the
|
||||
first message of a session, the TUI never drew the key hint again, in any mode.
|
||||
|
||||
**The case, in three steps, each guarding one direction.**
|
||||
1. A standing "Saved to x.improv", then `T`: in TileSelect, a mode other than
|
||||
the one the message was written in, the row must hold TileSelect's hint and
|
||||
not the stale message. This is the ticket's assertion.
|
||||
2. `Space` cycles an axis and writes a message: that message must be on the row
|
||||
after its own key. This guards against expiring too eagerly.
|
||||
3. `Esc` back to Normal: the axis message is gone and Normal's hint is drawn.
|
||||
|
||||
It drives keys through `draw.rs::press`, the function `run_tui` calls, rather
|
||||
than `App::handle_key`, because the expiry is the front end's rule.
|
||||
|
||||
**Red, and the rival.**
|
||||
- With `press` delegating straight to `handle_key` (the old behaviour), it fails
|
||||
at step 1: `" TILES Saved to x.improv …"`.
|
||||
- With the expiry moved *after* dispatch, it fails at step 2: "the axis cycle
|
||||
must say what it did", since every message is wiped before it can be drawn.
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
use crate::formula::FormulaId;
|
||||
use crate::model::category::MEASURE_CATEGORY;
|
||||
use crate::model::cell::CellKey;
|
||||
use crate::protocol::FormulaSlot;
|
||||
use crate::ui::app::AppMode;
|
||||
use crate::ui::effect::{self, Effect};
|
||||
use crate::view::RecordCol;
|
||||
@@ -1042,6 +1044,12 @@ impl Cmd for CommitAndAdvance {
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum CommitKind {
|
||||
Formula,
|
||||
/// Edit the formula the editor was opened on. Never parsed: `execute` reads
|
||||
/// it off the mode's slot, the capture made at open (improvise-km71).
|
||||
ReplaceFormula {
|
||||
id: FormulaId,
|
||||
was: String,
|
||||
},
|
||||
Category,
|
||||
/// Add an item to this category, resolved from [`AppMode::ItemAdd`] at
|
||||
/// construction so `execute` stays pure over the kind and its buffer.
|
||||
@@ -1065,10 +1073,28 @@ pub fn parse_commit_kind(arg: &str, mode: &AppMode) -> Result<CommitKind, String
|
||||
}
|
||||
|
||||
impl CommitKind {
|
||||
/// A formula commit inside an editor opened on an existing formula edits
|
||||
/// that one. Read in `execute`, never at parse — `context/code/command.md`.
|
||||
fn in_mode(&self, mode: &AppMode) -> CommitKind {
|
||||
match (self, mode) {
|
||||
(
|
||||
Self::Formula,
|
||||
AppMode::FormulaEdit {
|
||||
slot: FormulaSlot::Existing { id, was },
|
||||
..
|
||||
},
|
||||
) => Self::ReplaceFormula {
|
||||
id: id.clone(),
|
||||
was: was.clone(),
|
||||
},
|
||||
(kind, _) => kind.clone(),
|
||||
}
|
||||
}
|
||||
|
||||
/// The minibuffer this kind consumes.
|
||||
pub fn buffer(&self) -> &'static str {
|
||||
match self {
|
||||
Self::Formula => "formula",
|
||||
Self::Formula | Self::ReplaceFormula { .. } => "formula",
|
||||
Self::Category => "category",
|
||||
Self::Item { .. } => "item",
|
||||
}
|
||||
@@ -1078,7 +1104,7 @@ impl CommitKind {
|
||||
/// minibuffer.
|
||||
fn on_empty(&self) -> AppMode {
|
||||
match self {
|
||||
Self::Formula => AppMode::FormulaPanel,
|
||||
Self::Formula | Self::ReplaceFormula { .. } => AppMode::FormulaPanel,
|
||||
Self::Category | Self::Item { .. } => AppMode::CategoryPanel,
|
||||
}
|
||||
}
|
||||
@@ -1087,7 +1113,7 @@ impl CommitKind {
|
||||
/// categories and items are entered in batches, formulas are not.
|
||||
fn on_commit(&self) -> Option<AppMode> {
|
||||
match self {
|
||||
Self::Formula => Some(AppMode::FormulaPanel),
|
||||
Self::Formula | Self::ReplaceFormula { .. } => Some(AppMode::FormulaPanel),
|
||||
Self::Category | Self::Item { .. } => None,
|
||||
}
|
||||
}
|
||||
@@ -1095,6 +1121,7 @@ impl CommitKind {
|
||||
fn status(&self, name: &str) -> String {
|
||||
match self {
|
||||
Self::Formula => "Formula added".to_string(),
|
||||
Self::ReplaceFormula { .. } => "Formula updated".to_string(),
|
||||
Self::Category => format!("Added category \"{name}\""),
|
||||
Self::Item { .. } => format!("Added \"{name}\""),
|
||||
}
|
||||
@@ -1108,6 +1135,11 @@ impl CommitKind {
|
||||
raw: name.to_string(),
|
||||
target_category: "_Measure".to_string(),
|
||||
}),
|
||||
Self::ReplaceFormula { id, was } => Box::new(effect::ReplaceFormula {
|
||||
id: id.clone(),
|
||||
was: was.clone(),
|
||||
raw: name.to_string(),
|
||||
}),
|
||||
Self::Category => Box::new(effect::AddCategory(name.to_string())),
|
||||
Self::Item { category } => Box::new(effect::AddItem {
|
||||
category: category.clone(),
|
||||
@@ -1128,18 +1160,19 @@ impl Cmd for CommitBuffer {
|
||||
"commit-buffer"
|
||||
}
|
||||
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
let raw = read_buffer(ctx, self.kind.buffer());
|
||||
let kind = self.kind.in_mode(ctx.mode);
|
||||
let raw = read_buffer(ctx, kind.buffer());
|
||||
let name = raw.trim();
|
||||
if name.is_empty() {
|
||||
return vec![effect::change_mode(self.kind.on_empty())];
|
||||
return vec![effect::change_mode(kind.on_empty())];
|
||||
}
|
||||
let mut effects: Vec<Box<dyn Effect>> = vec![
|
||||
effect::set_status(self.kind.status(name)),
|
||||
effect::set_status(kind.status(name)),
|
||||
// The add goes last: it is the only party that knows whether it worked,
|
||||
// so the optimistic status must not clobber its error (improvise-6t3).
|
||||
self.kind.add_effect(name),
|
||||
kind.add_effect(name),
|
||||
];
|
||||
effects.extend(self.kind.on_commit().map(effect::change_mode));
|
||||
effects.extend(kind.on_commit().map(effect::change_mode));
|
||||
effects
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,7 +4,7 @@ use crate::ui::app::AppMode;
|
||||
use crate::ui::effect::{self, Effect, Panel, ViewTarget};
|
||||
use crate::view::Axis;
|
||||
|
||||
use super::core::{Cmd, CmdContext};
|
||||
use super::core::{Cmd, CmdContext, write_buffer};
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
@@ -685,6 +685,25 @@ impl Cmd for DeleteFormulaAtCursor {
|
||||
}
|
||||
}
|
||||
|
||||
/// Open the formula editor on the formula under the cursor, seeded with its
|
||||
/// text and naming it from here on — the commit never re-reads the cursor.
|
||||
#[derive(Debug)]
|
||||
pub struct EditFormulaAtCursor;
|
||||
impl Cmd for EditFormulaAtCursor {
|
||||
fn name(&self) -> &'static str {
|
||||
"edit-formula-at-cursor"
|
||||
}
|
||||
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
match ctx.model.formulas().get(ctx.formula_cursor) {
|
||||
Some(f) => vec![
|
||||
effect::change_mode(AppMode::formula_replace(f.id(), f.raw.clone())),
|
||||
write_buffer("formula", f.raw.clone()),
|
||||
],
|
||||
None => vec![],
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ── Category panel commands ─────────────────────────────────────────────────
|
||||
|
||||
/// Cycle the axis assignment of the category at the cursor.
|
||||
|
||||
@@ -382,6 +382,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
None,
|
||||
);
|
||||
r.register_nullary(|| Box::new(DeleteFormulaAtCursor));
|
||||
r.register_nullary(|| Box::new(EditFormulaAtCursor));
|
||||
r.register_nullary(|| Box::new(AddRecordRow));
|
||||
r.register_nullary(|| Box::new(TogglePruneEmpty));
|
||||
r.register_nullary(|| Box::new(ToggleRecordsMode));
|
||||
|
||||
@@ -587,7 +587,20 @@ impl App {
|
||||
f.target
|
||||
)
|
||||
});
|
||||
derived.or_else(|| self.aggregate_refusal(cell.record, &key))
|
||||
let refusal = self.group_refusal(&key).or(derived);
|
||||
refusal.or_else(|| self.aggregate_refusal(cell.record, &key))
|
||||
}
|
||||
|
||||
/// A coordinate naming a group rather than an item is no cell in ANY view, so
|
||||
/// this asks the document, never the layout (improvise-b114).
|
||||
fn group_refusal(&self, key: &CellKey) -> Option<String> {
|
||||
let model = &self.model_state.workbook.model;
|
||||
key.0.iter().find_map(|(cat, name)| {
|
||||
let category = model.category(cat)?;
|
||||
let group = !category.items.contains_key(name.as_str())
|
||||
&& !category.items_in_group(name).is_empty();
|
||||
group.then(|| format!("{name} is a group of {cat}, so there is no one cell to write"))
|
||||
})
|
||||
}
|
||||
|
||||
/// A record the store does not hold is known only by its layout position, and
|
||||
@@ -4864,3 +4877,261 @@ mod basis_tiers {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Editing a formula from the panel (improvise-km71), driven through
|
||||
/// `handle_key` at a NONZERO cursor over three formulas.
|
||||
#[cfg(test)]
|
||||
mod formula_edit_tests {
|
||||
use super::*;
|
||||
use crate::formula::parse_formula;
|
||||
|
||||
const PROFIT: &str = "Profit = Revenue - Cost";
|
||||
const TAX: &str = "Tax = Revenue * 0.1";
|
||||
const MARGIN: &str = "Margin = Profit / Revenue";
|
||||
|
||||
fn app_with_three_formulas() -> App {
|
||||
let mut wb = Workbook::new("T");
|
||||
wb.add_item("_Measure", "Revenue").unwrap();
|
||||
wb.add_item("_Measure", "Cost").unwrap();
|
||||
for raw in [PROFIT, TAX, MARGIN] {
|
||||
wb.model
|
||||
.add_formula(parse_formula(raw, "_Measure").unwrap());
|
||||
}
|
||||
let mut app = App::new(wb, None);
|
||||
app.view_state.formula_panel_open = true;
|
||||
app.view_state.mode = AppMode::FormulaPanel;
|
||||
app.view_state.formula_cursor = 1;
|
||||
app
|
||||
}
|
||||
|
||||
fn press(app: &mut App, code: KeyCode) {
|
||||
app.handle_key(KeyEvent::new(code, KeyModifiers::NONE))
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
/// Clear the minibuffer by keys and type `text`, as a user would.
|
||||
fn retype(app: &mut App, text: &str) {
|
||||
let len = app.view_state.buffers.get("formula").map_or(0, String::len);
|
||||
for _ in 0..len {
|
||||
press(app, KeyCode::Backspace);
|
||||
}
|
||||
for c in text.chars() {
|
||||
press(app, KeyCode::Char(c));
|
||||
}
|
||||
}
|
||||
|
||||
fn raws(app: &App) -> Vec<String> {
|
||||
app.model_state
|
||||
.workbook
|
||||
.model
|
||||
.formulas()
|
||||
.iter()
|
||||
.map(|f| f.raw.clone())
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn buffer(app: &App) -> Option<&str> {
|
||||
app.view_state.buffers.get("formula").map(String::as_str)
|
||||
}
|
||||
|
||||
fn open_editor(app: &mut App) {
|
||||
press(app, KeyCode::Enter);
|
||||
assert!(
|
||||
matches!(app.view_state.mode, AppMode::FormulaEdit { .. }),
|
||||
"Enter on a formula must open the editor, got {:?}",
|
||||
app.view_state.mode
|
||||
);
|
||||
}
|
||||
|
||||
fn in_editor(app: &App) -> bool {
|
||||
matches!(app.view_state.mode, AppMode::FormulaEdit { .. })
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn enter_opens_the_editor_seeded_with_the_formula_under_the_cursor() {
|
||||
let mut app = app_with_three_formulas();
|
||||
open_editor(&mut app);
|
||||
assert_eq!(buffer(&app), Some(TAX), "cursor 1 is Tax, not the first");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn e_opens_the_same_editor() {
|
||||
let mut app = app_with_three_formulas();
|
||||
press(&mut app, KeyCode::Char('e'));
|
||||
assert!(in_editor(&app), "{:?}", app.view_state.mode);
|
||||
assert_eq!(buffer(&app), Some(TAX));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn committing_an_edit_replaces_the_formula_in_place() {
|
||||
let mut app = app_with_three_formulas();
|
||||
open_editor(&mut app);
|
||||
retype(&mut app, "Tax = Revenue * 0.2");
|
||||
press(&mut app, KeyCode::Enter);
|
||||
assert_eq!(raws(&app), vec![PROFIT, "Tax = Revenue * 0.2", MARGIN]);
|
||||
assert_eq!(app.view_state.mode, AppMode::FormulaPanel);
|
||||
}
|
||||
|
||||
/// The rival "add instead of replace" passes the test above, because
|
||||
/// `add_formula` replaces by target. A rename is where it cannot hide.
|
||||
#[test]
|
||||
fn changing_the_target_renames_the_formula_in_its_own_place() {
|
||||
let mut app = app_with_three_formulas();
|
||||
open_editor(&mut app);
|
||||
retype(&mut app, "Levy = Revenue * 0.1");
|
||||
press(&mut app, KeyCode::Enter);
|
||||
assert_eq!(raws(&app), vec![PROFIT, "Levy = Revenue * 0.1", MARGIN]);
|
||||
let measures = app.model_state.workbook.model.measure_item_names();
|
||||
assert!(
|
||||
!measures.contains(&"Tax".to_string()),
|
||||
"Tax was a formula target only, so it goes: {measures:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_rename_says_which_formulas_still_name_the_old_target() {
|
||||
let mut app = app_with_three_formulas();
|
||||
app.view_state.formula_cursor = 0;
|
||||
open_editor(&mut app);
|
||||
retype(&mut app, "Gross = Revenue - Cost");
|
||||
press(&mut app, KeyCode::Enter);
|
||||
assert_eq!(raws(&app)[0], "Gross = Revenue - Cost");
|
||||
let status = &app.view_state.status_msg;
|
||||
assert!(
|
||||
status.contains("Profit") && status.contains("Margin"),
|
||||
"Margin still refers to Profit, and the user must be told: {status:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_rename_onto_another_formulas_target_refuses_and_keeps_both() {
|
||||
let mut app = app_with_three_formulas();
|
||||
open_editor(&mut app);
|
||||
retype(&mut app, "Margin = Revenue * 0.1");
|
||||
press(&mut app, KeyCode::Enter);
|
||||
assert_eq!(raws(&app), vec![PROFIT, TAX, MARGIN]);
|
||||
assert!(in_editor(&app), "{:?}", app.view_state.mode);
|
||||
assert!(app.view_state.status_msg.contains("Margin"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_parse_error_keeps_the_editor_the_text_and_the_formula() {
|
||||
let mut app = app_with_three_formulas();
|
||||
open_editor(&mut app);
|
||||
retype(&mut app, "Tax = = ");
|
||||
press(&mut app, KeyCode::Enter);
|
||||
assert_eq!(raws(&app), vec![PROFIT, TAX, MARGIN]);
|
||||
assert!(in_editor(&app), "{:?}", app.view_state.mode);
|
||||
assert_eq!(buffer(&app), Some("Tax = = "));
|
||||
assert!(
|
||||
app.view_state.status_msg.contains("Formula error"),
|
||||
"{:?}",
|
||||
app.view_state.status_msg
|
||||
);
|
||||
}
|
||||
|
||||
/// The editor names its formula when it OPENS. Deleting an earlier one
|
||||
/// moves the edited formula to index 0 and puts `Margin` under cursor 1.
|
||||
#[test]
|
||||
fn an_edit_lands_on_its_formula_after_an_earlier_one_is_deleted_meanwhile() {
|
||||
let mut app = app_with_three_formulas();
|
||||
open_editor(&mut app);
|
||||
retype(&mut app, "Tax = Revenue * 0.2");
|
||||
let model = &mut app.model_state.workbook.model;
|
||||
model.remove_formula("Profit", "_Measure");
|
||||
press(&mut app, KeyCode::Enter);
|
||||
assert_eq!(raws(&app), vec!["Tax = Revenue * 0.2", MARGIN]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_edit_whose_formula_was_deleted_meanwhile_refuses_out_loud() {
|
||||
let mut app = app_with_three_formulas();
|
||||
open_editor(&mut app);
|
||||
retype(&mut app, "Tax = Revenue * 0.2");
|
||||
let model = &mut app.model_state.workbook.model;
|
||||
model.remove_formula("Tax", "_Measure");
|
||||
press(&mut app, KeyCode::Enter);
|
||||
assert_eq!(raws(&app), vec![PROFIT, MARGIN], "nothing re-added");
|
||||
assert!(in_editor(&app), "{:?}", app.view_state.mode);
|
||||
assert_eq!(buffer(&app), Some("Tax = Revenue * 0.2"));
|
||||
assert!(app.last_batch_refused());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_edit_whose_formula_was_changed_meanwhile_refuses_out_loud() {
|
||||
let mut app = app_with_three_formulas();
|
||||
open_editor(&mut app);
|
||||
retype(&mut app, "Tax = Revenue * 0.2");
|
||||
let theirs = parse_formula("Tax = Revenue * 0.3", "_Measure").unwrap();
|
||||
app.model_state.workbook.model.add_formula(theirs);
|
||||
press(&mut app, KeyCode::Enter);
|
||||
assert_eq!(raws(&app), vec![PROFIT, "Tax = Revenue * 0.3", MARGIN]);
|
||||
assert!(in_editor(&app), "{:?}", app.view_state.mode);
|
||||
assert!(app.last_batch_refused());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn esc_leaves_the_formula_untouched() {
|
||||
let mut app = app_with_three_formulas();
|
||||
open_editor(&mut app);
|
||||
retype(&mut app, "Tax = 0");
|
||||
press(&mut app, KeyCode::Esc);
|
||||
assert_eq!(raws(&app), vec![PROFIT, TAX, MARGIN]);
|
||||
assert_eq!(app.view_state.mode, AppMode::FormulaPanel);
|
||||
}
|
||||
|
||||
/// `n` still adds: the replace slot must not leak into a fresh editor.
|
||||
#[test]
|
||||
fn a_new_formula_after_an_edit_is_added_not_replaced() {
|
||||
let mut app = app_with_three_formulas();
|
||||
open_editor(&mut app);
|
||||
press(&mut app, KeyCode::Esc);
|
||||
press(&mut app, KeyCode::Char('n'));
|
||||
assert_eq!(buffer(&app), Some(""), "a new formula starts empty");
|
||||
retype(&mut app, "Levy = Revenue * 0.3");
|
||||
press(&mut app, KeyCode::Enter);
|
||||
assert_eq!(
|
||||
raws(&app),
|
||||
vec![PROFIT, TAX, MARGIN, "Levy = Revenue * 0.3"]
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-kb1d: a Region-targeted formula at index 1. Renaming it would
|
||||
/// strand `Total` as an empty item, so the edit refuses and stays open.
|
||||
#[test]
|
||||
fn renaming_a_formula_outside_measure_refuses_and_keeps_the_editor() {
|
||||
let mut wb = Workbook::new("T");
|
||||
wb.add_item("_Measure", "Revenue").unwrap();
|
||||
wb.add_item("_Measure", "Cost").unwrap();
|
||||
wb.add_category("Region").unwrap();
|
||||
wb.add_item("Region", "East").unwrap();
|
||||
wb.add_item("Region", "West").unwrap();
|
||||
for (raw, cat) in [
|
||||
(PROFIT, "_Measure"),
|
||||
("Total = East + West", "Region"),
|
||||
(MARGIN, "_Measure"),
|
||||
] {
|
||||
wb.model.add_formula(parse_formula(raw, cat).unwrap());
|
||||
}
|
||||
let mut app = App::new(wb, None);
|
||||
app.view_state.formula_panel_open = true;
|
||||
app.view_state.mode = AppMode::FormulaPanel;
|
||||
app.view_state.formula_cursor = 1;
|
||||
let regions = |app: &App| {
|
||||
let region = app.model_state.workbook.model.category("Region").unwrap();
|
||||
region
|
||||
.ordered_item_names()
|
||||
.iter()
|
||||
.map(|s| s.to_string())
|
||||
.collect::<Vec<_>>()
|
||||
};
|
||||
let before = regions(&app);
|
||||
open_editor(&mut app);
|
||||
retype(&mut app, "Sum = East + West");
|
||||
press(&mut app, KeyCode::Enter);
|
||||
assert_eq!(raws(&app), vec![PROFIT, "Total = East + West", MARGIN]);
|
||||
assert_eq!(regions(&app), before, "no ghost item, nothing appended");
|
||||
assert!(in_editor(&app), "{:?}", app.view_state.mode);
|
||||
assert_eq!(buffer(&app), Some("Sum = East + West"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,6 +2,8 @@ use std::collections::BTreeMap;
|
||||
use std::fmt::Debug;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use crate::formula::FormulaId;
|
||||
use crate::model::Model;
|
||||
use crate::model::cell::{CellKey, CellValue};
|
||||
use crate::view::{Axis, RecordCol, RecordRow};
|
||||
|
||||
@@ -246,6 +248,61 @@ impl Effect for RemoveFormula {
|
||||
}
|
||||
}
|
||||
|
||||
/// Edit the formula `id` names in place. Every refusal stops the batch, so the
|
||||
/// editor stays open over the user's text — `context/code/command.md`.
|
||||
#[derive(Debug)]
|
||||
pub struct ReplaceFormula {
|
||||
pub id: FormulaId,
|
||||
pub was: String,
|
||||
pub raw: String,
|
||||
}
|
||||
impl Effect for ReplaceFormula {
|
||||
fn apply(&self, app: &mut App) {
|
||||
match self.replace(&mut app.model_state.workbook.model) {
|
||||
Ok(Some(status)) => app.view_state.status_msg = status,
|
||||
Ok(None) => {}
|
||||
Err(why) => refuse_and_stop(app, why),
|
||||
}
|
||||
}
|
||||
fn kind(&self) -> EffectKind {
|
||||
EffectKind::Model(ModelChange::CellData)
|
||||
}
|
||||
}
|
||||
|
||||
impl ReplaceFormula {
|
||||
/// The replacement, and what a rename leaves naming the old target.
|
||||
fn replace(&self, model: &mut Model) -> Result<Option<String>, String> {
|
||||
let old = model
|
||||
.formula(&self.id)
|
||||
.filter(|f| f.raw == self.was)
|
||||
.ok_or_else(|| format!("\"{}\" was changed or removed meanwhile", self.was))?;
|
||||
let old_target = old.target.clone();
|
||||
let new = crate::formula::parse_formula(&self.raw, &old.target_category)
|
||||
.map_err(|e| format!("Formula error: {e}"))?;
|
||||
let new_target = new.target.clone();
|
||||
model
|
||||
.replace_formula(&self.id, new)
|
||||
.map_err(|e| e.to_string())?;
|
||||
Ok((new_target != old_target).then(|| renamed(model, &old_target, &new_target)))
|
||||
}
|
||||
}
|
||||
|
||||
fn renamed(model: &Model, old: &str, new: &str) -> String {
|
||||
let still: Vec<&str> = model
|
||||
.formulas()
|
||||
.iter()
|
||||
.filter(|f| f.mentions(old))
|
||||
.map(|f| f.target.as_str())
|
||||
.collect();
|
||||
match still.is_empty() {
|
||||
true => format!("Renamed {old} to {new}"),
|
||||
false => format!(
|
||||
"Renamed {old} to {new}; {old} is still used by {}",
|
||||
still.join(", ")
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
/// Refresh the derived grid layout mid-batch: `apply_effects` rebuilds only once
|
||||
/// after the batch, so an effect that *reads* it needs this first (improvise-ehz).
|
||||
#[derive(Debug)]
|
||||
|
||||
@@ -162,6 +162,7 @@ impl Host {
|
||||
let undo = self.app.snapshot();
|
||||
let mirrors = self.sessions.clone();
|
||||
let mut out = Vec::new();
|
||||
let mut asker_reset = false;
|
||||
for cmd in batch {
|
||||
if let Some(reason) = reduce::broken_chain(&self.app, cmd.command()) {
|
||||
return self.undo_frame(id, undo, mirrors, reason);
|
||||
@@ -169,7 +170,23 @@ impl Host {
|
||||
// Only a mutation runs `apply_effects`, which resets both flags — so
|
||||
// after anything else they are STALE. A refusal need not abort (cz8d).
|
||||
let mutation = matches!(cmd.command(), Command::Model { .. });
|
||||
out.extend(self.command(id, cmd));
|
||||
let command = cmd.command();
|
||||
let mut answered = self.command(id, cmd);
|
||||
// The asker folded this placement BEFORE hearing its reset, so it is owed
|
||||
// again after it (improvise-wdmp, `context/code/command.md`).
|
||||
if asker_reset && places_cursor(command) {
|
||||
answered.insert(
|
||||
0,
|
||||
Addressed {
|
||||
to: id,
|
||||
msg: ServerFrame::Commands(vec![command.clone()]),
|
||||
},
|
||||
);
|
||||
}
|
||||
asker_reset |= answered
|
||||
.iter()
|
||||
.any(|a| a.to == id && is_cursor_reset(&a.msg));
|
||||
out.extend(answered);
|
||||
let failed = self.app.last_batch_refused() || self.app.last_batch_aborted();
|
||||
if mutation && failed {
|
||||
return self.undo_frame(id, undo, mirrors, Rejection::MutationRefused);
|
||||
@@ -266,6 +283,19 @@ impl Host {
|
||||
}
|
||||
}
|
||||
|
||||
/// A command that writes this client's cursor, and so survives a reset only if
|
||||
/// it is folded after it.
|
||||
fn places_cursor(cmd: &Command) -> bool {
|
||||
matches!(
|
||||
cmd,
|
||||
Command::SetSelected { .. } | Command::SetRowOffset(_) | Command::SetColOffset(_)
|
||||
)
|
||||
}
|
||||
|
||||
fn is_cursor_reset(msg: &ServerFrame) -> bool {
|
||||
matches!(msg, ServerFrame::Commands(cmds) if cmds.contains(&Command::ResetCursor))
|
||||
}
|
||||
|
||||
/// 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)]
|
||||
@@ -1576,6 +1606,58 @@ mod tests {
|
||||
assert_eq!(model.get_cell(&target), Some(&CellValue::Number(7.0)));
|
||||
}
|
||||
|
||||
/// improvise-b114: a coordinate naming a group is no item, in any view — see
|
||||
/// context/test-evidence/command.md.
|
||||
#[test]
|
||||
fn a_wire_write_naming_a_measure_group_is_rejected_in_any_view() {
|
||||
for collapsed in [true, false] {
|
||||
let mut wb = Workbook::new("T");
|
||||
wb.add_category("Month").unwrap();
|
||||
for month in ["Jan", "Feb"] {
|
||||
wb.add_item("Month", month).unwrap();
|
||||
for measure in ["Revenue", "Cost"] {
|
||||
wb.add_item_in_group("_Measure", measure, "Money").unwrap();
|
||||
let key = month_cell(month, measure);
|
||||
wb.model.set_cell(key, CellValue::Number(10.0));
|
||||
}
|
||||
}
|
||||
wb.add_item("_Measure", "Units").unwrap();
|
||||
wb.active_view_mut().set_axis("Month", Axis::Row);
|
||||
wb.active_view_mut().set_axis("_Measure", Axis::Column);
|
||||
if collapsed {
|
||||
wb.active_view_mut()
|
||||
.toggle_group_collapse("_Measure", "Money");
|
||||
}
|
||||
let mut app = App::new(wb, None);
|
||||
app.rebuild_layout();
|
||||
let mut host = Host::new(app);
|
||||
let before = document_text(&host);
|
||||
let answer = wire_write(&mut host, &month_cell("Feb", "Money"), false);
|
||||
assert!(refused(&answer), "collapsed={collapsed}: {answer:?}");
|
||||
let after = document_text(&host);
|
||||
assert_eq!(after, before, "collapsed={collapsed}: a group was stored");
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-xur5 (the reviewer's `probe_fold_hidden_measure`): a fold's record
|
||||
/// with a measure the host draws nowhere — see context/test-evidence/command.md.
|
||||
#[test]
|
||||
fn a_wire_write_to_a_fold_record_with_an_undrawn_measure_is_rejected() {
|
||||
let hidden = month_cell("Q2", "Cost");
|
||||
let bogus = month_cell("Q2", "Bogus");
|
||||
let bare = CellKey::new(vec![("Month".to_string(), "Q2".to_string())]);
|
||||
for key in [hidden, bogus, bare] {
|
||||
let mut host = host_on_a_fold();
|
||||
let view = host.app.model_state.workbook.active_view_mut();
|
||||
view.hide_item("_Measure", "Cost");
|
||||
host.app.rebuild_layout();
|
||||
let before = document_text(&host);
|
||||
let answer = wire_write(&mut host, &key, false);
|
||||
assert!(refused(&answer), "{key:?} accepted: {answer:?}");
|
||||
assert_eq!(document_text(&host), before, "{key:?} stored a group");
|
||||
}
|
||||
}
|
||||
|
||||
/// A STORED record is writable whatever the host's view does to it — see
|
||||
/// context/test-evidence/command.md (improvise-sjdb).
|
||||
#[test]
|
||||
@@ -1706,6 +1788,71 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-wdmp: the asker's own frame placed its cursor after the reframe,
|
||||
/// and the relayed reset undid it. Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn a_frame_that_reframes_and_then_places_the_cursor_leaves_the_asker_placed() {
|
||||
use improvise_protocol::{AppMode, ClientState};
|
||||
|
||||
// With a placement after the reframe, and without one: the asker ends where
|
||||
// the TUI would, and its client agrees with its mirror either way.
|
||||
for (placed, expected) in [(true, (1, 0)), (false, (0, 0))] {
|
||||
let mut host = host();
|
||||
let (asker, snapshot) = host.connect(10, 10);
|
||||
let (bystander, _) = host.connect(10, 10);
|
||||
let mut client = ClientState::default();
|
||||
match snapshot.msg {
|
||||
ServerFrame::Snapshot(s) => client.bootstrap(*s),
|
||||
other => panic!("setup: connect answers a Snapshot, got {other:?}"),
|
||||
}
|
||||
let send = |host: &mut Host, client: &mut ClientState, frame: Vec<Command>| {
|
||||
client.fold_outgoing(&frame);
|
||||
let answer = host.handle(asker, ClientFrame::Commands(frame));
|
||||
for msg in to(&answer, asker) {
|
||||
match msg {
|
||||
ServerFrame::Commands(cmds) => cmds.iter().for_each(|c| client.fold(c)),
|
||||
ServerFrame::Accepted => client.accepted(),
|
||||
other => panic!("the frame must be accepted: {other:?}"),
|
||||
}
|
||||
}
|
||||
answer
|
||||
};
|
||||
// Off the origin first, so a reset that never arrived is visible.
|
||||
send(
|
||||
&mut host,
|
||||
&mut client,
|
||||
vec![Command::SetSelected { row: 1, col: 0 }],
|
||||
);
|
||||
|
||||
// The `+cat` commit's shape: tidy-up, the schema change, then (maybe)
|
||||
// the cursor put back where the user was (resolve.rs::create_column).
|
||||
let add = ModelCommand::AddCategory("Product".into());
|
||||
let mut frame = vec![
|
||||
Command::ChangeMode(AppMode::Normal),
|
||||
crate::ui::reduce::caught_up(host.app(), add),
|
||||
];
|
||||
if placed {
|
||||
frame.push(Command::SetSelected { row: 1, col: 0 });
|
||||
}
|
||||
let answer = send(&mut host, &mut client, frame);
|
||||
assert!(
|
||||
to(&answer, bystander).contains(&ServerFrame::Commands(vec![Command::ResetCursor])),
|
||||
"setup: the frame must really reframe, or nothing here is at stake"
|
||||
);
|
||||
|
||||
let mirror = host.sessions.get(asker).unwrap().view().cursor();
|
||||
assert_eq!(
|
||||
mirror.selected, expected,
|
||||
"setup, placed={placed}: the mirror"
|
||||
);
|
||||
assert_eq!(
|
||||
client.view.cursor(),
|
||||
mirror,
|
||||
"placed={placed}: the asker's client and its mirror disagree"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// A refused batch is answered to the **asker and nobody else** (ADR 001):
|
||||
/// nothing happened, so there is nothing to tell the others.
|
||||
#[test]
|
||||
|
||||
@@ -15,9 +15,9 @@ use improvise_io::import::analyzer::{DateComponent, FieldKind};
|
||||
use improvise_io::import::wizard::{ImportWizard, WizardStep};
|
||||
use improvise_protocol::{
|
||||
AxisFrame, Basis, BasisCache, CategoryRow, CategoryTreeCache, CategoryTreeUpdate, CellDisplay,
|
||||
CellId, Command, CreateKind, DocumentCache, FormulaCache, FrameEntry, FrameGroup, GridCache,
|
||||
GridExtent, GridUpdate, Projection, RecordCell, Rejection, Snapshot, TileBarCache, TileEntry,
|
||||
ViewListCache, ViewState, ViewSummary, WidgetCache, WizardCache, WizardDateField,
|
||||
CellId, Command, CreateKind, DocumentCache, FormulaCache, FormulaEntry, FrameEntry, FrameGroup,
|
||||
GridCache, GridExtent, GridUpdate, Projection, RecordCell, Rejection, Snapshot, TileBarCache,
|
||||
TileEntry, ViewListCache, ViewState, ViewSummary, WidgetCache, WizardCache, WizardDateField,
|
||||
WizardFieldKind, WizardProposal, WizardStepKind, basis, reduce_view,
|
||||
};
|
||||
|
||||
@@ -386,14 +386,15 @@ pub fn category_rows(model: &Model, view: &View) -> Vec<CategoryRow> {
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// The raw text of every formula, which is all the formula panel draws.
|
||||
pub fn formula_texts(app: &App) -> Vec<String> {
|
||||
/// Every formula's text, which the panel draws, and its identity, which an
|
||||
/// edit names it by (improvise-km71).
|
||||
pub fn formula_entries(app: &App) -> Vec<FormulaEntry> {
|
||||
app.model_state
|
||||
.workbook
|
||||
.model
|
||||
.formulas()
|
||||
.iter()
|
||||
.map(|f| f.raw.clone())
|
||||
.map(FormulaEntry::from)
|
||||
.collect()
|
||||
}
|
||||
|
||||
@@ -450,10 +451,10 @@ pub fn category_tree_in(app: &App, view: &View) -> CategoryTreeCache {
|
||||
}
|
||||
}
|
||||
|
||||
/// The formula panel's cache: the raw text of every formula.
|
||||
/// The formula panel's cache: every formula's text and identity.
|
||||
pub fn formulas_of(app: &App) -> FormulaCache {
|
||||
FormulaCache {
|
||||
formulas: formula_texts(app),
|
||||
formulas: formula_entries(app),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1837,8 +1838,13 @@ mod tests {
|
||||
client.fold(&projections);
|
||||
}
|
||||
assert_eq!(
|
||||
client.formulas.formulas,
|
||||
vec!["Total = Revenue".to_string()],
|
||||
client
|
||||
.formulas
|
||||
.formulas
|
||||
.iter()
|
||||
.map(|f| f.raw.as_str())
|
||||
.collect::<Vec<_>>(),
|
||||
vec!["Total = Revenue"],
|
||||
"the client must be holding the formula before it can lose it"
|
||||
);
|
||||
|
||||
@@ -1852,7 +1858,7 @@ mod tests {
|
||||
);
|
||||
assert_eq!(
|
||||
client.formulas.formulas,
|
||||
Vec::<String>::new(),
|
||||
Vec::<FormulaEntry>::new(),
|
||||
"the client is still showing a formula the document no longer has"
|
||||
);
|
||||
}
|
||||
@@ -1877,8 +1883,13 @@ mod tests {
|
||||
let mut client = ClientCaches::default();
|
||||
client.fold(&snapshot.seed);
|
||||
assert_eq!(
|
||||
client.formulas.formulas,
|
||||
vec!["Total = Revenue".to_string()],
|
||||
client
|
||||
.formulas
|
||||
.formulas
|
||||
.iter()
|
||||
.map(|f| f.raw.as_str())
|
||||
.collect::<Vec<_>>(),
|
||||
vec!["Total = Revenue"],
|
||||
"the client must be holding the formula before it can lose it"
|
||||
);
|
||||
|
||||
@@ -1901,7 +1912,7 @@ mod tests {
|
||||
);
|
||||
assert_eq!(
|
||||
client.formulas.formulas,
|
||||
Vec::<String>::new(),
|
||||
Vec::<FormulaEntry>::new(),
|
||||
"the client is still showing a formula the document no longer has"
|
||||
);
|
||||
}
|
||||
@@ -6022,4 +6033,60 @@ mod column_totals {
|
||||
assert_eq!(total(book(false), "Revenue").text, "7.00");
|
||||
assert_eq!(total(book(true), "Revenue").text, "7.00");
|
||||
}
|
||||
|
||||
/// The column's grid and its index, for a test that reads rows and total together.
|
||||
fn column(wb: Workbook, measure: &str) -> (GridCache, usize) {
|
||||
let mut app = App::new(wb, None);
|
||||
app.rebuild_layout();
|
||||
let col = (0..app.layout.col_count())
|
||||
.find(|&c| app.layout.col_label(c) == measure)
|
||||
.unwrap_or_else(|| panic!("no column {measure}"));
|
||||
(grid_of(&app, WHOLE), col)
|
||||
}
|
||||
|
||||
/// North 25/30, South 27/30, East -4/0: the rows draw 0.83, 0.90, div/0.
|
||||
fn margins_with_a_zero_revenue_row() -> Workbook {
|
||||
money(&[
|
||||
("North", 30.0, 5.0),
|
||||
("South", 30.0, 3.0),
|
||||
("East", 0.0, 4.0),
|
||||
])
|
||||
}
|
||||
|
||||
/// improvise-b5dp, user ruling: a formula total is the pooled ratio, so a row's
|
||||
/// div/0 does not poison it. context/test-evidence/command.md.
|
||||
#[test]
|
||||
fn a_formula_rows_error_does_not_poison_the_pooled_total() {
|
||||
let mut wb = margins_with_a_zero_revenue_row();
|
||||
for f in ["Profit = Revenue - Cost", "Margin = Profit / Revenue"] {
|
||||
let formula = crate::formula::parse_formula(f, "_Measure").unwrap();
|
||||
wb.model.add_formula(formula);
|
||||
}
|
||||
let (grid, col) = column(wb, "Margin");
|
||||
let rows: Vec<bool> = grid.cells.iter().map(|r| r[col].error).collect();
|
||||
assert_eq!(rows, vec![false, false, true], "East's row must draw div/0");
|
||||
let t = &grid.totals[col];
|
||||
assert_eq!(
|
||||
(t.text.as_str(), t.error),
|
||||
("0.80", false),
|
||||
"48 / 60, pooled"
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-b5dp: the same ruling for a formula whose own aggregate is zero
|
||||
/// at one row. context/test-evidence/command.md.
|
||||
#[test]
|
||||
fn a_share_columns_total_pools_past_a_rows_div_zero() {
|
||||
let mut wb = margins_with_a_zero_revenue_row();
|
||||
let share = crate::formula::parse_formula("Share = Revenue / SUM(Revenue)", "_Measure");
|
||||
wb.model.add_formula(share.unwrap());
|
||||
let (grid, col) = column(wb, "Share");
|
||||
assert!(grid.cells[2][col].error, "East's row must draw div/0");
|
||||
let t = &grid.totals[col];
|
||||
assert_eq!(
|
||||
(t.text.as_str(), t.error),
|
||||
("1.00", false),
|
||||
"60 / 60, pooled"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -180,6 +180,11 @@ pub fn model_effects(app: &App, cmd: &ModelCommand) -> Vec<Box<dyn Effect>> {
|
||||
target: target.clone(),
|
||||
target_category: target_category.clone(),
|
||||
}),
|
||||
ModelCommand::ReplaceFormula { id, was, raw } => Box::new(effect::ReplaceFormula {
|
||||
id: id.clone(),
|
||||
was: was.clone(),
|
||||
raw: raw.clone(),
|
||||
}),
|
||||
ModelCommand::CleanEmptyRecords => Box::new(effect::CleanEmptyRecords),
|
||||
ModelCommand::SortData => Box::new(effect::SortData),
|
||||
|
||||
@@ -687,6 +692,13 @@ pub(crate) mod tests {
|
||||
target: "Profit".into(),
|
||||
target_category: "_Measure".into(),
|
||||
},
|
||||
ModelCommand::ReplaceFormula {
|
||||
id: crate::formula::parse_formula("Profit = Revenue", "_Measure")
|
||||
.unwrap()
|
||||
.id(),
|
||||
was: "Profit = Revenue".into(),
|
||||
raw: "Profit = Revenue - Cost".into(),
|
||||
},
|
||||
ModelCommand::CleanEmptyRecords,
|
||||
ModelCommand::SortData,
|
||||
ModelCommand::AddCategory("Product".into()),
|
||||
@@ -751,6 +763,7 @@ pub(crate) mod tests {
|
||||
ModelCommand::RemoveItem { .. } => "RemoveItem",
|
||||
ModelCommand::AddFormula { .. } => "AddFormula",
|
||||
ModelCommand::RemoveFormula { .. } => "RemoveFormula",
|
||||
ModelCommand::ReplaceFormula { .. } => "ReplaceFormula",
|
||||
ModelCommand::CleanEmptyRecords => "CleanEmptyRecords",
|
||||
ModelCommand::SortData => "SortData",
|
||||
ModelCommand::AddCategory(_) => "AddCategory",
|
||||
@@ -918,15 +931,47 @@ pub(crate) mod tests {
|
||||
/// to the same mutation lands, `Dirty` included.
|
||||
#[test]
|
||||
fn a_model_command_lands_where_its_keypress_lands() {
|
||||
/// Two categories on different axes: a one-category fixture cannot tell
|
||||
/// a transpose from a cycle (`doctrine(check_the_checker)`).
|
||||
/// Two categories on two axes, plus cells and 2+-entry view maps: why each
|
||||
/// is needed is in `context/test-evidence/command.md` (improvise-djcn).
|
||||
fn pivoted() -> App {
|
||||
use crate::model::cell::{CellKey, CellValue};
|
||||
let mut app = app();
|
||||
app.model_state.workbook.add_category("Month").unwrap();
|
||||
app.model_state.workbook.add_item("Month", "Jan").unwrap();
|
||||
let view = app.model_state.workbook.active_view_mut();
|
||||
let wb = &mut app.model_state.workbook;
|
||||
let items = [
|
||||
("Region", ["South", "East", "West"]),
|
||||
("Month", ["Jan", "Feb", "Mar"]),
|
||||
("Channel", ["Web", "Store", "Phone"]),
|
||||
("Year", ["2024", "2025", "2026"]),
|
||||
];
|
||||
for (category, names) in items {
|
||||
wb.add_category(category).ok();
|
||||
names.iter().for_each(|n| drop(wb.add_item(category, *n)));
|
||||
}
|
||||
for (n, (region, month)) in [("North", "Jan"), ("South", "Feb"), ("East", "Jan")]
|
||||
.into_iter()
|
||||
.enumerate()
|
||||
{
|
||||
let key = CellKey::new(vec![
|
||||
("Region".into(), region.into()),
|
||||
("Month".into(), month.into()),
|
||||
("Channel".into(), "Web".into()),
|
||||
("Year".into(), "2024".into()),
|
||||
]);
|
||||
wb.model.set_cell(key, CellValue::Number(n as f64 + 1.0));
|
||||
}
|
||||
let view = wb.active_view_mut();
|
||||
view.set_axis("Region", Axis::Row);
|
||||
view.set_axis("Month", Axis::Column);
|
||||
view.set_axis("Channel", Axis::Page);
|
||||
view.set_axis("Year", Axis::Page);
|
||||
view.set_page_selection("Channel", "Web");
|
||||
view.set_page_selection("Year", "2024");
|
||||
view.hide_item("Region", "West");
|
||||
view.hide_item("Region", "South");
|
||||
view.hide_item("Month", "Mar");
|
||||
view.toggle_group_collapse("Region", "Coastal");
|
||||
view.toggle_group_collapse("Region", "Inland");
|
||||
view.toggle_group_collapse("Month", "Q1");
|
||||
app.rebuild_layout();
|
||||
app
|
||||
}
|
||||
|
||||
@@ -194,7 +194,7 @@ impl RecordEntry {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Default)]
|
||||
#[derive(Clone, Default)]
|
||||
pub struct DataStore {
|
||||
/// Primary storage: interned keys, insertion-ordered so records mode can
|
||||
/// show rows in entry order. That order is correctness — core.md §8.
|
||||
@@ -208,6 +208,37 @@ pub struct DataStore {
|
||||
records: HashMap<RecordHash, RecordEntry>,
|
||||
}
|
||||
|
||||
/// Resolved through the symbol table and sorted wherever the storage is a
|
||||
/// `HashMap`, so two equal stores print alike (improvise-djcn).
|
||||
impl std::fmt::Debug for DataStore {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
let DataStore {
|
||||
cells,
|
||||
symbols,
|
||||
index,
|
||||
records,
|
||||
} = self;
|
||||
let cells: Vec<_> = cells
|
||||
.iter()
|
||||
.map(|(key, value)| (self.to_cell_key(key), value))
|
||||
.collect();
|
||||
let index: std::collections::BTreeMap<_, std::collections::BTreeSet<_>> = index
|
||||
.iter()
|
||||
.map(|((cat, item), keys)| {
|
||||
let pair = (symbols.resolve(*cat), symbols.resolve(*item));
|
||||
(pair, keys.iter().map(|k| self.to_cell_key(k).0).collect())
|
||||
})
|
||||
.collect();
|
||||
let records: std::collections::BTreeMap<_, _> = records.iter().collect();
|
||||
f.debug_struct("DataStore")
|
||||
.field("cells", &cells)
|
||||
.field("symbols", symbols)
|
||||
.field("index", &index)
|
||||
.field("records", &records)
|
||||
.finish()
|
||||
}
|
||||
}
|
||||
|
||||
impl DataStore {
|
||||
pub fn new() -> Self {
|
||||
Self::default()
|
||||
@@ -347,16 +378,30 @@ impl DataStore {
|
||||
/// Positions in `cells` of every cell matching `partial`, **in insertion
|
||||
/// order**. The index narrows; the primary map orders (§8).
|
||||
fn matching_positions(&self, partial: &[(String, String)]) -> Vec<usize> {
|
||||
// Intern the partial key (lookup only, no new symbols)
|
||||
let interned_partial: Vec<(Symbol, Symbol)> = partial
|
||||
self.intern_partial(partial)
|
||||
.map(|pairs| self.positions_of(&pairs))
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
/// `partial` as symbols, or `None` when a name was never interned, which
|
||||
/// no stored cell can then match.
|
||||
fn intern_partial(&self, partial: &[(String, String)]) -> Option<Vec<(Symbol, Symbol)>> {
|
||||
partial
|
||||
.iter()
|
||||
.filter_map(|(c, i)| Some((self.symbols.get(c)?, self.symbols.get(i)?)))
|
||||
.collect();
|
||||
.map(|(c, i)| Some((self.symbols.get(c)?, self.symbols.get(i)?)))
|
||||
.collect()
|
||||
}
|
||||
|
||||
if interned_partial.len() < partial.len() {
|
||||
return vec![];
|
||||
}
|
||||
/// How many candidates narrowing by `pairs` would visit: its smallest index set.
|
||||
fn narrowing_cost(&self, pairs: &[(Symbol, Symbol)]) -> usize {
|
||||
pairs
|
||||
.iter()
|
||||
.map(|pair| self.index.get(pair).map_or(0, HashSet::len))
|
||||
.min()
|
||||
.unwrap_or(self.cells.len())
|
||||
}
|
||||
|
||||
fn positions_of(&self, interned_partial: &[(Symbol, Symbol)]) -> Vec<usize> {
|
||||
let mut sets: Vec<&HashSet<InternedKey>> = interned_partial
|
||||
.iter()
|
||||
.filter_map(|pair| self.index.get(pair))
|
||||
@@ -395,6 +440,71 @@ impl DataStore {
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// The union of `matching_values` (or, `exact`, of `get`) over every key `partial`
|
||||
/// extends by one listed item per spanned category — scanned once (improvise-yb4b).
|
||||
pub fn spanned_values(
|
||||
&self,
|
||||
partial: &[(String, String)],
|
||||
spans: &[(&str, Vec<&str>)],
|
||||
exact: bool,
|
||||
) -> Vec<&CellValue> {
|
||||
let wanted: Option<Vec<(Symbol, HashSet<Symbol>)>> = spans
|
||||
.iter()
|
||||
.map(|(cat, items)| {
|
||||
let items = items.iter().filter_map(|i| self.symbols.get(i)).collect();
|
||||
Some((self.symbols.get(cat)?, items))
|
||||
})
|
||||
.collect();
|
||||
let Some(wanted) = wanted else {
|
||||
return Vec::new();
|
||||
};
|
||||
let admits = |ikey: &InternedKey| {
|
||||
(!exact || ikey.0.len() == partial.len() + wanted.len())
|
||||
&& wanted
|
||||
.iter()
|
||||
.all(|(cat, items)| ikey.0.iter().any(|(c, i)| c == cat && items.contains(i)))
|
||||
};
|
||||
self.spanned_positions(partial, &wanted)
|
||||
.into_iter()
|
||||
.filter_map(|n| self.cells.get_index(n))
|
||||
.filter(|(ikey, _)| admits(ikey))
|
||||
.map(|(_, value)| value)
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Candidates for a span: narrowed once by `partial`, or once per covered item
|
||||
/// of its smallest span, whichever visits fewer. Sorted (improvise-jq0m).
|
||||
fn spanned_positions(
|
||||
&self,
|
||||
partial: &[(String, String)],
|
||||
wanted: &[(Symbol, HashSet<Symbol>)],
|
||||
) -> Vec<usize> {
|
||||
self.intern_partial(partial)
|
||||
.map(|base| self.narrowed(&base, wanted))
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
fn narrowed(
|
||||
&self,
|
||||
base: &[(Symbol, Symbol)],
|
||||
wanted: &[(Symbol, HashSet<Symbol>)],
|
||||
) -> Vec<usize> {
|
||||
let with = |cat: Symbol, item: Symbol| [base, &[(cat, item)]].concat();
|
||||
let per_item: Vec<Vec<(Symbol, Symbol)>> = wanted
|
||||
.iter()
|
||||
.min_by_key(|(_, items)| items.len())
|
||||
.map(|(cat, items)| items.iter().map(|i| with(*cat, *i)).collect())
|
||||
.unwrap_or_default();
|
||||
let item_cost: usize = per_item.iter().map(|p| self.narrowing_cost(p)).sum();
|
||||
let narrow_per_item = !per_item.is_empty() && item_cost < self.narrowing_cost(base);
|
||||
let mut positions: Vec<usize> = match narrow_per_item {
|
||||
true => per_item.iter().flat_map(|p| self.positions_of(p)).collect(),
|
||||
false => self.positions_of(base),
|
||||
};
|
||||
positions.sort_unstable();
|
||||
positions
|
||||
}
|
||||
|
||||
/// Every cell matching `partial`, in insertion order. Allocates a
|
||||
/// `CellKey` per match — prefer `matching_values` when values suffice.
|
||||
#[allow(dead_code)]
|
||||
@@ -738,6 +848,41 @@ mod data_store {
|
||||
"the fast path and the general path must agree"
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-yb4b: one scan answers what a lookup per member answered, in both
|
||||
/// modes. context/test-evidence/core.md.
|
||||
#[test]
|
||||
fn a_spanned_scan_finds_what_a_lookup_per_member_finds() {
|
||||
let mut store = DataStore::new();
|
||||
let cells: [(&[(&str, &str)], f64); 7] = [
|
||||
(&[("M", "Rev"), ("R", "North"), ("P", "A")], 1.0),
|
||||
(&[("M", "Rev"), ("R", "South"), ("P", "A")], 2.0),
|
||||
(&[("M", "Rev"), ("R", "East"), ("P", "A")], 4.0),
|
||||
(&[("M", "Rev"), ("R", "North"), ("P", "A"), ("X", "x")], 8.0),
|
||||
(&[("M", "Rev"), ("P", "A")], 16.0),
|
||||
(&[("M", "Cost"), ("R", "North"), ("P", "A")], 32.0),
|
||||
(&[("M", "Rev"), ("R", "North"), ("P", "B")], 64.0),
|
||||
];
|
||||
for (coords, v) in cells {
|
||||
store.set(key(coords), CellValue::Number(v));
|
||||
}
|
||||
let base = key(&[("M", "Rev"), ("P", "A")]);
|
||||
let covered = vec!["North", "South", "West"];
|
||||
let members: Vec<CellKey> = covered.iter().map(|r| base.clone().with("R", *r)).collect();
|
||||
let sum = |values: Vec<&CellValue>| values.iter().filter_map(|v| v.as_f64()).sum::<f64>();
|
||||
let per_member_exact = members.iter().filter_map(|k| store.get(k)).collect();
|
||||
let per_member_over = members
|
||||
.iter()
|
||||
.flat_map(|k| store.matching_values(&k.0))
|
||||
.collect();
|
||||
let spans = [("R", covered.clone())];
|
||||
let scan_exact = store.spanned_values(&base.0, &spans, true);
|
||||
let scan_over = store.spanned_values(&base.0, &spans, false);
|
||||
assert_eq!((sum(scan_exact), sum(per_member_exact)), (3.0, 3.0));
|
||||
assert_eq!((sum(scan_over), sum(per_member_over)), (11.0, 11.0));
|
||||
let unknown = [("Nowhere", covered)];
|
||||
assert!(store.spanned_values(&base.0, &unknown, false).is_empty());
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -910,6 +1055,60 @@ mod prop_tests {
|
||||
"returned key {result_key} does not match partial {partial:?}");
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-jq0m: whichever narrowing it picks, a span returns what a lookup
|
||||
/// per member returns, in store order. context/test-evidence/core.md.
|
||||
#[test]
|
||||
fn a_span_finds_what_its_members_find_in_store_order(
|
||||
cells in prop::collection::vec((0u8..6, 0u8..3, 0u8..2, any::<bool>()), 0..40),
|
||||
covered_r in prop::collection::vec(any::<bool>(), 7),
|
||||
covered_p in prop::option::of(prop::collection::vec(any::<bool>(), 3)),
|
||||
base_m in 0u8..2,
|
||||
exact in any::<bool>(),
|
||||
) {
|
||||
let mut store = DataStore::new();
|
||||
for (n, (r, p, m, extra)) in cells.iter().enumerate() {
|
||||
let mut key = CellKey::new(vec![
|
||||
("P".into(), format!("p{p}")),
|
||||
("M".into(), format!("m{m}")),
|
||||
]);
|
||||
// r = 5 leaves the record without the spanned category at all.
|
||||
key = match r { 5 => key, r => key.with("R", format!("r{r}")) };
|
||||
key = match extra { true => key.with("X", "x"), false => key };
|
||||
store.set(key, CellValue::Number(n as f64));
|
||||
}
|
||||
// r6 is never stored, so it is a covered item the store never interned.
|
||||
let pick = |prefix: &str, flags: &[bool]| -> Vec<String> {
|
||||
flags.iter().enumerate().filter(|(_, f)| **f)
|
||||
.map(|(i, _)| format!("{prefix}{i}")).collect()
|
||||
};
|
||||
let rs = pick("r", &covered_r);
|
||||
let ps = covered_p.as_deref().map(|f| pick("p", f));
|
||||
let base = CellKey::new(vec![("M".into(), format!("m{base_m}"))]);
|
||||
let mut spans: Vec<(&str, Vec<&str>)> = vec![("R", rs.iter().map(String::as_str).collect())];
|
||||
if let Some(ps) = &ps {
|
||||
spans.push(("P", ps.iter().map(String::as_str).collect()));
|
||||
}
|
||||
let mut members = vec![base.clone()];
|
||||
for (cat, items) in &spans {
|
||||
members = members.iter()
|
||||
.flat_map(|k| items.iter().map(|i| k.clone().with(*cat, *i)))
|
||||
.collect();
|
||||
}
|
||||
let matched: std::collections::HashSet<CellKey> = members.iter()
|
||||
.flat_map(|k| match exact {
|
||||
true => store.get(k).map(|_| k.clone()).into_iter().collect::<Vec<_>>(),
|
||||
false => store.matching_cells(&k.0).into_iter().map(|(k, _)| k).collect(),
|
||||
})
|
||||
.collect();
|
||||
let expected: Vec<CellValue> = store.iter_cells()
|
||||
.filter(|(k, _)| matched.contains(k))
|
||||
.map(|(_, v)| v.clone())
|
||||
.collect();
|
||||
let got: Vec<CellValue> = store.spanned_values(&base.0, &spans, exact)
|
||||
.into_iter().cloned().collect();
|
||||
prop_assert_eq!(got, expected);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -5,12 +5,22 @@ use std::collections::HashMap;
|
||||
pub struct Symbol(u64);
|
||||
|
||||
/// Bidirectional string ↔ Symbol mapping.
|
||||
#[derive(Debug, Clone, Default)]
|
||||
#[derive(Clone, Default)]
|
||||
pub struct SymbolTable {
|
||||
to_id: HashMap<String, Symbol>,
|
||||
to_str: Vec<String>,
|
||||
}
|
||||
|
||||
/// The interned strings, sorted: which `Symbol` each got is interning order,
|
||||
/// not content, so two equal stores print alike (improvise-djcn).
|
||||
impl std::fmt::Debug for SymbolTable {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
let SymbolTable { to_id, to_str: _ } = self;
|
||||
let interned: std::collections::BTreeSet<&String> = to_id.keys().collect();
|
||||
f.debug_set().entries(interned).finish()
|
||||
}
|
||||
}
|
||||
|
||||
impl SymbolTable {
|
||||
#[allow(dead_code)]
|
||||
pub fn new() -> Self {
|
||||
|
||||
@@ -5,7 +5,7 @@ use indexmap::{IndexMap, IndexSet};
|
||||
|
||||
use super::category::{Category, CategoryId, CategoryKind, MEASURE_CATEGORY};
|
||||
use super::cell::{CellKey, CellValue, DataStore};
|
||||
use crate::formula::{AggFunc, Expr, Filter, Formula};
|
||||
use crate::formula::{AggFunc, Expr, Filter, Formula, FormulaId};
|
||||
|
||||
const MAX_CATEGORIES: usize = 12;
|
||||
|
||||
@@ -189,6 +189,27 @@ struct Span<'a> {
|
||||
over: &'a [String],
|
||||
}
|
||||
|
||||
/// A fold's reach as data: `base` is the key with each expanded category removed,
|
||||
/// and `spans` the covered items of each, in expansion order (improvise-yb4b).
|
||||
struct Expansion<'a> {
|
||||
base: CellKey,
|
||||
spans: Vec<(&'a str, Vec<&'a str>)>,
|
||||
}
|
||||
|
||||
impl Expansion<'_> {
|
||||
/// One key per member, the cross product in expansion order.
|
||||
fn members(&self) -> Vec<CellKey> {
|
||||
let mut expanded = vec![self.base.clone()];
|
||||
for (cat_name, covered) in &self.spans {
|
||||
expanded = expanded
|
||||
.iter()
|
||||
.flat_map(|k| covered.iter().map(|m| k.clone().with(*cat_name, *m)))
|
||||
.collect();
|
||||
}
|
||||
expanded
|
||||
}
|
||||
}
|
||||
|
||||
/// Where a formula's references are read: at one record, or across the members
|
||||
/// a collapsed group's fold covers, as a None axis reads them (core.md §14).
|
||||
#[derive(Clone, Copy)]
|
||||
@@ -353,16 +374,25 @@ pub struct Model {
|
||||
/// cache in hash order, which is not a change (improvise-pc3b, gotcha 18).
|
||||
impl std::fmt::Debug for Model {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
let mut aggs: Vec<_> = self.measure_agg.iter().collect();
|
||||
aggs.sort_by(|a, b| a.0.cmp(b.0));
|
||||
let mut cache: Vec<_> = self.formula_cache.iter().collect();
|
||||
// Destructured, so a new field is a compile error here (improvise-djcn).
|
||||
let Model {
|
||||
name,
|
||||
categories,
|
||||
data,
|
||||
formulas,
|
||||
next_category_id,
|
||||
measure_agg,
|
||||
formula_cache,
|
||||
} = self;
|
||||
let aggs: std::collections::BTreeMap<_, _> = measure_agg.iter().collect();
|
||||
let mut cache: Vec<_> = formula_cache.iter().collect();
|
||||
cache.sort_by(|a, b| a.0.0.cmp(&b.0.0));
|
||||
f.debug_struct("Model")
|
||||
.field("name", &self.name)
|
||||
.field("categories", &self.categories)
|
||||
.field("data", &self.data)
|
||||
.field("formulas", &self.formulas)
|
||||
.field("next_category_id", &self.next_category_id)
|
||||
.field("name", name)
|
||||
.field("categories", categories)
|
||||
.field("data", data)
|
||||
.field("formulas", formulas)
|
||||
.field("next_category_id", next_category_id)
|
||||
.field("measure_agg", &aggs)
|
||||
.field("formula_cache", &cache)
|
||||
.finish()
|
||||
@@ -532,6 +562,38 @@ impl Model {
|
||||
&self.formulas
|
||||
}
|
||||
|
||||
pub fn formula(&self, id: &FormulaId) -> Option<&Formula> {
|
||||
self.formulas.iter().find(|f| f.id() == *id)
|
||||
}
|
||||
|
||||
/// Put `formula` where `id` is, keeping its position; refuses rather than
|
||||
/// let a new target displace another formula — `context/code/core.md` §18.
|
||||
pub fn replace_formula(&mut self, id: &FormulaId, formula: Formula) -> Result<()> {
|
||||
let new_id = formula.id();
|
||||
let renames = new_id != *id;
|
||||
// A regular category's target is a real item; renaming it is sboz's (kb1d).
|
||||
if renames && !CategoryKind::of_name(&formula.target_category).is_measure() {
|
||||
return Err(anyhow!(
|
||||
"Renaming a formula outside _Measure is not supported yet; delete and re-add it"
|
||||
));
|
||||
}
|
||||
if renames && self.formula(&new_id).is_some() {
|
||||
return Err(anyhow!("A formula for {} already exists", formula.target));
|
||||
}
|
||||
let pos = self
|
||||
.formulas
|
||||
.iter()
|
||||
.position(|f| f.id() == *id)
|
||||
.ok_or_else(|| anyhow!("That formula no longer exists"))?;
|
||||
if !CategoryKind::of_name(&formula.target_category).is_measure()
|
||||
&& let Some(cat) = self.categories.get_mut(&formula.target_category)
|
||||
{
|
||||
cat.add_item(&formula.target);
|
||||
}
|
||||
self.formulas[pos] = formula;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// The formula that owns `key`, if one does. **The one home** of "is this
|
||||
/// cell derived" — every other answer to it is a second derivation.
|
||||
pub fn formula_for(&self, key: &CellKey) -> Option<&Formula> {
|
||||
@@ -697,13 +759,13 @@ impl Model {
|
||||
span: Span,
|
||||
memo: &FoldMemo,
|
||||
) -> Option<CellValue> {
|
||||
let members = self.group_expansion(key, span);
|
||||
match (members.is_empty(), self.formula_for(key)) {
|
||||
let expansion = self.group_expansion(key, span);
|
||||
match (expansion.spans.is_empty(), self.formula_for(key)) {
|
||||
(true, _) => self.evaluate_unfolded(key, agg),
|
||||
(false, Some(formula)) => {
|
||||
self.fold_formula_once(formula, key, &members, (agg, span, memo))
|
||||
self.fold_formula_once(formula, key, &expansion, (agg, span, memo))
|
||||
}
|
||||
(false, None) => self.fold_stored(key, &members, agg),
|
||||
(false, None) => self.fold_stored(key, &expansion, agg),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -713,7 +775,7 @@ impl Model {
|
||||
&self,
|
||||
formula: &Formula,
|
||||
key: &CellKey,
|
||||
members: &[CellKey],
|
||||
expansion: &Expansion,
|
||||
(agg, span, memo): (Aggregation, Span, &FoldMemo),
|
||||
) -> Option<CellValue> {
|
||||
let seen = memo.get(key, span);
|
||||
@@ -725,7 +787,7 @@ impl Model {
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
FOLD_FORMULA_EVALUATIONS.with(|n| n.set(n.get() + 1));
|
||||
let reach = Reach::Fold { agg, span, memo };
|
||||
let value = self.eval_formula_folded(formula, key, members, agg, reach);
|
||||
let value = self.eval_formula_folded(formula, key, expansion, agg, reach);
|
||||
memo.put(key, span, Some(value.clone()));
|
||||
value
|
||||
}
|
||||
@@ -743,9 +805,9 @@ impl Model {
|
||||
aggregate_cell_values(&self.data.matching_values(&key.0), self.agg_for(key))
|
||||
}
|
||||
|
||||
/// One key per covered member where `key` names a collapsed GROUP rather than
|
||||
/// an item, or omits a spanned category — the cross product. Empty when neither.
|
||||
fn group_expansion(&self, key: &CellKey, span: Span) -> Vec<CellKey> {
|
||||
/// Each category `key` folds, with its covered members: a collapsed GROUP it
|
||||
/// names, or a spanned category it omits. No spans when neither.
|
||||
fn group_expansion<'a>(&'a self, key: &'a CellKey, span: Span<'a>) -> Expansion<'a> {
|
||||
let grouped = key.0.iter().map(|(cat_name, name)| {
|
||||
let members: Vec<&str> = self
|
||||
.category(cat_name)
|
||||
@@ -762,24 +824,21 @@ impl Model {
|
||||
let items = self.category(cat_name).map(|cat| cat.ordered_item_names());
|
||||
(cat_name.as_str(), items.unwrap_or_default())
|
||||
});
|
||||
let mut expanded = vec![key.clone()];
|
||||
for (cat_name, members) in grouped.chain(spanned) {
|
||||
let covered: Vec<&str> = members
|
||||
.into_iter()
|
||||
.filter(|m| (span.covers)(cat_name, m))
|
||||
.collect();
|
||||
if covered.is_empty() {
|
||||
continue;
|
||||
}
|
||||
expanded = expanded
|
||||
.iter()
|
||||
.flat_map(|k| covered.iter().map(|m| k.clone().with(cat_name, *m)))
|
||||
.collect();
|
||||
}
|
||||
match expanded.as_slice() {
|
||||
[only] if only == key => Vec::new(),
|
||||
_ => expanded,
|
||||
}
|
||||
let spans: Vec<(&str, Vec<&str>)> = grouped
|
||||
.chain(spanned)
|
||||
.map(|(cat_name, members)| {
|
||||
let covered: Vec<&str> = members
|
||||
.into_iter()
|
||||
.filter(|m| (span.covers)(cat_name, m))
|
||||
.collect();
|
||||
(cat_name, covered)
|
||||
})
|
||||
.filter(|(_, covered)| !covered.is_empty())
|
||||
.collect();
|
||||
let base = spans
|
||||
.iter()
|
||||
.fold(key.clone(), |k, (cat_name, _)| k.without(cat_name));
|
||||
Expansion { base, spans }
|
||||
}
|
||||
|
||||
/// A coordinate as a formula reads it. At a fold a group's name is no item, so
|
||||
@@ -805,30 +864,28 @@ impl Model {
|
||||
|
||||
/// The contexts an aggregate inside a formula pools over at `context`.
|
||||
fn contexts_of(&self, context: &CellKey, reach: Reach) -> Vec<CellKey> {
|
||||
let members = match reach {
|
||||
Reach::Record => Vec::new(),
|
||||
Reach::Fold { span, .. } => self.group_expansion(context, span),
|
||||
let expansion = match reach {
|
||||
Reach::Record => None,
|
||||
Reach::Fold { span, .. } => Some(self.group_expansion(context, span)),
|
||||
};
|
||||
match members.is_empty() {
|
||||
true => vec![context.clone()],
|
||||
false => members,
|
||||
match expansion.filter(|e| !e.spans.is_empty()) {
|
||||
None => vec![context.clone()],
|
||||
Some(e) => e.members(),
|
||||
}
|
||||
}
|
||||
|
||||
/// A group's stored values fold ONCE over every record the group covers.
|
||||
/// A group's stored values fold ONCE over every record the group covers, in
|
||||
/// the store's own order: one scan, not a lookup per member (improvise-yb4b).
|
||||
fn fold_stored(
|
||||
&self,
|
||||
key: &CellKey,
|
||||
members: &[CellKey],
|
||||
expansion: &Expansion,
|
||||
agg: Aggregation,
|
||||
) -> Option<CellValue> {
|
||||
let values: Vec<&CellValue> = members
|
||||
.iter()
|
||||
.flat_map(|k| match agg {
|
||||
Aggregation::Exact => self.data.get(k).into_iter().collect(),
|
||||
Aggregation::OverHidden => self.data.matching_values(&k.0),
|
||||
})
|
||||
.collect();
|
||||
let exact = agg == Aggregation::Exact;
|
||||
let values = self
|
||||
.data
|
||||
.spanned_values(&expansion.base.0, &expansion.spans, exact);
|
||||
aggregate_cell_values(&values, self.agg_for(key))
|
||||
}
|
||||
|
||||
@@ -838,7 +895,7 @@ impl Model {
|
||||
&self,
|
||||
formula: &Formula,
|
||||
key: &CellKey,
|
||||
members: &[CellKey],
|
||||
expansion: &Expansion,
|
||||
agg: Aggregation,
|
||||
reach: Reach,
|
||||
) -> Option<CellValue> {
|
||||
@@ -847,7 +904,7 @@ impl Model {
|
||||
.as_ref()
|
||||
.is_none_or(|f| self.read_coordinate(key, &f.category, reach) == Some(f.item.as_str()));
|
||||
if !applies {
|
||||
return self.fold_stored(key, members, agg);
|
||||
return self.fold_stored(key, expansion, agg);
|
||||
}
|
||||
let depth = Self::MAX_EVAL_DEPTH;
|
||||
match eval_expr(
|
||||
@@ -2221,6 +2278,88 @@ mod formula_tests {
|
||||
assert_eq!(m.evaluate(&k), None);
|
||||
}
|
||||
|
||||
fn three_formulas() -> Model {
|
||||
let mut m = revenue_cost_model();
|
||||
for raw in [
|
||||
"Profit = Revenue - Cost",
|
||||
"Tax = Revenue * 0.1",
|
||||
"Margin = Profit / Revenue",
|
||||
] {
|
||||
m.add_formula(parse_formula(raw, "_Measure").unwrap());
|
||||
}
|
||||
m
|
||||
}
|
||||
|
||||
fn targets(m: &Model) -> Vec<&str> {
|
||||
m.formulas.iter().map(|f| f.target.as_str()).collect()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn replace_formula_renames_in_place_and_evaluates_under_the_new_name() {
|
||||
let mut m = three_formulas();
|
||||
let id = m.formulas[1].id();
|
||||
m.replace_formula(
|
||||
&id,
|
||||
parse_formula("Levy = Revenue * 0.5", "_Measure").unwrap(),
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(targets(&m), vec!["Profit", "Levy", "Margin"]);
|
||||
let levy = coord(&[("_Measure", "Levy"), ("Region", "East")]);
|
||||
assert_eq!(m.evaluate(&levy).and_then(|v| v.as_f64()), Some(500.0));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn replace_formula_refuses_to_displace_another_formula() {
|
||||
let mut m = three_formulas();
|
||||
let id = m.formulas[1].id();
|
||||
let onto_margin = parse_formula("Margin = Revenue", "_Measure").unwrap();
|
||||
assert!(m.replace_formula(&id, onto_margin).is_err());
|
||||
assert_eq!(targets(&m), vec!["Profit", "Tax", "Margin"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn replace_formula_refuses_an_id_that_names_nothing() {
|
||||
let mut m = three_formulas();
|
||||
let id = m.formulas[1].id();
|
||||
m.remove_formula("Tax", "_Measure");
|
||||
let tax = parse_formula("Tax = Revenue", "_Measure").unwrap();
|
||||
assert!(m.replace_formula(&id, tax).is_err());
|
||||
assert_eq!(targets(&m), vec!["Profit", "Margin"]);
|
||||
}
|
||||
|
||||
/// improvise-kb1d: a Region-targeted formula at index 1. A rename would
|
||||
/// leave `Total` as an empty item and append `Sum`, so it is refused.
|
||||
#[test]
|
||||
fn replace_formula_refuses_to_rename_a_target_outside_measure() {
|
||||
let mut m = three_formulas();
|
||||
m.formulas
|
||||
.insert(1, parse_formula("Total = East + West", "Region").unwrap());
|
||||
let shape = |m: &Model| {
|
||||
let region = m.category("Region").unwrap().ordered_item_names();
|
||||
let owned: Vec<String> = region.iter().map(|s| s.to_string()).collect();
|
||||
(
|
||||
targets(m).iter().map(|s| s.to_string()).collect::<Vec<_>>(),
|
||||
owned,
|
||||
)
|
||||
};
|
||||
let before = shape(&m);
|
||||
let id = m.formulas[1].id();
|
||||
let sum = parse_formula("Sum = East + West", "Region").unwrap();
|
||||
assert!(m.replace_formula(&id, sum).is_err());
|
||||
assert_eq!(shape(&m), before, "nothing moved, nothing added");
|
||||
let edit = parse_formula("Total = East - West", "Region").unwrap();
|
||||
m.replace_formula(&id, edit).unwrap();
|
||||
assert_eq!(targets(&m), vec!["Profit", "Total", "Tax", "Margin"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_formula_mentions_what_its_expression_or_its_where_names() {
|
||||
let margin = parse_formula("Margin = Profit / Revenue", "_Measure").unwrap();
|
||||
assert!(margin.mentions("Profit") && !margin.mentions("Tax"));
|
||||
let scoped = parse_formula("East = SUM(Revenue) WHERE Region = \"East\"", "_Measure");
|
||||
assert!(scoped.unwrap().mentions("Region"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sum_aggregation_across_region() {
|
||||
let mut m = revenue_cost_model();
|
||||
|
||||
@@ -27,7 +27,7 @@ pub fn unused_view_name<'a>(existing: impl IntoIterator<Item = &'a str>) -> Stri
|
||||
.expect("a name is free below usize::MAX, there being fewer views")
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
#[derive(Clone)]
|
||||
pub struct View {
|
||||
pub name: String,
|
||||
/// Axis assignment for each category
|
||||
@@ -44,6 +44,38 @@ pub struct View {
|
||||
pub prune_empty: bool,
|
||||
}
|
||||
|
||||
/// Sorted wherever the storage is a `HashMap`, so two equal views print alike;
|
||||
/// destructured, so a new field is a compile error here (improvise-djcn).
|
||||
impl std::fmt::Debug for View {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
use std::collections::{BTreeMap, BTreeSet};
|
||||
let View {
|
||||
name,
|
||||
category_axes,
|
||||
page_selections,
|
||||
hidden_items,
|
||||
collapsed_groups,
|
||||
number_format,
|
||||
prune_empty,
|
||||
} = self;
|
||||
let sorted = |m: &HashMap<String, HashSet<String>>| -> BTreeMap<String, BTreeSet<String>> {
|
||||
m.iter()
|
||||
.map(|(k, v)| (k.clone(), v.iter().cloned().collect()))
|
||||
.collect()
|
||||
};
|
||||
let pages: BTreeMap<_, _> = page_selections.iter().collect();
|
||||
f.debug_struct("View")
|
||||
.field("name", name)
|
||||
.field("category_axes", category_axes)
|
||||
.field("page_selections", &pages)
|
||||
.field("hidden_items", &sorted(hidden_items))
|
||||
.field("collapsed_groups", &sorted(collapsed_groups))
|
||||
.field("number_format", number_format)
|
||||
.field("prune_empty", prune_empty)
|
||||
.finish()
|
||||
}
|
||||
}
|
||||
|
||||
impl View {
|
||||
/// True for the on-demand scratch views, rebuilt rather than persisted.
|
||||
pub fn is_virtual(&self) -> bool {
|
||||
|
||||
@@ -10,6 +10,8 @@ repository = "https://github.com/fiddlerwoaroof/improvise"
|
||||
anyhow = "1"
|
||||
pest = "2.9.2"
|
||||
pest_derive = "2.9.2"
|
||||
# For FormulaId alone, which crosses the wire (improvise-km71); no AST derive.
|
||||
serde = { version = "1", features = ["derive"] }
|
||||
|
||||
[dev-dependencies]
|
||||
pest_meta = "2.9.2"
|
||||
|
||||
@@ -84,4 +84,47 @@ impl Formula {
|
||||
self.format = format;
|
||||
self
|
||||
}
|
||||
|
||||
/// Whether this formula's expression or its `WHERE` names `name`.
|
||||
pub fn mentions(&self, name: &str) -> bool {
|
||||
self.expr.mentions(name) || self.filter.as_ref().is_some_and(|f| f.names(name))
|
||||
}
|
||||
|
||||
/// Which formula this is. The one constructor of a [`FormulaId`].
|
||||
pub fn id(&self) -> FormulaId {
|
||||
FormulaId {
|
||||
target: self.target.clone(),
|
||||
target_category: self.target_category.clone(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Which formula an edit means, opaque so its content can change in one place:
|
||||
/// today a target and its category — `context/code/core.md` §18.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
|
||||
pub struct FormulaId {
|
||||
target: String,
|
||||
target_category: String,
|
||||
}
|
||||
|
||||
impl Expr {
|
||||
/// Whether this expression names `name` anywhere, as a reference or as
|
||||
/// either half of a coordinate filter.
|
||||
pub fn mentions(&self, name: &str) -> bool {
|
||||
match self {
|
||||
Expr::Number(_) => false,
|
||||
Expr::Ref(r) => r == name,
|
||||
Expr::BinOp(_, l, r) => l.mentions(name) || r.mentions(name),
|
||||
Expr::UnaryMinus(e) => e.mentions(name),
|
||||
Expr::Agg(_, e, f) => e.mentions(name) || f.as_ref().is_some_and(|f| f.names(name)),
|
||||
Expr::If(c, t, e) => c.mentions(name) || t.mentions(name) || e.mentions(name),
|
||||
Expr::Is(f) => f.names(name),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Filter {
|
||||
fn names(&self, name: &str) -> bool {
|
||||
self.category == name || self.item == name
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,6 +2,6 @@ pub mod ast;
|
||||
pub mod parser;
|
||||
pub mod quote;
|
||||
|
||||
pub use ast::{AggFunc, BinOp, Expr, Filter, Formula};
|
||||
pub use ast::{AggFunc, BinOp, Expr, Filter, Formula, FormulaId};
|
||||
pub use parser::parse_formula;
|
||||
pub use quote::{is_bare_name, quote_name};
|
||||
|
||||
@@ -94,15 +94,16 @@ impl BasisCache {
|
||||
}
|
||||
|
||||
impl BasisCache {
|
||||
/// A basis for records whose counters were read EARLIER, such as when an
|
||||
/// editor opened (improvise-0j0f). The document tiers are current.
|
||||
/// A basis for what was read EARLIER, such as when an editor opened: the
|
||||
/// document and its records then, the layout now (improvise-0j0f, improvise-vndi).
|
||||
pub fn claim_as_read(
|
||||
&self,
|
||||
document_generation: u64,
|
||||
read: impl IntoIterator<Item = (RecordHash, u64)>,
|
||||
) -> Option<Basis> {
|
||||
let current = self.current.as_ref()?;
|
||||
Some(Basis {
|
||||
document_generation: current.document_generation,
|
||||
document_generation,
|
||||
layout_version: current.layout_version,
|
||||
records: read.into_iter().collect(),
|
||||
})
|
||||
|
||||
@@ -101,6 +101,7 @@ pub(crate) fn advertised(mode: &AppMode) -> Vec<(&'static str, Option<Vec<String
|
||||
],
|
||||
AppMode::FormulaPanel => vec![
|
||||
("enter-mode", arg("formula-edit"), "new"),
|
||||
("edit-formula-at-cursor", None, "edit"),
|
||||
("delete-formula-at-cursor", None, "delete"),
|
||||
("move-panel-cursor", None, "nav"),
|
||||
("enter-mode", arg("normal"), "back"),
|
||||
@@ -720,6 +721,12 @@ fn panels(keys: KeysRow) -> Vec<HelpLine> {
|
||||
Some(&["formula-edit"]),
|
||||
"New formula",
|
||||
),
|
||||
keys(
|
||||
&ModeKey::FormulaPanel,
|
||||
"edit-formula-at-cursor",
|
||||
None,
|
||||
"Edit selected formula",
|
||||
),
|
||||
keys(
|
||||
&ModeKey::FormulaPanel,
|
||||
"delete-formula-at-cursor",
|
||||
|
||||
@@ -766,6 +766,8 @@ impl Keymaps {
|
||||
"enter-mode",
|
||||
vec!["formula-edit".into()],
|
||||
)
|
||||
.bind(KeyCode::Enter, "edit-formula-at-cursor")
|
||||
.bind(KeyCode::Char('e'), "edit-formula-at-cursor")
|
||||
.delete("delete-formula-at-cursor")
|
||||
.finish(),
|
||||
),
|
||||
|
||||
@@ -30,10 +30,10 @@ pub use keymap::{Binding, KeyPattern, Keymap, Keymaps, ModeKey};
|
||||
pub use reduce::{Command, HelpPage, ModelCommand, records_named, reduce_view};
|
||||
pub use render::{
|
||||
AxisFrame, CategoryRow, CategoryTreeCache, CategoryTreeUpdate, CellDisplay, CellId, CreateKind,
|
||||
DocumentCache, FormulaCache, FrameEntry, FrameGroup, GridCache, GridExtent, GridUpdate,
|
||||
MAX_ROW_HEADER_W, MIN_ROW_HEADER_W, Projection, RecordCell, Snapshot, TileBarCache, TileEntry,
|
||||
ViewListCache, ViewSummary, WidgetCache, WizardCache, WizardDateField, WizardFieldKind,
|
||||
WizardProposal, WizardStepKind, header_level_widths, opening_tile,
|
||||
DocumentCache, FormulaCache, FormulaEntry, FrameEntry, FrameGroup, GridCache, GridExtent,
|
||||
GridUpdate, MAX_ROW_HEADER_W, MIN_ROW_HEADER_W, Projection, RecordCell, Snapshot, TileBarCache,
|
||||
TileEntry, ViewListCache, ViewSummary, WidgetCache, WizardCache, WizardDateField,
|
||||
WizardFieldKind, WizardProposal, WizardStepKind, header_level_widths, opening_tile,
|
||||
};
|
||||
pub use resolve::{
|
||||
Click, KeyOutcome, OpenedEditor, Request, click, edited, read_buffer, resolve, typed,
|
||||
@@ -49,6 +49,9 @@ pub use improvise_core::view::layout::PageAxis;
|
||||
// without depending on `improvise-core` directly.
|
||||
pub use improvise_core::model::cell::CellValue;
|
||||
|
||||
// On the wire in `ModelCommand::ReplaceFormula` and in `AppMode::FormulaEdit`.
|
||||
pub use improvise_core::formula::FormulaId;
|
||||
|
||||
/// Number of pages in the Help screen. Lives here with the `help_page` state
|
||||
/// it bounds, so help-page effects stay terminal-independent.
|
||||
pub const HELP_PAGE_COUNT: usize = 5;
|
||||
@@ -140,6 +143,7 @@ pub enum AppMode {
|
||||
},
|
||||
FormulaEdit {
|
||||
minibuf: MinibufferConfig,
|
||||
slot: FormulaSlot,
|
||||
},
|
||||
FormulaPanel,
|
||||
CategoryPanel,
|
||||
@@ -177,6 +181,18 @@ pub enum AppMode {
|
||||
},
|
||||
}
|
||||
|
||||
/// What a formula editor commits to, captured when it OPENS and never
|
||||
/// recomputed at commit — `context/code/protocol.md` §1 (improvise-km71).
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||
pub enum FormulaSlot {
|
||||
New,
|
||||
/// Replace the formula `id` names, provided it still reads `was`.
|
||||
Existing {
|
||||
id: FormulaId,
|
||||
was: String,
|
||||
},
|
||||
}
|
||||
|
||||
/// `Normal` and `RecordsNormal` are one mode over two layouts, so the mode an
|
||||
/// end holds must answer for the grid it holds — `context/code/protocol.md`.
|
||||
pub fn settled_mode(mode: &AppMode, records: bool) -> AppMode {
|
||||
@@ -334,12 +350,22 @@ impl AppMode {
|
||||
}
|
||||
|
||||
pub fn formula_edit() -> Self {
|
||||
Self::formula_editor("formula: ", FormulaSlot::New)
|
||||
}
|
||||
|
||||
/// The editor opened on an existing formula, which it names from here on.
|
||||
pub fn formula_replace(id: FormulaId, was: String) -> Self {
|
||||
Self::formula_editor("edit formula: ", FormulaSlot::Existing { id, was })
|
||||
}
|
||||
|
||||
fn formula_editor(prompt: &str, slot: FormulaSlot) -> Self {
|
||||
Self::FormulaEdit {
|
||||
minibuf: MinibufferConfig {
|
||||
buffer_key: "formula".into(),
|
||||
prompt: "formula: ".into(),
|
||||
prompt: prompt.into(),
|
||||
color: MinibufColor::Cyan,
|
||||
},
|
||||
slot,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -11,7 +11,7 @@ use improvise_core::view::Axis;
|
||||
use crate::basis::Basis;
|
||||
use crate::render::{Projection, RecordCell};
|
||||
use crate::resolve::OpenedEditor;
|
||||
use crate::{AppMode, HELP_PAGE_COUNT, KeyCode, KeyModifiers, Panel, ViewState};
|
||||
use crate::{AppMode, FormulaId, HELP_PAGE_COUNT, KeyCode, KeyModifiers, Panel, ViewState};
|
||||
use improvise_core::model::record::RecordHash;
|
||||
|
||||
/// One message on the wire, in either direction.
|
||||
@@ -79,7 +79,7 @@ pub enum Command {
|
||||
Project(Projection),
|
||||
}
|
||||
|
||||
/// A mutation of the shared document: 24 variants, derived not invented (§2). A
|
||||
/// A mutation of the shared document: 25 variants, derived not invented (§2). A
|
||||
/// view-configuration variant **names its view** (improvise-lltt).
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||
pub enum ModelCommand {
|
||||
@@ -111,6 +111,13 @@ pub enum ModelCommand {
|
||||
target: String,
|
||||
target_category: String,
|
||||
},
|
||||
/// Edit the formula `id` names in place, if it still reads `was` — an edit
|
||||
/// of that formula, not a remove-then-add (improvise-km71).
|
||||
ReplaceFormula {
|
||||
id: FormulaId,
|
||||
was: String,
|
||||
raw: String,
|
||||
},
|
||||
CleanEmptyRecords,
|
||||
SortData,
|
||||
|
||||
@@ -180,6 +187,7 @@ pub fn records_named(cmd: &ModelCommand) -> Vec<RecordHash> {
|
||||
| ModelCommand::RemoveItem { .. }
|
||||
| ModelCommand::AddFormula { .. }
|
||||
| ModelCommand::RemoveFormula { .. }
|
||||
| ModelCommand::ReplaceFormula { .. }
|
||||
| ModelCommand::CleanEmptyRecords
|
||||
| ModelCommand::SortData
|
||||
| ModelCommand::AddCategory(_)
|
||||
@@ -593,6 +601,9 @@ mod tests {
|
||||
fn surrogate(_: &RecordHash) -> usize {
|
||||
1
|
||||
}
|
||||
fn formula(_: &FormulaId) -> usize {
|
||||
1
|
||||
}
|
||||
fn record(RecordCell { record, measure }: &RecordCell) -> usize {
|
||||
surrogate(record) + optional_name(measure)
|
||||
}
|
||||
@@ -627,6 +638,7 @@ mod tests {
|
||||
target,
|
||||
target_category,
|
||||
} => name(target) + name(target_category),
|
||||
ModelCommand::ReplaceFormula { id, was, raw } => formula(id) + name(was) + name(raw),
|
||||
ModelCommand::CleanEmptyRecords | ModelCommand::SortData => 0,
|
||||
ModelCommand::AddCategory(category)
|
||||
| ModelCommand::RemoveCategory(category)
|
||||
|
||||
@@ -9,7 +9,8 @@ use improvise_core::model::record::{AggregateHash, RecordHash};
|
||||
use improvise_core::view::Axis;
|
||||
use improvise_core::view::layout::PageAxis;
|
||||
|
||||
use crate::ViewState;
|
||||
use crate::{FormulaId, ViewState};
|
||||
use improvise_core::formula::Formula;
|
||||
|
||||
/// A widget's local copy of what it draws. **Total and deterministic**: an
|
||||
/// update naming something absent is dropped, never a panic.
|
||||
@@ -728,17 +729,42 @@ impl WidgetCache for CategoryTreeCache {
|
||||
}
|
||||
}
|
||||
|
||||
/// The formula panel's copy of the formula list (improvise-pca). One
|
||||
/// mutation: add and delete are the only effects, and both send whole.
|
||||
/// The formula panel's copy of the formula list (improvise-pca). Every
|
||||
/// formula effect sends it whole.
|
||||
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct FormulaCache {
|
||||
pub formulas: Vec<String>,
|
||||
pub formulas: Vec<FormulaEntry>,
|
||||
}
|
||||
|
||||
/// One formula as a client holds it: its text, and the identity an edit names
|
||||
/// it by, which the text alone cannot give a client (improvise-km71).
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct FormulaEntry {
|
||||
pub id: FormulaId,
|
||||
pub raw: String,
|
||||
}
|
||||
|
||||
impl From<&Formula> for FormulaEntry {
|
||||
fn from(f: &Formula) -> Self {
|
||||
Self {
|
||||
id: f.id(),
|
||||
raw: f.raw.clone(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// A `_Measure` formula's entry, identified exactly as the host identifies it.
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
impl From<&str> for FormulaEntry {
|
||||
fn from(raw: &str) -> Self {
|
||||
Self::from(&improvise_core::formula::parse_formula(raw, "_Measure").expect(raw))
|
||||
}
|
||||
}
|
||||
|
||||
impl WidgetCache for FormulaCache {
|
||||
type Update = Vec<String>;
|
||||
type Update = Vec<FormulaEntry>;
|
||||
|
||||
fn apply(&mut self, update: &Vec<String>) {
|
||||
fn apply(&mut self, update: &Vec<FormulaEntry>) {
|
||||
self.formulas = update.clone();
|
||||
}
|
||||
}
|
||||
@@ -882,8 +908,8 @@ pub struct WizardCache {
|
||||
pub enum Projection {
|
||||
Grid(GridUpdate),
|
||||
CategoryTree(CategoryTreeUpdate),
|
||||
/// The raw text of every formula.
|
||||
Formulas(Vec<String>),
|
||||
/// Every formula: its text and its identity.
|
||||
Formulas(Vec<FormulaEntry>),
|
||||
ViewList(Vec<ViewSummary>),
|
||||
TileBar(Vec<TileEntry>),
|
||||
/// Name, dirty marker and where the host keeps it (improvise-z754).
|
||||
@@ -1490,10 +1516,10 @@ mod tests {
|
||||
#[test]
|
||||
fn the_list_caches_hold_the_last_update() {
|
||||
let formulas = FormulaCache::folded(&[
|
||||
vec!["Profit = Revenue - Cost".to_string()],
|
||||
vec!["Profit = Revenue - Cost".into()],
|
||||
vec![
|
||||
"Profit = Revenue - Cost".to_string(),
|
||||
"Tax = Revenue * 0.1".to_string(),
|
||||
"Profit = Revenue - Cost".into(),
|
||||
"Tax = Revenue * 0.1".into(),
|
||||
],
|
||||
]);
|
||||
assert_eq!(formulas.formulas.len(), 2);
|
||||
@@ -1851,7 +1877,7 @@ mod tests {
|
||||
|
||||
assert_eq!(booted, (grid, formulas));
|
||||
assert_eq!(booted.0.cell(0, 0).unwrap().text, "5");
|
||||
assert_eq!(booted.1.formulas, vec!["Tax = Revenue * 0.1"]);
|
||||
assert_eq!(booted.1.formulas, vec!["Tax = Revenue * 0.1".into()]);
|
||||
}
|
||||
|
||||
/// Two subscribers, each folding only what is addressed to it — the
|
||||
|
||||
@@ -9,8 +9,8 @@ use crate::frame::ClientFrame;
|
||||
use crate::keymap::{Binding, Keymaps, ModeKey};
|
||||
use crate::viewmodel::{CatTreeEntry, axis_change_status, flatten_cat_tree};
|
||||
use crate::{
|
||||
AppMode, CellId, CellValue, Command, CreateKind, GridExtent, HelpPage, KeyCode, KeyEvent,
|
||||
ModelCommand, Panel, RecordCell, ViewState, records_named,
|
||||
AppMode, CellId, CellValue, Command, CreateKind, FormulaId, FormulaSlot, GridExtent, HelpPage,
|
||||
KeyCode, KeyEvent, ModelCommand, Panel, RecordCell, ViewState, records_named,
|
||||
};
|
||||
|
||||
/// The virtual category a new measure is filed under, spelled as
|
||||
@@ -187,8 +187,8 @@ fn click_view_row(state: &ClientState, index: usize) -> Option<Vec<Command>> {
|
||||
Some(commands)
|
||||
}
|
||||
|
||||
/// The formula panel binds **no** `Enter`, so the rule yields navigation and
|
||||
/// nothing else. That is the rule applying, not an omission — clients.md.
|
||||
/// Focus the panel, move its cursor, then open the editor on the row — what
|
||||
/// `F` then `Enter` do. `edit-formula-at-cursor` is the activation (km71).
|
||||
fn click_formula_row(state: &ClientState, index: usize) -> Option<Vec<Command>> {
|
||||
state.formulas.formulas.get(index)?;
|
||||
let mut commands = focus(state, Panel::Formula);
|
||||
@@ -196,9 +196,26 @@ fn click_formula_row(state: &ClientState, index: usize) -> Option<Vec<Command>>
|
||||
panel: Panel::Formula,
|
||||
cursor: index,
|
||||
});
|
||||
// Off `index`, never the cursor: `SetPanelCursor` is not folded yet.
|
||||
commands.extend(edit_formula_at(state, index));
|
||||
Some(commands)
|
||||
}
|
||||
|
||||
/// `EditFormulaAtCursor::execute` at an arbitrary index: open the editor on
|
||||
/// that formula, naming it, and seed the buffer with its text.
|
||||
fn edit_formula_at(state: &ClientState, index: usize) -> Vec<Command> {
|
||||
match state.formulas.formulas.get(index) {
|
||||
Some(entry) => vec![
|
||||
Command::ChangeMode(AppMode::formula_replace(
|
||||
entry.id.clone(),
|
||||
entry.raw.clone(),
|
||||
)),
|
||||
write_buffer("formula", entry.raw.clone()),
|
||||
],
|
||||
None => vec![],
|
||||
}
|
||||
}
|
||||
|
||||
/// Focus the panel, move its cursor, then activate — the three things the
|
||||
/// keyboard needs three keystrokes for, and `filter-to-item` is the activation.
|
||||
fn click_category_row(state: &ClientState, index: usize) -> Option<Vec<Command>> {
|
||||
@@ -456,6 +473,9 @@ fn composed(
|
||||
"Move cursor to a category header to change axis".to_string(),
|
||||
)],
|
||||
}),
|
||||
// ── the formula panel ────────────────────────────────────────
|
||||
"edit-formula-at-cursor" => Some(edit_formula_at(state, state.view.formula_cursor)),
|
||||
|
||||
"open-item-add-at-cursor" => Some(match cat_at_cursor(state) {
|
||||
Some(cat) => vec![Command::ChangeMode(AppMode::item_add(cat))],
|
||||
None => vec![Command::SetStatus(
|
||||
@@ -1333,10 +1353,11 @@ fn parse_axis(s: &str) -> Option<Axis> {
|
||||
}
|
||||
}
|
||||
|
||||
/// A mirror of `commit.rs`'s `CommitKind`, resolved against the mode exactly
|
||||
/// as `parse_commit_kind` does — only `Item` needs the mode.
|
||||
/// A mirror of `commit.rs`'s `CommitKind`, resolved against the mode as
|
||||
/// `parse_commit_kind` then `CommitKind::in_mode` do; there is no registry here.
|
||||
enum CommitKind {
|
||||
Formula,
|
||||
ReplaceFormula { id: FormulaId, was: String },
|
||||
Category,
|
||||
Item { category: String },
|
||||
}
|
||||
@@ -1344,6 +1365,16 @@ enum CommitKind {
|
||||
impl CommitKind {
|
||||
fn of(arg: &str, mode: &AppMode) -> Option<Self> {
|
||||
match (arg, mode) {
|
||||
(
|
||||
"formula",
|
||||
AppMode::FormulaEdit {
|
||||
slot: FormulaSlot::Existing { id, was },
|
||||
..
|
||||
},
|
||||
) => Some(Self::ReplaceFormula {
|
||||
id: id.clone(),
|
||||
was: was.clone(),
|
||||
}),
|
||||
("formula", _) => Some(Self::Formula),
|
||||
("category", _) => Some(Self::Category),
|
||||
("item", AppMode::ItemAdd { category, .. }) => Some(Self::Item {
|
||||
@@ -1355,7 +1386,7 @@ impl CommitKind {
|
||||
|
||||
fn buffer(&self) -> &'static str {
|
||||
match self {
|
||||
Self::Formula => "formula",
|
||||
Self::Formula | Self::ReplaceFormula { .. } => "formula",
|
||||
Self::Category => "category",
|
||||
Self::Item { .. } => "item",
|
||||
}
|
||||
@@ -1363,14 +1394,14 @@ impl CommitKind {
|
||||
|
||||
fn on_empty(&self) -> AppMode {
|
||||
match self {
|
||||
Self::Formula => AppMode::FormulaPanel,
|
||||
Self::Formula | Self::ReplaceFormula { .. } => AppMode::FormulaPanel,
|
||||
Self::Category | Self::Item { .. } => AppMode::CategoryPanel,
|
||||
}
|
||||
}
|
||||
|
||||
fn on_commit(&self) -> Option<AppMode> {
|
||||
match self {
|
||||
Self::Formula => Some(AppMode::FormulaPanel),
|
||||
Self::Formula | Self::ReplaceFormula { .. } => Some(AppMode::FormulaPanel),
|
||||
Self::Category | Self::Item { .. } => None,
|
||||
}
|
||||
}
|
||||
@@ -1378,6 +1409,7 @@ impl CommitKind {
|
||||
fn status(&self, name: &str) -> String {
|
||||
match self {
|
||||
Self::Formula => "Formula added".to_string(),
|
||||
Self::ReplaceFormula { .. } => "Formula updated".to_string(),
|
||||
Self::Category => format!("Added category \"{name}\""),
|
||||
Self::Item { .. } => format!("Added \"{name}\""),
|
||||
}
|
||||
@@ -1389,6 +1421,11 @@ impl CommitKind {
|
||||
raw: name.to_string(),
|
||||
target_category: "_Measure".to_string(),
|
||||
},
|
||||
Self::ReplaceFormula { id, was } => ModelCommand::ReplaceFormula {
|
||||
id: id.clone(),
|
||||
was: was.clone(),
|
||||
raw: name.to_string(),
|
||||
},
|
||||
Self::Category => ModelCommand::AddCategory(name.to_string()),
|
||||
Self::Item { category } => ModelCommand::AddItem {
|
||||
category: category.clone(),
|
||||
@@ -1944,11 +1981,12 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// The formula panel binds no `Enter`, so the rule yields navigation and
|
||||
/// nothing else. A mode change here would be an invented activation.
|
||||
/// The formula panel's `Enter` edits (improvise-km71), so a click opens the
|
||||
/// editor on the row CLICKED — `Margin`, where the cursor is not.
|
||||
#[test]
|
||||
fn a_formula_row_click_navigates_and_activates_nothing() {
|
||||
fn a_formula_row_click_navigates_and_opens_the_editor_on_that_row() {
|
||||
let state = with_panels();
|
||||
let margin = state.formulas.formulas[1].clone();
|
||||
let commands = click(&state, Click::FormulaRow(1));
|
||||
assert_eq!(
|
||||
commands,
|
||||
@@ -1962,28 +2000,30 @@ mod tests {
|
||||
panel: Panel::Formula,
|
||||
cursor: 1
|
||||
},
|
||||
Command::ChangeMode(AppMode::formula_replace(margin.id, margin.raw.clone())),
|
||||
Command::SetBuffer {
|
||||
name: "formula".into(),
|
||||
value: margin.raw,
|
||||
},
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
/// The asymmetry is a fact about the keymap, so it is asserted against
|
||||
/// the keymap — not against a reading of it. See `context/code/clients.md`.
|
||||
/// A click mirrors `Enter`, so the tie is asserted against what `Enter`
|
||||
/// resolves to, not against a reading of it. See `context/code/clients.md`.
|
||||
#[test]
|
||||
fn the_formula_panel_has_no_enter_to_mirror_and_the_view_panel_does() {
|
||||
let keymaps = Keymaps::shared();
|
||||
fn a_formula_row_click_ends_with_what_enter_does_on_that_row() {
|
||||
let mut state = with_panels();
|
||||
state.view.mode = AppMode::FormulaPanel;
|
||||
state.view.formula_cursor = 1;
|
||||
let KeyOutcome::Local(enter) = resolve(&state, ev(KeyCode::Enter)) else {
|
||||
panic!("Enter in the formula panel resolves here");
|
||||
};
|
||||
assert!(!enter.is_empty(), "Enter on a formula must do something");
|
||||
let clicked = click(&with_panels(), Click::FormulaRow(1));
|
||||
assert!(
|
||||
keymaps
|
||||
.lookup(&ModeKey::FormulaPanel, KeyCode::Enter, KeyModifiers::NONE)
|
||||
.is_none(),
|
||||
"if FormulaPanel gains an Enter binding, click_formula_row must \
|
||||
activate too — the click mirrors Enter, and this is the only \
|
||||
thing tying the two together"
|
||||
);
|
||||
assert!(
|
||||
keymaps
|
||||
.lookup(&ModeKey::ViewPanel, KeyCode::Enter, KeyModifiers::NONE)
|
||||
.is_some(),
|
||||
"and the view panel's click activates because its Enter does"
|
||||
clicked.ends_with(&enter),
|
||||
"the click's activation must be Enter's: {clicked:?} vs {enter:?}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -2195,7 +2235,7 @@ mod tests {
|
||||
"Net = Profit",
|
||||
]
|
||||
.into_iter()
|
||||
.map(str::to_string)
|
||||
.map(crate::FormulaEntry::from)
|
||||
.collect();
|
||||
state.views.views = ["Default", "Second", "Third"]
|
||||
.into_iter()
|
||||
|
||||
@@ -109,6 +109,24 @@ impl Client {
|
||||
.map_err(|e| JsError::new(&e.to_string()))
|
||||
}
|
||||
|
||||
/// What the editor opened on `target` shows: the seed the target captured,
|
||||
/// never the page's paint, which can be stale (improvise-ugj4).
|
||||
#[wasm_bindgen]
|
||||
pub fn edit_seed(&self, target: &str) -> Result<String, JsError> {
|
||||
let target: EditTarget =
|
||||
serde_json::from_str(target).map_err(|e| JsError::new(&e.to_string()))?;
|
||||
Ok(target.opened.seed)
|
||||
}
|
||||
|
||||
/// `target` after the user's first edit of its `<input>`, still opaque to
|
||||
/// the page (improvise-jibp).
|
||||
#[wasm_bindgen]
|
||||
pub fn edited_target(&self, target: &str) -> Result<String, JsError> {
|
||||
let target: EditTarget =
|
||||
serde_json::from_str(target).map_err(|e| JsError::new(&e.to_string()))?;
|
||||
serde_json::to_string(&target.edited()).map_err(|e| JsError::new(&e.to_string()))
|
||||
}
|
||||
|
||||
/// The frame for "the user typed `text` into the cell `target` names". By
|
||||
/// identity, never position: rows renumber while an editor is open.
|
||||
#[wasm_bindgen]
|
||||
@@ -134,8 +152,8 @@ impl Client {
|
||||
self.clicked(Click::ViewRow(index))
|
||||
}
|
||||
|
||||
/// The user clicked row `index` of the formula panel. Navigation only —
|
||||
/// that panel binds no `Enter` (improvise-iioi).
|
||||
/// The user clicked row `index` of the formula panel: focus it, move there,
|
||||
/// then open the editor on that formula, as `Enter` does (improvise-km71).
|
||||
#[wasm_bindgen]
|
||||
pub fn click_formula(&mut self, index: usize) -> Result<Option<String>, JsError> {
|
||||
self.clicked(Click::FormulaRow(index))
|
||||
@@ -267,6 +285,62 @@ mod tests {
|
||||
assert_eq!(client.typed_cell_frame(0, 0, "42"), None);
|
||||
}
|
||||
|
||||
/// BUG (improvise-ugj4, improvise-jibp) — see `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn the_page_opens_its_editor_on_what_the_target_captured() {
|
||||
let id = improvise_protocol::RecordCell {
|
||||
record: surrogate("fe4c8e5e9cbd578e0d388f4b2b5a7879"),
|
||||
measure: Some("Revenue".into()),
|
||||
};
|
||||
let at = |text: &str, stored: &str| {
|
||||
let mut cell = improvise_protocol::CellDisplay::number(text);
|
||||
cell.edit_text = Some(stored.into());
|
||||
cell.id = Some(CellId::Record(id.clone()));
|
||||
Projection::Grid(GridUpdate::Cells(vec![((0, 0), cell)]))
|
||||
};
|
||||
let mut client = init();
|
||||
client.bootstrap(Snapshot {
|
||||
view: ViewState::default(),
|
||||
seed: vec![
|
||||
Projection::Grid(GridUpdate::RowFrame(AxisFrame::flat(["North"]))),
|
||||
Projection::Grid(GridUpdate::ColFrame(AxisFrame::flat(["Revenue"]))),
|
||||
at("1,200.50", "1200.5"),
|
||||
Projection::Basis(improvise_protocol::basis::published(
|
||||
0,
|
||||
0,
|
||||
[(id.record, 0)].into_iter().collect(),
|
||||
)),
|
||||
],
|
||||
});
|
||||
let first = client.edit_target(0, 0).unwrap().expect("a record cell");
|
||||
assert_eq!(
|
||||
client.edit_seed(&first).unwrap(),
|
||||
"1200.5",
|
||||
"not the drawn text"
|
||||
);
|
||||
client.fold_frame(ServerFrame::Commands(vec![Command::Project(at(
|
||||
"777", "777",
|
||||
))]));
|
||||
let later = client.edit_target(0, 0).unwrap().expect("a record cell");
|
||||
assert_eq!(
|
||||
client.edit_seed(&later).unwrap(),
|
||||
"777",
|
||||
"not the cache at open"
|
||||
);
|
||||
assert_eq!(
|
||||
client.edit_seed(&first).unwrap(),
|
||||
"1200.5",
|
||||
"a target is one capture"
|
||||
);
|
||||
|
||||
assert!(client.typed_cell(&later, "777").unwrap().is_none());
|
||||
let edited = client.edited_target(&later).unwrap();
|
||||
assert!(
|
||||
client.typed_cell(&edited, "777").unwrap().is_some(),
|
||||
"an edited editor typed back to its seed composed nothing"
|
||||
);
|
||||
}
|
||||
|
||||
/// The end-to-end shape a JS harness drives: bootstrap, key, message.
|
||||
#[test]
|
||||
fn a_client_can_be_driven_entirely_through_json() {
|
||||
|
||||
@@ -133,9 +133,6 @@ pub struct RenderCell {
|
||||
/// May this client compose a write for it? `CellId::Record` alone.
|
||||
pub editable: bool,
|
||||
pub selected: bool,
|
||||
/// What an editor should start with: the stored value, not the formatted
|
||||
/// one. Seeding from `text` turns `1200` into the string `"1,200"`.
|
||||
pub edit_text: String,
|
||||
}
|
||||
|
||||
impl RenderCell {
|
||||
@@ -155,7 +152,6 @@ impl RenderCell {
|
||||
| None => false,
|
||||
},
|
||||
selected,
|
||||
edit_text: cell.edit_text.clone().unwrap_or_default(),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -377,8 +373,8 @@ fn formula_rows(state: &ClientState) -> Option<Vec<RenderFormulaRow>> {
|
||||
.formulas
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, text)| RenderFormulaRow {
|
||||
text: text.clone(),
|
||||
.map(|(i, entry)| RenderFormulaRow {
|
||||
text: entry.raw.clone(),
|
||||
selected: i == state.view.formula_cursor,
|
||||
})
|
||||
.collect()
|
||||
@@ -1026,10 +1022,6 @@ mod tests {
|
||||
"a row says which of the two it is; the page does not guess"
|
||||
);
|
||||
assert_eq!(value["rows"][0]["labels"][0], "North");
|
||||
assert_eq!(
|
||||
value["rows"][0]["cells"][0]["edit_text"], "1200",
|
||||
"an editor seeds from the stored value, never from the drawn one"
|
||||
);
|
||||
}
|
||||
|
||||
fn page_axis(cat: &str, selected: &str, items: Option<Vec<&str>>) -> PageAxis {
|
||||
|
||||
@@ -12,6 +12,8 @@ use serde::{Deserialize, Serialize};
|
||||
/// A struct so a field such as the seed can join it — `context/code/clients.md`.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct EditTarget {
|
||||
/// The document the editor opened on; a swap under it refuses the commit (improvise-vndi).
|
||||
pub document_generation: u64,
|
||||
pub cell: RecordCell,
|
||||
/// The record's write counter as this client had it at open.
|
||||
pub version: u64,
|
||||
@@ -19,6 +21,20 @@ pub struct EditTarget {
|
||||
pub opened: OpenedEditor,
|
||||
}
|
||||
|
||||
impl EditTarget {
|
||||
/// The same target after the user's first edit: a commit of its seed is then
|
||||
/// a write (improvise-jibp). The page's `<input>` is not a buffer `resolve` sees.
|
||||
pub fn edited(self) -> Self {
|
||||
Self {
|
||||
opened: OpenedEditor {
|
||||
touched: true,
|
||||
..self.opened
|
||||
},
|
||||
..self
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// What a refused batch says to the person who composed it. Worded here rather
|
||||
/// than on `Rejection`, and why the wording matters: `context/code/clients.md`.
|
||||
fn rejection_message(rejection: &Rejection) -> String {
|
||||
@@ -103,9 +119,12 @@ impl Client {
|
||||
/// version of it the user is looking at. `None` before any basis arrives.
|
||||
pub fn edit_target_at(&self, row: usize, col: usize) -> Option<EditTarget> {
|
||||
let cell = self.record_cell_at(row, col)?;
|
||||
let version = self.state().basis.current()?.record(cell.record)?;
|
||||
let basis = self.state().basis.current()?;
|
||||
let version = basis.record(cell.record)?;
|
||||
let document_generation = basis.document_generation;
|
||||
let seed = self.state().grid.cell(row, col)?.edit_text.clone();
|
||||
Some(EditTarget {
|
||||
document_generation,
|
||||
cell,
|
||||
version,
|
||||
opened: OpenedEditor::on(seed.unwrap_or_default()),
|
||||
@@ -118,10 +137,10 @@ impl Client {
|
||||
if untouched(Some(&target.opened), text) {
|
||||
return None;
|
||||
}
|
||||
let basis = self
|
||||
.state()
|
||||
.basis
|
||||
.claim_as_read([(target.cell.record, target.version)])?;
|
||||
let basis = self.state().basis.claim_as_read(
|
||||
target.document_generation,
|
||||
[(target.cell.record, target.version)],
|
||||
)?;
|
||||
let cmds = vec![Command::Model {
|
||||
basis,
|
||||
cmd: typed_cell_command(&improvise_protocol::CellId::Record(target.cell), text)?,
|
||||
@@ -304,6 +323,24 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// BUG (improvise-jibp) — see `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn a_mouse_target_marked_edited_writes_its_own_seed() {
|
||||
let mut client = client_showing(Some(CellId::Record(record("Revenue"))));
|
||||
let target = client.edit_target_at(0, 0).expect("a record cell");
|
||||
assert_eq!(
|
||||
client.typed_target_frame(target.clone(), "1200"),
|
||||
None,
|
||||
"control: an untouched editor committing its seed writes nothing"
|
||||
);
|
||||
let edited = target.edited();
|
||||
assert_eq!(edited.opened.seed, "1200", "marking must not move the seed");
|
||||
assert!(
|
||||
client.typed_target_frame(edited, "1200").is_some(),
|
||||
"an edited editor typed back to its seed was dropped"
|
||||
);
|
||||
}
|
||||
|
||||
/// BUG (improvise-ixxx) — see `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn a_mouse_commit_is_judged_against_the_seed_it_opened_with() {
|
||||
|
||||
@@ -817,6 +817,70 @@ fn an_edit_is_refused_when_the_cell_was_cleared_while_the_editor_was_open() {
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-vndi: an editor opened before another client's `LoadModel` must not
|
||||
/// write into the loaded document. Evidence: test-evidence/clients.md.
|
||||
#[test]
|
||||
fn an_edit_opened_before_a_document_swap_is_refused_in_the_new_document() {
|
||||
use improvise_command::ui::reduce::caught_up;
|
||||
use improvise_protocol::{ModelCommand, Rejection, ServerFrame};
|
||||
|
||||
let north = CellKey::new(vec![
|
||||
("Region".into(), "North".into()),
|
||||
("_Measure".into(), "Revenue".into()),
|
||||
]);
|
||||
// The incoming document keeps every name, view included (gotcha 39's
|
||||
// fixture warning), and differs only in the one value the edit targets.
|
||||
let path = std::path::PathBuf::from("other.improv");
|
||||
let mut host = host();
|
||||
host.app_mut().storage = Box::new(improvise_command::storage::MemoryFs::new());
|
||||
let mut other = host.app().model_state.workbook.clone();
|
||||
other
|
||||
.model
|
||||
.set_cell(north.clone(), CellValue::Number(5555.0));
|
||||
improvise_command::persistence::save_to(&*host.app().storage, &other, &path).unwrap();
|
||||
|
||||
let mut editor = Client::new();
|
||||
let (a, snapshot) = host.connect(10, 10);
|
||||
deliver(&mut editor, &[snapshot]);
|
||||
let (b, _) = host.connect(10, 10);
|
||||
|
||||
// A opens an editor on North/Revenue, which shows 1200.
|
||||
let target = editor
|
||||
.edit_target(0, 0)
|
||||
.expect("serialization must not fail")
|
||||
.expect("a stored cell has an edit target");
|
||||
|
||||
// B loads the other document; A folds its rebase Snapshot, editor still open.
|
||||
let generation = host.app().document_generation();
|
||||
let load = caught_up(host.app(), ModelCommand::LoadModel(path));
|
||||
let relayed = host.handle(b, ClientFrame::command(load));
|
||||
assert_eq!(
|
||||
host.app().document_generation(),
|
||||
generation + 1,
|
||||
"setup: the load must really replace the document"
|
||||
);
|
||||
let for_a: Vec<Addressed> = relayed.into_iter().filter(|f| f.to == a).collect();
|
||||
deliver(&mut editor, &for_a);
|
||||
|
||||
let commit = editor
|
||||
.typed_cell(&target, "42")
|
||||
.expect("serialization must not fail")
|
||||
.expect("a stored cell composes a write");
|
||||
let answer = send(&mut host, a, &commit);
|
||||
|
||||
assert!(
|
||||
answer
|
||||
.iter()
|
||||
.any(|f| matches!(f.msg, ServerFrame::Rejected(Rejection::DocumentReplaced))),
|
||||
"the edit was composed against the outgoing document: {answer:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
host.app().model_state.workbook.model.get_cell(&north),
|
||||
Some(&CellValue::Number(5555.0)),
|
||||
"the loaded document's value was overwritten by an edit its user never saw"
|
||||
);
|
||||
}
|
||||
|
||||
// ── numbers across the serialized wire (improvise-6oy4) ─────────────────
|
||||
|
||||
/// A value serde_json's default f64 parser reads back one ULP low. Evidence file.
|
||||
|
||||
@@ -2267,3 +2267,96 @@ fn every_projected_edit_text_commits_back_to_its_value() {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const FORMULAS: [&str; 3] = [
|
||||
"Profit = Revenue * 2",
|
||||
"Tax = Revenue * 0.1",
|
||||
"Margin = Profit / Revenue",
|
||||
];
|
||||
|
||||
fn add_formulas(app: &mut App) {
|
||||
for raw in FORMULAS {
|
||||
let f = improvise_command::formula::parse_formula(raw, "_Measure").unwrap();
|
||||
app.model_state.workbook.model.add_formula(f);
|
||||
}
|
||||
app.rebuild_layout();
|
||||
}
|
||||
|
||||
fn raws(app: &App) -> Vec<String> {
|
||||
let formulas = app.model_state.workbook.model.formulas();
|
||||
formulas.iter().map(|f| f.raw.clone()).collect()
|
||||
}
|
||||
|
||||
/// The formula panel's edit (improvise-km71), opened at a NONZERO cursor over
|
||||
/// three formulas, typed into and committed, with both ends agreeing throughout.
|
||||
#[test]
|
||||
fn editing_a_formula_from_the_panel_agrees_with_the_host() {
|
||||
let mut app = furnished_host();
|
||||
add_formulas(&mut app);
|
||||
app.view_state.formula_panel_open = true;
|
||||
app.view_state.mode = AppMode::FormulaPanel;
|
||||
app.view_state.formula_cursor = 1;
|
||||
let mut twin = twin_of(&app);
|
||||
add_formulas(&mut twin);
|
||||
let mut client = furnished_client(&app, (2, 1));
|
||||
client.formulas = improvise_command::ui::project::formulas_of(&app);
|
||||
|
||||
both_ends(&mut app, &mut twin, &mut client, "Enter");
|
||||
assert_eq!(
|
||||
client.view.buffers.get("formula"),
|
||||
app.view_state.buffers.get("formula"),
|
||||
"the two ends seeded the editor differently"
|
||||
);
|
||||
assert_eq!(
|
||||
app.view_state.buffers.get("formula").map(String::as_str),
|
||||
Some(FORMULAS[1]),
|
||||
"Enter must seed the formula under cursor 1"
|
||||
);
|
||||
for key in ["Backspace", "2", "Enter"] {
|
||||
both_ends(&mut app, &mut twin, &mut client, key);
|
||||
}
|
||||
assert_eq!(
|
||||
raws(&app),
|
||||
vec![FORMULAS[0], "Tax = Revenue * 0.2", FORMULAS[2]],
|
||||
"the commit must replace the formula it opened on, in place"
|
||||
);
|
||||
}
|
||||
|
||||
/// A rename through the browser: the one input that separates "replace" from
|
||||
/// "add", since `add_formula` upserts by target. The status diverges on purpose.
|
||||
#[test]
|
||||
fn renaming_a_formula_from_the_panel_lands_the_same_document_as_the_host() {
|
||||
let mut app = furnished_host();
|
||||
add_formulas(&mut app);
|
||||
app.view_state.formula_panel_open = true;
|
||||
app.view_state.mode = AppMode::FormulaPanel;
|
||||
app.view_state.formula_cursor = 1;
|
||||
let mut twin = twin_of(&app);
|
||||
add_formulas(&mut twin);
|
||||
let mut client = furnished_client(&app, (2, 1));
|
||||
client.formulas = improvise_command::ui::project::formulas_of(&app);
|
||||
both_ends(&mut app, &mut twin, &mut client, "Enter");
|
||||
for view in [&mut app.view_state, &mut twin.view_state, &mut client.view] {
|
||||
view.buffers
|
||||
.insert("formula".into(), "Levy = Revenue * 0.1".into());
|
||||
}
|
||||
|
||||
let event = BrowserKey::new("Enter").to_key_event().unwrap();
|
||||
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
|
||||
panic!("Enter in the formula editor resolves here");
|
||||
};
|
||||
for cmd in &cmds {
|
||||
client.fold(cmd);
|
||||
}
|
||||
apply_on(&mut twin, &cmds);
|
||||
reduce_key(&mut app, event);
|
||||
|
||||
assert_eq!(document_text(&twin), document_text(&app));
|
||||
assert_eq!(
|
||||
raws(&app),
|
||||
vec![FORMULAS[0], "Levy = Revenue * 0.1", FORMULAS[2]]
|
||||
);
|
||||
assert_eq!(client.view.mode, app.view_state.mode);
|
||||
assert_eq!(client.view.status_msg, "Formula updated");
|
||||
assert_eq!(app.view_state.status_msg, "Renamed Tax to Levy");
|
||||
}
|
||||
|
||||
Generated
+1
@@ -196,6 +196,7 @@ dependencies = [
|
||||
"anyhow",
|
||||
"pest",
|
||||
"pest_derive",
|
||||
"serde",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
@@ -9,6 +9,19 @@ import { test, expect } from "@playwright/test";
|
||||
// the same with the handshake window).
|
||||
import { NOTICE_KEY } from "../../web/shell.js";
|
||||
|
||||
/** Every string the page posts to its worker, kept on `window.__posted`. */
|
||||
function recordPosts() {
|
||||
window.__posted = [];
|
||||
const post = Worker.prototype.postMessage;
|
||||
Worker.prototype.postMessage = function (message, ...rest) {
|
||||
window.__posted.push(String(message));
|
||||
return post.call(this, message, ...rest);
|
||||
};
|
||||
}
|
||||
|
||||
const setCells = (page) =>
|
||||
page.evaluate(() => window.__posted.filter((m) => m.includes("SetCell")).length);
|
||||
|
||||
async function openApp(page) {
|
||||
await page.goto("/app/index.html");
|
||||
await expect(page.locator("#grid table")).toBeVisible({ timeout: 20_000 });
|
||||
@@ -251,15 +264,16 @@ test.describe("a pointer event reaches the client and its answer is painted", ()
|
||||
await expect(page.locator("#views li.active")).toContainText("Season");
|
||||
await expect(page.locator("#views li.selected")).toContainText("Season");
|
||||
|
||||
// A formula row: navigation only, because FormulaPanel binds no Enter and
|
||||
// a click mirrors Enter. The Rust guard is on the KEYMAP — clients.md.
|
||||
// `#grid input` toHaveCount(0) belongs here and is deliberately absent:
|
||||
// it cannot fail, measured by injection (improvise-qf57, clients.md).
|
||||
// A formula row: FormulaPanel's Enter edits the formula (improvise-km71)
|
||||
// and a click mirrors Enter, so the click opens the editor on that row.
|
||||
// `#grid input` toHaveCount(0) is deliberately absent (improvise-qf57).
|
||||
await page.keyboard.press("Shift+F");
|
||||
await expect(page.locator("#formulas li").first()).toBeVisible();
|
||||
await page.keyboard.press("Escape");
|
||||
const formula = await page.locator("#formulas li").first().innerText();
|
||||
await page.locator("#formulas li").first().click();
|
||||
await expect(page.locator("#mode")).toHaveText("FORMULAS");
|
||||
await expect(page.locator("#mode")).toHaveText("FORMULA");
|
||||
await expect(page.locator("#minibuffer")).toContainText(formula.trim());
|
||||
await expect(page.locator("#formulas li.selected")).toHaveCount(1);
|
||||
});
|
||||
|
||||
@@ -321,6 +335,76 @@ test.describe("a pointer event reaches the client and its answer is painted", ()
|
||||
await expect.poll(writes).toBe(1);
|
||||
});
|
||||
|
||||
// improvise-jibp. The page reports its first edit, so retyping the seed over
|
||||
// the selection is a write, not an untouched editor.
|
||||
test("a mouse editor typed back to its own seed still writes", async ({ page }) => {
|
||||
await page.addInitScript(recordPosts);
|
||||
await openApp(page);
|
||||
const cells = page.locator("#grid tbody td");
|
||||
await cells.nth(4).click();
|
||||
const input = cells.nth(4).locator("input");
|
||||
await expect(input).toHaveCount(1);
|
||||
const seed = await input.inputValue();
|
||||
expect(seed, "the fixture cell has no seed to retype").not.toBe("");
|
||||
await page.keyboard.type(seed);
|
||||
await expect(input).toHaveValue(seed);
|
||||
await page.keyboard.press("Enter");
|
||||
await expect.poll(() => setCells(page)).toBe(1);
|
||||
});
|
||||
|
||||
// improvise-ugj4. A frame that lands while an editor is open changes the
|
||||
// cache but not the paint; the next editor must open on what it captured.
|
||||
test("an editor opened after an unpainted write shows and commits what it captured", async ({
|
||||
page,
|
||||
}) => {
|
||||
await page.addInitScript(recordPosts);
|
||||
await page.addInitScript(() => {
|
||||
window.__frames = [];
|
||||
const add = Worker.prototype.addEventListener;
|
||||
Worker.prototype.addEventListener = function (type, listener, ...rest) {
|
||||
if (type !== "message") return add.call(this, type, listener, ...rest);
|
||||
window.__deliver = (data) => listener.call(this, new MessageEvent("message", { data }));
|
||||
const recorded = (event) => {
|
||||
if (typeof event.data === "string") window.__frames.push(event.data);
|
||||
return listener.call(this, event);
|
||||
};
|
||||
return add.call(this, type, recorded, ...rest);
|
||||
};
|
||||
});
|
||||
await openApp(page);
|
||||
const cells = page.locator("#grid tbody td");
|
||||
const target = cells.nth(4);
|
||||
const where = await target.evaluate((td) => [Number(td.dataset.row), Number(td.dataset.col)]);
|
||||
|
||||
await cells.nth(0).click();
|
||||
await expect(cells.nth(0).locator("input")).toHaveCount(1);
|
||||
// Another client's write to cell 4, delivered while cell 0's editor is open.
|
||||
await page.evaluate(([row, col]) => {
|
||||
let latest = null;
|
||||
const walk = (node) => {
|
||||
if (Array.isArray(node)) node.forEach(walk);
|
||||
else if (node && typeof node === "object") {
|
||||
for (const [key, value] of Object.entries(node)) {
|
||||
if (key === "Cells" && Array.isArray(value)) {
|
||||
for (const [[r, c], cell] of value) if (r === row && c === col) latest = cell;
|
||||
} else walk(value);
|
||||
}
|
||||
}
|
||||
};
|
||||
window.__frames.forEach((f) => walk(JSON.parse(f)));
|
||||
const moved = { ...latest, text: "777", edit_text: "777", value: { Number: 777 } };
|
||||
window.__deliver(JSON.stringify({ Commands: [{ Project: { Grid: { Cells: [[[row, col], moved]] } } }] }));
|
||||
}, where);
|
||||
|
||||
await target.click();
|
||||
const input = target.locator("input");
|
||||
await expect(input).toHaveCount(1);
|
||||
await expect(input, "the editor opened on the stale paint").toHaveValue("777");
|
||||
await page.keyboard.press("Enter");
|
||||
await page.waitForTimeout(300);
|
||||
expect(await setCells(page), "an untouched editor wrote the stale text").toBe(0);
|
||||
});
|
||||
|
||||
// improvise-b8ot moved these five surfaces onto one handler per stable root.
|
||||
// The three tests below are the properties a naive delegation loses, each of
|
||||
// which is silent: the page still paints, and only the pointer stops working.
|
||||
|
||||
+50
-2
@@ -79,7 +79,7 @@ pub fn run_tui(
|
||||
if event::poll(Duration::from_millis(100))? {
|
||||
match event::read()? {
|
||||
Event::Key(key) => {
|
||||
app.handle_key(convert_key_event(key))?;
|
||||
press(&mut app, convert_key_event(key))?;
|
||||
}
|
||||
Event::Resize(w, h) => {
|
||||
app.term_width = w;
|
||||
@@ -99,6 +99,13 @@ pub fn run_tui(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// One keypress in the TUI. A message lasts until the next key, so it is drawn
|
||||
/// once and then gives way to the hint (improvise-cq3d, `context/code/tui.md`).
|
||||
fn press(app: &mut App, key: key::KeyEvent) -> Result<()> {
|
||||
app.apply_effects(vec![Box::new(crate::ui::effect::SetStatus(String::new()))]);
|
||||
app.handle_key(key)
|
||||
}
|
||||
|
||||
// ── Key event conversion (TUI boundary) ──────────────────────────────────────
|
||||
|
||||
// The only place crossterm's key types and `command::key` may both appear.
|
||||
@@ -566,7 +573,8 @@ fn draw_welcome(f: &mut Frame, area: Rect) {
|
||||
|
||||
#[cfg(test)]
|
||||
mod bottom_bar_tests {
|
||||
use super::{draw_bottom_bar, minibuffer_line};
|
||||
use super::{draw_bottom_bar, minibuffer_line, press};
|
||||
use crate::command::key::{KeyCode, KeyEvent, KeyModifiers};
|
||||
use crate::ui::app::{App, AppMode};
|
||||
use crate::workbook::Workbook;
|
||||
use ratatui::{Terminal, backend::TestBackend, layout::Rect};
|
||||
@@ -587,6 +595,46 @@ mod bottom_bar_tests {
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// A message lasts until the next key, then the derived hint is back
|
||||
/// (improvise-cq3d). Why each step: `context/test-evidence/tui.md`.
|
||||
#[test]
|
||||
fn a_status_message_gives_way_to_the_hint_on_the_next_key() {
|
||||
let key = |code| KeyEvent::new(code, KeyModifiers::NONE);
|
||||
let mut app = App::new(Workbook::new("T"), None);
|
||||
app.view_state.status_msg = "Saved to x.improv".to_string();
|
||||
|
||||
press(&mut app, key(KeyCode::Char('T'))).unwrap();
|
||||
assert!(matches!(app.view_state.mode, AppMode::TileSelect));
|
||||
let row = bottom_row(&app, 240);
|
||||
assert!(
|
||||
!row.contains("Saved to"),
|
||||
"a stale message outlived a key: {row:?}"
|
||||
);
|
||||
assert!(
|
||||
row.contains(&app.hint_text()),
|
||||
"no hint in TileSelect: {row:?}"
|
||||
);
|
||||
|
||||
press(&mut app, key(KeyCode::Char(' '))).unwrap();
|
||||
let said = app.view_state.status_msg.clone();
|
||||
assert!(!said.is_empty(), "the axis cycle must say what it did");
|
||||
assert!(
|
||||
bottom_row(&app, 240).contains(&said),
|
||||
"a message was wiped before it was drawn"
|
||||
);
|
||||
|
||||
press(&mut app, key(KeyCode::Esc)).unwrap();
|
||||
let row = bottom_row(&app, 240);
|
||||
assert!(
|
||||
!row.contains(&said),
|
||||
"the axis message outlived the next key: {row:?}"
|
||||
);
|
||||
assert!(
|
||||
row.contains(&app.hint_text()),
|
||||
"no hint back in Normal: {row:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// The wizard overlay draws correct per-step hints; the status bar stacked a
|
||||
/// second static table under it (improvise-rr8n) — `context/test-evidence/tui.md`.
|
||||
#[test]
|
||||
|
||||
+14
-4
@@ -30,7 +30,7 @@ impl PanelContent for FormulaContent<'_> {
|
||||
}
|
||||
|
||||
fn title(&self) -> &str {
|
||||
" Formulas [n]ew [d]elete "
|
||||
" Formulas [n]ew [e]dit [d]elete "
|
||||
}
|
||||
|
||||
fn item_count(&self) -> usize {
|
||||
@@ -42,7 +42,7 @@ impl PanelContent for FormulaContent<'_> {
|
||||
}
|
||||
|
||||
fn render_item(&self, index: usize, is_selected: bool, inner: Rect, buf: &mut Buffer) {
|
||||
let raw = &self.formulas.formulas[index];
|
||||
let raw = &self.formulas.formulas[index].raw;
|
||||
let style = if is_selected {
|
||||
Style::default()
|
||||
.fg(Color::Black)
|
||||
@@ -79,6 +79,8 @@ impl PanelContent for FormulaContent<'_> {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::formula::parse_formula;
|
||||
use crate::protocol::FormulaEntry;
|
||||
use crate::ui::app::App;
|
||||
use crate::ui::panel::Panel;
|
||||
use crate::workbook::Workbook;
|
||||
@@ -101,7 +103,10 @@ mod tests {
|
||||
/// improvise-8l0c — `context/test-evidence/tui.md`.
|
||||
fn render_formulas(raws: &[&str], width: u16) -> Buffer {
|
||||
let cache = FormulaCache {
|
||||
formulas: raws.iter().map(|s| (*s).to_string()).collect(),
|
||||
formulas: raws
|
||||
.iter()
|
||||
.map(|s| FormulaEntry::from(&parse_formula(s, "_Measure").unwrap()))
|
||||
.collect(),
|
||||
};
|
||||
let area = Rect::new(0, 0, width, 5);
|
||||
let mut buf = Buffer::empty(area);
|
||||
@@ -151,7 +156,12 @@ mod tests {
|
||||
.map(|f| f.raw.clone())
|
||||
.collect();
|
||||
assert_eq!(was.len(), 2, "fixture must carry formulas");
|
||||
assert_eq!(was, project::formulas_of(&app).formulas);
|
||||
let cached: Vec<String> = project::formulas_of(&app)
|
||||
.formulas
|
||||
.into_iter()
|
||||
.map(|f| f.raw)
|
||||
.collect();
|
||||
assert_eq!(was, cached);
|
||||
}
|
||||
|
||||
/// Pins the drawn rows, so a change to how the cache is read shows up.
|
||||
|
||||
+120
-43
@@ -1,30 +1,28 @@
|
||||
//! Every `.improv` document this repository ships must load (improvise-odbc).
|
||||
//! Why a DIRECTORY WALK and not a list: `context/test-evidence/tui.md`.
|
||||
//! "Shipped" is "tracked", read off git's index: `context/test-evidence/tui.md`.
|
||||
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
/// The directories `.gitignore` un-ignores `*.improv` in: the definition of
|
||||
/// "shipped", read from the one place that states it (improvise-qlrh).
|
||||
fn shipped_directories(root: &Path) -> Vec<PathBuf> {
|
||||
let gitignore = std::fs::read_to_string(root.join(".gitignore")).expect("a .gitignore");
|
||||
gitignore
|
||||
.lines()
|
||||
.filter_map(|line| line.trim().strip_prefix('!'))
|
||||
.filter_map(|pattern| pattern.strip_suffix("/*.improv"))
|
||||
.map(|dir| root.join(dir))
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn documents_in(dir: &Path) -> Vec<PathBuf> {
|
||||
let mut found: Vec<PathBuf> = std::fs::read_dir(dir)
|
||||
.expect("a shipped-document directory")
|
||||
.map(|entry| entry.expect("a readable directory entry").path())
|
||||
// `extension()` of `x.improv.gz` is `gz`, so matching on the extension
|
||||
// alone would skip it and put back the hole this test closes.
|
||||
.filter(|p| {
|
||||
p.to_str()
|
||||
.is_some_and(|s| s.ends_with(".improv") || s.ends_with(".improv.gz"))
|
||||
})
|
||||
/// Every tracked `.improv` and `.improv.gz` under `root`. Outside a checkout this
|
||||
/// panics rather than answering empty (improvise-c0aj).
|
||||
fn shipped_documents(root: &Path) -> Vec<PathBuf> {
|
||||
let out = std::process::Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(root)
|
||||
.args(["ls-files", "-z", "--", "*.improv", "*.improv.gz"])
|
||||
.output()
|
||||
.expect("git runs: this test reads the index, so it needs a checkout");
|
||||
assert!(
|
||||
out.status.success(),
|
||||
"git ls-files failed in {}: {}",
|
||||
root.display(),
|
||||
String::from_utf8_lossy(&out.stderr)
|
||||
);
|
||||
let mut found: Vec<PathBuf> = String::from_utf8(out.stdout)
|
||||
.expect("UTF-8 paths")
|
||||
.split('\0')
|
||||
.filter(|p| !p.is_empty())
|
||||
.map(|p| root.join(p))
|
||||
.collect();
|
||||
found.sort();
|
||||
found
|
||||
@@ -33,30 +31,109 @@ fn documents_in(dir: &Path) -> Vec<PathBuf> {
|
||||
#[test]
|
||||
fn every_shipped_document_loads() {
|
||||
let root = Path::new(env!("CARGO_MANIFEST_DIR"));
|
||||
let dirs = shipped_directories(root);
|
||||
let documents = shipped_documents(root);
|
||||
assert!(
|
||||
!dirs.is_empty(),
|
||||
"precondition: .gitignore must un-ignore at least one *.improv directory"
|
||||
!documents.is_empty(),
|
||||
"precondition: the index must list at least one shipped document"
|
||||
);
|
||||
|
||||
for dir in dirs {
|
||||
let documents = documents_in(&dir);
|
||||
// Per directory, so a walk that silently loses one cannot pass (qlrh).
|
||||
for path in documents {
|
||||
let workbook = improvise::persistence::load(&path)
|
||||
.unwrap_or_else(|e| panic!("{} does not load: {e}", path.display()));
|
||||
assert!(
|
||||
!documents.is_empty(),
|
||||
"precondition: {} is un-ignored for *.improv but the walk found none there",
|
||||
dir.display()
|
||||
workbook.views.contains_key(&workbook.active_view),
|
||||
"{} loads but its initial view {:?} is not one of {:?}",
|
||||
path.display(),
|
||||
workbook.active_view,
|
||||
workbook.views.keys().collect::<Vec<_>>()
|
||||
);
|
||||
for path in documents {
|
||||
let workbook = improvise::persistence::load(&path)
|
||||
.unwrap_or_else(|e| panic!("{} does not load: {e}", path.display()));
|
||||
assert!(
|
||||
workbook.views.contains_key(&workbook.active_view),
|
||||
"{} loads but its initial view {:?} is not one of {:?}",
|
||||
path.display(),
|
||||
workbook.active_view,
|
||||
workbook.views.keys().collect::<Vec<_>>()
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// A scratch repository, so each way of shipping a document is set up for real.
|
||||
fn scratch_repo() -> tempfile::TempDir {
|
||||
let dir = tempfile::tempdir().expect("a temp dir");
|
||||
run_git(dir.path(), &["init", "-q"]);
|
||||
dir
|
||||
}
|
||||
|
||||
fn run_git(root: &Path, args: &[&str]) {
|
||||
let status = std::process::Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(root)
|
||||
.args(args)
|
||||
.status()
|
||||
.expect("git runs");
|
||||
assert!(status.success(), "git {args:?} failed");
|
||||
}
|
||||
|
||||
fn write(root: &Path, rel: &str, text: &str) {
|
||||
let path = root.join(rel);
|
||||
std::fs::create_dir_all(path.parent().expect("a parent")).expect("dirs");
|
||||
std::fs::write(path, text).expect("writes");
|
||||
}
|
||||
|
||||
fn walked(root: &Path, rel: &str) -> bool {
|
||||
shipped_documents(root).contains(&root.join(rel))
|
||||
}
|
||||
|
||||
/// improvise-c0aj: one test per way of shipping a document that reading the
|
||||
/// root `.gitignore` missed. Evidence: `context/test-evidence/tui.md`.
|
||||
#[test]
|
||||
fn a_single_file_un_ignore_is_walked() {
|
||||
let repo = scratch_repo();
|
||||
let root = repo.path();
|
||||
write(root, ".gitignore", "*.improv\n!web/demo.improv\n");
|
||||
write(root, "web/demo.improv", "x");
|
||||
run_git(root, &["add", ".gitignore", "web/demo.improv"]);
|
||||
assert!(walked(root, "web/demo.improv"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_gzipped_document_is_walked() {
|
||||
let repo = scratch_repo();
|
||||
let root = repo.path();
|
||||
write(root, ".gitignore", "*.improv\n");
|
||||
write(root, "docs/model.improv.gz", "x");
|
||||
run_git(root, &["add", ".gitignore", "docs/model.improv.gz"]);
|
||||
assert!(walked(root, "docs/model.improv.gz"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_document_un_ignored_by_a_nested_gitignore_is_walked() {
|
||||
let repo = scratch_repo();
|
||||
let root = repo.path();
|
||||
write(root, ".gitignore", "*.improv\n");
|
||||
write(root, "harness/.gitignore", "!keep.improv\n");
|
||||
write(root, "harness/keep.improv", "x");
|
||||
run_git(
|
||||
root,
|
||||
&[
|
||||
"add",
|
||||
".gitignore",
|
||||
"harness/.gitignore",
|
||||
"harness/keep.improv",
|
||||
],
|
||||
);
|
||||
assert!(walked(root, "harness/keep.improv"));
|
||||
}
|
||||
|
||||
/// An empty walk would pass every per-document assertion vacuously, so a
|
||||
/// directory git cannot read must stop the test instead.
|
||||
#[test]
|
||||
#[should_panic(expected = "git ls-files failed")]
|
||||
fn a_directory_that_is_not_a_checkout_fails_loudly() {
|
||||
let not_a_repo = tempfile::tempdir().expect("a temp dir");
|
||||
shipped_documents(not_a_repo.path());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_force_added_document_is_walked() {
|
||||
let repo = scratch_repo();
|
||||
let root = repo.path();
|
||||
write(root, ".gitignore", "*.improv\n");
|
||||
write(root, "secret/forced.improv", "x");
|
||||
run_git(root, &["add", ".gitignore"]);
|
||||
run_git(root, &["add", "-f", "secret/forced.improv"]);
|
||||
assert!(walked(root, "secret/forced.improv"));
|
||||
}
|
||||
|
||||
+9
-8
@@ -546,20 +546,19 @@ function gridTable(model) {
|
||||
|
||||
/** Open an inline editor. Committing sends a frame; the value that comes
|
||||
* back is the server's, never the string typed here. */
|
||||
function edit(td, row, col, seed) {
|
||||
function edit(td, row, col) {
|
||||
if (editing) return;
|
||||
// WHICH cell, captured now: rows renumber while the editor is open, so the
|
||||
// position is not an address by commit time (improvise-2wu3).
|
||||
const target = client.edit_target(row, col);
|
||||
let target = client.edit_target(row, col);
|
||||
if (!target) return;
|
||||
// `seed` is `RenderCell::edit_text` -- the STORED value, not the drawn one:
|
||||
// `td.textContent` has been through the number format.
|
||||
const original = seed;
|
||||
// What was on the screen, kept so closing can put it back: `close` no longer
|
||||
// always repaints, so restoring `seed` would leave `1200` where `1,200` was.
|
||||
// always repaints, so restoring the seed would leave `1200` where `1,200` was.
|
||||
const drawn = td.textContent;
|
||||
const input = document.createElement("input");
|
||||
input.value = original;
|
||||
// The seed the target captured, never the paint, which may be stale while
|
||||
// another editor is open: one capture shown and compared (improvise-ugj4).
|
||||
input.value = client.edit_seed(target);
|
||||
td.replaceChildren(input);
|
||||
input.focus();
|
||||
input.select();
|
||||
@@ -590,6 +589,8 @@ function edit(td, row, col, seed) {
|
||||
|
||||
// These two stay DIRECT while the five surfaces above are delegated: one
|
||||
// transient node, one at a time, and `blur` does not bubble (improvise-b8ot).
|
||||
// The first edit marks the target, so retyping the seed is still a write.
|
||||
input.addEventListener("input", () => (target = client.edited_target(target)), { once: true });
|
||||
input.addEventListener("keydown", (e) => {
|
||||
if (e.key === "Enter") dismiss(true);
|
||||
else if (e.key === "Escape") dismiss(false);
|
||||
@@ -745,7 +746,7 @@ el.grid.addEventListener("mousedown", (event) => {
|
||||
if (frame) transport.send(frame);
|
||||
// Every cell moves the selection; an editable one also opens its editor.
|
||||
// NOT repainted in that branch: a repaint would take the <input> away.
|
||||
if (action.editable) edit(td, action.row, action.col, action.editText);
|
||||
if (action.editable) edit(td, action.row, action.col);
|
||||
else repaint();
|
||||
});
|
||||
|
||||
|
||||
+2
-3
@@ -222,7 +222,6 @@ export function gridPlan(model) {
|
||||
row: dataRow,
|
||||
col,
|
||||
text: typing ? `${model.editing}▏` : cell.text,
|
||||
editText: cell.edit_text,
|
||||
editable: cell.editable,
|
||||
classes: {
|
||||
numeric: cell.numeric && !typing,
|
||||
@@ -260,7 +259,7 @@ export function gridPlan(model) {
|
||||
/** What a data cell carries on its node, so one delegated handler can read
|
||||
* back the answers `gridPlan` already made (improvise-b8ot, clients.md). */
|
||||
export function cellAttrs(cell) {
|
||||
return { row: String(cell.row), col: String(cell.col), edit: cell.editText };
|
||||
return { row: String(cell.row), col: String(cell.col) };
|
||||
}
|
||||
|
||||
/** …and the inverse: what a delegated mousedown MEANS. `null` where the event
|
||||
@@ -271,7 +270,7 @@ export function cellAction(dataset, editable) {
|
||||
// A node that carries no coordinates is not a cell: every header and the
|
||||
// corner reach here, and none of them means anything.
|
||||
if (row === null || col === null) return null;
|
||||
return { row, col, editable, editText: dataset.edit ?? "" };
|
||||
return { row, col, editable };
|
||||
}
|
||||
|
||||
/** The same, for the surfaces whose rows are named by one index: the panels,
|
||||
|
||||
@@ -80,9 +80,13 @@ test("the page still reaches the client through a handful of calls", () => {
|
||||
"click_page_item",
|
||||
"click_tile",
|
||||
"click_view",
|
||||
// improvise-ugj4: the editor shows the seed its target captured, not the paint.
|
||||
"edit_seed",
|
||||
// improvise-2wu3/0j0f: what an editor edits, captured when it opens, so
|
||||
// its commit names that record at that version and not a position.
|
||||
"edit_target",
|
||||
// improvise-jibp: the first edit of the <input> marks the target touched.
|
||||
"edited_target",
|
||||
"on_frame",
|
||||
"on_key_event",
|
||||
"render_model",
|
||||
|
||||
@@ -16,7 +16,6 @@ import {
|
||||
|
||||
const cell = (text, extra = {}) => ({
|
||||
text,
|
||||
edit_text: text,
|
||||
numeric: true,
|
||||
error: false,
|
||||
selected: false,
|
||||
@@ -40,7 +39,7 @@ function grouped() {
|
||||
{
|
||||
kind: "data",
|
||||
labels: ["Cold Brew"],
|
||||
cells: [cell("3", { editable: false }), cell("4", { edit_text: "4000" })],
|
||||
cells: [cell("3", { editable: false }), cell("4")],
|
||||
},
|
||||
],
|
||||
};
|
||||
@@ -50,7 +49,7 @@ test("a cell's attributes carry the plan's answers back to the handler", () => {
|
||||
const plan = gridPlan(grouped());
|
||||
const last = plan.body.at(-1).cells[1];
|
||||
const action = cellAction(cellAttrs(last), last.editable);
|
||||
assert.deepEqual(action, { row: 1, col: 1, editable: true, editText: "4000" });
|
||||
assert.deepEqual(action, { row: 1, col: 1, editable: true });
|
||||
});
|
||||
|
||||
// improvise-b8ot: the DATA row index is what a click sends, and `gridPlan`
|
||||
|
||||
@@ -8,7 +8,6 @@ import { gridPlan } from "../shell.js";
|
||||
|
||||
const cell = (text, extra = {}) => ({
|
||||
text,
|
||||
edit_text: text,
|
||||
numeric: true,
|
||||
error: false,
|
||||
selected: false,
|
||||
@@ -102,14 +101,6 @@ test("an empty edit buffer still means an editor is open", () => {
|
||||
assert.equal(plan.body[1].cells[0].text, "▏");
|
||||
});
|
||||
|
||||
test("the editor is seeded from the stored value, not the drawn one", () => {
|
||||
const model = grouped();
|
||||
model.rows[1].cells[0] = cell("20,000", { edit_text: "20000" });
|
||||
const plan = gridPlan(model);
|
||||
assert.equal(plan.body[1].cells[0].text, "20,000");
|
||||
assert.equal(plan.body[1].cells[0].editText, "20000");
|
||||
});
|
||||
|
||||
test("an empty totals strip is no strip at all, not a row of blanks", () => {
|
||||
// Gotcha 33: an EMPTY strip means this grid has no totals — records mode,
|
||||
// or an empty grid — so there is nothing to draw and nothing to pad.
|
||||
|
||||
Reference in New Issue
Block a user