Various improvements to bot responses.

- Do not display username pill with quoted HTML replies.
 - Do not attempt to create matrix.to link in plain text replies.
 - Move plain text formatting responsibility outside of matrix
   send_message function.

src/bot/command_execution.rs

@@ -18,7 +18,45 @@ pub(super) async fn handle_single_result(
     event_id: EventId,
 ) {
     let html = cmd_result.message_html(respond_to);
-    matrix::send_message(client, room.room_id(), &html, Some(event_id)).await;
+    let plain = cmd_result.message_plain(respond_to);
+    matrix::send_message(client, room.room_id(), (&html, &plain), Some(event_id)).await;
+}
+
+/// Format failure messages nicely in either HTML or plain text. If
+/// plain is true, plain-text will be returned. Otherwise, formatted
+/// HTML.
+fn format_failures(
+    errors: &[(&str, &BotError)],
+    commands_executed: usize,
+    respond_to: &str,
+    plain: bool,
+) -> String {
+    let respond_to = match plain {
+        true => respond_to.to_owned(),
+        false => format!(
+            "<a href=\"https://matrix.to/#/{}\">{}</a>",
+            respond_to, respond_to
+        ),
+    };
+
+    let failures: Vec<String> = errors
+        .iter()
+        .map(|&(cmd, err)| format!("<strong>{}:</strong> {}", cmd, err))
+        .collect();
+
+    let message = format!(
+        "{}: Executed {} commands ({} failed)\n\nFailures:\n{}",
+        respond_to,
+        commands_executed,
+        errors.len(),
+        failures.join("\n")
+    )
+    .replace("\n", "<br/>");
+
+    match plain {
+        true => html2text::from_read(message.as_bytes(), message.len()),
+        false => message,
+    }
 }
 
 /// Handle responding to multiple commands being executed. Will print
@@ -29,7 +67,7 @@ pub(super) async fn handle_multiple_results(
     respond_to: &str,
     room: &Joined,
 ) {
-    let respond_to = format!(
+    let user_pill = format!(
         "<a href=\"https://matrix.to/#/{}\">{}</a>",
         respond_to, respond_to
     );
@@ -42,25 +80,19 @@ pub(super) async fn handle_multiple_results(
         })
         .collect();
 
-    let message = if errors.len() == 0 {
+    let (message, plain) = if errors.len() == 0 {
-        format!("{}: Executed {} commands", respond_to, results.len())
+        (
-    } else {
+            format!("{}: Executed {} commands", user_pill, results.len()),
-        let failures: Vec<String> = errors
+            format!("{}: Executed {} commands", respond_to, results.len()),
-            .iter()
+        )
-            .map(|&(cmd, err)| format!("<strong>{}:</strong> {}", cmd, err))
+    } else {
-            .collect();
+        (
-
+            format_failures(&errors, results.len(), respond_to, false),
-        format!(
+            format_failures(&errors, results.len(), respond_to, true),
-            "{}: Executed {} commands ({} failed)\n\nFailures:\n{}",
-            respond_to,
-            results.len(),
-            errors.len(),
-            failures.join("\n")
         )
-        .replace("\n", "<br/>")
     };
 
-    matrix::send_message(client, room.room_id(), &message, None).await;
+    matrix::send_message(client, room.room_id(), (&message, &plain), None).await;
 }
 
 /// Create a context for command execution. Can fai if the room

src/commands/mod.rs

@@ -55,6 +55,8 @@ pub trait ResponseExtractor {
     /// HTML representation of the message, directly mentioning the
     /// username.
     fn message_html(&self, username: &str) -> String;
+
+    fn message_plain(&self, username: &str) -> String;
 }
 
 impl ResponseExtractor for ExecutionResult {
@@ -68,12 +70,23 @@ impl ResponseExtractor for ExecutionResult {
         );
 
         match self {
-            Ok(resp) => format!("<p>{}</p><p>{}</p>", username, resp.html).replace("\n", "<br/>"),
+            Ok(resp) => format!("<p>{}</p>", resp.html).replace("\n", "<br/>"),
-            Err(e) => {
+            Err(e) => format!("<p>{}: <strong>{}</strong></p>", username, e).replace("\n", "<br/>"),
-                format!("<p>{}</p><p><strong>{}</strong></p>", username, e).replace("\n", "<br/>")
-            }
         }
     }
+
+    fn message_plain(&self, username: &str) -> String {
+        let message = match self {
+            Ok(resp) => format!("{}", resp.html),
+            Err(e) => format!("{}", e),
+        };
+
+        format!(
+            "{}:\n{}",
+            username,
+            html2text::from_read(message.as_bytes(), message.len())
+        )
+    }
 }
 
 /// The trait that any command that can be executed must implement.
@@ -256,15 +269,6 @@ mod tests {
         assert_eq!(execution_allowed(&cmd, &ctx).is_err(), true);
     }
 
-    #[test]
-    fn command_result_extractor_creates_bubble() {
-        let result = Execution::success("test".to_string());
-        let message = result.message_html("@myuser:example.com");
-        assert!(message.contains(
-            "<a href=\"https://matrix.to/#/@myuser:example.com\">@myuser:example.com</a>"
-        ));
-    }
-
     #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
     async fn unrecognized_command() {
         let db_path = tempfile::NamedTempFile::new_in(".").unwrap();

src/matrix.rs

@@ -61,20 +61,22 @@ pub async fn get_rooms_for_user(
     Ok(rooms_for_user)
 }
 
+/// Send a message. The message is a tuple of HTML and plain text
+/// responses.
 pub async fn send_message(
     client: &Client,
     room_id: &RoomId,
-    message: &str,
+    message: (&str, &str),
     reply_to: Option<EventId>,
 ) {
+    let (html, plain) = message;
     let room = match client.get_joined_room(room_id) {
         Some(room) => room,
         _ => return,
     };
 
-    let plain = html2text::from_read(message.as_bytes(), message.len());
     let mut content = MessageEventContent::new(MessageType::Notice(
-        NoticeMessageEventContent::html(plain.trim(), message),
+        NoticeMessageEventContent::html(plain.trim(), html),
     ));
 
     content.relates_to = reply_to.map(|event_id| Relation::Reply {
@@ -86,7 +88,7 @@ pub async fn send_message(
     let result = room.send(content, None).await;
 
     if let Err(e) = result {
-        let message = extract_error_message(e);
+        let html = extract_error_message(e);
-        error!("Error sending message: {}", message);
+        error!("Error sending html: {}", html);
     };
 }