Support more scroll & edit actions#982
Conversation
There was a problem hiding this comment.
Pull request overview
Adds additional navigation/edit actions to Television, including word-wise input movement and more granular/non-cycling list navigation, aligning keybinding customization with the capabilities available in other panels.
Changes:
- Introduce word-movement input actions (
go_to_prev_word,go_to_next_word) and wire them through action → input handling. - Add half-page navigation and non-cycling (“nocycle”) variants for list selection; plumb a
cyclicflag through picker cursor movement. - Update help-panel action relevance and the sample config keybindings (alt-b/alt-f), plus picker unit tests for non-cycling behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
television/television.rs |
Extends cursor movement API with cyclic and handles new selection actions; updates render-trigger logic (but currently misses new nocycle variants). |
television/picker.rs |
Implements cyclic vs non-cyclic selection behavior and adds tests; contains a boundary-state bug for non-cyclic movement. |
television/action.rs |
Adds new Action variants and labels/docs for the new navigation/edit behaviors. |
television/input.rs |
Maps new word-movement actions to InputRequest. |
television/screen/help_panel.rs |
Includes new actions in help relevance filtering (but currently inconsistent across modes). |
.config/config.toml |
Adds default keybindings for word-wise navigation (alt-b / alt-f). |
Comments suppressed due to low confidence (1)
television/picker.rs:124
- Similar to
inner_next, in non-cyclic mode ifselectedis already at the first item thennew_selectedwon’t change, butrelative_selectis still decremented. This can make the relative state drift even though the absolute selection did not move. Consider making the boundary case a no-op (only update the relative selection whennew_selected != selected).
let new_selected = if cyclic {
selected.wrapping_add(total_items - 1) % total_items
} else {
selected.saturating_sub(1)
};
self.select(Some(new_selected));
if cyclic && new_selected == total_items - 1 {
self.relative_select(Some((height - 1).min(total_items - 1)));
} else {
self.relative_select(Some(relative_selected.saturating_sub(1)));
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks for the PR and sorry for the delay. I think each of these need separate PRs so we can discuss them in isolation. The current diff introduces too much logic to my taste and worsens code understandability (especially the nocycle logic). |
|
@alexpasmantier ok, thanks I moved out first two commits to separate PRs: If those merged I will rebase this one, since nocycle implementation depends on first two commits. |
📺 PR Description
Support more scroll & edit actions, see #977
Checklist