fix: handle /review arguments in TUI (#8823)

Handle /review <instructions> in the TUI and TUI2 by routing it as a
custom review command instead of plain text, wiring command dispatch and
adding composer coverage so typing /review text starts a review directly
rather than posting a message. User impact: /review with arguments now
kicks off the review flow, previously it would just forward as a plain
command and not actually start a review.
This commit is contained in:
Thibault Sottiaux
2026-01-07 05:14:55 -08:00
committed by GitHub
parent a59052341d
commit 124a09e577
4 changed files with 180 additions and 0 deletions

View File

@@ -73,6 +73,7 @@ const LARGE_PASTE_CHAR_THRESHOLD: usize = 1000;
pub enum InputResult {
Submitted(String),
Command(SlashCommand),
CommandWithArgs(SlashCommand, String),
None,
}
@@ -1274,6 +1275,18 @@ impl ChatComposer {
}
}
if !input_starts_with_space
&& let Some((name, rest)) = parse_slash_name(&text)
&& !rest.is_empty()
&& !name.contains('/')
&& let Some((_n, cmd)) = built_in_slash_commands()
.into_iter()
.find(|(command_name, _)| *command_name == name)
&& cmd == SlashCommand::Review
{
return (InputResult::CommandWithArgs(cmd, rest.to_string()), true);
}
let expanded_prompt = match expand_custom_prompt(&text, &self.custom_prompts) {
Ok(expanded) => expanded,
Err(err) => {
@@ -2841,6 +2854,9 @@ mod tests {
InputResult::Command(cmd) => {
assert_eq!(cmd.command(), "init");
}
InputResult::CommandWithArgs(_, _) => {
panic!("expected command dispatch without args for '/init'")
}
InputResult::Submitted(text) => {
panic!("expected command dispatch, but composer submitted literal text: {text}")
}
@@ -2849,6 +2865,44 @@ mod tests {
assert!(composer.textarea.is_empty(), "composer should be cleared");
}
#[test]
fn slash_review_with_args_dispatches_command_with_args() {
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,
);
type_chars_humanlike(&mut composer, &['/', 'r', 'e', 'v', 'i', 'e', 'w', ' ']);
type_chars_humanlike(&mut composer, &['f', 'i', 'x', ' ', 't', 'h', 'i', 's']);
let (result, _needs_redraw) =
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
match result {
InputResult::CommandWithArgs(cmd, args) => {
assert_eq!(cmd, SlashCommand::Review);
assert_eq!(args, "fix this");
}
InputResult::Command(cmd) => {
panic!("expected args for '/review', got bare command: {cmd:?}")
}
InputResult::Submitted(text) => {
panic!("expected command dispatch, got literal submit: {text}")
}
InputResult::None => panic!("expected CommandWithArgs result for '/review'"),
}
assert!(composer.textarea.is_empty(), "composer should be cleared");
}
#[test]
fn extract_args_supports_quoted_paths_single_arg() {
let args = extract_positional_args_for_prompt_line(
@@ -2914,6 +2968,9 @@ mod tests {
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
match result {
InputResult::Command(cmd) => assert_eq!(cmd.command(), "diff"),
InputResult::CommandWithArgs(_, _) => {
panic!("expected command dispatch without args for '/diff'")
}
InputResult::Submitted(text) => {
panic!("expected command dispatch after Tab completion, got literal submit: {text}")
}
@@ -2947,6 +3004,9 @@ mod tests {
InputResult::Command(cmd) => {
assert_eq!(cmd.command(), "mention");
}
InputResult::CommandWithArgs(_, _) => {
panic!("expected command dispatch without args for '/mention'")
}
InputResult::Submitted(text) => {
panic!("expected command dispatch, but composer submitted literal text: {text}")
}

View File

@@ -1641,6 +1641,9 @@ impl ChatWidget {
InputResult::Command(cmd) => {
self.dispatch_command(cmd);
}
InputResult::CommandWithArgs(cmd, args) => {
self.dispatch_command_with_args(cmd, args);
}
InputResult::None => {}
}
}
@@ -1837,6 +1840,33 @@ impl ChatWidget {
}
}
fn dispatch_command_with_args(&mut self, cmd: SlashCommand, args: String) {
if !cmd.available_during_task() && self.bottom_pane.is_task_running() {
let message = format!(
"'/{}' is disabled while a task is in progress.",
cmd.command()
);
self.add_to_history(history_cell::new_error_event(message));
self.request_redraw();
return;
}
let trimmed = args.trim();
match cmd {
SlashCommand::Review if !trimmed.is_empty() => {
self.submit_op(Op::Review {
review_request: ReviewRequest {
target: ReviewTarget::Custom {
instructions: trimmed.to_string(),
},
user_facing_hint: None,
},
});
}
_ => self.dispatch_command(cmd),
}
}
pub(crate) fn handle_paste(&mut self, text: String) {
self.bottom_pane.handle_paste(text);
}

View File

@@ -76,6 +76,7 @@ const LARGE_PASTE_CHAR_THRESHOLD: usize = 1000;
pub enum InputResult {
Submitted(String),
Command(SlashCommand),
CommandWithArgs(SlashCommand, String),
None,
}
@@ -1191,6 +1192,18 @@ impl ChatComposer {
}
}
if !input_starts_with_space
&& let Some((name, rest)) = parse_slash_name(&text)
&& !rest.is_empty()
&& !name.contains('/')
&& let Some((_n, cmd)) = built_in_slash_commands()
.into_iter()
.find(|(command_name, _)| *command_name == name)
&& cmd == SlashCommand::Review
{
return (InputResult::CommandWithArgs(cmd, rest.to_string()), true);
}
let expanded_prompt = match expand_custom_prompt(&text, &self.custom_prompts) {
Ok(expanded) => expanded,
Err(err) => {
@@ -2754,6 +2767,9 @@ mod tests {
InputResult::Command(cmd) => {
assert_eq!(cmd.command(), "init");
}
InputResult::CommandWithArgs(_, _) => {
panic!("expected command dispatch without args for '/init'")
}
InputResult::Submitted(text) => {
panic!("expected command dispatch, but composer submitted literal text: {text}")
}
@@ -2762,6 +2778,44 @@ mod tests {
assert!(composer.textarea.is_empty(), "composer should be cleared");
}
#[test]
fn slash_review_with_args_dispatches_command_with_args() {
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,
);
type_chars_humanlike(&mut composer, &['/', 'r', 'e', 'v', 'i', 'e', 'w', ' ']);
type_chars_humanlike(&mut composer, &['f', 'i', 'x', ' ', 't', 'h', 'i', 's']);
let (result, _needs_redraw) =
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
match result {
InputResult::CommandWithArgs(cmd, args) => {
assert_eq!(cmd, SlashCommand::Review);
assert_eq!(args, "fix this");
}
InputResult::Command(cmd) => {
panic!("expected args for '/review', got bare command: {cmd:?}")
}
InputResult::Submitted(text) => {
panic!("expected command dispatch, got literal submit: {text}")
}
InputResult::None => panic!("expected CommandWithArgs result for '/review'"),
}
assert!(composer.textarea.is_empty(), "composer should be cleared");
}
#[test]
fn extract_args_supports_quoted_paths_single_arg() {
let args = extract_positional_args_for_prompt_line(
@@ -2827,6 +2881,9 @@ mod tests {
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
match result {
InputResult::Command(cmd) => assert_eq!(cmd.command(), "diff"),
InputResult::CommandWithArgs(_, _) => {
panic!("expected command dispatch without args for '/diff'")
}
InputResult::Submitted(text) => {
panic!("expected command dispatch after Tab completion, got literal submit: {text}")
}
@@ -2860,6 +2917,9 @@ mod tests {
InputResult::Command(cmd) => {
assert_eq!(cmd.command(), "mention");
}
InputResult::CommandWithArgs(_, _) => {
panic!("expected command dispatch without args for '/mention'")
}
InputResult::Submitted(text) => {
panic!("expected command dispatch, but composer submitted literal text: {text}")
}

View File

@@ -1500,6 +1500,9 @@ impl ChatWidget {
InputResult::Command(cmd) => {
self.dispatch_command(cmd);
}
InputResult::CommandWithArgs(cmd, args) => {
self.dispatch_command_with_args(cmd, args);
}
InputResult::None => {}
}
}
@@ -1665,6 +1668,33 @@ impl ChatWidget {
}
}
fn dispatch_command_with_args(&mut self, cmd: SlashCommand, args: String) {
if !cmd.available_during_task() && self.bottom_pane.is_task_running() {
let message = format!(
"'/{}' is disabled while a task is in progress.",
cmd.command()
);
self.add_to_history(history_cell::new_error_event(message));
self.request_redraw();
return;
}
let trimmed = args.trim();
match cmd {
SlashCommand::Review if !trimmed.is_empty() => {
self.submit_op(Op::Review {
review_request: ReviewRequest {
target: ReviewTarget::Custom {
instructions: trimmed.to_string(),
},
user_facing_hint: None,
},
});
}
_ => self.dispatch_command(cmd),
}
}
pub(crate) fn handle_paste(&mut self, text: String) {
self.bottom_pane.handle_paste(text);
}