mirror of
https://github.com/openai/codex.git
synced 2026-10-15 16:48:52 +00:00
fix approval display
This commit is contained in:
@@ -344,10 +344,16 @@ impl App<'_> {
|
||||
}
|
||||
|
||||
let size = terminal.size()?;
|
||||
// Determine how tall the bottom pane should be, then clamp it so it never
|
||||
// exceeds the terminal height and also never grows beyond 10 rows. This keeps
|
||||
// the dynamically resizable bottom pane from taking over the screen while
|
||||
// still respecting very small terminals.
|
||||
let desired_height = match &self.app_state {
|
||||
AppState::Chat { widget } => widget.desired_height(),
|
||||
AppState::GitWarning { .. } => 10,
|
||||
};
|
||||
let max_allowed_height = size.height.min(10);
|
||||
let desired_height = desired_height.min(max_allowed_height);
|
||||
let mut area = terminal.viewport_area;
|
||||
area.height = desired_height;
|
||||
area.width = size.width;
|
||||
|
||||
@@ -65,12 +65,22 @@ impl<'a> BottomPaneView<'a> for ApprovalModalView<'a> {
|
||||
self.enqueue_request(req);
|
||||
None
|
||||
}
|
||||
|
||||
fn min_desired_height(&self) -> u16 {
|
||||
// Ensure there is enough room for all approval options (select mode)
|
||||
// plus the surrounding border drawn by the modal.
|
||||
(super::super::user_approval_widget::APPROVAL_OPTION_LABELS.len() as u16) + 2
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
use super::*;
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::user_approval_widget::APPROVAL_OPTION_LABELS;
|
||||
use ratatui::Terminal;
|
||||
use ratatui::backend::TestBackend;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::mpsc::channel;
|
||||
|
||||
@@ -101,4 +111,26 @@ mod tests {
|
||||
assert!(view.current.is_complete());
|
||||
assert!(view.is_complete());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn renders_all_select_options_with_min_height() {
|
||||
let (tx_raw, _rx) = channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx_raw);
|
||||
let view = ApprovalModalView::new(make_exec_request(), tx);
|
||||
|
||||
// Use the view's minimum desired height to render.
|
||||
let min_h = (APPROVAL_OPTION_LABELS.len() as u16) + 2; // options + border
|
||||
let mut terminal = Terminal::new(TestBackend::new(80, min_h)).unwrap();
|
||||
terminal
|
||||
.draw(|f| {
|
||||
view.render(f.area(), f.buffer_mut());
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
// Ensure every option label is present in the buffer output.
|
||||
let dump = format!("{:?}", terminal.backend());
|
||||
for label in APPROVAL_OPTION_LABELS {
|
||||
assert!(dump.contains(label), "missing option: {label}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -49,4 +49,11 @@ pub(crate) trait BottomPaneView<'a> {
|
||||
) -> Option<ApprovalRequest> {
|
||||
Some(request)
|
||||
}
|
||||
|
||||
/// Minimal height (in terminal rows) this view needs to be usable. The
|
||||
/// bottom pane will ensure at least this many rows are allocated while the
|
||||
/// view is active, even if the composer would normally request less.
|
||||
fn min_desired_height(&self) -> u16 {
|
||||
3
|
||||
}
|
||||
}
|
||||
|
||||
@@ -65,7 +65,12 @@ impl BottomPane<'_> {
|
||||
}
|
||||
|
||||
pub fn desired_height(&self) -> u16 {
|
||||
self.composer.desired_height()
|
||||
let composer_height = self.composer.desired_height();
|
||||
if let Some(view) = &self.active_view {
|
||||
composer_height.max(view.min_desired_height())
|
||||
} else {
|
||||
composer_height
|
||||
}
|
||||
}
|
||||
|
||||
/// Forward a key event to the active view or the composer.
|
||||
@@ -276,6 +281,7 @@ impl WidgetRef for &BottomPane<'_> {
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::user_approval_widget::APPROVAL_OPTION_LABELS;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::mpsc::channel;
|
||||
|
||||
@@ -301,4 +307,23 @@ mod tests {
|
||||
assert!(pane.ctrl_c_quit_hint_visible());
|
||||
assert_eq!(CancellationEvent::Ignored, pane.on_ctrl_c());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approval_modal_has_min_desired_height() {
|
||||
let (tx_raw, _rx) = channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx_raw);
|
||||
let mut pane = BottomPane::new(BottomPaneParams {
|
||||
app_event_tx: tx,
|
||||
has_input_focus: true,
|
||||
});
|
||||
// With no overlay, desired height is just the composer height (>= 3 typical).
|
||||
let base = pane.desired_height();
|
||||
assert!(base >= 1);
|
||||
|
||||
// Push an approval request – desired height should be at least enough
|
||||
// to show all options plus the modal border.
|
||||
pane.push_approval_request(exec_request());
|
||||
let min_modal = (APPROVAL_OPTION_LABELS.len() as u16) + 2;
|
||||
assert!(pane.desired_height() >= min_modal);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -55,34 +55,43 @@ struct SelectOption {
|
||||
enters_input_mode: bool,
|
||||
}
|
||||
|
||||
// User-visible labels for approval options, exported for tests and layout.
|
||||
pub(crate) const APPROVAL_OPTION_LABELS: &[&str] = &[
|
||||
"Yes (y)",
|
||||
"Yes, always approve this exact command for this session (a)",
|
||||
"Edit or give feedback (e)",
|
||||
"No, and keep going (n)",
|
||||
"No, and stop for now (esc)",
|
||||
];
|
||||
|
||||
// keep in same order as in the TS implementation
|
||||
const SELECT_OPTIONS: &[SelectOption] = &[
|
||||
SelectOption {
|
||||
label: "Yes (y)",
|
||||
label: APPROVAL_OPTION_LABELS[0],
|
||||
decision: Some(ReviewDecision::Approved),
|
||||
|
||||
enters_input_mode: false,
|
||||
},
|
||||
SelectOption {
|
||||
label: "Yes, always approve this exact command for this session (a)",
|
||||
label: APPROVAL_OPTION_LABELS[1],
|
||||
decision: Some(ReviewDecision::ApprovedForSession),
|
||||
|
||||
enters_input_mode: false,
|
||||
},
|
||||
SelectOption {
|
||||
label: "Edit or give feedback (e)",
|
||||
label: APPROVAL_OPTION_LABELS[2],
|
||||
decision: None,
|
||||
|
||||
enters_input_mode: true,
|
||||
},
|
||||
SelectOption {
|
||||
label: "No, and keep going (n)",
|
||||
label: APPROVAL_OPTION_LABELS[3],
|
||||
decision: Some(ReviewDecision::Denied),
|
||||
|
||||
enters_input_mode: false,
|
||||
},
|
||||
SelectOption {
|
||||
label: "No, and stop for now (esc)",
|
||||
label: APPROVAL_OPTION_LABELS[4],
|
||||
decision: Some(ReviewDecision::Abort),
|
||||
|
||||
enters_input_mode: false,
|
||||
|
||||
Reference in New Issue
Block a user