diff --git a/CLAUDE.md b/CLAUDE.md index 4a2e67c..2647817 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -98,10 +98,10 @@ Shared interactive I/O: `ask` / `choose` / `form` / `confirm` with pluggable bac Click app + readline REPL. Entry: `plyngent` / `python -m plyngent`. - **`plyngent chat`**: provider/model (flags or interactive; Tab via readline in `prompting`); sessions store `provider_name`/`model` and restore on resume; SQLite via `[database]` (file DB under user data when url unset; explicit `url = ":memory:"` kept + warn); workspace-bound; resume latest for cwd/`--workspace` by default (`--new` / `--session`). One-shot: `-p/--prompt` and non-TTY stdin; exit codes 0/1/2/3; `--yes` (YOLO on), `--stream/--no-stream`, `--quiet`. Root `--log-level`. -- Slash: Click group in `cli/slash.py` + `awaitlet` for async work; Tab completer from registry + ParamType `shell_complete`. Multiline `"""` … `"""`; `/edit` via `$EDITOR`. `/yolo on|off|once` for soft destructive confirms. `/model --persist` / `/models --persist` write model catalog entries into TOML. `/todos` for human show/push/pop/clear of the todo stack. +- Slash: Click group in `cli/slash.py` + `awaitlet` for async work; Tab completer from registry + ParamType `shell_complete`. Multiline `"""` … `"""`; `/edit` via `$VISUAL`/`$EDITOR` (blocking only). `/yolo on|off|once` for soft destructive confirms. `/model --persist` / `/models --persist` write model catalog entries into TOML. `/todos` for human show/push/pop/clear of the todo stack. - Explicit `/resume` or `--session` from another workspace prompts: **keep** / **update** / **abort**. - Failed/cancelled turns: user kept; **committed tool rounds kept** (side effects not re-run on `/retry`); only unfinished assistant rolled back; Ctrl+C cancels; TTY confirms off-loop; auto-retry 10s/20s/30s then `/retry`. -- **`plyngent providers`**, **`config path|edit`**. No providers + `$EDITOR` → optional edit then reload. +- **`plyngent providers`**, **`config path|edit`**. Config open: `$VISUAL`/`$EDITOR` (wait), else system default (`xdg-open` / `open` / `startfile`, non-blocking). `/edit` stays blocking-only. No providers → optional edit then reload when waited. - Tools default on; workspace defaults to cwd; `--max-rounds` default 32. Readline history under platformdirs (`repl_history`). PTY: `close_all` on chat exit. ### Composition utility: `Forward` descriptor diff --git a/README.md b/README.md index 5e26bfa..6a74c2a 100644 --- a/README.md +++ b/README.md @@ -86,7 +86,7 @@ Config: `prek.toml`. CI still runs the same checks in GitHub Actions. ```bash # 1) Create / open config plyngent config path -plyngent config edit # needs $EDITOR +plyngent config edit # $VISUAL/$EDITOR, else system open (xdg-open/open/startfile) # Minimal provider (OpenAI platform — Responses API; preset defaults to openai): # [providers.oai] @@ -111,7 +111,7 @@ Default config path (platformdirs): ```bash plyngent config path -plyngent config edit # opens $EDITOR (e.g. codium --wait) +plyngent config edit # $VISUAL/$EDITOR (e.g. codium --wait), else system default ``` Copy the example and fill in a real token: @@ -186,7 +186,7 @@ Exit codes (one-shot): ### Input ergonomics - **Multiline**: start a message with `"""`, end a later line with `"""`. -- **`/edit`**: compose a turn in `$EDITOR` (empty buffer cancels). +- **`/edit`**: compose a turn in `$VISUAL`/`$EDITOR` (blocking only; empty cancels). - **Tab**: completes slash commands and some arguments (provider, model, on/off, export, `/help` targets). ### Slash commands @@ -211,7 +211,7 @@ Type `/help` in the REPL for the live list. Common ones: | `/models` | List config + remote `GET /models` (always re-fetches) | | `/models --persist` | Merge remote catalog into TOML for this provider | | `/todos` | Todo/task stack: list, push, pop, done, clear | -| `/config` | Edit `plyngent.toml` in `$EDITOR` and reload | +| `/config` | Edit `plyngent.toml` ($VISUAL/$EDITOR or system open); reload after blocking editor | | `/quit` | Leave the REPL | User messages are saved immediately. On API error or Ctrl+C, partial assistant/tool output is discarded but the user message stays so `/retry` works after resume. Interactive auto-retry uses 10s / 20s / 30s delays. diff --git a/src/plyngent/cli/app.py b/src/plyngent/cli/app.py index 5ed3a3b..dfdd31b 100644 --- a/src/plyngent/cli/app.py +++ b/src/plyngent/cli/app.py @@ -525,10 +525,21 @@ def config_path_cmd(config_path: Path | None) -> None: help="Path to plyngent.toml (default: platform config dir).", ) def config_edit_cmd(config_path: Path | None) -> None: - """Open the config file in $EDITOR (supports e.g. ``codium --wait``).""" + """Open the config file in $VISUAL/$EDITOR, or system default if unset. + + Blocking editors (e.g. ``codium --wait``) wait for exit. Without VISUAL/EDITOR, + falls back to xdg-open / open / startfile (non-blocking). + """ path = resolve_config_path(config_path) - open_in_editor(path) - click.echo(f"edited {path}") + outcome = open_in_editor(path, allow_system_open=True) + if outcome == "system": + click.secho( + f"opened {path} with system default (not waiting for the app to exit)", + fg="yellow", + err=True, + ) + else: + click.echo(f"edited {path}") if __name__ == "__main__": diff --git a/src/plyngent/cli/editor.py b/src/plyngent/cli/editor.py index 0ea9744..04ccfd0 100644 --- a/src/plyngent/cli/editor.py +++ b/src/plyngent/cli/editor.py @@ -4,8 +4,9 @@ import contextlib import os import shlex import subprocess +import sys from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Literal import click @@ -14,6 +15,8 @@ from plyngent import config as config_mod if TYPE_CHECKING: from plyngent.config.store import ConfigStore +type OpenOutcome = Literal["waited", "system"] + _MINIMAL_CONFIG = """\ # plyngent configuration # edit providers below @@ -54,9 +57,16 @@ _MINIMAL_CONFIG = """\ def get_editor() -> str | None: - """Return the ``EDITOR`` environment value, or ``None`` if unset/empty.""" - value = os.environ.get("EDITOR", "").strip() - return value or None + """Return ``$VISUAL`` or ``$EDITOR``, or ``None`` if both unset/empty. + + ``VISUAL`` is preferred when set (common Unix convention for full-screen + editors); otherwise ``EDITOR``. + """ + for key in ("VISUAL", "EDITOR"): + value = os.environ.get(key, "").strip() + if value: + return value + return None def resolve_config_path(config_path: Path | None) -> Path: @@ -73,31 +83,14 @@ def ensure_config_file(path: Path) -> None: _ = path.write_text(_MINIMAL_CONFIG, encoding="utf-8") -def open_in_editor( - path: Path, - *, - editor: str | None = None, - ensure_exists: bool = True, -) -> None: - """Open ``path`` with ``EDITOR`` (supports values like ``codium --wait``). - - When ``ensure_exists`` is true (default), create a minimal config template - if the file is missing (used for ``plyngent config edit``). - """ - editor_cmd = editor if editor is not None else get_editor() - if editor_cmd is None: - msg = "EDITOR is not set" - raise click.ClickException(msg) - - if ensure_exists: - ensure_config_file(path) +def _run_blocking_editor(editor_cmd: str, path: Path) -> None: try: argv = [*shlex.split(editor_cmd, posix=os.name != "nt"), str(path)] except ValueError as exc: - msg = f"invalid EDITOR value {editor_cmd!r}: {exc}" + msg = f"invalid editor value {editor_cmd!r}: {exc}" raise click.ClickException(msg) from exc if not argv: - msg = "EDITOR is empty after parsing" + msg = "editor command is empty after parsing" raise click.ClickException(msg) try: @@ -114,15 +107,97 @@ def open_in_editor( raise click.ClickException(msg) -def edit_text_in_editor(initial: str = "", *, suffix: str = ".md") -> str | None: - """Edit ``initial`` in ``$EDITOR``; return text or ``None`` if empty/cancelled. +def _open_with_system_default(path: Path) -> None: + """Open *path* with the OS file association (non-blocking). - Uses a temporary file. Does not create a config template. + Linux: ``xdg-open``; macOS: ``open``; Windows: ``os.startfile``. + Does not wait for the application to exit. + """ + resolved = str(path.resolve()) + if sys.platform == "win32": + try: + os.startfile(resolved) # type: ignore[attr-defined] + except OSError as exc: + msg = f"failed to open with system default: {exc}" + raise click.ClickException(msg) from exc + return + + if sys.platform == "darwin": + argv = ["open", resolved] + else: + # Linux and other POSIX: Free Desktop opener + argv = ["xdg-open", resolved] + + try: + completed = subprocess.run( + argv, + check=False, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + except FileNotFoundError as exc: + cmd = argv[0] + msg = ( + f"{cmd} not found and neither VISUAL nor EDITOR is set; " + "set VISUAL or EDITOR to a blocking editor (e.g. nano, vim, codium --wait)" + ) + raise click.ClickException(msg) from exc + except OSError as exc: + msg = f"failed to open with system default: {exc}" + raise click.ClickException(msg) from exc + + if completed.returncode != 0: + msg = f"system open failed (exit {completed.returncode}); set VISUAL or EDITOR to a blocking editor" + raise click.ClickException(msg) + + +def open_in_editor( + path: Path, + *, + editor: str | None = None, + ensure_exists: bool = True, + allow_system_open: bool = True, +) -> OpenOutcome: + """Open ``path`` for editing. + + Prefer a blocking editor (``editor`` arg, else ``$VISUAL`` / ``$EDITOR``). + When none is set and ``allow_system_open`` is true, fall back to the OS + default association (``xdg-open`` / ``open`` / ``os.startfile``) — this + does **not** wait for the app to exit. + + Returns: + ``"waited"`` if a blocking editor ran to completion; + ``"system"`` if the OS opener was used (non-blocking). + + When ``ensure_exists`` is true (default), create a minimal config template + if the file is missing (used for ``plyngent config edit``). + """ + if ensure_exists: + ensure_config_file(path) + + editor_cmd = editor if editor is not None else get_editor() + if editor_cmd is not None: + _run_blocking_editor(editor_cmd, path) + return "waited" + + if not allow_system_open: + msg = "neither VISUAL nor EDITOR is set" + raise click.ClickException(msg) + + _open_with_system_default(path) + return "system" + + +def edit_text_in_editor(initial: str = "", *, suffix: str = ".md") -> str | None: + """Edit ``initial`` in a blocking editor; return text or ``None`` if empty. + + Uses a temporary file. Requires ``$VISUAL`` or ``$EDITOR`` (no system-open + fallback — we must wait for the process and re-read the buffer). """ import tempfile if get_editor() is None: - msg = "EDITOR is not set; cannot /edit" + msg = "neither VISUAL nor EDITOR is set; cannot /edit" raise click.ClickException(msg) with tempfile.NamedTemporaryFile( @@ -136,7 +211,7 @@ def edit_text_in_editor(initial: str = "", *, suffix: str = ".md") -> str | None _ = handle.write(initial) try: - open_in_editor(path, ensure_exists=False) + _ = open_in_editor(path, ensure_exists=False, allow_system_open=False) text = path.read_text(encoding="utf-8") finally: with contextlib.suppress(OSError): @@ -148,19 +223,24 @@ def edit_text_in_editor(initial: str = "", *, suffix: str = ".md") -> str | None return cleaned -def prompt_edit_config(path: Path, *, reason: str | None = None) -> bool: - """If ``EDITOR`` is set, ask whether to edit ``path``. Returns True if opened.""" - if get_editor() is None: - return False +def prompt_edit_config(path: Path, *, reason: str | None = None) -> OpenOutcome | None: + """Ask whether to edit ``path``. Returns open outcome, or ``None`` if skipped. + + Offers when a blocking editor is set **or** system open can be attempted. + """ + has_editor = get_editor() is not None + # System open is always attempted as fallback when no editor; we still + # prompt so the user can decline on headless hosts. message = f"{reason} Edit config file {path}?" if reason else f"Edit config file {path}?" + if not has_editor: + message = f"{message} (no VISUAL/EDITOR; will try system default open — non-blocking)" if not click.confirm(message, default=False): - return False - open_in_editor(path) - return True + return None + return open_in_editor(path, allow_system_open=True) def load_config_with_optional_edit(config_path: Path | None) -> ConfigStore: - """Load config; if there are no providers and EDITOR is set, offer to edit and reload. + """Load config; if there are no providers, offer to edit and reload when waited. Raises: config_mod.ConfigFormatError: Invalid TOML (caller should surface path). @@ -172,6 +252,13 @@ def load_config_with_optional_edit(config_path: Path | None) -> ConfigStore: reason = "No providers configured." if not path.exists(): reason = f"Config file not found ({path})." - if prompt_edit_config(path, reason=reason): + outcome = prompt_edit_config(path, reason=reason) + if outcome == "waited": store = config_mod.load(path) + elif outcome == "system": + click.secho( + f"opened {path} with system default (not waiting). Save the file, then re-run plyngent to load providers.", + fg="yellow", + err=True, + ) return store diff --git a/src/plyngent/cli/slash.py b/src/plyngent/cli/slash.py index 7f870dc..b9a1a05 100644 --- a/src/plyngent/cli/slash.py +++ b/src/plyngent/cli/slash.py @@ -87,7 +87,7 @@ HELP_FOOTER = ( "continue a prior chat after restart.\n" "\n" 'Multiline: start a message with """ then end a later line with """.\n' - "Long prompts: /edit opens $EDITOR.\n" + "Long prompts: /edit opens $VISUAL/$EDITOR (blocking).\n" ) @@ -453,10 +453,10 @@ def clear_cmd(state: ReplState) -> None: @slash.command("edit") @click.pass_obj def edit_cmd(state: ReplState) -> None: - """Compose a user message in ``$EDITOR``, then send it. + """Compose a user message in ``$VISUAL``/``$EDITOR``, then send it. Opens a temporary buffer; save and quit the editor to submit. - Empty buffer cancels. Requires ``EDITOR`` (e.g. ``codium --wait``). + Empty buffer cancels. Requires a blocking editor (no system-open fallback). """ from plyngent.cli.editor import edit_text_in_editor @@ -475,20 +475,27 @@ def edit_cmd(state: ReplState) -> None: @slash.command("config") @click.pass_obj def config_cmd(state: ReplState) -> None: - """Open plyngent.toml in ``$EDITOR``, then reload providers/agent settings. + """Open plyngent.toml in ``$VISUAL``/``$EDITOR`` (or system default), then reload. - Same file as ``plyngent config edit``. After the editor exits, config is - re-read; current provider/model are kept when still valid. + Same file as ``plyngent config edit``. After a **blocking** editor exits, + config is re-read. System-open fallback does not wait — reload is skipped + with a hint to re-run ``/config`` after saving. """ from plyngent import config as config_mod from plyngent.cli.editor import open_in_editor path = state.config.path try: - open_in_editor(path) + outcome = open_in_editor(path, allow_system_open=True) except click.ClickException as exc: click.echo(f"error: {exc}") return + if outcome == "system": + click.secho( + f"opened {path} with system default (not waiting). Save the file, then run /config again to reload.", + fg="yellow", + ) + return try: state.reload_config_from_disk() except (config_mod.ConfigFormatError, ValueError, OSError) as exc: diff --git a/tests/test_cli/test_editor.py b/tests/test_cli/test_editor.py index 8555355..ba81723 100644 --- a/tests/test_cli/test_editor.py +++ b/tests/test_cli/test_editor.py @@ -15,10 +15,13 @@ from plyngent.cli.editor import ( ) -def test_get_editor(monkeypatch: pytest.MonkeyPatch) -> None: +def test_get_editor_prefers_visual(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.delenv("EDITOR", raising=False) + monkeypatch.delenv("VISUAL", raising=False) assert get_editor() is None - monkeypatch.setenv("EDITOR", " codium --wait ") + monkeypatch.setenv("EDITOR", " nano ") + assert get_editor() == "nano" + monkeypatch.setenv("VISUAL", " codium --wait ") assert get_editor() == "codium --wait" @@ -49,7 +52,8 @@ def test_open_in_editor_splits_args(tmp_path: Path, monkeypatch: pytest.MonkeyPa return Result() monkeypatch.setattr("plyngent.cli.editor.subprocess.run", fake_run) - open_in_editor(path, editor="codium --wait") + outcome = open_in_editor(path, editor="codium --wait") + assert outcome == "waited" assert len(calls) == 1 assert calls[0][0] == "codium" assert calls[0][1] == "--wait" @@ -57,10 +61,33 @@ def test_open_in_editor_splits_args(tmp_path: Path, monkeypatch: pytest.MonkeyPa assert path.is_file() -def test_open_in_editor_missing_editor(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: +def test_open_in_editor_system_fallback( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: monkeypatch.delenv("EDITOR", raising=False) - with pytest.raises(Exception, match="EDITOR"): - open_in_editor(tmp_path / "x.toml", editor=None) + monkeypatch.delenv("VISUAL", raising=False) + path = tmp_path / "x.toml" + opened: list[str] = [] + + def fake_system(p: Path) -> None: + opened.append(str(p)) + + monkeypatch.setattr("plyngent.cli.editor._open_with_system_default", fake_system) + outcome = open_in_editor(path, editor=None, allow_system_open=True) + assert outcome == "system" + assert opened == [str(path)] + assert path.is_file() + + +def test_open_in_editor_no_fallback_requires_editor( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.delenv("EDITOR", raising=False) + monkeypatch.delenv("VISUAL", raising=False) + with pytest.raises(Exception, match=r"VISUAL|EDITOR"): + open_in_editor(tmp_path / "x.toml", editor=None, allow_system_open=False) def test_edit_text_in_editor(monkeypatch: pytest.MonkeyPatch) -> None: @@ -99,9 +126,13 @@ def test_edit_text_empty_cancels(monkeypatch: pytest.MonkeyPatch) -> None: assert edit_text_in_editor("") is None -def test_prompt_edit_no_editor(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: +def test_edit_text_no_system_fallback(monkeypatch: pytest.MonkeyPatch) -> None: + from plyngent.cli.editor import edit_text_in_editor + monkeypatch.delenv("EDITOR", raising=False) - assert prompt_edit_config(tmp_path / "p.toml") is False + monkeypatch.delenv("VISUAL", raising=False) + with pytest.raises(Exception, match=r"VISUAL|EDITOR"): + edit_text_in_editor("x") def test_prompt_edit_declined(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: @@ -111,7 +142,7 @@ def test_prompt_edit_declined(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) - return False monkeypatch.setattr("click.confirm", _confirm) - assert prompt_edit_config(tmp_path / "p.toml", reason="empty") is False + assert prompt_edit_config(tmp_path / "p.toml", reason="empty") is None def test_prompt_edit_accepted(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: @@ -124,15 +155,40 @@ def test_prompt_edit_accepted(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) - monkeypatch.setattr("click.confirm", _confirm) opened: list[Path] = [] - def fake_open(p: Path, *, editor: str | None = None) -> None: - del editor + def fake_open( + p: Path, + *, + editor: str | None = None, + ensure_exists: bool = True, + allow_system_open: bool = True, + ) -> str: + del editor, ensure_exists, allow_system_open opened.append(p) + return "waited" monkeypatch.setattr("plyngent.cli.editor.open_in_editor", fake_open) - assert prompt_edit_config(path, reason="No providers.") is True + assert prompt_edit_config(path, reason="No providers.") == "waited" assert opened == [path] +def test_prompt_edit_without_editor_can_system_open( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.delenv("EDITOR", raising=False) + monkeypatch.delenv("VISUAL", raising=False) + + def _confirm(*_a: object, **_k: object) -> bool: + return True + + monkeypatch.setattr("click.confirm", _confirm) + monkeypatch.setattr( + "plyngent.cli.editor.open_in_editor", + lambda *a, **k: "system", + ) + assert prompt_edit_config(tmp_path / "p.toml") == "system" + + def test_config_path_command(tmp_path: Path) -> None: runner = CliRunner() result = runner.invoke(main, ["config", "path", "--config", str(tmp_path / "a.toml")]) @@ -159,3 +215,21 @@ def test_config_edit_command(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> result = runner.invoke(main, ["config", "edit", "--config", str(path)]) assert result.exit_code == 0 assert calls and calls[0][-1] == str(path) + assert "edited" in result.output + + +def test_config_edit_system_fallback_message( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + path = tmp_path / "edit.toml" + monkeypatch.delenv("EDITOR", raising=False) + monkeypatch.delenv("VISUAL", raising=False) + monkeypatch.setattr( + "plyngent.cli.editor._open_with_system_default", + lambda p: None, + ) + runner = CliRunner() + result = runner.invoke(main, ["config", "edit", "--config", str(path)]) + assert result.exit_code == 0 + assert "system default" in result.output.lower() or "system default" in (result.stderr or "").lower()