diff --git a/cli.py b/cli.py index eb56695efc1..6ada4bdaba1 100644 --- a/cli.py +++ b/cli.py @@ -9789,31 +9789,6 @@ class HermesCLI: # EEXIST. The suffix keeps markdown highlighting without that bug. input_area.buffer.tempfile_suffix = '.md' - # prompt_toolkit's default fallback chain prefers /usr/bin/nano over - # /usr/bin/vi when neither $VISUAL nor $EDITOR is set. The TUI's - # resolveEditor() prefers nvim → vim → vi → nano. Override this single - # buffer's resolver so both surfaces pick the same editor. - import shlex - import subprocess - - def _hermes_pick_editor(filename: str) -> bool: - chosen = ( - os.environ.get('VISUAL') - or os.environ.get('EDITOR') - or shutil.which('nvim') - or shutil.which('vim') - or shutil.which('vi') - or shutil.which('nano') - ) - if not chosen: - return False - try: - return subprocess.call(shlex.split(chosen) + [filename]) == 0 - except OSError: - return False - - input_area.buffer._open_file_in_editor = _hermes_pick_editor - # Dynamic height: accounts for both explicit newlines AND visual # wrapping of long lines so the input area always fits its content. def _input_height(): diff --git a/ui-tui/src/lib/editor.test.ts b/ui-tui/src/lib/editor.test.ts index 4262de19886..07a3c4d130f 100644 --- a/ui-tui/src/lib/editor.test.ts +++ b/ui-tui/src/lib/editor.test.ts @@ -33,17 +33,22 @@ describe('resolveEditor', () => { expect(resolveEditor({ EDITOR: 'nvim', PATH: dir })).toBe('nvim') }) - it('prefers nvim over vim over vi over nano on $PATH', () => { + it('prefers system editor over nano over vi on $PATH', () => { exe('nano') exe('vi') - exe('vim') - const nvim = exe('nvim') + const editor = exe('editor') - expect(resolveEditor({ PATH: dir })).toBe(nvim) + expect(resolveEditor({ PATH: dir })).toBe(editor) }) - it('falls back to vi when only vi and nano exist', () => { - exe('nano') + it('falls back to nano when only nano and vi exist', () => { + const nano = exe('nano') + exe('vi') + + expect(resolveEditor({ PATH: dir })).toBe(nano) + }) + + it('falls back to vi when only vi exists', () => { const vi = exe('vi') expect(resolveEditor({ PATH: dir })).toBe(vi) @@ -59,9 +64,9 @@ describe('resolveEditor', () => { const a = mkdtempSync(join(tmpdir(), 'editor-a-')) const b = mkdtempSync(join(tmpdir(), 'editor-b-')) - writeFileSync(join(b, 'vim'), '#!/bin/sh\n') - chmodSync(join(b, 'vim'), 0o755) + writeFileSync(join(b, 'editor'), '#!/bin/sh\n') + chmodSync(join(b, 'editor'), 0o755) - expect(resolveEditor({ PATH: [a, b].join(delimiter) })).toBe(join(b, 'vim')) + expect(resolveEditor({ PATH: [a, b].join(delimiter) })).toBe(join(b, 'editor')) }) }) diff --git a/ui-tui/src/lib/editor.ts b/ui-tui/src/lib/editor.ts index 6898b7cb422..a871eb7e6df 100644 --- a/ui-tui/src/lib/editor.ts +++ b/ui-tui/src/lib/editor.ts @@ -6,33 +6,45 @@ import { delimiter, join } from 'node:path' * * Order of preference: * 1. $VISUAL / $EDITOR (user's explicit choice) - * 2. first executable found on $PATH from `nvim` → `vim` → `vi` → `nano` + * 2. prompt_toolkit-compatible system fallback: + * editor → nano → pico → vi → emacs * 3. literal `'vi'` so spawnSync still has something to try * - * Mirrors the override on `input_area.buffer._open_file_in_editor` in cli.py - * — both surfaces should pick the same editor so the CLI/TUI handoff - * doesn't surprise the user with nano in one and vim in the other. + * This intentionally mirrors prompt_toolkit's Buffer.open_in_editor() picker + * used by the classic CLI. In Cursor/VSCode terminals, nano is a better prompt + * editing default than dropping casual users into vi's modal interface. */ export function resolveEditor(env: NodeJS.ProcessEnv = process.env): string { - return env.VISUAL || env.EDITOR || findExecutable(env.PATH ?? '', 'nvim', 'vim', 'vi', 'nano') || 'vi' + return ( + env.VISUAL || + env.EDITOR || + findEditor(env.PATH ?? '', 'editor', 'nano', 'pico', 'vi', 'emacs') || + 'vi' + ) } -function findExecutable(path: string, ...names: string[]): null | string { +function findEditor(path: string, ...names: string[]): null | string { const dirs = path.split(delimiter).filter(Boolean) for (const name of names) { for (const dir of dirs) { const candidate = join(dir, name) - try { - accessSync(candidate, constants.X_OK) - + if (isExecutable(candidate)) { return candidate - } catch { - // not executable / not present; try next } } } return null } + +function isExecutable(path: string): boolean { + try { + accessSync(path, constants.X_OK) + + return true + } catch { + return false + } +}