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

refactor(cmd): delegate cell information to GridLayout in CmdContext

Browse Source 
Refactor CmdContext to use GridLayout for cell-related information instead of
storing redundant fields. This simplifies the context structure and ensures
commands always use the most up-to-date layout information.

- Add layout field to CmdContext
- Remove redundant row_count, col_count, cell_key, and none_cats fields
- Implement helper methods on CmdContext to delegate to layout
- Update all command implementations and tests to use the new methods

Co-Authored-By: fiddlerwoaroof/git-smart-commit (unsloth/gemma-4-26B-A4B-it-GGUF:UD-Q5_K_XL)
This commit is contained in:
Edward Langley
2026-04-06 23:18:39 -07:00
parent b450d1add6
commit 58372a8d8a
1 changed files with 104 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files

177
src/command/cmd.rs
View File

@ -12,6 +12,7 @@ use crate::view::{Axis, AxisEntry, GridLayout};
/// Read-only context available to commands for decision-making.
pub struct CmdContext<'a> {
pub model: &'a Model,
pub layout: &'a GridLayout,
pub mode: &'a AppMode,
pub selected: (usize, usize),
pub row_offset: usize,
@ -31,20 +32,12 @@ pub struct CmdContext<'a> {
pub tile_cat_idx: usize,
/// Named text buffers
pub buffers: &'a HashMap<String, String>,
/// Pre-resolved cell key at the cursor position (None if out of bounds)
pub cell_key: Option<crate::model::cell::CellKey>,
/// Grid dimensions (so commands don't need GridLayout)
pub row_count: usize,
pub col_count: usize,
/// Categories on Axis::None — aggregated away in the current view
pub none_cats: Vec<String>,
/// View navigation stacks (for drill back/forward)
pub view_back_stack: Vec<String>,
pub view_forward_stack: Vec<String>,
pub view_back_stack: &'a [String],
pub view_forward_stack: &'a [String],
/// Display value at the cursor — works uniformly for pivot and records mode.
pub display_value: String,
/// How many data rows/cols fit on screen (for viewport scrolling).
/// Defaults to generous fallbacks when unknown.
pub visible_rows: usize,
pub visible_cols: usize,
/// Expanded categories in the tree panel
@ -53,6 +46,21 @@ pub struct CmdContext<'a> {
pub key_code: KeyCode,
}
impl<'a> CmdContext<'a> {
pub fn cell_key(&self) -> Option<CellKey> {
self.layout.cell_key(self.selected.0, self.selected.1)
}
pub fn row_count(&self) -> usize {
self.layout.row_count()
}
pub fn col_count(&self) -> usize {
self.layout.col_count()
}
pub fn none_cats(&self) -> &[String] {
&self.layout.none_cats
}
}
impl<'a> CmdContext<'a> {
/// Resolve the category panel tree entry at the current cursor.
pub fn cat_tree_entry(&self) -> Option<crate::ui::cat_tree::CatTreeEntry> {
@ -247,8 +255,8 @@ impl CursorState {
Self {
row: ctx.selected.0,
col: ctx.selected.1,
row_count: ctx.row_count,
col_count: ctx.col_count,
row_count: ctx.row_count(),
col_count: ctx.col_count(),
row_offset: ctx.row_offset,
col_offset: ctx.col_offset,
visible_rows: ctx.visible_rows,
@ -443,7 +451,7 @@ impl Cmd for SaveAndQuit {
// ── Cell operations ──────────────────────────────────────────────────────────
// All cell commands take an explicit CellKey. The interactive spec fills it
// from ctx.cell_key; the parser fills it from Cat/Item coordinate args.
// from ctx.cell_key(); the parser fills it from Cat/Item coordinate args.
/// Clear a cell.
#[derive(Debug)]
@ -472,8 +480,7 @@ impl Cmd for YankCell {
"yank"
}
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
let layout = GridLayout::new(ctx.model, ctx.model.active_view());
let value = ctx.model.evaluate_aggregated(&self.key, &layout.none_cats);
let value = ctx.model.evaluate_aggregated(&self.key, ctx.none_cats());
vec![
Box::new(effect::SetYanked(value)),
effect::set_status("Yanked"),
@ -649,22 +656,22 @@ impl Cmd for EditOrDrill {
// Only consider regular (non-virtual, non-label) categories on None
// as true aggregation. Virtuals like _Index/_Dim are always None in
// pivot mode and don't imply aggregation.
let regular_none = ctx.none_cats.iter().any(|c| {
let regular_none = ctx.none_cats().iter().any(|c| {
ctx.model
.category(c)
.map(|cat| cat.kind.is_regular())
.unwrap_or(false)
});
// In records mode (synthetic key), always edit directly — no drilling.
let is_synthetic = ctx.cell_key.as_ref()
let is_synthetic = ctx
.cell_key()
.as_ref()
.and_then(|k| crate::view::synthetic_record_info(k))
.is_some();
let is_aggregated = !is_synthetic && regular_none;
if is_aggregated {
let Some(key) = ctx.cell_key.clone() else {
return vec![effect::set_status(
"cannot drill — no cell at cursor",
)];
let Some(key) = ctx.cell_key().clone() else {
return vec![effect::set_status("cannot drill — no cell at cursor")];
};
return DrillIntoCell { key }.execute(ctx);
}
@ -684,11 +691,15 @@ impl Cmd for AddRecordRow {
"add-record-row"
}
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
let is_records = ctx.cell_key.as_ref()
let is_records = ctx
.cell_key()
.as_ref()
.and_then(|k| crate::view::synthetic_record_info(k))
.is_some();
if !is_records {
return vec![effect::set_status("add-record-row only works in records mode")];
return vec![effect::set_status(
"add-record-row only works in records mode",
)];
}
// Build a CellKey from the current page filters
let view = ctx.model.active_view();
@ -725,11 +736,15 @@ impl Cmd for OpenRecordRow {
"open-record-row"
}
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
let is_records = ctx.cell_key.as_ref()
let is_records = ctx
.cell_key()
.as_ref()
.and_then(|k| crate::view::synthetic_record_info(k))
.is_some();
if !is_records {
return vec![effect::set_status("open-record-row only works in records mode")];
return vec![effect::set_status(
"open-record-row only works in records mode",
)];
}
let mut effects = AddRecordRow.execute(ctx);
effects.push(Box::new(effect::EnterEditAtCursor));
@ -790,10 +805,9 @@ impl Cmd for SearchNavigate {
return vec![];
}
let layout = GridLayout::new(ctx.model, ctx.model.active_view());
let (cur_row, cur_col) = ctx.selected;
let total_rows = layout.row_count().max(1);
let total_cols = layout.col_count().max(1);
let total_rows = ctx.row_count().max(1);
let total_cols = ctx.col_count().max(1);
let total = total_rows * total_cols;
let cur_flat = cur_row * total_cols + cur_col;
@ -801,11 +815,11 @@ impl Cmd for SearchNavigate {
.filter(|&flat| {
let ri = flat / total_cols;
let ci = flat % total_cols;
let key = match layout.cell_key(ri, ci) {
let key = match ctx.layout.cell_key(ri, ci) {
Some(k) => k,
None => return false,
};
let s = match ctx.model.evaluate_aggregated(&key, &layout.none_cats) {
let s = match ctx.model.evaluate_aggregated(&key, ctx.none_cats()) {
Some(CellValue::Number(n)) => format!("{n}"),
Some(CellValue::Text(t)) => t,
None => String::new(),
@ -1004,9 +1018,8 @@ impl Cmd for ToggleGroupUnderCursor {
"toggle-group-under-cursor"
}
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
let layout = GridLayout::new(ctx.model, ctx.model.active_view());
let sel_row = ctx.selected.0;
let Some((cat, group)) = layout.row_group_for(sel_row) else {
let Some((cat, group)) = ctx.layout.row_group_for(sel_row) else {
return vec![];
};
vec![
@ -1027,9 +1040,8 @@ impl Cmd for ToggleColGroupUnderCursor {
"toggle-col-group-under-cursor"
}
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
let layout = GridLayout::new(ctx.model, ctx.model.active_view());
let sel_col = ctx.selected.1;
let Some((cat, group)) = layout.col_group_for(sel_col) else {
let Some((cat, group)) = ctx.layout.col_group_for(sel_col) else {
return vec![];
};
// After toggling, col_count may shrink — clamp selection
@ -1053,12 +1065,12 @@ impl Cmd for HideSelectedRowItem {
"hide-selected-row-item"
}
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
let layout = GridLayout::new(ctx.model, ctx.model.active_view());
let Some(cat_name) = layout.row_cats.first().cloned() else {
let Some(cat_name) = ctx.layout.row_cats.first().cloned() else {
return vec![];
};
let sel_row = ctx.selected.0;
let Some(items) = layout
let Some(items) = ctx
.layout
.row_items
.iter()
.filter_map(|e| {
@ -1185,7 +1197,7 @@ impl Cmd for DrillIntoCell {
// Previously-aggregated categories (none_cats) stay on Axis::None so
// they don't filter records; they'll appear as columns in records mode.
// Skip virtual categories — we already set _Index/_Dim above.
for cat in &ctx.none_cats {
for cat in ctx.none_cats() {
if fixed_cats.contains(cat) || cat.starts_with('_') {
continue;
}
@ -2444,7 +2456,7 @@ pub fn default_registry() -> CmdRegistry {
}))
},
|_args, ctx| {
let key = ctx.cell_key.clone().ok_or("no cell at cursor")?;
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
Ok(Box::new(ClearCellCommand { key }))
},
);
@ -2592,12 +2604,14 @@ pub fn default_registry() -> CmdRegistry {
}))
},
|_args, ctx| {
let key = ctx.cell_key.clone().ok_or("no cell at cursor")?;
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
Ok(Box::new(YankCell { key }))
},
);
r.register(
&PasteCell { key: CellKey::new(vec![]) },
&PasteCell {
key: CellKey::new(vec![]),
},
|args| {
if args.is_empty() {
return Err("paste requires at least one Cat/Item coordinate".into());
@ -2607,11 +2621,11 @@ pub fn default_registry() -> CmdRegistry {
}))
},
|_args, ctx| {
let key = ctx.cell_key.clone().ok_or("no cell at cursor")?;
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
Ok(Box::new(PasteCell { key }))
},
);
// clear-cell is registered above (unified: ctx.cell_key or explicit coords)
// clear-cell is registered above (unified: ctx.cell_key() or explicit coords)
// ── View / page ──────────────────────────────────────────────────────
r.register_nullary(|| Box::new(TransposeAxes));
@ -2658,7 +2672,7 @@ pub fn default_registry() -> CmdRegistry {
}))
},
|_args, ctx| {
let key = ctx.cell_key.clone().ok_or("no cell at cursor")?;
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
Ok(Box::new(DrillIntoCell { key }))
},
);
@ -2885,7 +2899,7 @@ pub fn default_registry() -> CmdRegistry {
},
|_args, ctx| {
let value = read_buffer(ctx, "edit");
let key = ctx.cell_key.clone().ok_or("no cell at cursor")?;
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
Ok(Box::new(CommitCellEdit { key, value }))
},
);
@ -2898,7 +2912,7 @@ pub fn default_registry() -> CmdRegistry {
|_| Err("commit-and-advance-right requires context".into()),
|_args, ctx| {
let value = read_buffer(ctx, "edit");
let key = ctx.cell_key.clone().ok_or("no cell at cursor")?;
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
Ok(Box::new(CommitAndAdvanceRight {
key,
value,
@ -2948,12 +2962,16 @@ mod tests {
static EMPTY_EXPANDED: std::sync::LazyLock<std::collections::HashSet<String>> =
std::sync::LazyLock::new(std::collections::HashSet::new);
fn make_ctx(model: &Model) -> CmdContext<'_> {
fn make_layout(model: &Model) -> GridLayout {
GridLayout::new(model, model.active_view())
}
fn make_ctx<'a>(model: &'a Model, layout: &'a GridLayout) -> CmdContext<'a> {
let view = model.active_view();
let layout = GridLayout::new(model, view);
let (sr, sc) = view.selected;
CmdContext {
model,
layout,
mode: &AppMode::Normal,
selected: view.selected,
row_offset: view.row_offset,
@ -2970,9 +2988,8 @@ mod tests {
view_panel_cursor: 0,
tile_cat_idx: 0,
buffers: &EMPTY_BUFFERS,
none_cats: layout.none_cats.clone(),
view_back_stack: Vec::new(),
view_forward_stack: Vec::new(),
view_back_stack: &[],
view_forward_stack: &[],
display_value: {
let key = layout.cell_key(sr, sc);
key.as_ref()
@ -2980,9 +2997,6 @@ mod tests {
.map(|v| v.to_string())
.unwrap_or_default()
},
cell_key: layout.cell_key(sr, sc),
row_count: layout.row_count(),
col_count: layout.col_count(),
visible_rows: 20,
visible_cols: 8,
expanded_cats: &EMPTY_EXPANDED,
@ -3004,7 +3018,8 @@ mod tests {
#[test]
fn move_selection_down_produces_set_selected() {
let m = two_cat_model();
let ctx = make_ctx(&m);
let layout = make_layout(&m);
let ctx = make_ctx(&m, &layout);
let cmd = MoveSelection {
dr: 1,
dc: 0,
@ -3018,7 +3033,8 @@ mod tests {
#[test]
fn move_selection_clamps_to_bounds() {
let m = two_cat_model();
let ctx = make_ctx(&m);
let layout = make_layout(&m);
let ctx = make_ctx(&m, &layout);
// Try to move way past the end
let cmd = MoveSelection {
dr: 100,
@ -3034,7 +3050,8 @@ mod tests {
let m = two_cat_model();
let mut bufs = HashMap::new();
bufs.insert("command".to_string(), "q".to_string());
let mut ctx = make_ctx(&m);
let layout = make_layout(&m);
let mut ctx = make_ctx(&m, &layout);
ctx.dirty = true;
ctx.buffers = &bufs;
let cmd = ExecuteCommand;
@ -3048,7 +3065,8 @@ mod tests {
let m = two_cat_model();
let mut bufs = HashMap::new();
bufs.insert("command".to_string(), "q".to_string());
let mut ctx = make_ctx(&m);
let layout = make_layout(&m);
let mut ctx = make_ctx(&m, &layout);
ctx.buffers = &bufs;
let cmd = ExecuteCommand;
let effects = cmd.execute(&ctx);
@ -3067,9 +3085,10 @@ mod tests {
("Month".to_string(), "Jan".to_string()),
]);
m.set_cell(key, CellValue::Number(42.0));
let ctx = make_ctx(&m);
let layout = make_layout(&m);
let ctx = make_ctx(&m, &layout);
let cmd = ClearCellCommand {
key: ctx.cell_key.clone().unwrap(),
key: ctx.cell_key().clone().unwrap(),
};
let effects = cmd.execute(&ctx);
assert_eq!(effects.len(), 2); // ClearCell + MarkDirty
@ -3083,9 +3102,10 @@ mod tests {
("Month".to_string(), "Jan".to_string()),
]);
m.set_cell(key, CellValue::Number(99.0));
let ctx = make_ctx(&m);
let layout = make_layout(&m);
let ctx = make_ctx(&m, &layout);
let cmd = YankCell {
key: ctx.cell_key.clone().unwrap(),
key: ctx.cell_key().clone().unwrap(),
};
let effects = cmd.execute(&ctx);
assert_eq!(effects.len(), 2); // SetYanked + SetStatus
@ -3094,7 +3114,8 @@ mod tests {
#[test]
fn toggle_panel_open_and_focus() {
let m = two_cat_model();
let ctx = make_ctx(&m);
let layout = make_layout(&m);
let ctx = make_ctx(&m, &layout);
let cmd = TogglePanelAndFocus {
panel: effect::Panel::Formula,
open: true,
@ -3112,7 +3133,8 @@ mod tests {
#[test]
fn toggle_panel_close_and_unfocus() {
let m = two_cat_model();
let ctx = make_ctx(&m);
let layout = make_layout(&m);
let ctx = make_ctx(&m, &layout);
let cmd = TogglePanelAndFocus {
panel: effect::Panel::Formula,
open: false,
@ -3125,7 +3147,8 @@ mod tests {
#[test]
fn enter_advance_moves_down() {
let m = two_cat_model();
let ctx = make_ctx(&m);
let layout = make_layout(&m);
let ctx = make_ctx(&m, &layout);
let cmd = EnterAdvance {
cursor: CursorState::from_ctx(&ctx),
};
@ -3141,7 +3164,8 @@ mod tests {
#[test]
fn search_navigate_with_empty_query_returns_nothing() {
let m = two_cat_model();
let ctx = make_ctx(&m);
let layout = make_layout(&m);
let ctx = make_ctx(&m, &layout);
let cmd = SearchNavigate(true);
let effects = cmd.execute(&ctx);
assert!(effects.is_empty());
@ -3150,7 +3174,8 @@ mod tests {
#[test]
fn enter_edit_mode_produces_editing_mode() {
let m = two_cat_model();
let ctx = make_ctx(&m);
let layout = make_layout(&m);
let ctx = make_ctx(&m, &layout);
let cmd = EnterEditMode {
initial_value: String::new(),
};
@ -3163,7 +3188,8 @@ mod tests {
#[test]
fn enter_tile_select_with_categories() {
let m = two_cat_model();
let ctx = make_ctx(&m);
let layout = make_layout(&m);
let ctx = make_ctx(&m, &layout);
let cmd = EnterTileSelect;
let effects = cmd.execute(&ctx);
assert_eq!(effects.len(), 2); // SetTileCatIdx + ChangeMode
@ -3179,7 +3205,8 @@ mod tests {
// Models always have virtual categories (_Index, _Dim), so tile
// select always has something to operate on.
let m = Model::new("Empty");
let ctx = make_ctx(&m);
let layout = make_layout(&m);
let ctx = make_ctx(&m, &layout);
let cmd = EnterTileSelect;
let effects = cmd.execute(&ctx);
assert_eq!(effects.len(), 2); // SetTileCatIdx + ChangeMode
@ -3188,7 +3215,8 @@ mod tests {
#[test]
fn toggle_group_under_cursor_returns_empty_without_groups() {
let m = two_cat_model();
let ctx = make_ctx(&m);
let layout = make_layout(&m);
let ctx = make_ctx(&m, &layout);
let cmd = ToggleGroupUnderCursor;
let effects = cmd.execute(&ctx);
// No groups defined, so nothing to toggle
@ -3198,7 +3226,8 @@ mod tests {
#[test]
fn search_or_category_add_without_query_opens_category_add() {
let m = two_cat_model();
let ctx = make_ctx(&m);
let layout = make_layout(&m);
let ctx = make_ctx(&m, &layout);
let cmd = SearchOrCategoryAdd;
let effects = cmd.execute(&ctx);
assert_eq!(effects.len(), 2); // SetPanelOpen + ChangeMode
@ -3212,7 +3241,8 @@ mod tests {
#[test]
fn cycle_panel_focus_with_no_panels_open() {
let m = two_cat_model();
let ctx = make_ctx(&m);
let layout = make_layout(&m);
let ctx = make_ctx(&m, &layout);
let cmd = CyclePanelFocus {
formula_open: false,
category_open: false,
@ -3225,7 +3255,8 @@ mod tests {
#[test]
fn cycle_panel_focus_with_formula_panel_open() {
let m = two_cat_model();
let mut ctx = make_ctx(&m);
let layout = make_layout(&m);
let mut ctx = make_ctx(&m, &layout);
ctx.formula_panel_open = true;
let cmd = CyclePanelFocus {
formula_open: true,