partial broken fix for optional parameters (mostly message and reproxy commands)

PluralKit.Bot/CommandMeta/CommandTree.cs

@@ -281,13 +281,11 @@ public partial class CommandTree
             Commands.PermcheckChannel(var param, _) => ctx.Execute<Checks>(PermCheck, m => m.PermCheckChannel(ctx, param.target)),
             Commands.PermcheckGuild(var param, _) => ctx.Execute<Checks>(PermCheck, m => m.PermCheckGuild(ctx, param.target)),
             Commands.MessageProxyCheck(var param, _) => ctx.Execute<Checks>(ProxyCheck, m => m.MessageProxyCheck(ctx, param.target)),
-            Commands.MessageInfo(var param, var flags) => ctx.Execute<ProxiedMessage>(Message, m => m.GetMessage(ctx, param.target.MessageId, flags.GetReplyFormat(), flags.delete, flags.author, flags.show_embed)),
+            Commands.MessageInfo(var param, var flags) => ctx.Execute<ProxiedMessage>(Message, m => m.GetMessage(ctx, param.target, flags.GetReplyFormat(), flags.delete, flags.author, flags.show_embed)),
-            Commands.MessageAuthor(var param, var flags) => ctx.Execute<ProxiedMessage>(Message, m => m.GetMessage(ctx, param.target.MessageId, flags.GetReplyFormat(), false, true, flags.show_embed)),
+            Commands.MessageAuthor(var param, var flags) => ctx.Execute<ProxiedMessage>(Message, m => m.GetMessage(ctx, param.target, flags.GetReplyFormat(), false, true, flags.show_embed)),
-            Commands.MessageDelete(var param, var flags) => ctx.Execute<ProxiedMessage>(Message, m => m.GetMessage(ctx, param.target.MessageId, flags.GetReplyFormat(), true, false, flags.show_embed)),
+            Commands.MessageDelete(var param, var flags) => ctx.Execute<ProxiedMessage>(Message, m => m.GetMessage(ctx, param.target, flags.GetReplyFormat(), true, false, flags.show_embed)),
-            Commands.MessageEditSpecified(var param, var flags) => ctx.Execute<ProxiedMessage>(MessageEdit, m => m.EditMessage(ctx, param.target.MessageId, param.new_content, flags.regex, flags.no_space, flags.append, flags.prepend, flags.clear_embeds, flags.clear_attachments)),
+            Commands.MessageEdit(var param, var flags) => ctx.Execute<ProxiedMessage>(MessageEdit, m => m.EditMessage(ctx, param.target, param.new_content, flags.regex, flags.no_space, flags.append, flags.prepend, flags.clear_embeds, flags.clear_attachments)),
-            Commands.MessageEdit(var param, var flags) => ctx.Execute<ProxiedMessage>(MessageEdit, m => m.EditMessage(ctx, null, param.new_content, flags.regex, flags.no_space, flags.append, flags.prepend, flags.clear_embeds, flags.clear_attachments)),
+            Commands.MessageReproxy(var param, _) => ctx.Execute<ProxiedMessage>(MessageReproxy, m => m.ReproxyMessage(ctx, param.msg, param.member)),
-            Commands.MessageReproxySpecified(var param, _) => ctx.Execute<ProxiedMessage>(MessageReproxy, m => m.ReproxyMessage(ctx, param.msg.MessageId, param.member)),
-            Commands.MessageReproxy(var param, _) => ctx.Execute<ProxiedMessage>(MessageReproxy, m => m.ReproxyMessage(ctx, null, param.member)),
             Commands.Import(var param, var flags) => ctx.Execute<ImportExport>(Import, m => m.Import(ctx, param.url, flags.yes)),
             Commands.Export(_, _) => ctx.Execute<ImportExport>(Export, m => m.Export(ctx)),
             Commands.ServerConfigShow => ctx.Execute<ServerConfig>(null, m => m.ShowConfig(ctx)),

PluralKit.Bot/Commands/Message.cs

@@ -58,9 +58,9 @@ public class ProxiedMessage
         _redisService = redisService;
     }
 
-    public async Task ReproxyMessage(Context ctx, ulong? messageId, PKMember target)
+    public async Task ReproxyMessage(Context ctx, Message.Reference? messageRef, PKMember target)
     {
-        var (msg, systemId) = await GetMessageToEdit(ctx, messageId, ReproxyTimeout, true);
+        var (msg, systemId) = await GetMessageToEdit(ctx, messageRef?.MessageId, ReproxyTimeout, true);
 
         if (ctx.System.Id != systemId)
             throw new PKError("Can't reproxy a message sent by a different system.");
@@ -91,9 +91,9 @@ public class ProxiedMessage
         }
     }
 
-    public async Task EditMessage(Context ctx, ulong? messageId, string newContent, bool useRegex, bool noSpace, bool append, bool prepend, bool clearEmbeds, bool clearAttachments)
+    public async Task EditMessage(Context ctx, Message.Reference? messageRef, string newContent, bool useRegex, bool noSpace, bool append, bool prepend, bool clearEmbeds, bool clearAttachments)
     {
-        var (msg, systemId) = await GetMessageToEdit(ctx, messageId, EditTimeout, false);
+        var (msg, systemId) = await GetMessageToEdit(ctx, messageRef?.MessageId, EditTimeout, false);
 
         if (ctx.System.Id != systemId)
             throw new PKError("Can't edit a message sent by a different system.");
@@ -320,17 +320,20 @@ public class ProxiedMessage
         return lastMessage;
     }
 
-    public async Task GetMessage(Context ctx, ulong? messageId, ReplyFormat format, bool isDelete, bool author, bool showEmbed)
+    public async Task GetMessage(Context ctx, Message.Reference? messageRef, ReplyFormat format, bool isDelete, bool author, bool showEmbed)
     {
-        if (messageId == null)
+        if (ctx.Message.Type == Message.MessageType.Reply && ctx.Message.MessageReference?.MessageId != null)
+            messageRef = ctx.Message.MessageReference;
+
+        if (messageRef == null || messageRef.MessageId == null)
         {
             throw new PKSyntaxError("You must pass a message ID or link.");
         }
 
-        var message = await ctx.Repository.GetFullMessage(messageId.Value);
+        var message = await ctx.Repository.GetFullMessage(messageRef.MessageId.Value);
         if (message == null)
         {
-            await GetCommandMessage(ctx, messageId.Value, isDelete, showEmbed);
+            await GetCommandMessage(ctx, messageRef.MessageId.Value, isDelete, showEmbed);
             return;
         }
 

crates/command_definitions/src/message.rs

@@ -1,7 +1,7 @@
 use super::*;
 
 pub fn cmds() -> impl Iterator<Item = Command> {
-    let message = tokens!(("message", ["msg", "messageinfo"]), MessageRef);
+    let message = tokens!(("message", ["msg", "messageinfo"]), Optional(MessageRef));
 
     let author = ("author", ["sender", "a"]);
     let delete = ("delete", ["del", "d"]);
@@ -20,19 +20,16 @@ pub fn cmds() -> impl Iterator<Item = Command> {
     };
 
     [
+        apply_edit(command!(edit, Optional(MessageRef), new_content_param => "message_edit")),
+        command!(reproxy, Optional(("msg", MessageRef)), ("member", MemberRef) => "message_reproxy")
+            .help("Reproxies a message with a different member"),
+        command!(message, author => "message_author").help("Shows the author of a proxied message"),
+        command!(message, delete => "message_delete").help("Deletes a proxied message"),
+        apply_edit(command!(message, edit, new_content_param => "message_edit")),
         command!(message => "message_info")
             .flag(delete)
             .flag(author)
             .help("Shows information about a proxied message"),
-        command!(message, author => "message_author").help("Shows the author of a proxied message"),
-        command!(message, delete => "message_delete").help("Deletes a proxied message"),
-        apply_edit(command!(message, edit, new_content_param => "message_edit_specified")),
-        apply_edit(command!(edit, Skip(MessageRef), new_content_param => "message_edit_specified")),
-        apply_edit(command!(edit, new_content_param => "message_edit")),
-        command!(reproxy, ("member", MemberRef) => "message_reproxy")
-            .help("Reproxies a message with a different member"),
-        command!(reproxy, ("msg", MessageRef), ("member", MemberRef) => "message_reproxy_specified")
-            .help("Reproxies a message with a different member"),
     ]
     .into_iter()
 }

crates/command_parser/src/lib.rs

@@ -51,21 +51,31 @@ pub fn parse_command(
     let mut matched_tokens: Vec<(Tree, (Token, TokenMatchResult, usize))> = Vec::new();
     let mut filtered_tokens: Vec<Token> = Vec::new();
     loop {
-        // println!(
+        let mut possible_tokens = local_tree
-        //     "possible: {:?}",
+            .possible_tokens()
-        //     local_tree
+            .filter(|t| !filtered_tokens.contains(t))
-        //         .possible_tokens()
+            // .filter(|t| {
-        //         .filter(|t| filtered_tokens.contains(t))
+            //     if !filtered_tokens.is_empty() {
-        //         .collect::<Vec<_>>()
+            //         !matches!(t, Token::Parameter(param) if param.is_optional())
-        // );
+            //     } else {
-        let next = next_token(
+            //         true
-            local_tree
+            //     }
-                .possible_tokens()
+            // })
-                .filter(|t| !filtered_tokens.contains(t)),
+            .collect::<Vec<_>>();
-            &input,
+        // sort so parameters come last
-            current_pos,
+        // we always want to test values first
-        );
+        // parameters that parse the remainder come last (otherwise they would always match)
-        // println!("next: {:?}", next);
+        possible_tokens.sort_by(|a, b| match (a, b) {
+            (Token::Parameter(param), _) if param.is_remainder() => std::cmp::Ordering::Greater,
+            (_, Token::Parameter(param)) if param.is_remainder() => std::cmp::Ordering::Less,
+            (Token::Parameter(_), Token::Parameter(_)) => std::cmp::Ordering::Equal,
+            (Token::Parameter(_), _) => std::cmp::Ordering::Greater,
+            (_, Token::Parameter(_)) => std::cmp::Ordering::Less,
+            _ => std::cmp::Ordering::Equal,
+        });
+        println!("possible: {:?}", possible_tokens);
+        let next = next_token(possible_tokens.iter().cloned(), &input, current_pos);
+        println!("next: {:?}", next);
         match &next {
             Some((found_token, result, new_pos)) => {
                 match &result {
@@ -76,6 +86,11 @@ pub fn parse_command(
                         ));
                     }
                     TokenMatchResult::ParameterMatchError { input: raw, msg } => {
+                        if matches!(found_token, Token::Parameter(param) if param.is_skip())
+                            && possible_tokens.len() > 1
+                        {
+                            continue;
+                        }
                         return Err(format!(
                             "Parameter `{raw}` in command `{prefix}{input}` could not be parsed: {msg}."
                         ));
@@ -112,6 +127,7 @@ pub fn parse_command(
                     .pop()
                     .and_then(|m| matches!(m.1, (Token::Parameter(_), _, _)).then_some(m))
                 {
+                    println!("redoing previous branch: {:?}", match_next.0);
                     local_tree = match_tree;
                     filtered_tokens.push(match_next.0);
                     continue;

crates/command_parser/src/token.rs

@@ -76,16 +76,10 @@ impl Token {
                     name: param.name().into(),
                     value: matched,
                 },
-                Err(err) => {
+                Err(err) => TokenMatchResult::ParameterMatchError {
-                    if param.is_skip() {
+                    input: input.into(),
-                        return None;
+                    msg: err,
-                    } else {
+                },
-                        TokenMatchResult::ParameterMatchError {
-                            input: input.into(),
-                            msg: err,
-                        }
-                    }
-                }
             }),
             // don't add a _ match here!
         }

crates/command_parser/src/tree.rs

@@ -1,6 +1,6 @@
 use ordermap::OrderMap;
 
-use crate::{command::Command, token::Token};
+use crate::{command::Command, parameter::Skip, token::Token};
 
 #[derive(Debug, Clone)]
 pub struct TreeBranch {
@@ -21,7 +21,17 @@ impl TreeBranch {
     pub fn register_command(&mut self, command: Command) {
         let mut current_branch = self;
         // iterate over tokens in command
-        for token in command.tokens.clone() {
+        for (index, token) in command.tokens.clone().into_iter().enumerate() {
+            // if the token is an optional parameter, register rest of the tokens to a separate branch
+            // this allows optional parameters to work if they are not the last token
+            if matches!(token, Token::Parameter(ref param) if param.is_optional())
+                && index < command.tokens.len() - 1
+            {
+                current_branch.register_command(Command {
+                    tokens: command.tokens[index + 1..].to_vec(),
+                    ..command.clone()
+                });
+            }
             // recursively get or create a sub-branch for each token
             current_branch = current_branch
                 .branches
@@ -36,7 +46,7 @@ impl TreeBranch {
         self.current_command.clone()
     }
 
-    pub fn possible_tokens(&self) -> impl Iterator<Item = &Token> {
+    pub fn possible_tokens(&self) -> impl Iterator<Item = &Token> + Clone {
         self.branches.keys()
     }