From d5c271be200c7487eabcb068ba86cf1dfa68f135 Mon Sep 17 00:00:00 2001 From: dusk Date: Sun, 12 Jan 2025 04:23:46 +0900 Subject: [PATCH] refactor(commands): clearer token match typing, make tree.possible_commads return iterator instead of traversing the whole tree immediately --- crates/commands/src/commands.rs | 4 +- crates/commands/src/lib.rs | 88 ++++++++++++++++----------- crates/commands/src/token.rs | 104 ++++++++++++++++---------------- crates/commands/src/tree.rs | 31 ++++++---- 4 files changed, 125 insertions(+), 102 deletions(-) diff --git a/crates/commands/src/commands.rs b/crates/commands/src/commands.rs index d9ab69b7..04ada503 100644 --- a/crates/commands/src/commands.rs +++ b/crates/commands/src/commands.rs @@ -18,7 +18,7 @@ pub mod server_config; pub mod switch; pub mod system; -use std::fmt::Display; +use std::fmt::{Debug, Display}; use smol_str::SmolStr; @@ -27,7 +27,7 @@ use crate::{ token::{ToToken, Token}, }; -#[derive(Clone, Debug)] +#[derive(Debug, Clone)] pub struct Command { // TODO: fix hygiene pub tokens: Vec, diff --git a/crates/commands/src/lib.rs b/crates/commands/src/lib.rs index 8b77b83e..fd1e794a 100644 --- a/crates/commands/src/lib.rs +++ b/crates/commands/src/lib.rs @@ -1,4 +1,5 @@ #![feature(let_chains)] +#![feature(anonymous_lifetime_in_impl_trait)] pub mod commands; mod string; @@ -10,8 +11,9 @@ uniffi::include_scaffolding!("commands"); use core::panic; use std::collections::HashMap; use std::fmt::Write; +use std::ops::Not; -use smol_str::{format_smolstr, SmolStr}; +use smol_str::SmolStr; use tree::TreeBranch; pub use commands::Command; @@ -69,10 +71,10 @@ pub fn parse_command(prefix: String, input: String) -> CommandResult { loop { let possible_tokens = local_tree.possible_tokens().cloned().collect::>(); println!("possible: {:?}", possible_tokens); - let next = next_token(possible_tokens.clone(), &prefix, input.clone(), current_pos); + let next = next_token(possible_tokens.clone(), input.clone(), current_pos); println!("next: {:?}", next); match next { - Ok((found_token, arg, new_pos)) => { + Some(Ok((found_token, arg, new_pos))) => { current_pos = new_pos; if let Token::Flag = found_token { flags.insert(arg.unwrap().raw.into(), None); @@ -94,7 +96,15 @@ pub fn parse_command(prefix: String, input: String) -> CommandResult { panic!("found token could not match tree, at {input}"); } } - Err(None) => { + Some(Err((token, err))) => { + let error_msg = match err { + TokenMatchError::MissingParameter { name } => { + format!("Expected parameter `{name}` in command `{prefix}{input} {token}`.") + } + }; + return CommandResult::Err { error: error_msg }; + } + None => { if let Some(command) = local_tree.command() { println!("{} {params:?}", command.cb); return CommandResult::Ok { @@ -109,33 +119,19 @@ pub fn parse_command(prefix: String, input: String) -> CommandResult { let mut error = format!("Unknown command `{prefix}{input}`."); - let possible_commands = local_tree.possible_commands(2); - if !possible_commands.is_empty() { - error.push_str(" Perhaps you meant to use one of the commands below:\n"); - for command in possible_commands.iter().take(MAX_SUGGESTIONS) { - if !command.show_in_suggestions { - continue; - } - writeln!(&mut error, "- **{prefix}{command}** - *{}*", command.help) - .expect("oom"); - } - } else { - error.push_str("\n"); + if fmt_possible_commands(&mut error, &prefix, local_tree.possible_commands(2)).not() + { + error.push_str(" "); } error.push_str( - "For a list of possible commands, see .", + "For a list of all possible commands, see .", ); // todo: check if last token is a common incorrect unquote (multi-member names etc) // todo: check if this is a system name in pk;s command return CommandResult::Err { error }; } - Err(Some(short_circuit)) => { - return CommandResult::Err { - error: short_circuit.into(), - }; - } } } } @@ -143,16 +139,16 @@ pub fn parse_command(prefix: String, input: String) -> CommandResult { /// Find the next token from an either raw or partially parsed command string /// /// Returns: +/// - nothing (none matched) /// - matched token, to move deeper into the tree /// - matched value (if this command matched an user-provided value such as a member name) /// - end position of matched token -/// - optionally a short-circuit error +/// - error when matching fn next_token( possible_tokens: Vec, - prefix: &str, input: SmolStr, current_pos: usize, -) -> Result<(Token, Option, usize), Option> { +) -> Option, usize), (Token, TokenMatchError)>> { // get next parameter, matching quotes let param = crate::string::next_param(input.clone(), current_pos); println!("matched: {param:?}\n---"); @@ -163,14 +159,14 @@ fn next_token( if let Some((value, new_pos)) = param.clone() && value.starts_with('-') { - return Ok(( + return Some(Ok(( Token::Flag, Some(TokenMatchedValue { raw: value, param: None, }), new_pos, - )); + ))); } // iterate over tokens and run try_match @@ -178,17 +174,39 @@ fn next_token( // for FullString just send the whole string let input_to_match = param.clone().map(|v| v.0); match token.try_match(input_to_match) { - TokenMatchResult::Match(value) => { - return Ok((token, value, param.map(|v| v.1).unwrap_or(current_pos))) - } - TokenMatchResult::MissingParameter { name } => { - return Err(Some(format_smolstr!( - "Missing parameter `{name}` in command `{prefix}{input} {token}`." + Some(Ok(value)) => { + return Some(Ok(( + token, + value, + param.map(|v| v.1).unwrap_or(current_pos), ))) } - TokenMatchResult::NoMatch => {} + Some(Err(err)) => { + return Some(Err((token, err))); + } + None => {} // continue matching until we exhaust all tokens } } - Err(None) + None +} + +// todo: should probably move this somewhere else +/// returns true if wrote possible commands, false if not +fn fmt_possible_commands( + f: &mut String, + prefix: &str, + mut possible_commands: impl Iterator, +) -> bool { + if let Some(first) = possible_commands.next() { + f.push_str(" Perhaps you meant to use one of the commands below:\n"); + for command in std::iter::once(first).chain(possible_commands.take(MAX_SUGGESTIONS - 1)) { + if !command.show_in_suggestions { + continue; + } + writeln!(f, "- **{prefix}{command}** - *{}*", command.help).expect("oom"); + } + return true; + } + return false; } diff --git a/crates/commands/src/token.rs b/crates/commands/src/token.rs index fb4371ba..f216fa4a 100644 --- a/crates/commands/src/token.rs +++ b/crates/commands/src/token.rs @@ -45,20 +45,8 @@ pub enum Token { Flag, } -// #[macro_export] -// macro_rules! any { -// ($($token:expr),+) => { -// Token::Any(vec![$($token.to_token()),+]) -// }; -// } - #[derive(Debug)] -pub enum TokenMatchResult { - /// Token did not match. - NoMatch, - /// Token matched, optionally with a value. - Match(Option), - /// A required parameter was missing. +pub enum TokenMatchError { MissingParameter { name: ParamName }, } @@ -68,33 +56,45 @@ pub struct TokenMatchedValue { pub param: Option<(ParamName, Parameter)>, } -impl TokenMatchResult { - fn new_match(raw: impl Into) -> Self { - Self::Match(Some(TokenMatchedValue { +impl TokenMatchedValue { + fn new_match(raw: impl Into) -> TryMatchResult { + Some(Ok(Some(Self { raw: raw.into(), param: None, - })) + }))) } - fn new_match_param(raw: impl Into, param_name: ParamName, param: Parameter) -> Self { - Self::Match(Some(TokenMatchedValue { + fn new_match_param( + raw: impl Into, + param_name: ParamName, + param: Parameter, + ) -> TryMatchResult { + Some(Ok(Some(Self { raw: raw.into(), param: Some((param_name, param)), - })) + }))) } } -impl Token { - pub fn try_match(&self, input: Option) -> TokenMatchResult { - use TokenMatchResult::*; +/// 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, TokenMatchError>>; +impl Token { + pub fn try_match(&self, input: Option) -> TryMatchResult { let input = match input { Some(input) => input, None => { // short circuit on: return match self { // empty token - Self::Empty => Match(None), + Self::Empty => Some(Ok(None)), // missing paramaters Self::FullString(param_name) | Self::MemberRef(param_name) @@ -104,16 +104,16 @@ impl Token { | Self::Toggle(param_name) | Self::Enable(param_name) | Self::Disable(param_name) - | Self::Reset(param_name) => MissingParameter { name: param_name }, - Self::Any(tokens) => { - tokens.is_empty().then_some(NoMatch).unwrap_or_else(|| { - let mut results = tokens.iter().map(|t| t.try_match(None)); - results.find(|r| !matches!(r, NoMatch)).unwrap_or(NoMatch) - }) + | Self::Reset(param_name) => { + Some(Err(TokenMatchError::MissingParameter { name: param_name })) } + Self::Any(tokens) => tokens.is_empty().then_some(None).unwrap_or_else(|| { + let mut results = tokens.iter().map(|t| t.try_match(None)); + results.find(|r| !matches!(r, None)).unwrap_or(None) + }), // everything else doesnt match if no input anyway - Token::Value(_) => NoMatch, - Token::Flag => NoMatch, + Token::Value(_) => None, + Token::Flag => None, // don't add a _ match here! }; } @@ -122,31 +122,31 @@ impl Token { // try actually matching stuff match self { - Self::Empty => NoMatch, + Self::Empty => None, Self::Flag => unreachable!(), // matched upstream (dusk: i don't really like this tbh) Self::Any(tokens) => tokens .iter() .map(|t| t.try_match(Some(input.into()))) - .find(|r| !matches!(r, NoMatch)) - .unwrap_or(NoMatch), + .find(|r| !matches!(r, None)) + .unwrap_or(None), Self::Value(values) => values .iter() .any(|v| v.eq(input)) - .then(|| TokenMatchResult::new_match(input)) - .unwrap_or(NoMatch), - Self::FullString(param_name) => TokenMatchResult::new_match_param( + .then(|| TokenMatchedValue::new_match(input)) + .unwrap_or(None), + Self::FullString(param_name) => TokenMatchedValue::new_match_param( input, param_name, Parameter::OpaqueString { raw: input.into() }, ), - Self::SystemRef(param_name) => TokenMatchResult::new_match_param( + Self::SystemRef(param_name) => TokenMatchedValue::new_match_param( input, param_name, Parameter::SystemRef { system: input.into(), }, ), - Self::MemberRef(param_name) => TokenMatchResult::new_match_param( + Self::MemberRef(param_name) => TokenMatchedValue::new_match_param( input, param_name, Parameter::MemberRef { @@ -154,55 +154,55 @@ impl Token { }, ), Self::MemberPrivacyTarget(param_name) => match MemberPrivacyTarget::from_str(input) { - Ok(target) => TokenMatchResult::new_match_param( + Ok(target) => TokenMatchedValue::new_match_param( input, param_name, Parameter::MemberPrivacyTarget { target: target.as_ref().into(), }, ), - Err(_) => NoMatch, + Err(_) => None, }, Self::PrivacyLevel(param_name) => match PrivacyLevel::from_str(input) { - Ok(level) => TokenMatchResult::new_match_param( + Ok(level) => TokenMatchedValue::new_match_param( input, param_name, Parameter::PrivacyLevel { level: level.as_ref().into(), }, ), - Err(_) => NoMatch, + Err(_) => None, }, Self::Toggle(param_name) => match Enable::from_str(input) .map(Into::::into) .or_else(|_| Disable::from_str(input).map(Into::::into)) { - Ok(toggle) => TokenMatchResult::new_match_param( + Ok(toggle) => TokenMatchedValue::new_match_param( input, param_name, Parameter::Toggle { toggle }, ), - Err(_) => NoMatch, + Err(_) => None, }, Self::Enable(param_name) => match Enable::from_str(input) { - Ok(t) => TokenMatchResult::new_match_param( + Ok(t) => TokenMatchedValue::new_match_param( input, param_name, Parameter::Toggle { toggle: t.into() }, ), - Err(_) => NoMatch, + Err(_) => None, }, Self::Disable(param_name) => match Disable::from_str(input) { - Ok(t) => TokenMatchResult::new_match_param( + Ok(t) => TokenMatchedValue::new_match_param( input, param_name, Parameter::Toggle { toggle: t.into() }, ), - Err(_) => NoMatch, + Err(_) => None, }, Self::Reset(param_name) => match Reset::from_str(input) { - Ok(_) => TokenMatchResult::new_match_param(input, param_name, Parameter::Reset), - Err(_) => NoMatch, + Ok(_) => TokenMatchedValue::new_match_param(input, param_name, Parameter::Reset), + Err(_) => None, }, // don't add a _ match here! } diff --git a/crates/commands/src/tree.rs b/crates/commands/src/tree.rs index 520208df..f6810f3c 100644 --- a/crates/commands/src/tree.rs +++ b/crates/commands/src/tree.rs @@ -24,7 +24,7 @@ impl TreeBranch { current_branch = current_branch.branches.entry(token).or_insert(TreeBranch { current_command: None, branches: OrderMap::new(), - }) + }); } // when we're out of tokens, add an Empty branch with the callback and no sub-branches current_branch.branches.insert( @@ -44,20 +44,25 @@ impl TreeBranch { self.branches.keys() } - pub fn possible_commands(&self, max_depth: usize) -> Vec { - if max_depth == 0 { - return Vec::new(); + pub fn possible_commands(&self, max_depth: usize) -> impl Iterator { + // dusk: i am too lazy to write an iterator for this without using recursion so we box everything + fn box_iter<'a>( + iter: impl Iterator + 'a, + ) -> Box + 'a> { + Box::new(iter) } - let mut commands = Vec::new(); - for token in self.possible_tokens() { - if let Some(tree) = self.get_branch(token) { - if let Some(command) = tree.command() { - commands.push(command); - // we dont need to look further if we found a command - continue; - } - commands.append(&mut tree.possible_commands(max_depth - 1)); + + if max_depth == 0 { + return box_iter(std::iter::empty()); + } + let mut commands = box_iter(std::iter::empty()); + for branch in self.branches.values() { + if let Some(command) = branch.current_command.as_ref() { + commands = box_iter(commands.chain(std::iter::once(command))); + // we dont need to look further if we found a command (only Empty tokens have commands) + continue; } + commands = box_iter(commands.chain(branch.possible_commands(max_depth - 1))); } commands }