mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-13 03:52:00 +00:00
fix(windows-editor): default EDITOR=notepad so /edit and Ctrl+X Ctrl+E work
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)
This commit is contained in:
parent
5486ad2f2a
commit
1da89528e7
3 changed files with 97 additions and 4 deletions
|
|
@ -4698,9 +4698,17 @@ def edit_config():
|
||||||
editor = os.getenv('EDITOR') or os.getenv('VISUAL')
|
editor = os.getenv('EDITOR') or os.getenv('VISUAL')
|
||||||
|
|
||||||
if not editor:
|
if not editor:
|
||||||
# Try common editors
|
# Try common editors — order is platform-aware so Windows users
|
||||||
for cmd in ['nano', 'vim', 'vi', 'code', 'notepad']:
|
# land on a working editor (notepad) even without Git Bash or nano
|
||||||
import shutil
|
# 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):
|
if shutil.which(cmd):
|
||||||
editor = cmd
|
editor = cmd
|
||||||
break
|
break
|
||||||
|
|
|
||||||
|
|
@ -92,6 +92,9 @@ def configure_windows_stdio() -> bool:
|
||||||
|
|
||||||
Set ``HERMES_DISABLE_WINDOWS_UTF8=1`` in the environment to opt out
|
Set ``HERMES_DISABLE_WINDOWS_UTF8=1`` in the environment to opt out
|
||||||
(for diagnosing encoding-related bugs by forcing the old cp1252 path).
|
(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
|
global _CONFIGURED
|
||||||
|
|
||||||
|
|
@ -114,6 +117,16 @@ def configure_windows_stdio() -> bool:
|
||||||
# (PEP 540). Again, don't override an explicit setting.
|
# (PEP 540). Again, don't override an explicit setting.
|
||||||
os.environ.setdefault("PYTHONUTF8", "1")
|
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
|
# Flip the console code page first so that any subprocess that
|
||||||
# inherits the console (e.g. a launched shell) also sees CP_UTF8.
|
# inherits the console (e.g. a launched shell) also sees CP_UTF8.
|
||||||
_flip_console_code_page_to_utf8()
|
_flip_console_code_page_to_utf8()
|
||||||
|
|
@ -132,3 +145,36 @@ def configure_windows_stdio() -> bool:
|
||||||
|
|
||||||
_CONFIGURED = True
|
_CONFIGURED = True
|
||||||
return 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 ""
|
||||||
|
|
|
||||||
|
|
@ -73,6 +73,8 @@ class TestConfigureWindowsStdio:
|
||||||
monkeypatch.delenv("PYTHONIOENCODING", raising=False)
|
monkeypatch.delenv("PYTHONIOENCODING", raising=False)
|
||||||
monkeypatch.delenv("PYTHONUTF8", raising=False)
|
monkeypatch.delenv("PYTHONUTF8", raising=False)
|
||||||
monkeypatch.delenv("HERMES_DISABLE_WINDOWS_UTF8", raising=False)
|
monkeypatch.delenv("HERMES_DISABLE_WINDOWS_UTF8", raising=False)
|
||||||
|
monkeypatch.delenv("EDITOR", raising=False)
|
||||||
|
monkeypatch.delenv("VISUAL", raising=False)
|
||||||
|
|
||||||
reconfigure_calls = []
|
reconfigure_calls = []
|
||||||
|
|
||||||
|
|
@ -86,14 +88,51 @@ class TestConfigureWindowsStdio:
|
||||||
|
|
||||||
monkeypatch.setattr(stdio, "_reconfigure_stream", fake_reconfigure)
|
monkeypatch.setattr(stdio, "_reconfigure_stream", fake_reconfigure)
|
||||||
monkeypatch.setattr(stdio, "_flip_console_code_page_to_utf8", fake_flip)
|
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()
|
result = stdio.configure_windows_stdio()
|
||||||
assert result is True
|
assert result is True
|
||||||
assert os.environ.get("PYTHONIOENCODING") == "utf-8"
|
assert os.environ.get("PYTHONIOENCODING") == "utf-8"
|
||||||
assert os.environ.get("PYTHONUTF8") == "1"
|
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(cp_calls) == 1 # SetConsoleOutputCP path hit
|
||||||
assert len(reconfigure_calls) == 3 # stdout, stderr, stdin
|
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):
|
def test_respects_existing_env_var(self, monkeypatch):
|
||||||
"""User's explicit PYTHONIOENCODING wins over our default."""
|
"""User's explicit PYTHONIOENCODING wins over our default."""
|
||||||
from hermes_cli import stdio
|
from hermes_cli import stdio
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue