From f75cc9b8453e8c67fc8e497cc1871479f14e8b4a Mon Sep 17 00:00:00 2001 From: CJ van den Berg Date: Tue, 23 Dec 2025 13:45:13 +0100 Subject: [PATCH] refactor: use command.command_names index and improve logging of commands We now use the command name index more consistently to allow for pre-allocating command IDs and better logging when commands are not found. This is a major, but hopefully non-breaking, change to command execution. --- src/command.zig | 84 ++++++++++++++++++++++------------------- src/keybind/keybind.zig | 14 +++---- src/tui/editor.zig | 6 +-- src/tui/tui.zig | 6 +-- 4 files changed, 57 insertions(+), 53 deletions(-) diff --git a/src/command.zig b/src/command.zig index 82d097b..ae41689 100644 --- a/src/command.zig +++ b/src/command.zig @@ -3,6 +3,7 @@ const tp = @import("thespian"); const log = @import("log"); const cbor = @import("cbor"); +pub var log_execute: bool = false; pub var context_check: ?*const fn () void = null; pub const ID = usize; @@ -69,15 +70,10 @@ pub fn Closure(comptime T: type) type { } pub fn register(self: *Self) !void { - if (command_names.get(self.vtbl.name)) |id| { - self.vtbl.id = id; - reAddCommand(&self.vtbl) catch |e| return log.err("cmd", "reAddCommand", e); - // log.print("cmd", "reAddCommand({s}) => {d}", .{ self.vtbl.name, self.vtbl.id }); - } else { - self.vtbl.id = try addCommand(&self.vtbl); - command_names.put(self.vtbl.name, self.vtbl.id) catch |e| return log.err("cmd", "addCommand", e); - // log.print("cmd", "addCommand({s}) => {d}", .{ self.vtbl.name, self.vtbl.id }); - } + if (command_names.get(self.vtbl.name)) |id| + reAddCommand(id, &self.vtbl) catch |e| return log.err("cmd", "reAddCommand", e) + else + addCommand(&self.vtbl); } pub fn unregister(self: *Self) void { @@ -100,21 +96,31 @@ pub var commands: CommandTable = .empty; var command_names: std.StringHashMap(ID) = std.StringHashMap(ID).init(command_table_allocator); const command_table_allocator = std.heap.c_allocator; -fn addCommand(cmd: *Vtable) !ID { - try commands.append(command_table_allocator, cmd); - return commands.items.len - 1; +fn assignCommandId(name: []const u8) ID { + commands.append(command_table_allocator, null) catch |e| std.debug.panic("assignCommandId: {t}", .{e}); + const id = commands.items.len - 1; + command_names.put(name, id) catch |e| std.debug.panic("assignCommandId: {t}", .{e}); + return id; } -fn reAddCommand(cmd: *Vtable) !void { - if (commands.items[cmd.id] != null) return error.DuplicateCommand; - commands.items[cmd.id] = cmd; +fn addCommand(cmd: *Vtable) void { + commands.append(command_table_allocator, cmd) catch |e| std.debug.panic("addCommand: {t}", .{e}); + const id = commands.items.len - 1; + cmd.id = id; + command_names.put(cmd.name, id) catch |e| std.debug.panic("assignCommandId: {t}", .{e}); +} + +fn reAddCommand(id: ID, cmd: *Vtable) !void { + cmd.id = id; + if (commands.items[id] != null) return error.DuplicateCommand; + commands.items[id] = cmd; } pub fn removeCommand(id: ID) void { commands.items[id] = null; } -pub fn execute(id: ID, ctx: Context) tp.result { +pub fn execute(id: ID, name: []const u8, ctx: Context) tp.result { if (tp.env.get().enabled(tp.channel.debug)) trace: { var iter = ctx.args.buf; var len = cbor.decodeArrayHeader(&iter) catch break :trace; @@ -140,20 +146,26 @@ pub fn execute(id: ID, ctx: Context) tp.result { } if (context_check) |check| check(); if (id >= commands.items.len) - return tp.exit_fmt("CommandNotFound: {d}", .{id}); + return notFoundError(id, name); const cmd = commands.items[id]; if (cmd) |p| { - // var buf: [tp.max_message_size]u8 = undefined; - // log.print("cmd", "execute({s}) {s}", .{ p.name, ctx.args.to_json(&buf) catch "" }) catch |e| return tp.exit_error(e, @errorReturnTrace()); + if (log_execute) { + var buf: [tp.max_message_size]u8 = undefined; + log.print("cmd", "execute({d}) {s} {s}", .{ id, p.name, if (ctx.args.buf.len > 0) ctx.args.to_json(&buf) catch "(error)" else "" }); + } return p.run(p, ctx); } else { - return tp.exit_fmt("CommandNotAvailable: {d}", .{id}); + return notFoundError(id, name); } } pub fn get_id(name: []const u8) ?ID { - var id: ?ID = null; - return get_id_cache(name, &id); + const id = get_name_id(name); + return if (commands.items[id]) |_| id else null; +} + +pub fn get_name_id(name: []const u8) ID { + return command_names.get(name) orelse assignCommandId(name); } pub fn get_name(id: ID) ?[]const u8 { @@ -164,19 +176,17 @@ pub fn get_name(id: ID) ?[]const u8 { tp.trace(tp.channel.debug, .{ "command", "get_name", "null", id }); } if (id >= commands.items.len) return null; - return (commands.items[id] orelse return null).name; + if (commands.items[id]) |cmd| return cmd.name; + var iter = command_names.iterator(); + while (iter.next()) |kv| if (kv.value_ptr.* == id) + return kv.key_ptr.*; + return null; } -pub fn get_id_cache(name: []const u8, id: *?ID) ?ID { - for (commands.items) |cmd| { - if (cmd) |p| - if (std.mem.eql(u8, p.name, name)) { - id.* = p.id; - return p.id; - }; - } - tp.trace(tp.channel.debug, .{ "command", "get_id_cache", "failed", name }); - return null; +pub fn get_id_cache(name: []const u8, cached_id: *?ID) ID { + const id = get_name_id(name); + cached_id.* = id; + return id; } pub fn get_description(id: ID) ?[]const u8 { @@ -201,14 +211,12 @@ const suppressed_errors = std.StaticStringMap(void).initComptime(.{ }); pub fn executeName(name: []const u8, ctx: Context) tp.result { - const id = get_id(name); - if (id) |id_| return execute(id_, ctx); - return notFoundError(name); + return execute(get_name_id(name), name, ctx); } -pub fn notFoundError(name: []const u8) !void { +fn notFoundError(id: ID, name: []const u8) !void { if (!suppressed_errors.has(name)) - return tp.exit_fmt("CommandNotFound: {s}", .{name}); + return tp.exit_fmt("CommandNotFound: {s}({d})", .{ name, id }); } fn CmdDef(comptime T: type) type { diff --git a/src/keybind/keybind.zig b/src/keybind/keybind.zig index 6ab0e75..31d58d9 100644 --- a/src/keybind/keybind.zig +++ b/src/keybind/keybind.zig @@ -349,19 +349,17 @@ const Command = struct { fn execute(self: *@This()) !void { const id = self.command_id orelse - command.get_id_cache(self.command, &self.command_id) orelse { - return command.notFoundError(self.command); - }; + command.get_id_cache(self.command, &self.command_id); var buf: [2048]u8 = undefined; @memcpy(buf[0..self.args.len], self.args); if (integer_argument) |int_arg| { if (cbor.match(self.args, .{}) catch false and has_integer_argument(id)) { integer_argument = null; - try command.execute(id, command.fmt(.{int_arg})); + try command.execute(id, self.command, command.fmt(.{int_arg})); return; } } - try command.execute(id, .{ .args = .{ .buf = buf[0..self.args.len] } }); + try command.execute(id, self.command, .{ .args = .{ .buf = buf[0..self.args.len] } }); } fn execute_const(self: *const @This()) void { @@ -662,11 +660,9 @@ const BindingSet = struct { if (enable_insert_events) self.send_insert_event(globals.insert_command, globals.input_buffer.items); const id = globals.insert_command_id orelse - command.get_id_cache(globals.insert_command, &globals.insert_command_id) orelse { - return tp.exit_error(error.InputTargetNotFound, null); - }; + command.get_id_cache(globals.insert_command, &globals.insert_command_id); if (!builtin.is_test) { - try command.execute(id, command.fmt(.{globals.input_buffer.items})); + try command.execute(id, globals.insert_command, command.fmt(.{globals.input_buffer.items})); } } } diff --git a/src/tui/editor.zig b/src/tui/editor.zig index 8e2d716..2e5bac1 100644 --- a/src/tui/editor.zig +++ b/src/tui/editor.zig @@ -5734,7 +5734,7 @@ pub const Editor = struct { pub fn goto_next_diagnostic(self: *Self, _: Context) Result { if (self.diagnostics.items.len == 0) { if (command.get_id("goto_next_file")) |id| - return command.execute(id, .{}); + return command.execute(id, "goto_next_file", .{}); return; } self.sort_diagnostics(); @@ -5750,7 +5750,7 @@ pub const Editor = struct { pub fn goto_prev_diagnostic(self: *Self, _: Context) Result { if (self.diagnostics.items.len == 0) { if (command.get_id("goto_prev_file")) |id| - return command.execute(id, .{}); + return command.execute(id, "goto_prev_file", .{}); return; } self.sort_diagnostics(); @@ -6224,7 +6224,7 @@ pub const Editor = struct { } fn add_default_symbol_triggers(self: *Self) void { - const id = command.get_id("completion") orelse return; + const id = command.get_name_id("completion"); self.add_symbol_trigger('.', id, .insert) catch {}; } diff --git a/src/tui/tui.zig b/src/tui/tui.zig index 9d12feb..b498101 100644 --- a/src/tui/tui.zig +++ b/src/tui/tui.zig @@ -416,7 +416,7 @@ fn receive_safe(self: *Self, from: tp.pid_ref, m: tp.message) !void { if (try m.match(.{ "cmd", tp.extract(&cmd) })) return command.executeName(cmd, ctx) catch |e| self.logger.err(cmd, e); if (try m.match(.{ "cmd", tp.extract(&cmd_id) })) - return command.execute(cmd_id, ctx) catch |e| self.logger.err("command", e); + return command.execute(cmd_id, command.get_name(cmd_id) orelse "(unknown)", ctx) catch |e| self.logger.err("command", e); var arg: []const u8 = undefined; @@ -426,7 +426,7 @@ fn receive_safe(self: *Self, from: tp.pid_ref, m: tp.message) !void { } if (try m.match(.{ "cmd", tp.extract(&cmd_id), tp.extract_cbor(&arg) })) { ctx.args = .{ .buf = arg }; - return command.execute(cmd_id, ctx) catch |e| self.logger.err("command", e); + return command.execute(cmd_id, command.get_name(cmd_id) orelse "(unknown)", ctx) catch |e| self.logger.err("command", e); } if (try m.match(.{"quit"})) { project_manager.shutdown(); @@ -736,7 +736,7 @@ fn dispatch_event(ctx: *anyopaque, cbor_msg: []const u8) void { fn handle_system_clipboard(self: *Self, text: []const u8) !void { if (command.get_id("mini_mode_paste")) |id| - return command.execute(id, command.fmt(.{text})); + return command.execute(id, "mini_mode_paste", command.fmt(.{text})); { const text_ = try clipboard_system_clipboard_text(self.allocator);