refactor(tui): tighten editor handoff helpers

- editor.ts: collapse two private helpers into one flatMap-driven lookup,
  keep `isExecutable` as the only named primitive, document the fallback
  chain with prompt_toolkit parity
- editor.test.ts: hoist the `exe` helper out of `describe`, drop the
  empty afterEach + dead mkdir branch, materialize expected paths before
  the resolveEditor call so argument evaluation order doesn't bite
- useComposerState.openEditor: rmSync the mkdtemp dir (was leaking),
  early-return on bad exit / empty buffer, run cleanup in finally
- useInputHandlers: cheap `ch.toLowerCase() === 'g'` guard before the
  modifier check
- hermes-ink/screen.ts: pick up `npm run fix` import-sort cleanup so
  lint passes
This commit is contained in:
Brooklyn Nicholson 2026-04-25 20:24:06 -05:00
parent 7fd8dc0bfb
commit 83129e72de
5 changed files with 74 additions and 91 deletions

View file

@ -1,30 +1,27 @@
import { chmodSync, mkdirSync, mkdtempSync, writeFileSync } from 'node:fs'
import { chmodSync, mkdtempSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { delimiter, join } from 'node:path'
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
import { beforeEach, describe, expect, it } from 'vitest'
import { resolveEditor } from './editor.js'
const exe = (dir: string, name: string): string => {
const path = join(dir, name)
writeFileSync(path, '#!/bin/sh\nexit 0\n')
chmodSync(path, 0o755)
return path
}
describe('resolveEditor', () => {
let dir: string
const exe = (name: string) => {
const path = join(dir, name)
writeFileSync(path, '#!/bin/sh\nexit 0\n')
chmodSync(path, 0o755)
return path
}
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), 'editor-test-'))
})
afterEach(() => {
// tmp dir is small; let the OS reap it
})
it('honors $VISUAL above all else', () => {
expect(resolveEditor({ EDITOR: 'vim', PATH: dir, VISUAL: 'helix' })).toBe('helix')
})
@ -33,40 +30,30 @@ describe('resolveEditor', () => {
expect(resolveEditor({ EDITOR: 'nvim', PATH: dir })).toBe('nvim')
})
it('prefers system editor over nano over vi on $PATH', () => {
exe('nano')
exe('vi')
const editor = exe('editor')
it('prefers `editor` over nano over vi on $PATH', () => {
exe(dir, 'nano')
exe(dir, 'vi')
const expected = exe(dir, 'editor')
expect(resolveEditor({ PATH: dir })).toBe(editor)
expect(resolveEditor({ PATH: dir })).toBe(expected)
})
it('falls back to nano when only nano and vi exist', () => {
const nano = exe('nano')
exe('vi')
it('falls back to nano before vi when both exist', () => {
exe(dir, 'vi')
const expected = exe(dir, 'nano')
expect(resolveEditor({ PATH: dir })).toBe(nano)
expect(resolveEditor({ PATH: dir })).toBe(expected)
})
it('falls back to vi when only vi exists', () => {
const vi = exe('vi')
expect(resolveEditor({ PATH: dir })).toBe(vi)
it('returns literal "vi" when $PATH is empty', () => {
expect(resolveEditor({ PATH: '' })).toBe('vi')
})
it('returns literal "vi" when nothing on PATH and no env', () => {
mkdirSync(join(dir, 'empty'), { recursive: true })
expect(resolveEditor({ PATH: join(dir, 'empty') })).toBe('vi')
})
it('walks multi-entry PATH', () => {
it('walks multi-entry $PATH', () => {
const a = mkdtempSync(join(tmpdir(), 'editor-a-'))
const b = mkdtempSync(join(tmpdir(), 'editor-b-'))
const expected = exe(b, 'editor')
writeFileSync(join(b, 'editor'), '#!/bin/sh\n')
chmodSync(join(b, 'editor'), 0o755)
expect(resolveEditor({ PATH: [a, b].join(delimiter) })).toBe(join(b, 'editor'))
expect(resolveEditor({ PATH: [a, b].join(delimiter) })).toBe(expected)
})
})

View file

@ -2,44 +2,13 @@ import { accessSync, constants } from 'node:fs'
import { delimiter, join } from 'node:path'
/**
* Resolve which editor to launch when the user hits Ctrl+G / Alt+G.
*
* Order of preference:
* 1. $VISUAL / $EDITOR (user's explicit choice)
* 2. prompt_toolkit-compatible system fallback:
* editor nano pico vi emacs
* 3. literal `'vi'` so spawnSync still has something to try
*
* 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.
* Editor fallback chain when neither $VISUAL nor $EDITOR is set. Mirrors
* prompt_toolkit's `Buffer.open_in_editor()` picker so the classic CLI and
* the TUI launch the same editor on a given box.
*/
export function resolveEditor(env: NodeJS.ProcessEnv = process.env): string {
return (
env.VISUAL ||
env.EDITOR ||
findEditor(env.PATH ?? '', 'editor', 'nano', 'pico', 'vi', 'emacs') ||
'vi'
)
}
const FALLBACKS = ['editor', 'nano', 'pico', 'vi', 'emacs']
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)
if (isExecutable(candidate)) {
return candidate
}
}
}
return null
}
function isExecutable(path: string): boolean {
const isExecutable = (path: string): boolean => {
try {
accessSync(path, constants.X_OK)
@ -48,3 +17,24 @@ function isExecutable(path: string): boolean {
return false
}
}
/**
* Resolve the editor to launch when the user hits Ctrl+G / Alt+G.
*
* 1. $VISUAL / $EDITOR (user's explicit choice)
* 2. first FALLBACKS entry resolvable on $PATH
* 3. literal `'vi'` so spawnSync still has something to try
*/
export const resolveEditor = (env: NodeJS.ProcessEnv = process.env): string => {
if (env.VISUAL) {
return env.VISUAL
}
if (env.EDITOR) {
return env.EDITOR
}
const dirs = (env.PATH ?? '').split(delimiter).filter(Boolean)
return FALLBACKS.flatMap(name => dirs.map(d => join(d, name))).find(isExecutable) ?? 'vi'
}