From d0c6ca3de81f4d3aa8cc56325b4832a4819b5cfa Mon Sep 17 00:00:00 2001
From: projectmoon <projectmoon@agnos.is>
Date: Sat, 30 Jan 2021 22:13:06 +0000
Subject: [PATCH 1/3] Print out how many commands failed in a multi-command
 scenario.

The number of failing commands are now printed out when at least one
command in a multi-command execution fails. This commit does not
introduce printing out WHICH commands failed, nor their error
messages.

There was also some minor refactoring to move command response
handling into their own functions (one for single response, one for
multiple) so that the code is more readable.
---
 src/bot.rs | 86 ++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 58 insertions(+), 28 deletions(-)

diff --git a/src/bot.rs b/src/bot.rs
index feb1718..9445701 100644
--- a/src/bot.rs
+++ b/src/bot.rs
@@ -1,4 +1,4 @@
-use crate::commands::execute_command;
+use crate::commands::{execute_command, CommandResult, ExecutionError, ResponseExtractor};
 use crate::config::*;
 use crate::context::{Context, RoomContext};
 use crate::db::Database;
@@ -10,9 +10,10 @@ use matrix_sdk::Error as MatrixError;
 use matrix_sdk::{
     self,
     events::{
-        room::message::{MessageEventContent, NoticeMessageEventContent},
-        AnyMessageEventContent,
+        room::message::{MessageEventContent::Notice, NoticeMessageEventContent},
+        AnyMessageEventContent::RoomMessage,
     },
+    identifiers::RoomId,
     Client, ClientConfig, JoinedRoom, SyncSettings,
 };
 //use matrix_sdk_common_macros::async_trait;
@@ -64,6 +65,58 @@ fn extract_error_message(error: MatrixError) -> String {
     }
 }
 
+/// Handle responding to a single command being executed. Wil print
+/// out the full result of that command.
+async fn handle_single_result(
+    client: &Client,
+    cmd_result: &CommandResult,
+    respond_to: &str,
+    room_id: &RoomId,
+) {
+    let plain = cmd_result.message_plain(respond_to);
+    let html = cmd_result.message_html(respond_to);
+
+    let response = RoomMessage(Notice(NoticeMessageEventContent::html(plain, html)));
+
+    let result = client.room_send(&room_id, response, None).await;
+    if let Err(e) = result {
+        let message = extract_error_message(e);
+        error!("Error sending message: {}", message);
+    };
+}
+
+/// Handle responding to multiple commands being executed. Will print
+/// out how many commands succeeded and failed (if any failed).
+async fn handle_multiple_results(
+    client: &Client,
+    results: &[CommandResult],
+    respond_to: &str,
+    room_id: &RoomId,
+) {
+    // TODO we should also pass in the original command so we can
+    // properly link errors to commands in output.
+    let errors: Vec<&ExecutionError> = results.iter().filter_map(|r| r.as_ref().err()).collect();
+
+    let message = if errors.len() == 0 {
+        format!("{}: Executed {} commands", respond_to, results.len(),)
+    } else {
+        format!(
+            "{}: Executed {} commands ({} failed)",
+            respond_to,
+            results.len(),
+            errors.len()
+        )
+    };
+
+    let response = RoomMessage(Notice(NoticeMessageEventContent::html(&message, &message)));
+
+    let result = client.room_send(&room_id, response, None).await;
+    if let Err(e) = result {
+        let message = extract_error_message(e);
+        error!("Error sending message: {}", message);
+    };
+}
+
 impl DiceBot {
     /// Create a new dicebot with the given configuration and state
     /// actor. This function returns a Result because it is possible
@@ -139,34 +192,11 @@ impl DiceBot {
             results.push(cmd_result);
         }
 
-        use crate::commands::ResponseExtractor;
-
         if results.len() >= 1 {
             if results.len() == 1 {
-                let cmd_result = &results[0];
-                let response = AnyMessageEventContent::RoomMessage(MessageEventContent::Notice(
-                    NoticeMessageEventContent::html(
-                        cmd_result.message_plain(&sender_username),
-                        cmd_result.message_html(&sender_username),
-                    ),
-                ));
-
-                let result = self.client.room_send(&room_id, response, None).await;
-                if let Err(e) = result {
-                    let message = extract_error_message(e);
-                    error!("Error sending message: {}", message);
-                };
+                handle_single_result(&self.client, &results[0], sender_username, &room_id).await;
             } else if results.len() > 1 {
-                let message = format!("{}: Executed {} commands", sender_username, results.len());
-                let response = AnyMessageEventContent::RoomMessage(MessageEventContent::Notice(
-                    NoticeMessageEventContent::html(&message, &message),
-                ));
-
-                let result = self.client.room_send(&room_id, response, None).await;
-                if let Err(e) = result {
-                    let message = extract_error_message(e);
-                    error!("Error sending message: {}", message);
-                };
+                handle_multiple_results(&self.client, &results, sender_username, &room_id).await;
             }
 
             info!("[{}] {} executed: {}", room_name, sender_username, msg_body);
-- 
2.40.1


From a4e66a0ca681a4f56db379298861efd130733291 Mon Sep 17 00:00:00 2001
From: projectmoon <projectmoon@agnos.is>
Date: Sun, 31 Jan 2021 08:15:22 +0000
Subject: [PATCH 2/3] Basic output for multiple command failures.

Also replace newlines with <br/>s in HTML output.
---
 src/bot.rs      | 36 +++++++++++++++++++++++++-----------
 src/commands.rs |  4 ++--
 2 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/src/bot.rs b/src/bot.rs
index 9445701..f708282 100644
--- a/src/bot.rs
+++ b/src/bot.rs
@@ -89,26 +89,40 @@ async fn handle_single_result(
 /// out how many commands succeeded and failed (if any failed).
 async fn handle_multiple_results(
     client: &Client,
-    results: &[CommandResult],
+    results: &[(&str, CommandResult)],
     respond_to: &str,
     room_id: &RoomId,
 ) {
-    // TODO we should also pass in the original command so we can
-    // properly link errors to commands in output.
-    let errors: Vec<&ExecutionError> = results.iter().filter_map(|r| r.as_ref().err()).collect();
+    let errors: Vec<(&str, &ExecutionError)> = results
+        .into_iter()
+        .filter_map(|(cmd, result)| match result {
+            Err(e) => Some((*cmd, e)),
+            _ => None,
+        })
+        .collect();
 
     let message = if errors.len() == 0 {
-        format!("{}: Executed {} commands", respond_to, results.len(),)
+        format!("{}: Executed {} commands", respond_to, results.len())
     } else {
+        let failures: String = errors
+            .iter()
+            .map(|&(cmd, err)| format!("{}: {}", cmd, err))
+            .collect::<Vec<_>>()
+            .join("\n");
+
         format!(
-            "{}: Executed {} commands ({} failed)",
+            "{}: Executed {} commands ({} failed)\n\nFailures:\n{}",
             respond_to,
             results.len(),
-            errors.len()
+            errors.len(),
+            failures
         )
     };
 
-    let response = RoomMessage(Notice(NoticeMessageEventContent::html(&message, &message)));
+    // TODO Need separate code that handles message formatting and
+    // sending so we aren't littering codebase with replace calls.
+    let html = message.replace("\n", "<br/>");
+    let response = RoomMessage(Notice(NoticeMessageEventContent::html(&message, &html)));
 
     let result = client.room_send(&room_id, response, None).await;
     if let Err(e) = result {
@@ -175,7 +189,7 @@ impl DiceBot {
         let room_name = room.display_name().await.ok().unwrap_or_default();
         let room_id = room.room_id().clone();
 
-        let mut results = Vec::with_capacity(msg_body.lines().count());
+        let mut results: Vec<(&str, CommandResult)> = Vec::with_capacity(msg_body.lines().count());
 
         let commands = msg_body.trim().lines().filter(|line| line.starts_with("!"));
 
@@ -189,12 +203,12 @@ impl DiceBot {
             };
 
             let cmd_result = execute_command(&ctx).await;
-            results.push(cmd_result);
+            results.push((&command, cmd_result));
         }
 
         if results.len() >= 1 {
             if results.len() == 1 {
-                handle_single_result(&self.client, &results[0], sender_username, &room_id).await;
+                handle_single_result(&self.client, &results[0].1, sender_username, &room_id).await;
             } else if results.len() > 1 {
                 handle_multiple_results(&self.client, &results, sender_username, &room_id).await;
             }
diff --git a/src/commands.rs b/src/commands.rs
index fc6fe1f..654a2d0 100644
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -101,8 +101,8 @@ impl ResponseExtractor for CommandResult {
     /// Error message in bolded HTML.
     fn message_html(&self, username: &str) -> String {
         match self {
-            Ok(resp) => format!("<p>{}</p>\n{}", username, resp.html),
-            Err(e) => format!("<p>{}</p>\n{}", username, e.html()),
+            Ok(resp) => format!("<p>{}</p>\n{}", username, resp.html).replace("\n", "<br/>"),
+            Err(e) => format!("<p>{}</p>\n{}", username, e.html()).replace("\n", "<br/>"),
         }
     }
 }
-- 
2.40.1


From b3c4d8a38c5766ad336b5d63254a1d5ee7174dfd Mon Sep 17 00:00:00 2001
From: projectmoon <projectmoon@agnos.is>
Date: Sun, 31 Jan 2021 14:06:25 +0000
Subject: [PATCH 3/3] Centralize plain text formatting at point of message
 sending.

Instead of relying on all parts of the application to construct both
HTML and plain-text responses, we now construct only HTML responses,
and convert the HTML to plain text right before sending the message to
Matrix.

This is a first iteration, because the plain text has a few extra
newlines than it should, created by use of nested <p> tags.
---
 Cargo.lock                    | 223 ++++++++++++++++++++++++++++++++--
 Cargo.toml                    |   1 +
 src/bin/dicebot-cmd.rs        |   7 +-
 src/bot.rs                    |  47 ++-----
 src/commands.rs               |  42 ++-----
 src/commands/basic_rolling.rs |   3 +-
 src/commands/cofd.rs          |   3 +-
 src/commands/cthulhu.rs       |   6 +-
 src/commands/management.rs    |   6 +-
 src/commands/misc.rs          |   5 +-
 src/commands/variables.rs     |  20 ++-
 src/matrix.rs                 |  29 +++++
 12 files changed, 280 insertions(+), 112 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index c197ca5..78c1a2d 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -275,13 +275,14 @@ dependencies = [
  "dirs",
  "env_logger",
  "futures",
+ "html2text",
  "indoc",
  "itertools",
  "log",
  "matrix-sdk",
  "memmem",
  "nom",
- "phf",
+ "phf 0.7.24",
  "rand 0.7.3",
  "serde",
  "sled",
@@ -542,6 +543,16 @@ version = "0.1.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "a06f77d526c1a601b7c4cdd98f54b5eaabffc14d5f2f0296febdc7f357c6d3ba"
 
+[[package]]
+name = "futf"
+version = "0.1.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7c9c1ce3fa9336301af935ab852c437817d14cd33690446569392e65170aac3b"
+dependencies = [
+ "mac",
+ "new_debug_unreachable",
+]
+
 [[package]]
 name = "futures"
 version = "0.3.12"
@@ -767,6 +778,31 @@ dependencies = [
  "digest",
 ]
 
+[[package]]
+name = "html2text"
+version = "0.2.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a26379dcb715e237b96102a12b505c553e2bffa74bae2e54658748d298660ef1"
+dependencies = [
+ "html5ever",
+ "markup5ever_rcdom",
+ "unicode-width",
+]
+
+[[package]]
+name = "html5ever"
+version = "0.25.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "aafcf38a1a36118242d29b92e1b08ef84e67e4a5ed06e0a80be20e6a32bfed6b"
+dependencies = [
+ "log",
+ "mac",
+ "markup5ever",
+ "proc-macro2 1.0.24",
+ "quote 1.0.8",
+ "syn 1.0.60",
+]
+
 [[package]]
 name = "http"
 version = "0.2.3"
@@ -884,7 +920,7 @@ checksum = "61124eeebbd69b8190558df225adf7e4caafce0d743919e5d6b19652314ec5ec"
 dependencies = [
  "cfg-if 1.0.0",
  "js-sys",
- "time",
+ "time 0.2.25",
  "wasm-bindgen",
  "web-sys",
 ]
@@ -971,12 +1007,47 @@ dependencies = [
  "cfg-if 1.0.0",
 ]
 
+[[package]]
+name = "mac"
+version = "0.1.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
+
 [[package]]
 name = "maplit"
 version = "1.0.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "3e2e65a1a2e43cfcb47a895c4c8b10d1f4a61097f9f254f183aee60cad9c651d"
 
+[[package]]
+name = "markup5ever"
+version = "0.10.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "aae38d669396ca9b707bfc3db254bc382ddb94f57cc5c235f34623a669a01dab"
+dependencies = [
+ "log",
+ "phf 0.8.0",
+ "phf_codegen",
+ "serde",
+ "serde_derive",
+ "serde_json",
+ "string_cache",
+ "string_cache_codegen",
+ "tendril",
+]
+
+[[package]]
+name = "markup5ever_rcdom"
+version = "0.1.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f015da43bcd8d4f144559a3423f4591d69b8ce0652c905374da7205df336ae2b"
+dependencies = [
+ "html5ever",
+ "markup5ever",
+ "tendril",
+ "xml5ever",
+]
+
 [[package]]
 name = "matches"
 version = "0.1.8"
@@ -1135,6 +1206,12 @@ dependencies = [
  "tempfile",
 ]
 
+[[package]]
+name = "new_debug_unreachable"
+version = "1.0.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e4a24736216ec316047a1fc4252e27dabb04218aa4a3f37c6e7ddbf1f9782b54"
+
 [[package]]
 name = "nom"
 version = "5.1.2"
@@ -1285,7 +1362,26 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "b3da44b85f8e8dfaec21adae67f95d93244b2ecf6ad2a692320598dcc8e6dd18"
 dependencies = [
  "phf_macros",
- "phf_shared",
+ "phf_shared 0.7.24",
+]
+
+[[package]]
+name = "phf"
+version = "0.8.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3dfb61232e34fcb633f43d12c58f83c1df82962dcdfa565a4e866ffc17dafe12"
+dependencies = [
+ "phf_shared 0.8.0",
+]
+
+[[package]]
+name = "phf_codegen"
+version = "0.8.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "cbffee61585b0411840d3ece935cce9cb6321f01c45477d30066498cd5e1a815"
+dependencies = [
+ "phf_generator 0.8.0",
+ "phf_shared 0.8.0",
 ]
 
 [[package]]
@@ -1294,18 +1390,28 @@ version = "0.7.24"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "09364cc93c159b8b06b1f4dd8a4398984503483891b0c26b867cf431fb132662"
 dependencies = [
- "phf_shared",
+ "phf_shared 0.7.24",
  "rand 0.6.5",
 ]
 
+[[package]]
+name = "phf_generator"
+version = "0.8.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "17367f0cc86f2d25802b2c26ee58a7b23faeccf78a396094c13dced0d0182526"
+dependencies = [
+ "phf_shared 0.8.0",
+ "rand 0.7.3",
+]
+
 [[package]]
 name = "phf_macros"
 version = "0.7.24"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "bdb45e833315153371697760dad1831da99ce41884162320305e4f123ca3fe37"
 dependencies = [
- "phf_generator",
- "phf_shared",
+ "phf_generator 0.7.24",
+ "phf_shared 0.7.24",
  "proc-macro2 0.4.30",
  "quote 0.6.13",
  "syn 0.15.44",
@@ -1317,7 +1423,16 @@ version = "0.7.24"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "234f71a15de2288bcb7e3b6515828d22af7ec8598ee6d24c3b526fa0a80b67a0"
 dependencies = [
- "siphasher",
+ "siphasher 0.2.3",
+]
+
+[[package]]
+name = "phf_shared"
+version = "0.8.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c00cf8b9eafe68dde5e9eaa2cef8ee84a9336a47d566ec55ca16589633b65af7"
+dependencies = [
+ "siphasher 0.3.3",
 ]
 
 [[package]]
@@ -1405,6 +1520,12 @@ version = "0.2.10"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "ac74c624d6b2d21f425f752262f42188365d7b8ff1aff74c82e45136510a4857"
 
+[[package]]
+name = "precomputed-hash"
+version = "0.1.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "925383efa346730478fb4838dbe9137d2a47675ad789c546d150a6e1dd4ab31c"
+
 [[package]]
 name = "proc-macro-crate"
 version = "0.1.5"
@@ -1482,7 +1603,7 @@ dependencies = [
  "rand_isaac",
  "rand_jitter",
  "rand_os",
- "rand_pcg",
+ "rand_pcg 0.1.2",
  "rand_xorshift",
  "winapi",
 ]
@@ -1498,6 +1619,7 @@ dependencies = [
  "rand_chacha 0.2.2",
  "rand_core 0.5.1",
  "rand_hc 0.2.0",
+ "rand_pcg 0.2.1",
 ]
 
 [[package]]
@@ -1646,6 +1768,15 @@ dependencies = [
  "rand_core 0.4.2",
 ]
 
+[[package]]
+name = "rand_pcg"
+version = "0.2.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "16abd0c1b639e9eb4d7c50c0b8100b0d0f849be2349829c740fe8e6eb4816429"
+dependencies = [
+ "rand_core 0.5.1",
+]
+
 [[package]]
 name = "rand_xorshift"
 version = "0.1.1"
@@ -2082,6 +2213,12 @@ version = "0.2.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "0b8de496cf83d4ed58b6be86c3a275b8602f6ffe98d3024a869e124147a9a3ac"
 
+[[package]]
+name = "siphasher"
+version = "0.3.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "fa8f3741c7372e75519bd9346068370c9cdaabcc1f9599cbcf2a2719352286b7"
+
 [[package]]
 name = "slab"
 version = "0.4.2"
@@ -2185,6 +2322,31 @@ version = "0.1.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "213701ba3370744dcd1a12960caa4843b3d68b4d1c0a5d575e0d65b2ee9d16c0"
 
+[[package]]
+name = "string_cache"
+version = "0.8.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8ddb1139b5353f96e429e1a5e19fbaf663bddedaa06d1dbd49f82e352601209a"
+dependencies = [
+ "lazy_static",
+ "new_debug_unreachable",
+ "phf_shared 0.8.0",
+ "precomputed-hash",
+ "serde",
+]
+
+[[package]]
+name = "string_cache_codegen"
+version = "0.5.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f24c8e5e19d22a726626f1a5e16fe15b132dcf21d10177fa5a45ce7962996b97"
+dependencies = [
+ "phf_generator 0.8.0",
+ "phf_shared 0.8.0",
+ "proc-macro2 1.0.24",
+ "quote 1.0.8",
+]
+
 [[package]]
 name = "subtle"
 version = "2.4.0"
@@ -2239,6 +2401,17 @@ dependencies = [
  "winapi",
 ]
 
+[[package]]
+name = "tendril"
+version = "0.4.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a9ef557cb397a4f0a5a3a628f06515f78563f2209e64d47055d9dc6052bf5e33"
+dependencies = [
+ "futf",
+ "mac",
+ "utf-8",
+]
+
 [[package]]
 name = "termcolor"
 version = "1.1.2"
@@ -2277,6 +2450,16 @@ dependencies = [
  "once_cell",
 ]
 
+[[package]]
+name = "time"
+version = "0.1.43"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ca8a50ef2360fbd1eeb0ecd46795a87a19024eb4b53c5dc916ca1fd95fe62438"
+dependencies = [
+ "libc",
+ "winapi",
+]
+
 [[package]]
 name = "time"
 version = "0.2.25"
@@ -2485,6 +2668,12 @@ dependencies = [
  "tinyvec",
 ]
 
+[[package]]
+name = "unicode-width"
+version = "0.1.8"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9337591893a19b88d8d87f2cec1e73fad5cdfd10e5a6f349f498ad6ea2ffb1e3"
+
 [[package]]
 name = "unicode-xid"
 version = "0.1.0"
@@ -2525,6 +2714,12 @@ dependencies = [
  "percent-encoding",
 ]
 
+[[package]]
+name = "utf-8"
+version = "0.7.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "05e42f7c18b8f902290b009cde6d651262f956c98bc51bca4cd1d511c9cd85c7"
+
 [[package]]
 name = "uuid"
 version = "0.8.2"
@@ -2687,6 +2882,18 @@ dependencies = [
  "winapi",
 ]
 
+[[package]]
+name = "xml5ever"
+version = "0.16.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "0b1b52e6e8614d4a58b8e70cf51ec0cc21b256ad8206708bcff8139b5bbd6a59"
+dependencies = [
+ "log",
+ "mac",
+ "markup5ever",
+ "time 0.1.43",
+]
+
 [[package]]
 name = "zerocopy"
 version = "0.3.0"
diff --git a/Cargo.toml b/Cargo.toml
index 7c28ab4..a1d18bc 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -29,6 +29,7 @@ byteorder = "1.3"
 futures = "0.3"
 memmem = "0.1"
 bincode = "1.3"
+html2text = "0.2"
 phf = { version = "0.7", features = ["macros"] }
 matrix-sdk = { git = "https://github.com/matrix-org/matrix-rust-sdk", branch = "master" }
 
diff --git a/src/bin/dicebot-cmd.rs b/src/bin/dicebot-cmd.rs
index d5d4166..1c51489 100644
--- a/src/bin/dicebot-cmd.rs
+++ b/src/bin/dicebot-cmd.rs
@@ -25,9 +25,8 @@ async fn main() -> Result<(), BotError> {
         message_body: &input,
     };
 
-    println!(
-        "{}",
-        command.execute(&context).await.message_plain("fakeuser")
-    );
+    let message = command.execute(&context).await.message_html("fakeuser");
+    let message = html2text::from_read(message.as_bytes(), 80);
+    println!("{}", message.trim());
     Ok(())
 }
diff --git a/src/bot.rs b/src/bot.rs
index f708282..23b9d85 100644
--- a/src/bot.rs
+++ b/src/bot.rs
@@ -3,19 +3,11 @@ use crate::config::*;
 use crate::context::{Context, RoomContext};
 use crate::db::Database;
 use crate::error::BotError;
+use crate::matrix;
 use crate::state::DiceBotState;
 use dirs;
-use log::{error, info};
-use matrix_sdk::Error as MatrixError;
-use matrix_sdk::{
-    self,
-    events::{
-        room::message::{MessageEventContent::Notice, NoticeMessageEventContent},
-        AnyMessageEventContent::RoomMessage,
-    },
-    identifiers::RoomId,
-    Client, ClientConfig, JoinedRoom, SyncSettings,
-};
+use log::info;
+use matrix_sdk::{self, identifiers::RoomId, Client, ClientConfig, JoinedRoom, SyncSettings};
 //use matrix_sdk_common_macros::async_trait;
 use std::clone::Clone;
 use std::path::PathBuf;
@@ -56,15 +48,6 @@ fn create_client(config: &Config) -> Result<Client, BotError> {
     Ok(Client::new_with_config(homeserver_url, client_config)?)
 }
 
-/// Extracts more detailed error messages out of a matrix SDK error.
-fn extract_error_message(error: MatrixError) -> String {
-    use matrix_sdk::Error::RumaResponse;
-    match error {
-        RumaResponse(ruma_error) => ruma_error.to_string(),
-        _ => error.to_string(),
-    }
-}
-
 /// Handle responding to a single command being executed. Wil print
 /// out the full result of that command.
 async fn handle_single_result(
@@ -73,16 +56,8 @@ async fn handle_single_result(
     respond_to: &str,
     room_id: &RoomId,
 ) {
-    let plain = cmd_result.message_plain(respond_to);
     let html = cmd_result.message_html(respond_to);
-
-    let response = RoomMessage(Notice(NoticeMessageEventContent::html(plain, html)));
-
-    let result = client.room_send(&room_id, response, None).await;
-    if let Err(e) = result {
-        let message = extract_error_message(e);
-        error!("Error sending message: {}", message);
-    };
+    matrix::send_message(client, room_id, &html).await;
 }
 
 /// Handle responding to multiple commands being executed. Will print
@@ -106,7 +81,7 @@ async fn handle_multiple_results(
     } else {
         let failures: String = errors
             .iter()
-            .map(|&(cmd, err)| format!("{}: {}", cmd, err))
+            .map(|&(cmd, err)| format!("<strong>{}:</strong> {}", cmd, err))
             .collect::<Vec<_>>()
             .join("\n");
 
@@ -117,18 +92,10 @@ async fn handle_multiple_results(
             errors.len(),
             failures
         )
+        .replace("\n", "<br/>")
     };
 
-    // TODO Need separate code that handles message formatting and
-    // sending so we aren't littering codebase with replace calls.
-    let html = message.replace("\n", "<br/>");
-    let response = RoomMessage(Notice(NoticeMessageEventContent::html(&message, &html)));
-
-    let result = client.room_send(&room_id, response, None).await;
-    if let Err(e) = result {
-        let message = extract_error_message(e);
-        error!("Error sending message: {}", message);
-    };
+    matrix::send_message(client, room_id, &message).await;
 }
 
 impl DiceBot {
diff --git a/src/commands.rs b/src/commands.rs
index 654a2d0..c2aa26a 100644
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -23,22 +23,16 @@ pub enum CommandError {
 }
 
 /// A successfully executed command returns a message to be sent back
-/// to the user in both plain text and HTML, one of which will be
-/// displayed in the user's client depending on its capabilities.
+/// to the user in HTML (plain text used as a fallback by message
+/// formatter).
 #[derive(Debug)]
 pub struct Execution {
-    plain: String,
     html: String,
 }
 
 impl Execution {
-    pub fn new(plain: String, html: String) -> CommandResult {
-        Ok(Execution { plain, html })
-    }
-
-    /// Response message in plain text.
-    pub fn plain(&self) -> String {
-        self.plain.clone()
+    pub fn new(html: String) -> CommandResult {
+        Ok(Execution { html })
     }
 
     /// Response message in HTML.
@@ -47,10 +41,9 @@ impl Execution {
     }
 }
 
-/// Wraps a command execution failure. Provides plain-text and HTML
-/// formatting for any error message from the BotError type, similar
-/// to how Response provides formatting for successfully executed
-/// commands.
+/// Wraps a command execution failure. Provides HTML formatting for
+/// any error message from the BotError type, similar to how Execution
+/// provides formatting for successfully executed commands.
 #[derive(Error, Debug)]
 #[error("{0}")]
 pub struct ExecutionError(#[from] BotError);
@@ -62,11 +55,6 @@ impl From<crate::db::errors::DataError> for ExecutionError {
 }
 
 impl ExecutionError {
-    /// Error message in plain text.
-    pub fn plain(&self) -> String {
-        format!("{}", self.0)
-    }
-
     /// Error message in bolded HTML.
     pub fn html(&self) -> String {
         format!("<p><strong>{}</strong></p>", self.0)
@@ -80,29 +68,17 @@ pub type CommandResult = Result<Execution, ExecutionError>;
 /// Extract response messages out of a type, whether it is success or
 /// failure.
 pub trait ResponseExtractor {
-    /// Plain-text representation of the message, directly mentioning
-    /// the username.
-    fn message_plain(&self, username: &str) -> String;
-
     /// HTML representation of the message, directly mentioning the
     /// username.
     fn message_html(&self, username: &str) -> String;
 }
 
 impl ResponseExtractor for CommandResult {
-    /// Error message in plain text.
-    fn message_plain(&self, username: &str) -> String {
-        match self {
-            Ok(resp) => format!("{}\n{}", username, resp.plain()),
-            Err(e) => format!("{}\n{}", username, e.plain()),
-        }
-    }
-
     /// Error message in bolded HTML.
     fn message_html(&self, username: &str) -> String {
         match self {
-            Ok(resp) => format!("<p>{}</p>\n{}", username, resp.html).replace("\n", "<br/>"),
-            Err(e) => format!("<p>{}</p>\n{}", username, e.html()).replace("\n", "<br/>"),
+            Ok(resp) => format!("<p>{}</p><p>{}</p>", username, resp.html).replace("\n", "<br/>"),
+            Err(e) => format!("<p>{}</p><p>{}</p>", username, e.html()).replace("\n", "<br/>"),
         }
     }
 }
diff --git a/src/commands/basic_rolling.rs b/src/commands/basic_rolling.rs
index e104f6d..26489b6 100644
--- a/src/commands/basic_rolling.rs
+++ b/src/commands/basic_rolling.rs
@@ -14,12 +14,11 @@ impl Command for RollCommand {
 
     async fn execute(&self, _ctx: &Context<'_>) -> CommandResult {
         let roll = self.0.roll();
-        let plain = format!("Dice: {}\nResult: {}", self.0, roll);
         let html = format!(
             "<p><strong>Dice:</strong> {}</p><p><strong>Result</strong>: {}</p>",
             self.0, roll
         );
 
-        Execution::new(plain, html)
+        Execution::new(html)
     }
 }
diff --git a/src/commands/cofd.rs b/src/commands/cofd.rs
index 5e09697..7ac3a2a 100644
--- a/src/commands/cofd.rs
+++ b/src/commands/cofd.rs
@@ -15,12 +15,11 @@ impl Command for PoolRollCommand {
         let pool_with_ctx = DicePoolWithContext(&self.0, ctx);
         let rolled_pool = roll_pool(&pool_with_ctx).await?;
 
-        let plain = format!("Pool: {}\nResult: {}", rolled_pool, rolled_pool.roll);
         let html = format!(
             "<p><strong>Pool:</strong> {}</p><p><strong>Result</strong>: {}</p>",
             rolled_pool, rolled_pool.roll
         );
 
-        Execution::new(plain, html)
+        Execution::new(html)
     }
 }
diff --git a/src/commands/cthulhu.rs b/src/commands/cthulhu.rs
index f862d9c..c6d63a1 100644
--- a/src/commands/cthulhu.rs
+++ b/src/commands/cthulhu.rs
@@ -15,13 +15,12 @@ impl Command for CthRoll {
         let roll_with_ctx = DiceRollWithContext(&self.0, ctx);
         let executed_roll = regular_roll(&roll_with_ctx).await?;
 
-        let plain = format!("Roll: {}\nResult: {}", executed_roll, executed_roll.roll);
         let html = format!(
             "<p><strong>Roll:</strong> {}</p><p><strong>Result</strong>: {}</p>",
             executed_roll, executed_roll.roll
         );
 
-        Execution::new(plain, html)
+        Execution::new(html)
     }
 }
 
@@ -36,12 +35,11 @@ impl Command for CthAdvanceRoll {
     async fn execute(&self, _ctx: &Context<'_>) -> CommandResult {
         //TODO this will be converted to a result when supporting variables.
         let roll = self.0.roll();
-        let plain = format!("Roll: {}\nResult: {}", self.0, roll);
         let html = format!(
             "<p><strong>Roll:</strong> {}</p><p><strong>Result</strong>: {}</p>",
             self.0, roll
         );
 
-        Execution::new(plain, html)
+        Execution::new(html)
     }
 }
diff --git a/src/commands/management.rs b/src/commands/management.rs
index fc76a19..8f0bbc1 100644
--- a/src/commands/management.rs
+++ b/src/commands/management.rs
@@ -25,9 +25,7 @@ impl Command for ResyncCommand {
         )
         .await?;
 
-        let plain = "Room information resynced".to_string();
-        let html = "<p>Room information resynced.</p>".to_string();
-
-        Execution::new(plain, html)
+        let message = "Room information resynced.".to_string();
+        Execution::new(message)
     }
 }
diff --git a/src/commands/misc.rs b/src/commands/misc.rs
index 2e0c70c..aede558 100644
--- a/src/commands/misc.rs
+++ b/src/commands/misc.rs
@@ -17,8 +17,7 @@ impl Command for HelpCommand {
             _ => "There is no help for this topic",
         };
 
-        let plain = format!("Help: {}", help);
-        let html = format!("<p><strong>Help:</strong> {}", help.replace("\n", "<br/>"));
-        Execution::new(plain, html)
+        let html = format!("<strong>Help:</strong> {}", help.replace("\n", "<br/>"));
+        Execution::new(html)
     }
 }
diff --git a/src/commands/variables.rs b/src/commands/variables.rs
index 0f49cae..91d12d5 100644
--- a/src/commands/variables.rs
+++ b/src/commands/variables.rs
@@ -24,12 +24,11 @@ impl Command for GetAllVariablesCommand {
         variable_list.sort();
 
         let value = variable_list.join("\n");
-        let plain = format!("Variables:\n{}", value);
         let html = format!(
-            "<p><strong>Variables:</strong><br/>{}",
+            "<strong>Variables:</strong><br/>{}",
             value.replace("\n", "<br/>")
         );
-        Execution::new(plain, html)
+        Execution::new(html)
     }
 }
 
@@ -52,9 +51,8 @@ impl Command for GetVariableCommand {
             Err(e) => return Err(e.into()),
         };
 
-        let plain = format!("Variable: {}", value);
-        let html = format!("<p><strong>Variable:</strong> {}", value);
-        Execution::new(plain, html)
+        let html = format!("<strong>Variable:</strong> {}", value);
+        Execution::new(html)
     }
 }
 
@@ -74,9 +72,8 @@ impl Command for SetVariableCommand {
         ctx.db.variables.set_user_variable(&key, name, value)?;
 
         let content = format!("{} = {}", name, value);
-        let plain = format!("Set Variable: {}", content);
-        let html = format!("<p><strong>Set Variable:</strong> {}", content);
-        Execution::new(plain, html)
+        let html = format!("<strong>Set Variable:</strong> {}", content);
+        Execution::new(html)
     }
 }
 
@@ -99,8 +96,7 @@ impl Command for DeleteVariableCommand {
             Err(e) => return Err(e.into()),
         };
 
-        let plain = format!("Remove Variable: {}", value);
-        let html = format!("<p><strong>Remove Variable:</strong> {}", value);
-        Execution::new(plain, html)
+        let html = format!("<strong>Remove Variable:</strong> {}", value);
+        Execution::new(html)
     }
 }
diff --git a/src/matrix.rs b/src/matrix.rs
index 94e3244..6dfec76 100644
--- a/src/matrix.rs
+++ b/src/matrix.rs
@@ -1,5 +1,20 @@
+use log::error;
+use matrix_sdk::events::{
+    room::message::{MessageEventContent::Notice, NoticeMessageEventContent},
+    AnyMessageEventContent::RoomMessage,
+};
+use matrix_sdk::Error as MatrixError;
 use matrix_sdk::{identifiers::RoomId, Client};
 
+/// Extracts more detailed error messages out of a matrix SDK error.
+fn extract_error_message(error: MatrixError) -> String {
+    use matrix_sdk::Error::RumaResponse;
+    match error {
+        RumaResponse(ruma_error) => ruma_error.to_string(),
+        _ => error.to_string(),
+    }
+}
+
 /// Retrieve a list of users in a given room.
 pub async fn get_users_in_room(client: &Client, room_id: &RoomId) -> Vec<String> {
     if let Some(joined_room) = client.get_joined_room(room_id) {
@@ -21,3 +36,17 @@ pub async fn get_users_in_room(client: &Client, room_id: &RoomId) -> Vec<String>
         vec![]
     }
 }
+
+pub async fn send_message(client: &Client, room_id: &RoomId, message: &str) {
+    let plain = html2text::from_read(message.as_bytes(), message.len());
+    let response = RoomMessage(Notice(NoticeMessageEventContent::html(
+        plain.trim(),
+        message,
+    )));
+
+    let result = client.room_send(&room_id, response, None).await;
+    if let Err(e) = result {
+        let message = extract_error_message(e);
+        error!("Error sending message: {}", message);
+    };
+}
-- 
2.40.1