Fix command parser returning non-commands/empty messages as errors.

This behavior became broken again after switching away from the
macro-based command parsing. The bot would return any non !command
message as an error, which would cause it to read more messages, and
return those as errors, until finally the matrix SDK would throw up.

Command parser now more properly handles empty messages and
non-commands, but we also simply abort processing if the incoming
message doesn't start with an exclamation point.

src/bot.rs

@@ -1,6 +1,6 @@
 use crate::commands::parse_command;
 use dirs;
-use log::{error, info, warn};
+use log::{error, info, trace, warn};
 use matrix_sdk::{
     self,
     events::{
@@ -92,12 +92,19 @@ impl EventEmitter for DiceBot {
                 (String::new(), String::new())
             };
 
+            //Command parser can handle non-commands, but faster to
+            //not parse them.
+            if !msg_body.starts_with("!") {
+                trace!("Ignoring non-command: {}", msg_body);
+                return;
+            }
+
             let (plain, html) = match parse_command(&msg_body) {
                 Ok(Some(command)) => {
                     let command = command.execute();
                     (command.plain().into(), command.html().into())
                 }
-                Ok(None) => return,
+                Ok(None) => return, //Ignore non-commands.
                 Err(e) => {
                     let message = format!("Error parsing command: {}", e);
                     let html_message = format!("<p><strong>{}</strong></p>", message);

src/commands.rs

@@ -82,14 +82,15 @@ impl Command for HelpCommand {
 
 /// Parse a command string into a dynamic command execution trait
 /// object. Returns an error if a command was recognized but not
-/// parsed correctly. Returns None if no command was recognized.
+/// parsed correctly. Returns Ok(None) if no command was recognized.
 pub fn parse_command(s: &str) -> Result<Option<Box<dyn Command>>, String> {
     match parser::parse_command(s) {
         Ok((input, command)) => match (input, &command) {
-            //This clause prevents bot from spamming messages to itself
+            //Any command, or text transformed into non-command is
-            //after executing a previous command.
+            //sent upwards.
             ("", Some(_)) | (_, None) => Ok(command),
 
+            //TODO replcae with nom all_consuming?
             //Any unconsumed input (whitespace should already be
             // stripped) is considered a parsing error.
             _ => Err(format!("{}: malformed expression", s)),

src/commands/parser.rs

@@ -2,7 +2,10 @@ use crate::cofd::parser::{create_chance_die, parse_dice_pool};
 use crate::commands::{Command, HelpCommand, PoolRollCommand, RollCommand};
 use crate::dice::parser::parse_element_expression;
 use crate::help::parse_help_topic;
-use nom::{bytes::complete::tag, character::complete::alpha1, IResult};
+use nom::bytes::streaming::tag;
+use nom::error::ErrorKind as NomErrorKind;
+use nom::Err as NomErr;
+use nom::{character::complete::alpha1, IResult};
 
 // Parse a roll expression.
 fn parse_roll(input: &str) -> IResult<&str, Box<dyn Command>> {
@@ -30,9 +33,9 @@ fn help(topic: &str) -> IResult<&str, Box<dyn Command>> {
 /// rest of the line) and returns a tuple of (command_input, command).
 /// Whitespace at the start and end of the command input is removed.
 fn split_command(input: &str) -> IResult<&str, &str> {
-    //let (input, _) = eat_whitespace(input)?;
     let input = input.trim_start();
     let (input, _) = tag("!")(input)?;
+
     let (mut command_input, command) = alpha1(input)?;
     command_input = command_input.trim();
     Ok((command_input, command))
@@ -40,17 +43,28 @@ fn split_command(input: &str) -> IResult<&str, &str> {
 
 /// Potentially parse a command expression. If we recognize the
 /// command, an error should be raised if the command is misparsed. If
-/// we don't recognize the command, ignore it and return none
+/// we don't recognize the command, ignore it and return None.
 pub fn parse_command(input: &str) -> IResult<&str, Option<Box<dyn Command>>> {
-    let (cmd_input, cmd) = split_command(input)?;
+    match split_command(input) {
+        Ok((cmd_input, cmd)) => match cmd {
+            "r" | "roll" => parse_roll(cmd_input).map(|(input, command)| (input, Some(command))),
+            "rp" | "pool" => {
+                parse_pool_roll(cmd_input).map(|(input, command)| (input, Some(command)))
+            }
+            "chance" => chance_die().map(|(input, command)| (input, Some(command))),
+            "help" => help(cmd_input).map(|(input, command)| (input, Some(command))),
+            // No recognized command, ignore this.
+            _ => Ok((input, None)),
+        },
 
-    match cmd {
+        //TODO better way to do this?
-        "r" | "roll" => parse_roll(cmd_input).map(|(input, command)| (input, Some(command))),
+        //If the input is not a command, or the message is incomplete
-        "rp" | "pool" => parse_pool_roll(cmd_input).map(|(input, command)| (input, Some(command))),
+        //(empty), we declare this to be a non-command, and don't do
-        "chance" => chance_die().map(|(input, command)| (input, Some(command))),
+        //anything else with it.
-        "help" => help(cmd_input).map(|(input, command)| (input, Some(command))),
+        Err(NomErr::Error((_, NomErrorKind::Tag))) | Err(NomErr::Incomplete(_)) => Ok(("", None)),
-        // No recognized command, ignore this.
+
-        _ => Ok((input, None)),
+        //All other errors passed up.
+        Err(e) => Err(e),
     }
 }
 
@@ -58,6 +72,26 @@ pub fn parse_command(input: &str) -> IResult<&str, Option<Box<dyn Command>>> {
 mod tests {
     use super::*;
 
+    #[test]
+    fn non_command_test() {
+        let result = parse_command("not a command");
+        assert!(result.is_ok());
+        assert!(result.unwrap().1.is_none());
+    }
+
+    #[test]
+    fn empty_message_test() {
+        let result = parse_command("");
+        assert!(result.is_ok());
+        assert!(result.unwrap().1.is_none());
+    }
+
+    #[test]
+    fn just_exclamation_point_test() {
+        let result = parse_command("!");
+        assert!(result.is_err());
+    }
+
     #[test]
     fn basic_command_test() {
         assert_eq!(
@@ -76,7 +110,6 @@ mod tests {
 
     #[test]
     fn whitespace_at_end_test() {
-        //TODO currently it does not chop whitespace off the end.
         assert_eq!(
             ("1d4", "roll"),
             split_command("!roll 1d4   ").expect("got parsing error")