Make Command system more flexible

src/bot/botrulez/full_help.rs

@@ -3,7 +3,6 @@ use clap::Parser;
 
 use crate::api::Message;
 use crate::bot::command::{ClapCommand, Context};
-use crate::bot::commands::CommandInfo;
 use crate::conn;
 
 /// Show full bot help.
@@ -25,7 +24,7 @@ impl FullHelp {
 }
 
 pub trait HasDescriptions {
-    fn descriptions(&self) -> &[CommandInfo];
+    fn descriptions(&self, ctx: &Context) -> Vec<String>;
 }
 
 #[async_trait]
@@ -50,19 +49,9 @@ where
             result.push('\n');
         }
 
-        for help in bot.descriptions() {
+        for description in bot.descriptions(ctx) {
-            if !help.visible {
+            result.push_str(&description);
-                continue;
+            result.push('\n');
-            }
-
-            let usage = help.kind.usage(&help.name, &ctx.joined.session.name);
-            let line = if let Some(description) = &help.description {
-                format!("{usage} - {description}\n")
-            } else {
-                format!("{usage}\n")
-            };
-
-            result.push_str(&line);
         }
 
         if !self.after.is_empty() {

src/bot/command.rs

@@ -1,4 +1,6 @@
+mod bang;
 mod clap;
+mod hidden;
 
 use std::future::Future;
 
@@ -7,45 +9,13 @@ use async_trait::async_trait;
 use crate::api::{self, Message, MessageId};
 use crate::conn::{self, ConnTx, Joined};
 
-pub use self::clap::{Clap, ClapCommand};
+pub use self::bang::*;
+pub use self::clap::*;
+pub use self::hidden::*;
 
 use super::instance::InstanceConfig;
 
-#[derive(Clone, Copy, PartialEq, Eq)]
-pub enum Kind {
-    /// Global commands always respond. They override any specific or general
-    /// commands of the same name.
-    Global,
-    /// General commands only respond if no nick is specified.
-    General,
-    /// Specific commands only respond if the bot's current nick is specified.
-    Specific,
-}
-
-impl Kind {
-    pub fn global_and_general_usage(cmd_name: &str) -> String {
-        format!("!{cmd_name}")
-    }
-
-    pub fn specific_nick(nick: &str) -> String {
-        nick.replace(char::is_whitespace, "")
-    }
-
-    pub fn specific_usage(cmd_name: &str, nick: &str) -> String {
-        format!("!{cmd_name} @{}", Self::specific_nick(nick))
-    }
-
-    pub fn usage(self, cmd_name: &str, nick: &str) -> String {
-        match self {
-            Self::Global | Self::General => Self::global_and_general_usage(cmd_name),
-            Self::Specific => Self::specific_usage(cmd_name, nick),
-        }
-    }
-}
-
 pub struct Context {
-    pub name: String,
-    pub kind: Kind,
     pub config: InstanceConfig,
     pub conn_tx: ConnTx,
     pub joined: Joined,
@@ -75,9 +45,10 @@ impl Context {
     }
 }
 
+#[allow(unused_variables)]
 #[async_trait]
 pub trait Command<B, E> {
-    fn description(&self) -> Option<String> {
+    fn description(&self, ctx: &Context) -> Option<String> {
         None
     }
 

src/bot/command/bang.rs

@@ -0,0 +1,233 @@
+use async_trait::async_trait;
+
+use crate::api::Message;
+
+use super::{Command, Context};
+
+/// Parse leading whitespace followed by an `!`-initiated command.
+///
+/// Returns the command name and the remaining text with one leading whitespace
+/// removed. The remaining text may be the empty string.
+fn parse_command<'a>(text: &'a str, prefix: &str) -> Option<(&'a str, &'a str)> {
+    let text = text.trim_start();
+    let text = text.strip_prefix(prefix)?;
+    let (name, rest) = text.split_once(char::is_whitespace).unwrap_or((text, ""));
+    if name.is_empty() {
+        return None;
+    }
+    Some((name, rest))
+}
+
+/// Parse leading whitespace followed by an `@`-initiated nick.
+///
+/// Returns the nick and the remaining text with one leading whitespace removed.
+/// The remaining text may be the empty string.
+fn parse_specific(text: &str) -> Option<(&str, &str)> {
+    let text = text.trim_start();
+    let text = text.strip_prefix('@')?;
+    let (name, rest) = text.split_once(char::is_whitespace).unwrap_or((text, ""));
+    if name.is_empty() {
+        return None;
+    }
+    Some((name, rest))
+}
+
+fn specific_nick(nick: &str) -> String {
+    nick.replace(char::is_whitespace, "")
+}
+
+fn normalize_specific_nick(nick: &str) -> String {
+    specific_nick(nick).to_lowercase()
+}
+
+pub struct Global<C> {
+    prefix: String,
+    name: String,
+    inner: C,
+}
+
+impl<C> Global<C> {
+    pub fn new<S: ToString>(name: S, inner: C) -> Self {
+        Self {
+            prefix: "!".to_string(),
+            name: name.to_string(),
+            inner,
+        }
+    }
+
+    pub fn prefix<S: ToString>(mut self, prefix: S) -> Self {
+        self.prefix = prefix.to_string();
+        self
+    }
+}
+
+#[async_trait]
+impl<B, E, C> Command<B, E> for Global<C>
+where
+    B: Send,
+    C: Command<B, E> + Send + Sync,
+{
+    fn description(&self, ctx: &Context) -> Option<String> {
+        let inner = self.inner.description(ctx)?;
+        Some(format!("{}{} - {inner}", self.prefix, self.name))
+    }
+
+    async fn execute(&self, arg: &str, msg: &Message, ctx: &Context, bot: &mut B) -> Result<(), E> {
+        // TODO Replace with let-else
+        let (name, rest) = match parse_command(arg, &self.prefix) {
+            Some(parsed) => parsed,
+            None => return Ok(()),
+        };
+
+        if name != self.name {
+            return Ok(());
+        }
+
+        self.inner.execute(rest, msg, ctx, bot).await
+    }
+}
+
+pub struct General<C> {
+    prefix: String,
+    name: String,
+    inner: C,
+}
+
+impl<C> General<C> {
+    pub fn new<S: ToString>(name: S, inner: C) -> Self {
+        Self {
+            prefix: "!".to_string(),
+            name: name.to_string(),
+            inner,
+        }
+    }
+
+    pub fn prefix<S: ToString>(mut self, prefix: S) -> Self {
+        self.prefix = prefix.to_string();
+        self
+    }
+}
+
+#[async_trait]
+impl<B, E, C> Command<B, E> for General<C>
+where
+    B: Send,
+    C: Command<B, E> + Send + Sync,
+{
+    fn description(&self, ctx: &Context) -> Option<String> {
+        let inner = self.inner.description(ctx)?;
+        Some(format!("{}{} - {inner}", self.prefix, self.name))
+    }
+
+    async fn execute(&self, arg: &str, msg: &Message, ctx: &Context, bot: &mut B) -> Result<(), E> {
+        // TODO Replace with let-else
+        let (name, rest) = match parse_command(arg, &self.prefix) {
+            Some(parsed) => parsed,
+            None => return Ok(()),
+        };
+
+        if name != self.name {
+            return Ok(());
+        }
+
+        if parse_specific(rest).is_some() {
+            // The command looks like a specific command. If we treated it like
+            // a general command match, we would interpret other bots' specific
+            // commands as general commands.
+            return Ok(());
+        }
+
+        self.inner.execute(rest, msg, ctx, bot).await
+    }
+}
+
+pub struct Specific<C> {
+    prefix: String,
+    name: String,
+    inner: C,
+}
+
+impl<C> Specific<C> {
+    pub fn new<S: ToString>(name: S, inner: C) -> Self {
+        Self {
+            prefix: "!".to_string(),
+            name: name.to_string(),
+            inner,
+        }
+    }
+
+    pub fn prefix<S: ToString>(mut self, prefix: S) -> Self {
+        self.prefix = prefix.to_string();
+        self
+    }
+}
+
+#[async_trait]
+impl<B, E, C> Command<B, E> for Specific<C>
+where
+    B: Send,
+    C: Command<B, E> + Send + Sync,
+{
+    fn description(&self, ctx: &Context) -> Option<String> {
+        let inner = self.inner.description(ctx)?;
+        let nick = specific_nick(&ctx.joined.session.name);
+        Some(format!("{}{} @{nick} - {inner}", self.prefix, self.name))
+    }
+
+    async fn execute(&self, arg: &str, msg: &Message, ctx: &Context, bot: &mut B) -> Result<(), E> {
+        // TODO Replace with let-else
+        let (name, rest) = match parse_command(arg, &self.prefix) {
+            Some(parsed) => parsed,
+            None => return Ok(()),
+        };
+
+        if name != self.name {
+            return Ok(());
+        }
+
+        // TODO Replace with let-else
+        let (nick, rest) = match parse_specific(rest) {
+            Some(parsed) => parsed,
+            None => return Ok(()),
+        };
+
+        if normalize_specific_nick(nick) != normalize_specific_nick(&ctx.joined.session.name) {
+            return Ok(());
+        }
+
+        self.inner.execute(rest, msg, ctx, bot).await
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::{parse_command, parse_specific};
+
+    #[test]
+    fn test_parse_command() {
+        assert_eq!(parse_command("!foo", "!"), Some(("foo", "")));
+        assert_eq!(parse_command("    !foo", "!"), Some(("foo", "")));
+        assert_eq!(parse_command("!foo    ", "!"), Some(("foo", "   ")));
+        assert_eq!(parse_command("    !foo    ", "!"), Some(("foo", "   ")));
+        assert_eq!(parse_command("!foo @bar", "!"), Some(("foo", "@bar")));
+        assert_eq!(parse_command("!foo    @bar", "!"), Some(("foo", "   @bar")));
+        assert_eq!(parse_command("!foo @bar   ", "!"), Some(("foo", "@bar   ")));
+        assert_eq!(parse_command("! foo @bar", "!"), None);
+        assert_eq!(parse_command("!", "!"), None);
+        assert_eq!(parse_command("?foo", "!"), None);
+    }
+
+    #[test]
+    fn test_parse_specific() {
+        assert_eq!(parse_specific("@foo"), Some(("foo", "")));
+        assert_eq!(parse_specific("    @foo"), Some(("foo", "")));
+        assert_eq!(parse_specific("@foo    "), Some(("foo", "   ")));
+        assert_eq!(parse_specific("    @foo    "), Some(("foo", "   ")));
+        assert_eq!(parse_specific("@foo !bar"), Some(("foo", "!bar")));
+        assert_eq!(parse_specific("@foo     !bar"), Some(("foo", "    !bar")));
+        assert_eq!(parse_specific("@foo !bar    "), Some(("foo", "!bar    ")));
+        assert_eq!(parse_specific("@ foo !bar"), None);
+        assert_eq!(parse_specific("@"), None);
+        assert_eq!(parse_specific("?foo"), None);
+    }
+}

src/bot/command/clap.rs

@@ -106,7 +106,7 @@ where
     C: ClapCommand<B, E> + Send + Sync,
     C::Args: Parser + Send,
 {
-    fn description(&self) -> Option<String> {
+    fn description(&self, _ctx: &Context) -> Option<String> {
         C::Args::command().get_about().map(|s| format!("{s}"))
     }
 
@@ -119,7 +119,9 @@ where
             }
         };
 
-        args.insert(0, ctx.kind.usage(&ctx.name, &ctx.joined.session.name));
+        // Hacky, but it should work fine in most cases
+        let usage = msg.content.strip_suffix(arg).unwrap_or("<command>").trim();
+        args.insert(0, usage.to_string());
 
         let args = match C::Args::try_parse_from(args) {
             Ok(args) => args,

src/bot/command/hidden.rs

@@ -0,0 +1,23 @@
+use async_trait::async_trait;
+
+use crate::api::Message;
+
+use super::{Command, Context};
+
+pub struct Hidden<C>(pub C);
+
+#[async_trait]
+impl<B, E, C> Command<B, E> for Hidden<C>
+where
+    B: Send,
+    C: Command<B, E> + Send + Sync,
+{
+    fn description(&self, _ctx: &Context) -> Option<String> {
+        // Default implementation, repeated here for emphasis.
+        None
+    }
+
+    async fn execute(&self, arg: &str, msg: &Message, ctx: &Context, bot: &mut B) -> Result<(), E> {
+        self.0.execute(arg, msg, ctx, bot).await
+    }
+}

src/bot/commands.rs

@@ -1,153 +1,31 @@
-use std::collections::HashMap;
-
 use crate::api::packet::ParsedPacket;
 use crate::api::{Data, SendEvent};
 use crate::conn;
 
-use super::command::{Command, Context, Kind};
+use super::command::{Command, Context};
 use super::instance::{InstanceConfig, Snapshot};
 
-fn normalize_specific_nick(nick: &str) -> String {
-    Kind::specific_nick(nick).to_lowercase()
-}
-
-/// Parse leading whitespace followed by an `!`-initiated command.
-///
-/// Returns the command name and the remaining text with one leading whitespace
-/// removed. The remaining text may be the empty string.
-fn parse_command(text: &str) -> Option<(&str, &str)> {
-    let text = text.trim_start();
-    let text = text.strip_prefix('!')?;
-    let (name, rest) = text.split_once(char::is_whitespace).unwrap_or((text, ""));
-    if name.is_empty() {
-        return None;
-    }
-    Some((name, rest))
-}
-
-/// Parse leading whitespace followed by an `@`-initiated nick.
-///
-/// Returns the nick and the remaining text with one leading whitespace removed.
-/// The remaining text may be the empty string.
-fn parse_specific(text: &str) -> Option<(&str, &str)> {
-    let text = text.trim_start();
-    let text = text.strip_prefix('@')?;
-    let (name, rest) = text.split_once(char::is_whitespace).unwrap_or((text, ""));
-    if name.is_empty() {
-        return None;
-    }
-    Some((name, rest))
-}
-
-pub struct CommandInfo {
-    pub kind: Kind,
-    pub name: String,
-    pub description: Option<String>,
-    pub visible: bool,
-}
-
-struct CommandWrapper<B, E> {
-    command: Box<dyn Command<B, E>>,
-    visible: bool,
-}
-
 pub struct Commands<B, E> {
-    global: HashMap<String, CommandWrapper<B, E>>,
+    commands: Vec<Box<dyn Command<B, E> + Send + Sync>>,
-    general: HashMap<String, CommandWrapper<B, E>>,
-    specific: HashMap<String, CommandWrapper<B, E>>,
 }
 
 impl<B, E> Commands<B, E> {
     pub fn new() -> Self {
-        Self {
+        Self { commands: vec![] }
-            global: HashMap::new(),
-            general: HashMap::new(),
-            specific: HashMap::new(),
-        }
     }
 
-    /// Global commands always respond. They override any specific or general
+    pub fn add<C>(&mut self, command: C)
-    /// commands of the same name.
-    ///
-    /// Use this if your bot "owns" the command and no other bot uses it.
-    pub fn global<S, C>(mut self, name: S, command: C, visible: bool) -> Self
     where
-        S: ToString,
+        C: Command<B, E> + Send + Sync + 'static,
-        C: Command<B, E> + 'static,
     {
-        let command = Box::new(command);
+        self.commands.push(Box::new(command));
-        let info = CommandWrapper { command, visible };
-        self.global.insert(name.to_string(), info);
-        self
     }
 
-    /// General commands only respond if no nick is specified.
+    pub fn descriptions(&self, ctx: &Context) -> Vec<String> {
-    ///
+        self.commands
-    /// Use this if your or any other bot has a specific command of the same
+            .iter()
-    /// name.
+            .filter_map(|c| c.description(ctx))
-    pub fn general<S, C>(mut self, name: S, command: C, visible: bool) -> Self
+            .collect::<Vec<_>>()
-    where
-        S: ToString,
-        C: Command<B, E> + 'static,
-    {
-        let command = Box::new(command);
-        let info = CommandWrapper { command, visible };
-        self.general.insert(name.to_string(), info);
-        self
-    }
-
-    /// Specific commands only respond if the bot's current nick is specified.
-    pub fn specific<S, C>(mut self, name: S, command: C, visible: bool) -> Self
-    where
-        S: ToString,
-        C: Command<B, E> + 'static,
-    {
-        let command = Box::new(command);
-        let info = CommandWrapper { command, visible };
-        self.specific.insert(name.to_string(), info);
-        self
-    }
-
-    pub fn descriptions(&self) -> Vec<CommandInfo> {
-        let mut keys = (self.global.keys())
-            .chain(self.general.keys())
-            .chain(self.specific.keys())
-            .collect::<Vec<_>>();
-        keys.sort_unstable();
-        keys.dedup();
-
-        let mut result = vec![];
-        for name in keys {
-            if let Some(wrapper) = self.global.get(name) {
-                result.push(CommandInfo {
-                    name: name.clone(),
-                    kind: Kind::Global,
-                    visible: wrapper.visible,
-                    description: wrapper.command.description(),
-                });
-                continue; // Shadows general and specific commands
-            }
-
-            if let Some(wrapper) = self.general.get(name) {
-                result.push(CommandInfo {
-                    name: name.clone(),
-                    kind: Kind::General,
-                    visible: wrapper.visible,
-                    description: wrapper.command.description(),
-                });
-            }
-
-            if let Some(wrapper) = self.specific.get(name) {
-                result.push(CommandInfo {
-                    name: name.clone(),
-                    kind: Kind::Specific,
-                    visible: wrapper.visible,
-                    description: wrapper.command.description(),
-                });
-            }
-        }
-
-        result
     }
 
     /// Returns `true` if a command was found and executed, `false` otherwise.
@@ -157,63 +35,28 @@ impl<B, E> Commands<B, E> {
         packet: &ParsedPacket,
         snapshot: &Snapshot,
         bot: &mut B,
-    ) -> Result<bool, E> {
+    ) -> Result<(), E> {
         let msg = match &packet.content {
             Ok(Data::SendEvent(SendEvent(msg))) => msg,
-            _ => return Ok(false),
+            _ => return Ok(()),
         };
 
         let joined = match &snapshot.state {
-            conn::State::Joining(_) => return Ok(false),
+            conn::State::Joining(_) => return Ok(()),
             conn::State::Joined(joined) => joined.clone(),
         };
 
-        let (cmd_name, rest) = match parse_command(&msg.content) {
+        let ctx = Context {
-            Some(parsed) => parsed,
-            None => return Ok(false),
-        };
-
-        let mut ctx = Context {
-            name: cmd_name.to_string(),
-            kind: Kind::Global,
             config: config.clone(),
             conn_tx: snapshot.conn_tx.clone(),
             joined,
         };
 
-        if let Some(wrapper) = self.global.get(cmd_name) {
+        for command in &self.commands {
-            ctx.kind = Kind::Global;
+            command.execute(&msg.content, msg, &ctx, bot).await?;
-            wrapper.command.execute(rest, msg, &ctx, bot).await?;
-            return Ok(true);
         }
 
-        if let Some((cmd_nick, rest)) = parse_specific(rest) {
+        Ok(())
-            if let Some(wrapper) = self.specific.get(cmd_name) {
-                let nick_norm = normalize_specific_nick(&ctx.joined.session.name);
-                let cmd_nick_norm = normalize_specific_nick(cmd_nick);
-                if nick_norm == cmd_nick_norm {
-                    ctx.kind = Kind::Specific;
-                    wrapper.command.execute(rest, msg, &ctx, bot).await?;
-                    return Ok(true);
-                }
-            }
-
-            // The command looks like a specific command. If we treated it like
-            // a general command just because the nick doesn't match, we would
-            // interpret other bots' specific commands as general commands.
-            //
-            // To call a specific command with a mention as its first positional
-            // argument, -- can be used.
-            return Ok(false);
-        }
-
-        if let Some(wrapper) = self.general.get(cmd_name) {
-            ctx.kind = Kind::General;
-            wrapper.command.execute(rest, msg, &ctx, bot).await?;
-            return Ok(true);
-        }
-
-        Ok(false)
     }
 }
 
@@ -222,36 +65,3 @@ impl<B, E> Default for Commands<B, E> {
         Self::new()
     }
 }
-
-#[cfg(test)]
-mod test {
-    use super::{parse_command, parse_specific};
-
-    #[test]
-    fn test_parse_command() {
-        assert_eq!(parse_command("!foo"), Some(("foo", "")));
-        assert_eq!(parse_command("    !foo"), Some(("foo", "")));
-        assert_eq!(parse_command("!foo    "), Some(("foo", "   ")));
-        assert_eq!(parse_command("    !foo    "), Some(("foo", "   ")));
-        assert_eq!(parse_command("!foo @bar"), Some(("foo", "@bar")));
-        assert_eq!(parse_command("!foo     @bar"), Some(("foo", "    @bar")));
-        assert_eq!(parse_command("!foo @bar    "), Some(("foo", "@bar    ")));
-        assert_eq!(parse_command("! foo @bar"), None);
-        assert_eq!(parse_command("!"), None);
-        assert_eq!(parse_command("?foo"), None);
-    }
-
-    #[test]
-    fn test_parse_specific() {
-        assert_eq!(parse_specific("@foo"), Some(("foo", "")));
-        assert_eq!(parse_specific("    @foo"), Some(("foo", "")));
-        assert_eq!(parse_specific("@foo    "), Some(("foo", "   ")));
-        assert_eq!(parse_specific("    @foo    "), Some(("foo", "   ")));
-        assert_eq!(parse_specific("@foo !bar"), Some(("foo", "!bar")));
-        assert_eq!(parse_specific("@foo     !bar"), Some(("foo", "    !bar")));
-        assert_eq!(parse_specific("@foo !bar    "), Some(("foo", "!bar    ")));
-        assert_eq!(parse_specific("@ foo !bar"), None);
-        assert_eq!(parse_specific("@"), None);
-        assert_eq!(parse_specific("?foo"), None);
-    }
-}