mirror of
https://github.com/getcompanion-ai/co-mono.git
synced 2026-04-16 01:03:49 +00:00
refactor: address PR feedback - merge setWidget, use KeyId for shortcuts
1. Merge setWidget and setWidgetComponent into single overloaded method
- Accepts either string[] or component factory function
- Uses single Map<string, Component> internally
- String arrays wrapped in Container with Text components
2. Use KeyId type for registerShortcut instead of plain string
- Import Key from @mariozechner/pi-tui
- Update plan-mode example to use Key.shift('p')
- Type-safe shortcut registration
3. Fix tool API docs
- Both built-in and custom tools can be enabled/disabled
- Removed incorrect 'custom tools always active' statement
4. Use matchesKey instead of matchShortcut (already done in rebase)
This commit is contained in:
parent
e3c2616713
commit
8ecb1d6c0b
11 changed files with 76 additions and 106 deletions
|
|
@ -93,7 +93,6 @@ function createNoOpUIContext(): HookUIContext {
|
|||
notify: () => {},
|
||||
setStatus: () => {},
|
||||
setWidget: () => {},
|
||||
setWidgetComponent: () => {},
|
||||
custom: async () => undefined as never,
|
||||
setEditorText: () => {},
|
||||
getEditorText: () => "",
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import { createRequire } from "node:module";
|
|||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import type { KeyId } from "@mariozechner/pi-tui";
|
||||
import { createJiti } from "jiti";
|
||||
import { getAgentDir } from "../../config.js";
|
||||
import type { HookMessage } from "../messages.js";
|
||||
|
|
@ -103,8 +104,8 @@ export interface HookFlag {
|
|||
* Keyboard shortcut registered by a hook.
|
||||
*/
|
||||
export interface HookShortcut {
|
||||
/** Shortcut string (e.g., "shift+p", "ctrl+shift+x") */
|
||||
shortcut: string;
|
||||
/** Key identifier (e.g., Key.shift("p"), "ctrl+x") */
|
||||
shortcut: KeyId;
|
||||
/** Description for help */
|
||||
description?: string;
|
||||
/** Handler function */
|
||||
|
|
@ -153,7 +154,7 @@ export interface LoadedHook {
|
|||
/** Flag values (set after CLI parsing) */
|
||||
flagValues: Map<string, boolean | string>;
|
||||
/** Keyboard shortcuts registered by this hook */
|
||||
shortcuts: Map<string, HookShortcut>;
|
||||
shortcuts: Map<KeyId, HookShortcut>;
|
||||
/** Set the send message handler for this hook's pi.sendMessage() */
|
||||
setSendMessageHandler: (handler: SendMessageHandler) => void;
|
||||
/** Set the append entry handler for this hook's pi.appendEntry() */
|
||||
|
|
@ -226,7 +227,7 @@ function createHookAPI(
|
|||
commands: Map<string, RegisteredCommand>;
|
||||
flags: Map<string, HookFlag>;
|
||||
flagValues: Map<string, boolean | string>;
|
||||
shortcuts: Map<string, HookShortcut>;
|
||||
shortcuts: Map<KeyId, HookShortcut>;
|
||||
setSendMessageHandler: (handler: SendMessageHandler) => void;
|
||||
setAppendEntryHandler: (handler: AppendEntryHandler) => void;
|
||||
setGetActiveToolsHandler: (handler: GetActiveToolsHandler) => void;
|
||||
|
|
@ -249,7 +250,7 @@ function createHookAPI(
|
|||
const commands = new Map<string, RegisteredCommand>();
|
||||
const flags = new Map<string, HookFlag>();
|
||||
const flagValues = new Map<string, boolean | string>();
|
||||
const shortcuts = new Map<string, HookShortcut>();
|
||||
const shortcuts = new Map<KeyId, HookShortcut>();
|
||||
|
||||
// Cast to HookAPI - the implementation is more general (string event names)
|
||||
// but the interface has specific overloads for type safety in hooks
|
||||
|
|
@ -300,7 +301,7 @@ function createHookAPI(
|
|||
return flagValues.get(name);
|
||||
},
|
||||
registerShortcut(
|
||||
shortcut: string,
|
||||
shortcut: KeyId,
|
||||
options: {
|
||||
description?: string;
|
||||
handler: (ctx: HookContext) => Promise<void> | void;
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@
|
|||
|
||||
import type { AgentMessage } from "@mariozechner/pi-agent-core";
|
||||
import type { ImageContent, Model } from "@mariozechner/pi-ai";
|
||||
import type { KeyId } from "@mariozechner/pi-tui";
|
||||
import { theme } from "../../modes/interactive/theme/theme.js";
|
||||
import type { ModelRegistry } from "../model-registry.js";
|
||||
import type { SessionManager } from "../session-manager.js";
|
||||
|
|
@ -52,7 +53,6 @@ const noOpUIContext: HookUIContext = {
|
|||
notify: () => {},
|
||||
setStatus: () => {},
|
||||
setWidget: () => {},
|
||||
setWidgetComponent: () => {},
|
||||
custom: async () => undefined as never,
|
||||
setEditorText: () => {},
|
||||
getEditorText: () => "",
|
||||
|
|
@ -223,11 +223,12 @@ export class HookRunner {
|
|||
* Conflicts with built-in shortcuts are skipped with a warning.
|
||||
* Conflicts between hooks are logged as warnings.
|
||||
*/
|
||||
getShortcuts(): Map<string, HookShortcut> {
|
||||
const allShortcuts = new Map<string, HookShortcut>();
|
||||
getShortcuts(): Map<KeyId, HookShortcut> {
|
||||
const allShortcuts = new Map<KeyId, HookShortcut>();
|
||||
for (const hook of this.hooks) {
|
||||
for (const [key, shortcut] of hook.shortcuts) {
|
||||
const normalizedKey = key.toLowerCase();
|
||||
// Normalize to lowercase for comparison (KeyId is string at runtime)
|
||||
const normalizedKey = key.toLowerCase() as KeyId;
|
||||
|
||||
// Check for built-in shortcut conflicts
|
||||
if (HookRunner.RESERVED_SHORTCUTS.has(normalizedKey)) {
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@
|
|||
|
||||
import type { AgentMessage } from "@mariozechner/pi-agent-core";
|
||||
import type { ImageContent, Model, TextContent, ToolResultMessage } from "@mariozechner/pi-ai";
|
||||
import type { Component, TUI } from "@mariozechner/pi-tui";
|
||||
import type { Component, KeyId, TUI } from "@mariozechner/pi-tui";
|
||||
import type { Theme } from "../../modes/interactive/theme/theme.js";
|
||||
import type { CompactionPreparation, CompactionResult } from "../compaction/index.js";
|
||||
import type { ExecOptions, ExecResult } from "../exec.js";
|
||||
|
|
@ -79,13 +79,13 @@ export interface HookUIContext {
|
|||
* Supports multi-line content. Pass undefined to clear.
|
||||
* Text can include ANSI escape codes for styling.
|
||||
*
|
||||
* For simple text displays, use this method. For custom components, use setWidgetComponent().
|
||||
* Accepts either an array of styled strings, or a factory function that creates a Component.
|
||||
*
|
||||
* @param key - Unique key to identify this widget (e.g., hook name)
|
||||
* @param lines - Array of lines to display, or undefined to clear
|
||||
* @param content - Array of lines to display, or undefined to clear
|
||||
*
|
||||
* @example
|
||||
* // Show a todo list
|
||||
* // Show a todo list with styled strings
|
||||
* ctx.ui.setWidget("plan-todos", [
|
||||
* theme.fg("accent", "Plan Progress:"),
|
||||
* "☑ " + theme.fg("muted", theme.strikethrough("Step 1: Read files")),
|
||||
|
|
@ -96,7 +96,7 @@ export interface HookUIContext {
|
|||
* // Clear the widget
|
||||
* ctx.ui.setWidget("plan-todos", undefined);
|
||||
*/
|
||||
setWidget(key: string, lines: string[] | undefined): void;
|
||||
setWidget(key: string, content: string[] | undefined): void;
|
||||
|
||||
/**
|
||||
* Set a custom component as a widget (above the editor, below "Working..." indicator).
|
||||
|
|
@ -107,21 +107,18 @@ export interface HookUIContext {
|
|||
* Components are rendered inline without taking focus - they cannot handle keyboard input.
|
||||
*
|
||||
* @param key - Unique key to identify this widget (e.g., hook name)
|
||||
* @param factory - Function that creates the component, or undefined to clear
|
||||
* @param content - Factory function that creates the component, or undefined to clear
|
||||
*
|
||||
* @example
|
||||
* // Show a custom progress component
|
||||
* ctx.ui.setWidgetComponent("my-progress", (tui, theme) => {
|
||||
* ctx.ui.setWidget("my-progress", (tui, theme) => {
|
||||
* return new MyProgressComponent(tui, theme);
|
||||
* });
|
||||
*
|
||||
* // Clear the widget
|
||||
* ctx.ui.setWidgetComponent("my-progress", undefined);
|
||||
* ctx.ui.setWidget("my-progress", undefined);
|
||||
*/
|
||||
setWidgetComponent(
|
||||
key: string,
|
||||
factory: ((tui: TUI, theme: Theme) => Component & { dispose?(): void }) | undefined,
|
||||
): void;
|
||||
setWidget(key: string, content: ((tui: TUI, theme: Theme) => Component & { dispose?(): void }) | undefined): void;
|
||||
|
||||
/**
|
||||
* Show a custom component with keyboard focus.
|
||||
|
|
@ -813,7 +810,7 @@ export interface HookAPI {
|
|||
|
||||
/**
|
||||
* Set the active tools by name.
|
||||
* Only built-in tools can be enabled/disabled. Custom tools are always active.
|
||||
* Both built-in and custom tools can be enabled/disabled.
|
||||
* Changes take effect on the next agent turn.
|
||||
* Note: This will invalidate prompt caching for the next request.
|
||||
*
|
||||
|
|
@ -871,11 +868,13 @@ export interface HookAPI {
|
|||
* Register a keyboard shortcut for this hook.
|
||||
* The handler is called when the shortcut is pressed in interactive mode.
|
||||
*
|
||||
* @param shortcut - Shortcut definition (e.g., "shift+p", "ctrl+shift+x")
|
||||
* @param shortcut - Key identifier (e.g., Key.shift("p"), "ctrl+x")
|
||||
* @param options - Shortcut configuration
|
||||
*
|
||||
* @example
|
||||
* pi.registerShortcut("shift+p", {
|
||||
* import { Key } from "@mariozechner/pi-tui";
|
||||
*
|
||||
* pi.registerShortcut(Key.shift("p"), {
|
||||
* description: "Toggle plan mode",
|
||||
* handler: async (ctx) => {
|
||||
* // toggle plan mode
|
||||
|
|
@ -883,7 +882,7 @@ export interface HookAPI {
|
|||
* });
|
||||
*/
|
||||
registerShortcut(
|
||||
shortcut: string,
|
||||
shortcut: KeyId,
|
||||
options: {
|
||||
/** Description shown in help */
|
||||
description?: string;
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@
|
|||
|
||||
import { Agent, type AgentTool, type ThinkingLevel } from "@mariozechner/pi-agent-core";
|
||||
import type { Model } from "@mariozechner/pi-ai";
|
||||
import type { KeyId } from "@mariozechner/pi-tui";
|
||||
import { join } from "path";
|
||||
import { getAgentDir } from "../config.js";
|
||||
import { AgentSession } from "./agent-session.js";
|
||||
|
|
@ -349,7 +350,7 @@ function createLoadedHooksFromDefinitions(definitions: Array<{ path?: string; fa
|
|||
const commands = new Map<string, any>();
|
||||
const flags = new Map<string, any>();
|
||||
const flagValues = new Map<string, boolean | string>();
|
||||
const shortcuts = new Map<string, any>();
|
||||
const shortcuts = new Map<KeyId, any>();
|
||||
let sendMessageHandler: (
|
||||
message: any,
|
||||
options?: { triggerTurn?: boolean; deliverAs?: "steer" | "followUp" },
|
||||
|
|
@ -389,7 +390,7 @@ function createLoadedHooksFromDefinitions(definitions: Array<{ path?: string; fa
|
|||
}
|
||||
},
|
||||
getFlag: (name: string) => flagValues.get(name),
|
||||
registerShortcut: (shortcut: string, options: any) => {
|
||||
registerShortcut: (shortcut: KeyId, options: any) => {
|
||||
shortcuts.set(shortcut, { shortcut, hookPath, ...options });
|
||||
},
|
||||
newSession: (options?: any) => newSessionHandler(options),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue