From 92b7ac7f13c10fb3f22e8894a960b8d5384f1a4d Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Fri, 1 May 2026 00:36:54 +0200 Subject: [PATCH] Make lane actions match the dmux cockpit menu The cockpit control frame previously reused the broader pane-menu defaults, which left extra actions and old hotkeys in the selected-lane path. The menu wrapper now exposes the requested lane actions and the control frame centers the menu as an overlay while continuing to return action intents instead of executing them. Constraint: User limited edits to cockpit menu/control and related tests Rejected: Editing src/cockpit/pane-menu.js | it still backs older pane-menu contracts and direct tests Confidence: high Scope-risk: narrow Directive: Keep selected-lane menu hotkeys aligned with src/cockpit/menu.js before changing control help text Tested: node --test test/cockpit-menu.test.js test/cockpit-control.test.js test/cockpit-kitty-integration.test.js Tested: npm test Co-authored-by: OmX --- src/cockpit/control.js | 46 ++++++- src/cockpit/menu.js | 169 ++++++++++++++++++++++++- test/cockpit-control.test.js | 19 ++- test/cockpit-kitty-integration.test.js | 2 +- test/cockpit-menu.test.js | 20 ++- 5 files changed, 233 insertions(+), 23 deletions(-) diff --git a/src/cockpit/control.js b/src/cockpit/control.js index 8f2a15a..62886e8 100644 --- a/src/cockpit/control.js +++ b/src/cockpit/control.js @@ -10,7 +10,7 @@ const { createPaneMenuState, normalizePaneMenuKey, renderPaneMenu, -} = require('./pane-menu'); +} = require('./menu'); const DEFAULT_REFRESH_MS = 2000; const DEFAULT_SETTINGS = { @@ -35,7 +35,7 @@ const SETTINGS_FIELDS = [ const MENU_ITEMS = PANE_MENU_ITEMS; const PANE_ACTION_IDS = new Set(PANE_MENU_ITEMS.map((item) => item.id)); -const DIRECT_DETAIL_PANE_KEYS = new Set(['x', 'b', 'f', 'h', 'P', 'a', 'A', 'r']); +const DIRECT_DETAIL_PANE_KEYS = new Set(['v', 'h', 'x', 'p', 'r', 'c', 'o', 'a', 'b', 'f', 'T', 'A']); function text(value, fallback = '') { if (typeof value === 'string') return value.trim() || fallback; @@ -449,6 +449,35 @@ function padAnsi(value, width) { return visible >= width ? raw : `${raw}${' '.repeat(width - visible)}`; } +function visibleWidth(value) { + return stripAnsi(value).length; +} + +function centerLine(value, width) { + const raw = String(value || ''); + const left = Math.max(0, Math.floor((width - visibleWidth(raw)) / 2)); + return `${' '.repeat(left)}${raw}`; +} + +function overlayCenteredBox(baseLines, overlayText) { + const overlay = splitLines(overlayText); + const width = Math.max( + ...baseLines.map((line) => visibleWidth(line)), + ...overlay.map((line) => visibleWidth(line)), + ); + const height = Math.max(baseLines.length, overlay.length + 2); + const lines = [...baseLines]; + + while (lines.length < height) lines.push(''); + + const top = Math.max(0, Math.floor((height - overlay.length) / 2)); + for (let index = 0; index < overlay.length; index += 1) { + lines[top + index] = centerLine(overlay[index], width); + } + + return lines; +} + function selectedField(state) { const current = normalizeControlState(state); return SETTINGS_FIELDS[current.settingsIndex] || SETTINGS_FIELDS[0]; @@ -480,7 +509,7 @@ function renderDetailsPanel(state) { lines.push(`locks: ${Number.isFinite(session.lockCount) ? session.lockCount : 0}`); } - lines.push('', 'keys: up/down select m/Alt+Shift+M menu x/b/f/h/P/a/A/r pane actions s settings q quit'); + lines.push('', 'keys: up/down select m/Alt+Shift+M menu v/h/x/p/r/c/o/a/b/f/T/A pane actions s settings q quit'); if (current.error) { lines.push('', `error: ${text(current.error)}`); } @@ -513,7 +542,10 @@ function renderControlFrame(state) { const current = normalizeControlState(state); const width = number(current.settings.sidebarWidth, DEFAULT_SETTINGS.sidebarWidth); const sidebar = splitLines(renderSidebar(current, { width, noColor: true })); - const panel = splitLines(renderPanel(current)); + const framePanelState = current.mode === 'menu' + ? normalizeControlState({ ...current, mode: 'details' }) + : current; + const panel = splitLines(renderPanel(framePanelState)); const leftWidth = Math.max(width, ...sidebar.map((line) => stripAnsi(line).length)); const max = Math.max(sidebar.length, panel.length); const lines = []; @@ -522,7 +554,11 @@ function renderControlFrame(state) { lines.push(`${padAnsi(sidebar[index] || '', leftWidth)} ${panel[index] || ''}`.trimEnd()); } - return `${lines.join('\n')}\n`; + const rendered = current.mode === 'menu' + ? overlayCenteredBox(lines, renderMenuPanel(current)) + : lines; + + return `${rendered.join('\n')}\n`; } function optionalSettingsModule() { diff --git a/src/cockpit/menu.js b/src/cockpit/menu.js index 34c1ca0..ccb3e32 100644 --- a/src/cockpit/menu.js +++ b/src/cockpit/menu.js @@ -1,3 +1,170 @@ 'use strict'; -module.exports = require('./pane-menu'); +const paneMenu = require('./pane-menu'); + +const { + PANE_MENU_ACTIONS, + PANE_MENU_ACTION_IDS, + PANE_MENU_FOOTER, + normalizePaneMenuKey, +} = paneMenu; + +const PANE_MENU_ITEMS = Object.freeze([ + { id: PANE_MENU_ACTION_IDS.VIEW, label: 'View', hotkey: 'v', needsSession: true }, + { id: PANE_MENU_ACTION_IDS.HIDE_PANE, label: 'Hide Pane', hotkey: 'h', needsSession: true }, + { id: PANE_MENU_ACTION_IDS.CLOSE, label: 'Close', hotkey: 'x', danger: true, needsSession: true }, + { id: PANE_MENU_ACTION_IDS.MERGE, label: 'Merge / Finish', hotkey: 'm', needsSession: true, needsWorktree: true, needsBranch: true }, + { id: PANE_MENU_ACTION_IDS.CREATE_PR, label: 'Create GitHub PR', hotkey: 'p', needsSession: true, needsWorktree: true, needsBranch: true }, + { id: PANE_MENU_ACTION_IDS.RENAME, label: 'Rename', hotkey: 'r', needsSession: true }, + { id: PANE_MENU_ACTION_IDS.COPY_PATH, label: 'Copy Path', hotkey: 'c', needsSession: true, needsWorktree: true }, + { id: PANE_MENU_ACTION_IDS.OPEN_EDITOR, label: 'Open in Editor', hotkey: 'o', needsSession: true, needsWorktree: true }, + { id: PANE_MENU_ACTION_IDS.TOGGLE_AUTOPILOT, label: 'Toggle Autopilot', hotkey: 'a', needsSession: true, needsWorktree: true, needsBranch: true }, + { id: PANE_MENU_ACTION_IDS.CREATE_CHILD_WORKTREE, label: 'Create Child Worktree', hotkey: 'b', needsSession: true, needsWorktree: true, needsBranch: true }, + { id: PANE_MENU_ACTION_IDS.BROWSE_FILES, label: 'Browse Files', hotkey: 'f', needsSession: true, needsWorktree: true }, + { id: PANE_MENU_ACTION_IDS.ADD_TERMINAL, label: 'Add Terminal to Worktree', hotkey: 'T', needsSession: true, needsWorktree: true }, + { id: PANE_MENU_ACTION_IDS.ADD_AGENT, label: 'Add Agent to Worktree', hotkey: 'A', needsSession: true, needsWorktree: true, needsBranch: true }, +]); + +function firstString(...values) { + for (const value of values) { + if (typeof value === 'string' && value.trim().length > 0) { + return value.trim(); + } + } + return ''; +} + +function fileName(value) { + const text = String(value || '').replace(/[/\\]+$/, ''); + const parts = text.split(/[/\\]+/).filter(Boolean); + return parts[parts.length - 1] || ''; +} + +function selectedPaneName(session = {}, context = {}) { + return firstString( + context.name, + session.displayName, + session.paneName, + session.name, + session.agentName, + session.agent, + fileName(session.worktreePath), + fileName(session.path), + session.branch, + session.id, + 'selected pane', + ); +} + +function paneMenuTitle(name) { + const text = String(name || '').trim() || 'selected pane'; + return text.startsWith('Menu:') ? text : `Menu: ${text}`; +} + +function selectedSession(context = {}) { + return context.session || context.selectedSession || context.pane || context.lane || null; +} + +function resolveBranch(session = {}, context = {}) { + return firstString( + context.branch, + session.branch, + session.lane && session.lane.branch, + ); +} + +function resolveWorktreePath(session = {}, context = {}) { + return firstString( + context.worktreePath, + context.path, + session.worktreePath, + session.worktree && session.worktree.path, + session.path, + ); +} + +function resolveWorktreeExists(session = {}, context = {}, worktreePath = '') { + if (typeof context.worktreeExists === 'boolean') return context.worktreeExists; + if (typeof session.worktreeExists === 'boolean') return session.worktreeExists; + return worktreePath.length > 0; +} + +function disabledReason(item, context) { + if (item.needsSession && !context.selected) return 'No pane selected'; + + const reasons = []; + if (item.needsWorktree && !context.worktreeExists) reasons.push('Worktree missing'); + if (item.needsBranch && !context.branch) reasons.push('Branch missing'); + return reasons.join('; '); +} + +function createPaneMenuItems(context) { + return PANE_MENU_ITEMS.map((item) => { + const reason = disabledReason(item, context); + return { + id: item.id, + label: item.label, + hotkey: item.hotkey, + shortcut: item.hotkey, + enabled: reason.length === 0, + danger: Boolean(item.danger), + reason, + }; + }); +} + +function createPaneMenuState(options = {}) { + const session = selectedSession(options); + const selected = Boolean(session) && options.selected !== false; + const source = session || {}; + const branch = selected ? resolveBranch(source, options) : ''; + const worktreePath = selected ? resolveWorktreePath(source, options) : ''; + const context = { + selected, + branch, + worktreePath, + worktreeExists: selected && resolveWorktreeExists(source, options, worktreePath), + }; + const items = Array.isArray(options.items) && options.items.length > 0 + ? options.items.map((item) => ({ ...item })) + : createPaneMenuItems(context); + + return paneMenu.createPaneMenuState({ + ...options, + session, + title: paneMenuTitle(firstString(options.title, selectedPaneName(source, options))), + items, + }); +} + +function applyPaneMenuKey(state = {}, rawKey) { + return paneMenu.applyPaneMenuKey(createPaneMenuState(state), rawKey); +} + +function renderPaneMenu(state = {}, options = {}) { + const selectedIndex = Number.isInteger(options.selectedIndex) + ? options.selectedIndex + : state.selectedIndex; + return paneMenu.renderPaneMenu(createPaneMenuState({ ...state, selectedIndex }), options).replace(/\u25b6/g, '>'); +} + +function buildLaneMenu(session, context = {}) { + return createPaneMenuState({ ...context, session }); +} + +function renderLaneMenu(menu, options = {}) { + return renderPaneMenu(menu, options); +} + +module.exports = { + PANE_MENU_ACTIONS, + PANE_MENU_ACTION_IDS, + PANE_MENU_FOOTER, + PANE_MENU_ITEMS, + applyPaneMenuKey, + buildLaneMenu, + createPaneMenuState, + normalizePaneMenuKey, + renderLaneMenu, + renderPaneMenu, +}; diff --git a/test/cockpit-control.test.js b/test/cockpit-control.test.js index 4148714..e3c55bf 100644 --- a/test/cockpit-control.test.js +++ b/test/cockpit-control.test.js @@ -204,11 +204,17 @@ test('applyCockpitAction routes pane menu hotkeys to pane action intents', () => worktreePath: '/tmp/one', }); - state = applyCockpitAction(state, { type: 'key', key: 'P' }); - assert.equal(state.lastIntent.type, 'project-focus'); + state = applyCockpitAction(state, { type: 'key', key: 'p' }); + assert.equal(state.lastIntent.type, 'create-pr'); state = applyCockpitAction(state, { type: 'key', key: 'r' }); - assert.equal(state.lastIntent.type, 'reopen-closed-worktree'); + assert.equal(state.lastIntent.type, 'rename'); + + state = applyCockpitAction(state, { type: 'key', key: 'T' }); + assert.equal(state.lastIntent.type, 'add-terminal'); + + state = applyCockpitAction(state, { type: 'key', key: 'A' }); + assert.equal(state.lastIntent.type, 'add-agent'); }); test('renderControlFrame renders sidebar with details, menu, and settings modes', () => { @@ -224,9 +230,12 @@ test('renderControlFrame renders sidebar with details, menu, and settings modes' assert.match(details, /session: one/); const menu = renderControlFrame(applyCockpitAction(baseState, { type: 'key', key: 'm' })); + assert.match(menu, /^ {2,}┌/m); assert.match(menu, /Menu: codex/); - assert.match(menu, /View\s+\[j\]/); - assert.match(menu, /Project Focus\s+\[P\]/); + assert.match(menu, /> View\s+\[v\]/); + assert.match(menu, /Merge \/ Finish\s+\[m\]/); + assert.match(menu, /Add Terminal to Worktree\s+\[T\]/); + assert.doesNotMatch(menu, /Project Focus/); const settings = renderControlFrame(applyCockpitAction(baseState, { type: 'key', key: 's' })); assert.match(settings, /gx cockpit settings/); diff --git a/test/cockpit-kitty-integration.test.js b/test/cockpit-kitty-integration.test.js index a284e72..2bdcd58 100644 --- a/test/cockpit-kitty-integration.test.js +++ b/test/cockpit-kitty-integration.test.js @@ -163,7 +163,7 @@ test('cockpit pane menu opens and selects a lane terminal action', () => { state = applyCockpitAction(state, { type: 'key', key: 'm' }); assert.equal(state.mode, 'menu'); - state = applyCockpitAction(state, { type: 'key', key: 'A' }); + state = applyCockpitAction(state, { type: 'key', key: 'T' }); assert.deepEqual(state.lastIntent, { type: 'add-terminal', diff --git a/test/cockpit-menu.test.js b/test/cockpit-menu.test.js index e47db4d..7dc664b 100644 --- a/test/cockpit-menu.test.js +++ b/test/cockpit-menu.test.js @@ -34,9 +34,8 @@ test('buildLaneMenu returns the expected dmux-style pane actions', () => { 'View', 'Hide Pane', 'Close', - 'Merge', + 'Merge / Finish', 'Create GitHub PR', - 'Project Focus', 'Rename', 'Copy Path', 'Open in Editor', @@ -45,7 +44,6 @@ test('buildLaneMenu returns the expected dmux-style pane actions', () => { 'Browse Files', 'Add Terminal to Worktree', 'Add Agent to Worktree', - 'Reopen Closed Worktree', ], ); assert.deepEqual(enabledIds(menu), [ @@ -54,7 +52,6 @@ test('buildLaneMenu returns the expected dmux-style pane actions', () => { PANE_MENU_ACTION_IDS.CLOSE, PANE_MENU_ACTION_IDS.MERGE, PANE_MENU_ACTION_IDS.CREATE_PR, - PANE_MENU_ACTION_IDS.PROJECT_FOCUS, PANE_MENU_ACTION_IDS.RENAME, PANE_MENU_ACTION_IDS.COPY_PATH, PANE_MENU_ACTION_IDS.OPEN_EDITOR, @@ -63,10 +60,12 @@ test('buildLaneMenu returns the expected dmux-style pane actions', () => { PANE_MENU_ACTION_IDS.BROWSE_FILES, PANE_MENU_ACTION_IDS.ADD_TERMINAL, PANE_MENU_ACTION_IDS.ADD_AGENT, - PANE_MENU_ACTION_IDS.REOPEN_CLOSED_WORKTREE, ]); assert.equal(itemById(menu, PANE_MENU_ACTION_IDS.CLOSE).danger, true); - assert.equal(itemById(menu, PANE_MENU_ACTION_IDS.PROJECT_FOCUS).shortcut, 'P'); + assert.equal(itemById(menu, PANE_MENU_ACTION_IDS.VIEW).shortcut, 'v'); + assert.equal(itemById(menu, PANE_MENU_ACTION_IDS.CREATE_PR).shortcut, 'p'); + assert.equal(itemById(menu, PANE_MENU_ACTION_IDS.ADD_TERMINAL).shortcut, 'T'); + assert.equal(itemById(menu, PANE_MENU_ACTION_IDS.ADD_AGENT).shortcut, 'A'); }); test('buildLaneMenu disables every lane action when no session is selected', () => { @@ -92,9 +91,7 @@ test('buildLaneMenu disables worktree actions when the worktree is missing', () assert.equal(itemById(menu, 'view').enabled, true); assert.equal(itemById(menu, 'hide-pane').enabled, true); assert.equal(itemById(menu, 'close').enabled, true); - assert.equal(itemById(menu, 'project-focus').enabled, true); assert.equal(itemById(menu, 'rename').enabled, true); - assert.equal(itemById(menu, 'reopen-closed-worktree').enabled, true); for (const id of ['merge', 'create-pr', 'copy-path', 'open-editor', 'toggle-autopilot', 'create-child-worktree', 'browse-files', 'add-terminal', 'add-agent']) { const item = itemById(menu, id); @@ -113,12 +110,12 @@ test('buildLaneMenu disables branch actions when the branch is missing', () => { assert.equal(itemById(menu, 'view').enabled, true); assert.equal(itemById(menu, 'hide-pane').enabled, true); - assert.equal(itemById(menu, 'project-focus').enabled, true); + assert.equal(itemById(menu, 'close').enabled, true); + assert.equal(itemById(menu, 'rename').enabled, true); assert.equal(itemById(menu, 'copy-path').enabled, true); assert.equal(itemById(menu, 'open-editor').enabled, true); assert.equal(itemById(menu, 'browse-files').enabled, true); assert.equal(itemById(menu, 'add-terminal').enabled, true); - assert.equal(itemById(menu, 'reopen-closed-worktree').enabled, true); for (const id of ['merge', 'create-pr', 'toggle-autopilot', 'create-child-worktree', 'add-agent']) { const item = itemById(menu, id); @@ -138,8 +135,9 @@ test('renderLaneMenu renders a boxed menu with an ASCII fallback', () => { const unicodeOutput = renderLaneMenu(menu, { selectedIndex: 2 }); assert.match(unicodeOutput, /^┌/); assert.match(unicodeOutput, /Menu: codex/); + assert.match(unicodeOutput, /> Close\s+\[x\]/); assert.match(unicodeOutput, /Close\s+\[x\]/); - assert.match(unicodeOutput, /Project Focus\s+\[P\]/); + assert.match(unicodeOutput, /Merge \/ Finish\s+\[m\]/); assert.match(unicodeOutput, /Create GitHub PR/); const asciiOutput = renderLaneMenu(menu, { unicode: false });