Fixing the "? for shortcuts"

- Only show the hint when composer is empty
- Don't reset footer on new task updates
- Reorder the elements
- Align the "?" and "/" with overlay on and off

Based on #4364
This commit is contained in:
Ahmed Ibrahim
2025-09-29 17:10:04 -07:00
committed by GitHub
parent 98efd352ae
commit c64da4ff71
24 changed files with 246 additions and 146 deletions

View File

@@ -22,7 +22,6 @@ use super::footer::FooterMode;
use super::footer::FooterProps;
use super::footer::esc_hint_mode;
use super::footer::footer_height;
use super::footer::prompt_mode;
use super::footer::render_footer;
use super::footer::reset_mode_after_activity;
use super::footer::toggle_shortcut_mode;
@@ -102,7 +101,7 @@ enum ActivePopup {
File(FileSearchPopup),
}
const FOOTER_SPACING_HEIGHT: u16 = 1;
const FOOTER_SPACING_HEIGHT: u16 = 0;
impl ChatComposer {
pub fn new(
@@ -143,11 +142,7 @@ impl ChatComposer {
pub fn desired_height(&self, width: u16) -> u16 {
let footer_props = self.footer_props();
let footer_hint_height = footer_height(footer_props);
let footer_spacing = if footer_hint_height > 0 {
FOOTER_SPACING_HEIGHT
} else {
0
};
let footer_spacing = Self::footer_spacing(footer_hint_height);
let footer_total_height = footer_hint_height + footer_spacing;
self.textarea
.desired_height(width.saturating_sub(LIVE_PREFIX_COLS))
@@ -162,11 +157,7 @@ impl ChatComposer {
fn layout_areas(&self, area: Rect) -> [Rect; 3] {
let footer_props = self.footer_props();
let footer_hint_height = footer_height(footer_props);
let footer_spacing = if footer_hint_height > 0 {
FOOTER_SPACING_HEIGHT
} else {
0
};
let footer_spacing = Self::footer_spacing(footer_hint_height);
let footer_total_height = footer_hint_height + footer_spacing;
let popup_constraint = match &self.active_popup {
ActivePopup::Command(popup) => {
@@ -188,6 +179,14 @@ impl ChatComposer {
[composer_rect, textarea_rect, popup_rect]
}
fn footer_spacing(footer_hint_height: u16) -> u16 {
if footer_hint_height == 0 {
0
} else {
FOOTER_SPACING_HEIGHT
}
}
pub fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> {
let [_, textarea_rect, _] = self.layout_areas(area);
let state = *self.textarea_state.borrow();
@@ -337,7 +336,7 @@ impl ChatComposer {
pub fn set_ctrl_c_quit_hint(&mut self, show: bool, has_focus: bool) {
self.ctrl_c_quit_hint = show;
if show {
self.footer_mode = prompt_mode();
self.footer_mode = FooterMode::CtrlCReminder;
} else {
self.footer_mode = reset_mode_after_activity(self.footer_mode);
}
@@ -1261,12 +1260,14 @@ impl ChatComposer {
return false;
}
let toggles = match key_event.code {
KeyCode::Char('?') if key_event.modifiers.is_empty() => true,
KeyCode::BackTab => true,
KeyCode::Tab if key_event.modifiers.contains(KeyModifiers::SHIFT) => true,
_ => false,
};
let toggles = matches!(
key_event,
KeyEvent {
code: KeyCode::Char('?'),
modifiers: KeyModifiers::NONE,
..
} if self.is_empty()
);
if !toggles {
return false;
@@ -1288,12 +1289,13 @@ impl ChatComposer {
}
fn footer_mode(&self) -> FooterMode {
if matches!(self.footer_mode, FooterMode::EscHint) {
FooterMode::EscHint
} else if self.ctrl_c_quit_hint {
FooterMode::CtrlCReminder
} else {
self.footer_mode
match self.footer_mode {
FooterMode::EscHint => FooterMode::EscHint,
FooterMode::ShortcutOverlay => FooterMode::ShortcutOverlay,
FooterMode::CtrlCReminder => FooterMode::CtrlCReminder,
FooterMode::ShortcutPrompt if self.ctrl_c_quit_hint => FooterMode::CtrlCReminder,
FooterMode::ShortcutPrompt if !self.is_empty() => FooterMode::Empty,
other => other,
}
}
@@ -1405,9 +1407,6 @@ impl ChatComposer {
pub fn set_task_running(&mut self, running: bool) {
self.is_task_running = running;
if running {
self.footer_mode = prompt_mode();
}
}
pub(crate) fn set_esc_backtrack_hint(&mut self, show: bool) {
@@ -1431,12 +1430,9 @@ impl WidgetRef for ChatComposer {
popup.render_ref(popup_rect, buf);
}
ActivePopup::None => {
let footer_hint_height = footer_height(self.footer_props());
let footer_spacing = if footer_hint_height > 0 {
FOOTER_SPACING_HEIGHT
} else {
0
};
let footer_props = self.footer_props();
let footer_hint_height = footer_height(footer_props);
let footer_spacing = Self::footer_spacing(footer_hint_height);
let hint_rect = if footer_spacing > 0 && footer_hint_height > 0 {
let [_, hint_rect] = Layout::vertical([
Constraint::Length(footer_spacing),
@@ -1447,10 +1443,7 @@ impl WidgetRef for ChatComposer {
} else {
popup_rect
};
let mut footer_rect = hint_rect;
footer_rect.x = footer_rect.x.saturating_add(2);
footer_rect.width = footer_rect.width.saturating_sub(2);
render_footer(footer_rect, buf, self.footer_props());
render_footer(hint_rect, buf, footer_props);
}
}
let style = user_message_style(terminal_palette::default_bg());
@@ -1566,8 +1559,10 @@ mod tests {
false,
);
setup(&mut composer);
let footer_lines = footer_height(composer.footer_props());
let height = footer_lines + 8;
let footer_props = composer.footer_props();
let footer_lines = footer_height(footer_props);
let footer_spacing = ChatComposer::footer_spacing(footer_lines);
let height = footer_lines + footer_spacing + 8;
let mut terminal = Terminal::new(TestBackend::new(width, height)).unwrap();
terminal
.draw(|f| f.render_widget_ref(composer, f.area()))
@@ -1621,6 +1616,76 @@ mod tests {
composer.set_esc_backtrack_hint(true);
},
);
snapshot_composer_state("footer_mode_hidden_while_typing", true, |composer| {
type_chars_humanlike(composer, &['h']);
});
}
#[test]
fn question_mark_only_toggles_on_first_char() {
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyModifiers;
let (tx, _rx) = unbounded_channel::<AppEvent>();
let sender = AppEventSender::new(tx);
let mut composer = ChatComposer::new(
true,
sender,
false,
"Ask Codex to do anything".to_string(),
false,
);
let (result, needs_redraw) =
composer.handle_key_event(KeyEvent::new(KeyCode::Char('?'), KeyModifiers::NONE));
assert_eq!(result, InputResult::None);
assert!(needs_redraw, "toggling overlay should request redraw");
assert_eq!(composer.footer_mode, FooterMode::ShortcutOverlay);
// Toggle back to prompt mode so subsequent typing captures characters.
let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('?'), KeyModifiers::NONE));
assert_eq!(composer.footer_mode, FooterMode::ShortcutPrompt);
type_chars_humanlike(&mut composer, &['h']);
assert_eq!(composer.textarea.text(), "h");
assert_eq!(composer.footer_mode(), FooterMode::Empty);
let (result, needs_redraw) =
composer.handle_key_event(KeyEvent::new(KeyCode::Char('?'), KeyModifiers::NONE));
assert_eq!(result, InputResult::None);
assert!(needs_redraw, "typing should still mark the view dirty");
std::thread::sleep(ChatComposer::recommended_paste_flush_delay());
let _ = composer.flush_paste_burst_if_due();
assert_eq!(composer.textarea.text(), "h?");
assert_eq!(composer.footer_mode, FooterMode::ShortcutPrompt);
assert_eq!(composer.footer_mode(), FooterMode::Empty);
}
#[test]
fn shortcut_overlay_persists_while_task_running() {
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyModifiers;
let (tx, _rx) = unbounded_channel::<AppEvent>();
let sender = AppEventSender::new(tx);
let mut composer = ChatComposer::new(
true,
sender,
false,
"Ask Codex to do anything".to_string(),
false,
);
let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('?'), KeyModifiers::NONE));
assert_eq!(composer.footer_mode, FooterMode::ShortcutOverlay);
composer.set_task_running(true);
assert_eq!(composer.footer_mode, FooterMode::ShortcutOverlay);
assert_eq!(composer.footer_mode(), FooterMode::ShortcutOverlay);
}
#[test]

View File

@@ -1,11 +1,12 @@
use crate::ui_consts::FOOTER_INDENT_COLS;
use crossterm::event::KeyCode;
use crossterm::event::KeyModifiers;
use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use ratatui::style::Stylize;
use ratatui::text::Line;
use ratatui::text::Span;
use ratatui::widgets::WidgetRef;
use std::iter;
#[derive(Clone, Copy, Debug)]
pub(crate) struct FooterProps {
@@ -21,12 +22,14 @@ pub(crate) enum FooterMode {
ShortcutPrompt,
ShortcutOverlay,
EscHint,
Empty,
}
pub(crate) fn toggle_shortcut_mode(current: FooterMode, ctrl_c_hint: bool) -> FooterMode {
if ctrl_c_hint {
if ctrl_c_hint && matches!(current, FooterMode::CtrlCReminder) {
return current;
}
match current {
FooterMode::ShortcutOverlay | FooterMode::CtrlCReminder => FooterMode::ShortcutPrompt,
_ => FooterMode::ShortcutOverlay,
@@ -43,15 +46,14 @@ pub(crate) fn esc_hint_mode(current: FooterMode, is_task_running: bool) -> Foote
pub(crate) fn reset_mode_after_activity(current: FooterMode) -> FooterMode {
match current {
FooterMode::EscHint | FooterMode::ShortcutOverlay => FooterMode::ShortcutPrompt,
FooterMode::EscHint
| FooterMode::ShortcutOverlay
| FooterMode::CtrlCReminder
| FooterMode::Empty => FooterMode::ShortcutPrompt,
other => other,
}
}
pub(crate) fn prompt_mode() -> FooterMode {
FooterMode::ShortcutPrompt
}
pub(crate) fn footer_height(props: FooterProps) -> u16 {
footer_lines(props).len() as u16
}
@@ -70,24 +72,16 @@ pub(crate) fn render_footer(area: Rect, buf: &mut Buffer, props: FooterProps) {
fn footer_lines(props: FooterProps) -> Vec<Line<'static>> {
match props.mode {
FooterMode::CtrlCReminder => {
vec![ctrl_c_reminder_line(CtrlCReminderState {
is_task_running: props.is_task_running,
})]
}
FooterMode::ShortcutPrompt => vec![Line::from(vec!["? for shortcuts".dim()])],
FooterMode::CtrlCReminder => vec![ctrl_c_reminder_line(CtrlCReminderState {
is_task_running: props.is_task_running,
})],
FooterMode::ShortcutPrompt => vec![dim_line(indent_text("? for shortcuts"))],
FooterMode::ShortcutOverlay => shortcut_overlay_lines(ShortcutsState {
use_shift_enter_hint: props.use_shift_enter_hint,
esc_backtrack_hint: props.esc_backtrack_hint,
is_task_running: props.is_task_running,
}),
FooterMode::EscHint => {
vec![esc_hint_line(ShortcutsState {
use_shift_enter_hint: props.use_shift_enter_hint,
esc_backtrack_hint: props.esc_backtrack_hint,
is_task_running: props.is_task_running,
})]
}
FooterMode::EscHint => vec![esc_hint_line(props.esc_backtrack_hint)],
FooterMode::Empty => Vec::new(),
}
}
@@ -100,7 +94,6 @@ struct CtrlCReminderState {
struct ShortcutsState {
use_shift_enter_hint: bool,
esc_backtrack_hint: bool,
is_task_running: bool,
}
fn ctrl_c_reminder_line(state: CtrlCReminderState) -> Line<'static> {
@@ -109,28 +102,54 @@ fn ctrl_c_reminder_line(state: CtrlCReminderState) -> Line<'static> {
} else {
"quit"
};
Line::from(vec![
Span::from(format!(" ctrl + c again to {action}")).dim(),
])
let text = format!("ctrl + c again to {action}");
dim_line(indent_text(&text))
}
fn esc_hint_line(state: ShortcutsState) -> Line<'static> {
let text = if state.esc_backtrack_hint {
" esc again to edit previous message"
fn esc_hint_line(esc_backtrack_hint: bool) -> Line<'static> {
let text = if esc_backtrack_hint {
"esc again to edit previous message"
} else {
" esc esc to edit previous message"
"esc esc to edit previous message"
};
Line::from(vec![Span::from(text).dim()])
dim_line(indent_text(text))
}
fn shortcut_overlay_lines(state: ShortcutsState) -> Vec<Line<'static>> {
let mut rendered = Vec::new();
let mut commands = String::new();
let mut newline = String::new();
let mut file_paths = String::new();
let mut paste_image = String::new();
let mut edit_previous = String::new();
let mut quit = String::new();
let mut show_transcript = String::new();
for descriptor in SHORTCUTS {
if let Some(text) = descriptor.overlay_entry(state) {
rendered.push(text);
match descriptor.id {
ShortcutId::Commands => commands = text,
ShortcutId::InsertNewline => newline = text,
ShortcutId::FilePaths => file_paths = text,
ShortcutId::PasteImage => paste_image = text,
ShortcutId::EditPrevious => edit_previous = text,
ShortcutId::Quit => quit = text,
ShortcutId::ShowTranscript => show_transcript = text,
}
}
}
build_columns(rendered)
let ordered = vec![
commands,
newline,
file_paths,
paste_image,
edit_previous,
quit,
String::new(),
show_transcript,
];
build_columns(ordered)
}
fn build_columns(entries: Vec<String>) -> Vec<Line<'static>> {
@@ -138,11 +157,20 @@ fn build_columns(entries: Vec<String>) -> Vec<Line<'static>> {
return Vec::new();
}
const COLUMNS: usize = 3;
const MAX_PADDED_WIDTHS: [usize; COLUMNS - 1] = [24, 28];
const MIN_PADDED_WIDTHS: [usize; COLUMNS - 1] = [22, 0];
const COLUMNS: usize = 2;
const COLUMN_PADDING: [usize; COLUMNS] = [4, 4];
const COLUMN_GAP: usize = 4;
let rows = entries.len().div_ceil(COLUMNS);
let target_len = rows * COLUMNS;
let mut entries = entries;
if entries.len() < target_len {
entries.extend(std::iter::repeat_n(
String::new(),
target_len - entries.len(),
));
}
let mut column_widths = [0usize; COLUMNS];
for (idx, entry) in entries.iter().enumerate() {
@@ -150,39 +178,43 @@ fn build_columns(entries: Vec<String>) -> Vec<Line<'static>> {
column_widths[column] = column_widths[column].max(entry.len());
}
let mut lines = Vec::new();
for row in 0..rows {
let mut line = String::from(" ");
for col in 0..COLUMNS {
let idx = row * COLUMNS + col;
if idx >= entries.len() {
continue;
}
let entry = &entries[idx];
if col < COLUMNS - 1 {
let max_width = MAX_PADDED_WIDTHS[col];
let mut target_width = column_widths[col];
target_width = target_width.max(MIN_PADDED_WIDTHS[col]).min(max_width);
let pad_width = target_width + 2;
line.push_str(&format!("{entry:<pad_width$}"));
} else {
if col != 0 {
line.push_str(" ");
}
line.push_str(entry);
}
}
lines.push(Line::from(vec![Span::from(line).dim()]));
for (idx, width) in column_widths.iter_mut().enumerate() {
*width += COLUMN_PADDING[idx];
}
lines
entries
.chunks(COLUMNS)
.map(|chunk| {
let mut line = String::new();
for (col, entry) in chunk.iter().enumerate() {
line.push_str(entry);
if col < COLUMNS - 1 {
let target_width = column_widths[col];
let padding = target_width.saturating_sub(entry.len()) + COLUMN_GAP;
line.push_str(&" ".repeat(padding));
}
}
let indented = indent_text(&line);
dim_line(indented)
})
.collect()
}
fn indent_text(text: &str) -> String {
let mut indented = String::with_capacity(FOOTER_INDENT_COLS + text.len());
indented.extend(iter::repeat_n(' ', FOOTER_INDENT_COLS));
indented.push_str(text);
indented
}
fn dim_line(text: String) -> Line<'static> {
Line::from(text).dim()
}
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
enum ShortcutId {
Commands,
InsertNewline,
ChangeMode,
FilePaths,
PasteImage,
EditPrevious,
@@ -236,13 +268,6 @@ impl ShortcutDescriptor {
fn overlay_entry(&self, state: ShortcutsState) -> Option<String> {
let binding = self.binding_for(state)?;
let label = match self.id {
ShortcutId::Quit => {
if state.is_task_running {
" to interrupt"
} else {
self.label
}
}
ShortcutId::EditPrevious => {
if state.esc_backtrack_hint {
" again to edit previous message"
@@ -252,12 +277,7 @@ impl ShortcutDescriptor {
}
_ => self.label,
};
let text = match self.id {
ShortcutId::Quit if state.is_task_running => {
format!("{}{} to interrupt", self.prefix, binding.overlay_text)
}
_ => format!("{}{}{}", self.prefix, binding.overlay_text, label),
};
let text = format!("{}{}{}", self.prefix, binding.overlay_text, label);
Some(text)
}
}
@@ -293,17 +313,6 @@ const SHORTCUTS: &[ShortcutDescriptor] = &[
prefix: "",
label: " for newline",
},
ShortcutDescriptor {
id: ShortcutId::ChangeMode,
bindings: &[ShortcutBinding {
code: KeyCode::BackTab,
modifiers: KeyModifiers::SHIFT,
overlay_text: "shift + tab",
condition: DisplayCondition::Always,
}],
prefix: "",
label: " to change mode",
},
ShortcutDescriptor {
id: ShortcutId::FilePaths,
bindings: &[ShortcutBinding {

View File

@@ -11,4 +11,4 @@ expression: terminal.backend()
" "
" "
" "
" ? for shortcuts "
" "

View File

@@ -1,5 +1,6 @@
---
source: tui/src/bottom_pane/chat_composer.rs
assertion_line: 1938
expression: terminal.backend()
---
" "

View File

@@ -1,5 +1,6 @@
---
source: tui/src/bottom_pane/chat_composer.rs
assertion_line: 1497
expression: terminal.backend()
---
" "
@@ -10,4 +11,4 @@ expression: terminal.backend()
" "
" "
" "
" ctrl + c again to interrupt "
" ctrl + c again to interrupt "

View File

@@ -1,5 +1,6 @@
---
source: tui/src/bottom_pane/chat_composer.rs
assertion_line: 1497
expression: terminal.backend()
---
" "
@@ -10,4 +11,4 @@ expression: terminal.backend()
" "
" "
" "
" ctrl + c again to quit "
" ctrl + c again to quit "

View File

@@ -1,5 +1,6 @@
---
source: tui/src/bottom_pane/chat_composer.rs
assertion_line: 1497
expression: terminal.backend()
---
" "
@@ -10,4 +11,4 @@ expression: terminal.backend()
" "
" "
" "
" esc esc to edit previous message "
" esc esc to edit previous message "

View File

@@ -1,5 +1,6 @@
---
source: tui/src/bottom_pane/chat_composer.rs
assertion_line: 1497
expression: terminal.backend()
---
" "
@@ -10,4 +11,4 @@ expression: terminal.backend()
" "
" "
" "
" esc again to edit previous message "
" esc again to edit previous message "

View File

@@ -1,5 +1,6 @@
---
source: tui/src/bottom_pane/chat_composer.rs
assertion_line: 1497
expression: terminal.backend()
---
" "
@@ -10,4 +11,4 @@ expression: terminal.backend()
" "
" "
" "
" esc esc to edit previous message "
" esc esc to edit previous message "

View File

@@ -0,0 +1,12 @@
---
source: tui/src/bottom_pane/chat_composer.rs
expression: terminal.backend()
---
" "
" h "
" "
" "
" "
" "
" "
" "

View File

@@ -1,5 +1,6 @@
---
source: tui/src/bottom_pane/chat_composer.rs
assertion_line: 1497
expression: terminal.backend()
---
" "
@@ -10,4 +11,4 @@ expression: terminal.backend()
" "
" "
" "
" esc again to edit previous message "
" esc again to edit previous message "

View File

@@ -1,5 +1,6 @@
---
source: tui/src/bottom_pane/chat_composer.rs
assertion_line: 1497
expression: terminal.backend()
---
" "
@@ -10,6 +11,7 @@ expression: terminal.backend()
" "
" "
" "
" / for commands shift + enter for newline shift + tab to change mode "
" @ for file paths ctrl + v to paste images esc again to edit previous message "
" ctrl + c to exit ctrl + t to view transcript "
" / for commands shift + enter for newline "
" @ for file paths ctrl + v to paste images "
" esc again to edit previous message ctrl + c to exit "
" ctrl + t to view transcript "

View File

@@ -11,4 +11,4 @@ expression: terminal.backend()
" "
" "
" "
" ? for shortcuts "
" "

View File

@@ -11,4 +11,4 @@ expression: terminal.backend()
" "
" "
" "
" ? for shortcuts "
" "

View File

@@ -11,4 +11,4 @@ expression: terminal.backend()
" "
" "
" "
" ? for shortcuts "
" "

View File

@@ -1,5 +1,6 @@
---
source: tui/src/bottom_pane/footer.rs
assertion_line: 389
expression: terminal.backend()
---
" ctrl + c again to quit "

View File

@@ -1,5 +1,6 @@
---
source: tui/src/bottom_pane/footer.rs
assertion_line: 389
expression: terminal.backend()
---
" ctrl + c again to interrupt "

View File

@@ -1,5 +1,6 @@
---
source: tui/src/bottom_pane/footer.rs
assertion_line: 389
expression: terminal.backend()
---
" esc esc to edit previous message "

View File

@@ -1,5 +1,6 @@
---
source: tui/src/bottom_pane/footer.rs
assertion_line: 389
expression: terminal.backend()
---
" esc again to edit previous message "

View File

@@ -1,5 +1,6 @@
---
source: tui/src/bottom_pane/footer.rs
assertion_line: 389
expression: terminal.backend()
---
"? for shortcuts "
" ? for shortcuts "

View File

@@ -1,7 +1,9 @@
---
source: tui/src/bottom_pane/footer.rs
assertion_line: 389
expression: terminal.backend()
---
" / for commands shift + enter for newline shift + tab to change m"
" @ for file paths ctrl + v to paste images esc again to edit previ"
" ctrl + c to exit ctrl + t to view transcript "
" / for commands shift + enter for newline "
" @ for file paths ctrl + v to paste images "
" esc again to edit previous message ctrl + c to exit "
" ctrl + t to view transcript "

View File

@@ -13,6 +13,3 @@ expression: term.backend().vt100().screen().contents()
Summarize recent commits
? for shortcuts

View File

@@ -1,5 +1,6 @@
---
source: tui/src/chatwidget/tests.rs
assertion_line: 1445
expression: terminal.backend()
---
" "
@@ -8,6 +9,5 @@ expression: terminal.backend()
" "
" Ask Codex to do anything "
" "
" "
" ? for shortcuts "
" "

View File

@@ -8,3 +8,4 @@
/// - Status indicator lines begin with this many spaces for alignment.
/// - User history lines account for this many columns (e.g., "▌ ") when wrapping.
pub(crate) const LIVE_PREFIX_COLS: u16 = 2;
pub(crate) const FOOTER_INDENT_COLS: usize = LIVE_PREFIX_COLS as usize;