From b53bd12fe4c2b5518049c61692090fe26a786d30 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 7 May 2026 16:46:37 -0700 Subject: [PATCH] fix(windows-editor): default EDITOR=notepad so /edit and Ctrl+X Ctrl+E work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-existing Windows bug surfaced while reviewing the portable-MinGit install: prompt_toolkit's Buffer.open_in_editor() falls back to POSIX absolute paths (/usr/bin/nano, /usr/bin/vi, /usr/bin/emacs) that don't exist on native Windows. When neither $EDITOR nor $VISUAL is set, Ctrl+X Ctrl+E ("open prompt in editor") and /edit both silently do nothing on Windows — the user hits the key, nothing happens, no error. This wasn't caused by MinGit (full Git for Windows doesn't fix it either, because the Windows Python subprocess call resolves `/usr/bin/nano` as `C:\usr\bin\nano`, which doesn't exist even with nano installed). Fixes: - hermes_cli/stdio.py::configure_windows_stdio now sets EDITOR=notepad on Windows if neither EDITOR nor VISUAL is set. notepad.exe is in every Windows install, works as a blocking editor (subprocess.call waits for the window to close), and writes back to the file. - hermes_cli/config.py (hermes config edit): reorder fallback list so Windows tries notepad first — previously nano led the list, which required Git Bash / WSL to be in PATH. - Users who want VSCode / Neovim / Notepad++ can still override via $env:EDITOR — that's checked before our default kicks in. Docstring spells out the common overrides. The Ink TUI (`hermes --tui`) already handled Windows correctly via ui-tui/src/lib/editor.ts falling back to notepad.exe on win32 — this commit brings the classic prompt_toolkit CLI into parity. 3 new tests in test_windows_native_support.py verify: - EDITOR=notepad gets set when unset on Windows - Explicit $EDITOR is respected - $VISUAL is respected (not overwritten by our default) --- hermes_cli/config.py | 16 ++++++-- hermes_cli/stdio.py | 46 ++++++++++++++++++++++ tests/tools/test_windows_native_support.py | 39 ++++++++++++++++++ 3 files changed, 97 insertions(+), 4 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index cb6753864f..424d394d74 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -4708,11 +4708,19 @@ def edit_config(): # Find editor editor = os.getenv('EDITOR') or os.getenv('VISUAL') - + if not editor: - # Try common editors - for cmd in ['nano', 'vim', 'vi', 'code', 'notepad']: - import shutil + # Try common editors — order is platform-aware so Windows users + # land on a working editor (notepad) even without Git Bash or nano + # installed. On POSIX, prefer nano/vim over code/notepad because + # it's more likely to be present on headless / server systems. + import shutil + import sys as _sys + if _sys.platform == "win32": + candidates = ['notepad', 'code', 'vim', 'vi', 'nano'] + else: + candidates = ['nano', 'vim', 'vi', 'code', 'notepad'] + for cmd in candidates: if shutil.which(cmd): editor = cmd break diff --git a/hermes_cli/stdio.py b/hermes_cli/stdio.py index cfa27e2cab..e010304d98 100644 --- a/hermes_cli/stdio.py +++ b/hermes_cli/stdio.py @@ -92,6 +92,9 @@ def configure_windows_stdio() -> bool: Set ``HERMES_DISABLE_WINDOWS_UTF8=1`` in the environment to opt out (for diagnosing encoding-related bugs by forcing the old cp1252 path). + + Also sets a sensible default ``EDITOR`` on Windows if none is already + set — see :func:`_default_windows_editor`. """ global _CONFIGURED @@ -114,6 +117,16 @@ def configure_windows_stdio() -> bool: # (PEP 540). Again, don't override an explicit setting. os.environ.setdefault("PYTHONUTF8", "1") + # Set EDITOR to a working Windows default if neither EDITOR nor VISUAL + # is set. prompt_toolkit's ``open_in_editor`` falls back to POSIX-only + # paths (``/usr/bin/nano``, ``/usr/bin/vi``) that don't exist on + # Windows — Ctrl+X Ctrl+E and ``/edit`` silently do nothing there + # otherwise. This happens even with full Git for Windows installed, + # so it's not a MinGit-specific issue. + _default_editor = _default_windows_editor() + if _default_editor and not os.environ.get("EDITOR") and not os.environ.get("VISUAL"): + os.environ["EDITOR"] = _default_editor + # Flip the console code page first so that any subprocess that # inherits the console (e.g. a launched shell) also sees CP_UTF8. _flip_console_code_page_to_utf8() @@ -132,3 +145,36 @@ def configure_windows_stdio() -> bool: _CONFIGURED = True return True + + +def _default_windows_editor() -> str: + """Return a Windows-appropriate default for ``$EDITOR``. + + Priority order, first match wins: + + 1. ``notepad`` — ships with every Windows install, no deps, works as a + blocking editor (``subprocess.call(["notepad", file])`` blocks until + the user closes the window). This is the "always-works" default. + + The prompt_toolkit buffer's ``open_in_editor`` and Hermes's + ``hermes config edit`` both honour ``$EDITOR``. Users who prefer a + different editor can override: + + - VSCode: ``$env:EDITOR = "code --wait"`` (``--wait`` is critical; + without it the editor returns immediately and any input is lost) + - Notepad++: ``$env:EDITOR = "'C:\\Program Files\\Notepad++\\notepad++.exe' -multiInst -nosession"`` + - Neovim: ``$env:EDITOR = "nvim"`` (if installed) + + Set this before launching Hermes (User env var in Windows Settings, or + export in a PowerShell profile) and Hermes picks it up automatically. + """ + import shutil + + # notepad.exe is always in %SystemRoot%\System32 on Windows, so shutil.which + # will reliably find it. Return the bare name so prompt_toolkit's shlex + # split doesn't trip over a path containing spaces. + if shutil.which("notepad"): + return "notepad" + # On the extreme off-chance notepad is missing (WinPE, Nano Server), fall + # back to nothing and let prompt_toolkit's silent no-op do its thing. + return "" diff --git a/tests/tools/test_windows_native_support.py b/tests/tools/test_windows_native_support.py index 1503bf1124..fa1a0ed8e3 100644 --- a/tests/tools/test_windows_native_support.py +++ b/tests/tools/test_windows_native_support.py @@ -73,6 +73,8 @@ class TestConfigureWindowsStdio: monkeypatch.delenv("PYTHONIOENCODING", raising=False) monkeypatch.delenv("PYTHONUTF8", raising=False) monkeypatch.delenv("HERMES_DISABLE_WINDOWS_UTF8", raising=False) + monkeypatch.delenv("EDITOR", raising=False) + monkeypatch.delenv("VISUAL", raising=False) reconfigure_calls = [] @@ -86,14 +88,51 @@ class TestConfigureWindowsStdio: monkeypatch.setattr(stdio, "_reconfigure_stream", fake_reconfigure) monkeypatch.setattr(stdio, "_flip_console_code_page_to_utf8", fake_flip) + # Pretend notepad.exe is on PATH (it always is on real Windows hosts, + # but not on the Linux CI runner — mock it so the editor default + # survives). + monkeypatch.setattr(stdio, "_default_windows_editor", lambda: "notepad") result = stdio.configure_windows_stdio() assert result is True assert os.environ.get("PYTHONIOENCODING") == "utf-8" assert os.environ.get("PYTHONUTF8") == "1" + # EDITOR must be set so prompt_toolkit's open_in_editor finds + # a working program on Windows (it defaults to /usr/bin/nano). + assert os.environ.get("EDITOR") == "notepad" assert len(cp_calls) == 1 # SetConsoleOutputCP path hit assert len(reconfigure_calls) == 3 # stdout, stderr, stdin + def test_respects_existing_editor_var(self, monkeypatch): + """User's explicit EDITOR wins over our default.""" + from hermes_cli import stdio + + monkeypatch.setattr(stdio, "is_windows", lambda: True) + monkeypatch.setenv("EDITOR", "code --wait") + monkeypatch.setattr(stdio, "_reconfigure_stream", lambda *a, **kw: None) + monkeypatch.setattr(stdio, "_flip_console_code_page_to_utf8", lambda: None) + monkeypatch.setattr(stdio, "_default_windows_editor", lambda: "notepad") + + stdio.configure_windows_stdio() + assert os.environ["EDITOR"] == "code --wait" + + def test_respects_existing_visual_var(self, monkeypatch): + """VISUAL takes precedence over our EDITOR default too.""" + from hermes_cli import stdio + + monkeypatch.setattr(stdio, "is_windows", lambda: True) + monkeypatch.delenv("EDITOR", raising=False) + monkeypatch.setenv("VISUAL", "nvim") + monkeypatch.setattr(stdio, "_reconfigure_stream", lambda *a, **kw: None) + monkeypatch.setattr(stdio, "_flip_console_code_page_to_utf8", lambda: None) + monkeypatch.setattr(stdio, "_default_windows_editor", lambda: "notepad") + + stdio.configure_windows_stdio() + # EDITOR should NOT be set when VISUAL already is (prompt_toolkit + # checks VISUAL first anyway, but we also shouldn't override it). + assert os.environ.get("EDITOR", "") != "notepad" + assert os.environ["VISUAL"] == "nvim" + def test_respects_existing_env_var(self, monkeypatch): """User's explicit PYTHONIOENCODING wins over our default.""" from hermes_cli import stdio