diff --git a/docs/reference/keyboard-shortcuts.md b/docs/reference/keyboard-shortcuts.md index 1402422c6b..4ef748f656 100644 --- a/docs/reference/keyboard-shortcuts.md +++ b/docs/reference/keyboard-shortcuts.md @@ -118,6 +118,7 @@ available combinations. | Show warning when trying to move focus away from shell input. | `Tab (no Shift)` | | Move focus from Gemini to the active shell. | `Tab (no Shift)` | | Move focus from the shell back to Gemini. | `Shift + Tab` | +| Open the current plan in an external editor. | `Ctrl + X` | | Clear the terminal screen and redraw the UI. | `Ctrl + L` | | Restart the application. | `R` | | Suspend the CLI and move it to the background. | `Ctrl + Z` | diff --git a/packages/cli/src/config/keyBindings.ts b/packages/cli/src/config/keyBindings.ts index 4813abd368..4c0c091141 100644 --- a/packages/cli/src/config/keyBindings.ts +++ b/packages/cli/src/config/keyBindings.ts @@ -94,6 +94,7 @@ export enum Command { EXPAND_PASTE = 'app.expandPaste', FOCUS_SHELL_INPUT = 'app.focusShellInput', UNFOCUS_SHELL_INPUT = 'app.unfocusShellInput', + OPEN_PLAN_IN_EDITOR = 'plan.openInEditor', CLEAR_SCREEN = 'app.clearScreen', RESTART_APP = 'app.restart', SUSPEND_APP = 'app.suspend', @@ -294,6 +295,7 @@ export const defaultKeyBindings: KeyBindingConfig = { [Command.EXPAND_PASTE]: [{ key: 'o', ctrl: true }], [Command.FOCUS_SHELL_INPUT]: [{ key: 'tab', shift: false }], [Command.UNFOCUS_SHELL_INPUT]: [{ key: 'tab', shift: true }], + [Command.OPEN_PLAN_IN_EDITOR]: [{ key: 'x', ctrl: true }], [Command.CLEAR_SCREEN]: [{ key: 'l', ctrl: true }], [Command.RESTART_APP]: [{ key: 'r' }], [Command.SUSPEND_APP]: [{ key: 'z', ctrl: true }], @@ -414,6 +416,7 @@ export const commandCategories: readonly CommandCategory[] = [ Command.SHOW_SHELL_INPUT_UNFOCUS_WARNING, Command.FOCUS_SHELL_INPUT, Command.UNFOCUS_SHELL_INPUT, + Command.OPEN_PLAN_IN_EDITOR, Command.CLEAR_SCREEN, Command.RESTART_APP, Command.SUSPEND_APP, @@ -522,6 +525,7 @@ export const commandDescriptions: Readonly> = { 'Show warning when trying to move focus away from shell input.', [Command.FOCUS_SHELL_INPUT]: 'Move focus from Gemini to the active shell.', [Command.UNFOCUS_SHELL_INPUT]: 'Move focus from the shell back to Gemini.', + [Command.OPEN_PLAN_IN_EDITOR]: 'Open the current plan in an external editor.', [Command.CLEAR_SCREEN]: 'Clear the terminal screen and redraw the UI.', [Command.RESTART_APP]: 'Restart the application.', [Command.SUSPEND_APP]: 'Suspend the CLI and move it to the background.', diff --git a/packages/cli/src/ui/components/AskUserDialog.tsx b/packages/cli/src/ui/components/AskUserDialog.tsx index 1d31b1a1f4..afa2011b9e 100644 --- a/packages/cli/src/ui/components/AskUserDialog.tsx +++ b/packages/cli/src/ui/components/AskUserDialog.tsx @@ -183,6 +183,10 @@ interface AskUserDialogProps { * Height constraint for scrollable content. */ availableHeight?: number; + /** + * Additional shortcuts to display in the footer. + */ + extraFooterActions?: string[]; } interface ReviewViewProps { @@ -925,6 +929,7 @@ export const AskUserDialog: React.FC = ({ onActiveTextInputChange, width, availableHeight: availableHeightProp, + extraFooterActions, }) => { const uiState = useContext(UIStateContext); const availableHeight = @@ -1143,6 +1148,7 @@ export const AskUserDialog: React.FC = ({ ? undefined : '↑/↓ to navigate' } + extraActions={extraFooterActions} /> ); diff --git a/packages/cli/src/ui/components/ExitPlanModeDialog.test.tsx b/packages/cli/src/ui/components/ExitPlanModeDialog.test.tsx index 26b61829a0..034bbced54 100644 --- a/packages/cli/src/ui/components/ExitPlanModeDialog.test.tsx +++ b/packages/cli/src/ui/components/ExitPlanModeDialog.test.tsx @@ -16,6 +16,7 @@ import { validatePlanContent, processSingleFileContent, type FileSystemService, + openFileInEditor, } from '@google/gemini-cli-core'; import * as fs from 'node:fs'; @@ -27,6 +28,13 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { validatePlanPath: vi.fn(async () => null), validatePlanContent: vi.fn(async () => null), processSingleFileContent: vi.fn(), + openFileInEditor: vi.fn(async () => ({ modified: true })), + IdeClient: { + getInstance: vi.fn(async () => ({ + isDiffingEnabled: vi.fn(() => false), + openDiff: vi.fn(), + })), + }, }; }); @@ -204,7 +212,7 @@ Implement a comprehensive authentication system with multiple providers. writeKey(stdin, '\r'); await waitFor(() => { - expect(onApprove).toHaveBeenCalledWith(ApprovalMode.AUTO_EDIT); + expect(onApprove).toHaveBeenCalledWith(ApprovalMode.AUTO_EDIT, false); }); }); @@ -223,7 +231,7 @@ Implement a comprehensive authentication system with multiple providers. writeKey(stdin, '\r'); await waitFor(() => { - expect(onApprove).toHaveBeenCalledWith(ApprovalMode.DEFAULT); + expect(onApprove).toHaveBeenCalledWith(ApprovalMode.DEFAULT, false); }); }); @@ -254,7 +262,7 @@ Implement a comprehensive authentication system with multiple providers. writeKey(stdin, '\r'); await waitFor(() => { - expect(onFeedback).toHaveBeenCalledWith('Add tests'); + expect(onFeedback).toHaveBeenCalledWith('Add tests', false); }); }); @@ -350,7 +358,7 @@ Implement a comprehensive authentication system with multiple providers. writeKey(stdin, '2'); await waitFor(() => { - expect(onApprove).toHaveBeenCalledWith(ApprovalMode.DEFAULT); + expect(onApprove).toHaveBeenCalledWith(ApprovalMode.DEFAULT, false); }); }); @@ -531,10 +539,50 @@ Implement a comprehensive authentication system with multiple providers. writeKey(stdin, '\r'); await waitFor(() => { - expect(onApprove).toHaveBeenCalledWith(ApprovalMode.DEFAULT); + expect(onApprove).toHaveBeenCalledWith(ApprovalMode.DEFAULT, false); }); expect(onFeedback).not.toHaveBeenCalled(); }); + + it('opens editor when Ctrl+X is pressed and reloads plan', async () => { + const { stdin, lastFrame } = renderDialog({ useAlternateBuffer }); + + await act(async () => { + vi.runAllTimers(); + }); + + await waitFor(() => { + expect(lastFrame()).toContain('Add user authentication'); + }); + + const updatedPlanContent = 'Updated plan content'; + vi.mocked(processSingleFileContent).mockResolvedValue({ + llmContent: updatedPlanContent, + returnDisplay: 'Read file', + }); + + writeKey(stdin, '\x18'); // Ctrl+X + + await waitFor(() => { + expect(openFileInEditor).toHaveBeenCalled(); + }); + + // Advance timers for reload + await act(async () => { + vi.runAllTimers(); + }); + + await waitFor(() => { + expect(lastFrame()).toContain(updatedPlanContent); + }); + + // Submit to verify planModified is true + writeKey(stdin, '\r'); + + await waitFor(() => { + expect(onApprove).toHaveBeenCalledWith(ApprovalMode.AUTO_EDIT, true); + }); + }); }, ); }); diff --git a/packages/cli/src/ui/components/ExitPlanModeDialog.tsx b/packages/cli/src/ui/components/ExitPlanModeDialog.tsx index 8777136d86..553eb7084f 100644 --- a/packages/cli/src/ui/components/ExitPlanModeDialog.tsx +++ b/packages/cli/src/ui/components/ExitPlanModeDialog.tsx @@ -5,7 +5,7 @@ */ import type React from 'react'; -import { useEffect, useState } from 'react'; +import { useCallback, useEffect, useMemo, useState } from 'react'; import { Box, Text } from 'ink'; import { ApprovalMode, @@ -14,15 +14,24 @@ import { QuestionType, type Config, processSingleFileContent, + coreEvents, + openFileInEditor, + IdeClient, + debugLogger, + isValidEditorType, } from '@google/gemini-cli-core'; import { theme } from '../semantic-colors.js'; import { useConfig } from '../contexts/ConfigContext.js'; import { AskUserDialog } from './AskUserDialog.js'; +import { useSettings } from '../contexts/SettingsContext.js'; +import { useKeypress } from '../hooks/useKeypress.js'; +import { KeypressPriority } from '../contexts/KeypressContext.js'; +import { Command, keyMatchers } from '../keyMatchers.js'; export interface ExitPlanModeDialogProps { planPath: string; - onApprove: (approvalMode: ApprovalMode) => void; - onFeedback: (feedback: string) => void; + onApprove: (approvalMode: ApprovalMode, planModified: boolean) => void; + onFeedback: (feedback: string, planModified: boolean) => void; onCancel: () => void; width: number; availableHeight?: number; @@ -38,6 +47,7 @@ interface PlanContentState { status: PlanStatus; content?: string; error?: string; + reload: () => void; } enum ApprovalOption { @@ -53,10 +63,15 @@ const StatusMessage: React.FC<{ }> = ({ children }) => {children}; function usePlanContent(planPath: string, config: Config): PlanContentState { - const [state, setState] = useState({ + const [nonce, setNonce] = useState(0); + const [state, setState] = useState>({ status: PlanStatus.Loading, }); + const reload = useCallback(() => { + setNonce((n) => n + 1); + }, []); + useEffect(() => { let ignore = false; setState({ status: PlanStatus.Loading }); @@ -120,9 +135,9 @@ function usePlanContent(planPath: string, config: Config): PlanContentState { return () => { ignore = true; }; - }, [planPath, config]); + }, [planPath, config, nonce]); - return state; + return useMemo(() => ({ ...state, reload }), [state, reload]); } export const ExitPlanModeDialog: React.FC = ({ @@ -134,9 +149,11 @@ export const ExitPlanModeDialog: React.FC = ({ availableHeight, }) => { const config = useConfig(); + const settings = useSettings(); const planState = usePlanContent(planPath, config); const [showLoading, setShowLoading] = useState(false); - + const [isModified, setIsModified] = useState(false); + const [isEditing, setIsEditing] = useState(false); useEffect(() => { if (planState.status !== PlanStatus.Loading) { setShowLoading(false); @@ -150,6 +167,80 @@ export const ExitPlanModeDialog: React.FC = ({ return () => clearTimeout(timer); }, [planState.status]); + const performOpenInEditor = useCallback(async () => { + if (isEditing) return; + setIsEditing(true); + try { + const ideClient = await IdeClient.getInstance(); + const preferredEditorRaw = settings.merged.general.preferredEditor; + const preferredEditor = + typeof preferredEditorRaw === 'string' && + isValidEditorType(preferredEditorRaw) + ? preferredEditorRaw + : undefined; + + const result = await openFileInEditor(planPath, { + preferredEditor, + ideClient, + readTextFile: (path: string) => + config.getFileSystemService().readTextFile(path), + writeTextFile: (path, content) => + config.getFileSystemService().writeTextFile(path, content), + }); + + if (result.modified) { + setIsModified(true); + planState.reload(); + } + } catch (err) { + coreEvents.emitFeedback( + 'error', + '[ExitPlanModeDialog] external editor error', + err, + ); + } finally { + setIsEditing(false); + } + }, [ + isEditing, + settings.merged.general.preferredEditor, + planPath, + config, + planState, + ]); + + const handleOpenInEditor = useCallback(() => { + void performOpenInEditor(); + }, [performOpenInEditor]); + + const syncIde = useCallback( + async (outcome: 'accepted' | 'rejected') => { + try { + const ideClient = await IdeClient.getInstance(); + if (ideClient.isDiffingEnabled()) { + await ideClient.resolveDiffFromCli(planPath, outcome); + } + } catch (err) { + debugLogger.error('ExitPlanModeDialog: IDE sync failed:', err); + } + }, + [planPath], + ); + + useKeypress( + (key) => { + if (keyMatchers[Command.OPEN_PLAN_IN_EDITOR](key)) { + handleOpenInEditor(); + return true; + } + return false; + }, + { + isActive: planState.status === PlanStatus.Loaded && !isEditing, + priority: KeypressPriority.Critical, + }, + ); + if (planState.status === PlanStatus.Loading) { if (!showLoading) { return null; @@ -209,17 +300,24 @@ export const ExitPlanModeDialog: React.FC = ({ ]} onSubmit={(answers) => { const answer = answers['0']; + // Sync IDE state first + void syncIde('accepted'); + if (answer === ApprovalOption.Auto) { - onApprove(ApprovalMode.AUTO_EDIT); + onApprove(ApprovalMode.AUTO_EDIT, isModified); } else if (answer === ApprovalOption.Manual) { - onApprove(ApprovalMode.DEFAULT); + onApprove(ApprovalMode.DEFAULT, isModified); } else if (answer) { - onFeedback(answer); + onFeedback(answer, isModified); } }} - onCancel={onCancel} + onCancel={() => { + void syncIde('rejected'); + onCancel(); + }} width={width} availableHeight={availableHeight} + extraFooterActions={['Ctrl+X to open in editor']} /> ); diff --git a/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap index 587ded8f29..0cd2e60862 100644 --- a/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap @@ -23,7 +23,7 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select · ↑/↓ to navigate · Esc to cancel +Enter to select · ↑/↓ to navigate · Ctrl+X to open in editor · Esc to cancel " `; @@ -50,7 +50,7 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select · ↑/↓ to navigate · Esc to cancel +Enter to select · ↑/↓ to navigate · Ctrl+X to open in editor · Esc to cancel " `; @@ -82,7 +82,7 @@ Implementation Steps Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select · ↑/↓ to navigate · Esc to cancel +Enter to select · ↑/↓ to navigate · Ctrl+X to open in editor · Esc to cancel " `; @@ -109,7 +109,7 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select · ↑/↓ to navigate · Esc to cancel +Enter to select · ↑/↓ to navigate · Ctrl+X to open in editor · Esc to cancel " `; @@ -136,7 +136,7 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select · ↑/↓ to navigate · Esc to cancel +Enter to select · ↑/↓ to navigate · Ctrl+X to open in editor · Esc to cancel " `; @@ -163,7 +163,7 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select · ↑/↓ to navigate · Esc to cancel +Enter to select · ↑/↓ to navigate · Ctrl+X to open in editor · Esc to cancel " `; @@ -216,7 +216,7 @@ Testing Strategy Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select · ↑/↓ to navigate · Esc to cancel +Enter to select · ↑/↓ to navigate · Ctrl+X to open in editor · Esc to cancel " `; @@ -243,6 +243,6 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select · ↑/↓ to navigate · Esc to cancel +Enter to select · ↑/↓ to navigate · Ctrl+X to open in editor · Esc to cancel " `; diff --git a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap index b5e013ef48..df09b0e0e8 100644 --- a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap @@ -85,7 +85,7 @@ exports[`ToolConfirmationQueue > renders ExitPlanMode tool confirmation with Suc │ Approves plan but requires confirmation for each tool │ │ 3. Type your feedback... │ │ │ -│ Enter to select · ↑/↓ to navigate · Esc to cancel │ +│ Enter to select · ↑/↓ to navigate · Ctrl+X to open in editor · Esc to cancel │ ╰──────────────────────────────────────────────────────────────────────────────╯ " `; diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index c4e73b73f6..4409195c6e 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -340,16 +340,18 @@ export const ToolConfirmationMessage: React.FC< bodyContent = ( { + onApprove={(approvalMode, planModified) => { handleConfirm(ToolConfirmationOutcome.ProceedOnce, { approved: true, approvalMode, + planModified, }); }} - onFeedback={(feedback) => { + onFeedback={(feedback, planModified) => { handleConfirm(ToolConfirmationOutcome.ProceedOnce, { approved: false, feedback, + planModified, }); }} onCancel={() => { diff --git a/packages/cli/src/ui/components/shared/DialogFooter.tsx b/packages/cli/src/ui/components/shared/DialogFooter.tsx index af75074645..be849abde6 100644 --- a/packages/cli/src/ui/components/shared/DialogFooter.tsx +++ b/packages/cli/src/ui/components/shared/DialogFooter.tsx @@ -15,6 +15,8 @@ export interface DialogFooterProps { navigationActions?: string; /** Exit shortcut (defaults to "Esc to cancel") */ cancelAction?: string; + /** Additional shortcuts to display */ + extraActions?: string[]; } /** @@ -25,11 +27,15 @@ export const DialogFooter: React.FC = ({ primaryAction, navigationActions, cancelAction = 'Esc to cancel', + extraActions, }) => { const parts = [primaryAction]; if (navigationActions) { parts.push(navigationActions); } + if (extraActions) { + parts.push(...extraActions); + } parts.push(cancelAction); return ( diff --git a/packages/core/src/tools/exit-plan-mode.test.ts b/packages/core/src/tools/exit-plan-mode.test.ts index 22de81fc7f..3af966d445 100644 --- a/packages/core/src/tools/exit-plan-mode.test.ts +++ b/packages/core/src/tools/exit-plan-mode.test.ts @@ -240,6 +240,28 @@ Read and follow the plan strictly during implementation.`, expect(mockConfig.setApprovedPlanPath).toHaveBeenCalledWith(expectedPath); }); + it('should include modified note when planModified is true', async () => { + const planRelativePath = createPlanFile('test.md', '# Content'); + const invocation = tool.build({ plan_path: planRelativePath }); + + const confirmDetails = await invocation.shouldConfirmExecute( + new AbortController().signal, + ); + expect(confirmDetails).not.toBe(false); + if (confirmDetails === false) return; + + await confirmDetails.onConfirm(ToolConfirmationOutcome.ProceedOnce, { + approved: true, + approvalMode: ApprovalMode.DEFAULT, + planModified: true, + }); + + const result = await invocation.execute(new AbortController().signal); + expect(result.llmContent).toContain( + 'Note: The user modified the plan file in an external editor.', + ); + }); + it('should return feedback message when plan is rejected with feedback', async () => { const planRelativePath = createPlanFile('test.md', '# Content'); const invocation = tool.build({ plan_path: planRelativePath }); diff --git a/packages/core/src/tools/exit-plan-mode.ts b/packages/core/src/tools/exit-plan-mode.ts index c11eaa119e..6f9a922efe 100644 --- a/packages/core/src/tools/exit-plan-mode.ts +++ b/packages/core/src/tools/exit-plan-mode.ts @@ -228,9 +228,12 @@ export class ExitPlanModeInvocation extends BaseToolInvocation< logPlanExecution(this.config, new PlanExecutionEvent(newMode)); const description = getApprovalModeDescription(newMode); + const modifiedNote = payload.planModified + ? '\n\nNote: The user modified the plan file in an external editor.' + : ''; return { - llmContent: `Plan approved. Switching to ${description}. + llmContent: `Plan approved.${modifiedNote} Switching to ${description}. The approved implementation plan is stored at: ${resolvedPlanPath} Read and follow the plan strictly during implementation.`, @@ -238,9 +241,12 @@ Read and follow the plan strictly during implementation.`, }; } else { const feedback = payload?.feedback?.trim(); + const modifiedNote = payload?.planModified + ? ' (Note: The user also modified the plan file in an external editor)' + : ''; if (feedback) { return { - llmContent: `Plan rejected. User feedback: ${feedback} + llmContent: `Plan rejected. User feedback: ${feedback}${modifiedNote} The plan is stored at: ${resolvedPlanPath} Revise the plan based on the feedback.`, @@ -248,7 +254,7 @@ Revise the plan based on the feedback.`, }; } else { return { - llmContent: `Plan rejected. No feedback provided. + llmContent: `Plan rejected. No feedback provided.${modifiedNote} The plan is stored at: ${resolvedPlanPath} Ask the user for specific feedback on how to improve the plan.`, diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts index acbbd7bfff..190db9ad64 100644 --- a/packages/core/src/tools/tools.ts +++ b/packages/core/src/tools/tools.ts @@ -734,6 +734,8 @@ export interface ToolExitPlanModeConfirmationPayload { approvalMode?: ApprovalMode; /** If rejected, the user's feedback */ feedback?: string; + /** Whether the user modified the plan file in an external editor */ + planModified?: boolean; } export type ToolConfirmationPayload = diff --git a/packages/core/src/utils/editor.test.ts b/packages/core/src/utils/editor.test.ts index d46c58d677..2b6b23302d 100644 --- a/packages/core/src/utils/editor.test.ts +++ b/packages/core/src/utils/editor.test.ts @@ -22,6 +22,7 @@ import { isEditorAvailable, isEditorAvailableAsync, resolveEditorAsync, + openFileInEditor, type EditorType, } from './editor.js'; import { coreEvents, CoreEvent } from './events.js'; @@ -710,4 +711,86 @@ describe('editor utils', () => { expect(emitSpy).toHaveBeenCalledWith(CoreEvent.RequestEditorSelection); }); }); + + describe('openFileInEditor', () => { + it('should return modified: true when no readTextFile is provided (legacy behavior)', async () => { + (spawnSync as Mock).mockReturnValue({ status: 0 }); + + const result = await openFileInEditor('test.txt'); + + expect(result).toEqual({ modified: true }); + }); + + it('should return modified: true when content changed', async () => { + (spawnSync as Mock).mockReturnValue({ status: 0 }); + const readTextFile = vi + .fn() + .mockResolvedValueOnce('initial content') + .mockResolvedValueOnce('changed content'); + + const result = await openFileInEditor('test.txt', { readTextFile }); + + expect(result).toEqual({ modified: true }); + expect(readTextFile).toHaveBeenCalledTimes(2); + }); + + it('should return modified: false when content is same', async () => { + (spawnSync as Mock).mockReturnValue({ status: 0 }); + const readTextFile = vi + .fn() + .mockResolvedValueOnce('initial content') + .mockResolvedValueOnce('initial content'); + + const result = await openFileInEditor('test.txt', { readTextFile }); + + expect(result).toEqual({ modified: false }); + expect(readTextFile).toHaveBeenCalledTimes(2); + }); + + it("should return modified: true if file was created (didn't exist before)", async () => { + (spawnSync as Mock).mockReturnValue({ status: 0 }); + const readTextFile = vi + .fn() + .mockRejectedValueOnce(new Error('ENOENT')) + .mockResolvedValueOnce('new content'); + + const result = await openFileInEditor('test.txt', { readTextFile }); + + expect(result).toEqual({ modified: true }); + expect(readTextFile).toHaveBeenCalledTimes(2); + }); + + it('should return modified: true when using GUI spawn and content changed', async () => { + const mockChild = { + on: vi.fn((event, cb) => { + if (event === 'close') { + setTimeout(() => cb(0), 0); + } + }), + }; + (spawn as Mock).mockReturnValue(mockChild); + + const readTextFile = vi + .fn() + .mockResolvedValueOnce('initial content') + .mockResolvedValueOnce('changed content'); + + const result = await openFileInEditor('test.txt', { + readTextFile, + preferredEditor: 'vscode', + }); + + expect(result).toEqual({ modified: true }); + expect(readTextFile).toHaveBeenCalledTimes(2); + }); + + it('should emit ExternalEditorClosed event', async () => { + (spawnSync as Mock).mockReturnValue({ status: 0 }); + const emitSpy = vi.spyOn(coreEvents, 'emit'); + + await openFileInEditor('test.txt'); + + expect(emitSpy).toHaveBeenCalledWith(CoreEvent.ExternalEditorClosed); + }); + }); }); diff --git a/packages/core/src/utils/editor.ts b/packages/core/src/utils/editor.ts index cdc1e1d4a5..d68c00fdf4 100644 --- a/packages/core/src/utils/editor.ts +++ b/packages/core/src/utils/editor.ts @@ -4,11 +4,30 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { exec, execSync, spawn, spawnSync } from 'node:child_process'; +import { exec, spawn, spawnSync, execSync } from 'node:child_process'; import { promisify } from 'node:util'; import { once } from 'node:events'; import { debugLogger } from './debugLogger.js'; import { coreEvents, CoreEvent, type EditorSelectedPayload } from './events.js'; +import type { DiffUpdateResult } from '../ide/ide-client.js'; + +const execAsync = promisify(exec); + +/** + * Interface for an object that can open a diff in an IDE. + * Decouples editor utility from IdeClient implementation to avoid circular dependencies. + */ +export interface OpenFileIdeClient { + isDiffingEnabled(): boolean; + openDiff(filePath: string, content: string): Promise; +} + +export interface OpenFileInEditorOptions { + preferredEditor?: EditorType; + ideClient?: OpenFileIdeClient; + readTextFile?: (path: string) => Promise; + writeTextFile?: (path: string, content: string) => Promise; +} const GUI_EDITORS = [ 'vscode', @@ -61,7 +80,7 @@ export function getEditorDisplayName(editor: EditorType): string { return EDITOR_DISPLAY_NAMES[editor] || editor; } -function isValidEditorType(editor: string): editor is EditorType { +export function isValidEditorType(editor: string): editor is EditorType { return EDITORS_SET.has(editor); } @@ -78,8 +97,6 @@ interface DiffCommand { args: string[]; } -const execAsync = promisify(exec); - function getCommandExistsCmd(cmd: string): string { return process.platform === 'win32' ? `where.exe ${cmd}` @@ -141,11 +158,10 @@ export function hasValidEditorCommand(editor: EditorType): boolean { export async function hasValidEditorCommandAsync( editor: EditorType, ): Promise { - return Promise.any( - getEditorCommands(editor).map((cmd) => - commandExistsAsync(cmd).then((exists) => exists || Promise.reject()), - ), - ).catch(() => false); + const results = await Promise.allSettled( + getEditorCommands(editor).map((cmd) => commandExistsAsync(cmd)), + ); + return results.some((r) => r.status === 'fulfilled' && r.value); } export function getEditorCommand(editor: EditorType): string { @@ -336,3 +352,130 @@ export async function openDiff( }); }); } + +/** + * Opens a file in an editor. + * If an IDE client is provided and connected, it uses openDiff for an integrated experience. + * Otherwise, it falls back to external editors (GUI or Terminal). + */ +export async function openFileInEditor( + filePath: string, + options: OpenFileInEditorOptions = {}, +): Promise<{ modified: boolean }> { + const { ideClient, preferredEditor, readTextFile, writeTextFile } = options; + + // 1. Try IDE Flow + if (ideClient?.isDiffingEnabled() && readTextFile && writeTextFile) { + debugLogger.debug(`openFileInEditor: Using IDE flow for ${filePath}`); + try { + const currentContent = await readTextFile(filePath); + const result = await ideClient.openDiff(filePath, currentContent); + if (result.status === 'accepted' && result.content !== undefined) { + if (result.content !== currentContent) { + await writeTextFile(filePath, result.content); + return { modified: true }; + } + } + return { modified: false }; + } catch (err) { + debugLogger.error( + 'openFileInEditor: IDE flow failed, falling back:', + err, + ); + // Fall through to external editor + } + } + + // 2. Resolve external editor command + let command: string | undefined = undefined; + const args = [filePath]; + + if (preferredEditor) { + command = getEditorCommand(preferredEditor); + if (isGuiEditor(preferredEditor)) { + args.unshift('--wait'); + } + } + + if (!command) { + command = + process.env['VISUAL'] ?? + process.env['EDITOR'] ?? + (process.platform === 'win32' ? 'notepad' : 'vim'); + } + + // DEFINITIVE FIX for Vim E138: Always add -i NONE when we detect vim/nvim + const commandBase = command.toLowerCase(); + if (commandBase.includes('vim') || commandBase.includes('nvim')) { + args.unshift('-i', 'NONE'); + } + + const useGuiSpawn = preferredEditor && isGuiEditor(preferredEditor); + debugLogger.debug( + `openFileInEditor: Using external editor: ${command} ${args.join(' ')} (GUI spawn: ${useGuiSpawn})`, + ); + + const initialContent = readTextFile + ? await readTextFile(filePath).catch(() => undefined) + : undefined; + + return new Promise<{ modified: boolean }>((resolve, reject) => { + const wasRaw = process.stdin.isRaw; + if (!useGuiSpawn && wasRaw) { + process.stdin.setRawMode(false); + } + + const onExit = async (status: number | null, error?: Error) => { + if (!useGuiSpawn && wasRaw) { + process.stdin.setRawMode(true); + } + coreEvents.emit(CoreEvent.ExternalEditorClosed); + + if (error) { + reject(error); + } else if (status !== null && status !== 0) { + reject(new Error(`Editor exited with status ${status}`)); + } else { + if (readTextFile) { + try { + const finalContent = await readTextFile(filePath); + resolve({ modified: initialContent !== finalContent }); + } catch (err) { + debugLogger.error( + `openFileInEditor: Failed to read file after editor exit: ${filePath}`, + err, + ); + // Fallback to true if we can't read the file but it existed or was supposed to + resolve({ modified: true }); + } + } else { + // Assume modified if external editor was used and closed successfully + resolve({ modified: true }); + } + } + }; + + if (useGuiSpawn) { + const child = spawn(command, args, { + stdio: 'inherit', + shell: process.platform === 'win32', + }); + child.on('close', (code) => { + void onExit(code); + }); + child.on('error', (err) => { + void onExit(null, err); + }); + } else { + try { + const result = spawnSync(command, args, { + stdio: 'inherit', + shell: process.platform === 'win32', + }); + void onExit(result.status, result.error || undefined); + } catch (err: unknown) { + void onExit(null, err instanceof Error ? err : new Error(String(err))); + } + } + }); +}