Merge commit 'cea0ff8d' into ms-zset

This commit is contained in:
Claude (main-sync)
2026-10-10 11:16:36 -07:00
committed by Edward Langley
16 changed files with 1057 additions and 152 deletions
@@ -178,10 +178,11 @@ different thing that generates the same slug: the merged file declares that id t
and the load is refused naming both. Stable identity across copies of a file is not
promised (improvise-8tk5).
**Built code.** `crates/improvise-core/src/model/id.rs` implements the accepted steps
(`context/code/core.md` §20). A follow-up must replace rather than strip, collapse
`_`, trim a trailing `_` and both ends' `-`, prepend `n`, make `claim` refuse what a
generated id could not be, and add the virtual views and `IdKind::Group`.
**Built code.** `crates/improvise-core/src/model/id.rs` implements this clause
(`context/code/core.md` §20): `slugify` follows the six steps, `is_id_syntax` is the
syntax, `claim` refuses what a generated id could not be, `VIRTUAL_IDS` holds the five
virtual ids, `IdKind::Group` exists, and `natural_cmp` and `display_suffixes` implement
natural order and clause 2's suffixes. No other code calls them yet.
### 5. Persistence
@@ -310,7 +311,7 @@ therefore shows B as `east-1`, never as a bare `east`.
- Scripts that name things by display break and must be rewritten over ids.
- Two measures can share a display, and are shown as `Profit` and `Profit #2`.
- Old files' `_Measure` declarations and hyphen- or digit-led names change on first save.
- `id.rs` and its tests need the follow-up in clause 4.
- `id.rs` implements clause 4; the code that sorts, shows and creates ids does not call it yet.
## Open questions
+82 -48
View File
@@ -1688,48 +1688,63 @@ probability (`> ||[||]` is a self-parent), so `generated_file_parses` accepts a
## 20. How an id is made — `model/id.rs` (ADR 007 clause 4, improvise-cq1q.2)
`slugify` and `IdAllocator` are the first piece of ADR 007. They are pure
functions of the display and the allocator's own history. Nothing calls them
yet: the creation sites move onto them in the re-key step (improvise-cq1q.4).
`slugify`, `IdAllocator`, `natural_cmp` and `display_suffixes` are the first
piece of ADR 007. They are pure functions of their arguments and the
allocator's own history. Nothing calls them yet: the creation sites move onto them in the re-key step (improvise-cq1q.4).
Each rule traces to ADR 007's clause 4 (slugify, the counter, hand-written
ids) or clause 1 (reserved ids).
ids, natural order), clause 1 (reserved ids) or clause 2 (display suffixes),
as amended by amendment 1.
**Case folding is full Unicode folding, and it comes from `unicase`.**
`str::to_lowercase` is not folding: `ß` lowercases to itself and is then
stripped, so `Straße` would slug to `strae` (clause 4, step 1: full case folding). The same goes for `ſ`
(long s) and the `fi` ligature. `unicase` 2.9 is `no_std` and has no
`str::to_lowercase` is not folding: `ß` lowercases to itself and then
becomes `_`, so `Straße` would slug to `stra_e` (clause 4, step 1: full case
folding). The same goes for `ſ` (long s) and the `fi` ligature. `unicase` 2.9 is `no_std` and has no
dependencies, so it passes the wasm gate. Its `to_folded_case` applies the
C+F mappings of Unicode's CaseFolding.txt, and its own tests pin `Maße` →
`masse`. Most of what folding produces is stripped anyway: only an ASCII
letter survives step 3. Deriving that ASCII-producing subset of the table by
hand was rejected. It is a second copy of the Unicode data, and nothing would
`masse`. Most of what folding produces becomes `_` anyway: only ASCII
letters, digits and `-` survive step 2. Deriving that ASCII-producing subset of
the table by hand was rejected. It is a second copy of the Unicode data, and nothing would
say when it drifted.
**A "space" is U+0020.** The accepted clause 4 says "replace each run of spaces
with one `_`". A tab or a no-break space is not a space here, so it is
stripped by step 3. Only a run of U+0020 becomes one `_`. An underscore the
display already had is never merged with the one a space run makes. So `a_ b`
slugs to `a__b`: the steps are applied as written, and no extra
underscore-collapsing step exists. A trailing run survives, so `East ` slugs
to `east_`. Only *leading* underscores are removed (step 4).
**Slugify is the amendment's six steps** (ADR 007 amendment 1, clause 4):
casefold; every character outside `a`–`z`, `0`–`9` and `-` becomes `_`; runs of
`_` collapse; leading and trailing `_` and `-` are removed, all of them (the
trim is `trim_matches` over both characters, so `_-_East-_-` is `east`);
`n` is prepended to a slug that starts with a digit; nothing left is the
caller's `<kind>-<n>`. The order matters in one place: the trim runs before
the `n`, so `-_7up` is `n7up` and a leading `-` can never be hidden behind the
prefix. Whitespace of every kind, punctuation and every non-ASCII letter
become `_`, so `Region(code)` is `region_code` and `Crème Brûlée` is
`cr_me_br_l_e`. Folding is `unicase`'s, which maps `ß` to `ss`; a character
that folds to a combining mark (`İ` folds to `i` plus U+0307) leaves the `i`
and turns the mark into `_`, which the trim then removes. A hyphen between
underscores survives (`a - b` is `a_-_b`).
**ADR 007's amendment changes these rules, and this code predates it.** The
amendment (`context/adr/007-stable-identities-amendment-1.md`, accepted 2026-10-09)
says in its clause 4:
- every character outside `a`–`z`, `0`–`9` and `-` becomes `_` instead of being
stripped (not only U+0020), so `Region(code)` slugs to `region_code`, while
`Q1-2024` keeps its `-`;
- runs of `_` collapse, so `a_ b` and `a__b` both slug to `a_b`;
- leading and trailing `_` and `-` are removed, so `East ` → `east`, `-East-` →
`east`, and `-` alone takes `<kind>-<n>`;
- `n` is prepended to a slug that starts with a digit, so `2024` → `n2024`;
- a hand-written id must obey every syntax rule of a generated one.
**The id syntax has one validator, `is_id_syntax`**: a lower-case ASCII
letter, then `a`–`z`, `0`–`9`, `-` and single underscores, never ending in
`_` or `-`. Slugify's non-empty output always satisfies it and is a fixed point
of itself, and a `-<n>` suffix keeps the shape, so a generated id always
validates (properties in `model::id::tests`). A virtual id fails it by design,
because the leading `_` is the reservation.
This section, `slugify`, and the tests
`slugify_collapses_a_run_of_spaces_to_one_underscore` (its `east_` case) and
`slugify_keeps_an_underscore_the_display_already_had` describe the earlier rule
above. They change in a follow-up the amendment calls for. The amendment's
clause 4 lists every difference.
**Natural order is `natural_cmp`** (clause 4, "Ids are always sorted in
natural order"). An id is split into alternating digit and non-digit runs, and
the runs are compared pairwise: non-digit runs by bytes; digit runs by value
(strip leading zeros, shorter is smaller, then bytes), and a tie on value goes
to the run with fewer leading zeros, immediately and not after the later runs.
The id that runs out of runs first sorts first. No digit run is parsed into an
integer, so a run of any length compares correctly. Where two ids disagree
about the kind of run at a position (only possible for text that is not an
id), the runs compare as bytes. It is a total order and equal only for equal
strings. **Nothing calls it yet**: `CellKey`, `DataStore`, the writer and the GP
dataset still sort with `String::cmp` until the re-key step.
**Equal displays are suffixed by `display_suffixes`** (clause 2). Given
`(id, display)` pairs it returns what each is shown as, in input order: the
first of a group of equal displays in natural id order keeps the bare display,
the rest get ` #2`, ` #3`, …, and a number is skipped when a literal display
already reads `<display> #<n>` anywhere in the scope. Displays are equal
byte for byte, not case-folded. It is pure and nothing renders through it yet.
**The allocator records every id it admits and releases none.** That is the
whole of retirement (clause 4, "within a session a removed id is never handed
@@ -1750,17 +1765,14 @@ name happens to be free, because clause 4 spells the result
`<kind>-<n>`.
**`claim` is the only door for a hand-written id.** It checks, in this order:
the alphabet (non-empty, `a`–`z`, `0`–`9`, `-`, `_`), the reserved leading
`_` (only `_index`, `_dim` and `_measure` may carry one, per clause 1, and
the match is exact: `_indexer` and `_measure-1` are Reserved, since "no other
id may start with `_`"), and
duplicates. It accepts `-`, `-x`, `x-`, `a__b` and `2024`, all of which the
amendment refuses (a hand-written id obeys the syntax of a generated one). It never renames.
Refusing is the loader's job, and the loader
the reserved leading `_` (only `_index`, `_dim`, `_measure`, `_records` and
`_drill` may carry one, and the match is exact: `_indexer` and `_measure-1`
are Reserved), the syntax (`is_id_syntax`; `-x`, `x-`, `2024`, `a__b` and `x_`
are Malformed, `east-1`, `item-4`, `a-7` and `q1_-_2024` pass), and
duplicates. A virtual id skips the syntax check, since it fails it by design.
It never renames. Refusing is the loader's job, and the loader
must name both lines of a duplicate; `ClaimError` carries only the id, so the
loader keeps the line numbers. The virtual views `_records` and `_drill` are
not yet in `VIRTUAL_IDS`. improvise-p003 item 1 asks for them, and they arrive
with the ADR text fix (improvise-cq1q.1) rather than ahead of it.
loader keeps the line numbers. `IdKind::Group` spells `group-<n>`.
## 21. A display beside the key (ADR 007 clause 2, improvise-cq1q.3)
@@ -1812,11 +1824,33 @@ beside the key and never in its place:
- The browser's render model keeps its field names and carries the display in them
(`RenderPageAxis.cat`, `RenderCatRow.label`), so `web/` is unchanged.
Still keys: `GridLayout::row_label`/`col_label` (commands and the status line
match on them), `edit_text`, the active-view marker, and everything status
messages and the view title say (improvise-cq1q.13). A display change alone does
not move `layout_fingerprint`, so it resets no cursor; no production path changes
one yet.
Still keys: `GridLayout::row_label`/`col_label` (commands match on them),
`edit_text` and the active-view marker. A display change alone does not move
`layout_fingerprint`, so it resets no cursor; no production path changes one yet.
**What the third slice covers (improvise-cq1q.13):** the view title and every
message that names a thing which exists. The title is the active view's display:
the TUI reads `app.view().display()`, the browser `ViewListCache::display_of`
(key when the list lacks the view). The statuses are the axis change (panel and
tile bar), `Filter:`, `Deleted category/item`, the non-item
refusal, and the item-add prompt. Both ends say them through the same
`viewmodel` builders, so a wording is not spelled twice. The host reads
`Model::category_display`/`item_display`; the client reads
`CategoryTreeCache::category_display`/`item_display`. Each falls back to the key.
**A message that names something which does not exist says what was typed.**
`Unknown category 'x'`, `No view named "x"`, `View 'x' not found` and the
`workbook.rs` errors are lookup misses: there is no display to find, and the typed
text is what the person needs to correct. `Added category "x"` is optimistic and
names what was just typed, whose display equals its key at creation. A stray
coordinate in `non_item_refusal` is a key for the same reason; only the category
beside it is a display. **A display must never reach a lookup**: the item-add
mode keeps the category key (`AppMode::item_add_shown(key, shown)`) and the prompt
alone carries the display. A formula-result refusal still says the target's key
(improvise-cq1q.13 deferred it). A target in `_Measure` is implied by its formula
and is no item, so it has no display to read. One in a regular category is a real
item, but `CellId::Derived` carries only the target, so neither end can find its
category. Both need that surrogate to carry the category first.
`AxisFrame::cat_displays` is projected and validated, but nothing paints it yet
(the corner label has no consumer); that is a gap to close when one does, not a
+169 -13
View File
@@ -1914,30 +1914,108 @@ identity on the view.
header stays for the sibling quarter that has data. Rival: `prune_axis_items`
keying open runs by level alone. This test and the `NESTED` enumeration fail.
## `model/id.rs` (ADR 007 clause 4, improvise-cq1q.2)
## `model/id.rs` (ADR 007 clause 4 and amendment 1, improvise-cq1q.2, improvise-cq1q.14)
These tests were written before `slugify` and `IdAllocator` had bodies. With
`todo!()` bodies all eleven failed. Each rival below was then injected into the
finished module, run against `cargo test -p improvise-core --lib model::id`, and
Each rival below was injected into the finished module, run against `cargo test -p improvise-core --lib model::id`, and
reverted. The reasoning is `context/code/core.md` §20.
### model::id::tests::slugify_case_folds_rather_than_lowercases
Rival: `display.to_lowercase()` in place of `unicase`'s full folding. Only
this test fails: `left: "strae"`, `right: "strasse"`. `ß` lowercases to itself
and step 3 strips it. `slugify_follows_the_adrs_examples` cannot see this rival,
this test fails: `ß` lowercases to itself and becomes `_`, so `stra_e` against
`strasse`. `slugify_follows_the_amendments_table` cannot see this rival,
because each of its displays lowercases and folds alike.
### model::id::tests::slugify_collapses_a_run_of_spaces_to_one_underscore
### model::id::tests::slugify_follows_the_amendments_table
Rival: every space becomes `_`, with runs not collapsed. Only this test fails:
`"time___period"`. No other fixture has two spaces in a row.
Every row of the amendment's clause 4 table, plus `-`, `%` and the empty
display. Rivals, each of which fails this test: trimming only leading `_`
(`East_` stays), trimming `_` and not `-` (`-East-`), not collapsing runs of
`_` (`a__b`), not prepending `n` (`2024`), and stripping instead of replacing
(`Region(code)` is `regioncode`). The last also fails
`slugify_keeps_a_hyphen_between_underscores` and
`a_slug_taken_by_another_kind_takes_a_suffix`.
### model::id::tests::slugify_keeps_an_underscore_the_display_already_had
### model::id::tests::slugify_trims_hyphens_and_underscores_until_neither_is_at_an_end
Rival: `.replace("__", "_")` after the space step, which reads the accepted
clause 4 as "no doubled underscores". Only this test fails: `left: "a_b"`. The rule
collapses runs of *spaces*. An underscore that was in the display is kept.
Rival: trim leading `_` only, or `_` only at both ends. `_-_East-_-` keeps a
hyphen, and `-_-` is not empty. A single `trim_matches` over both characters
passes; two sequential single-character trims would not.
### model::id::tests::slugify_trims_before_it_prepends_the_n
Rival: no `n` prefix, which fails this test, the table and the property below.
`-_7up` is `n7up` only if the trim runs first; prepending first would leave the
`-_` in front of the `n`.
### model::id::tests::slugify_output_has_the_id_syntax_and_is_a_fixed_point
The property over arbitrary displays: a non-empty slug satisfies
`is_id_syntax` and slugifying it returns it. Rivals that fail it (and
`generated_ids_are_distinct_well_formed_and_never_reserved`): no `_`
collapse, no `n` prefix, keeping a trailing `_` or `-`.
### model::id::tests::id_syntax_agrees_with_an_independent_checker
`is_id_syntax` against a hand-written scanner of `^[a-z](_?[a-z0-9-])*$` that
does not end in `-`, over `[a-cA-C0-9 _%é-]{0,8}`, so uppercase, space, symbols
and non-ASCII are reached. Dropping the alphabet check also fails it. Rivals:
allowing `__`, allowing a leading digit and allowing a trailing `-` each fail
here and in `claim_refuses_what_a_generated_id_could_not_be`.
### model::id::tests::natural_order_decides_the_zeros_tie_at_its_own_run_not_after_the_rest
Rival: compare leading zeros only after every run compares equal in value.
`a-7c` then sorts after `a-007b`, and this test fails alone.
### model::id::tests::suffixed_displays_are_distinct_when_literals_already_hold_suffixes
Displays are drawn from `A`, `A #2`, `A #3` and `B`, so literal suffixes meet
generated ones. Rival: numbering without the skip fails it.
### model::id::tests::claim_refuses_what_a_generated_id_could_not_be
Rival: `claim` keeps the old alphabet-only check. `-x`, `x-`, `2024`, `a__b`,
`x_` and `-` are admitted, and the test fails. It also pins the accepted ids
`east-1`, `item-4`, `a-7` and `q1_-_2024`.
### model::id::tests::claim_admits_each_virtual_id_once
Rival: `VIRTUAL_IDS` without `_records` and `_drill`. The test spells the five
ids out instead of iterating the constant, because an iteration over the
constant passes with the constant shortened (the first version did).
### model::id::tests::natural_order_puts_a_counter_suffix_in_numeric_order
Rivals: comparing digit runs as strings (`profit-10` before `profit-2`) fails
this test, `natural_order_compares_digit_runs_of_any_length_by_value` and
`a_suffix_numbers_equal_displays_in_natural_id_order`; plain `str::cmp` fails
those and `natural_order_ties_on_value_go_to_fewer_leading_zeros`.
### model::id::tests::natural_order_ties_on_value_go_to_fewer_leading_zeros
Rivals: more zeros first, or no tie-break (`a-7` equal to `a-007`). Both fail this test and the any-length test (`x000` after `x0`).
### model::id::tests::natural_order_puts_the_id_that_runs_out_of_runs_first
Rival: the longer id first. Only this test fails.
### model::id::tests::natural_order_is_a_total_order
Antisymmetry, equality only for equal strings and transitivity over
`[a-c0-3_-]{0,7}`, which includes ids starting with a digit.
### model::id::tests::a_suffix_numbers_equal_displays_in_natural_id_order
Rival: ordering the group by `str::cmp` on the ids. `profit-10` is numbered
before `profit-2` and this test fails alone.
### model::id::tests::a_suffix_skips_a_number_a_literal_display_already_holds
Rival: numbering without the skip. A literal `Profit #2` and a generated
`Profit #2` are then both shown, and this test fails alone. A rival that starts
numbering at ` #1` also fails it and
`a_suffix_is_gapless_when_a_thing_goes_away`.
### model::id::tests::the_suffix_is_the_next_counter_value_not_the_lowest_free_number
@@ -2144,3 +2222,81 @@ The display "Eastern Seaboard" against the key "E" gives 17, not the 4 minimum.
Rival: `col_groups` labels a run with `g.name`. The browser test for group displays
covered row groups only; this one folds a column frame whose group "Q1" is shown as
"First quarter" and fails on the key.
### draw::content_tests::the_grid_title_names_the_views_display_not_its_key
improvise-cq1q.13, TUI half. The view is displayed "Quarterly overview of regional
sales" under the key "Default", and the whole frame is drawn through `draw`. Rival:
`draw_content` passes `app.view_state.active_view` to the widget; the title reads
`View: Default` and this test fails on both assertions.
### render::tests::the_title_names_the_active_views_display_not_its_key
The browser half. Rival: `render_model` takes `view_name` from
`state.view.active_view`; the test fails. The second half is the control: a view
the list does not hold falls back to its key rather than to an empty title.
### resolve::display_tests::a_panel_status_names_what_a_person_sees_not_the_keys
The browser resolver's panel statuses: axis cycle, filter, delete category, delete
item. Displays are 36 to 38 characters against keys of 4 to 6. Rivals, each run:
the cycle status passes the key; the filter composes `SetPageSelection` with the
item's display; the item removal names the display. Each fails here, the last two
on the `Region/North` and `RemoveItem` assertions that pin the lookup to keys.
### resolve::display_tests::a_tile_status_names_the_display
Rival: the tile route passes `tile.category`; fails on the key.
### resolve::display_tests::the_item_prompt_names_the_display_and_the_mode_keeps_the_key
Rival: the mode is built with the display as its category
(`item_add_shown(shown, ..)`); the prompt reads right and the commit would add to
a category that does not exist. The test fails on `category == "Region"`.
### resolve::display_tests::a_refused_cell_names_the_category_it_shows
Rival: the non-item refusal gets the category key. The stray coordinate stays a key
because it names no item and has no display.
### command::cmd::panel::tests::a_panel_status_names_what_a_person_sees_not_the_keys
The host half of the panel statuses and the item-add prompt. Rivals, each run: the
axis status passes `cat_name` (fails); the filter status passes the item key
(fails); the item-add mode is built from the display (fails on the
`category: "Type"` assertion).
Review follow-up: the same test now also pins the operands. Rivals: the host
`RemoveCategory` and `SetPageSelection` given the display fail on the
`RemoveCategory("Type")` and `item: "Food"` assertions. The client twin fails the
same way on `RemoveCategory`.
### command::cmd::tile::tests::a_tile_status_names_the_display_not_the_key
Rival: the status formats `name`. It fails on both the present display and the
absent key.
### ui::app::non_item_key_tests::a_non_item_refusal_names_the_categorys_display
Rival, run once per site: the refusal in `EditOrDrill` (the `i` key) and the one in
`CmdContext::value_cell_key` (the `p` key) each pass the key. Each fails here.
### ui::app::non_item_key_tests::a_commit_on_a_non_item_cell_names_the_categorys_display
The third site, `uneditable` in `cmd/core.rs`, which only a commit reaches once an
editor is open (`:enter-mode editing`). Rival: it passes the key; the two tests
above stay green and this one fails.
### ui::app::non_item_key_tests::a_refused_wire_write_names_the_category_it_shows
The wire's own refusal in `Grids::unwritable`. Rival: the category key; fails. The
formula arm (once a third copy of the derived wording, now `derived_refusal`) is
pinned to its wording so the consolidation cannot change what it says.
### resolve::display_tests::the_declines_an_advance_and_a_commit_reach_name_the_category_it_shows
From the Opus review of #339: the browser's `declined_at` non-item arm could pass the
key with the whole suite green. It is reached by `reopened_at` (an advance),
`refused_here` (a commit) and `settle_declined_editor`. Rival: the arm passes
`category`; the test fails on all three assertions, and the display is 36 characters
against the key "Region".
@@ -39,7 +39,9 @@ pub fn uneditable(model: &Model, layout: &GridLayout, row: usize, col: usize) ->
.to_string(),
),
Some(CellId::Derived { target }) => Some(derived_refusal(&target)),
Some(CellId::NonItem { category, item }) => Some(non_item_refusal(&category, &item)),
Some(CellId::NonItem { category, item }) => {
Some(non_item_refusal(model.category_display(&category), &item))
}
Some(CellId::Record(_) | CellId::Create(_) | CellId::Coordinate { .. }) | None => None,
}
}
@@ -133,7 +135,10 @@ impl<'a> CmdContext<'a> {
Some(CellId::Create(_) | CellId::Coordinate { .. }) => {
Err(NO_VALUE_REFUSAL.to_string())
}
Some(CellId::NonItem { category, item }) => Err(non_item_refusal(&category, &item)),
Some(CellId::NonItem { category, item }) => Err(non_item_refusal(
self.model.category_display(&category),
&item,
)),
None => Err(match self.cell_key() {
Some(_) => NO_VALUE_REFUSAL,
None => NO_CELL,
@@ -393,7 +393,8 @@ impl Cmd for EditOrDrill {
// refused later, under a minibuffer that hides why (improvise-yc9r).
if let Some(improvise_protocol::CellId::NonItem { category, item }) = ctx.cursor_cell_id() {
return vec![effect::refuse(improvise_protocol::non_item_refusal(
&category, &item,
ctx.model.category_display(&category),
&item,
))];
}
vec![
@@ -1,4 +1,6 @@
use improvise_protocol::viewmodel::axis_change_status;
use improvise_protocol::viewmodel::{
axis_change_status, deleted_category_status, deleted_item_status, filter_status,
};
use crate::ui::app::AppMode;
use crate::ui::effect::{self, Effect, Panel, ViewTarget};
@@ -186,6 +188,57 @@ mod tests {
);
}
/// improvise-cq1q.13: evidence in `context/test-evidence/core.md`.
#[test]
fn a_panel_status_names_what_a_person_sees_not_the_keys() {
const TYPE: &str = "Kind of food and drink";
const FOOD: &str = "Foodstuffs of every sort";
let mut wb = two_cat_model();
let ty = wb.model.category_mut("Type").unwrap();
ty.set_display(TYPE);
ty.set_item_display("Food", FOOD);
let layout = make_layout(&wb);
let reg = make_registry();
let mut ctx = make_ctx(&wb, &layout, &reg);
let expanded = std::collections::HashSet::from(["Type".to_string()]);
ctx.expanded_cats = &expanded;
let said = |effects: &[Box<dyn Effect>]| format!("{:?}", effects.last().unwrap());
ctx.cat_panel_cursor = 3;
assert_eq!(ctx.cat_at_cursor().as_deref(), Some("Type"), "setup");
assert!(said(&CycleAxisAtCursor.execute(&ctx)).contains(&format!("{TYPE} → ")));
assert!(said(&DeleteCategoryAtCursor.execute(&ctx)).contains(&format!("'{TYPE}'")));
let removal = format!("{:?}", DeleteCategoryAtCursor.execute(&ctx));
assert!(removal.contains("RemoveCategory(\"Type\")"), "{removal}");
let prompt = format!("{:?}", OpenItemAddAtCursor.execute(&ctx));
assert!(
prompt.contains(&format!("add item to {TYPE}: ")),
"{prompt}"
);
assert!(
prompt.contains("category: \"Type\""),
"the key stays: {prompt}"
);
ctx.cat_panel_cursor = 4;
let filtering = format!("{:?}", FilterToItem.execute(&ctx));
assert!(filtering.contains("item: \"Food\""), "keys: {filtering}");
let filter = said(&FilterToItem.execute(&ctx));
assert!(
filter.contains(&format!("Filter: {TYPE} = {FOOD}")),
"{filter}"
);
let delete = format!("{:?}", DeleteCategoryAtCursor.execute(&ctx));
assert!(
delete.contains(&format!("Deleted item '{FOOD}' from '{TYPE}'")),
"{delete}"
);
assert!(
delete.contains("item: \"Food\""),
"the effect still names the key: {delete}"
);
}
#[test]
fn cycle_panel_focus_with_no_panels_open() {
let m = two_cat_model();
@@ -718,8 +771,12 @@ impl Cmd for CycleAxisAtCursor {
ViewTarget::Active,
cat_name.clone(),
))];
effects
.extend(cycled.map(|axis| effect::set_status(axis_change_status(&cat_name, axis))));
effects.extend(cycled.map(|axis| {
effect::set_status(axis_change_status(
ctx.model.category_display(&cat_name),
axis,
))
}));
effects
} else {
vec![effect::refuse(
@@ -738,7 +795,10 @@ impl Cmd for OpenItemAddAtCursor {
}
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
if let Some(cat_name) = ctx.cat_at_cursor() {
vec![effect::change_mode(AppMode::item_add(cat_name))]
let shown = ctx.model.category_display(&cat_name).to_string();
vec![effect::change_mode(AppMode::item_add_shown(
cat_name, &shown,
))]
} else {
vec![effect::refuse(
"No category selected. Press n to add a category first.",
@@ -790,7 +850,10 @@ impl Cmd for FilterToItem {
category: cat_name.clone(),
item: item_name.clone(),
}),
effect::set_status(format!("Filter: {cat_name} = {item_name}")),
effect::set_status(filter_status(
ctx.model.category_display(&cat_name),
ctx.model.item_display(&cat_name, &item_name),
)),
]
}
Some(CatTreeEntry::Category { .. }) => {
@@ -816,7 +879,7 @@ impl Cmd for DeleteCategoryAtCursor {
Some(CatTreeEntry::Category { name, .. }) => {
vec![
Box::new(effect::RemoveCategory(name.clone())),
effect::set_status(format!("Deleted category '{name}'")),
effect::set_status(deleted_category_status(ctx.model.category_display(&name))),
]
}
Some(CatTreeEntry::Item {
@@ -829,7 +892,10 @@ impl Cmd for DeleteCategoryAtCursor {
category: cat_name.clone(),
item: item_name.clone(),
}),
effect::set_status(format!("Deleted item '{item_name}' from '{cat_name}'")),
effect::set_status(deleted_item_status(
ctx.model.item_display(&cat_name, &item_name),
ctx.model.category_display(&cat_name),
)),
];
effects.extend(retreat_from_deleted_item(ctx));
effects
@@ -8,6 +8,28 @@ mod tests {
use super::*;
use crate::command::cmd::test_helpers::*;
#[test]
fn a_tile_status_names_the_display_not_the_key() {
let mut m = two_cat_model();
m.model
.category_mut("Type")
.unwrap()
.set_display("Kind of food and drink");
let layout = make_layout(&m);
let reg = make_registry();
let mut ctx = make_ctx(&m, &layout, &reg);
ctx.tile_cat_idx = ctx
.view
.tileable_cats(&ctx.model.category_names())
.iter()
.position(|c| *c == "Type")
.unwrap();
let effects = TileAxisOp { axis: None }.execute(&ctx);
let said = format!("{:?}", effects.last().unwrap());
assert!(said.contains("Kind of food and drink → "), "{said}");
assert!(!said.contains("Type →"), "the key was said: {said}");
}
#[test]
fn tile_axis_cycle_produces_cycle_effect() {
let m = two_cat_model();
@@ -161,7 +183,10 @@ impl Cmd for TileAxisOp {
}),
None => Box::new(effect::CycleAxis(ViewTarget::Active, name.to_string())),
};
let status = format!("{} → {}", name, new_axis.plain_label());
let status = improvise_protocol::viewmodel::axis_change_status(
ctx.model.category_display(name),
new_axis,
);
vec![axis_effect, effect::set_status(status)]
} else {
vec![]
+86 -9
View File
@@ -332,12 +332,9 @@ impl Grids<'_> {
pub fn unwritable(&self, view: &str, cell: &RecordCell) -> Option<String> {
let key = self.resolve_cell(view, cell)?;
let model = &self.app.model_state.workbook.model;
let derived = model.formula_for(&key).map(|f| {
format!(
"{} is computed by a formula — edit the formula, not the cell",
f.target
)
});
let derived = model
.formula_for(&key)
.map(|f| crate::command::cmd::core::derived_refusal(&f.target));
let refusal = self.app.non_item_refusal(&key).or(derived);
refusal.or_else(|| self.aggregate_refusal(view, cell.record, &key))
}
@@ -772,9 +769,9 @@ impl App {
/// (improvise-b114, improvise-nacd). See `context/code/command.md`.
fn non_item_refusal(&self, key: &CellKey) -> Option<String> {
let model = &self.model_state.workbook.model;
model
.first_non_item(key)
.map(|(cat, name)| improvise_protocol::non_item_refusal(cat, name))
model.first_non_item(key).map(|(cat, name)| {
improvise_protocol::non_item_refusal(model.category_display(cat), name)
})
}
/// Point **this client's** slice at `name`, if the document has such a view,
@@ -5620,6 +5617,71 @@ mod non_item_key_tests {
assert_eq!(app.view_state.status_msg, refusal("Mars"));
}
#[test]
fn a_refused_wire_write_names_the_category_it_shows() {
const REGION: &str = "Sales territory of the whole company";
let mut app = paged_app("East");
let stray = CellKey::new(vec![
("Month".into(), "Jan".into()),
("Region".into(), "Mars".into()),
("_Measure".into(), "Revenue".into()),
]);
let model = &mut app.model_state.workbook.model;
model.set_cell(stray.clone(), CellValue::Number(2.0));
model.category_mut("Region").unwrap().set_display(REGION);
model.add_formula(
crate::formula::parse_formula("Profit = Revenue - 1", "_Measure").unwrap(),
);
let refused = |key: &CellKey, measure: &str| {
let cell = RecordCell {
record: RecordHash::of(key),
measure: Some(measure.into()),
};
app.unwritable(app.view_name(), &cell)
};
let why = refused(&stray, "Revenue").expect("a non-item is refused");
assert!(why.contains(REGION), "{why}");
assert!(
why.starts_with("Mars "),
"the stray coordinate is a key: {why}"
);
let profit = CellKey::new(vec![
("Month".into(), "Jan".into()),
("Region".into(), "East".into()),
("_Measure".into(), "Profit".into()),
]);
let why = refused(&profit, "Profit").expect("a formula's result is refused");
assert!(why.starts_with("Profit is computed by a formula"), "{why}");
}
#[test]
fn a_non_item_refusal_names_the_categorys_display() {
let mut app = paged_app("Mars");
app.model_state
.workbook
.model
.category_mut("Region")
.unwrap()
.set_display("Sales territory of the whole company");
press(&mut app, "i");
assert_eq!(
app.view_state.status_msg,
improvise_protocol::non_item_refusal("Sales territory of the whole company", "Mars"),
);
app.view_state.yanked = Some(CellValue::Number(7.0));
press(&mut app, "p");
assert!(
app.view_state
.status_msg
.ends_with(&improvise_protocol::non_item_refusal(
"Sales territory of the whole company",
"Mars"
)),
"{}",
app.view_state.status_msg
);
}
/// improvise-yc9r: `p` on a non-item cell writes no orphan. Evidence file.
#[test]
fn p_on_a_non_item_cell_writes_nothing() {
@@ -5655,6 +5717,21 @@ mod non_item_key_tests {
assert!(status.ends_with(&refusal("Mars")), "{status}");
}
#[test]
fn a_commit_on_a_non_item_cell_names_the_categorys_display() {
const REGION: &str = "Sales territory of the whole company";
let mut app = paged_app("Mars");
let region = app.model_state.workbook.model.category_mut("Region");
region.unwrap().set_display(REGION);
press(&mut app, ":enter-mode editing\n");
press(&mut app, "5\n");
let said = &app.view_state.status_msg;
assert!(
said.ends_with(&improvise_protocol::non_item_refusal(REGION, "Mars")),
"{said}"
);
}
/// `uneditable` declines a non-item cell, so an advance re-opens no editor
/// on one (#265 meets #277). Evidence: `context/test-evidence/command.md`.
#[test]
+319 -43
View File
@@ -1,6 +1,7 @@
//! How an id is made: `slugify`, and the document's `IdAllocator` (ADR 007 clause 4).
//! Reasoning: `context/code/core.md` §20.
use std::collections::HashSet;
//! How an id is made: `slugify`, and the document's `IdAllocator` (ADR 007 clause 4
//! and its amendment). Reasoning: `context/code/core.md` §20.
use std::cmp::Ordering;
use std::collections::{HashMap, HashSet};
/// What a generated id names; spells the `<kind>` of an empty slug's `<kind>-<n>`.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -9,6 +10,7 @@ pub enum IdKind {
Item,
Formula,
View,
Group,
}
impl IdKind {
@@ -18,38 +20,111 @@ impl IdKind {
IdKind::Item => "item",
IdKind::Formula => "formula",
IdKind::View => "view",
IdKind::Group => "group",
}
}
}
/// The only ids that may begin with `_` (answer 23).
pub const VIRTUAL_IDS: [&str; 3] = ["_index", "_dim", "_measure"];
/// The only ids that may begin with `_` (amendment 1, clause 1).
pub const VIRTUAL_IDS: [&str; 5] = ["_index", "_dim", "_measure", "_records", "_drill"];
/// A display name's slug; empty when nothing survives (answers 16, 18, 21, 23, 26).
/// A display name's slug; empty when nothing survives (amendment 1, clause 4).
pub fn slugify(display: &str) -> String {
let folded = unicase::UniCase::new(display).to_folded_case();
let kept: String = underscore_space_runs(&folded)
let mut slug = String::new();
for c in folded
.chars()
.filter(|&c| in_id_alphabet(c))
.collect();
kept.trim_start_matches('_').to_string()
.map(|c| if in_slug_alphabet(c) { c } else { '_' })
{
if !(c == '_' && slug.ends_with('_')) {
slug.push(c);
}
}
let trimmed = slug.trim_matches(|c| c == '_' || c == '-');
if trimmed.starts_with(|c: char| c.is_ascii_digit()) {
format!("n{trimmed}")
} else {
trimmed.to_string()
}
}
fn underscore_space_runs(s: &str) -> String {
let (out, _) = s
.chars()
.fold((String::new(), false), |(mut out, prev), c| {
let space = c == ' ';
if !(space && prev) {
out.push(if space { '_' } else { c });
}
(out, space)
});
out
fn in_slug_alphabet(c: char) -> bool {
matches!(c, 'a'..='z' | '0'..='9' | '-')
}
fn in_id_alphabet(c: char) -> bool {
matches!(c, 'a'..='z' | '0'..='9' | '-' | '_')
/// Whether `id` has the syntax of a generated id: a letter, then `a`-`z`, `0`-`9`,
/// `-` and single underscores, never ending in `_` or `-`. A virtual id fails it.
pub fn is_id_syntax(id: &str) -> bool {
let b = id.as_bytes();
let edges = matches!(b.first(), Some(b'a'..=b'z')) && !matches!(b.last(), Some(b'_' | b'-'));
edges && b.windows(2).all(|w| w != b"__") && id.chars().all(|c| in_slug_alphabet(c) || c == '_')
}
/// Ids in natural order: digit runs by value, then fewer leading zeros first.
pub fn natural_cmp(a: &str, b: &str) -> Ordering {
let (mut ra, mut rb) = (runs(a), runs(b));
loop {
match (ra.next(), rb.next()) {
(None, None) => return Ordering::Equal,
(None, Some(_)) => return Ordering::Less,
(Some(_), None) => return Ordering::Greater,
(Some(x), Some(y)) => match run_cmp(x, y) {
Ordering::Equal => {}
other => return other,
},
}
}
}
fn runs(s: &str) -> impl Iterator<Item = &str> {
let mut rest = s;
std::iter::from_fn(move || {
let digit = rest.chars().next()?.is_ascii_digit();
let end = rest
.find(|c: char| c.is_ascii_digit() != digit)
.unwrap_or(rest.len());
let (run, tail) = rest.split_at(end);
rest = tail;
Some(run)
})
}
fn run_cmp(x: &str, y: &str) -> Ordering {
let digits = |r: &str| r.starts_with(|c: char| c.is_ascii_digit());
if digits(x) && digits(y) {
let (vx, vy) = (x.trim_start_matches('0'), y.trim_start_matches('0'));
vx.len()
.cmp(&vy.len())
.then_with(|| vx.cmp(vy))
.then_with(|| x.len().cmp(&y.len()))
} else {
x.cmp(y)
}
}
/// What each `(id, display)` pair is shown as: equal displays get ` #2`, ` #3`, …
/// in natural id order, skipping numbers a literal display already holds.
pub fn display_suffixes(items: &[(&str, &str)]) -> Vec<String> {
let literal: HashSet<&str> = items.iter().map(|&(_, d)| d).collect();
let mut groups: HashMap<&str, Vec<usize>> = HashMap::new();
for (i, &(_, d)) in items.iter().enumerate() {
groups.entry(d).or_default().push(i);
}
let mut shown: Vec<String> = items.iter().map(|&(_, d)| d.to_string()).collect();
for (display, mut at) in groups {
at.sort_by(|&i, &j| natural_cmp(items[i].0, items[j].0));
let mut n = 2;
for &i in at.iter().skip(1) {
shown[i] = loop {
let candidate = format!("{display} #{n}");
n += 1;
if !literal.contains(candidate.as_str()) {
break candidate;
}
};
}
}
shown
}
/// Why a hand-written id cannot enter the document.
@@ -81,12 +156,13 @@ impl IdAllocator {
Self::default()
}
/// Admit a hand-written id verbatim: it is canonical and never renamed (answer 29).
/// Admit a hand-written id verbatim: it is canonical and never renamed.
pub fn claim(&mut self, id: &str) -> Result<(), ClaimError> {
let refusal = if id.is_empty() || !id.chars().all(in_id_alphabet) {
Some(ClaimError::Malformed(id.to_string()))
} else if id.starts_with('_') && !VIRTUAL_IDS.contains(&id) {
let virtual_id = VIRTUAL_IDS.contains(&id);
let refusal = if id.starts_with('_') && !virtual_id {
Some(ClaimError::Reserved(id.to_string()))
} else if !virtual_id && !is_id_syntax(id) {
Some(ClaimError::Malformed(id.to_string()))
} else if self.seen.contains(id) {
Some(ClaimError::Duplicate(id.to_string()))
} else {
@@ -132,16 +208,26 @@ mod tests {
use proptest::prelude::*;
#[test]
fn slugify_follows_the_adrs_examples() {
fn slugify_follows_the_amendments_table() {
let cases = [
("East", "east"),
("Time Period", "time_period"),
("Coastal_East", "coastal_east"),
("Q1-2024", "q1-2024"),
("Region(code)", "regioncode"),
("Café", "caf"),
("_Draft", "draft"),
("Time\tPeriod", "time_period"),
("Time \u{200B} Period", "time_period"),
("a_ b", "a_b"),
("a__b", "a_b"),
("Q1 - 2024", "q1_-_2024"),
("East ", "east"),
("East_", "east"),
("-East-", "east"),
("Region(code)", "region_code"),
("Crème Brûlée", "cr_me_br_l_e"),
("2024", "n2024"),
("_2024", "n2024"),
("2024-Q1", "n2024-q1"),
("Ü", ""),
("-", ""),
("%", ""),
("", ""),
];
for (display, slug) in cases {
assert_eq!(slugify(display), slug, "slugify({display:?})");
@@ -153,18 +239,25 @@ mod tests {
assert_eq!(slugify("Straße"), "strasse");
assert_eq!(slugify("ſtate"), "state");
assert_eq!(slugify("file"), "file");
assert_eq!(slugify("İ"), "i");
}
#[test]
fn slugify_collapses_a_run_of_spaces_to_one_underscore() {
assert_eq!(slugify("Time Period"), "time_period");
assert_eq!(slugify("East "), "east_");
fn slugify_trims_hyphens_and_underscores_until_neither_is_at_an_end() {
assert_eq!(slugify("_-_East-_-"), "east");
assert_eq!(slugify("-_-"), "");
}
#[test]
fn slugify_keeps_an_underscore_the_display_already_had() {
assert_eq!(slugify("a_ b"), "a__b");
assert_eq!(slugify("a__b"), "a__b");
fn slugify_trims_before_it_prepends_the_n() {
assert_eq!(slugify("-_7up"), "n7up");
assert_eq!(slugify("n-7"), "n-7");
}
#[test]
fn slugify_keeps_a_hyphen_between_underscores() {
assert_eq!(slugify("a - b"), "a_-_b");
assert_eq!(slugify("a_-b"), "a_-b");
}
#[test]
@@ -173,6 +266,8 @@ mod tests {
assert_eq!(ids.generate(IdKind::Category, "Ü"), "category-1");
assert_eq!(ids.generate(IdKind::Item, "Ö"), "item-2");
assert_eq!(ids.generate(IdKind::View, ""), "view-3");
assert_eq!(ids.generate(IdKind::Group, "-"), "group-4");
assert_eq!(ids.generate(IdKind::Formula, "%"), "formula-5");
}
#[test]
@@ -197,6 +292,15 @@ mod tests {
assert_eq!(ids.generate(IdKind::Item, "EAST"), "east-3");
}
#[test]
fn a_hyphenated_slug_and_a_counter_suffix_never_collide() {
let mut ids = IdAllocator::new();
assert_eq!(ids.generate(IdKind::Item, "East"), "east");
assert_eq!(ids.generate(IdKind::Item, "East-1"), "east-1");
assert_eq!(ids.generate(IdKind::Item, "east"), "east-2");
assert_eq!(ids.generate(IdKind::Item, "East-1"), "east-1-3");
}
#[test]
fn a_claimed_id_keeps_its_spelling_and_generation_steps_past_it() {
let mut ids = IdAllocator::new();
@@ -224,10 +328,21 @@ mod tests {
);
}
#[test]
fn claim_refuses_what_a_generated_id_could_not_be() {
let mut ids = IdAllocator::new();
for bad in ["-x", "x-", "2024", "a__b", "x_", "-", "a b", "é"] {
assert_eq!(ids.claim(bad), Err(ClaimError::Malformed(bad.into())));
}
for good in ["east-1", "item-4", "a-7", "q1_-_2024", "a_b", "x"] {
assert_eq!(ids.claim(good), Ok(()), "{good}");
}
}
#[test]
fn claim_refuses_an_id_that_only_begins_like_a_virtual_one() {
let mut ids = IdAllocator::new();
for lookalike in ["_indexer", "_measure-1", "_dim_2", "_measures"] {
for lookalike in ["_indexer", "_measure-1", "_dim_2", "_measures", "_recordsx"] {
assert_eq!(
ids.claim(lookalike),
Err(ClaimError::Reserved(lookalike.into()))
@@ -235,10 +350,20 @@ mod tests {
}
}
#[test]
fn claim_admits_each_virtual_id_once() {
let mut ids = IdAllocator::new();
for v in ["_index", "_dim", "_measure", "_records", "_drill"] {
assert_eq!(ids.claim(v), Ok(()));
assert_eq!(ids.claim(v), Err(ClaimError::Duplicate(v.into())));
}
}
#[test]
fn a_display_with_leading_spaces_never_slugs_into_the_reserved_space() {
assert_eq!(slugify(" Draft"), "draft");
assert_eq!(slugify(" _ Draft"), "draft");
assert_eq!(slugify("_Drill"), "drill");
}
#[test]
@@ -272,7 +397,159 @@ mod tests {
assert_eq!(run(), run());
}
#[test]
fn natural_order_puts_a_counter_suffix_in_numeric_order() {
let mut ids = vec!["profit-10", "profit-2", "profit", "profit-1"];
ids.sort_by(|a, b| natural_cmp(a, b));
assert_eq!(ids, ["profit", "profit-1", "profit-2", "profit-10"]);
}
#[test]
fn natural_order_ties_on_value_go_to_fewer_leading_zeros() {
assert_eq!(natural_cmp("a-7", "a-007"), Ordering::Less);
assert_eq!(natural_cmp("a-007", "a-7"), Ordering::Greater);
assert_eq!(natural_cmp("a-07", "a-007"), Ordering::Less);
assert_eq!(natural_cmp("a-7", "a-7"), Ordering::Equal);
}
#[test]
fn natural_order_compares_digit_runs_of_any_length_by_value() {
let big = "9".repeat(40);
let bigger = format!("1{}", "0".repeat(40));
assert_eq!(
natural_cmp(&format!("x{big}"), &format!("x{bigger}")),
Ordering::Less
);
assert_eq!(natural_cmp("x9", "x10"), Ordering::Less);
assert_eq!(natural_cmp("x000", "x0"), Ordering::Greater);
}
#[test]
fn natural_order_decides_the_zeros_tie_at_its_own_run_not_after_the_rest() {
assert_eq!(natural_cmp("a-7c", "a-007b"), Ordering::Less);
assert_eq!(natural_cmp("a-7b", "a-007a"), Ordering::Less);
assert_eq!(natural_cmp("a-007a", "a-7b"), Ordering::Greater);
}
#[test]
fn natural_order_puts_the_id_that_runs_out_of_runs_first() {
assert_eq!(natural_cmp("a", "a-1"), Ordering::Less);
assert_eq!(natural_cmp("a-1", "a-1-b"), Ordering::Less);
assert_eq!(natural_cmp("a-1b", "a-1"), Ordering::Greater);
}
#[test]
fn natural_order_compares_non_digit_runs_by_bytes() {
assert_eq!(natural_cmp("b1", "a2"), Ordering::Greater);
assert_eq!(natural_cmp("a-1", "a_1"), Ordering::Less);
}
#[test]
fn a_suffix_numbers_equal_displays_in_natural_id_order() {
let items = [
("profit-10", "Profit"),
("profit", "Profit"),
("profit-2", "Profit"),
("profit-1", "Profit"),
("cost", "Cost"),
];
assert_eq!(
display_suffixes(&items),
["Profit #4", "Profit", "Profit #3", "Profit #2", "Cost"]
);
}
#[test]
fn a_suffix_skips_a_number_a_literal_display_already_holds() {
let items = [
("profit", "Profit"),
("profit-1", "Profit"),
("profit-2", "Profit"),
("p2", "Profit #2"),
];
assert_eq!(
display_suffixes(&items),
["Profit", "Profit #3", "Profit #4", "Profit #2"]
);
}
#[test]
fn a_suffix_is_gapless_when_a_thing_goes_away() {
let three = [("p", "Profit"), ("p-1", "Profit"), ("p-2", "Profit")];
assert_eq!(
display_suffixes(&three),
["Profit", "Profit #2", "Profit #3"]
);
let two = [("p", "Profit"), ("p-2", "Profit")];
assert_eq!(display_suffixes(&two), ["Profit", "Profit #2"]);
}
#[test]
fn a_unique_display_is_shown_unchanged() {
let items = [("a", "East"), ("b", "West")];
assert_eq!(display_suffixes(&items), ["East", "West"]);
assert!(display_suffixes(&[]).is_empty());
}
/// `^[a-z](_?[a-z0-9-])*$` and not ending in `-`, without the crate's own helpers.
fn regex_free_match(id: &str) -> bool {
let mut cs = id.chars().peekable();
if !matches!(cs.next(), Some('a'..='z')) {
return false;
}
while cs.peek().is_some() {
if cs.peek() == Some(&'_') {
cs.next();
}
if !matches!(cs.next(), Some('a'..='z' | '0'..='9' | '-')) {
return false;
}
}
!id.ends_with('-')
}
fn id_text() -> impl Strategy<Value = String> {
"[a-c0-3_-]{0,7}"
}
proptest! {
#[test]
fn slugify_output_has_the_id_syntax_and_is_a_fixed_point(display in ".{0,12}") {
let slug = slugify(&display);
prop_assert!(slug.is_empty() || is_id_syntax(&slug), "{slug:?}");
prop_assert_eq!(slugify(&slug), slug);
}
#[test]
fn id_syntax_agrees_with_an_independent_checker(id in "[a-cA-C0-9 _%é-]{0,8}") {
prop_assert_eq!(is_id_syntax(&id), regex_free_match(&id), "{}", id);
}
#[test]
fn natural_order_is_a_total_order(a in id_text(), b in id_text(), c in id_text()) {
prop_assert_eq!(natural_cmp(&a, &b), natural_cmp(&b, &a).reverse());
prop_assert_eq!(natural_cmp(&a, &b) == Ordering::Equal, a == b);
if natural_cmp(&a, &b) != Ordering::Greater && natural_cmp(&b, &c) != Ordering::Greater {
prop_assert!(natural_cmp(&a, &c) != Ordering::Greater);
}
}
#[test]
fn suffixed_displays_are_distinct_when_literals_already_hold_suffixes(
shared in proptest::collection::vec(0usize..4, 0..8)
) {
let displays = ["A", "A #2", "A #3", "B"];
let ids: Vec<String> = (0..shared.len()).map(|n| format!("x-{n}")).collect();
let items: Vec<(&str, &str)> = ids
.iter()
.zip(&shared)
.map(|(i, &d)| (i.as_str(), displays[d]))
.collect();
let shown = display_suffixes(&items);
let distinct: HashSet<&String> = shown.iter().collect();
prop_assert_eq!(distinct.len(), shown.len());
}
#[test]
fn generated_ids_are_distinct_well_formed_and_never_reserved(
displays in proptest::collection::vec(".{0,6}", 0..24)
@@ -281,8 +558,7 @@ mod tests {
let mut out = HashSet::new();
for d in &displays {
let id = ids.generate(IdKind::Item, d);
prop_assert!(!id.is_empty() && !id.starts_with('_'), "{id:?}");
prop_assert!(id.chars().all(|c| matches!(c, 'a'..='z' | '0'..='9' | '-' | '_')));
prop_assert!(is_id_syntax(&id), "{id:?}");
prop_assert!(out.insert(id.clone()), "reissued {id:?}");
}
}
+8 -1
View File
@@ -402,7 +402,14 @@ impl AppMode {
}
pub fn item_add(category: String) -> Self {
let prompt = format!("add item to {category}: ");
let shown = category.clone();
Self::item_add_shown(category, &shown)
}
/// The prompt names the category by what a person sees; `category` stays the
/// key the commit adds the item to (improvise-cq1q.13).
pub fn item_add_shown(category: String, shown: &str) -> Self {
let prompt = format!("add item to {shown}: ");
Self::ItemAdd {
category,
minibuf: MinibufferConfig {
+26 -2
View File
@@ -68,8 +68,8 @@ pub enum CellId {
NonItem { category: String, item: String },
}
/// Why a cell whose coordinate names no item takes no write. One wording for the
/// projection's decline and the host's refusal (improvise-gk51).
/// Why a cell naming no item takes no write (improvise-gk51). `category` is a
/// display; `item` is the stray key, which has none (`core.md` §21).
pub fn non_item_refusal(category: &str, item: &str) -> String {
format!("{item} is no item of {category}, so there is no one cell to write")
}
@@ -838,6 +838,19 @@ impl CategoryTreeCache {
pub fn category(&self, name: &str) -> Option<&CategoryRow> {
self.categories.iter().find(|c| c.name == name)
}
/// What a person sees for a category key; the key when the cache lacks it.
/// For a message only: never pass the result to a lookup (core.md §21).
pub fn category_display<'a>(&'a self, name: &'a str) -> &'a str {
self.category(name).map_or(name, |c| c.display.as_str())
}
/// What a person sees for an item of `cat`; the key when either is absent.
pub fn item_display<'a>(&'a self, cat: &str, item: &'a str) -> &'a str {
self.category(cat)
.and_then(|c| c.items.iter().find(|i| i.name == item))
.map_or(item, |i| i.display.as_str())
}
}
/// Three mutations, because the effects are three *shapes*. `Items` and
@@ -955,6 +968,17 @@ pub struct ViewListCache {
pub views: Vec<ViewSummary>,
}
impl ViewListCache {
/// What a person sees for a view key; the key when the list lacks it.
/// For a title or a message only, never a lookup (core.md §21).
pub fn display_of<'a>(&'a self, key: &'a str) -> &'a str {
self.views
.iter()
.find(|v| v.name == key)
.map_or(key, |v| v.display.as_str())
}
}
impl WidgetCache for ViewListCache {
type Update = Vec<ViewSummary>;
+187 -12
View File
@@ -7,7 +7,10 @@ use serde::{Deserialize, Serialize};
use crate::client::{ClientState, Opened, OpenedCell, OpenedTarget};
use crate::frame::ClientFrame;
use crate::keymap::{Binding, Keymaps, ModeKey};
use crate::viewmodel::{CatTreeEntry, axis_change_status, flatten_cat_tree};
use crate::viewmodel::{
CatTreeEntry, axis_change_status, deleted_category_status, deleted_item_status, filter_status,
flatten_cat_tree,
};
use crate::{
AppMode, CellId, CellValue, Command, CreateKind, FormulaId, FormulaSlot, GridExtent, HelpPage,
KeyCode, KeyEvent, ModelCommand, Panel, RECORD_COORDS_CANNOT_BE_EMPTY, RecordCell, ViewState,
@@ -579,8 +582,9 @@ fn composed(
category: cat.clone(),
},
)?];
let shown = state.categories.category_display(&cat);
commands
.extend(cycled.map(|axis| Command::SetStatus(axis_change_status(&cat, axis))));
.extend(cycled.map(|axis| Command::SetStatus(axis_change_status(shown, axis))));
commands
}
None => vec![Command::SetStatus(
@@ -592,7 +596,10 @@ fn composed(
"delete-formula-at-cursor" => delete_formula_at_cursor(state),
"open-item-add-at-cursor" => Some(match cat_at_cursor(state) {
Some(cat) => vec![Command::ChangeMode(AppMode::item_add(cat))],
Some(cat) => {
let shown = state.categories.category_display(&cat).to_string();
vec![Command::ChangeMode(AppMode::item_add_shown(cat, &shown))]
}
None => vec![Command::SetStatus(
"No category selected. Press n to add a category first.".to_string(),
)],
@@ -961,7 +968,7 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
// them; declining rather than `None` keeps that a door, not a hole.
CellId::Create(_) => Some(vec![Command::SetStatus(not_yet("creating a column"))]),
CellId::NonItem { category, item } => Some(vec![Command::SetStatus(
crate::non_item_refusal(category, item),
crate::non_item_refusal(state.categories.category_display(category), item),
)]),
}
}
@@ -1400,7 +1407,10 @@ pub(crate) fn declined_at(state: &ClientState, row: usize, col: usize) -> Option
match projected_at(state, row, col)?.id.as_ref()? {
CellId::Aggregate(_) => Some(AGGREGATE_DECLINE.to_string()),
CellId::Derived { target } => Some(derived_decline(target)),
CellId::NonItem { category, item } => Some(crate::non_item_refusal(category, item)),
CellId::NonItem { category, item } => Some(crate::non_item_refusal(
state.categories.category_display(category),
item,
)),
CellId::Record(_) | CellId::Create(_) | CellId::Coordinate { .. } => None,
}
}
@@ -1613,7 +1623,7 @@ fn filter_to_item_at(state: &ClientState, index: usize) -> Option<Vec<Command>>
Some(CatTreeEntry::Item {
cat_name,
item_name,
..
display,
}) => Some(vec![
model(
state,
@@ -1631,7 +1641,10 @@ fn filter_to_item_at(state: &ClientState, index: usize) -> Option<Vec<Command>>
item: item_name.clone(),
},
)?,
Command::SetStatus(format!("Filter: {cat_name} = {item_name}")),
Command::SetStatus(filter_status(
state.categories.category_display(&cat_name),
&display,
)),
]),
Some(CatTreeEntry::Category { .. }) => Some(toggle_cat_expand_at(state, index)),
None => Some(Vec::new()),
@@ -1642,14 +1655,14 @@ fn filter_to_item_at(state: &ClientState, index: usize) -> Option<Vec<Command>>
/// row deletes the item, and an empty tree says so.
fn delete_at_cat_cursor(state: &ClientState) -> Option<Vec<Command>> {
match cat_tree_entry(state) {
Some(CatTreeEntry::Category { name, .. }) => Some(vec![
Some(CatTreeEntry::Category { name, display, .. }) => Some(vec![
model(state, ModelCommand::RemoveCategory(name.clone()))?,
Command::SetStatus(format!("Deleted category '{name}'")),
Command::SetStatus(deleted_category_status(&display)),
]),
Some(CatTreeEntry::Item {
cat_name,
item_name,
..
display,
}) => {
let mut commands = vec![
model(
@@ -1659,7 +1672,10 @@ fn delete_at_cat_cursor(state: &ClientState) -> Option<Vec<Command>> {
item: item_name.clone(),
},
)?,
Command::SetStatus(format!("Deleted item '{item_name}' from '{cat_name}'")),
Command::SetStatus(deleted_item_status(
&display,
state.categories.category_display(&cat_name),
)),
];
commands.extend(retreat_from_deleted_item(state));
Some(commands)
@@ -1786,7 +1802,7 @@ fn tile_axis_at(state: &ClientState, index: usize, axis: Option<Axis>) -> Option
};
Some(vec![
model(state, cmd)?,
Command::SetStatus(axis_change_status(&tile.category, new_axis)),
Command::SetStatus(axis_change_status(&tile.display, new_axis)),
])
}
@@ -5014,3 +5030,162 @@ mod opened_editor_wire_tests {
);
}
}
#[cfg(test)]
mod display_tests {
use super::*;
use crate::{
AxisFrame, CategoryItem, CategoryRow, CategoryTreeUpdate, CellDisplay, GridExtent,
GridUpdate, KeyModifiers, Projection, Snapshot, TileEntry, ViewState,
};
const REGION: &str = "Sales territory of the whole company";
const NORTH: &str = "Northern reaches and the colder coasts";
fn ev(code: KeyCode) -> KeyEvent {
KeyEvent::new(code, KeyModifiers::NONE)
}
fn shown(name: &str, display: &str) -> CategoryItem {
CategoryItem {
name: name.into(),
display: display.into(),
}
}
/// Every key differs from, and is far shorter than, what a person sees.
fn displayed(cell: Option<CellId>) -> ClientState {
let mut state = ClientState::default();
let mut first = CellDisplay::number("1");
first.id = cell;
state.bootstrap(Snapshot {
view: ViewState::default(),
seed: vec![
Projection::CategoryTree(CategoryTreeUpdate::Categories(vec![CategoryRow {
name: "Region".into(),
display: REGION.into(),
items: vec![shown("North", NORTH)],
axis: Axis::Row,
}])),
Projection::TileBar(vec![TileEntry {
category: "Region".into(),
display: REGION.into(),
axis: Axis::Row,
kind: crate::CategoryKind::Regular,
}]),
Projection::Grid(GridUpdate::RowFrame(AxisFrame::flat(["North"]))),
Projection::Grid(GridUpdate::ColFrame(AxisFrame::flat(["Total"]))),
Projection::Grid(GridUpdate::Cells(vec![((0, 0), first)])),
Projection::Grid(GridUpdate::Extent(GridExtent { rows: 1, cols: 1 })),
Projection::Basis(crate::basis::published(7, 3, Default::default())),
],
});
state.window = (1, 1);
state
}
fn last_status(commands: Vec<Command>) -> String {
match commands.last() {
Some(Command::SetStatus(said)) => said.clone(),
other => panic!("no status last in {commands:?}: {other:?}"),
}
}
fn local(state: &ClientState, code: KeyCode) -> Vec<Command> {
match resolve(state, ev(code)) {
KeyOutcome::Local(commands) => commands,
other => panic!("{code:?} did not resolve locally: {other:?}"),
}
}
#[test]
fn a_panel_status_names_what_a_person_sees_not_the_keys() {
let mut state = displayed(None);
state.view.mode = AppMode::CategoryPanel;
state.view.expanded_cats.insert("Region".into());
state.view.cat_panel_cursor = 0;
let cycled = last_status(local(&state, KeyCode::Char(' ')));
assert_eq!(cycled, format!("{REGION} → Col"));
let removal = delete_at_cat_cursor(&state).unwrap();
assert!(
removal.iter().any(|c| matches!(c, Command::Model {
cmd: ModelCommand::RemoveCategory(name), ..
} if name == "Region")),
"the removal names the key: {removal:?}"
);
let deleted = last_status(removal);
assert_eq!(deleted, format!("Deleted category '{REGION}'"));
state.view.cat_panel_cursor = 1;
let filtering = filter_to_item(&state).unwrap();
let named: Vec<String> = filtering
.iter()
.filter_map(|c| match c {
Command::Model {
cmd: ModelCommand::SetPageSelection { category, item, .. },
..
} => Some(format!("{category}/{item}")),
_ => None,
})
.collect();
assert_eq!(named, ["Region/North"], "the lookup names keys");
let filtered = last_status(filtering);
assert_eq!(filtered, format!("Filter: {REGION} = {NORTH}"));
let removal = delete_at_cat_cursor(&state).unwrap();
assert!(
removal.iter().any(|c| matches!(c, Command::Model {
cmd: ModelCommand::RemoveItem { category, item }, ..
} if category == "Region" && item == "North")),
"the removal names keys: {removal:?}"
);
let deleted = last_status(removal);
assert_eq!(deleted, format!("Deleted item '{NORTH}' from '{REGION}'"));
}
#[test]
fn a_tile_status_names_the_display() {
let mut state = displayed(None);
state.view.mode = AppMode::TileSelect;
let said = last_status(tile_axis(&state, None).unwrap());
assert_eq!(said, format!("{REGION} → Col"));
}
#[test]
fn the_item_prompt_names_the_display_and_the_mode_keeps_the_key() {
let mut state = displayed(None);
state.view.mode = AppMode::CategoryPanel;
state.view.cat_panel_cursor = 0;
let commands = local(&state, KeyCode::Char('a'));
let Some(Command::ChangeMode(AppMode::ItemAdd { category, minibuf })) = commands
.iter()
.find(|c| matches!(c, Command::ChangeMode(_)))
else {
panic!("no item-add mode in {commands:?}");
};
assert_eq!(category, "Region", "the commit adds to the KEY");
assert_eq!(minibuf.prompt, format!("add item to {REGION}: "));
}
#[test]
fn the_declines_an_advance_and_a_commit_reach_name_the_category_it_shows() {
let stray = displayed(Some(CellId::NonItem {
category: "Region".into(),
item: "Mars".into(),
}));
let want = crate::non_item_refusal(REGION, "Mars");
assert_eq!(declined_at(&stray, 0, 0), Some(want.clone()));
assert_eq!(refused_here(&stray, "commit-cell-edit"), Some(want.clone()));
let advanced = reopened_at(&stray, 0, 0, AppMode::Normal, false);
assert_eq!(advanced, vec![Command::SetStatus(want)]);
}
#[test]
fn a_refused_cell_names_the_category_it_shows() {
let stray = displayed(Some(CellId::NonItem {
category: "Region".into(),
item: "Mars".into(),
}));
let said = last_status(local(&stray, KeyCode::Char('i')));
assert!(said.contains(REGION) && said.starts_with("Mars "), "{said}");
}
}
+17 -2
View File
@@ -37,12 +37,27 @@ impl CatTreeEntry {
}
}
/// What a moved axis says on the status line. One home: the tile bar and the
/// category panel are two routes to one operation — improvise-25bf.
/// What a moved axis says on the status line: one home for the tile bar and the
/// panel (improvise-25bf). Takes displays; `core.md` §21.
pub fn axis_change_status(category: &str, axis: Axis) -> String {
format!("{category} → {}", axis.plain_label())
}
/// What `filter-to-item` says; both ends, so one wording (improvise-cq1q.13).
pub fn filter_status(category: &str, item: &str) -> String {
format!("Filter: {category} = {item}")
}
/// What deleting a category at the panel cursor says.
pub fn deleted_category_status(category: &str) -> String {
format!("Deleted category '{category}'")
}
/// What deleting an item at the panel cursor says.
pub fn deleted_item_status(item: &str, category: &str) -> String {
format!("Deleted item '{item}' from '{category}'")
}
/// Flatten the category panel: a header per category, then its items when
/// expanded. The result index is `ViewState::cat_panel_cursor`.
pub fn flatten_cat_tree(
+19 -1
View File
@@ -243,7 +243,7 @@ pub fn render_model(state: &ClientState) -> RenderModel {
mode: improvise_protocol::mode_label(&state.view.mode).to_string(),
status: state.view.status_msg.clone(),
status_stands: state.status_stands(),
view_name: state.view.active_view.clone(),
view_name: state.views.display_of(&state.view.active_view).to_string(),
doc_name: state.document.name.clone(),
doc_dirty: state.document.dirty,
doc_stored_as: state.document.stored_as.clone(),
@@ -883,6 +883,24 @@ mod tests {
assert!(views[0].active, "the active view is still found by its key");
}
#[test]
fn the_title_names_the_active_views_display_not_its_key() {
let mut client = init();
let state = client.state_mut();
state.fold(&Command::Project(Projection::ViewList(vec![ViewSummary {
name: "Default".into(),
display: "Quarterly overview of regional sales".into(),
axes: vec![],
}])));
state.view.active_view = "Default".into();
assert_eq!(
client.model().view_name,
"Quarterly overview of regional sales"
);
client.state_mut().view.active_view = "Unlisted".into();
assert_eq!(client.model().view_name, "Unlisted", "no display: the key");
}
#[test]
fn the_grid_page_bar_and_item_rows_paint_displays() {
use improvise_protocol::{AxisFrame, CategoryItem, FrameGroup};
+25 -1
View File
@@ -409,7 +409,7 @@ fn draw_content(f: &mut Frame, area: Rect, app: &App) {
GridWidget::new(
&grid_cache,
app.cursor(),
&app.view_state.active_view,
app.view().display(),
&app.layout,
&app.view_state.mode,
&app.view_state.search_query,
@@ -1201,6 +1201,30 @@ mod content_tests {
.collect()
}
/// improvise-cq1q.13: evidence in `context/test-evidence/core.md`.
#[test]
fn the_grid_title_names_the_views_display_not_its_key() {
let mut wb = wide_grid();
wb.active_view_mut()
.set_display("Quarterly overview of regional sales");
let mut app = App::new(wb, None);
(app.term_width, app.term_height) = (100, 24);
let mut terminal = Terminal::new(TestBackend::new(100, 24)).expect("test backend");
terminal.draw(|f| draw(f, &app)).expect("draw");
let text: String = terminal
.backend()
.buffer()
.content()
.iter()
.map(|c| c.symbol())
.collect();
assert!(
text.contains("View: Quarterly overview of regional sales"),
"the title drew something else"
);
assert!(!text.contains("View: Default"), "the key was drawn");
}
/// improvise-gtn7.1: the cursor went behind a side panel —
/// `context/test-evidence/tui.md`.
#[test]
+5 -4
View File
@@ -23,7 +23,8 @@ pub struct GridWidget<'a> {
/// This client's cursor in the active view (improvise-8uz). Passed in
/// rather than read off `View`, which no longer carries per-client state.
pub cursor: ViewCursor,
pub view_name: &'a str,
/// What the title shows: the view's display, never its key (improvise-cq1q.13).
pub view_display: &'a str,
pub layout: &'a GridLayout,
pub mode: &'a AppMode,
pub search_query: &'a str,
@@ -36,7 +37,7 @@ impl<'a> GridWidget<'a> {
pub fn new(
cache: &'a GridCache,
cursor: ViewCursor,
view_name: &'a str,
view_display: &'a str,
layout: &'a GridLayout,
mode: &'a AppMode,
search_query: &'a str,
@@ -46,7 +47,7 @@ impl<'a> GridWidget<'a> {
Self {
cache,
cursor,
view_name,
view_display,
layout,
mode,
search_query,
@@ -444,7 +445,7 @@ impl<'a> Widget for GridWidget<'a> {
fn render(self, area: Rect, buf: &mut Buffer) {
let block = Block::default()
.borders(Borders::ALL)
.title(format!(" View: {} ", self.view_name));
.title(format!(" View: {} ", self.view_display));
let inner = block.inner(area);
block.render(area, buf);