fix(display): defer the formula-result display: the surrogate cannot find the target's category (improvise-cq1q.13)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
committed by
Edward Langley
parent
bb7fe88547
commit
300b5afc7f
@@ -1798,8 +1798,8 @@ Still keys: `GridLayout::row_label`/`col_label` (commands match on them),
|
||||
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 formula-result and non-item
|
||||
refusals, and the item-add prompt. Both ends say them through the same
|
||||
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.
|
||||
@@ -1812,9 +1812,11 @@ 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 target has no display path yet
|
||||
(`set_item_display` needs an explicit `_Measure` item), so `derived_refusal` reads
|
||||
`item_display("_Measure", target)` and in practice says the key.
|
||||
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
|
||||
|
||||
@@ -2175,11 +2175,10 @@ 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_displays_it_shows
|
||||
### resolve::display_tests::a_refused_cell_names_the_category_it_shows
|
||||
|
||||
Rival: the non-item refusal gets the category key. The derived arm is covered by
|
||||
the item display for `_Measure`; the stray coordinate stays a key because it names
|
||||
no item and has no display.
|
||||
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
|
||||
|
||||
@@ -2193,12 +2192,6 @@ axis status passes `cat_name` (fails); the filter status passes the item key
|
||||
Rival: the status formats `name`. It fails on both the present display and the
|
||||
absent key.
|
||||
|
||||
### command::cmd::cell::tests::a_refusal_names_the_formulas_display_not_its_key
|
||||
|
||||
A formula target has no display path today: `set_item_display` needs an explicit
|
||||
`_Measure` item, so the fixture adds one named like the target. Rival:
|
||||
`derived_refusal` skips the lookup; the test fails on "Operating profit...".
|
||||
|
||||
### 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
|
||||
@@ -2210,8 +2203,8 @@ The third site, `uneditable` in `cmd/core.rs`, which only a commit reaches once
|
||||
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_displays_it_shows
|
||||
### ui::app::non_item_key_tests::a_refused_wire_write_names_the_category_it_shows
|
||||
|
||||
The wire's own refusals in `Grids::unwritable`: the non-item arm and the formula
|
||||
arm (once a third copy of the derived wording, now `derived_refusal`). Rivals: the
|
||||
category key; a model with no display for the formula. Each fails.
|
||||
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.
|
||||
|
||||
@@ -494,24 +494,6 @@ mod tests {
|
||||
app
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_refusal_names_the_formulas_display_not_its_key() {
|
||||
let mut app = formula_app();
|
||||
let measure = app.model_state.workbook.model.category_mut("_Measure");
|
||||
let measure = measure.unwrap();
|
||||
measure.add_item("Profit");
|
||||
measure.set_item_display("Profit", "Operating profit after all costs");
|
||||
app.cursor_mut().selected = (1, 2);
|
||||
press(&mut app, 'p');
|
||||
assert!(
|
||||
app.view_state
|
||||
.status_msg
|
||||
.contains("Operating profit after all costs is computed"),
|
||||
"got {:?}",
|
||||
app.view_state.status_msg
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-54g1 — see context/test-evidence/command.md.
|
||||
#[test]
|
||||
fn p_onto_a_formula_owned_cell_writes_nothing_and_names_the_formula() {
|
||||
|
||||
@@ -23,8 +23,7 @@ pub const INSIDE_A_DRILL_REFUSAL: &str =
|
||||
"A value is not staged inside a drill — edit the cell instead";
|
||||
|
||||
/// What a command says when the cell it would write is a formula's result.
|
||||
pub fn derived_refusal(model: &Model, target: &str) -> String {
|
||||
let target = model.item_display("_Measure", target);
|
||||
pub fn derived_refusal(target: &str) -> String {
|
||||
format!("{target} is computed by a formula — edit the formula, not the cell")
|
||||
}
|
||||
|
||||
@@ -39,7 +38,7 @@ pub fn uneditable(model: &Model, layout: &GridLayout, row: usize, col: usize) ->
|
||||
}
|
||||
.to_string(),
|
||||
),
|
||||
Some(CellId::Derived { target }) => Some(derived_refusal(model, &target)),
|
||||
Some(CellId::Derived { target }) => Some(derived_refusal(&target)),
|
||||
Some(CellId::NonItem { category, item }) => {
|
||||
Some(non_item_refusal(model.category_display(&category), &item))
|
||||
}
|
||||
@@ -132,7 +131,7 @@ impl<'a> CmdContext<'a> {
|
||||
match self.cursor_cell_id() {
|
||||
Some(CellId::Record(_)) => self.stored_cell_key(),
|
||||
Some(CellId::Aggregate(_)) => Err(self.aggregate_refusal()),
|
||||
Some(CellId::Derived { target }) => Err(derived_refusal(self.model, &target)),
|
||||
Some(CellId::Derived { target }) => Err(derived_refusal(&target)),
|
||||
Some(CellId::Create(_) | CellId::Coordinate { .. }) => {
|
||||
Err(NO_VALUE_REFUSAL.to_string())
|
||||
}
|
||||
|
||||
@@ -386,7 +386,6 @@ impl Cmd for EditOrDrill {
|
||||
// the synthetic key names no measure, so the record's key is resolved.
|
||||
if let Some(formula) = ctx.owning_formula() {
|
||||
return vec![effect::refuse(super::core::derived_refusal(
|
||||
ctx.model,
|
||||
&formula.target,
|
||||
))];
|
||||
}
|
||||
|
||||
@@ -334,7 +334,7 @@ impl Grids<'_> {
|
||||
let model = &self.app.model_state.workbook.model;
|
||||
let derived = model
|
||||
.formula_for(&key)
|
||||
.map(|f| crate::command::cmd::core::derived_refusal(model, &f.target));
|
||||
.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))
|
||||
}
|
||||
@@ -5618,9 +5618,8 @@ mod non_item_key_tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_refused_wire_write_names_the_displays_it_shows() {
|
||||
fn a_refused_wire_write_names_the_category_it_shows() {
|
||||
const REGION: &str = "Sales territory of the whole company";
|
||||
const PROFIT: &str = "Operating profit after all costs";
|
||||
let mut app = paged_app("East");
|
||||
let stray = CellKey::new(vec![
|
||||
("Month".into(), "Jan".into()),
|
||||
@@ -5633,9 +5632,6 @@ mod non_item_key_tests {
|
||||
model.add_formula(
|
||||
crate::formula::parse_formula("Profit = Revenue - 1", "_Measure").unwrap(),
|
||||
);
|
||||
let measure = model.category_mut("_Measure").unwrap();
|
||||
measure.add_item("Profit");
|
||||
measure.set_item_display("Profit", PROFIT);
|
||||
let refused = |key: &CellKey, measure: &str| {
|
||||
let cell = RecordCell {
|
||||
record: RecordHash::of(key),
|
||||
@@ -5655,7 +5651,7 @@ mod non_item_key_tests {
|
||||
("_Measure".into(), "Profit".into()),
|
||||
]);
|
||||
let why = refused(&profit, "Profit").expect("a formula's result is refused");
|
||||
assert!(why.starts_with(PROFIT), "{why}");
|
||||
assert!(why.starts_with("Profit is computed by a formula"), "{why}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -963,9 +963,7 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
|
||||
"an aggregate is edited by drilling in — {}",
|
||||
not_yet("drill")
|
||||
))]),
|
||||
CellId::Derived { target } => {
|
||||
Some(vec![Command::SetStatus(derived_decline(state, target))])
|
||||
}
|
||||
CellId::Derived { target } => Some(vec![Command::SetStatus(derived_decline(target))]),
|
||||
// Both creating columns are records-only, so the guard above takes
|
||||
// them; declining rather than `None` keeps that a door, not a hole.
|
||||
CellId::Create(_) => Some(vec![Command::SetStatus(not_yet("creating a column"))]),
|
||||
@@ -1408,7 +1406,7 @@ fn reopened_at(
|
||||
pub(crate) fn declined_at(state: &ClientState, row: usize, col: usize) -> Option<String> {
|
||||
match projected_at(state, row, col)?.id.as_ref()? {
|
||||
CellId::Aggregate(_) => Some(AGGREGATE_DECLINE.to_string()),
|
||||
CellId::Derived { target } => Some(derived_decline(state, target)),
|
||||
CellId::Derived { target } => Some(derived_decline(target)),
|
||||
CellId::NonItem { category, item } => Some(crate::non_item_refusal(
|
||||
state.categories.category_display(category),
|
||||
item,
|
||||
@@ -1445,8 +1443,7 @@ const AGGREGATE_DECLINE: &str =
|
||||
"Cannot write to an aggregate — it combines several records, so there is no one cell to write";
|
||||
|
||||
/// What both ends say of a formula's result: the host's `derived_refusal`.
|
||||
fn derived_decline(state: &ClientState, target: &str) -> String {
|
||||
let target = state.categories.item_display("_Measure", target);
|
||||
fn derived_decline(target: &str) -> String {
|
||||
format!("{target} is computed by a formula — edit the formula, not the cell")
|
||||
}
|
||||
|
||||
@@ -5044,7 +5041,6 @@ mod display_tests {
|
||||
|
||||
const REGION: &str = "Sales territory of the whole company";
|
||||
const NORTH: &str = "Northern reaches and the colder coasts";
|
||||
const TOTAL: &str = "Grand total of everything counted";
|
||||
|
||||
fn ev(code: KeyCode) -> KeyEvent {
|
||||
KeyEvent::new(code, KeyModifiers::NONE)
|
||||
@@ -5065,20 +5061,12 @@ mod display_tests {
|
||||
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,
|
||||
},
|
||||
CategoryRow {
|
||||
name: "_Measure".into(),
|
||||
display: "_Measure".into(),
|
||||
items: vec![shown("Total", TOTAL)],
|
||||
axis: Axis::Column,
|
||||
},
|
||||
])),
|
||||
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(),
|
||||
@@ -5172,12 +5160,7 @@ mod display_tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_refused_cell_names_the_displays_it_shows() {
|
||||
let derived = displayed(Some(CellId::Derived {
|
||||
target: "Total".into(),
|
||||
}));
|
||||
let said = last_status(local(&derived, KeyCode::Char('i')));
|
||||
assert!(said.starts_with(TOTAL), "{said}");
|
||||
fn a_refused_cell_names_the_category_it_shows() {
|
||||
let stray = displayed(Some(CellId::NonItem {
|
||||
category: "Region".into(),
|
||||
item: "Mars".into(),
|
||||
|
||||
Reference in New Issue
Block a user