Merge pull request #192 from fiddlerwoaroof/fix/jgzw-tile-virtuals
jgzw: keep _Index/_Dim tileable, and pin the offered set by name
This commit is contained in:
@@ -2874,6 +2874,43 @@ flag: tidy-up goes where an abort cannot reach it.
|
||||
stays in `TileSelect` mode so the user can adjust several tiles. `axis: None` cycles;
|
||||
`axis: Some(a)` sets.
|
||||
|
||||
**The tile bar offers `_Index` and `_Dim`, and that is deliberate (improvise-jgzw).**
|
||||
Every tile call site passes `Model::category_names()`, virtual categories included,
|
||||
so tile cursor 0 is `_Index`. The ticket asked whether that was a leak. It is not,
|
||||
for three reasons, each checked rather than argued:
|
||||
|
||||
- **It is a decision the user already made.** improvise-kos: "`_Dim` and `_Index`
|
||||
should work as pivot axes outside Records/Drill view". A category that works as a
|
||||
pivot axis is one the axis picker offers.
|
||||
- **The tile bar is the only way out of a records layout that was saved.**
|
||||
improvise-jgzw's "check what it does" found that hand-assigning `_Index` → Row and
|
||||
`_Dim` → Column in a real view (tile bar or `:set-axis`) is **written** to the
|
||||
`.improv`, because a real view persists every axis, and it reopens as records mode.
|
||||
(`VIRTUAL_VIEW_NAMES`' "never-persisted `_Index`/`_Dim` on axes" is true only of
|
||||
`_Records`/`_Drill`.) In that document, `R` (`toggle-records-mode`) emits
|
||||
`ViewBack` on an empty back stack, so it leaves the layout as it was and sets the
|
||||
status to "Pivot mode". That is filed as improvise-gj5p. Moving the two axes back through
|
||||
the tile bar is what gets out. In the browser, which declines
|
||||
`toggle-records-mode`, the tile bar is also the only way in, and improvise-bd9o
|
||||
already made that route safe instead of closing it
|
||||
(`the_tile_bar_route_into_records_mode_does_not_strand_the_user`).
|
||||
- **What filtering would cost was measured.** With `View::tileable_cats` filtering
|
||||
on `CategoryKind::is_index_or_dim`, that bd9o test and the TUI's
|
||||
`renders_each_tile_with_its_axis_label` fail, as well as the two characterization
|
||||
tests here.
|
||||
|
||||
What the default cursor does cost, stated: one cycle key on tile 0 puts `_Index`
|
||||
on Row alone, and in a pivot view that empties the grid (0 rows on
|
||||
`web/demo.improv`). The status line says `_Index → Row`, and three more presses
|
||||
undo it. If that turns out to matter, the fix is the order the tile bar lists
|
||||
categories in, not taking entries out of it.
|
||||
|
||||
**`_Measure` must stay tileable.** It is on an axis in every shipped document, so
|
||||
`regular_category_names()`, the advice for *prompts*, is the wrong filter here.
|
||||
`the_tile_bar_offers_every_category_by_name_measure_and_records_axes_included`
|
||||
names the whole offered list with `_Measure` on a column, and fails if either
|
||||
decision changes.
|
||||
|
||||
### `cmd/effect_cmds.rs`
|
||||
|
||||
The parseable model-mutation commands are thin `Cmd` wrappers around effects,
|
||||
|
||||
@@ -5313,3 +5313,27 @@ refusal is recorded and the mode is not `Quit`. `import-json` failed first,
|
||||
before `refuse_and_stop` was applied to its arms. For `load`, the injected fault
|
||||
is removing its abort (`refuse_and_stop` back to `app.refuse`), and the test
|
||||
then fails with "`load absent.improv . q` quit having loaded nothing".
|
||||
|
||||
## `command/cmd/tile.rs` — the tile bar's offered set, by name (improvise-jgzw)
|
||||
|
||||
### the_tile_bar_offers_every_category_by_name_measure_and_records_axes_included
|
||||
|
||||
This test pins a decision, not a bug. The argument is in `context/code/command.md`
|
||||
under `cmd/tile.rs`. Before this test, no tile assertion named the whole offered
|
||||
set. The two older ones named only the category at cursor 0, so a filter that
|
||||
changed what the bar offers past the cursor was invisible. The fixture puts
|
||||
`_Measure` on **Column**, as every shipped document does. The test then walks the
|
||||
cursor past the end and asserts the exact list
|
||||
`[_Index, _Dim, _Measure, Type, Month]`.
|
||||
|
||||
Rivals, injected, full workspace run:
|
||||
|
||||
- **`regular_category_names()` in `TileAxisOp`**, the obvious "no virtual
|
||||
categories in pickers" fix, which drops `_Measure` too. It fails this test
|
||||
(`["Type", "Month"]`), the two characterization tests above it, and
|
||||
`lockstep.rs::a_locally_resolved_tile_axis_change_agrees_with_the_host`.
|
||||
- **`View::tileable_cats` filtering `is_index_or_dim`**, the ticket's option (b).
|
||||
With the list asserted this test fails by construction, so the point is what
|
||||
*else* fails: `renders_each_tile_with_its_axis_label` (TUI) and
|
||||
`the_tile_bar_route_into_records_mode_does_not_strand_the_user`. The second one
|
||||
is improvise-bd9o's own test of the route this decision keeps.
|
||||
|
||||
@@ -59,6 +59,26 @@ mod tests {
|
||||
assert!(effects.is_empty());
|
||||
}
|
||||
|
||||
/// improvise-jgzw: `_Index`/`_Dim` stay tileable on purpose, and `_Measure` on
|
||||
/// a column must too — `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn the_tile_bar_offers_every_category_by_name_measure_and_records_axes_included() {
|
||||
let mut m = two_cat_model();
|
||||
m.active_view_mut().set_axis("_Measure", Axis::Column);
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let mut ctx = make_ctx(&m, &layout, ®);
|
||||
let mut offered = Vec::new();
|
||||
for idx in 0..8 {
|
||||
ctx.tile_cat_idx = idx;
|
||||
let dbg = effects_debug(&TileAxisOp { axis: None }.execute(&ctx));
|
||||
if let Some(rest) = dbg.split("CycleAxis(Active, \"").nth(1) {
|
||||
offered.push(rest.split('"').next().unwrap_or_default().to_string());
|
||||
}
|
||||
}
|
||||
assert_eq!(offered, ["_Index", "_Dim", "_Measure", "Type", "Month"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn move_tile_cursor_right() {
|
||||
let m = two_cat_model();
|
||||
|
||||
Reference in New Issue
Block a user