From b2325b715ea2513b09696ce00876672d5d87396e Mon Sep 17 00:00:00 2001
From: gnuxie <gnuxie@element.io>
Date: Tue, 5 Jul 2022 12:14:46 +0100
Subject: [PATCH] Feedback from review

---
 src/commands/CommandHandler.ts   |  2 +-
 src/commands/DumpRulesCommand.ts | 10 +++++++---
 src/models/BanList.ts            |  2 +-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/commands/CommandHandler.ts b/src/commands/CommandHandler.ts
index c6d0866..3ed8fec 100644
--- a/src/commands/CommandHandler.ts
+++ b/src/commands/CommandHandler.ts
@@ -135,7 +135,7 @@ export async function handleCommand(roomId: string, event: { content: { body: st
                 "!mjolnir redact <event permalink>                                   - Redacts a message by permalink\n" +
                 "!mjolnir kick <user ID> [room alias/ID] [reason]                    - Kicks a user in a particular room or all protected rooms\n" +
                 "!mjolnir rules                                                      - Lists the rules currently in use by Mjolnir\n" +
-                "!mjolnir rules matching <entity>                                    - Lists the rules in use that will match this entity e.g. @foo:example.com will show all the user and server rules, including globs, that match them." +
+                "!mjolnir rules matching <user|room|server>                          - Lists the rules in use that will match this entity e.g. `!rules matching @foo:example.com` will show all the user and server rules, including globs, that match this user." +
                 "!mjolnir sync                                                       - Force updates of all lists and re-apply rules\n" +
                 "!mjolnir verify                                                     - Ensures Mjolnir can moderate all your rooms\n" +
                 "!mjolnir list create <shortcode> <alias localpart>                  - Creates a new ban list with the given shortcode and alias\n" +
diff --git a/src/commands/DumpRulesCommand.ts b/src/commands/DumpRulesCommand.ts
index 48bae10..41d45d2 100644
--- a/src/commands/DumpRulesCommand.ts
+++ b/src/commands/DumpRulesCommand.ts
@@ -21,12 +21,12 @@ import { htmlEscape } from "../utils";
 
 /**
  * List all of the rules that match a given entity.
- * The reaso why you want to test against all rules and not just e.g. user or server is because
+ * The reason why you want to test against all rules and not just e.g. user or server is because
  * there are situations where rules of different types can ban other entities e.g. server ACL can cause users to be banned.
  * @param roomId The room the command is from.
  * @param event The event containing the command.
  * @param mjolnir A mjolnir to fetch the watched lists from.
- * @param entity e.g. a user or room id.
+ * @param entity a user, room id or server.
  * @returns When a response has been sent to the command.
  */
 export async function execRulesMatchingCommand(roomId: string, event: any, mjolnir: Mjolnir, entity: string) {
@@ -42,10 +42,14 @@ export async function execRulesMatchingCommand(roomId: string, event: any, mjoln
         const matchesInfo = `Found ${matches.length} ` + (matches.length === 1 ? 'match:' : 'matches:');
         const shortcodeInfo = list.listShortcode ? ` (shortcode: ${htmlEscape(list.listShortcode)})` : '';
 
+        // FIXME: I feel like it already replaces pills, but
+        // just double check.
+        //await replaceRoomIdsWithPills()
         html += `<a href="${htmlEscape(list.roomRef)}">${htmlEscape(list.roomId)}</a>${shortcodeInfo} ${matchesInfo}<br/><ul>`;
         text += `${list.roomRef}${shortcodeInfo} ${matchesInfo}:\n`;
 
         for (const rule of matches) {
+            // If we know the rule kind, we will give it a readable name, otherwise just use its name.
             let ruleKind: string = rule.kind;
             if (ruleKind === RULE_USER) {
                 ruleKind = 'user';
@@ -62,8 +66,8 @@ export async function execRulesMatchingCommand(roomId: string, event: any, mjoln
     }
 
     if (text.length === 0) {
+        html += `No results for ${htmlEscape(entity)}`;
         text += `No results for ${entity}`;
-        html += `No results for ${entity}`;
     }
     const reply = RichReply.createFor(roomId, event, text, html);
     reply["msgtype"] = "m.notice";
diff --git a/src/models/BanList.ts b/src/models/BanList.ts
index 9e44217..3b56f7f 100644
--- a/src/models/BanList.ts
+++ b/src/models/BanList.ts
@@ -186,7 +186,7 @@ class BanList extends EventEmitter {
      * Return all of the rules in this list that will match the provided entity.
      * If the entity is a user, then we match the domain part against server rules too.
      * @param ruleKind The type of rule for the entity e.g. `RULE_USER`.
-     * @param entity The entity to test e.g. the user id or server name.
+     * @param entity The entity to test e.g. the user id, server name or a room id.
      * @returns All of the rules that match this entity.
      */
     public rulesMatchingEntity(entity: string, ruleKind?: string): ListRule[] {