fix: make admin command parsing more robust

There was a bug in how we parsed admin commands, apparently we never
tested if we could parse `!backlog X` or unknown admin commands.

This commit also make updates to the backlog command construction to
make sure that we don't try to access messages in the backlog that
don't exist.

src/bot.zig

@@ -140,10 +140,10 @@ pub const Bot = struct {
                 return .{ .JOIN = .{ .prefix = prefix, .channels = msg.channel } };
             },
             .backlog => |backlog| {
-                if (self.backlog.insertions == 0) {
+                if (self.backlog.len() == 0) {
                     return Error.NoMessage;
                 }
-                if (backlog.history > self.backlog.len()) {
+                if (backlog.history >= self.backlog.len()) {
                     return Error.NoMessage;
                 }
                 if (self.backlog.get_backwards(backlog.history)) |message| {

src/commands.zig

@@ -55,18 +55,22 @@ pub const AdminCommand = union(enum) {
             if (command.consume_str("status")) |_| {
                 return .status;
             }
-            if (command.consume_str("join").?.consume_char(' ')) |join| {
+            if (command.consume_str("join")) |join| {
-                if (join.rest[0] != '#') {
+                if (join.consume_space()) |channel| {
+                    if (channel.rest[0] != '#') {
                         return .{ .err = .{ .message = "channels must start with \"#\"" } };
                     }
                     return .{ .join = .{ .channel = join.rest } };
                 }
+            }
             if (command.consume_str("backlog")) |backlog| {
-                const history = std.fmt.parseInt(u16, backlog.rest, 10) catch |err| {
+                if (backlog.consume_space()) |history| {
-                    std.debug.print("failed to parse int ('{s}') with error: {}\n", .{ backlog.rest, err });
+                    const historyOffset = std.fmt.parseInt(u16, history.rest, 10) catch |err| {
+                        std.debug.print("failed to parse int ('{s}') with error: {}\n", .{ history.rest, err });
                         return null;
                     };
-                return .{ .backlog = .{ .history = history } };
+                    return .{ .backlog = .{ .history = historyOffset } };
+                }
             }
             std.log.debug("unknown command: \"{s}\"", .{command.rest});
         }
@@ -104,3 +108,16 @@ test "parse admin commands" {
         cmd,
     );
 }
+
+test "parse backlog admin commands" {
+    const cmd = AdminCommand.parse("!backlog 1") orelse unreachable;
+    try std.testing.expectEqual(
+        AdminCommand{ .backlog = .{ .history = 1 } },
+        cmd,
+    );
+}
+
+test "parse unknown admin commands" {
+    const cmd = AdminCommand.parse("!history 1");
+    try std.testing.expectEqual(null, cmd);
+}

src/main.zig

@@ -120,28 +120,38 @@ pub const BotAdapter = struct {
     }
 };
 
-// test "substitute" {
+test "substitute" {
-//     var bot_adapter = try BotAdapter.init(std.testing.allocator);
+    var bot_adapter = try BotAdapter.init(std.testing.allocator);
-//     defer bot_adapter.deinit();
+    defer bot_adapter.deinit();
-//     const prefix = zircon.Prefix{ .nick = "jassob", .user = "jassob", .host = "localhost" };
+    const prefix = zircon.Prefix{ .nick = "jassob", .user = "jassob", .host = "localhost" };
-//     const msg = zircon.Message{
+    const msg = zircon.Message{
-//         .PRIVMSG = .{
+        .PRIVMSG = .{
-//             .prefix = prefix,
+            .prefix = prefix,
-//             .targets = "#test",
+            .targets = "#test",
-//             .text = "hello world",
+            .text = "hello world",
-//         },
+        },
-//     };
+    };
-//     if (bot_adapter.callback(msg)) |_| {
+    if (bot_adapter.callback(msg)) |_| {
-//         @panic("unexpected response");
+        @panic("unexpected response");
-//     }
+    }
-//     const cmd_msg = zircon.Message{
+    const cmd_msg = zircon.Message{
-//         .PRIVMSG = .{
+        .PRIVMSG = .{
-//             .prefix = prefix,
+            .prefix = prefix,
-//             .targets = "#test",
+            .targets = "#test",
-//             .text = "s/world/zig/",
+            .text = "s/world/zig/",
-//         },
+        },
-//     };
+    };
-//     const response = bot_adapter.callback(cmd_msg);
+    const response = bot_adapter.callback(cmd_msg);
-//     try std.testing.expect(response != null);
+    try std.testing.expect(response != null);
-//     try std.testing.expectEqualStrings("jassob: \"hello zig\"", response.?.PRIVMSG.text);
+    try std.testing.expectEqualStrings("jassob: \"hello zig\"", response.?.PRIVMSG.text);
-// }
+}
+
+test "get empty backlog message" {
+    var bot_adapter = try BotAdapter.init(std.testing.allocator);
+    defer bot_adapter.deinit();
+    const prefix = zircon.Prefix{ .nick = "jassob", .user = "jassob", .host = "localhost" };
+    const msg = zircon.Message{
+        .PRIVMSG = .{ .prefix = prefix, .targets = "#eru-admin", .text = "!backlog 0" },
+    };
+    try std.testing.expectEqualDeep("no matching message", bot_adapter.callback(msg).?.PRIVMSG.text);
+}

src/parser.zig

@@ -9,10 +9,7 @@ pub const Parser = struct {
     rest: []const u8,
     end_idx: usize,
 
-    pub fn seek(self: *const Parser, skip: usize) Parser {
+    // Initializes a Parser for s.
-        return .{ .original = self.original, .rest = self.rest[skip..], .end_idx = self.end_idx + skip };
-    }
-
     pub fn init(s: []const u8) Parser {
         return .{
             .original = s,
@@ -21,6 +18,25 @@ pub const Parser = struct {
         };
     }
 
+    // Seek the parser window of the text forward skip bytes and return a new Parser.
+    pub fn seek(self: *const Parser, skip: usize) Parser {
+        return .{ .original = self.original, .rest = self.rest[skip..], .end_idx = self.end_idx + skip };
+    }
+
+    // Attempts to consume at least one whitespace character from the input text.
+    pub fn consume_space(self: *const Parser) ?Parser {
+        if (!std.ascii.isWhitespace(self.rest[0])) {
+            return null;
+        }
+        for (self.rest[1..], 1..) |c, idx| {
+            if (!std.ascii.isWhitespace(c)) {
+                return self.seek(idx);
+            }
+        }
+        return self.seek(self.rest.len);
+    }
+
+    // Attempts to consume a character c.
     pub fn consume_char(self: *const Parser, c: u8) ?Parser {
         if (self.rest[0] != c) {
             return null;
@@ -28,6 +44,7 @@ pub const Parser = struct {
         return self.seek(1);
     }
 
+    // Attempts to consume a string s.
     pub fn consume_str(self: *const Parser, s: []const u8) ?Parser {
         const len = s.len;
         if (self.rest.len < len) {
@@ -39,16 +56,32 @@ pub const Parser = struct {
         return self.seek(len);
     }
 
+    // Finds the next occurrence of c (idx) in the current parser
+    // window and extracts it.
+    //
+    // Returns a new parser window that starts after idx and the
+    // extracted byte slice.
     pub fn take_until_char(self: *const Parser, c: u8) struct { Parser, []const u8 } {
         const idx = std.mem.indexOfScalar(u8, self.rest, c) orelse unreachable;
         return .{ self.seek(idx), self.rest[0..idx] };
     }
 
+    // Take the current character and advance the parser one step.
     pub fn take_char(self: *const Parser) struct { Parser, u8 } {
         return .{ self.seek(1), self.rest[0] };
     }
 
+    // Return the currently accepted text.
     pub fn parsed(self: *const Parser) []const u8 {
         return self.original[0..self.end_idx];
     }
 };
+
+test "parser can skip whitespace" {
+    var parser = init("Hello,  World");
+    parser = parser.consume_str("Hello,").?;
+    parser = parser.consume_space().?;
+    parser = parser.consume_str("World").?;
+
+    try std.testing.expectEqual("Hello,  World", parser.parsed());
+}