Skip to content

Commit 7b37f50

Browse files
NagyViktNagyViktOmX
authored
Make lane actions match the dmux cockpit menu (#511)
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: NagyVikt <nagy.viktordp@gmail.com> Co-authored-by: OmX <omx@oh-my-codex.dev>
1 parent ba382dd commit 7b37f50

5 files changed

Lines changed: 233 additions & 23 deletions

File tree

src/cockpit/control.js

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ const {
1010
createPaneMenuState,
1111
normalizePaneMenuKey,
1212
renderPaneMenu,
13-
} = require('./pane-menu');
13+
} = require('./menu');
1414

1515
const DEFAULT_REFRESH_MS = 2000;
1616
const DEFAULT_SETTINGS = {
@@ -35,7 +35,7 @@ const SETTINGS_FIELDS = [
3535

3636
const MENU_ITEMS = PANE_MENU_ITEMS;
3737
const PANE_ACTION_IDS = new Set(PANE_MENU_ITEMS.map((item) => item.id));
38-
const DIRECT_DETAIL_PANE_KEYS = new Set(['x', 'b', 'f', 'h', 'P', 'a', 'A', 'r']);
38+
const DIRECT_DETAIL_PANE_KEYS = new Set(['v', 'h', 'x', 'p', 'r', 'c', 'o', 'a', 'b', 'f', 'T', 'A']);
3939

4040
function text(value, fallback = '') {
4141
if (typeof value === 'string') return value.trim() || fallback;
@@ -449,6 +449,35 @@ function padAnsi(value, width) {
449449
return visible >= width ? raw : `${raw}${' '.repeat(width - visible)}`;
450450
}
451451

452+
function visibleWidth(value) {
453+
return stripAnsi(value).length;
454+
}
455+
456+
function centerLine(value, width) {
457+
const raw = String(value || '');
458+
const left = Math.max(0, Math.floor((width - visibleWidth(raw)) / 2));
459+
return `${' '.repeat(left)}${raw}`;
460+
}
461+
462+
function overlayCenteredBox(baseLines, overlayText) {
463+
const overlay = splitLines(overlayText);
464+
const width = Math.max(
465+
...baseLines.map((line) => visibleWidth(line)),
466+
...overlay.map((line) => visibleWidth(line)),
467+
);
468+
const height = Math.max(baseLines.length, overlay.length + 2);
469+
const lines = [...baseLines];
470+
471+
while (lines.length < height) lines.push('');
472+
473+
const top = Math.max(0, Math.floor((height - overlay.length) / 2));
474+
for (let index = 0; index < overlay.length; index += 1) {
475+
lines[top + index] = centerLine(overlay[index], width);
476+
}
477+
478+
return lines;
479+
}
480+
452481
function selectedField(state) {
453482
const current = normalizeControlState(state);
454483
return SETTINGS_FIELDS[current.settingsIndex] || SETTINGS_FIELDS[0];
@@ -480,7 +509,7 @@ function renderDetailsPanel(state) {
480509
lines.push(`locks: ${Number.isFinite(session.lockCount) ? session.lockCount : 0}`);
481510
}
482511

483-
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');
512+
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');
484513
if (current.error) {
485514
lines.push('', `error: ${text(current.error)}`);
486515
}
@@ -513,7 +542,10 @@ function renderControlFrame(state) {
513542
const current = normalizeControlState(state);
514543
const width = number(current.settings.sidebarWidth, DEFAULT_SETTINGS.sidebarWidth);
515544
const sidebar = splitLines(renderSidebar(current, { width, noColor: true }));
516-
const panel = splitLines(renderPanel(current));
545+
const framePanelState = current.mode === 'menu'
546+
? normalizeControlState({ ...current, mode: 'details' })
547+
: current;
548+
const panel = splitLines(renderPanel(framePanelState));
517549
const leftWidth = Math.max(width, ...sidebar.map((line) => stripAnsi(line).length));
518550
const max = Math.max(sidebar.length, panel.length);
519551
const lines = [];
@@ -522,7 +554,11 @@ function renderControlFrame(state) {
522554
lines.push(`${padAnsi(sidebar[index] || '', leftWidth)} ${panel[index] || ''}`.trimEnd());
523555
}
524556

525-
return `${lines.join('\n')}\n`;
557+
const rendered = current.mode === 'menu'
558+
? overlayCenteredBox(lines, renderMenuPanel(current))
559+
: lines;
560+
561+
return `${rendered.join('\n')}\n`;
526562
}
527563

528564
function optionalSettingsModule() {

src/cockpit/menu.js

Lines changed: 168 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,170 @@
11
'use strict';
22

3-
module.exports = require('./pane-menu');
3+
const paneMenu = require('./pane-menu');
4+
5+
const {
6+
PANE_MENU_ACTIONS,
7+
PANE_MENU_ACTION_IDS,
8+
PANE_MENU_FOOTER,
9+
normalizePaneMenuKey,
10+
} = paneMenu;
11+
12+
const PANE_MENU_ITEMS = Object.freeze([
13+
{ id: PANE_MENU_ACTION_IDS.VIEW, label: 'View', hotkey: 'v', needsSession: true },
14+
{ id: PANE_MENU_ACTION_IDS.HIDE_PANE, label: 'Hide Pane', hotkey: 'h', needsSession: true },
15+
{ id: PANE_MENU_ACTION_IDS.CLOSE, label: 'Close', hotkey: 'x', danger: true, needsSession: true },
16+
{ id: PANE_MENU_ACTION_IDS.MERGE, label: 'Merge / Finish', hotkey: 'm', needsSession: true, needsWorktree: true, needsBranch: true },
17+
{ id: PANE_MENU_ACTION_IDS.CREATE_PR, label: 'Create GitHub PR', hotkey: 'p', needsSession: true, needsWorktree: true, needsBranch: true },
18+
{ id: PANE_MENU_ACTION_IDS.RENAME, label: 'Rename', hotkey: 'r', needsSession: true },
19+
{ id: PANE_MENU_ACTION_IDS.COPY_PATH, label: 'Copy Path', hotkey: 'c', needsSession: true, needsWorktree: true },
20+
{ id: PANE_MENU_ACTION_IDS.OPEN_EDITOR, label: 'Open in Editor', hotkey: 'o', needsSession: true, needsWorktree: true },
21+
{ id: PANE_MENU_ACTION_IDS.TOGGLE_AUTOPILOT, label: 'Toggle Autopilot', hotkey: 'a', needsSession: true, needsWorktree: true, needsBranch: true },
22+
{ id: PANE_MENU_ACTION_IDS.CREATE_CHILD_WORKTREE, label: 'Create Child Worktree', hotkey: 'b', needsSession: true, needsWorktree: true, needsBranch: true },
23+
{ id: PANE_MENU_ACTION_IDS.BROWSE_FILES, label: 'Browse Files', hotkey: 'f', needsSession: true, needsWorktree: true },
24+
{ id: PANE_MENU_ACTION_IDS.ADD_TERMINAL, label: 'Add Terminal to Worktree', hotkey: 'T', needsSession: true, needsWorktree: true },
25+
{ id: PANE_MENU_ACTION_IDS.ADD_AGENT, label: 'Add Agent to Worktree', hotkey: 'A', needsSession: true, needsWorktree: true, needsBranch: true },
26+
]);
27+
28+
function firstString(...values) {
29+
for (const value of values) {
30+
if (typeof value === 'string' && value.trim().length > 0) {
31+
return value.trim();
32+
}
33+
}
34+
return '';
35+
}
36+
37+
function fileName(value) {
38+
const text = String(value || '').replace(/[/\\]+$/, '');
39+
const parts = text.split(/[/\\]+/).filter(Boolean);
40+
return parts[parts.length - 1] || '';
41+
}
42+
43+
function selectedPaneName(session = {}, context = {}) {
44+
return firstString(
45+
context.name,
46+
session.displayName,
47+
session.paneName,
48+
session.name,
49+
session.agentName,
50+
session.agent,
51+
fileName(session.worktreePath),
52+
fileName(session.path),
53+
session.branch,
54+
session.id,
55+
'selected pane',
56+
);
57+
}
58+
59+
function paneMenuTitle(name) {
60+
const text = String(name || '').trim() || 'selected pane';
61+
return text.startsWith('Menu:') ? text : `Menu: ${text}`;
62+
}
63+
64+
function selectedSession(context = {}) {
65+
return context.session || context.selectedSession || context.pane || context.lane || null;
66+
}
67+
68+
function resolveBranch(session = {}, context = {}) {
69+
return firstString(
70+
context.branch,
71+
session.branch,
72+
session.lane && session.lane.branch,
73+
);
74+
}
75+
76+
function resolveWorktreePath(session = {}, context = {}) {
77+
return firstString(
78+
context.worktreePath,
79+
context.path,
80+
session.worktreePath,
81+
session.worktree && session.worktree.path,
82+
session.path,
83+
);
84+
}
85+
86+
function resolveWorktreeExists(session = {}, context = {}, worktreePath = '') {
87+
if (typeof context.worktreeExists === 'boolean') return context.worktreeExists;
88+
if (typeof session.worktreeExists === 'boolean') return session.worktreeExists;
89+
return worktreePath.length > 0;
90+
}
91+
92+
function disabledReason(item, context) {
93+
if (item.needsSession && !context.selected) return 'No pane selected';
94+
95+
const reasons = [];
96+
if (item.needsWorktree && !context.worktreeExists) reasons.push('Worktree missing');
97+
if (item.needsBranch && !context.branch) reasons.push('Branch missing');
98+
return reasons.join('; ');
99+
}
100+
101+
function createPaneMenuItems(context) {
102+
return PANE_MENU_ITEMS.map((item) => {
103+
const reason = disabledReason(item, context);
104+
return {
105+
id: item.id,
106+
label: item.label,
107+
hotkey: item.hotkey,
108+
shortcut: item.hotkey,
109+
enabled: reason.length === 0,
110+
danger: Boolean(item.danger),
111+
reason,
112+
};
113+
});
114+
}
115+
116+
function createPaneMenuState(options = {}) {
117+
const session = selectedSession(options);
118+
const selected = Boolean(session) && options.selected !== false;
119+
const source = session || {};
120+
const branch = selected ? resolveBranch(source, options) : '';
121+
const worktreePath = selected ? resolveWorktreePath(source, options) : '';
122+
const context = {
123+
selected,
124+
branch,
125+
worktreePath,
126+
worktreeExists: selected && resolveWorktreeExists(source, options, worktreePath),
127+
};
128+
const items = Array.isArray(options.items) && options.items.length > 0
129+
? options.items.map((item) => ({ ...item }))
130+
: createPaneMenuItems(context);
131+
132+
return paneMenu.createPaneMenuState({
133+
...options,
134+
session,
135+
title: paneMenuTitle(firstString(options.title, selectedPaneName(source, options))),
136+
items,
137+
});
138+
}
139+
140+
function applyPaneMenuKey(state = {}, rawKey) {
141+
return paneMenu.applyPaneMenuKey(createPaneMenuState(state), rawKey);
142+
}
143+
144+
function renderPaneMenu(state = {}, options = {}) {
145+
const selectedIndex = Number.isInteger(options.selectedIndex)
146+
? options.selectedIndex
147+
: state.selectedIndex;
148+
return paneMenu.renderPaneMenu(createPaneMenuState({ ...state, selectedIndex }), options).replace(/\u25b6/g, '>');
149+
}
150+
151+
function buildLaneMenu(session, context = {}) {
152+
return createPaneMenuState({ ...context, session });
153+
}
154+
155+
function renderLaneMenu(menu, options = {}) {
156+
return renderPaneMenu(menu, options);
157+
}
158+
159+
module.exports = {
160+
PANE_MENU_ACTIONS,
161+
PANE_MENU_ACTION_IDS,
162+
PANE_MENU_FOOTER,
163+
PANE_MENU_ITEMS,
164+
applyPaneMenuKey,
165+
buildLaneMenu,
166+
createPaneMenuState,
167+
normalizePaneMenuKey,
168+
renderLaneMenu,
169+
renderPaneMenu,
170+
};

test/cockpit-control.test.js

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -204,11 +204,17 @@ test('applyCockpitAction routes pane menu hotkeys to pane action intents', () =>
204204
worktreePath: '/tmp/one',
205205
});
206206

207-
state = applyCockpitAction(state, { type: 'key', key: 'P' });
208-
assert.equal(state.lastIntent.type, 'project-focus');
207+
state = applyCockpitAction(state, { type: 'key', key: 'p' });
208+
assert.equal(state.lastIntent.type, 'create-pr');
209209

210210
state = applyCockpitAction(state, { type: 'key', key: 'r' });
211-
assert.equal(state.lastIntent.type, 'reopen-closed-worktree');
211+
assert.equal(state.lastIntent.type, 'rename');
212+
213+
state = applyCockpitAction(state, { type: 'key', key: 'T' });
214+
assert.equal(state.lastIntent.type, 'add-terminal');
215+
216+
state = applyCockpitAction(state, { type: 'key', key: 'A' });
217+
assert.equal(state.lastIntent.type, 'add-agent');
212218
});
213219

214220
test('renderControlFrame renders sidebar with details, menu, and settings modes', () => {
@@ -224,9 +230,12 @@ test('renderControlFrame renders sidebar with details, menu, and settings modes'
224230
assert.match(details, /session: one/);
225231

226232
const menu = renderControlFrame(applyCockpitAction(baseState, { type: 'key', key: 'm' }));
233+
assert.match(menu, /^ {2,}/m);
227234
assert.match(menu, /Menu: codex/);
228-
assert.match(menu, /View\s+\[j\]/);
229-
assert.match(menu, /Project Focus\s+\[P\]/);
235+
assert.match(menu, /> View\s+\[v\]/);
236+
assert.match(menu, /Merge \/ Finish\s+\[m\]/);
237+
assert.match(menu, /Add Terminal to Worktree\s+\[T\]/);
238+
assert.doesNotMatch(menu, /Project Focus/);
230239

231240
const settings = renderControlFrame(applyCockpitAction(baseState, { type: 'key', key: 's' }));
232241
assert.match(settings, /gx cockpit settings/);

test/cockpit-kitty-integration.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ test('cockpit pane menu opens and selects a lane terminal action', () => {
163163
state = applyCockpitAction(state, { type: 'key', key: 'm' });
164164
assert.equal(state.mode, 'menu');
165165

166-
state = applyCockpitAction(state, { type: 'key', key: 'A' });
166+
state = applyCockpitAction(state, { type: 'key', key: 'T' });
167167

168168
assert.deepEqual(state.lastIntent, {
169169
type: 'add-terminal',

test/cockpit-menu.test.js

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,8 @@ test('buildLaneMenu returns the expected dmux-style pane actions', () => {
3434
'View',
3535
'Hide Pane',
3636
'Close',
37-
'Merge',
37+
'Merge / Finish',
3838
'Create GitHub PR',
39-
'Project Focus',
4039
'Rename',
4140
'Copy Path',
4241
'Open in Editor',
@@ -45,7 +44,6 @@ test('buildLaneMenu returns the expected dmux-style pane actions', () => {
4544
'Browse Files',
4645
'Add Terminal to Worktree',
4746
'Add Agent to Worktree',
48-
'Reopen Closed Worktree',
4947
],
5048
);
5149
assert.deepEqual(enabledIds(menu), [
@@ -54,7 +52,6 @@ test('buildLaneMenu returns the expected dmux-style pane actions', () => {
5452
PANE_MENU_ACTION_IDS.CLOSE,
5553
PANE_MENU_ACTION_IDS.MERGE,
5654
PANE_MENU_ACTION_IDS.CREATE_PR,
57-
PANE_MENU_ACTION_IDS.PROJECT_FOCUS,
5855
PANE_MENU_ACTION_IDS.RENAME,
5956
PANE_MENU_ACTION_IDS.COPY_PATH,
6057
PANE_MENU_ACTION_IDS.OPEN_EDITOR,
@@ -63,10 +60,12 @@ test('buildLaneMenu returns the expected dmux-style pane actions', () => {
6360
PANE_MENU_ACTION_IDS.BROWSE_FILES,
6461
PANE_MENU_ACTION_IDS.ADD_TERMINAL,
6562
PANE_MENU_ACTION_IDS.ADD_AGENT,
66-
PANE_MENU_ACTION_IDS.REOPEN_CLOSED_WORKTREE,
6763
]);
6864
assert.equal(itemById(menu, PANE_MENU_ACTION_IDS.CLOSE).danger, true);
69-
assert.equal(itemById(menu, PANE_MENU_ACTION_IDS.PROJECT_FOCUS).shortcut, 'P');
65+
assert.equal(itemById(menu, PANE_MENU_ACTION_IDS.VIEW).shortcut, 'v');
66+
assert.equal(itemById(menu, PANE_MENU_ACTION_IDS.CREATE_PR).shortcut, 'p');
67+
assert.equal(itemById(menu, PANE_MENU_ACTION_IDS.ADD_TERMINAL).shortcut, 'T');
68+
assert.equal(itemById(menu, PANE_MENU_ACTION_IDS.ADD_AGENT).shortcut, 'A');
7069
});
7170

7271
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', ()
9291
assert.equal(itemById(menu, 'view').enabled, true);
9392
assert.equal(itemById(menu, 'hide-pane').enabled, true);
9493
assert.equal(itemById(menu, 'close').enabled, true);
95-
assert.equal(itemById(menu, 'project-focus').enabled, true);
9694
assert.equal(itemById(menu, 'rename').enabled, true);
97-
assert.equal(itemById(menu, 'reopen-closed-worktree').enabled, true);
9895

9996
for (const id of ['merge', 'create-pr', 'copy-path', 'open-editor', 'toggle-autopilot', 'create-child-worktree', 'browse-files', 'add-terminal', 'add-agent']) {
10097
const item = itemById(menu, id);
@@ -113,12 +110,12 @@ test('buildLaneMenu disables branch actions when the branch is missing', () => {
113110

114111
assert.equal(itemById(menu, 'view').enabled, true);
115112
assert.equal(itemById(menu, 'hide-pane').enabled, true);
116-
assert.equal(itemById(menu, 'project-focus').enabled, true);
113+
assert.equal(itemById(menu, 'close').enabled, true);
114+
assert.equal(itemById(menu, 'rename').enabled, true);
117115
assert.equal(itemById(menu, 'copy-path').enabled, true);
118116
assert.equal(itemById(menu, 'open-editor').enabled, true);
119117
assert.equal(itemById(menu, 'browse-files').enabled, true);
120118
assert.equal(itemById(menu, 'add-terminal').enabled, true);
121-
assert.equal(itemById(menu, 'reopen-closed-worktree').enabled, true);
122119

123120
for (const id of ['merge', 'create-pr', 'toggle-autopilot', 'create-child-worktree', 'add-agent']) {
124121
const item = itemById(menu, id);
@@ -138,8 +135,9 @@ test('renderLaneMenu renders a boxed menu with an ASCII fallback', () => {
138135
const unicodeOutput = renderLaneMenu(menu, { selectedIndex: 2 });
139136
assert.match(unicodeOutput, /^/);
140137
assert.match(unicodeOutput, /Menu: codex/);
138+
assert.match(unicodeOutput, /> Close\s+\[x\]/);
141139
assert.match(unicodeOutput, /Close\s+\[x\]/);
142-
assert.match(unicodeOutput, /Project Focus\s+\[P\]/);
140+
assert.match(unicodeOutput, /Merge \/ Finish\s+\[m\]/);
143141
assert.match(unicodeOutput, /Create GitHub PR/);
144142

145143
const asciiOutput = renderLaneMenu(menu, { unicode: false });

0 commit comments

Comments
 (0)