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

refactor(ui): optimize record sharing and centralize layout management

Browse Source 
Refactor `App` and `DrillState` to use `Rc` for efficient sharing of frozen
records and integrate a persistent `layout` field.

- Update `DrillState` to use `Rc<Vec<(CellKey, CellValue)>>` for records.
- Add `layout` field to `App` .
- Implement `rebuild_layout()` in `App` to refresh the grid layout.
- Ensure `layout` is rebuilt after applying effects and handling key
  events.
- Update `App::new` and `App::cmd_context` to use the new layout
  management.

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-07 09:29:45 -07:00
parent 492d309277
commit 9251e37180
1 changed files with 241 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

271
src/ui/app.rs
View File

@ -5,23 +5,25 @@ use std::path::PathBuf;
use std::sync::Arc;
use std::time::{Duration, Instant};
use std::rc::Rc;
use crate::command::cmd::CmdContext;
use crate::command::keymap::{Keymap, KeymapSet};
use crate::import::wizard::ImportWizard;
use crate::model::cell::CellValue;
use crate::model::Model;
use crate::persistence;
use crate::ui::grid::{
compute_col_widths, compute_row_header_width, compute_visible_cols, parse_number_format,
};
use crate::view::GridLayout;
/// Drill-down state: frozen record snapshot + pending edits that have not
/// yet been applied to the model.
#[derive(Debug, Clone, Default)]
pub struct DrillState {
/// Frozen snapshot of records shown in the drill view.
pub records: Vec<(
crate::model::cell::CellKey,
crate::model::cell::CellValue,
)>,
/// Frozen snapshot of records shown in the drill view (Rc for cheap cloning).
pub records: Rc<Vec<(crate::model::cell::CellKey, crate::model::cell::CellValue)>>,
/// Pending edits keyed by (record_idx, column_name) → new string value.
/// column_name is either "Value" or a category name.
pub pending_edits: std::collections::HashMap<(usize, String), String>,
@ -100,11 +102,18 @@ pub struct App {
pub buffers: HashMap<String, String>,
/// Transient keymap for Emacs-style prefix key sequences (g→gg, y→yy, etc.)
pub transient_keymap: Option<Arc<Keymap>>,
/// Current grid layout, derived from model + view + drill_state.
/// Rebuilt via `rebuild_layout()` after state changes.
pub layout: GridLayout,
keymap_set: KeymapSet,
}
impl App {
pub fn new(model: Model, file_path: Option<PathBuf>) -> Self {
let layout = {
let view = model.active_view();
GridLayout::with_frozen_records(&model, view, None)
};
Self {
model,
file_path,
@ -131,17 +140,26 @@ impl App {
expanded_cats: std::collections::HashSet::new(),
buffers: HashMap::new(),
transient_keymap: None,
layout,
keymap_set: KeymapSet::default_keymaps(),
}
}
/// Rebuild the grid layout from current model, view, and drill state.
/// Note: `with_frozen_records` already handles pruning internally.
pub fn rebuild_layout(&mut self) {
let view = self.model.active_view();
let frozen = self.drill_state.as_ref().map(|s| Rc::clone(&s.records));
self.layout = GridLayout::with_frozen_records(&self.model, view, frozen);
}
pub fn cmd_context(&self, key: KeyCode, _mods: KeyModifiers) -> CmdContext<'_> {
let view = self.model.active_view();
let frozen_records = self.drill_state.as_ref().map(|s| s.records.clone());
let layout = GridLayout::with_frozen_records(&self.model, view, frozen_records);
let layout = &self.layout;
let (sel_row, sel_col) = view.selected;
CmdContext {
model: &self.model,
layout,
mode: &self.mode,
selected: view.selected,
row_offset: view.row_offset,
@ -158,34 +176,39 @@ impl App {
cat_panel_cursor: self.cat_panel_cursor,
view_panel_cursor: self.view_panel_cursor,
tile_cat_idx: self.tile_cat_idx,
cell_key: layout.cell_key(sel_row, sel_col),
row_count: layout.row_count(),
col_count: layout.col_count(),
none_cats: layout.none_cats.clone(),
view_back_stack: self.view_back_stack.clone(),
view_forward_stack: self.view_forward_stack.clone(),
records_col: if layout.is_records_mode() {
Some(layout.col_label(sel_col))
view_back_stack: &self.view_back_stack,
view_forward_stack: &self.view_forward_stack,
display_value: {
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) {
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 {
None
},
records_value: if layout.is_records_mode() {
// 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))
self.model
.get_cell(k)
.map(|v| v.to_string())
.unwrap_or_default()
}
} else {
None
String::new()
}
},
// Approximate visible rows/cols from terminal size.
// Chrome: title(1) + border(2) + col_headers(n_col_levels) + separator(1)
// + tile_bar(1) + status_bar(1) = ~8 rows of chrome.
visible_rows: (self.term_height as usize).saturating_sub(8),
// Visible cols depends on column widths — use a rough estimate.
// The grid renderer does the precise calculation.
visible_cols: ((self.term_width as usize).saturating_sub(30) / 12).max(1),
visible_cols: {
let (fmt_comma, fmt_decimals) = parse_number_format(&view.number_format);
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,
key_code: key,
}
@ -195,6 +218,7 @@ impl App {
for effect in effects {
effect.apply(self);
}
self.rebuild_layout();
}
/// True when the model has no categories yet (show welcome screen)
@ -203,6 +227,8 @@ impl App {
}
pub fn handle_key(&mut self, key: KeyEvent) -> Result<()> {
self.rebuild_layout();
// Transient keymap (prefix key sequence) takes priority
if let Some(transient) = self.transient_keymap.take() {
let effects = {
@ -247,7 +273,7 @@ impl App {
pub fn hint_text(&self) -> &'static str {
match &self.mode {
AppMode::Normal => "hjkl:nav i:edit R:records P:prune F/C/V:panels T:tiles [:]:page >:drill ::cmd",
AppMode::Editing { .. } => "Enter:commit Esc:cancel",
AppMode::Editing { .. } => "Enter:commit Tab:commit+right Esc:cancel",
AppMode::FormulaPanel => "n:new d:delete jk:nav Esc:back",
AppMode::FormulaEdit { .. } => "Enter:save Esc:cancel — type: Name = expression",
AppMode::CategoryPanel => "jk:nav Space:cycle-axis n:new-cat a:add-items d:delete Esc:back",
@ -371,6 +397,187 @@ mod tests {
assert_eq!(app.buffers.get("command").map(|s| s.as_str()), Some("q"));
}
#[test]
fn col_offset_scrolls_when_cursor_moves_past_visible_columns() {
use crate::model::cell::{CellKey, CellValue};
// Create a model with 8 wide columns. Column item names are 30 chars
// each → column widths ~31 chars. With term_width=80, row header ~4,
// data area ~76 → only ~2 columns actually fit. But the rough estimate
// (80−30)/12 = 4 over-counts, so viewport_effects never scrolls.
let mut m = Model::new("T");
m.add_category("Row").unwrap();
m.add_category("Col").unwrap();
m.category_mut("Row").unwrap().add_item("R1");
for i in 0..8 {
let name = format!("VeryLongColumnItemName_{i:03}");
m.category_mut("Col").unwrap().add_item(&name);
}
// Populate a value so the model isn't empty
let key = CellKey::new(vec![
("Row".to_string(), "R1".to_string()),
("Col".to_string(), "VeryLongColumnItemName_000".to_string()),
]);
m.set_cell(key, CellValue::Number(1.0));
let mut app = App::new(m, None);
app.term_width = 80;
// Press 'l' (right) 3 times to move cursor to column 3.
// Only ~2 columns fit in 76 chars of data area (each col ~26 chars wide),
// so column 3 is well off-screen. The buggy estimate (80−30)/12 = 4
// thinks 4 columns fit, so it won't scroll until col 4.
for _ in 0..3 {
app.handle_key(KeyEvent::new(KeyCode::Char('l'), KeyModifiers::NONE))
.unwrap();
}
assert_eq!(
app.model.active_view().selected.1,
3,
"cursor should be at column 3"
);
assert!(
app.model.active_view().col_offset > 0,
"col_offset should scroll when cursor moves past visible area (only ~2 cols fit \
in 80-char terminal with 26-char-wide columns), but col_offset is {}",
app.model.active_view().col_offset
);
}
#[test]
fn home_jumps_to_first_col() {
let mut app = two_col_model();
app.model.active_view_mut().selected = (1, 1);
app.handle_key(KeyEvent::new(KeyCode::Home, KeyModifiers::NONE))
.unwrap();
assert_eq!(app.model.active_view().selected, (1, 0));
}
#[test]
fn end_jumps_to_last_col() {
let mut app = two_col_model();
app.model.active_view_mut().selected = (1, 0);
app.handle_key(KeyEvent::new(KeyCode::End, KeyModifiers::NONE))
.unwrap();
assert_eq!(app.model.active_view().selected, (1, 1));
}
#[test]
fn page_down_scrolls_by_three_quarters_visible() {
let mut app = two_col_model();
// Add enough rows
for i in 0..30 {
app.model
.category_mut("Row")
.unwrap()
.add_item(&format!("R{i}"));
}
app.term_height = 28; // ~20 visible rows → delta = 15
app.model.active_view_mut().selected = (0, 0);
app.handle_key(KeyEvent::new(KeyCode::PageDown, KeyModifiers::NONE))
.unwrap();
assert_eq!(app.model.active_view().selected.1, 0, "column preserved");
assert!(
app.model.active_view().selected.0 > 0,
"row should advance on PageDown"
);
// 3/4 of ~20 = 15
assert_eq!(app.model.active_view().selected.0, 15);
}
#[test]
fn page_up_scrolls_backward() {
let mut app = two_col_model();
for i in 0..30 {
app.model
.category_mut("Row")
.unwrap()
.add_item(&format!("R{i}"));
}
app.term_height = 28;
app.model.active_view_mut().selected = (20, 0);
app.handle_key(KeyEvent::new(KeyCode::PageUp, KeyModifiers::NONE))
.unwrap();
assert_eq!(app.model.active_view().selected.0, 5);
}
#[test]
fn jump_last_row_scrolls_with_small_terminal() {
let mut app = two_col_model();
// Total rows: A, B, C + R0..R9 = 13 rows. Last row = 12.
for i in 0..10 {
app.model.category_mut("Row").unwrap().add_item(&format!("R{i}"));
}
app.term_height = 13; // ~5 visible rows
app.model.active_view_mut().selected = (0, 0);
// G jumps to last row (row 12)
app.handle_key(KeyEvent::new(KeyCode::Char('G'), KeyModifiers::NONE))
.unwrap();
let last = app.model.active_view().selected.0;
assert_eq!(last, 12, "should be at last row");
// With only ~5 visible rows and 13 rows, offset should scroll.
// Bug: hardcoded 20 means `12 >= 0 + 20` is false → no scroll.
let offset = app.model.active_view().row_offset;
assert!(
offset > 0,
"row_offset should scroll when last row is beyond visible area, but is {offset}"
);
}
#[test]
fn ctrl_d_scrolls_viewport_with_small_terminal() {
let mut app = two_col_model();
for i in 0..30 {
app.model
.category_mut("Row")
.unwrap()
.add_item(&format!("R{i}"));
}
app.term_height = 13; // ~5 visible rows
app.model.active_view_mut().selected = (0, 0);
// Ctrl+d scrolls by 5 rows
app.handle_key(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL))
.unwrap();
assert_eq!(app.model.active_view().selected.0, 5);
// Press Ctrl+d again — now at row 10 with only 5 visible rows,
// row_offset should have scrolled (not stay at 0 due to hardcoded 20)
app.handle_key(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL))
.unwrap();
assert_eq!(app.model.active_view().selected.0, 10);
assert!(
app.model.active_view().row_offset > 0,
"row_offset should scroll with small terminal, but is {}",
app.model.active_view().row_offset
);
}
#[test]
fn tab_in_edit_mode_commits_and_moves_right() {
let mut app = two_col_model();
app.model.active_view_mut().selected = (0, 0);
// Enter edit mode
app.handle_key(KeyEvent::new(KeyCode::Char('i'), KeyModifiers::NONE))
.unwrap();
assert!(matches!(app.mode, AppMode::Editing { .. }));
// Type a digit
app.handle_key(KeyEvent::new(KeyCode::Char('5'), KeyModifiers::NONE))
.unwrap();
// Press Tab — should commit, move right, re-enter edit mode
app.handle_key(KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE))
.unwrap();
// Should be in edit mode on column 1
assert!(
matches!(app.mode, AppMode::Editing { .. }),
"should be in edit mode after Tab, but mode is {:?}",
app.mode
);
assert_eq!(
app.model.active_view().selected.1,
1,
"should have moved to column 1"
);
}
#[test]
fn command_mode_buffer_cleared_on_reentry() {
use crossterm::event::KeyEvent;