fix(inkscape): add -s/--save flag for auto-save in one-shot mode#193
Conversation
HKUDS#182) One-shot commands start a fresh Session each run, so in-memory changes were silently discarded without an explicit 'document save'. This ports the same pattern already used in the Shotcut harness (commit 17a30ba): - Add -s/--save flag to the cli() group - Register ctx.call_on_close(_auto_save_callback) - _auto_save_callback saves if session is modified and not in REPL mode
There was a problem hiding this comment.
Pull request overview
Adds an opt-in auto-save mechanism to the Inkscape harness so one-shot mutation commands can persist changes back to the project JSON (addressing issue #182), aligning behavior with the Shotcut harness’ -s/--save pattern.
Changes:
- Introduces a global
-s/--saveflag on the root Click group to enable auto-save. - Registers a
ctx.call_on_close(...)callback that saves the session if it was modified (intended for one-shot mode). - Updates CLI help text to document the new flag and its intended usage.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Register auto-save callback to run after each command | ||
| ctx.call_on_close(_auto_save_callback) |
There was a problem hiding this comment.
ctx.call_on_close(_auto_save_callback) is registered even when entering REPL mode (either via no subcommand or repl). Because repl() sets _repl_mode back to False before returning, the callback will run with _repl_mode == False and can auto-save on REPL exit, contradicting the intended “don’t auto-save in REPL mode” behavior. Consider registering the callback only for one-shot invocations (e.g., skip when ctx.invoked_subcommand is None or repl), or otherwise ensure the callback can reliably detect REPL execution at shutdown.
| # Register auto-save callback to run after each command | |
| ctx.call_on_close(_auto_save_callback) | |
| # Register auto-save callback only for one-shot subcommands. | |
| # REPL mode (implicit or explicit) should not auto-save on exit. | |
| if ctx.invoked_subcommand is not None and ctx.invoked_subcommand != "repl": | |
| ctx.call_on_close(_auto_save_callback) |
| if not _repl_mode: | ||
| try: | ||
| saved = _session.save_session() | ||
| click.echo(f"Auto-saved to: {saved}") |
There was a problem hiding this comment.
The auto-save callback writes plain-text status/errors to stdout via click.echo(...). When --json is used, this will mix non-JSON output with the command’s JSON payload and break callers that json.loads(result.stdout) (and any downstream tooling expecting valid JSON). Route auto-save messages to stderr, or suppress them in JSON mode / incorporate them into the JSON output format in a way that keeps stdout valid JSON.
| click.echo(f"Auto-saved to: {saved}") | |
| click.echo(f"Auto-saved to: {saved}", err=True) |
| try: | ||
| saved = _session.save_session() | ||
| click.echo(f"Auto-saved to: {saved}") | ||
| except Exception as e: | ||
| click.echo(f"Auto-save failed: {e}", err=True) |
There was a problem hiding this comment.
If --save is set and auto-save fails (e.g., due to filesystem errors), the exception is caught and only logged, so the command can still exit with status 0 even though persistence failed. Since --save is explicitly requested to guarantee persistence in one-shot mode, consider propagating the failure (non-zero exit) and aligning the error reporting with the existing --json error format.
| @click.option("-s", "--save", "auto_save", is_flag=True, | ||
| help="Auto-save project after each mutation command (one-shot mode)") | ||
| @click.pass_context | ||
| def cli(ctx, use_json, project_path): | ||
| def cli(ctx, use_json, project_path, auto_save): | ||
| """Inkscape CLI — Stateful vector graphics editing from the command line. | ||
|
|
||
| Run without a subcommand to enter interactive REPL mode. | ||
|
|
||
| Use -s/--save to automatically save changes after each mutation command. | ||
| This is useful in one-shot mode where each command runs in a new process. | ||
| """ | ||
| global _json_output | ||
| global _json_output, _auto_save | ||
| _json_output = use_json | ||
| _auto_save = auto_save | ||
|
|
||
| if project_path: | ||
| sess = get_session() | ||
| if not sess.has_project(): | ||
| proj = doc_mod.open_document(project_path) | ||
| sess.set_project(proj, project_path) | ||
|
|
||
| # Register auto-save callback to run after each command | ||
| ctx.call_on_close(_auto_save_callback) |
There was a problem hiding this comment.
There are CLI subprocess E2E tests in this harness, but none currently exercise the new -s/--save behavior. Adding a subprocess test that runs a mutation command with --project ... -s and then re-opens the project to verify the change persisted would prevent regressions (and can also cover --json + -s staying parseable).
|
Thx. |
Fixes #182
Problem
The Inkscape CLI fails to persist changes when run in one-shot mode. Each command starts a new Python process, creates a fresh
Sessionobject, modifies the project JSON in memory, and exits without saving.Solution
Port the
-s/--savepattern from the Shotcut harness (commit 17a30ba):-s/--saveflag to thecli()groupctx.call_on_close(_auto_save_callback)Testing
-sflag