refactor!: unify records and pivot mode cell handling
Refactor records mode to use synthetic CellKeys (_Index, _Dim) for all columns, allowing uniform handling of display values and edits across both pivot and records modes. - Introduce `synthetic_record_info` to extract metadata from synthetic keys. - Update `GridLayout::cell_key` to return synthetic keys in records mode. - Add `GridLayout::resolve_display` to handle value resolution for synthetic keys. - Replace `records_col` and `records_value` in `CmdContext` with a unified `display_value`. - Update `EditOrDrill` and `AddRecordRow` to use synthetic key detection. - Refactor `CommitCellEdit` to use a shared `commit_cell_value` helper. BREAKING CHANGE: CmdContext fields `records_col` and `records_value` are replaced by `display_value` . Co-Authored-By: fiddlerwoaroof/git-smart-commit (unsloth/gemma-4-31B-it-GGUF:UD-Q5_K_XL)
This commit is contained in:
@ -41,12 +41,8 @@ pub struct CmdContext<'a> {
|
|||||||
/// View navigation stacks (for drill back/forward)
|
/// View navigation stacks (for drill back/forward)
|
||||||
pub view_back_stack: Vec<String>,
|
pub view_back_stack: Vec<String>,
|
||||||
pub view_forward_stack: Vec<String>,
|
pub view_forward_stack: Vec<String>,
|
||||||
/// Records-mode info (drill view). None for normal pivot views.
|
/// Display value at the cursor — works uniformly for pivot and records mode.
|
||||||
/// When Some, edits stage to drill_state.pending_edits.
|
pub display_value: String,
|
||||||
pub records_col: Option<String>,
|
|
||||||
/// The display value at the cursor in records mode (including any
|
|
||||||
/// pending edit override). None for normal pivot views.
|
|
||||||
pub records_value: Option<String>,
|
|
||||||
/// How many data rows/cols fit on screen (for viewport scrolling).
|
/// How many data rows/cols fit on screen (for viewport scrolling).
|
||||||
/// Defaults to generous fallbacks when unknown.
|
/// Defaults to generous fallbacks when unknown.
|
||||||
pub visible_rows: usize,
|
pub visible_rows: usize,
|
||||||
@ -688,7 +684,11 @@ impl Cmd for EditOrDrill {
|
|||||||
.map(|cat| cat.kind.is_regular())
|
.map(|cat| cat.kind.is_regular())
|
||||||
.unwrap_or(false)
|
.unwrap_or(false)
|
||||||
});
|
});
|
||||||
let is_aggregated = ctx.records_col.is_none() && regular_none;
|
// In records mode (synthetic key), always edit directly — no drilling.
|
||||||
|
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 {
|
if is_aggregated {
|
||||||
let Some(key) = ctx.cell_key.clone() else {
|
let Some(key) = ctx.cell_key.clone() else {
|
||||||
return vec![effect::set_status(
|
return vec![effect::set_status(
|
||||||
@ -697,18 +697,10 @@ impl Cmd for EditOrDrill {
|
|||||||
};
|
};
|
||||||
return DrillIntoCell { key }.execute(ctx);
|
return DrillIntoCell { key }.execute(ctx);
|
||||||
}
|
}
|
||||||
// Edit path: prefer records display value (includes pending edits),
|
EnterEditMode {
|
||||||
// else the underlying cell's stored value.
|
initial_value: ctx.display_value.clone(),
|
||||||
let initial_value = if let Some(v) = &ctx.records_value {
|
}
|
||||||
v.clone()
|
.execute(ctx)
|
||||||
} else {
|
|
||||||
ctx.cell_key
|
|
||||||
.as_ref()
|
|
||||||
.and_then(|k| ctx.model.get_cell(k).cloned())
|
|
||||||
.map(|v| v.to_string())
|
|
||||||
.unwrap_or_default()
|
|
||||||
};
|
|
||||||
EnterEditMode { initial_value }.execute(ctx)
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -721,7 +713,10 @@ impl Cmd for AddRecordRow {
|
|||||||
"add-record-row"
|
"add-record-row"
|
||||||
}
|
}
|
||||||
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||||
if ctx.records_col.is_none() {
|
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
|
// Build a CellKey from the current page filters
|
||||||
@ -1928,14 +1923,34 @@ impl Cmd for PopChar {
|
|||||||
|
|
||||||
/// Commit a cell edit: set cell value, advance cursor, return to Normal.
|
/// Commit a cell edit: set cell value, advance cursor, return to Normal.
|
||||||
/// In records mode, stages the edit in drill_state.pending_edits instead of
|
/// In records mode, stages the edit in drill_state.pending_edits instead of
|
||||||
/// writing directly to the model.
|
/// Commit a cell value: for synthetic records keys, stage in drill pending edits
|
||||||
|
/// or apply directly; for real keys, write to the model.
|
||||||
|
fn commit_cell_value(key: &CellKey, value: &str, effects: &mut Vec<Box<dyn Effect>>) {
|
||||||
|
if let Some((record_idx, col_name)) = crate::view::synthetic_record_info(key) {
|
||||||
|
effects.push(Box::new(effect::SetDrillPendingEdit {
|
||||||
|
record_idx,
|
||||||
|
col_name,
|
||||||
|
new_value: value.to_string(),
|
||||||
|
}));
|
||||||
|
} else if value.is_empty() {
|
||||||
|
effects.push(Box::new(effect::ClearCell(key.clone())));
|
||||||
|
effects.push(effect::mark_dirty());
|
||||||
|
} else if let Ok(n) = value.parse::<f64>() {
|
||||||
|
effects.push(Box::new(effect::SetCell(key.clone(), CellValue::Number(n))));
|
||||||
|
effects.push(effect::mark_dirty());
|
||||||
|
} else {
|
||||||
|
effects.push(Box::new(effect::SetCell(key.clone(), CellValue::Text(value.to_string()))));
|
||||||
|
effects.push(effect::mark_dirty());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Commit a cell edit: set cell value, advance cursor, return to editing.
|
||||||
|
/// In records mode with drill, stages the edit in drill_state.pending_edits.
|
||||||
|
/// In records mode without drill or in pivot mode, writes directly to the model.
|
||||||
#[derive(Debug)]
|
#[derive(Debug)]
|
||||||
pub struct CommitCellEdit {
|
pub struct CommitCellEdit {
|
||||||
pub key: crate::model::cell::CellKey,
|
pub key: CellKey,
|
||||||
pub value: String,
|
pub value: String,
|
||||||
/// Records-mode edit: (record_idx, column_name). When Some, stage in
|
|
||||||
/// pending_edits; otherwise write to the model directly.
|
|
||||||
pub records_edit: Option<(usize, String)>,
|
|
||||||
}
|
}
|
||||||
impl Cmd for CommitCellEdit {
|
impl Cmd for CommitCellEdit {
|
||||||
fn name(&self) -> &'static str {
|
fn name(&self) -> &'static str {
|
||||||
@ -1944,29 +1959,7 @@ impl Cmd for CommitCellEdit {
|
|||||||
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||||
let mut effects: Vec<Box<dyn Effect>> = Vec::new();
|
let mut effects: Vec<Box<dyn Effect>> = Vec::new();
|
||||||
|
|
||||||
if let Some((record_idx, col_name)) = &self.records_edit {
|
commit_cell_value(&self.key, &self.value, &mut effects);
|
||||||
// Stage the edit in drill_state.pending_edits
|
|
||||||
effects.push(Box::new(effect::SetDrillPendingEdit {
|
|
||||||
record_idx: *record_idx,
|
|
||||||
col_name: col_name.clone(),
|
|
||||||
new_value: self.value.clone(),
|
|
||||||
}));
|
|
||||||
} else if self.value.is_empty() {
|
|
||||||
effects.push(Box::new(effect::ClearCell(self.key.clone())));
|
|
||||||
effects.push(effect::mark_dirty());
|
|
||||||
} else if let Ok(n) = self.value.parse::<f64>() {
|
|
||||||
effects.push(Box::new(effect::SetCell(
|
|
||||||
self.key.clone(),
|
|
||||||
CellValue::Number(n),
|
|
||||||
)));
|
|
||||||
effects.push(effect::mark_dirty());
|
|
||||||
} else {
|
|
||||||
effects.push(Box::new(effect::SetCell(
|
|
||||||
self.key.clone(),
|
|
||||||
CellValue::Text(self.value.clone()),
|
|
||||||
)));
|
|
||||||
effects.push(effect::mark_dirty());
|
|
||||||
}
|
|
||||||
// Advance cursor down (typewriter-style) and re-enter edit mode
|
// Advance cursor down (typewriter-style) and re-enter edit mode
|
||||||
// at the new cell so the user can continue data entry.
|
// at the new cell so the user can continue data entry.
|
||||||
let adv = EnterAdvance {
|
let adv = EnterAdvance {
|
||||||
|
|||||||
@ -164,28 +164,34 @@ impl App {
|
|||||||
none_cats: layout.none_cats.clone(),
|
none_cats: layout.none_cats.clone(),
|
||||||
view_back_stack: self.view_back_stack.clone(),
|
view_back_stack: self.view_back_stack.clone(),
|
||||||
view_forward_stack: self.view_forward_stack.clone(),
|
view_forward_stack: self.view_forward_stack.clone(),
|
||||||
records_col: if layout.is_records_mode() {
|
display_value: {
|
||||||
Some(layout.col_label(sel_col))
|
let key = layout.cell_key(sel_row, sel_col);
|
||||||
|
if let Some(k) = &key {
|
||||||
|
if let Some((idx, dim)) = crate::view::synthetic_record_info(k) {
|
||||||
|
// Synthetic records key: check pending drill edits first
|
||||||
|
self.drill_state.as_ref()
|
||||||
|
.and_then(|s| s.pending_edits.get(&(idx, dim)).cloned())
|
||||||
|
.or_else(|| layout.resolve_display(k))
|
||||||
|
.unwrap_or_default()
|
||||||
} else {
|
} else {
|
||||||
None
|
self.model.get_cell(k)
|
||||||
},
|
.map(|v| v.to_string())
|
||||||
records_value: if layout.is_records_mode() {
|
.unwrap_or_default()
|
||||||
// Check pending edits first, then fall back to original
|
}
|
||||||
let col_name = layout.col_label(sel_col);
|
|
||||||
let pending = self.drill_state.as_ref().and_then(|s| {
|
|
||||||
s.pending_edits.get(&(sel_row, col_name.clone())).cloned()
|
|
||||||
});
|
|
||||||
pending.or_else(|| layout.records_display(sel_row, sel_col))
|
|
||||||
} else {
|
} else {
|
||||||
None
|
String::new()
|
||||||
|
}
|
||||||
},
|
},
|
||||||
// Approximate visible rows/cols from terminal size.
|
// Approximate visible rows from terminal size.
|
||||||
// Chrome: title(1) + border(2) + col_headers(n_col_levels) + separator(1)
|
// Chrome: title(1) + border(2) + col_headers(n_col_levels) + separator(1)
|
||||||
// + tile_bar(1) + status_bar(1) = ~8 rows of chrome.
|
// + tile_bar(1) + status_bar(1) = ~8 rows of chrome.
|
||||||
visible_rows: (self.term_height as usize).saturating_sub(8),
|
visible_rows: (self.term_height as usize).saturating_sub(8),
|
||||||
// Visible cols depends on column widths — use a rough estimate.
|
visible_cols: {
|
||||||
// The grid renderer does the precise calculation.
|
let (fmt_comma, fmt_decimals) = parse_number_format(&view.number_format);
|
||||||
visible_cols: ((self.term_width as usize).saturating_sub(30) / 12).max(1),
|
let col_widths = compute_col_widths(&self.model, &layout, fmt_comma, fmt_decimals);
|
||||||
|
let row_header_width = compute_row_header_width(&layout);
|
||||||
|
compute_visible_cols(&col_widths, row_header_width, self.term_width, view.col_offset)
|
||||||
|
},
|
||||||
expanded_cats: &self.expanded_cats,
|
expanded_cats: &self.expanded_cats,
|
||||||
key_code: key,
|
key_code: key,
|
||||||
}
|
}
|
||||||
|
|||||||
@ -97,15 +97,7 @@ pub struct EnterEditAtCursor;
|
|||||||
impl Effect for EnterEditAtCursor {
|
impl Effect for EnterEditAtCursor {
|
||||||
fn apply(&self, app: &mut App) {
|
fn apply(&self, app: &mut App) {
|
||||||
let ctx = app.cmd_context(crossterm::event::KeyCode::Null, crossterm::event::KeyModifiers::NONE);
|
let ctx = app.cmd_context(crossterm::event::KeyCode::Null, crossterm::event::KeyModifiers::NONE);
|
||||||
let value = if let Some(v) = &ctx.records_value {
|
let value = ctx.display_value.clone();
|
||||||
v.clone()
|
|
||||||
} else {
|
|
||||||
ctx.cell_key
|
|
||||||
.as_ref()
|
|
||||||
.and_then(|k| ctx.model.get_cell(k).cloned())
|
|
||||||
.map(|v| v.to_string())
|
|
||||||
.unwrap_or_default()
|
|
||||||
};
|
|
||||||
drop(ctx);
|
drop(ctx);
|
||||||
app.buffers.insert("edit".to_string(), value);
|
app.buffers.insert("edit".to_string(), value);
|
||||||
app.mode = AppMode::Editing {
|
app.mode = AppMode::Editing {
|
||||||
|
|||||||
@ -2,6 +2,14 @@ use crate::model::cell::{CellKey, CellValue};
|
|||||||
use crate::model::Model;
|
use crate::model::Model;
|
||||||
use crate::view::{Axis, View};
|
use crate::view::{Axis, View};
|
||||||
|
|
||||||
|
/// Extract (record_index, dim_name) from a synthetic records-mode CellKey.
|
||||||
|
/// Returns None for normal pivot-mode keys.
|
||||||
|
pub fn synthetic_record_info(key: &CellKey) -> Option<(usize, String)> {
|
||||||
|
let idx: usize = key.get("_Index")?.parse().ok()?;
|
||||||
|
let dim = key.get("_Dim")?.to_string();
|
||||||
|
Some((idx, dim))
|
||||||
|
}
|
||||||
|
|
||||||
/// One entry on a grid axis: either a visual group header or a data-item tuple.
|
/// One entry on a grid axis: either a visual group header or a data-item tuple.
|
||||||
///
|
///
|
||||||
/// `GroupHeader` entries are always visible so the user can see the group label
|
/// `GroupHeader` entries are always visible so the user can see the group label
|
||||||
@ -352,18 +360,36 @@ impl GridLayout {
|
|||||||
.unwrap_or_default()
|
.unwrap_or_default()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Resolve the display string for a synthetic records-mode CellKey.
|
||||||
|
/// Returns None for non-synthetic (pivot) keys.
|
||||||
|
pub fn resolve_display(&self, key: &CellKey) -> Option<String> {
|
||||||
|
let (idx, dim) = synthetic_record_info(key)?;
|
||||||
|
let records = self.records.as_ref()?;
|
||||||
|
let (orig_key, value) = records.get(idx)?;
|
||||||
|
if dim == "Value" {
|
||||||
|
Some(value.to_string())
|
||||||
|
} else {
|
||||||
|
Some(orig_key.get(&dim).unwrap_or("").to_string())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Build the CellKey for the data cell at (row, col), including the active
|
/// Build the CellKey for the data cell at (row, col), including the active
|
||||||
/// page-axis filter. Returns None if row or col is out of bounds.
|
/// page-axis filter. Returns None if row or col is out of bounds.
|
||||||
/// In records mode: returns the real underlying CellKey when the column
|
/// In records mode: returns a synthetic `(_Index, _Dim)` key for every column.
|
||||||
/// is "Value" (editable); returns None for coord columns (read-only).
|
|
||||||
pub fn cell_key(&self, row: usize, col: usize) -> Option<CellKey> {
|
pub fn cell_key(&self, row: usize, col: usize) -> Option<CellKey> {
|
||||||
if let Some(records) = &self.records {
|
if self.records.is_some() {
|
||||||
// Records mode: only the Value column maps to a real, editable cell.
|
let records = self.records.as_ref().unwrap();
|
||||||
if self.col_label(col) == "Value" {
|
if row >= records.len() {
|
||||||
return records.get(row).map(|(k, _)| k.clone());
|
|
||||||
} else {
|
|
||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
|
let col_label = self.col_label(col);
|
||||||
|
if col_label.is_empty() {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
return Some(CellKey::new(vec![
|
||||||
|
("_Index".to_string(), row.to_string()),
|
||||||
|
("_Dim".to_string(), col_label),
|
||||||
|
]));
|
||||||
}
|
}
|
||||||
let row_item = self
|
let row_item = self
|
||||||
.row_items
|
.row_items
|
||||||
@ -527,7 +553,7 @@ fn cross_product(model: &Model, view: &View, cats: &[String]) -> Vec<AxisEntry>
|
|||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::{AxisEntry, GridLayout};
|
use super::{synthetic_record_info, AxisEntry, GridLayout};
|
||||||
use crate::model::cell::{CellKey, CellValue};
|
use crate::model::cell::{CellKey, CellValue};
|
||||||
use crate::model::Model;
|
use crate::model::Model;
|
||||||
use crate::view::Axis;
|
use crate::view::Axis;
|
||||||
@ -592,40 +618,66 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn records_mode_cell_key_editable_for_value_column() {
|
fn records_mode_cell_key_returns_synthetic_for_all_columns() {
|
||||||
let mut m = records_model();
|
let mut m = records_model();
|
||||||
let v = m.active_view_mut();
|
let v = m.active_view_mut();
|
||||||
v.set_axis("_Index", Axis::Row);
|
v.set_axis("_Index", Axis::Row);
|
||||||
v.set_axis("_Dim", Axis::Column);
|
v.set_axis("_Dim", Axis::Column);
|
||||||
let layout = GridLayout::new(&m, m.active_view());
|
let layout = GridLayout::new(&m, m.active_view());
|
||||||
assert!(layout.is_records_mode());
|
assert!(layout.is_records_mode());
|
||||||
// Find the "Value" column index
|
|
||||||
let cols: Vec<String> = (0..layout.col_count()).map(|i| layout.col_label(i)).collect();
|
let cols: Vec<String> = (0..layout.col_count()).map(|i| layout.col_label(i)).collect();
|
||||||
|
// All columns return synthetic keys
|
||||||
let value_col = cols.iter().position(|c| c == "Value").unwrap();
|
let value_col = cols.iter().position(|c| c == "Value").unwrap();
|
||||||
// cell_key should be Some for Value column
|
let key = layout.cell_key(0, value_col).unwrap();
|
||||||
let key = layout.cell_key(0, value_col);
|
assert_eq!(key.get("_Index"), Some("0"));
|
||||||
assert!(key.is_some(), "Value column should be editable");
|
assert_eq!(key.get("_Dim"), Some("Value"));
|
||||||
// cell_key should be None for coord columns
|
|
||||||
let region_col = cols.iter().position(|c| c == "Region").unwrap();
|
let region_col = cols.iter().position(|c| c == "Region").unwrap();
|
||||||
assert!(
|
let key = layout.cell_key(0, region_col).unwrap();
|
||||||
layout.cell_key(0, region_col).is_none(),
|
assert_eq!(key.get("_Index"), Some("0"));
|
||||||
"Region column should not be editable"
|
assert_eq!(key.get("_Dim"), Some("Region"));
|
||||||
);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn records_mode_cell_key_maps_to_real_cell() {
|
fn records_mode_resolve_display_returns_values() {
|
||||||
let mut m = records_model();
|
let mut m = records_model();
|
||||||
let v = m.active_view_mut();
|
let v = m.active_view_mut();
|
||||||
v.set_axis("_Index", Axis::Row);
|
v.set_axis("_Index", Axis::Row);
|
||||||
v.set_axis("_Dim", Axis::Column);
|
v.set_axis("_Dim", Axis::Column);
|
||||||
let layout = GridLayout::new(&m, m.active_view());
|
let layout = GridLayout::new(&m, m.active_view());
|
||||||
let cols: Vec<String> = (0..layout.col_count()).map(|i| layout.col_label(i)).collect();
|
let cols: Vec<String> = (0..layout.col_count()).map(|i| layout.col_label(i)).collect();
|
||||||
|
|
||||||
|
// Value column resolves to the cell value
|
||||||
let value_col = cols.iter().position(|c| c == "Value").unwrap();
|
let value_col = cols.iter().position(|c| c == "Value").unwrap();
|
||||||
// The CellKey at (0, Value) should look up a real cell value
|
|
||||||
let key = layout.cell_key(0, value_col).unwrap();
|
let key = layout.cell_key(0, value_col).unwrap();
|
||||||
let val = m.evaluate(&key);
|
let display = layout.resolve_display(&key);
|
||||||
assert!(val.is_some(), "cell_key should resolve to a real cell");
|
assert!(display.is_some(), "Value column should resolve");
|
||||||
|
|
||||||
|
// Category column resolves to the coordinate value
|
||||||
|
let region_col = cols.iter().position(|c| c == "Region").unwrap();
|
||||||
|
let key = layout.cell_key(0, region_col).unwrap();
|
||||||
|
let display = layout.resolve_display(&key).unwrap();
|
||||||
|
assert!(!display.is_empty(), "Region column should resolve to a value");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn synthetic_record_info_returns_none_for_pivot_keys() {
|
||||||
|
let key = CellKey::new(vec![
|
||||||
|
("Region".to_string(), "East".to_string()),
|
||||||
|
("Product".to_string(), "Shoes".to_string()),
|
||||||
|
]);
|
||||||
|
assert!(synthetic_record_info(&key).is_none());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn synthetic_record_info_extracts_index_and_dim() {
|
||||||
|
let key = CellKey::new(vec![
|
||||||
|
("_Index".to_string(), "3".to_string()),
|
||||||
|
("_Dim".to_string(), "Region".to_string()),
|
||||||
|
]);
|
||||||
|
let (idx, dim) = synthetic_record_info(&key).unwrap();
|
||||||
|
assert_eq!(idx, 3);
|
||||||
|
assert_eq!(dim, "Region");
|
||||||
}
|
}
|
||||||
|
|
||||||
fn coord(pairs: &[(&str, &str)]) -> CellKey {
|
fn coord(pairs: &[(&str, &str)]) -> CellKey {
|
||||||
|
|||||||
@ -3,5 +3,5 @@ pub mod layout;
|
|||||||
pub mod types;
|
pub mod types;
|
||||||
|
|
||||||
pub use axis::Axis;
|
pub use axis::Axis;
|
||||||
pub use layout::{AxisEntry, GridLayout};
|
pub use layout::{synthetic_record_info, AxisEntry, GridLayout};
|
||||||
pub use types::View;
|
pub use types::View;
|
||||||
|
|||||||
Reference in New Issue
Block a user