From 4f92a8ee1ee5ddb2505fcb33ff834f052ac512fb Mon Sep 17 00:00:00 2001 From: Ammar Date: Wed, 29 Oct 2025 00:39:43 +0000 Subject: [PATCH 1/8] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20make=20command?= =?UTF-8?q?=20palette=20a=20workspace=20switcher=20by=20default?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the command palette primarily a workspace switcher while keeping all commands accessible via prefix keys. This focuses the palette on the most common use case (switching workspaces) without sacrificing functionality. **Changes:** - Default view shows only workspace commands (switch, create, rename, etc.) - Type `>` prefix to see all commands across all sections - Type `/` prefix for slash commands (existing behavior) - Updated placeholder text to reflect new behavior **Code quality improvements:** - Extracted repeated state reset logic into `resetPaletteState()` helper - Exported command section names as `COMMAND_SECTIONS` constant - Removed unused `searchQuery` variable - Updated documentation and Storybook stories **Documentation:** - Added command palette section to docs/keybinds.md explaining three modes - Updated Storybook story with accurate feature descriptions _Generated with `cmux`_ --- docs/keybinds.md | 10 +++++ src/components/CommandPalette.stories.tsx | 17 ++++--- src/components/CommandPalette.tsx | 55 ++++++++++++++++------- src/utils/commands/sources.ts | 25 ++++++++--- 4 files changed, 75 insertions(+), 32 deletions(-) diff --git a/docs/keybinds.md b/docs/keybinds.md index 5a3c1d644..fedfebc1f 100644 --- a/docs/keybinds.md +++ b/docs/keybinds.md @@ -54,6 +54,16 @@ When documentation shows `Ctrl`, it means: | Open command palette | `Ctrl+Shift+P` | | Toggle sidebar | `Ctrl+P` | +### Command Palette + +The command palette (`Ctrl+Shift+P`) is primarily a **workspace switcher** by default: + +- **Default**: Shows only workspace commands (switch, create, rename, etc.) +- **`>` prefix**: Shows all commands (navigation, chat, modes, projects, etc.) +- **`/` prefix**: Shows slash command suggestions for inserting into chat + +This design keeps the palette focused on the most common use case (workspace switching) while still providing quick access to all commands when needed. + ## Tips - **Vim-inspired navigation**: We use `J`/`K` for next/previous navigation, similar to Vim diff --git a/src/components/CommandPalette.stories.tsx b/src/components/CommandPalette.stories.tsx index 90fd91ca0..13767d880 100644 --- a/src/components/CommandPalette.stories.tsx +++ b/src/components/CommandPalette.stories.tsx @@ -12,7 +12,7 @@ const mockCommands: CommandAction[] = [ id: "workspace.create", title: "Create New Workspace", subtitle: "Start a new workspace in this project", - section: "Workspace", + section: "Workspaces", keywords: ["new", "add", "workspace"], shortcutHint: "⌘N", run: () => action("command-executed")("workspace.create"), @@ -21,7 +21,7 @@ const mockCommands: CommandAction[] = [ id: "workspace.switch", title: "Switch Workspace", subtitle: "Navigate to a different workspace", - section: "Workspace", + section: "Workspaces", keywords: ["change", "go to", "workspace"], shortcutHint: "⌘P", run: () => action("command-executed")("workspace.switch"), @@ -30,7 +30,7 @@ const mockCommands: CommandAction[] = [ id: "workspace.delete", title: "Delete Workspace", subtitle: "Remove the current workspace", - section: "Workspace", + section: "Workspaces", keywords: ["remove", "delete", "workspace"], run: () => action("command-executed")("workspace.delete"), }, @@ -185,15 +185,14 @@ export const Default: Story = {
Features:
- • Type to filter commands by title, subtitle, or keywords + • By default, shows workspace switching commands +
• Type > to see all commands across all sections +
• Type / to see slash commands for chat input
- • Use ↑↓ arrow keys to navigate -
- • Press Enter to execute a command + • Use ↑↓ arrow keys to navigate, Enter to execute
• Press Escape to close -
• Start with / to see slash commands -
• Commands are organized into sections (Workspace, Chat, Mode, Settings, Project, +
• Commands are organized into sections (Workspaces, Chat, Mode, Settings, Project, Help) diff --git a/src/components/CommandPalette.tsx b/src/components/CommandPalette.tsx index 307b20b73..74040ec96 100644 --- a/src/components/CommandPalette.tsx +++ b/src/components/CommandPalette.tsx @@ -42,32 +42,34 @@ export const CommandPalette: React.FC = ({ getSlashContext }>(null); const [promptError, setPromptError] = useState(null); + const resetPaletteState = useCallback(() => { + setActivePrompt(null); + setPromptError(null); + setQuery(""); + }, []); + // Close palette with Escape useEffect(() => { const onKey = (e: KeyboardEvent) => { if (matchesKeybind(e, KEYBINDS.CANCEL) && isOpen) { e.preventDefault(); - setActivePrompt(null); - setPromptError(null); - setQuery(""); + resetPaletteState(); close(); } }; window.addEventListener("keydown", onKey); return () => window.removeEventListener("keydown", onKey); - }, [isOpen, close]); + }, [isOpen, close, resetPaletteState]); // Reset state whenever palette visibility changes useEffect(() => { if (!isOpen) { - setActivePrompt(null); - setPromptError(null); - setQuery(""); + resetPaletteState(); } else { setPromptError(null); setQuery(""); } - }, [isOpen]); + }, [isOpen, resetPaletteState]); const rawActions = getActions(); @@ -200,7 +202,15 @@ export const CommandPalette: React.FC = ({ getSlashContext } satisfies { groups: PaletteGroup[]; emptyText: string | undefined }; } - const filtered = [...rawActions].sort((a, b) => { + // Filter actions based on prefix + const showAllCommands = q.startsWith(">"); + + // When no prefix is used, only show workspace-related commands + const actionsToShow = showAllCommands + ? rawActions + : rawActions.filter((action) => action.section === COMMAND_SECTIONS.WORKSPACES); + + const filtered = [...actionsToShow].sort((a, b) => { const ai = recentIndex.has(a.id) ? recentIndex.get(a.id)! : 9999; const bi = recentIndex.has(b.id) ? recentIndex.get(b.id)! : 9999; if (ai !== bi) return ai - bi; @@ -298,7 +308,10 @@ export const CommandPalette: React.FC = ({ getSlashContext }, [currentField, activePrompt]); const isSlashQuery = !currentField && query.trim().startsWith("/"); - const shouldUseCmdkFilter = currentField ? currentField.type === "select" : !isSlashQuery; + const isCommandQuery = !currentField && query.trim().startsWith(">"); + const shouldUseCmdkFilter = currentField + ? currentField.type === "select" + : !isSlashQuery && !isCommandQuery; let groups: PaletteGroup[] = generalResults.groups; let emptyText: string | undefined = generalResults.emptyText; @@ -355,9 +368,7 @@ export const CommandPalette: React.FC = ({ getSlashContext
{ - setActivePrompt(null); - setPromptError(null); - setQuery(""); + resetPaletteState(); close(); }} > @@ -365,6 +376,18 @@ export const CommandPalette: React.FC = ({ getSlashContext className="bg-separator border-border text-lighter font-primary w-[min(720px,92vw)] overflow-hidden rounded-lg border shadow-[0_10px_40px_rgba(0,0,0,0.4)]" onMouseDown={(e: React.MouseEvent) => e.stopPropagation()} shouldFilter={shouldUseCmdkFilter} + filter={(value, search) => { + // When using ">" prefix, filter using the text after ">" + if (isCommandQuery && search.startsWith(">")) { + const actualSearch = search.slice(1).trim().toLowerCase(); + if (!actualSearch) return 1; + if (value.toLowerCase().includes(actualSearch)) return 1; + return 0; + } + // Default cmdk filtering for other cases + if (value.toLowerCase().includes(search.toLowerCase())) return 1; + return 0; + }} > = ({ getSlashContext ? currentField.type === "text" ? (currentField.placeholder ?? "Type value…") : (currentField.placeholder ?? "Search options…") - : `Type a command… (${formatKeybind(KEYBINDS.CANCEL)} to close, ${formatKeybind(KEYBINDS.SEND_MESSAGE)} to send in chat)` + : `Switch workspaces or type > for all commands, / for slash commands…` } autoFocus onKeyDown={(e: React.KeyboardEvent) => { @@ -389,9 +412,7 @@ export const CommandPalette: React.FC = ({ getSlashContext } else if (e.key === "Escape") { e.preventDefault(); e.stopPropagation(); - setActivePrompt(null); - setPromptError(null); - setQuery(""); + resetPaletteState(); close(); } return; diff --git a/src/utils/commands/sources.ts b/src/utils/commands/sources.ts index 99fa6ba1a..1b8c62b91 100644 --- a/src/utils/commands/sources.ts +++ b/src/utils/commands/sources.ts @@ -44,13 +44,26 @@ export interface BuildSourcesParams { const THINKING_LEVELS: ThinkingLevel[] = ["off", "low", "medium", "high"]; +/** + * Command palette section names + * Exported for use in filtering and command organization + */ +export const COMMAND_SECTIONS = { + WORKSPACES: "Workspaces", + NAVIGATION: "Navigation", + CHAT: "Chat", + MODE: "Modes & Model", + HELP: "Help", + PROJECTS: "Projects", +} as const; + const section = { - workspaces: "Workspaces", - navigation: "Navigation", - chat: "Chat", - mode: "Modes & Model", - help: "Help", - projects: "Projects", + workspaces: COMMAND_SECTIONS.WORKSPACES, + navigation: COMMAND_SECTIONS.NAVIGATION, + chat: COMMAND_SECTIONS.CHAT, + mode: COMMAND_SECTIONS.MODE, + help: COMMAND_SECTIONS.HELP, + projects: COMMAND_SECTIONS.PROJECTS, }; export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandAction[]> { From a3162c2d50130add085dbbef3ac32ef9b672ad00 Mon Sep 17 00:00:00 2001 From: Ammar Date: Wed, 29 Oct 2025 01:05:34 +0000 Subject: [PATCH 2/8] refine: only show workspace switching by default, not mutations Changed the default filter to only show `ws:switch:*` commands, excluding workspace mutations like create, delete, and rename. These management commands are still accessible via the `>` prefix. **Why:** - Workspace switching is the primary use case - Create/delete/rename are mutations that should require explicit intent - Keeps the default palette extremely focused and fast **Changes:** - Filter by command ID prefix `ws:switch:*` instead of section name - Updated documentation to clarify "switching" vs "workspace management" - Updated Storybook story descriptions --- docs/keybinds.md | 6 +++--- src/components/CommandPalette.stories.tsx | 4 ++-- src/components/CommandPalette.tsx | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/keybinds.md b/docs/keybinds.md index fedfebc1f..9bdf262f7 100644 --- a/docs/keybinds.md +++ b/docs/keybinds.md @@ -58,11 +58,11 @@ When documentation shows `Ctrl`, it means: The command palette (`Ctrl+Shift+P`) is primarily a **workspace switcher** by default: -- **Default**: Shows only workspace commands (switch, create, rename, etc.) -- **`>` prefix**: Shows all commands (navigation, chat, modes, projects, etc.) +- **Default**: Shows only workspace switching commands (no mutations like create/delete/rename) +- **`>` prefix**: Shows all commands (navigation, chat, modes, projects, workspace management, etc.) - **`/` prefix**: Shows slash command suggestions for inserting into chat -This design keeps the palette focused on the most common use case (workspace switching) while still providing quick access to all commands when needed. +This design keeps the palette focused on the most common use case (switching between workspaces) while still providing quick access to all commands when needed. ## Tips diff --git a/src/components/CommandPalette.stories.tsx b/src/components/CommandPalette.stories.tsx index 13767d880..c207218bf 100644 --- a/src/components/CommandPalette.stories.tsx +++ b/src/components/CommandPalette.stories.tsx @@ -185,8 +185,8 @@ export const Default: Story = {
Features:
- • By default, shows workspace switching commands -
• Type > to see all commands across all sections + • By default, shows only workspace switching commands (no create/delete/rename) +
• Type > to see all commands including workspace management
• Type / to see slash commands for chat input
• Use ↑↓ arrow keys to navigate, Enter to execute diff --git a/src/components/CommandPalette.tsx b/src/components/CommandPalette.tsx index 74040ec96..2e6323279 100644 --- a/src/components/CommandPalette.tsx +++ b/src/components/CommandPalette.tsx @@ -205,10 +205,10 @@ export const CommandPalette: React.FC = ({ getSlashContext // Filter actions based on prefix const showAllCommands = q.startsWith(">"); - // When no prefix is used, only show workspace-related commands + // When no prefix is used, only show workspace switching commands (not mutations like create/delete/rename) const actionsToShow = showAllCommands ? rawActions - : rawActions.filter((action) => action.section === COMMAND_SECTIONS.WORKSPACES); + : rawActions.filter((action) => action.id.startsWith("ws:switch:")); const filtered = [...actionsToShow].sort((a, b) => { const ai = recentIndex.has(a.id) ? recentIndex.get(a.id)! : 9999; From aa1228e41b9a6690c4b0ba6cc55aaeefb2cdcb2b Mon Sep 17 00:00:00 2001 From: Ammar Date: Wed, 29 Oct 2025 01:43:21 +0000 Subject: [PATCH 3/8] fix: enable cmdk filtering for > queries Fixed an issue where typing `>abc` wouldn't filter commands because `shouldUseCmdkFilter` was set to false for command queries. Now cmdk filtering is enabled for `>` queries so users can search through all commands. Addresses Codex review comment about unsearchable "all commands" mode. **Before:** - `shouldFilter={false}` for `>` queries - Custom filter was never called - Typing `>abc` showed all commands unfiltered **After:** - `shouldFilter={true}` for `>` queries - Custom filter strips `>` prefix and filters normally - Typing `>abc` narrows down to matching commands --- src/components/CommandPalette.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/components/CommandPalette.tsx b/src/components/CommandPalette.tsx index 2e6323279..a9d656f86 100644 --- a/src/components/CommandPalette.tsx +++ b/src/components/CommandPalette.tsx @@ -309,9 +309,8 @@ export const CommandPalette: React.FC = ({ getSlashContext const isSlashQuery = !currentField && query.trim().startsWith("/"); const isCommandQuery = !currentField && query.trim().startsWith(">"); - const shouldUseCmdkFilter = currentField - ? currentField.type === "select" - : !isSlashQuery && !isCommandQuery; + // Enable cmdk filtering for all cases except slash queries (which we handle manually) + const shouldUseCmdkFilter = currentField ? currentField.type === "select" : !isSlashQuery; let groups: PaletteGroup[] = generalResults.groups; let emptyText: string | undefined = generalResults.emptyText; From 5ca6ba563daf3d2ef686f3fe9adba4be6a82a73d Mon Sep 17 00:00:00 2001 From: Ammar Date: Sat, 8 Nov 2025 02:10:55 +0000 Subject: [PATCH 4/8] fix: exclude switching commands when using > prefix Fixed bug where typing ">" still showed workspace switching commands along with all other commands. Now cleanly separates: - Default: only switching commands (ws:switch:*) - With ">": all other commands (mutations, nav, chat, etc.) EXCEPT switching This creates a clean mental model: the palette is a switcher by default, and ">" shows "everything else". **Changes:** - Fixed filter to exclude ws:switch:* when showAllCommands is true - Added comprehensive test suite (6 tests) in CommandPalette.test.ts - Updated docs to clarify "two modes" behavior - Updated AGENTS.md to require tests in repo when doing TDD **Tests verify:** - Default shows only 2 switching commands - Default excludes mutations (new/remove/rename) - ">" shows 6 commands, all non-switching - Clean separation: no overlap between modes - Together both modes cover all commands --- docs/AGENTS.md | 1 + docs/keybinds.md | 10 +- src/components/CommandPalette.stories.tsx | 19 ++-- src/components/CommandPalette.test.ts | 112 ++++++++++++++++++++++ src/components/CommandPalette.tsx | 5 +- 5 files changed, 134 insertions(+), 13 deletions(-) create mode 100644 src/components/CommandPalette.test.ts diff --git a/docs/AGENTS.md b/docs/AGENTS.md index fc4c68c35..73c331e1f 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -209,6 +209,7 @@ This project uses **Make** as the primary build orchestrator. See `Makefile` for **TDD is the preferred development style for agents.** +- **When asked to do TDD, write tests in the repository** - Create proper test files (e.g., `src/utils/foo.test.ts`) that run with `bun test` or `jest`, not temporary scripts in `/tmp`. Tests should be committed with the implementation. - Prefer relocated complex logic into places where they're easily tested - E.g. pure functions in `utils` are easier to test than complex logic in a React component - Strive for broad coverage with minimal tests diff --git a/docs/keybinds.md b/docs/keybinds.md index 9bdf262f7..63d438e46 100644 --- a/docs/keybinds.md +++ b/docs/keybinds.md @@ -56,13 +56,13 @@ When documentation shows `Ctrl`, it means: ### Command Palette -The command palette (`Ctrl+Shift+P`) is primarily a **workspace switcher** by default: +The command palette (`Ctrl+Shift+P`) has two modes: -- **Default**: Shows only workspace switching commands (no mutations like create/delete/rename) -- **`>` prefix**: Shows all commands (navigation, chat, modes, projects, workspace management, etc.) -- **`/` prefix**: Shows slash command suggestions for inserting into chat +- **Default (no prefix)**: Workspace switcher - shows only switching commands +- **`>` prefix**: Command mode - shows all other commands (create/delete/rename workspaces, navigation, chat, modes, projects, etc.) +- **`/` prefix**: Slash commands - shows slash command suggestions for inserting into chat -This design keeps the palette focused on the most common use case (switching between workspaces) while still providing quick access to all commands when needed. +This separation keeps the switcher clean and fast while making all other commands easily accessible via `>`. ## Tips diff --git a/src/components/CommandPalette.stories.tsx b/src/components/CommandPalette.stories.tsx index c207218bf..7ec208d01 100644 --- a/src/components/CommandPalette.stories.tsx +++ b/src/components/CommandPalette.stories.tsx @@ -183,17 +183,24 @@ export const Default: Story = { reopen it.

- Features: + Two Modes: +
• Default: Workspace switcher (only shows switching commands) +
•{" "} + + Type > + + : Command mode (shows all other commands) +
•{" "} + + Type / + + : Slash commands for chat input
- • By default, shows only workspace switching commands (no create/delete/rename) -
• Type > to see all commands including workspace management -
• Type / to see slash commands for chat input
• Use ↑↓ arrow keys to navigate, Enter to execute
• Press Escape to close -
• Commands are organized into sections (Workspaces, Chat, Mode, Settings, Project, - Help) +
• Commands organized into sections (Workspaces, Chat, Mode, Settings, Project, Help)
diff --git a/src/components/CommandPalette.test.ts b/src/components/CommandPalette.test.ts new file mode 100644 index 000000000..3c07c42bd --- /dev/null +++ b/src/components/CommandPalette.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, test } from "bun:test"; + +/** + * Tests for command palette filtering logic + * Verifies the "workspace switcher by default, commands with >" behavior + */ + +interface Action { + id: string; + title: string; + section: string; +} + +const mockActions: Action[] = [ + { id: "ws:switch:1", title: "Switch to Workspace A", section: "Workspaces" }, + { id: "ws:switch:2", title: "Switch to Workspace B", section: "Workspaces" }, + { id: "ws:new", title: "Create New Workspace", section: "Workspaces" }, + { id: "ws:remove", title: "Remove Current Workspace", section: "Workspaces" }, + { id: "ws:rename", title: "Rename Current Workspace", section: "Workspaces" }, + { id: "ws:open-terminal", title: "Open Workspace in Terminal", section: "Workspaces" }, + { id: "nav1", title: "Toggle Sidebar", section: "Navigation" }, + { id: "chat1", title: "Clear Chat", section: "Chat" }, +]; + +/** + * Simulates the filtering logic in CommandPalette.tsx + * This is the behavior we're testing to catch regressions + */ +function filterActions(query: string, actions: Action[]): Action[] { + const q = query.trim(); + + if (q.startsWith("/")) { + return []; // Slash commands handled separately + } + + const showAllCommands = q.startsWith(">"); + + // Default: show only workspace switching commands + // With ">": show all commands EXCEPT workspace switching + const filtered = showAllCommands + ? actions.filter((action) => !action.id.startsWith("ws:switch:")) + : actions.filter((action) => action.id.startsWith("ws:switch:")); + + return filtered; +} + +describe("CommandPalette filtering", () => { + test("default (no prefix) shows only workspace switching commands", () => { + const result = filterActions("", mockActions); + + expect(result).toHaveLength(2); + expect(result.every((a) => a.id.startsWith("ws:switch:"))).toBe(true); + expect(result.some((a) => a.id === "ws:switch:1")).toBe(true); + expect(result.some((a) => a.id === "ws:switch:2")).toBe(true); + }); + + test("default query excludes workspace mutations", () => { + const result = filterActions("", mockActions); + + expect(result.some((a) => a.id === "ws:new")).toBe(false); + expect(result.some((a) => a.id === "ws:remove")).toBe(false); + expect(result.some((a) => a.id === "ws:rename")).toBe(false); + }); + + test("> prefix shows all commands EXCEPT switching", () => { + const result = filterActions(">", mockActions); + + // Should show 6 commands (3 workspace mutations + 1 terminal + 1 nav + 1 chat) + expect(result).toHaveLength(6); + + // Should NOT include switching commands + expect(result.every((a) => !a.id.startsWith("ws:switch:"))).toBe(true); + + // Should include workspace mutations + expect(result.some((a) => a.id === "ws:new")).toBe(true); + expect(result.some((a) => a.id === "ws:remove")).toBe(true); + expect(result.some((a) => a.id === "ws:rename")).toBe(true); + + // Should include other sections + expect(result.some((a) => a.id === "nav1")).toBe(true); + expect(result.some((a) => a.id === "chat1")).toBe(true); + }); + + test(">query with text shows non-switching commands (cmdk filters further)", () => { + const result = filterActions(">new", mockActions); + + // Our filter shows all non-switching commands + // (cmdk's built-in filter will narrow this down by "new") + expect(result).toHaveLength(6); + expect(result.every((a) => !a.id.startsWith("ws:switch:"))).toBe(true); + }); + + test("/ prefix returns empty (slash commands handled separately)", () => { + const result = filterActions("/", mockActions); + expect(result).toHaveLength(0); + }); + + test("clean separation: switching XOR other commands", () => { + const defaultResult = filterActions("", mockActions); + const commandResult = filterActions(">", mockActions); + + // No overlap + const defaultIds = new Set(defaultResult.map((a) => a.id)); + const commandIds = new Set(commandResult.map((a) => a.id)); + const intersection = [...defaultIds].filter((id) => commandIds.has(id)); + + expect(intersection).toHaveLength(0); + + // Together they cover all non-slash commands + expect(defaultResult.length + commandResult.length).toBe(mockActions.length); + }); +}); diff --git a/src/components/CommandPalette.tsx b/src/components/CommandPalette.tsx index a9d656f86..c8610312e 100644 --- a/src/components/CommandPalette.tsx +++ b/src/components/CommandPalette.tsx @@ -205,9 +205,10 @@ export const CommandPalette: React.FC = ({ getSlashContext // Filter actions based on prefix const showAllCommands = q.startsWith(">"); - // When no prefix is used, only show workspace switching commands (not mutations like create/delete/rename) + // When no prefix is used, only show workspace switching commands + // When ">" prefix is used, show all commands EXCEPT workspace switching (show mutations, nav, chat, etc.) const actionsToShow = showAllCommands - ? rawActions + ? rawActions.filter((action) => !action.id.startsWith("ws:switch:")) : rawActions.filter((action) => action.id.startsWith("ws:switch:")); const filtered = [...actionsToShow].sort((a, b) => { From deab377efd38d9408940c7b06c8e9b34574e89a2 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sat, 8 Nov 2025 02:13:06 +0000 Subject: [PATCH 5/8] refactor: extract filtering logic to utility function Extracted command palette filtering logic to `commandPaletteFiltering.ts` to eliminate duplication between implementation and tests. **Changes:** - Created `filterCommandsByPrefix()` utility function - Updated CommandPalette.tsx to use the utility - Updated tests to test the actual utility (not duplicate logic) - Added comprehensive JSDoc documentation **Benefits:** - Single source of truth for filtering logic - Tests verify actual implementation, not duplicate - Logic can be reused if needed elsewhere - Better separation of concerns Addresses review feedback about duplication. --- src/components/CommandPalette.test.ts | 37 ++++++-------------------- src/components/CommandPalette.tsx | 11 +++----- src/utils/commandPaletteFiltering.ts | 38 +++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 37 deletions(-) create mode 100644 src/utils/commandPaletteFiltering.ts diff --git a/src/components/CommandPalette.test.ts b/src/components/CommandPalette.test.ts index 3c07c42bd..8b8ff34b8 100644 --- a/src/components/CommandPalette.test.ts +++ b/src/components/CommandPalette.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { filterCommandsByPrefix } from "@/utils/commandPaletteFiltering"; /** * Tests for command palette filtering logic @@ -22,31 +23,9 @@ const mockActions: Action[] = [ { id: "chat1", title: "Clear Chat", section: "Chat" }, ]; -/** - * Simulates the filtering logic in CommandPalette.tsx - * This is the behavior we're testing to catch regressions - */ -function filterActions(query: string, actions: Action[]): Action[] { - const q = query.trim(); - - if (q.startsWith("/")) { - return []; // Slash commands handled separately - } - - const showAllCommands = q.startsWith(">"); - - // Default: show only workspace switching commands - // With ">": show all commands EXCEPT workspace switching - const filtered = showAllCommands - ? actions.filter((action) => !action.id.startsWith("ws:switch:")) - : actions.filter((action) => action.id.startsWith("ws:switch:")); - - return filtered; -} - describe("CommandPalette filtering", () => { test("default (no prefix) shows only workspace switching commands", () => { - const result = filterActions("", mockActions); + const result = filterCommandsByPrefix("", mockActions); expect(result).toHaveLength(2); expect(result.every((a) => a.id.startsWith("ws:switch:"))).toBe(true); @@ -55,7 +34,7 @@ describe("CommandPalette filtering", () => { }); test("default query excludes workspace mutations", () => { - const result = filterActions("", mockActions); + const result = filterCommandsByPrefix("", mockActions); expect(result.some((a) => a.id === "ws:new")).toBe(false); expect(result.some((a) => a.id === "ws:remove")).toBe(false); @@ -63,7 +42,7 @@ describe("CommandPalette filtering", () => { }); test("> prefix shows all commands EXCEPT switching", () => { - const result = filterActions(">", mockActions); + const result = filterCommandsByPrefix(">", mockActions); // Should show 6 commands (3 workspace mutations + 1 terminal + 1 nav + 1 chat) expect(result).toHaveLength(6); @@ -82,7 +61,7 @@ describe("CommandPalette filtering", () => { }); test(">query with text shows non-switching commands (cmdk filters further)", () => { - const result = filterActions(">new", mockActions); + const result = filterCommandsByPrefix(">new", mockActions); // Our filter shows all non-switching commands // (cmdk's built-in filter will narrow this down by "new") @@ -91,13 +70,13 @@ describe("CommandPalette filtering", () => { }); test("/ prefix returns empty (slash commands handled separately)", () => { - const result = filterActions("/", mockActions); + const result = filterCommandsByPrefix("/", mockActions); expect(result).toHaveLength(0); }); test("clean separation: switching XOR other commands", () => { - const defaultResult = filterActions("", mockActions); - const commandResult = filterActions(">", mockActions); + const defaultResult = filterCommandsByPrefix("", mockActions); + const commandResult = filterCommandsByPrefix(">", mockActions); // No overlap const defaultIds = new Set(defaultResult.map((a) => a.id)); diff --git a/src/components/CommandPalette.tsx b/src/components/CommandPalette.tsx index c8610312e..434e882ef 100644 --- a/src/components/CommandPalette.tsx +++ b/src/components/CommandPalette.tsx @@ -5,6 +5,7 @@ import type { CommandAction } from "@/contexts/CommandRegistryContext"; import { formatKeybind, KEYBINDS, isEditableElement, matchesKeybind } from "@/utils/ui/keybinds"; import { getSlashCommandSuggestions } from "@/utils/slashCommands/suggestions"; import { CUSTOM_EVENTS, createCustomEvent } from "@/constants/events"; +import { filterCommandsByPrefix } from "@/utils/commandPaletteFiltering"; interface CommandPaletteProps { getSlashContext?: () => { providerNames: string[]; workspaceId?: string }; @@ -202,14 +203,8 @@ export const CommandPalette: React.FC = ({ getSlashContext } satisfies { groups: PaletteGroup[]; emptyText: string | undefined }; } - // Filter actions based on prefix - const showAllCommands = q.startsWith(">"); - - // When no prefix is used, only show workspace switching commands - // When ">" prefix is used, show all commands EXCEPT workspace switching (show mutations, nav, chat, etc.) - const actionsToShow = showAllCommands - ? rawActions.filter((action) => !action.id.startsWith("ws:switch:")) - : rawActions.filter((action) => action.id.startsWith("ws:switch:")); + // Filter actions based on prefix (extracted to utility for testing) + const actionsToShow = filterCommandsByPrefix(q, rawActions); const filtered = [...actionsToShow].sort((a, b) => { const ai = recentIndex.has(a.id) ? recentIndex.get(a.id)! : 9999; diff --git a/src/utils/commandPaletteFiltering.ts b/src/utils/commandPaletteFiltering.ts new file mode 100644 index 000000000..b7956c383 --- /dev/null +++ b/src/utils/commandPaletteFiltering.ts @@ -0,0 +1,38 @@ +/** + * Filtering logic for command palette + * Separates workspace switching from all other commands + */ + +export interface CommandActionMinimal { + id: string; +} + +/** + * Filters commands based on query prefix + * + * @param query - User's search query + * @param actions - All available actions + * @returns Filtered actions based on mode: + * - Default (no prefix): Only workspace switching commands (ws:switch:*) + * - ">" prefix: All commands EXCEPT workspace switching + * - "/" prefix: Empty (slash commands handled separately) + */ +export function filterCommandsByPrefix( + query: string, + actions: T[] +): T[] { + const q = query.trim(); + + // Slash commands are handled separately in the component + if (q.startsWith("/")) { + return []; + } + + const showAllCommands = q.startsWith(">"); + + // Default: show only workspace switching commands + // With ">": show all commands EXCEPT workspace switching + return showAllCommands + ? actions.filter((action) => !action.id.startsWith("ws:switch:")) + : actions.filter((action) => action.id.startsWith("ws:switch:")); +} From 113bf095a7209663359d6fe5ed9136695887e6e7 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sat, 8 Nov 2025 02:16:13 +0000 Subject: [PATCH 6/8] refactor: make tests property-based, not data-dependent Refactored tests to focus on behavioral properties rather than specific mock data. Tests now verify the filtering rules themselves, not hardcoded expectations about specific command counts. **Changes:** - Removed large mockActions array - Each test creates minimal data to test specific property - Tests organized by property (e.g., "default mode shows only ws:switch:*") - Tests are resilient to command list changes **Properties tested:** 1. Default mode only returns ws:switch:* commands 2. > mode never returns ws:switch:* commands 3. Modes partition the command space (no overlap, no gaps) 4. / prefix always returns empty 5. Text after > doesn't affect our filter (cmdk handles it) **Benefits:** - Tests won't break when commands are added/removed - Clear intent - each test verifies one property - More maintainable - minimal test data per property - Better test names describe what they verify Addresses review feedback about brittle tests. --- src/components/CommandPalette.test.ts | 167 +++++++++++++++----------- 1 file changed, 97 insertions(+), 70 deletions(-) diff --git a/src/components/CommandPalette.test.ts b/src/components/CommandPalette.test.ts index 8b8ff34b8..5a704912f 100644 --- a/src/components/CommandPalette.test.ts +++ b/src/components/CommandPalette.test.ts @@ -3,89 +3,116 @@ import { filterCommandsByPrefix } from "@/utils/commandPaletteFiltering"; /** * Tests for command palette filtering logic - * Verifies the "workspace switcher by default, commands with >" behavior + * Property-based tests that verify behavior regardless of specific command data */ -interface Action { - id: string; - title: string; - section: string; -} - -const mockActions: Action[] = [ - { id: "ws:switch:1", title: "Switch to Workspace A", section: "Workspaces" }, - { id: "ws:switch:2", title: "Switch to Workspace B", section: "Workspaces" }, - { id: "ws:new", title: "Create New Workspace", section: "Workspaces" }, - { id: "ws:remove", title: "Remove Current Workspace", section: "Workspaces" }, - { id: "ws:rename", title: "Rename Current Workspace", section: "Workspaces" }, - { id: "ws:open-terminal", title: "Open Workspace in Terminal", section: "Workspaces" }, - { id: "nav1", title: "Toggle Sidebar", section: "Navigation" }, - { id: "chat1", title: "Clear Chat", section: "Chat" }, -]; - describe("CommandPalette filtering", () => { - test("default (no prefix) shows only workspace switching commands", () => { - const result = filterCommandsByPrefix("", mockActions); - - expect(result).toHaveLength(2); - expect(result.every((a) => a.id.startsWith("ws:switch:"))).toBe(true); - expect(result.some((a) => a.id === "ws:switch:1")).toBe(true); - expect(result.some((a) => a.id === "ws:switch:2")).toBe(true); + describe("property: default mode shows only ws:switch:* commands", () => { + test("all results start with ws:switch:", () => { + const actions = [ + { id: "ws:switch:1" }, + { id: "ws:switch:2" }, + { id: "ws:new" }, + { id: "nav:toggle" }, + ]; + + const result = filterCommandsByPrefix("", actions); + + expect(result.every((a) => a.id.startsWith("ws:switch:"))).toBe(true); + }); + + test("excludes all non-switching commands", () => { + const actions = [ + { id: "ws:switch:1" }, + { id: "ws:new" }, + { id: "ws:remove" }, + { id: "nav:toggle" }, + ]; + + const result = filterCommandsByPrefix("", actions); + + expect(result.some((a) => !a.id.startsWith("ws:switch:"))).toBe(false); + }); }); - test("default query excludes workspace mutations", () => { - const result = filterCommandsByPrefix("", mockActions); - - expect(result.some((a) => a.id === "ws:new")).toBe(false); - expect(result.some((a) => a.id === "ws:remove")).toBe(false); - expect(result.some((a) => a.id === "ws:rename")).toBe(false); + describe("property: > mode shows all EXCEPT ws:switch:* commands", () => { + test("no results start with ws:switch:", () => { + const actions = [ + { id: "ws:switch:1" }, + { id: "ws:new" }, + { id: "nav:toggle" }, + { id: "chat:clear" }, + ]; + + const result = filterCommandsByPrefix(">", actions); + + expect(result.every((a) => !a.id.startsWith("ws:switch:"))).toBe(true); + }); + + test("includes all non-switching commands", () => { + const actions = [ + { id: "ws:switch:1" }, + { id: "ws:new" }, + { id: "ws:remove" }, + { id: "nav:toggle" }, + ]; + + const result = filterCommandsByPrefix(">", actions); + + // Should include workspace mutations + expect(result.some((a) => a.id === "ws:new")).toBe(true); + expect(result.some((a) => a.id === "ws:remove")).toBe(true); + // Should include navigation + expect(result.some((a) => a.id === "nav:toggle")).toBe(true); + // Should NOT include switching + expect(result.some((a) => a.id === "ws:switch:1")).toBe(false); + }); }); - test("> prefix shows all commands EXCEPT switching", () => { - const result = filterCommandsByPrefix(">", mockActions); - - // Should show 6 commands (3 workspace mutations + 1 terminal + 1 nav + 1 chat) - expect(result).toHaveLength(6); - - // Should NOT include switching commands - expect(result.every((a) => !a.id.startsWith("ws:switch:"))).toBe(true); - - // Should include workspace mutations - expect(result.some((a) => a.id === "ws:new")).toBe(true); - expect(result.some((a) => a.id === "ws:remove")).toBe(true); - expect(result.some((a) => a.id === "ws:rename")).toBe(true); - - // Should include other sections - expect(result.some((a) => a.id === "nav1")).toBe(true); - expect(result.some((a) => a.id === "chat1")).toBe(true); + describe("property: modes partition the command space", () => { + test("default + > modes cover all commands (no overlap, no gaps)", () => { + const actions = [ + { id: "ws:switch:1" }, + { id: "ws:switch:2" }, + { id: "ws:new" }, + { id: "ws:remove" }, + { id: "nav:toggle" }, + { id: "chat:clear" }, + ]; + + const defaultResult = filterCommandsByPrefix("", actions); + const commandResult = filterCommandsByPrefix(">", actions); + + // No overlap - disjoint sets + const defaultIds = new Set(defaultResult.map((a) => a.id)); + const commandIds = new Set(commandResult.map((a) => a.id)); + const intersection = [...defaultIds].filter((id) => commandIds.has(id)); + expect(intersection).toHaveLength(0); + + // No gaps - covers everything + expect(defaultResult.length + commandResult.length).toBe(actions.length); + }); }); - test(">query with text shows non-switching commands (cmdk filters further)", () => { - const result = filterCommandsByPrefix(">new", mockActions); + describe("property: / prefix always returns empty", () => { + test("returns empty array regardless of actions", () => { + const actions = [{ id: "ws:switch:1" }, { id: "ws:new" }, { id: "nav:toggle" }]; - // Our filter shows all non-switching commands - // (cmdk's built-in filter will narrow this down by "new") - expect(result).toHaveLength(6); - expect(result.every((a) => !a.id.startsWith("ws:switch:"))).toBe(true); + expect(filterCommandsByPrefix("/", actions)).toHaveLength(0); + expect(filterCommandsByPrefix("/help", actions)).toHaveLength(0); + expect(filterCommandsByPrefix("/ ", actions)).toHaveLength(0); + }); }); - test("/ prefix returns empty (slash commands handled separately)", () => { - const result = filterCommandsByPrefix("/", mockActions); - expect(result).toHaveLength(0); - }); - - test("clean separation: switching XOR other commands", () => { - const defaultResult = filterCommandsByPrefix("", mockActions); - const commandResult = filterCommandsByPrefix(">", mockActions); - - // No overlap - const defaultIds = new Set(defaultResult.map((a) => a.id)); - const commandIds = new Set(commandResult.map((a) => a.id)); - const intersection = [...defaultIds].filter((id) => commandIds.has(id)); + describe("property: query with > prefix applies to all non-switching", () => { + test(">text shows same set as > (cmdk filters further)", () => { + const actions = [{ id: "ws:switch:1" }, { id: "ws:new" }, { id: "nav:toggle" }]; - expect(intersection).toHaveLength(0); + // Our filter doesn't care about text after >, just the prefix + const resultEmpty = filterCommandsByPrefix(">", actions); + const resultWithText = filterCommandsByPrefix(">abc", actions); - // Together they cover all non-slash commands - expect(defaultResult.length + commandResult.length).toBe(mockActions.length); + expect(resultEmpty).toEqual(resultWithText); + }); }); }); From 1b045050f9a85c1501a318104fba577ddd4f4b5d Mon Sep 17 00:00:00 2001 From: Ammar Date: Sat, 8 Nov 2025 02:40:08 +0000 Subject: [PATCH 7/8] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20centralize=20com?= =?UTF-8?q?mand=20ID=20construction?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Create single source of truth for command ID patterns to eliminate magic string duplication across the codebase. **Changes:** - Add src/utils/commandIds.ts with CommandIds builders and CommandIdMatchers - Update sources.ts to use centralized builders (e.g., CommandIds.workspaceSwitch(id)) - Update filtering logic to use CommandIdMatchers.isWorkspaceSwitch() - Update tests to use centralized builders, ensuring consistency **Benefits:** - Single source of truth for all command ID patterns - No more duplicated magic strings ('ws:switch:', etc.) - Type-safe ID construction - Easy to find all command ID patterns in one place - Changes to ID format only need to happen in one file --- src/components/CommandPalette.test.ts | 71 ++++++++++++++------------ src/utils/commandIds.ts | 72 +++++++++++++++++++++++++++ src/utils/commandPaletteFiltering.ts | 6 ++- src/utils/commands/sources.ts | 45 +++++++++-------- 4 files changed, 139 insertions(+), 55 deletions(-) create mode 100644 src/utils/commandIds.ts diff --git a/src/components/CommandPalette.test.ts b/src/components/CommandPalette.test.ts index 5a704912f..7dc4ea8de 100644 --- a/src/components/CommandPalette.test.ts +++ b/src/components/CommandPalette.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; import { filterCommandsByPrefix } from "@/utils/commandPaletteFiltering"; +import { CommandIds, CommandIdMatchers } from "@/utils/commandIds"; /** * Tests for command palette filtering logic @@ -10,74 +11,74 @@ describe("CommandPalette filtering", () => { describe("property: default mode shows only ws:switch:* commands", () => { test("all results start with ws:switch:", () => { const actions = [ - { id: "ws:switch:1" }, - { id: "ws:switch:2" }, - { id: "ws:new" }, - { id: "nav:toggle" }, + { id: CommandIds.workspaceSwitch("1") }, + { id: CommandIds.workspaceSwitch("2") }, + { id: CommandIds.workspaceNew() }, + { id: CommandIds.navToggleSidebar() }, ]; const result = filterCommandsByPrefix("", actions); - expect(result.every((a) => a.id.startsWith("ws:switch:"))).toBe(true); + expect(result.every((a) => CommandIdMatchers.isWorkspaceSwitch(a.id))).toBe(true); }); test("excludes all non-switching commands", () => { const actions = [ - { id: "ws:switch:1" }, - { id: "ws:new" }, - { id: "ws:remove" }, - { id: "nav:toggle" }, + { id: CommandIds.workspaceSwitch("1") }, + { id: CommandIds.workspaceNew() }, + { id: CommandIds.workspaceRemove() }, + { id: CommandIds.navToggleSidebar() }, ]; const result = filterCommandsByPrefix("", actions); - expect(result.some((a) => !a.id.startsWith("ws:switch:"))).toBe(false); + expect(result.some((a) => !CommandIdMatchers.isWorkspaceSwitch(a.id))).toBe(false); }); }); describe("property: > mode shows all EXCEPT ws:switch:* commands", () => { test("no results start with ws:switch:", () => { const actions = [ - { id: "ws:switch:1" }, - { id: "ws:new" }, - { id: "nav:toggle" }, - { id: "chat:clear" }, + { id: CommandIds.workspaceSwitch("1") }, + { id: CommandIds.workspaceNew() }, + { id: CommandIds.navToggleSidebar() }, + { id: CommandIds.chatClear() }, ]; const result = filterCommandsByPrefix(">", actions); - expect(result.every((a) => !a.id.startsWith("ws:switch:"))).toBe(true); + expect(result.every((a) => !CommandIdMatchers.isWorkspaceSwitch(a.id))).toBe(true); }); test("includes all non-switching commands", () => { const actions = [ - { id: "ws:switch:1" }, - { id: "ws:new" }, - { id: "ws:remove" }, - { id: "nav:toggle" }, + { id: CommandIds.workspaceSwitch("1") }, + { id: CommandIds.workspaceNew() }, + { id: CommandIds.workspaceRemove() }, + { id: CommandIds.navToggleSidebar() }, ]; const result = filterCommandsByPrefix(">", actions); // Should include workspace mutations - expect(result.some((a) => a.id === "ws:new")).toBe(true); - expect(result.some((a) => a.id === "ws:remove")).toBe(true); + expect(result.some((a) => a.id === CommandIds.workspaceNew())).toBe(true); + expect(result.some((a) => a.id === CommandIds.workspaceRemove())).toBe(true); // Should include navigation - expect(result.some((a) => a.id === "nav:toggle")).toBe(true); + expect(result.some((a) => a.id === CommandIds.navToggleSidebar())).toBe(true); // Should NOT include switching - expect(result.some((a) => a.id === "ws:switch:1")).toBe(false); + expect(result.some((a) => a.id === CommandIds.workspaceSwitch("1"))).toBe(false); }); }); describe("property: modes partition the command space", () => { test("default + > modes cover all commands (no overlap, no gaps)", () => { const actions = [ - { id: "ws:switch:1" }, - { id: "ws:switch:2" }, - { id: "ws:new" }, - { id: "ws:remove" }, - { id: "nav:toggle" }, - { id: "chat:clear" }, + { id: CommandIds.workspaceSwitch("1") }, + { id: CommandIds.workspaceSwitch("2") }, + { id: CommandIds.workspaceNew() }, + { id: CommandIds.workspaceRemove() }, + { id: CommandIds.navToggleSidebar() }, + { id: CommandIds.chatClear() }, ]; const defaultResult = filterCommandsByPrefix("", actions); @@ -96,7 +97,11 @@ describe("CommandPalette filtering", () => { describe("property: / prefix always returns empty", () => { test("returns empty array regardless of actions", () => { - const actions = [{ id: "ws:switch:1" }, { id: "ws:new" }, { id: "nav:toggle" }]; + const actions = [ + { id: CommandIds.workspaceSwitch("1") }, + { id: CommandIds.workspaceNew() }, + { id: CommandIds.navToggleSidebar() }, + ]; expect(filterCommandsByPrefix("/", actions)).toHaveLength(0); expect(filterCommandsByPrefix("/help", actions)).toHaveLength(0); @@ -106,7 +111,11 @@ describe("CommandPalette filtering", () => { describe("property: query with > prefix applies to all non-switching", () => { test(">text shows same set as > (cmdk filters further)", () => { - const actions = [{ id: "ws:switch:1" }, { id: "ws:new" }, { id: "nav:toggle" }]; + const actions = [ + { id: CommandIds.workspaceSwitch("1") }, + { id: CommandIds.workspaceNew() }, + { id: CommandIds.navToggleSidebar() }, + ]; // Our filter doesn't care about text after >, just the prefix const resultEmpty = filterCommandsByPrefix(">", actions); diff --git a/src/utils/commandIds.ts b/src/utils/commandIds.ts new file mode 100644 index 000000000..e3cafad9f --- /dev/null +++ b/src/utils/commandIds.ts @@ -0,0 +1,72 @@ +/** + * Centralized command ID construction and matching + * Single source of truth for all command ID patterns + */ + +/** + * Command ID builders - construct IDs with consistent patterns + */ +export const CommandIds = { + // Workspace commands + workspaceSwitch: (workspaceId: string) => `ws:switch:${workspaceId}` as const, + workspaceNew: () => "ws:new" as const, + workspaceNewInProject: () => "ws:new-in-project" as const, + workspaceRemove: () => "ws:remove" as const, + workspaceRemoveAny: () => "ws:remove-any" as const, + workspaceRename: () => "ws:rename" as const, + workspaceRenameAny: () => "ws:rename-any" as const, + workspaceOpenTerminal: () => "ws:open-terminal" as const, + workspaceOpenTerminalCurrent: () => "ws:open-terminal-current" as const, + + // Navigation commands + navNext: () => "nav:next" as const, + navPrev: () => "nav:prev" as const, + navToggleSidebar: () => "nav:toggleSidebar" as const, + + // Chat commands + chatClear: () => "chat:clear" as const, + chatTruncate: (pct: number) => `chat:truncate:${pct}` as const, + chatInterrupt: () => "chat:interrupt" as const, + chatJumpBottom: () => "chat:jumpBottom" as const, + + // Mode commands + modeToggle: () => "mode:toggle" as const, + modelChange: () => "model:change" as const, + thinkingSetLevel: () => "thinking:set-level" as const, + + // Project commands + projectAdd: () => "project:add" as const, + projectRemove: (projectPath: string) => `project:remove:${projectPath}` as const, + + // Help commands + helpKeybinds: () => "help:keybinds" as const, +} as const; + +/** + * Command ID prefixes for pattern matching + */ +const COMMAND_ID_PREFIXES = { + WS_SWITCH: "ws:switch:", + CHAT_TRUNCATE: "chat:truncate:", + PROJECT_REMOVE: "project:remove:", +} as const; + +/** + * Command ID matchers - test if an ID matches a pattern + */ +export const CommandIdMatchers = { + /** + * Check if ID is a workspace switching command (ws:switch:*) + */ + isWorkspaceSwitch: (id: string): boolean => id.startsWith(COMMAND_ID_PREFIXES.WS_SWITCH), + + /** + * Check if ID is a chat truncate command (chat:truncate:*) + */ + isChatTruncate: (id: string): boolean => id.startsWith(COMMAND_ID_PREFIXES.CHAT_TRUNCATE), + + /** + * Check if ID is a project remove command (project:remove:*) + */ + isProjectRemove: (id: string): boolean => id.startsWith(COMMAND_ID_PREFIXES.PROJECT_REMOVE), +} as const; diff --git a/src/utils/commandPaletteFiltering.ts b/src/utils/commandPaletteFiltering.ts index b7956c383..e92902d84 100644 --- a/src/utils/commandPaletteFiltering.ts +++ b/src/utils/commandPaletteFiltering.ts @@ -3,6 +3,8 @@ * Separates workspace switching from all other commands */ +import { CommandIdMatchers } from "@/utils/commandIds"; + export interface CommandActionMinimal { id: string; } @@ -33,6 +35,6 @@ export function filterCommandsByPrefix( // Default: show only workspace switching commands // With ">": show all commands EXCEPT workspace switching return showAllCommands - ? actions.filter((action) => !action.id.startsWith("ws:switch:")) - : actions.filter((action) => action.id.startsWith("ws:switch:")); + ? actions.filter((action) => !CommandIdMatchers.isWorkspaceSwitch(action.id)) + : actions.filter((action) => CommandIdMatchers.isWorkspaceSwitch(action.id)); } diff --git a/src/utils/commands/sources.ts b/src/utils/commands/sources.ts index 1b8c62b91..8018717ac 100644 --- a/src/utils/commands/sources.ts +++ b/src/utils/commands/sources.ts @@ -2,6 +2,7 @@ import type { CommandAction } from "@/contexts/CommandRegistryContext"; import { formatKeybind, KEYBINDS } from "@/utils/ui/keybinds"; import type { ThinkingLevel } from "@/types/thinking"; import { CUSTOM_EVENTS, createCustomEvent } from "@/constants/events"; +import { CommandIds } from "@/utils/commandIds"; import type { ProjectConfig } from "@/config"; import type { FrontendWorkspaceMetadata } from "@/types/workspace"; @@ -77,7 +78,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi selected: NonNullable ): CommandAction => { return { - id: "ws:new", + id: CommandIds.workspaceNew(), title: "Create New Workspace…", subtitle: `for ${selected.projectName}`, section: section.workspaces, @@ -101,7 +102,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi const isCurrent = selected?.workspaceId === meta.id; const isStreaming = p.streamingModels?.has(meta.id) ?? false; list.push({ - id: `ws:switch:${meta.id}`, + id: CommandIds.workspaceSwitch(meta.id), title: `${isCurrent ? "• " : ""}Switch to ${meta.name}`, subtitle: `${meta.projectName}${isStreaming ? " • streaming" : ""}`, section: section.workspaces, @@ -120,7 +121,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi if (selected?.namedWorkspacePath) { const workspaceDisplayName = `${selected.projectName}/${selected.namedWorkspacePath.split("/").pop() ?? selected.namedWorkspacePath}`; list.push({ - id: "ws:open-terminal-current", + id: CommandIds.workspaceOpenTerminalCurrent(), title: "Open Current Workspace in Terminal", subtitle: workspaceDisplayName, section: section.workspaces, @@ -130,7 +131,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi }, }); list.push({ - id: "ws:remove", + id: CommandIds.workspaceRemove(), title: "Remove Current Workspace…", subtitle: workspaceDisplayName, section: section.workspaces, @@ -140,7 +141,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi }, }); list.push({ - id: "ws:rename", + id: CommandIds.workspaceRename(), title: "Rename Current Workspace…", subtitle: workspaceDisplayName, section: section.workspaces, @@ -168,7 +169,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi if (p.workspaceMetadata.size > 0) { list.push({ - id: "ws:open-terminal", + id: CommandIds.workspaceOpenTerminal(), title: "Open Workspace in Terminal…", section: section.workspaces, run: () => undefined, @@ -198,7 +199,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi }, }); list.push({ - id: "ws:rename-any", + id: CommandIds.workspaceRenameAny(), title: "Rename Workspace…", section: section.workspaces, run: () => undefined, @@ -240,7 +241,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi }, }); list.push({ - id: "ws:remove-any", + id: CommandIds.workspaceRemoveAny(), title: "Remove Workspace…", section: section.workspaces, run: () => undefined, @@ -283,21 +284,21 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi // Navigation / Interface actions.push(() => [ { - id: "nav:next", + id: CommandIds.navNext(), title: "Next Workspace", section: section.navigation, shortcutHint: formatKeybind(KEYBINDS.NEXT_WORKSPACE), run: () => p.onNavigateWorkspace("next"), }, { - id: "nav:prev", + id: CommandIds.navPrev(), title: "Previous Workspace", section: section.navigation, shortcutHint: formatKeybind(KEYBINDS.PREV_WORKSPACE), run: () => p.onNavigateWorkspace("prev"), }, { - id: "nav:toggleSidebar", + id: CommandIds.navToggleSidebar(), title: "Toggle Sidebar", section: section.navigation, shortcutHint: formatKeybind(KEYBINDS.TOGGLE_SIDEBAR), @@ -311,7 +312,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi if (p.selectedWorkspace) { const id = p.selectedWorkspace.workspaceId; list.push({ - id: "chat:clear", + id: CommandIds.chatClear(), title: "Clear History", section: section.chat, run: async () => { @@ -320,7 +321,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi }); for (const pct of [0.75, 0.5, 0.25]) { list.push({ - id: `chat:truncate:${pct}`, + id: CommandIds.chatTruncate(pct), title: `Truncate History to ${Math.round((1 - pct) * 100)}%`, section: section.chat, run: async () => { @@ -329,7 +330,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi }); } list.push({ - id: "chat:interrupt", + id: CommandIds.chatInterrupt(), title: "Interrupt Streaming", section: section.chat, run: async () => { @@ -337,7 +338,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi }, }); list.push({ - id: "chat:jumpBottom", + id: CommandIds.chatJumpBottom(), title: "Jump to Bottom", section: section.chat, shortcutHint: formatKeybind(KEYBINDS.JUMP_TO_BOTTOM), @@ -355,7 +356,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi actions.push(() => { const list: CommandAction[] = [ { - id: "mode:toggle", + id: CommandIds.modeToggle(), title: "Toggle Plan/Exec Mode", section: section.mode, shortcutHint: formatKeybind(KEYBINDS.TOGGLE_MODE), @@ -365,7 +366,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi }, }, { - id: "model:change", + id: CommandIds.modelChange(), title: "Change Model…", section: section.mode, shortcutHint: formatKeybind(KEYBINDS.OPEN_MODEL_SELECTOR), @@ -387,7 +388,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi const currentLevel = p.getThinkingLevel(workspaceId); list.push({ - id: "thinking:set-level", + id: CommandIds.thinkingSetLevel(), title: "Set Thinking Effort…", subtitle: `Current: ${levelDescriptions[currentLevel] ?? currentLevel}`, section: section.mode, @@ -430,7 +431,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi // Help / Docs actions.push(() => [ { - id: "help:keybinds", + id: CommandIds.helpKeybinds(), title: "Show Keyboard Shortcuts", section: section.help, run: () => { @@ -447,13 +448,13 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi actions.push(() => { const list: CommandAction[] = [ { - id: "project:add", + id: CommandIds.projectAdd(), title: "Add Project…", section: section.projects, run: () => p.onAddProject(), }, { - id: "ws:new-in-project", + id: CommandIds.workspaceNewInProject(), title: "Create New Workspace in Project…", section: section.projects, run: () => undefined, @@ -485,7 +486,7 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi for (const [projectPath] of p.projects.entries()) { const projectName = projectPath.split("/").pop() ?? projectPath; list.push({ - id: `project:remove:${projectPath}`, + id: CommandIds.projectRemove(projectPath), title: `Remove Project ${projectName}…`, section: section.projects, run: () => p.onRemoveProject(projectPath), From 98df4b2657bc893972dc8624618a0d677dc25c00 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sat, 8 Nov 2025 15:47:54 +0000 Subject: [PATCH 8/8] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20eliminate=20pref?= =?UTF-8?q?ix=20duplication=20in=20commandIds?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move COMMAND_ID_PREFIXES definition before CommandIds to use prefixes in builder functions, eliminating string literal duplication. **Before:** - Builder: `ws:switch:${workspaceId}` - Prefix: "ws:switch:" **After:** - Prefix defined once: "ws:switch:" - Builder: `${COMMAND_ID_PREFIXES.WS_SWITCH}${workspaceId}` --- src/utils/commandIds.ts | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/utils/commandIds.ts b/src/utils/commandIds.ts index e3cafad9f..e30ae8854 100644 --- a/src/utils/commandIds.ts +++ b/src/utils/commandIds.ts @@ -3,12 +3,23 @@ * Single source of truth for all command ID patterns */ +/** + * Command ID prefixes for pattern matching + * Single source of truth for all dynamic ID patterns + */ +const COMMAND_ID_PREFIXES = { + WS_SWITCH: "ws:switch:", + CHAT_TRUNCATE: "chat:truncate:", + PROJECT_REMOVE: "project:remove:", +} as const; + /** * Command ID builders - construct IDs with consistent patterns */ export const CommandIds = { // Workspace commands - workspaceSwitch: (workspaceId: string) => `ws:switch:${workspaceId}` as const, + workspaceSwitch: (workspaceId: string) => + `${COMMAND_ID_PREFIXES.WS_SWITCH}${workspaceId}` as const, workspaceNew: () => "ws:new" as const, workspaceNewInProject: () => "ws:new-in-project" as const, workspaceRemove: () => "ws:remove" as const, @@ -25,7 +36,7 @@ export const CommandIds = { // Chat commands chatClear: () => "chat:clear" as const, - chatTruncate: (pct: number) => `chat:truncate:${pct}` as const, + chatTruncate: (pct: number) => `${COMMAND_ID_PREFIXES.CHAT_TRUNCATE}${pct}` as const, chatInterrupt: () => "chat:interrupt" as const, chatJumpBottom: () => "chat:jumpBottom" as const, @@ -36,21 +47,13 @@ export const CommandIds = { // Project commands projectAdd: () => "project:add" as const, - projectRemove: (projectPath: string) => `project:remove:${projectPath}` as const, + projectRemove: (projectPath: string) => + `${COMMAND_ID_PREFIXES.PROJECT_REMOVE}${projectPath}` as const, // Help commands helpKeybinds: () => "help:keybinds" as const, } as const; -/** - * Command ID prefixes for pattern matching - */ -const COMMAND_ID_PREFIXES = { - WS_SWITCH: "ws:switch:", - CHAT_TRUNCATE: "chat:truncate:", - PROJECT_REMOVE: "project:remove:", -} as const; - /** * Command ID matchers - test if an ID matches a pattern */