refactor(command_parser): combine TokenMatchError and TokenMatchedValue into a single result type

crates/command_parser/src/lib.rs

@@ -18,7 +18,7 @@ use flag::{Flag, FlagMatchError, FlagValueMatchError};
 use parameter::ParameterValue;
 use smol_str::SmolStr;
 use string::MatchedFlag;
-use token::{Token, TokenMatchError, TokenMatchValue};
+use token::{Token, TokenMatchResult};
 
 // todo: this should come from the bot probably
 const MAX_SUGGESTIONS: usize = 7;
@@ -55,40 +55,42 @@ pub fn parse_command(
         let next = next_token(local_tree.possible_tokens(), &input, current_pos);
         println!("next: {:?}", next);
         match next {
-            Some(Ok((found_token, arg, new_pos))) => {
+            Some((found_token, result, new_pos)) => {
-                current_pos = new_pos;
+                match &result {
-                current_token_idx += 1;
+                    // todo: better error messages for these?
-
+                    TokenMatchResult::MissingParameter { name } => {
-                if let Some(arg) = arg.as_ref() {
+                        return Err(format!("Expected parameter `{name}` in command `{prefix}{input} {found_token}`."));
-                    // insert arg as paramater if this is a parameter
-                    if let Some((param_name, param)) = arg.param.as_ref() {
-                        params.insert(param_name.to_string(), param.clone());
                     }
+                    TokenMatchResult::ParameterMatchError { input: raw, msg } => {
+                        return Err(format!("Parameter `{raw}` in command `{prefix}{input}` could not be parsed: {msg}."));
+                    }
+                    // don't use a catch-all here, we want to make sure compiler errors when new errors are added
+                    TokenMatchResult::MatchedParameter { .. } | TokenMatchResult::MatchedValue => {}
                 }
 
+                // add parameter if any
+                if let TokenMatchResult::MatchedParameter { name, value } = result {
+                    params.insert(name.to_string(), value);
+                }
+
+                // move to the next branch
                 if let Some(next_tree) = local_tree.get_branch(&found_token) {
                     local_tree = next_tree.clone();
                 } else {
                     panic!("found token {found_token:?} could not match tree, at {input}");
                 }
-            }
+
-            Some(Err((token, err))) => {
+                // advance our position on the input
-                let error_msg = match err {
+                current_pos = new_pos;
-                    TokenMatchError::MissingParameter { name } => {
+                current_token_idx += 1;
-                        format!("Expected parameter `{name}` in command `{prefix}{input} {token}`.")
-                    }
-                    TokenMatchError::ParameterMatchError { input: raw, msg } => {
-                        format!("Parameter `{raw}` in command `{prefix}{input}` could not be parsed: {msg}.")
-                    }
-                };
-                return Err(error_msg);
             }
             None => {
-                // if it said command not found on a flag, output better error message
                 let mut error = format!("Unknown command `{prefix}{input}`.");
 
-                if fmt_possible_commands(&mut error, &prefix, local_tree.possible_commands(2)).not()
+                if fmt_possible_commands(&mut error, &prefix, local_tree.possible_commands(1)).not()
                 {
+                    // add a space between the unknown command and "for a list of all possible commands"
+                    // message if we didn't add any possible suggestions
                     error.push_str(" ");
                 }
 
@@ -231,7 +233,7 @@ fn next_token<'a>(
     possible_tokens: impl Iterator<Item = &'a Token>,
     input: &str,
     current_pos: usize,
-) -> Option<Result<(&'a Token, Option<TokenMatchValue>, usize), (&'a Token, TokenMatchError)>> {
+) -> Option<(&'a Token, TokenMatchResult, usize)> {
     // get next parameter, matching quotes
     let matched = string::next_param(&input, current_pos);
     println!("matched: {matched:?}\n---");
@@ -248,21 +250,18 @@ fn next_token<'a>(
                 .then_some(&input[current_pos..])
                 .unwrap_or(v.value)
         });
+        let next_pos = match matched {
+            // return last possible pos if we matched remaining,
+            Some(_) if match_remaining => input.len(),
+            // otherwise use matched param next pos,
+            Some(ref param) => param.next_pos,
+            // and if didnt match anything we stay where we are
+            None => current_pos,
+        };
         match token.try_match(input_to_match) {
-            Some(Ok(value)) => {
+            Some(result) => {
-                println!("matched token: {}", token);
+                //println!("matched token: {}", token);
-                let next_pos = match matched {
+                return Some((token, result, next_pos));
-                    // return last possible pos if we matched remaining,
-                    Some(_) if match_remaining => input.len(),
-                    // otherwise use matched param next pos,
-                    Some(param) => param.next_pos,
-                    // and if didnt match anything we stay where we are
-                    None => current_pos,
-                };
-                return Some(Ok((token, value, next_pos)));
-            }
-            Some(Err(err)) => {
-                return Some(Err((token, err)));
             }
             None => {} // continue matching until we exhaust all tokens
         }

crates/command_parser/src/parameter.rs

@@ -2,7 +2,7 @@ use std::{fmt::Debug, str::FromStr};
 
 use smol_str::SmolStr;
 
-use crate::token::Token;
+use crate::token::{Token, TokenMatchResult};
 
 #[derive(Debug, Clone)]
 pub enum ParameterValue {
@@ -192,8 +192,12 @@ impl FromStr for Toggle {
     type Err = SmolStr;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let matches_self =
+        let matches_self = |toggle: &Self| {
-            |toggle: &Self| matches!(Token::from(*toggle).try_match(Some(s)), Some(Ok(None)));
+            matches!(
+                Token::from(*toggle).try_match(Some(s)),
+                Some(TokenMatchResult::MatchedValue)
+            )
+        };
         [Self::On, Self::Off]
             .into_iter()
             .find(matches_self)

crates/command_parser/src/token.rs

@@ -17,46 +17,26 @@ pub enum Token {
 }
 
 #[derive(Debug)]
-pub enum TokenMatchError {
+pub enum TokenMatchResult {
-    ParameterMatchError { input: SmolStr, msg: SmolStr },
+    MatchedValue,
-    MissingParameter { name: SmolStr },
+    MatchedParameter {
+        name: SmolStr,
+        value: ParameterValue,
+    },
+    ParameterMatchError {
+        input: SmolStr,
+        msg: SmolStr,
+    },
+    MissingParameter {
+        name: SmolStr,
+    },
 }
 
-#[derive(Debug)]
+// q: why not have a NoMatch variant in TokenMatchResult?
-pub(super) struct TokenMatchValue {
-    pub raw: SmolStr,
-    pub param: Option<(SmolStr, ParameterValue)>,
-}
-
-impl TokenMatchValue {
-    fn new_match(raw: impl Into<SmolStr>) -> TryMatchResult {
-        Some(Ok(Some(Self {
-            raw: raw.into(),
-            param: None,
-        })))
-    }
-
-    fn new_match_param(
-        raw: impl Into<SmolStr>,
-        param_name: impl Into<SmolStr>,
-        param: ParameterValue,
-    ) -> TryMatchResult {
-        Some(Ok(Some(Self {
-            raw: raw.into(),
-            param: Some((param_name.into(), param)),
-        })))
-    }
-}
-
-/// None -> no match
-/// Some(Ok(None)) -> match, no value
-/// Some(Ok(Some(_))) -> match, with value
-/// Some(Err(_)) -> error while matching
-// q: why do this while we could have a NoMatch in TokenMatchError?
 // a: because we want to differentiate between no match and match failure (it matched with an error)
 //    "no match" has a different charecteristic because we want to continue matching other tokens...
 //    ...while "match failure" means we should stop matching and return the error
-type TryMatchResult = Option<Result<Option<TokenMatchValue>, TokenMatchError>>;
+type TryMatchResult = Option<TokenMatchResult>;
 
 impl Token {
     pub(super) fn try_match(&self, input: Option<&str>) -> TryMatchResult {
@@ -66,9 +46,9 @@ impl Token {
                 // short circuit on:
                 return match self {
                     // missing paramaters
-                    Self::Parameter(param) => Some(Err(TokenMatchError::MissingParameter {
+                    Self::Parameter(param) => Some(TokenMatchResult::MissingParameter {
                         name: param.name().into(),
-                    })),
+                    }),
                     // everything else doesnt match if no input anyway
                     Self::Value { .. } => None,
                     // don't add a _ match here!
@@ -81,15 +61,18 @@ impl Token {
         match self {
             Self::Value { name, aliases } => (aliases.iter().chain(std::iter::once(name)))
                 .any(|v| v.eq(input))
-                .then(|| TokenMatchValue::new_match(input))
+                .then(|| TokenMatchResult::MatchedValue),
-                .unwrap_or(None),
+            Self::Parameter(param) => Some(match param.kind().match_value(input) {
-            Self::Parameter(param) => match param.kind().match_value(input) {
+                Ok(matched) => TokenMatchResult::MatchedParameter {
-                Ok(matched) => TokenMatchValue::new_match_param(input, param.name(), matched),
+                    name: param.name().into(),
-                Err(err) => Some(Err(TokenMatchError::ParameterMatchError {
+                    value: matched,
+                },
+                Err(err) => TokenMatchResult::ParameterMatchError {
                     input: input.into(),
                     msg: err,
-                })),
+                },
-            }, // don't add a _ match here!
+            }),
+            // don't add a _ match here!
         }
     }
 }