fix(core): improve view robustness and axis management
`records_display` now returns `None` for out-of-bounds columns. Added `try_axis_of` to `View` for non-panicking axis retrieval. `cycle_axis` now uses `try_axis_of` to avoid panicking on unknown categories. Add regression tests for out-of-bounds access and unknown category cycling. Co-Authored-By: fiddlerwoaroof/git-smart-commit (unsloth/gemma-4-26B-A4B-it-GGUF:UD-Q5_K_XL)
This commit is contained in:
@@ -182,11 +182,15 @@ impl GridLayout {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Get the display string for the cell at (row, col) in records mode.
|
/// Get the display string for the cell at (row, col) in records mode.
|
||||||
/// Returns None for normal (non-records) layouts.
|
/// Returns None for normal (non-records) layouts and for out-of-bounds
|
||||||
|
/// rows/columns (symmetric with `cell_key`).
|
||||||
pub fn records_display(&self, row: usize, col: usize) -> Option<String> {
|
pub fn records_display(&self, row: usize, col: usize) -> Option<String> {
|
||||||
let records = self.records.as_ref()?;
|
let records = self.records.as_ref()?;
|
||||||
let record = records.get(row)?;
|
let record = records.get(row)?;
|
||||||
let col_item = self.col_label(col);
|
let col_item = self.col_label(col);
|
||||||
|
if col_item.is_empty() {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
if col_item == "Value" {
|
if col_item == "Value" {
|
||||||
Some(record.1.to_string())
|
Some(record.1.to_string())
|
||||||
} else {
|
} else {
|
||||||
@@ -806,6 +810,23 @@ mod tests {
|
|||||||
assert_eq!(last_value, "999", "new record should be the last row");
|
assert_eq!(last_value, "999", "new record should be the last row");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Regression test for improvise-byz: `records_display` for an
|
||||||
|
/// out-of-bounds column called `col_label(col)`, which returns `""`;
|
||||||
|
/// `""` fails the `== "Value"` check, gets looked up as a category name
|
||||||
|
/// in the record (never found), and the function returned `Some("")` —
|
||||||
|
/// masking view/data mismatches. It must return `None`, symmetric with
|
||||||
|
/// `cell_key`, which explicitly returns `None` for an empty col label.
|
||||||
|
#[test]
|
||||||
|
fn records_display_out_of_bounds_col_returns_none() {
|
||||||
|
let mut wb = records_workbook();
|
||||||
|
let v = wb.active_view_mut();
|
||||||
|
v.set_axis("_Index", Axis::Row);
|
||||||
|
v.set_axis("_Dim", Axis::Column);
|
||||||
|
let layout = GridLayout::new(&wb.model, wb.active_view());
|
||||||
|
assert!(layout.is_records_mode());
|
||||||
|
assert_eq!(layout.records_display(0, layout.col_count()), None);
|
||||||
|
}
|
||||||
|
|
||||||
fn coord(pairs: &[(&str, &str)]) -> CellKey {
|
fn coord(pairs: &[(&str, &str)]) -> CellKey {
|
||||||
CellKey::new(
|
CellKey::new(
|
||||||
pairs
|
pairs
|
||||||
|
|||||||
@@ -111,13 +111,28 @@ impl View {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Axis assignment for a registered category.
|
||||||
|
///
|
||||||
|
/// # Panics
|
||||||
|
///
|
||||||
|
/// Panics if `cat_name` is not registered with this view. Registration
|
||||||
|
/// is guaranteed for every model category: `Workbook::new`,
|
||||||
|
/// `Workbook::add_category`, `Workbook::add_label_category`, and
|
||||||
|
/// `Workbook::create_view` all call `View::on_category_added` for every
|
||||||
|
/// view, so any name obtained from `Model::categories` or from this
|
||||||
|
/// view's own `category_axes` is safe. Use [`View::try_axis_of`] when
|
||||||
|
/// the name comes from anywhere else.
|
||||||
pub fn axis_of(&self, cat_name: &str) -> Axis {
|
pub fn axis_of(&self, cat_name: &str) -> Axis {
|
||||||
*self
|
self.try_axis_of(cat_name)
|
||||||
.category_axes
|
|
||||||
.get(cat_name)
|
|
||||||
.expect("axis_of called for category not registered with this view")
|
.expect("axis_of called for category not registered with this view")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Non-panicking variant of [`View::axis_of`]: returns `None` when the
|
||||||
|
/// category is not registered with this view.
|
||||||
|
pub fn try_axis_of(&self, cat_name: &str) -> Option<Axis> {
|
||||||
|
self.category_axes.get(cat_name).copied()
|
||||||
|
}
|
||||||
|
|
||||||
pub fn categories_on(&self, axis: Axis) -> Vec<&str> {
|
pub fn categories_on(&self, axis: Axis) -> Vec<&str> {
|
||||||
self.category_axes
|
self.category_axes
|
||||||
.iter()
|
.iter()
|
||||||
@@ -208,18 +223,21 @@ impl View {
|
|||||||
self.col_offset = 0;
|
self.col_offset = 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Cycle axis for a category: Row → Column → Page → None → Row
|
/// Cycle axis for a category: Row → Column → Page → None → Row.
|
||||||
|
/// No-op for an unregistered category, consistent with `set_axis`.
|
||||||
pub fn cycle_axis(&mut self, cat_name: &str) {
|
pub fn cycle_axis(&mut self, cat_name: &str) {
|
||||||
let next = match self.axis_of(cat_name) {
|
if let Some(current) = self.try_axis_of(cat_name) {
|
||||||
Axis::Row => Axis::Column,
|
let next = match current {
|
||||||
Axis::Column => Axis::Page,
|
Axis::Row => Axis::Column,
|
||||||
Axis::Page => Axis::None,
|
Axis::Column => Axis::Page,
|
||||||
Axis::None => Axis::Row,
|
Axis::Page => Axis::None,
|
||||||
};
|
Axis::None => Axis::Row,
|
||||||
self.set_axis(cat_name, next);
|
};
|
||||||
self.selected = (0, 0);
|
self.set_axis(cat_name, next);
|
||||||
self.row_offset = 0;
|
self.selected = (0, 0);
|
||||||
self.col_offset = 0;
|
self.row_offset = 0;
|
||||||
|
self.col_offset = 0;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -305,6 +323,31 @@ mod tests {
|
|||||||
v.axis_of("Ghost");
|
v.axis_of("Ghost");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Regression test for improvise-t8s: `cycle_axis` panicked (via the
|
||||||
|
/// `expect` in `axis_of`) when given a category not registered with the
|
||||||
|
/// view, while `set_axis` silently no-ops on the same input. `cycle_axis`
|
||||||
|
/// now goes through `try_axis_of` and no-ops on unknown categories,
|
||||||
|
/// consistent with `set_axis`.
|
||||||
|
#[test]
|
||||||
|
fn cycle_axis_unknown_category_is_noop() {
|
||||||
|
let mut v = view_with_cats(&["Region"]);
|
||||||
|
v.cycle_axis("Ghost");
|
||||||
|
assert_eq!(v.axis_of("Region"), Axis::Row);
|
||||||
|
assert_eq!(v.try_axis_of("Ghost"), None);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn try_axis_of_known_category_returns_some() {
|
||||||
|
let v = view_with_cats(&["Region"]);
|
||||||
|
assert_eq!(v.try_axis_of("Region"), Some(Axis::Row));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn try_axis_of_unknown_category_returns_none() {
|
||||||
|
let v = View::new("Test");
|
||||||
|
assert_eq!(v.try_axis_of("Ghost"), None);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn page_selection_set_and_get() {
|
fn page_selection_set_and_get() {
|
||||||
let mut v = view_with_cats(&["Region", "Product", "Time"]);
|
let mut v = view_with_cats(&["Region", "Product", "Time"]);
|
||||||
|
|||||||
Reference in New Issue
Block a user