diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 4b7439416a..efb3173e52 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -4876,7 +4876,7 @@ impl ChatWidget { }; self.app_event_tx .send(AppEvent::BeginWindowsSandboxGrantReadRoot { - path: prepared_args.path.text, + path: prepared_args.path, }); self.bottom_pane.drain_pending_submission_state(); } diff --git a/codex-rs/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index 3a9369d148..a0fb3e7825 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/codex-rs/tui/src/slash_command.rs @@ -11,8 +11,10 @@ use crate::slash_command_protocol::SlashCommandParseInput; use crate::slash_command_protocol::SlashCommandUsageErrorKind; use crate::slash_command_protocol::SlashSerializedText; pub(crate) use crate::slash_command_protocol::SlashTextArg; -use crate::slash_command_protocol::SlashTokenArg; -use crate::slash_command_protocol::SlashTokenValue; +use crate::slash_command_protocol::enum_choice; +use crate::slash_command_protocol::from_str_value; +use crate::slash_command_protocol::string; +use crate::slash_command_protocol::text; /// Commands that can be invoked by starting a message with a leading slash. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, EnumString, AsRefStr, IntoStaticStr)] @@ -104,62 +106,19 @@ pub(crate) enum FastSlashCommandArgs { Status, } -impl SlashTokenValue for FastSlashCommandArgs { - fn parse_token(token: SlashTokenArg) -> Result { - match token.text.as_str() { - value if value.eq_ignore_ascii_case("on") => Ok(Self::On), - value if value.eq_ignore_ascii_case("off") => Ok(Self::Off), - value if value.eq_ignore_ascii_case("status") => Ok(Self::Status), - _ => Err(SlashCommandUsageErrorKind::InvalidInlineArgs), - } - } +const FAST_MODE_CHOICES: &[(&str, FastSlashCommandArgs)] = &[ + ("on", FastSlashCommandArgs::On), + ("off", FastSlashCommandArgs::Off), + ("status", FastSlashCommandArgs::Status), +]; - fn serialize_token(&self) -> SlashTokenArg { - let text = match self { - Self::On => "on", - Self::Off => "off", - Self::Status => "status", - }; - SlashTokenArg::new(text.to_string(), Vec::new()) - } -} - -impl SlashTokenValue for FeedbackCategory { - fn parse_token(token: SlashTokenArg) -> Result { - match token.text.as_str() { - "bad-result" => Ok(Self::BadResult), - "good-result" => Ok(Self::GoodResult), - "bug" => Ok(Self::Bug), - "safety-check" => Ok(Self::SafetyCheck), - "other" => Ok(Self::Other), - _ => Err(SlashCommandUsageErrorKind::InvalidInlineArgs), - } - } - - fn serialize_token(&self) -> SlashTokenArg { - let text = match self { - Self::BadResult => "bad-result", - Self::GoodResult => "good-result", - Self::Bug => "bug", - Self::SafetyCheck => "safety-check", - Self::Other => "other", - }; - SlashTokenArg::new(text.to_string(), Vec::new()) - } -} - -impl SlashTokenValue for StatusLineItem { - fn parse_token(token: SlashTokenArg) -> Result { - token - .text - .parse() - .map_err(|_| SlashCommandUsageErrorKind::InvalidInlineArgs) - } - - fn serialize_token(&self) -> SlashTokenArg { - SlashTokenArg::new(self.to_string(), Vec::new()) - } -} +const FEEDBACK_CATEGORY_CHOICES: &[(&str, FeedbackCategory)] = &[ + ("bad-result", FeedbackCategory::BadResult), + ("good-result", FeedbackCategory::GoodResult), + ("bug", FeedbackCategory::Bug), + ("safety-check", FeedbackCategory::SafetyCheck), + ("other", FeedbackCategory::Other), +]; pub(crate) trait SlashCommandInlineArgs: SlashCommandArgs + Sized { const USAGE_LINES: &'static [&'static str]; @@ -175,14 +134,14 @@ pub(crate) struct FastArgs { impl SlashCommandArgs for FastArgs { fn parse(input: SlashCommandParseInput<'_>) -> Result { let mut parser = SlashArgsParser::new(input)?; - let mode = parser.positional::()?; + let mode = parser.positional(&enum_choice(FAST_MODE_CHOICES).ascii_case_insensitive())?; parser.finish()?; Ok(Self { mode }) } fn serialize(&self) -> SlashSerializedText { let mut serializer = SlashArgsSerializer::default(); - serializer.positional(&self.mode); + serializer.positional(&self.mode, &enum_choice(FAST_MODE_CHOICES)); serializer.finish() } } @@ -203,13 +162,13 @@ pub(crate) struct RenameArgs { impl SlashCommandArgs for RenameArgs { fn parse(input: SlashCommandParseInput<'_>) -> Result { let parser = SlashArgsParser::new(input)?; - let title = parser.required_remainder()?; + let title = parser.required_remainder(&text())?; Ok(Self { title }) } fn serialize(&self) -> SlashSerializedText { let mut serializer = SlashArgsSerializer::default(); - serializer.remainder(&self.title); + serializer.remainder(&self.title, &text()); serializer.finish() } } @@ -230,13 +189,13 @@ pub(crate) struct PlanArgs { impl SlashCommandArgs for PlanArgs { fn parse(input: SlashCommandParseInput<'_>) -> Result { let parser = SlashArgsParser::new(input)?; - let prompt = parser.required_remainder()?; + let prompt = parser.required_remainder(&text())?; Ok(Self { prompt }) } fn serialize(&self) -> SlashSerializedText { let mut serializer = SlashArgsSerializer::default(); - serializer.remainder(&self.prompt); + serializer.remainder(&self.prompt, &text()); serializer.finish() } } @@ -257,13 +216,13 @@ pub(crate) struct ReviewArgs { impl SlashCommandArgs for ReviewArgs { fn parse(input: SlashCommandParseInput<'_>) -> Result { let parser = SlashArgsParser::new(input)?; - let instructions = parser.required_remainder()?; + let instructions = parser.required_remainder(&text())?; Ok(Self { instructions }) } fn serialize(&self) -> SlashSerializedText { let mut serializer = SlashArgsSerializer::default(); - serializer.remainder(&self.instructions); + serializer.remainder(&self.instructions, &text()); serializer.finish() } } @@ -278,15 +237,15 @@ impl SlashCommandInlineArgs for ReviewArgs { #[derive(Debug, Clone, PartialEq)] pub(crate) struct SandboxReadRootArgs { - pub(crate) path: SlashTokenArg, + pub(crate) path: String, } impl SlashCommandArgs for SandboxReadRootArgs { fn parse(input: SlashCommandParseInput<'_>) -> Result { let mut parser = SlashArgsParser::new(input)?; - let path = match parser.named::("path")? { + let path = match parser.named("path", &string())? { Some(path) => path, - None => parser.positional::()?, + None => parser.positional(&string())?, }; parser.finish()?; Ok(Self { path }) @@ -294,7 +253,7 @@ impl SlashCommandArgs for SandboxReadRootArgs { fn serialize(&self) -> SlashSerializedText { let mut serializer = SlashArgsSerializer::default(); - serializer.positional(&self.path); + serializer.positional(&self.path, &string()); serializer.finish() } } @@ -318,14 +277,14 @@ pub(crate) struct FeedbackArgs { impl SlashCommandArgs for FeedbackArgs { fn parse(input: SlashCommandParseInput<'_>) -> Result { let mut parser = SlashArgsParser::new(input)?; - let category = parser.positional::()?; + let category = parser.positional(&enum_choice(FEEDBACK_CATEGORY_CHOICES))?; parser.finish()?; Ok(Self { category }) } fn serialize(&self) -> SlashSerializedText { let mut serializer = SlashArgsSerializer::default(); - serializer.positional(&self.category); + serializer.positional(&self.category, &enum_choice(FEEDBACK_CATEGORY_CHOICES)); serializer.finish() } } @@ -349,7 +308,7 @@ pub(crate) struct StatuslineArgs { impl SlashCommandArgs for StatuslineArgs { fn parse(input: SlashCommandParseInput<'_>) -> Result { let mut parser = SlashArgsParser::new(input)?; - let items = parser.positional_list::()?; + let items = parser.positional_list(&from_str_value::())?; parser.finish()?; if items.is_empty() { return Err(SlashCommandUsageErrorKind::InvalidInlineArgs); @@ -359,7 +318,10 @@ impl SlashCommandArgs for StatuslineArgs { fn serialize(&self) -> SlashSerializedText { let mut serializer = SlashArgsSerializer::default(); - serializer.list::(self.items.iter().cloned()); + serializer.list( + self.items.iter().cloned(), + &from_str_value::(), + ); serializer.finish() } } @@ -1197,10 +1159,8 @@ mod tests { prop_oneof![plain_text_arg_strategy(), placeholder_text_arg_strategy(),].boxed() } - fn token_arg_strategy() -> BoxedStrategy { - token_text_strategy() - .prop_map(|text| SlashTokenArg::new(text, Vec::new())) - .boxed() + fn string_arg_strategy() -> BoxedStrategy { + token_text_strategy().boxed() } impl SlashCommand { @@ -1222,7 +1182,7 @@ mod tests { SlashCommand::SandboxReadRoot => vec![ bare, SlashCommandInvocation::SandboxReadRoot(SandboxReadRootArgs { - path: SlashTokenArg::new("/tmp/test-dir".to_string(), Vec::new()), + path: "/tmp/test-dir".to_string(), }), ], SlashCommand::Review => vec![ @@ -1322,7 +1282,7 @@ mod tests { .boxed(), SlashCommand::SandboxReadRoot => prop_oneof![ bare, - token_arg_strategy().prop_map(|path| { + string_arg_strategy().prop_map(|path| { SlashCommandInvocation::SandboxReadRoot(SandboxReadRootArgs { path }) }), ] @@ -1558,7 +1518,7 @@ mod tests { assert_eq!( invocation, SlashCommandInvocation::SandboxReadRoot(SandboxReadRootArgs { - path: SlashTokenArg::new("/tmp/test dir".to_string(), Vec::new()), + path: "/tmp/test dir".to_string(), }) ); assert_eq!( diff --git a/codex-rs/tui/src/slash_command_protocol.rs b/codex-rs/tui/src/slash_command_protocol.rs index de7939502d..4b585d9e6f 100644 --- a/codex-rs/tui/src/slash_command_protocol.rs +++ b/codex-rs/tui/src/slash_command_protocol.rs @@ -1,4 +1,6 @@ use std::collections::HashMap; +use std::marker::PhantomData; +use std::str::FromStr; use codex_protocol::user_input::ByteRange; use codex_protocol::user_input::TextElement; @@ -89,28 +91,153 @@ impl SlashTextArg { } } -pub(crate) trait SlashTokenValue: Sized { - fn parse_token(token: SlashTokenArg) -> Result; - fn serialize_token(&self) -> SlashTokenArg; +pub(crate) trait SlashTokenValueSpec { + fn parse_token(&self, token: SlashTokenArg) -> Result; + fn serialize_token(&self, value: &T) -> SlashTokenArg; } -impl SlashTokenValue for SlashTokenArg { - fn parse_token(token: SlashTokenArg) -> Result { +pub(crate) trait SlashTextValueSpec { + fn parse_text(&self, text: SlashTextArg) -> Result; + fn serialize_text(&self, value: &T) -> SlashTextArg; +} + +#[allow(dead_code)] +#[derive(Debug, Clone, Copy)] +pub(crate) struct SlashTokenSpec; + +#[allow(dead_code)] +pub(crate) fn token() -> SlashTokenSpec { + SlashTokenSpec +} + +impl SlashTokenValueSpec for SlashTokenSpec { + fn parse_token( + &self, + token: SlashTokenArg, + ) -> Result { Ok(token) } - fn serialize_token(&self) -> SlashTokenArg { - self.clone() + fn serialize_token(&self, value: &SlashTokenArg) -> SlashTokenArg { + value.clone() } } -impl SlashTokenValue for String { - fn parse_token(token: SlashTokenArg) -> Result { +#[derive(Debug, Clone, Copy)] +pub(crate) struct SlashStringSpec; + +pub(crate) fn string() -> SlashStringSpec { + SlashStringSpec +} + +impl SlashTokenValueSpec for SlashStringSpec { + fn parse_token(&self, token: SlashTokenArg) -> Result { Ok(token.text) } - fn serialize_token(&self) -> SlashTokenArg { - SlashTokenArg::new(self.clone(), Vec::new()) + fn serialize_token(&self, value: &String) -> SlashTokenArg { + SlashTokenArg::new(value.clone(), Vec::new()) + } +} + +#[derive(Debug, Clone, Copy)] +pub(crate) struct SlashTextSpec; + +pub(crate) fn text() -> SlashTextSpec { + SlashTextSpec +} + +impl SlashTextValueSpec for SlashTextSpec { + fn parse_text(&self, text: SlashTextArg) -> Result { + Ok(text) + } + + fn serialize_text(&self, value: &SlashTextArg) -> SlashTextArg { + value.clone() + } +} + +#[derive(Debug, Clone, Copy)] +pub(crate) struct SlashEnumChoiceSpec { + choices: &'static [(&'static str, T)], + ascii_case_insensitive: bool, +} + +pub(crate) fn enum_choice(choices: &'static [(&'static str, T)]) -> SlashEnumChoiceSpec +where + T: Clone + PartialEq + 'static, +{ + SlashEnumChoiceSpec { + choices, + ascii_case_insensitive: false, + } +} + +impl SlashEnumChoiceSpec { + pub(crate) fn ascii_case_insensitive(mut self) -> Self { + self.ascii_case_insensitive = true; + self + } +} + +impl SlashTokenValueSpec for SlashEnumChoiceSpec +where + T: Clone + PartialEq + 'static, +{ + fn parse_token(&self, token: SlashTokenArg) -> Result { + self.choices + .iter() + .find_map(|(literal, value)| { + let matches = if self.ascii_case_insensitive { + token.text.eq_ignore_ascii_case(literal) + } else { + token.text == *literal + }; + matches.then(|| value.clone()) + }) + .ok_or(SlashCommandUsageErrorKind::InvalidInlineArgs) + } + + fn serialize_token(&self, value: &T) -> SlashTokenArg { + let literal = match self + .choices + .iter() + .find_map(|(literal, choice)| (choice == value).then_some(*literal)) + { + Some(literal) => literal, + None => panic!("missing enum choice serializer mapping"), + }; + SlashTokenArg::new(literal.to_string(), Vec::new()) + } +} + +#[derive(Debug, Clone, Copy)] +pub(crate) struct SlashFromStrSpec { + _phantom: PhantomData, +} + +pub(crate) fn from_str_value() -> SlashFromStrSpec +where + T: FromStr + ToString, +{ + SlashFromStrSpec { + _phantom: PhantomData, + } +} + +impl SlashTokenValueSpec for SlashFromStrSpec +where + T: FromStr + ToString, +{ + fn parse_token(&self, token: SlashTokenArg) -> Result { + token + .text + .parse() + .map_err(|_| SlashCommandUsageErrorKind::InvalidInlineArgs) + } + + fn serialize_token(&self, value: &T) -> SlashTokenArg { + SlashTokenArg::new(value.to_string(), Vec::new()) } } @@ -157,48 +284,55 @@ impl<'a> SlashArgsParser<'a> { }) } - pub(crate) fn positional(&mut self) -> Result + pub(crate) fn positional(&mut self, spec: &S) -> Result where - T: SlashTokenValue, + S: SlashTokenValueSpec, { let Some(token) = self.positionals.get(self.next_positional).cloned() else { return Err(SlashCommandUsageErrorKind::InvalidInlineArgs); }; self.next_positional += 1; - T::parse_token(token) + spec.parse_token(token) } #[allow(dead_code)] - pub(crate) fn optional_positional(&mut self) -> Result, SlashCommandUsageErrorKind> + pub(crate) fn optional_positional( + &mut self, + spec: &S, + ) -> Result, SlashCommandUsageErrorKind> where - T: SlashTokenValue, + S: SlashTokenValueSpec, { if self.next_positional >= self.positionals.len() { Ok(None) } else { - self.positional().map(Some) + self.positional(spec).map(Some) } } #[cfg_attr(not(test), allow(dead_code))] - pub(crate) fn positional_list(&mut self) -> Result, SlashCommandUsageErrorKind> + pub(crate) fn positional_list( + &mut self, + spec: &S, + ) -> Result, SlashCommandUsageErrorKind> where - T: SlashTokenValue, + S: SlashTokenValueSpec, { let mut values = Vec::new(); while self.next_positional < self.positionals.len() { - values.push(self.positional()?); + values.push(self.positional(spec)?); } Ok(values) } #[cfg_attr(not(test), allow(dead_code))] - pub(crate) fn named( + pub(crate) fn named( &mut self, key: &'static str, + spec: &S, ) -> Result, SlashCommandUsageErrorKind> where - T: SlashTokenValue, + S: SlashTokenValueSpec, { if self.duplicates.contains_key(key) { return Err(SlashCommandUsageErrorKind::InvalidInlineArgs); @@ -206,15 +340,23 @@ impl<'a> SlashArgsParser<'a> { let Some(value) = self.named.remove(key) else { return Ok(None); }; - T::parse_token(value).map(Some) + spec.parse_token(value).map(Some) } - pub(crate) fn remainder(&self) -> Option { + pub(crate) fn remainder(&self, spec: &S) -> Result, SlashCommandUsageErrorKind> + where + S: SlashTextValueSpec, + { parse_remainder_text_arg(self.input.args, self.input.text_elements) + .map(|value| spec.parse_text(value)) + .transpose() } - pub(crate) fn required_remainder(&self) -> Result { - self.remainder() + pub(crate) fn required_remainder(&self, spec: &S) -> Result + where + S: SlashTextValueSpec, + { + self.remainder(spec)? .ok_or(SlashCommandUsageErrorKind::InvalidInlineArgs) } @@ -235,45 +377,49 @@ pub(crate) struct SlashArgsSerializer { } impl SlashArgsSerializer { - pub(crate) fn positional(&mut self, value: &T) + pub(crate) fn positional(&mut self, value: &T, spec: &S) where - T: SlashTokenValue, + S: SlashTokenValueSpec, { self.fragments - .push(serialize_token(&value.serialize_token())); + .push(serialize_token(&spec.serialize_token(value))); } #[cfg_attr(not(test), allow(dead_code))] - pub(crate) fn list(&mut self, values: I) + pub(crate) fn list(&mut self, values: I, spec: &S) where - T: SlashTokenValue, I: IntoIterator, + S: SlashTokenValueSpec, { for value in values { - self.positional(&value); + self.positional(&value, spec); } } #[allow(dead_code)] - pub(crate) fn named(&mut self, key: &'static str, value: &T) + pub(crate) fn named(&mut self, key: &'static str, value: &T, spec: &S) where - T: SlashTokenValue, + S: SlashTokenValueSpec, { - let serialized_value = serialize_token(&value.serialize_token()); + let serialized_value = serialize_token(&spec.serialize_token(value)); self.fragments .push(serialized_value.prepend_inline(&format!("--{key}="))); } - pub(crate) fn remainder(&mut self, value: &SlashTextArg) { - if remainder_can_roundtrip_raw(value) { + pub(crate) fn remainder(&mut self, value: &T, spec: &S) + where + S: SlashTextValueSpec, + { + let serialized = spec.serialize_text(value); + if remainder_can_roundtrip_raw(&serialized) { self.fragments.push(SlashSerializedText { - text: value.text.clone(), - text_elements: value.text_elements.clone(), + text: serialized.text.clone(), + text_elements: serialized.text_elements, }); } else { self.fragments.push(serialize_token(&SlashTokenArg::new( - value.text.clone(), - value.text_elements.clone(), + serialized.text.clone(), + serialized.text_elements, ))); } } @@ -554,23 +700,7 @@ mod tests { Off, } - impl SlashTokenValue for Switch { - fn parse_token(token: SlashTokenArg) -> Result { - match token.text.as_str() { - "on" => Ok(Self::On), - "off" => Ok(Self::Off), - _ => Err(SlashCommandUsageErrorKind::InvalidInlineArgs), - } - } - - fn serialize_token(&self) -> SlashTokenArg { - let text = match self { - Self::On => "on", - Self::Off => "off", - }; - SlashTokenArg::new(text.to_string(), Vec::new()) - } - } + const SWITCH_CHOICES: &[(&str, Switch)] = &[("on", Switch::On), ("off", Switch::Off)]; #[test] fn parser_supports_positional_list_and_named_args() { @@ -580,13 +710,16 @@ mod tests { }) .unwrap(); - assert_eq!(parser.positional::(), Ok(Switch::On)); assert_eq!( - parser.positional_list::(), + parser.positional(&enum_choice(SWITCH_CHOICES)), + Ok(Switch::On) + ); + assert_eq!( + parser.positional_list(&string()), Ok(vec!["first".to_string(), "second".to_string()]) ); assert_eq!( - parser.named::("path"), + parser.named("path", &string()), Ok(Some("some dir".to_string())) ); assert_eq!(parser.finish(), Ok(())); @@ -600,17 +733,20 @@ mod tests { }) .unwrap(); - assert_eq!(parser.positional::(), Ok(Switch::On)); - assert_eq!(parser.optional_positional::(), Ok(None)); + assert_eq!( + parser.positional(&enum_choice(SWITCH_CHOICES)), + Ok(Switch::On) + ); + assert_eq!(parser.optional_positional(&string()), Ok(None)); assert_eq!(parser.finish(), Ok(())); } #[test] fn serializer_stably_formats_named_args_after_positionals() { let mut serializer = SlashArgsSerializer::default(); - serializer.positional(&Switch::On); - serializer.list::(["first".to_string(), "second".to_string()]); - serializer.named("path", &"some dir".to_string()); + serializer.positional(&Switch::On, &enum_choice(SWITCH_CHOICES)); + serializer.list(["first".to_string(), "second".to_string()], &string()); + serializer.named("path", &"some dir".to_string(), &string()); assert_eq!( serializer.finish(), @@ -629,7 +765,7 @@ mod tests { vec![TextElement::new((7..18).into(), Some(placeholder.clone()))], ); let mut serializer = SlashArgsSerializer::default(); - serializer.remainder(&prompt); + serializer.remainder(&prompt, &text()); assert_eq!( serializer.finish(), @@ -644,7 +780,7 @@ mod tests { fn remainder_quotes_shell_sensitive_text_when_needed() { let prompt = SlashTextArg::new("a\"\" a\"".to_string(), Vec::new()); let mut serializer = SlashArgsSerializer::default(); - serializer.remainder(&prompt); + serializer.remainder(&prompt, &text()); assert_eq!( serializer.finish(), @@ -659,7 +795,7 @@ mod tests { text_elements: &[], }) .unwrap() - .required_remainder(), + .required_remainder(&text()), Ok(prompt) ); } diff --git a/codex-rs/tui_app_server/src/chatwidget.rs b/codex-rs/tui_app_server/src/chatwidget.rs index 6991b6c755..1e3d249e80 100644 --- a/codex-rs/tui_app_server/src/chatwidget.rs +++ b/codex-rs/tui_app_server/src/chatwidget.rs @@ -5020,7 +5020,7 @@ impl ChatWidget { }; self.app_event_tx .send(AppEvent::BeginWindowsSandboxGrantReadRoot { - path: prepared_args.path.text, + path: prepared_args.path, }); self.bottom_pane.drain_pending_submission_state(); } diff --git a/codex-rs/tui_app_server/src/slash_command.rs b/codex-rs/tui_app_server/src/slash_command.rs index d70011bebf..b5bd8f0900 100644 --- a/codex-rs/tui_app_server/src/slash_command.rs +++ b/codex-rs/tui_app_server/src/slash_command.rs @@ -11,8 +11,10 @@ use crate::slash_command_protocol::SlashCommandParseInput; use crate::slash_command_protocol::SlashCommandUsageErrorKind; use crate::slash_command_protocol::SlashSerializedText; pub(crate) use crate::slash_command_protocol::SlashTextArg; -use crate::slash_command_protocol::SlashTokenArg; -use crate::slash_command_protocol::SlashTokenValue; +use crate::slash_command_protocol::enum_choice; +use crate::slash_command_protocol::from_str_value; +use crate::slash_command_protocol::string; +use crate::slash_command_protocol::text; /// Commands that can be invoked by starting a message with a leading slash. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, EnumString, AsRefStr, IntoStaticStr)] @@ -103,62 +105,19 @@ pub(crate) enum FastSlashCommandArgs { Status, } -impl SlashTokenValue for FastSlashCommandArgs { - fn parse_token(token: SlashTokenArg) -> Result { - match token.text.as_str() { - value if value.eq_ignore_ascii_case("on") => Ok(Self::On), - value if value.eq_ignore_ascii_case("off") => Ok(Self::Off), - value if value.eq_ignore_ascii_case("status") => Ok(Self::Status), - _ => Err(SlashCommandUsageErrorKind::InvalidInlineArgs), - } - } +const FAST_MODE_CHOICES: &[(&str, FastSlashCommandArgs)] = &[ + ("on", FastSlashCommandArgs::On), + ("off", FastSlashCommandArgs::Off), + ("status", FastSlashCommandArgs::Status), +]; - fn serialize_token(&self) -> SlashTokenArg { - let text = match self { - Self::On => "on", - Self::Off => "off", - Self::Status => "status", - }; - SlashTokenArg::new(text.to_string(), Vec::new()) - } -} - -impl SlashTokenValue for FeedbackCategory { - fn parse_token(token: SlashTokenArg) -> Result { - match token.text.as_str() { - "bad-result" => Ok(Self::BadResult), - "good-result" => Ok(Self::GoodResult), - "bug" => Ok(Self::Bug), - "safety-check" => Ok(Self::SafetyCheck), - "other" => Ok(Self::Other), - _ => Err(SlashCommandUsageErrorKind::InvalidInlineArgs), - } - } - - fn serialize_token(&self) -> SlashTokenArg { - let text = match self { - Self::BadResult => "bad-result", - Self::GoodResult => "good-result", - Self::Bug => "bug", - Self::SafetyCheck => "safety-check", - Self::Other => "other", - }; - SlashTokenArg::new(text.to_string(), Vec::new()) - } -} - -impl SlashTokenValue for StatusLineItem { - fn parse_token(token: SlashTokenArg) -> Result { - token - .text - .parse() - .map_err(|_| SlashCommandUsageErrorKind::InvalidInlineArgs) - } - - fn serialize_token(&self) -> SlashTokenArg { - SlashTokenArg::new(self.to_string(), Vec::new()) - } -} +const FEEDBACK_CATEGORY_CHOICES: &[(&str, FeedbackCategory)] = &[ + ("bad-result", FeedbackCategory::BadResult), + ("good-result", FeedbackCategory::GoodResult), + ("bug", FeedbackCategory::Bug), + ("safety-check", FeedbackCategory::SafetyCheck), + ("other", FeedbackCategory::Other), +]; pub(crate) trait SlashCommandInlineArgs: SlashCommandArgs + Sized { const USAGE_LINES: &'static [&'static str]; @@ -174,14 +133,14 @@ pub(crate) struct FastArgs { impl SlashCommandArgs for FastArgs { fn parse(input: SlashCommandParseInput<'_>) -> Result { let mut parser = SlashArgsParser::new(input)?; - let mode = parser.positional::()?; + let mode = parser.positional(&enum_choice(FAST_MODE_CHOICES).ascii_case_insensitive())?; parser.finish()?; Ok(Self { mode }) } fn serialize(&self) -> SlashSerializedText { let mut serializer = SlashArgsSerializer::default(); - serializer.positional(&self.mode); + serializer.positional(&self.mode, &enum_choice(FAST_MODE_CHOICES)); serializer.finish() } } @@ -202,13 +161,13 @@ pub(crate) struct RenameArgs { impl SlashCommandArgs for RenameArgs { fn parse(input: SlashCommandParseInput<'_>) -> Result { let parser = SlashArgsParser::new(input)?; - let title = parser.required_remainder()?; + let title = parser.required_remainder(&text())?; Ok(Self { title }) } fn serialize(&self) -> SlashSerializedText { let mut serializer = SlashArgsSerializer::default(); - serializer.remainder(&self.title); + serializer.remainder(&self.title, &text()); serializer.finish() } } @@ -229,13 +188,13 @@ pub(crate) struct PlanArgs { impl SlashCommandArgs for PlanArgs { fn parse(input: SlashCommandParseInput<'_>) -> Result { let parser = SlashArgsParser::new(input)?; - let prompt = parser.required_remainder()?; + let prompt = parser.required_remainder(&text())?; Ok(Self { prompt }) } fn serialize(&self) -> SlashSerializedText { let mut serializer = SlashArgsSerializer::default(); - serializer.remainder(&self.prompt); + serializer.remainder(&self.prompt, &text()); serializer.finish() } } @@ -256,13 +215,13 @@ pub(crate) struct ReviewArgs { impl SlashCommandArgs for ReviewArgs { fn parse(input: SlashCommandParseInput<'_>) -> Result { let parser = SlashArgsParser::new(input)?; - let instructions = parser.required_remainder()?; + let instructions = parser.required_remainder(&text())?; Ok(Self { instructions }) } fn serialize(&self) -> SlashSerializedText { let mut serializer = SlashArgsSerializer::default(); - serializer.remainder(&self.instructions); + serializer.remainder(&self.instructions, &text()); serializer.finish() } } @@ -277,15 +236,15 @@ impl SlashCommandInlineArgs for ReviewArgs { #[derive(Debug, Clone, PartialEq)] pub(crate) struct SandboxReadRootArgs { - pub(crate) path: SlashTokenArg, + pub(crate) path: String, } impl SlashCommandArgs for SandboxReadRootArgs { fn parse(input: SlashCommandParseInput<'_>) -> Result { let mut parser = SlashArgsParser::new(input)?; - let path = match parser.named::("path")? { + let path = match parser.named("path", &string())? { Some(path) => path, - None => parser.positional::()?, + None => parser.positional(&string())?, }; parser.finish()?; Ok(Self { path }) @@ -293,7 +252,7 @@ impl SlashCommandArgs for SandboxReadRootArgs { fn serialize(&self) -> SlashSerializedText { let mut serializer = SlashArgsSerializer::default(); - serializer.positional(&self.path); + serializer.positional(&self.path, &string()); serializer.finish() } } @@ -317,14 +276,14 @@ pub(crate) struct FeedbackArgs { impl SlashCommandArgs for FeedbackArgs { fn parse(input: SlashCommandParseInput<'_>) -> Result { let mut parser = SlashArgsParser::new(input)?; - let category = parser.positional::()?; + let category = parser.positional(&enum_choice(FEEDBACK_CATEGORY_CHOICES))?; parser.finish()?; Ok(Self { category }) } fn serialize(&self) -> SlashSerializedText { let mut serializer = SlashArgsSerializer::default(); - serializer.positional(&self.category); + serializer.positional(&self.category, &enum_choice(FEEDBACK_CATEGORY_CHOICES)); serializer.finish() } } @@ -348,7 +307,7 @@ pub(crate) struct StatuslineArgs { impl SlashCommandArgs for StatuslineArgs { fn parse(input: SlashCommandParseInput<'_>) -> Result { let mut parser = SlashArgsParser::new(input)?; - let items = parser.positional_list::()?; + let items = parser.positional_list(&from_str_value::())?; parser.finish()?; if items.is_empty() { return Err(SlashCommandUsageErrorKind::InvalidInlineArgs); @@ -358,7 +317,10 @@ impl SlashCommandArgs for StatuslineArgs { fn serialize(&self) -> SlashSerializedText { let mut serializer = SlashArgsSerializer::default(); - serializer.list::(self.items.iter().cloned()); + serializer.list( + self.items.iter().cloned(), + &from_str_value::(), + ); serializer.finish() } } @@ -1183,10 +1145,8 @@ mod tests { prop_oneof![plain_text_arg_strategy(), placeholder_text_arg_strategy(),].boxed() } - fn token_arg_strategy() -> BoxedStrategy { - token_text_strategy() - .prop_map(|text| SlashTokenArg::new(text, Vec::new())) - .boxed() + fn string_arg_strategy() -> BoxedStrategy { + token_text_strategy().boxed() } impl SlashCommand { @@ -1208,7 +1168,7 @@ mod tests { SlashCommand::SandboxReadRoot => vec![ bare, SlashCommandInvocation::SandboxReadRoot(SandboxReadRootArgs { - path: SlashTokenArg::new("/tmp/test-dir".to_string(), Vec::new()), + path: "/tmp/test-dir".to_string(), }), ], SlashCommand::Review => vec![ @@ -1307,7 +1267,7 @@ mod tests { .boxed(), SlashCommand::SandboxReadRoot => prop_oneof![ bare, - token_arg_strategy().prop_map(|path| { + string_arg_strategy().prop_map(|path| { SlashCommandInvocation::SandboxReadRoot(SandboxReadRootArgs { path }) }), ] @@ -1542,7 +1502,7 @@ mod tests { assert_eq!( invocation, SlashCommandInvocation::SandboxReadRoot(SandboxReadRootArgs { - path: SlashTokenArg::new("/tmp/test dir".to_string(), Vec::new()), + path: "/tmp/test dir".to_string(), }) ); assert_eq!( diff --git a/codex-rs/tui_app_server/src/slash_command_protocol.rs b/codex-rs/tui_app_server/src/slash_command_protocol.rs index de7939502d..4b585d9e6f 100644 --- a/codex-rs/tui_app_server/src/slash_command_protocol.rs +++ b/codex-rs/tui_app_server/src/slash_command_protocol.rs @@ -1,4 +1,6 @@ use std::collections::HashMap; +use std::marker::PhantomData; +use std::str::FromStr; use codex_protocol::user_input::ByteRange; use codex_protocol::user_input::TextElement; @@ -89,28 +91,153 @@ impl SlashTextArg { } } -pub(crate) trait SlashTokenValue: Sized { - fn parse_token(token: SlashTokenArg) -> Result; - fn serialize_token(&self) -> SlashTokenArg; +pub(crate) trait SlashTokenValueSpec { + fn parse_token(&self, token: SlashTokenArg) -> Result; + fn serialize_token(&self, value: &T) -> SlashTokenArg; } -impl SlashTokenValue for SlashTokenArg { - fn parse_token(token: SlashTokenArg) -> Result { +pub(crate) trait SlashTextValueSpec { + fn parse_text(&self, text: SlashTextArg) -> Result; + fn serialize_text(&self, value: &T) -> SlashTextArg; +} + +#[allow(dead_code)] +#[derive(Debug, Clone, Copy)] +pub(crate) struct SlashTokenSpec; + +#[allow(dead_code)] +pub(crate) fn token() -> SlashTokenSpec { + SlashTokenSpec +} + +impl SlashTokenValueSpec for SlashTokenSpec { + fn parse_token( + &self, + token: SlashTokenArg, + ) -> Result { Ok(token) } - fn serialize_token(&self) -> SlashTokenArg { - self.clone() + fn serialize_token(&self, value: &SlashTokenArg) -> SlashTokenArg { + value.clone() } } -impl SlashTokenValue for String { - fn parse_token(token: SlashTokenArg) -> Result { +#[derive(Debug, Clone, Copy)] +pub(crate) struct SlashStringSpec; + +pub(crate) fn string() -> SlashStringSpec { + SlashStringSpec +} + +impl SlashTokenValueSpec for SlashStringSpec { + fn parse_token(&self, token: SlashTokenArg) -> Result { Ok(token.text) } - fn serialize_token(&self) -> SlashTokenArg { - SlashTokenArg::new(self.clone(), Vec::new()) + fn serialize_token(&self, value: &String) -> SlashTokenArg { + SlashTokenArg::new(value.clone(), Vec::new()) + } +} + +#[derive(Debug, Clone, Copy)] +pub(crate) struct SlashTextSpec; + +pub(crate) fn text() -> SlashTextSpec { + SlashTextSpec +} + +impl SlashTextValueSpec for SlashTextSpec { + fn parse_text(&self, text: SlashTextArg) -> Result { + Ok(text) + } + + fn serialize_text(&self, value: &SlashTextArg) -> SlashTextArg { + value.clone() + } +} + +#[derive(Debug, Clone, Copy)] +pub(crate) struct SlashEnumChoiceSpec { + choices: &'static [(&'static str, T)], + ascii_case_insensitive: bool, +} + +pub(crate) fn enum_choice(choices: &'static [(&'static str, T)]) -> SlashEnumChoiceSpec +where + T: Clone + PartialEq + 'static, +{ + SlashEnumChoiceSpec { + choices, + ascii_case_insensitive: false, + } +} + +impl SlashEnumChoiceSpec { + pub(crate) fn ascii_case_insensitive(mut self) -> Self { + self.ascii_case_insensitive = true; + self + } +} + +impl SlashTokenValueSpec for SlashEnumChoiceSpec +where + T: Clone + PartialEq + 'static, +{ + fn parse_token(&self, token: SlashTokenArg) -> Result { + self.choices + .iter() + .find_map(|(literal, value)| { + let matches = if self.ascii_case_insensitive { + token.text.eq_ignore_ascii_case(literal) + } else { + token.text == *literal + }; + matches.then(|| value.clone()) + }) + .ok_or(SlashCommandUsageErrorKind::InvalidInlineArgs) + } + + fn serialize_token(&self, value: &T) -> SlashTokenArg { + let literal = match self + .choices + .iter() + .find_map(|(literal, choice)| (choice == value).then_some(*literal)) + { + Some(literal) => literal, + None => panic!("missing enum choice serializer mapping"), + }; + SlashTokenArg::new(literal.to_string(), Vec::new()) + } +} + +#[derive(Debug, Clone, Copy)] +pub(crate) struct SlashFromStrSpec { + _phantom: PhantomData, +} + +pub(crate) fn from_str_value() -> SlashFromStrSpec +where + T: FromStr + ToString, +{ + SlashFromStrSpec { + _phantom: PhantomData, + } +} + +impl SlashTokenValueSpec for SlashFromStrSpec +where + T: FromStr + ToString, +{ + fn parse_token(&self, token: SlashTokenArg) -> Result { + token + .text + .parse() + .map_err(|_| SlashCommandUsageErrorKind::InvalidInlineArgs) + } + + fn serialize_token(&self, value: &T) -> SlashTokenArg { + SlashTokenArg::new(value.to_string(), Vec::new()) } } @@ -157,48 +284,55 @@ impl<'a> SlashArgsParser<'a> { }) } - pub(crate) fn positional(&mut self) -> Result + pub(crate) fn positional(&mut self, spec: &S) -> Result where - T: SlashTokenValue, + S: SlashTokenValueSpec, { let Some(token) = self.positionals.get(self.next_positional).cloned() else { return Err(SlashCommandUsageErrorKind::InvalidInlineArgs); }; self.next_positional += 1; - T::parse_token(token) + spec.parse_token(token) } #[allow(dead_code)] - pub(crate) fn optional_positional(&mut self) -> Result, SlashCommandUsageErrorKind> + pub(crate) fn optional_positional( + &mut self, + spec: &S, + ) -> Result, SlashCommandUsageErrorKind> where - T: SlashTokenValue, + S: SlashTokenValueSpec, { if self.next_positional >= self.positionals.len() { Ok(None) } else { - self.positional().map(Some) + self.positional(spec).map(Some) } } #[cfg_attr(not(test), allow(dead_code))] - pub(crate) fn positional_list(&mut self) -> Result, SlashCommandUsageErrorKind> + pub(crate) fn positional_list( + &mut self, + spec: &S, + ) -> Result, SlashCommandUsageErrorKind> where - T: SlashTokenValue, + S: SlashTokenValueSpec, { let mut values = Vec::new(); while self.next_positional < self.positionals.len() { - values.push(self.positional()?); + values.push(self.positional(spec)?); } Ok(values) } #[cfg_attr(not(test), allow(dead_code))] - pub(crate) fn named( + pub(crate) fn named( &mut self, key: &'static str, + spec: &S, ) -> Result, SlashCommandUsageErrorKind> where - T: SlashTokenValue, + S: SlashTokenValueSpec, { if self.duplicates.contains_key(key) { return Err(SlashCommandUsageErrorKind::InvalidInlineArgs); @@ -206,15 +340,23 @@ impl<'a> SlashArgsParser<'a> { let Some(value) = self.named.remove(key) else { return Ok(None); }; - T::parse_token(value).map(Some) + spec.parse_token(value).map(Some) } - pub(crate) fn remainder(&self) -> Option { + pub(crate) fn remainder(&self, spec: &S) -> Result, SlashCommandUsageErrorKind> + where + S: SlashTextValueSpec, + { parse_remainder_text_arg(self.input.args, self.input.text_elements) + .map(|value| spec.parse_text(value)) + .transpose() } - pub(crate) fn required_remainder(&self) -> Result { - self.remainder() + pub(crate) fn required_remainder(&self, spec: &S) -> Result + where + S: SlashTextValueSpec, + { + self.remainder(spec)? .ok_or(SlashCommandUsageErrorKind::InvalidInlineArgs) } @@ -235,45 +377,49 @@ pub(crate) struct SlashArgsSerializer { } impl SlashArgsSerializer { - pub(crate) fn positional(&mut self, value: &T) + pub(crate) fn positional(&mut self, value: &T, spec: &S) where - T: SlashTokenValue, + S: SlashTokenValueSpec, { self.fragments - .push(serialize_token(&value.serialize_token())); + .push(serialize_token(&spec.serialize_token(value))); } #[cfg_attr(not(test), allow(dead_code))] - pub(crate) fn list(&mut self, values: I) + pub(crate) fn list(&mut self, values: I, spec: &S) where - T: SlashTokenValue, I: IntoIterator, + S: SlashTokenValueSpec, { for value in values { - self.positional(&value); + self.positional(&value, spec); } } #[allow(dead_code)] - pub(crate) fn named(&mut self, key: &'static str, value: &T) + pub(crate) fn named(&mut self, key: &'static str, value: &T, spec: &S) where - T: SlashTokenValue, + S: SlashTokenValueSpec, { - let serialized_value = serialize_token(&value.serialize_token()); + let serialized_value = serialize_token(&spec.serialize_token(value)); self.fragments .push(serialized_value.prepend_inline(&format!("--{key}="))); } - pub(crate) fn remainder(&mut self, value: &SlashTextArg) { - if remainder_can_roundtrip_raw(value) { + pub(crate) fn remainder(&mut self, value: &T, spec: &S) + where + S: SlashTextValueSpec, + { + let serialized = spec.serialize_text(value); + if remainder_can_roundtrip_raw(&serialized) { self.fragments.push(SlashSerializedText { - text: value.text.clone(), - text_elements: value.text_elements.clone(), + text: serialized.text.clone(), + text_elements: serialized.text_elements, }); } else { self.fragments.push(serialize_token(&SlashTokenArg::new( - value.text.clone(), - value.text_elements.clone(), + serialized.text.clone(), + serialized.text_elements, ))); } } @@ -554,23 +700,7 @@ mod tests { Off, } - impl SlashTokenValue for Switch { - fn parse_token(token: SlashTokenArg) -> Result { - match token.text.as_str() { - "on" => Ok(Self::On), - "off" => Ok(Self::Off), - _ => Err(SlashCommandUsageErrorKind::InvalidInlineArgs), - } - } - - fn serialize_token(&self) -> SlashTokenArg { - let text = match self { - Self::On => "on", - Self::Off => "off", - }; - SlashTokenArg::new(text.to_string(), Vec::new()) - } - } + const SWITCH_CHOICES: &[(&str, Switch)] = &[("on", Switch::On), ("off", Switch::Off)]; #[test] fn parser_supports_positional_list_and_named_args() { @@ -580,13 +710,16 @@ mod tests { }) .unwrap(); - assert_eq!(parser.positional::(), Ok(Switch::On)); assert_eq!( - parser.positional_list::(), + parser.positional(&enum_choice(SWITCH_CHOICES)), + Ok(Switch::On) + ); + assert_eq!( + parser.positional_list(&string()), Ok(vec!["first".to_string(), "second".to_string()]) ); assert_eq!( - parser.named::("path"), + parser.named("path", &string()), Ok(Some("some dir".to_string())) ); assert_eq!(parser.finish(), Ok(())); @@ -600,17 +733,20 @@ mod tests { }) .unwrap(); - assert_eq!(parser.positional::(), Ok(Switch::On)); - assert_eq!(parser.optional_positional::(), Ok(None)); + assert_eq!( + parser.positional(&enum_choice(SWITCH_CHOICES)), + Ok(Switch::On) + ); + assert_eq!(parser.optional_positional(&string()), Ok(None)); assert_eq!(parser.finish(), Ok(())); } #[test] fn serializer_stably_formats_named_args_after_positionals() { let mut serializer = SlashArgsSerializer::default(); - serializer.positional(&Switch::On); - serializer.list::(["first".to_string(), "second".to_string()]); - serializer.named("path", &"some dir".to_string()); + serializer.positional(&Switch::On, &enum_choice(SWITCH_CHOICES)); + serializer.list(["first".to_string(), "second".to_string()], &string()); + serializer.named("path", &"some dir".to_string(), &string()); assert_eq!( serializer.finish(), @@ -629,7 +765,7 @@ mod tests { vec![TextElement::new((7..18).into(), Some(placeholder.clone()))], ); let mut serializer = SlashArgsSerializer::default(); - serializer.remainder(&prompt); + serializer.remainder(&prompt, &text()); assert_eq!( serializer.finish(), @@ -644,7 +780,7 @@ mod tests { fn remainder_quotes_shell_sensitive_text_when_needed() { let prompt = SlashTextArg::new("a\"\" a\"".to_string(), Vec::new()); let mut serializer = SlashArgsSerializer::default(); - serializer.remainder(&prompt); + serializer.remainder(&prompt, &text()); assert_eq!( serializer.finish(), @@ -659,7 +795,7 @@ mod tests { text_elements: &[], }) .unwrap() - .required_remainder(), + .required_remainder(&text()), Ok(prompt) ); }