From 5b78165e3385d25b9e41e7e8bfb23914b3e4c87f Mon Sep 17 00:00:00 2001 From: Erich Blume Date: Tue, 13 Jan 2026 09:02:48 -0800 Subject: [PATCH] Rework edit command to open plist in $EDITOR Instead of a CLI for editing plist arguments programmatically, the edit command now opens the plist file directly in the user's $EDITOR (falls back to vi). This provides more flexibility for editing any plist field. Co-Authored-By: Claude Sonnet 4.5 --- README.md | 10 ++-- mcquack.py | 34 ++++--------- test_mcquack.py | 125 ++++++++++++++++++++++++------------------------ 3 files changed, 78 insertions(+), 91 deletions(-) diff --git a/README.md b/README.md index da167bc..6a330ae 100644 --- a/README.md +++ b/README.md @@ -37,8 +37,8 @@ mcquack launch /path/to/your/script # Show the current arguments configured in the plist mcquack show /path/to/your/script -# Edit the arguments in the plist (note the -- separator) -mcquack edit /path/to/your/script -- --new-arg1 --new-arg2 +# Edit the plist in $EDITOR (falls back to vi) +mcquack edit /path/to/your/script # Unload (stop) the LaunchAgent mcquack unload /path/to/your/script @@ -49,8 +49,8 @@ mcquack delete /path/to/your/script ### Passing Arguments to Your Script -When passing arguments to your script with `create` or `edit`, you **must** use `--` to -separate mcquack's options from arguments intended for your script: +When passing arguments to your script with `create`, you **must** use `--` to separate +mcquack's options from arguments intended for your script: ```bash # Correct: passes --config and --debug to your script @@ -66,6 +66,8 @@ mcquack create my_script.sh -- --help The `--` separator ensures that flags like `--help`, `--verbose`, etc. are passed to your script rather than being interpreted by mcquack itself. +To modify arguments after creation, use `mcquack edit` to open the plist in your editor. + ## How it works mcquack creates plist files in `~/Library/LaunchAgents/` with the naming convention: diff --git a/mcquack.py b/mcquack.py index ef98e63..634e106 100755 --- a/mcquack.py +++ b/mcquack.py @@ -190,21 +190,11 @@ def show( @app.command() def edit( script: Annotated[Path, typer.Argument(help="Path to the executable script")], - script_args: Annotated[ - list[str] | None, - typer.Argument(help="New arguments for the script (must come after '--')"), - ] = None, ) -> None: - """Edit the arguments in a plist. + """Edit a plist in $EDITOR. - To update script arguments, use '--' to separate mcquack options from - script arguments. For example: - - mcquack edit my_script.sh -- --verbose --debug - - This replaces all existing script arguments. To clear arguments, omit them: - - mcquack edit my_script.sh + Opens the plist file for the given script in your default editor. + After editing, the agent is automatically reloaded. """ script_path = script.resolve() plist_path = get_plist_path(script_path) @@ -213,22 +203,18 @@ def edit( typer.echo(f"Error: {plist_path} does not exist. Use 'create' first.", err=True) raise typer.Exit(1) - with open(plist_path, "rb") as f: - plist = plistlib.load(f) - - program_args = plist.get("ProgramArguments", []) - script_in_plist = program_args[0] if program_args else str(script_path) - - # Update arguments - plist["ProgramArguments"] = [script_in_plist] + (script_args or []) + editor = os.environ.get("EDITOR", "vi") # Unload first subprocess.run(["launchctl", "unload", str(plist_path)], capture_output=True) - with open(plist_path, "wb") as f: - plistlib.dump(plist, f) + # Open in editor + result = subprocess.run([editor, str(plist_path)]) + if result.returncode != 0: + typer.echo(f"Error: editor exited with code {result.returncode}", err=True) + raise typer.Exit(1) - typer.echo(f"Updated: {plist_path}") + typer.echo(f"Edited: {plist_path}") # Reload result = subprocess.run( diff --git a/test_mcquack.py b/test_mcquack.py index a8f7cad..7301094 100644 --- a/test_mcquack.py +++ b/test_mcquack.py @@ -1,6 +1,7 @@ """Tests for mcquack.""" import plistlib +import subprocess import xml.etree.ElementTree as ET from pathlib import Path @@ -102,70 +103,83 @@ class TestCreate: class TestEdit: - def test_edit_add_arguments(self, mock_dirs, mock_script, mock_launchctl): - """Test adding arguments to an existing agent.""" - # Create without arguments + def test_edit_opens_editor(self, mock_dirs, mock_script, mock_launchctl, monkeypatch): + """Test that edit opens the plist in $EDITOR.""" + # Create an agent first runner.invoke(app, ["create", str(mock_script)]) - # Edit to add arguments - result = runner.invoke(app, ["edit", str(mock_script), "--", "--verbose", "--config", "test.json"]) - assert result.exit_code == 0 - assert "Updated:" in result.stdout - assert "Reloaded:" in result.stdout + # Track editor calls + editor_calls = [] - # Verify arguments were added - program_args = read_plist_args(get_plist_path(mock_dirs)) - assert program_args[1:] == ["--verbose", "--config", "test.json"] + def mock_editor_run(args, **kwargs): + editor_calls.append(args) + return subprocess.CompletedProcess(args, 0) - def test_edit_remove_arguments(self, mock_dirs, mock_script, mock_launchctl): - """Test removing arguments from an existing agent.""" - # Create with arguments - runner.invoke(app, ["create", str(mock_script), "--", "--old-arg", "old-value"]) + # Mock subprocess.run to capture editor call + original_run = subprocess.run + + def patched_run(args, **kwargs): + if args and args[0] == "test-editor": + return mock_editor_run(args, **kwargs) + return original_run(args, **kwargs) + + monkeypatch.setattr(subprocess, "run", patched_run) + monkeypatch.setenv("EDITOR", "test-editor") - # Edit with no arguments (clears them) result = runner.invoke(app, ["edit", str(mock_script)]) assert result.exit_code == 0 + assert "Edited:" in result.stdout + assert "Reloaded:" in result.stdout - # Verify arguments were removed - program_args = read_plist_args(get_plist_path(mock_dirs)) - assert len(program_args) == 1 # Only the script path remains + # Verify editor was called with the plist path + assert len(editor_calls) == 1 + assert editor_calls[0][0] == "test-editor" + assert str(get_plist_path(mock_dirs)) in editor_calls[0][1] - def test_edit_replace_arguments(self, mock_dirs, mock_script, mock_launchctl): - """Test replacing arguments on an existing agent.""" - # Create with initial arguments - runner.invoke(app, ["create", str(mock_script), "--", "--old"]) + def test_edit_uses_vi_as_default(self, mock_dirs, mock_script, mock_launchctl, monkeypatch): + """Test that edit falls back to vi if $EDITOR is not set.""" + runner.invoke(app, ["create", str(mock_script)]) - # Edit to replace with new arguments - result = runner.invoke(app, ["edit", str(mock_script), "--", "--new", "value"]) + editor_calls = [] + original_run = subprocess.run + + def patched_run(args, **kwargs): + if args and args[0] == "vi": + editor_calls.append(args) + return subprocess.CompletedProcess(args, 0) + return original_run(args, **kwargs) + + monkeypatch.setattr(subprocess, "run", patched_run) + monkeypatch.delenv("EDITOR", raising=False) + + result = runner.invoke(app, ["edit", str(mock_script)]) assert result.exit_code == 0 - - # Verify arguments were replaced - program_args = read_plist_args(get_plist_path(mock_dirs)) - assert program_args[1:] == ["--new", "value"] + assert len(editor_calls) == 1 + assert editor_calls[0][0] == "vi" def test_edit_nonexistent(self, mock_dirs, mock_script): """Test editing nonexistent agent.""" - result = runner.invoke(app, ["edit", str(mock_script), "--", "--arg"]) + result = runner.invoke(app, ["edit", str(mock_script)]) assert result.exit_code == 1 assert "does not exist" in result.output - @pytest.mark.parametrize("new_args", [ - ["--single"], - ["--multi", "arg1", "arg2"], - [], - ]) - def test_edit_various_arguments(self, mock_dirs, mock_script, mock_launchctl, new_args): - """Test editing with various argument configurations.""" - runner.invoke(app, ["create", str(mock_script), "--", "--initial"]) + def test_edit_editor_failure(self, mock_dirs, mock_script, mock_launchctl, monkeypatch): + """Test that edit fails if editor exits with error.""" + runner.invoke(app, ["create", str(mock_script)]) - cmd = ["edit", str(mock_script)] - if new_args: - cmd += ["--"] + new_args - result = runner.invoke(app, cmd) - assert result.exit_code == 0 + original_run = subprocess.run - program_args = read_plist_args(get_plist_path(mock_dirs)) - assert program_args[1:] == new_args + def patched_run(args, **kwargs): + if args and args[0] == "failing-editor": + return subprocess.CompletedProcess(args, 1) + return original_run(args, **kwargs) + + monkeypatch.setattr(subprocess, "run", patched_run) + monkeypatch.setenv("EDITOR", "failing-editor") + + result = runner.invoke(app, ["edit", str(mock_script)]) + assert result.exit_code == 1 + assert "editor exited with code" in result.output class TestShow: @@ -264,26 +278,11 @@ class TestArgumentSeparator: program_args = read_plist_args(plist_path) assert "--verbose" in program_args - def test_edit_help_without_separator(self, mock_dirs, mock_script, mock_launchctl): + def test_edit_help(self, mock_dirs, mock_script, mock_launchctl): """Verify that edit --help shows edit command help.""" - # First create an agent - runner.invoke(app, ["create", str(mock_script)]) - - result = runner.invoke(app, ["edit", str(mock_script), "--help"]) + result = runner.invoke(app, ["edit", "--help"]) assert result.exit_code == 0 - assert "Edit the arguments" in result.stdout or "Usage:" in result.stdout - - def test_edit_help_with_separator(self, mock_dirs, mock_script, mock_launchctl): - """Verify that edit -- --help passes --help as a new argument.""" - # First create an agent - runner.invoke(app, ["create", str(mock_script)]) - - result = runner.invoke(app, ["edit", str(mock_script), "--", "--help"]) - assert result.exit_code == 0 - - plist_path = get_plist_path(mock_dirs) - program_args = read_plist_args(plist_path) - assert "--help" in program_args + assert "Edit a plist" in result.stdout or "Usage:" in result.stdout def test_create_mixed_args_with_separator(self, mock_dirs, mock_script, mock_launchctl): """Verify multiple dash-prefixed args after -- are all passed to script."""