fix(cli): restore 'Modify with editor' option in external terminals (#17621)

This commit is contained in:
Abhi
2026-01-26 21:24:25 -05:00
committed by GitHub
parent ad0bece6d6
commit 68649c8dec
4 changed files with 162 additions and 10 deletions

View File

@@ -32,6 +32,7 @@ describe('ToolConfirmationMessage', () => {
vi.mocked(useToolActions).mockReturnValue({
confirm: mockConfirm,
cancel: vi.fn(),
isDiffingEnabled: false,
});
const mockConfig = {
@@ -274,4 +275,92 @@ describe('ToolConfirmationMessage', () => {
expect(lastFrame()).toContain('Allow for all future sessions');
});
});
describe('Modify with external editor option', () => {
const editConfirmationDetails: ToolCallConfirmationDetails = {
type: 'edit',
title: 'Confirm Edit',
fileName: 'test.txt',
filePath: '/test.txt',
fileDiff: '...diff...',
originalContent: 'a',
newContent: 'b',
onConfirm: vi.fn(),
};
it('should show "Modify with external editor" when NOT in IDE mode', () => {
const mockConfig = {
isTrustedFolder: () => true,
getIdeMode: () => false,
} as unknown as Config;
vi.mocked(useToolActions).mockReturnValue({
confirm: vi.fn(),
cancel: vi.fn(),
isDiffingEnabled: false,
});
const { lastFrame } = renderWithProviders(
<ToolConfirmationMessage
callId="test-call-id"
confirmationDetails={editConfirmationDetails}
config={mockConfig}
availableTerminalHeight={30}
terminalWidth={80}
/>,
);
expect(lastFrame()).toContain('Modify with external editor');
});
it('should show "Modify with external editor" when in IDE mode but diffing is NOT enabled', () => {
const mockConfig = {
isTrustedFolder: () => true,
getIdeMode: () => true,
} as unknown as Config;
vi.mocked(useToolActions).mockReturnValue({
confirm: vi.fn(),
cancel: vi.fn(),
isDiffingEnabled: false,
});
const { lastFrame } = renderWithProviders(
<ToolConfirmationMessage
callId="test-call-id"
confirmationDetails={editConfirmationDetails}
config={mockConfig}
availableTerminalHeight={30}
terminalWidth={80}
/>,
);
expect(lastFrame()).toContain('Modify with external editor');
});
it('should NOT show "Modify with external editor" when in IDE mode AND diffing is enabled', () => {
const mockConfig = {
isTrustedFolder: () => true,
getIdeMode: () => true,
} as unknown as Config;
vi.mocked(useToolActions).mockReturnValue({
confirm: vi.fn(),
cancel: vi.fn(),
isDiffingEnabled: true,
});
const { lastFrame } = renderWithProviders(
<ToolConfirmationMessage
callId="test-call-id"
confirmationDetails={editConfirmationDetails}
config={mockConfig}
availableTerminalHeight={30}
terminalWidth={80}
/>,
);
expect(lastFrame()).not.toContain('Modify with external editor');
});
});
});

View File

@@ -52,7 +52,7 @@ export const ToolConfirmationMessage: React.FC<
availableTerminalHeight,
terminalWidth,
}) => {
const { confirm } = useToolActions();
const { confirm, isDiffingEnabled } = useToolActions();
const settings = useSettings();
const allowPermanentApproval =
@@ -111,9 +111,9 @@ export const ToolConfirmationMessage: React.FC<
});
}
}
// We hide "Modify with external editor" if IDE mode is active, assuming
// the IDE provides a better interface (diff view) for this.
if (!config.getIdeMode()) {
// We hide "Modify with external editor" if IDE mode is active AND
// the IDE is actually capable of showing a diff (connected).
if (!config.getIdeMode() || !isDiffingEnabled) {
options.push({
label: 'Modify with external editor',
value: ToolConfirmationOutcome.ModifyWithEditor,
@@ -210,7 +210,13 @@ export const ToolConfirmationMessage: React.FC<
});
}
return options;
}, [confirmationDetails, isTrustedFolder, allowPermanentApproval, config]);
}, [
confirmationDetails,
isTrustedFolder,
allowPermanentApproval,
config,
isDiffingEnabled,
]);
const availableBodyContentHeight = useCallback(() => {
if (availableTerminalHeight === undefined) {