Skip to content

Assert list_state is selected in Detail movement methods instead of silently defaulting to 0 #48

@ooloth

Description

@ooloth

Why

All four movement methods on Screen::Detailmove_up, move_down, move_page_up, move_page_down — use .unwrap_or(0) when reading the selected list index. But apply_enter_action always calls ds.select(Some(0)) when entering the detail screen, so an unselected state in Detail is a programmer error, not a recoverable case. Silent defaulting to 0 masks the violation.

Current state

ui/tui/src/state/mod.rs lines 57, 74, 107, 124 all read:

let i = view.list_state.selected().unwrap_or(0);

The .unwrap_or(0) treats an impossible state as valid and silently computes a movement from the wrong baseline.

Ideal state

Each of the four methods asserts the invariant before reading the selection:

assert!(
    view.list_state.selected().is_some(),
    "list_state must be selected in Detail screen"
);
let i = view.list_state.selected().unwrap_or(0);

A programmer error that produces an unselected Detail state fails immediately at the first movement rather than silently computing from index 0.

Starting points

  • ui/tui/src/state/mod.rsmove_up, move_down, move_page_up, move_page_down in the Screen::Detail arm (lines 57, 74, 107, 124)

QA plan

  1. Read ui/tui/src/state/mod.rs — expect assert!(view.list_state.selected().is_some(), ...) at the top of each of the four Detail-arm movement methods
  2. Run cargo test — expect all tests pass

Done when

All four Detail-arm movement methods assert that list_state is selected before reading the index.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions