simplified patch keyboard logic

This commit is contained in:
Roy Han
2026-03-03 10:13:22 -08:00
parent 9a4b8c2f7c
commit 48e38a127d
3 changed files with 148 additions and 176 deletions

View File

@@ -250,7 +250,8 @@ impl ApprovalOverlay {
fn patch_selected_index(&self) -> Option<usize> {
self.patch_state()
.and_then(PatchOverlayState::selected_index)
.map(|_| self.list.scroll_state().selected_idx)
.flatten()
}
fn patch_note_text(&self) -> String {
@@ -260,28 +261,23 @@ impl ApprovalOverlay {
}
fn patch_layout(&self, width: u16) -> Option<PatchLayout> {
self.current_request
.as_ref()
.and_then(|request| PatchLayout::new(request, &self.options, self.patch_state(), width))
}
fn patch_select_index(&mut self, idx: usize) {
let len = self.options.len();
let idx = idx.min(len.saturating_sub(1));
if let Some(state) = self.patch_state_mut() {
state.options_state.selected_idx = Some(idx);
if idx != PATCH_REJECT_OPTION_INDEX {
state.notes_visible = false;
}
state.note_submit_attempted = false;
}
self.current_request.as_ref().and_then(|request| {
PatchLayout::new(
request,
&self.options,
self.list.scroll_state(),
self.patch_state(),
width,
)
})
}
fn open_patch_notes(&mut self) {
if self.options.len() <= PATCH_REJECT_OPTION_INDEX {
return;
}
self.patch_select_index(PATCH_REJECT_OPTION_INDEX);
self.list
.set_selected_visible_index(PATCH_REJECT_OPTION_INDEX);
if let Some(state) = self.patch_state_mut() {
state.notes_visible = true;
state.focus = PatchFocus::Notes;
@@ -290,21 +286,6 @@ impl ApprovalOverlay {
}
}
fn move_patch_selection(&mut self, move_down: bool) {
let options_len = self.options.len();
if let Some(state) = self.patch_state_mut() {
if move_down {
state.options_state.move_down_wrap(options_len);
} else {
state.options_state.move_up_wrap(options_len);
}
if state.options_state.selected_idx != Some(PATCH_REJECT_OPTION_INDEX) {
state.notes_visible = false;
}
state.note_submit_attempted = false;
}
}
fn handle_exec_decision(&self, id: &str, command: &[String], decision: ReviewDecision) {
let Some(request) = self.current_request.as_ref() else {
return;
@@ -409,160 +390,132 @@ impl ApprovalOverlay {
}
}
fn try_handle_option_shortcut(&mut self, key_event: &KeyEvent) -> bool {
fn try_handle_shortcut(&mut self, key_event: &KeyEvent) -> bool {
if self.try_handle_global_shortcut(key_event) {
return true;
}
if self
.patch_state()
.is_some_and(PatchOverlayState::focus_is_notes)
{
return false;
}
self.options
.iter()
.position(|opt| {
opt.shortcuts()
.any(|shortcut| shortcut.is_press(*key_event))
})
.map(|idx| {
self.apply_selection(idx);
})
.map(|idx| self.apply_selection(idx))
.is_some()
}
fn handle_patch_key_event(&mut self, key_event: KeyEvent) {
fn sync_patch_state_with_list_selection(&mut self) {
let selected_idx = self.patch_selected_index();
if let Some(state) = self.patch_state_mut() {
if selected_idx != Some(PATCH_REJECT_OPTION_INDEX) {
state.notes_visible = false;
}
state.note_submit_attempted = false;
}
}
fn handle_patch_notes_key_event(&mut self, key_event: KeyEvent) -> bool {
if key_event.kind == KeyEventKind::Release {
return;
return true;
}
if self.done || self.current_complete {
return;
return true;
}
if !self
.patch_state()
.is_some_and(PatchOverlayState::focus_is_notes)
{
return false;
}
if self.try_handle_global_shortcut(&key_event) {
return;
return true;
}
match self.patch_state().map(|state| state.focus) {
Some(PatchFocus::Options) => match key_event {
KeyEvent {
code: KeyCode::Char('y'),
modifiers: KeyModifiers::NONE,
..
} => self.apply_selection(0),
KeyEvent {
code: KeyCode::Char('a'),
modifiers: KeyModifiers::NONE,
..
} => self.apply_selection(1),
KeyEvent {
code: KeyCode::Char('n'),
modifiers: KeyModifiers::NONE,
..
}
| KeyEvent {
code: KeyCode::Tab,
modifiers: KeyModifiers::NONE,
..
} => self.open_patch_notes(),
KeyEvent {
code: KeyCode::Char('1'),
modifiers: KeyModifiers::NONE,
..
} => self.apply_selection(0),
KeyEvent {
code: KeyCode::Char('2'),
modifiers: KeyModifiers::NONE,
..
} => self.apply_selection(1),
KeyEvent {
code: KeyCode::Char('3'),
modifiers: KeyModifiers::NONE,
..
} => self.open_patch_notes(),
KeyEvent {
code: KeyCode::Up | KeyCode::Char('k'),
modifiers: KeyModifiers::NONE,
..
} => self.move_patch_selection(false),
KeyEvent {
code: KeyCode::Down | KeyCode::Char('j'),
modifiers: KeyModifiers::NONE,
..
} => self.move_patch_selection(true),
KeyEvent {
code: KeyCode::Enter,
modifiers: KeyModifiers::NONE,
..
} => match self.patch_selected_index() {
Some(PATCH_REJECT_OPTION_INDEX) => self.open_patch_notes(),
Some(idx) => self.apply_selection(idx),
None => {}
},
_ => {}
},
Some(PatchFocus::Notes) => {
if matches!(
key_event,
KeyEvent {
code: KeyCode::Tab,
modifiers: KeyModifiers::NONE,
..
}
) {
if let Some(state) = self.patch_state_mut() {
state.focus = PatchFocus::Options;
state.note_submit_attempted = false;
}
return;
}
if matches!(
key_event,
KeyEvent {
code: KeyCode::Enter,
modifiers: KeyModifiers::NONE,
..
}
) {
let text = self.patch_note_text();
if text.trim().is_empty() {
if let Some(state) = self.patch_state_mut() {
state.note_submit_attempted = true;
}
return;
}
if let Some(ApprovalRequest::ApplyPatch { thread_id, id, .. }) =
self.current_request.as_ref()
{
self.app_event_tx
.send(AppEvent::RejectPatchApprovalWithNotes {
thread_id: *thread_id,
approval_id: id.clone(),
text,
});
self.current_complete = true;
self.advance_queue();
}
return;
}
if let Some(state) = self.patch_state_mut() {
let _ = state.composer.handle_key_event(key_event);
if !state.composer.current_text_with_pending().trim().is_empty() {
state.note_submit_attempted = false;
}
}
if matches!(
key_event,
KeyEvent {
code: KeyCode::Tab,
modifiers: KeyModifiers::NONE,
..
}
None => {}
) {
if let Some(state) = self.patch_state_mut() {
state.focus = PatchFocus::Options;
state.note_submit_attempted = false;
}
return true;
}
if matches!(
key_event,
KeyEvent {
code: KeyCode::Enter,
modifiers: KeyModifiers::NONE,
..
}
) {
let text = self.patch_note_text();
if text.trim().is_empty() {
if let Some(state) = self.patch_state_mut() {
state.note_submit_attempted = true;
}
return true;
}
if let Some(ApprovalRequest::ApplyPatch { thread_id, id, .. }) =
self.current_request.as_ref()
{
self.app_event_tx
.send(AppEvent::RejectPatchApprovalWithNotes {
thread_id: *thread_id,
approval_id: id.clone(),
text,
});
self.current_complete = true;
self.advance_queue();
}
return true;
}
if let Some(state) = self.patch_state_mut() {
let _ = state.composer.handle_key_event(key_event);
if !state.composer.current_text_with_pending().trim().is_empty() {
state.note_submit_attempted = false;
}
}
true
}
}
impl BottomPaneView for ApprovalOverlay {
fn handle_key_event(&mut self, key_event: KeyEvent) {
if self.handle_patch_notes_key_event(key_event) {
return;
}
if matches!(
self.current_request,
Some(ApprovalRequest::ApplyPatch { .. })
) && matches!(
key_event,
KeyEvent {
code: KeyCode::Tab,
modifiers: KeyModifiers::NONE,
..
}
) {
self.handle_patch_key_event(key_event);
self.open_patch_notes();
return;
}
if self.try_handle_global_shortcut(&key_event)
|| self.try_handle_option_shortcut(&key_event)
{
if self.try_handle_shortcut(&key_event) {
return;
}
self.list.handle_key_event(key_event);
if self.patch_state().is_some() {
self.sync_patch_state_with_list_selection();
}
if let Some(idx) = self.list.take_last_selected_index() {
self.apply_selection(idx);
}
@@ -662,7 +615,7 @@ impl Renderable for ApprovalOverlay {
let Some(state) = self.patch_state() else {
return self.list.cursor_pos(area);
};
if !state.focus_is_notes() || !state.notes_visible() {
if !state.focus_is_notes() || !state.notes_visible(self.patch_selected_index()) {
return self.list.cursor_pos(area);
}
self.patch_layout(area.width)

View File

@@ -40,7 +40,6 @@ pub(super) enum PatchFocus {
pub(super) struct PatchOverlayState {
pub(super) focus: PatchFocus,
pub(super) options_state: ScrollState,
pub(super) composer: ChatComposer,
pub(super) notes_visible: bool,
pub(super) note_submit_attempted: bool,
@@ -59,20 +58,12 @@ impl PatchOverlayState {
composer.set_footer_hint_override(Some(Vec::new()));
Self {
focus: PatchFocus::Options,
options_state: ScrollState {
selected_idx: Some(0),
..Default::default()
},
composer,
notes_visible: false,
note_submit_attempted: false,
}
}
pub(super) fn selected_index(&self) -> Option<usize> {
self.options_state.selected_idx
}
pub(super) fn focus_is_notes(&self) -> bool {
matches!(self.focus, PatchFocus::Notes)
}
@@ -81,13 +72,15 @@ impl PatchOverlayState {
self.composer.current_text_with_pending()
}
pub(super) fn notes_visible(&self) -> bool {
self.options_state.selected_idx == Some(PATCH_REJECT_OPTION_INDEX)
pub(super) fn notes_visible(&self, selected_idx: Option<usize>) -> bool {
selected_idx == Some(PATCH_REJECT_OPTION_INDEX)
&& (self.notes_visible || !self.note_text().trim().is_empty())
}
pub(super) fn note_error_visible(&self) -> bool {
self.notes_visible() && self.note_submit_attempted && self.note_text().trim().is_empty()
pub(super) fn note_error_visible(&self, selected_idx: Option<usize>) -> bool {
self.notes_visible(selected_idx)
&& self.note_submit_attempted
&& self.note_text().trim().is_empty()
}
fn notes_input_height(&self, width: u16) -> u16 {
@@ -114,6 +107,7 @@ impl PatchLayout {
pub(super) fn new(
request: &ApprovalRequest,
options: &[ApprovalOption],
options_state: ScrollState,
state: Option<&PatchOverlayState>,
width: u16,
) -> Option<Self> {
@@ -127,16 +121,16 @@ impl PatchLayout {
let title_lines = wrap_patch_title(width);
let header = build_header(request);
let header_height = header.desired_height(width);
let mut options_state = state.map(|state| state.options_state).unwrap_or_default();
let mut options_state = options_state;
if options_state.selected_idx.is_none() {
options_state.selected_idx = Some(0);
}
let rows = patch_option_rows(options, options_state.selected_idx);
let options_height =
measure_rows_height(&rows, &options_state, rows.len().max(1), width.max(1));
let hint_lines = patch_hint_lines(request, state, width);
let validation_lines = patch_validation_lines(state, width);
let show_notes = state.is_some_and(PatchOverlayState::notes_visible);
let hint_lines = patch_hint_lines(request, options_state.selected_idx, state, width);
let validation_lines = patch_validation_lines(options_state.selected_idx, state, width);
let show_notes = state.is_some_and(|state| state.notes_visible(options_state.selected_idx));
let notes_height = if show_notes {
state
.map(|state| state.notes_input_height(width))
@@ -329,16 +323,17 @@ fn patch_option_rows(
fn patch_hint_lines(
request: &ApprovalRequest,
selected_idx: Option<usize>,
state: Option<&PatchOverlayState>,
width: u16,
) -> Vec<Line<'static>> {
let mut hint = if state.is_some_and(PatchOverlayState::notes_visible) {
let mut hint = if state.is_some_and(|state| state.notes_visible(selected_idx)) {
if state.is_some_and(PatchOverlayState::focus_is_notes) {
"enter to send | tab to go back | esc to interrupt".to_string()
} else {
"enter or tab to edit follow up | esc to interrupt".to_string()
}
} else if state.and_then(PatchOverlayState::selected_index) == Some(PATCH_REJECT_OPTION_INDEX) {
} else if selected_idx == Some(PATCH_REJECT_OPTION_INDEX) {
"tab to follow up | esc to interrupt".to_string()
} else {
"esc to interrupt".to_string()
@@ -353,8 +348,12 @@ fn patch_hint_lines(
.collect()
}
fn patch_validation_lines(state: Option<&PatchOverlayState>, width: u16) -> Vec<Line<'static>> {
if !state.is_some_and(PatchOverlayState::note_error_visible) {
fn patch_validation_lines(
selected_idx: Option<usize>,
state: Option<&PatchOverlayState>,
width: u16,
) -> Vec<Line<'static>> {
if !state.is_some_and(|state| state.note_error_visible(selected_idx)) {
return Vec::new();
}

View File

@@ -476,6 +476,26 @@ impl ListSelectionView {
self.last_selected_actual_idx.take()
}
pub(crate) fn scroll_state(&self) -> ScrollState {
self.state
}
pub(crate) fn set_selected_visible_index(&mut self, idx: usize) {
let before = self.selected_actual_idx();
let len = self.visible_len();
if len == 0 {
self.state.reset();
return;
}
self.state.selected_idx = Some(idx.min(len - 1));
let visible = Self::max_visible_rows(len);
self.state.clamp_selection(len);
self.state.ensure_visible(len, visible);
if self.selected_actual_idx() != before {
self.fire_selection_changed();
}
}
fn rows_width(total_width: u16) -> u16 {
total_width.saturating_sub(2)
}