fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #214 from fiddlerwoaroof/wave/km71-edit-formula

Browse Source 
improvise-km71: edit a formula from the formula panel, in place
This commit is contained in:
Ed Langley
2026-09-23 09:00:07 -07:00
committed by GitHub
parent a634b3f356 463bef149d
commit 4502ed2b11
31 changed files with 1114 additions and 134 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Cargo.lock
Generated
+1
View File
@@ -897,6 +897,7 @@ dependencies = [
"pest_derive",
"pest_meta",
"proptest",
"serde",
]
[[package]]
context/code/clients.md
+36 -14
View File
@@ -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
context/code/command.md
+50
View File
@@ -3538,6 +3538,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
context/code/core.md
+54
View File
@@ -1292,3 +1292,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.
context/code/formula-and-io.md
+5 -1
View File
@@ -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.
context/code/protocol.md
+20 -3
View File
@@ -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.
context/test-evidence/clients.md
+43 -30
View File
@@ -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
context/test-evidence/command.md
+50
View File
@@ -5655,6 +5655,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
@@ -5930,6 +5970,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`
crates/improvise-command/src/command/cmd/commit.rs
+41 -8
View File
@@ -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
}
}
crates/improvise-command/src/command/cmd/panel.rs
+20 -1
View File
@@ -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.
crates/improvise-command/src/command/cmd/registry.rs
+1
View File
@@ -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));
crates/improvise-command/src/ui/app.rs
+258
View File
@@ -4864,3 +4864,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"));
}
}
crates/improvise-command/src/ui/effect.rs
+57
View File
@@ -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)]
crates/improvise-command/src/ui/project.rs
+25 -14
View File
@@ -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,
};
@@ -375,14 +375,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()
}
@@ -439,10 +440,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),
}
}
@@ -1826,8 +1827,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"
);
@@ -1841,7 +1847,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"
);
}
@@ -1866,8 +1872,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"
);
@@ -1890,7 +1901,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"
);
}
crates/improvise-command/src/ui/reduce.rs
+13
View File
@@ -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",
crates/improvise-core/src/model/types.rs
+115 -1
View File
@@ -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;
@@ -496,6 +496,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> {
@@ -2137,6 +2169,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();
crates/improvise-formula/Cargo.toml
+2
View File
@@ -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"
crates/improvise-formula/src/ast.rs
+43
View File
@@ -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
}
}
crates/improvise-formula/src/lib.rs
+1 -1
View File
@@ -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};
crates/improvise-protocol/src/help.rs
+7
View File
@@ -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",
crates/improvise-protocol/src/keymap.rs
+2
View File
@@ -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(),
),
crates/improvise-protocol/src/lib.rs
+31 -5
View File
@@ -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,
}
}
crates/improvise-protocol/src/reduce.rs
+14 -2
View File
@@ -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)
crates/improvise-protocol/src/render.rs
+38 -12
View File
@@ -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
crates/improvise-protocol/src/resolve.rs
+69 -29
View File
@@ -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()
crates/improvise-wasm-client/src/lib.rs
+2 -2
View File
@@ -134,8 +134,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))
crates/improvise-wasm-client/src/render.rs
+2 -2
View File
@@ -377,8 +377,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()
crates/improvise-wasm-client/tests/lockstep.rs
+93
View 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");
}
harness/opfs-contract/Cargo.lock
Generated
+1
View File
@@ -196,6 +196,7 @@ dependencies = [
"anyhow",
"pest",
"pest_derive",
"serde",
]
[[package]]
harness/tests/contract.spec.js
+6 -5
View File
@@ -251,15 +251,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);
});
src/ui/formula_panel.rs
+14 -4
View File
@@ -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.