- Remove blank lines between doc comments or #[test] attributes and the item (S8855, S8856)
- type_tool: drop the redundant else after continue in the IME event arm (S8862)
- ui_state: pass OsStr::to_os_string instead of an equivalent closure (S1612)
- macos packaging: [[ ]] for the attempt test (S7688); bind validate_staple's argument to a local (S7679)
* chore: resolve SonarCloud maintainability issues
* fix: format imports and fix effect_entry argument type
* fix: restore test closure and wildcard import that broke compilation
* refactor: split composite_to_surface to reduce cognitive complexity
* feat(updates): startup update prompt modal and direct self-updating
Release checks and the install go through craft-update's Updater: the engine's Help ▸ Check for Updates command keeps its status machine, its transport asks craft-update instead of a private GitHub client, and a found release opens the startup prompt. An install holds back while documents are unsaved, and a finished update closes through the guarded close path. craft-update stays out of the wasm build.
Ref: storytold/craft-libs#319
* fix(updates): address review feedback
Wake the UI when a manual check is queued from a menu callback; the update prompt owns the keyboard while it is open (LightCraft); Download opens through the desktop URL hook (FilmCraft); the Help menu dispatches through menus::invoke (FigCraft); print jobs and comment drafts hold an install back (WordCraft); Check for Updates is a registered command (GridCraft); an install holds back while documents are unsaved.
Conflicts: the batch's wheel reads (#5547 Animated Zoom) keep their extra
argument with main's ids (`Id::unique`, `ui.scope_id()`); crop presets keep
#6620's `apply_choice` with main's accessible name; Bevel & Emboss keeps
#9370's per-mode id with main's name argument; the Stroke field keeps
#7203's gradients with main's name; tone.rs keeps #9139's move of the
Histogram panel to histogram_panel.rs (main only renamed ids there);
translation catalogs as unions.
Not flagged by git, the batch's new code follows egui 0.37 too: dropdowns
take an accessible name (Histogram Channel/Source #9139, tool Mode #5504,
Art History Style #1610, New Snapshot From #3567), MouseWheel events carry
`source`, `WidgetType` is `Role` (ColorButton is ColorWell, #7203),
`Id::new`/`Ui::id` are `Id::unique`/`Ui::scope_id`.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* egui037: name each control for what it sets, and order the eager fonts for the UI
CodeRabbit review follow-ups on the migration: the Artboard X/Y fields, the CAF
dropdowns, the channel Solidity field, the GPU backend/adapter dropdowns, the
corner radius fields and the canvas extension colour all had an accessible name
that named another control (or was shared by four). The eager craft-fonts
registration now puts the UI Japanese face first, as the lazy loader does, so
Japanese UI is not drawn in the manifest's Mincho face and shared Han characters
do not take Japanese forms.
Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
* egui037: rustfmt the eager craft-font order
---------
Co-authored-by: Dylan M. Taylor <dylan@dylanmtaylor.com>
Co-authored-by: Codebuff <noreply@codebuff.com>
Photoshop reads and writes IFF; PhotoCraft opened Amiga IFF/ILBM files
but had no writer. Saving as .iff now writes a FORM ILBM with a BMHD and
a ByteRun1-compressed BODY of 24 bitplanes (red, green and blue, least
significant bit first), one plane row at a time. An image with alpha
also gets a mask plane (masking 1): alpha of 128 or more is opaque, and
when any pixel is not fully opaque the export warns that alpha is
reduced to on/off transparency. Gray is expanded to RGB, 16-bit and
float images are reduced to 8 bits, and images wider or taller than
65535 pixels are refused.
The reader also opens 24-bitplane ILBMs, so a saved file reopens with
the same pixels. A transparent color index in a 24-bitplane file is
refused as unsupported.
IFF leaves ASYMMETRIC_EXCEPTIONS, Save As offers Amiga IFF, and the
codecs README, file-format parity and G10 are updated.
* fix(engine): defined and imported patterns survive a restart (#9317)
The pattern library lived only in memory: patterns defined with Edit >
Define Pattern or imported from a .pat file were gone after a restart,
while their Patterns panel folders (saved with the preferences) still
named them. Photoshop keeps them.
The preset store now writes the user patterns (the library minus the
built-ins, in order) to patterns.pat, Photoshop's own pattern file
format, whenever their ids, names, sizes or order change, and loads
them when the store attaches. Like actions.json, a file that fails to
load is warned about and never overwritten. The web store accepts the
new record.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Patterns: persist same-id pixel replacements, retry a failed removal
CodeRabbit (discussion_r4369506324, r4369506326):
- an imported .pat whose id is already in the library replaces its
pixels, which the id/name/size key didn't show, so the old pixels came
back after a restart; PatternLibrary::pixels_rev is bumped on such a
replacement and is part of the key;
- a failed patterns.pat removal recorded the empty library as saved and
was never retried, so the deleted pattern came back; it now stays
pending, as a failed write does.
Tests: a_same_id_pattern_replacement_survives_a_restart,
a_failed_patterns_removal_is_retried.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Patterns: persist replaced built-ins and ASL replacements
CodeRabbit (discussion_r4369536978, r4369536981):
- a .pat import that replaced a built-in pattern by id was filtered out
as a built-in and never saved; only built-ins exactly as shipped
(is_pristine_builtin) are skipped now, and on load a saved built-in
takes the shipped one's place unless it changed before attachment;
- style.presets.importAsl replaced patterns by id without bumping
pixels_rev; both imports now go through PatternLibrary::upsert.
Tests: an_imported_builtin_replacement_survives_a_restart,
an_asl_pattern_replacement_bumps_pixels_rev.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* .pat: refuse names and counts the reader won't load, on write too
CodeRabbit (review 5641404908, outside-diff, two findings): now that the
preset store writes the user patterns, a file its own loader rejects or
truncates loses patterns at the next start.
- a name of 65,536+ UTF-16 units (with its NUL) was written although the
reader refuses it; write_pattern refuses it now;
- more than 100,000 patterns were written, and the reader kept the first
100,000 without a word; both now refuse counts over MAX_PAT_PATTERNS.
Tests: a_name_the_reader_refuses_is_refused_on_write,
more_patterns_than_a_pat_file_holds_are_refused.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Patterns: an early same-id import with other pixels is saved
CodeRabbit (review 5641454449, outside-diff, preset_store.rs:1191-1202):
a pattern imported before the store finished loading, with the id, name
and size of a saved one but other pixels, was appended (no pixels_rev
bump), won the merge, and then matched the loaded file's key, so it was
never written and a restart brought the old pixels back. The merge now
bumps pixels_rev when the copy it keeps differs from the saved one.
Test: an_early_same_id_import_wins_and_is_saved.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Merged with our fix-up d5139d177 (pr-1199-fixup), which merged main as of 8f09d122d.
Conflicts with main since then and with batch 8:
- photocraft-cli: main split the CLI into clap modules (#7071); the model wiring
(local_headless, configure_cli_models, batch's shared backend, droplet) is re-applied there.
- Cargo.lock regenerated: crates/ml pins ureq =3.4.2, so the app's ureq goes 3.5.0 -> 3.4.2.
- cutout_cmds: main's subject area (#8235) kept for the classical path beside the model path.
- file_cmds batch: main's per-input closure takes `s` later, so the backend and
integrations prefs are read before the closure instead of inside it.
- prefs_ui: model-choice filter with #4573's pref_choice_label; Integrations stays visible
(#4097) with the model choices only where models run; the translation-coverage test main
deleted (c3aa3f9e5) stays deleted; old section_fields test calls take PreferenceFieldOptions.
- Semantic clashes git did not flag: #4097's Integrations test literals in
apps/photocraft take ..Default::default() for the new model fields; main's and #4573's
section_fields test calls pass PreferenceFieldOptions.
- engine lib.rs, prefs.rs, smartselect tests, snapshot example, jobs_ui_tests, people.toml,
docs and i18n catalogs merged as unions.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
panels.rs: the PR's snapshot rows next to main's History Brush source
and scroll-to-current; ui-parity.md: main's status line.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- prefs_ui `unimplemented_preferences_are_hidden_from_the_dialog` still asserted the
Integrations section is hidden, so test (ubuntu/windows) failed (FredApps' review).
It now asserts both rows are visible; scratchDisks stays hidden.
- `integrations()` parsed the whole preferences file strictly, so one wrong-typed value
anywhere (`{"interface":{"enableAnimations":"nope"},"integrations":{"allowAgentControl":false}}`)
reset Allow Agent Control to on while the dialog (lenient load, #6611) showed it off.
The section is now read on its own, and field by field when it doesn't read as a whole
(a list like `[false]` reads as a whole, as in the app); a test compares it with the
app's own load.
- Both rows are read only at launch: they say "Applies at next launch." like System Title Bar.
- Removed the stale "not by these yet" comment in HIDDEN_UNTIL_IMPLEMENTED.
Co-authored-by: Erica Franco <20589100+Erica4fr@users.noreply.github.com>
Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts with #5547 (Animated Zoom) resolved in wheel_nav.rs, canvas.rs and
panels.rs: the Navigator and canvas pass both Animated Zoom and
!animations_off; with Animation Effects off, the sideways part of an
animated wheel zoom pans by the frame's own events folded by their
modifiers (#4573's unsmoothed scroll) instead of the plain-event raw.x,
so Ctrl + wheel still pans sideways there. #4573's modifier test runs
with Animated Zoom off and on.
Reconcile main 486afd82cc while preserving reviewed lifecycle, capture, IME and settings migrations. Include canonical dependency pins and the generated lockfile.
All nine CI stages passed: 11,580 passing test executions, zero failures and 91 ignored across 93 summaries. Native/browser acceptance and paired app performance remain open.
The checks parse() runs after clap built their messages from a const
and two format! calls. ServerOptionError holds them as thiserror
variants beside ValueError, each with the clap ErrorKind it reports
as: the PHOTOCRAFT_CONTROL_PORT host:port refusal, bridge roots, and a
token flag without its transport. The output is unchanged.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The review round of the #7071 stack, which #9167 merged without
(it took the pre-review heads):
- A bare --help or -h before `--` is help after any parse error, not
only after a missing value: `mcp stray.psd --help` and
`serve --port abc --port 1 -h` print the subcommand's help.
- mcp and serve --help carry #8035's rules: what the automation roots
grant, that bridge roots are refused, and that the token flags need
--bridge or --port (or PHOTOCRAFT_CONTROL_PORT).
- check_server takes the subcommand and whether a transport is set;
the bridge-roots check moves next to the other mcp checks.
- The server tests match clap's "a value is required for" text, and
mcp.md in the book and docs names PHOTOCRAFT_CONTROL_PORT.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Add the man-and-completions example, which writes photocraft-cli's
completions for bash, zsh, fish, elvish and PowerShell (clap_complete)
and its man pages (clap_mangen: photocraft-cli.1 and one per
subcommand, such as photocraft-cli-convert.1) into a directory:
cargo run -p photocraft-cli --example man-and-completions -- <out-dir>
clap_complete and clap_mangen are dev-dependencies, so the binary is
unchanged. clap_mangen gets its env feature, so the pages name each
flag's environment variable as --help does; clap already has env, so
Cargo.lock gains no package for it. The library exposes only
`command()`, the clap definition; the args module stays private.
A test runs Command::debug_assert and checks that every shell's
completions and every page generate. Another repeats the example's two
generate_to calls and pins the exact file list: the five completion
files under the names the packages install, photocraft-cli.1, one page
per subcommand, and no pages for the help subcommand.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Step 5 of the issue: launch_args' hand-written loop, USAGE and
LaunchArgs become a clap derive, which flattens photocraft-automation's
ControlArgs and AutomationArgs. `photocraft --help` comes from the doc
comments; `--version` prints the same `photocraft <long version>` line
(build_info::long_version_static), and `-V` now does too. --control
uses photocraft_automation::args::control_port instead of a local copy.
The cases the loop handled by hand are kept:
- An old macOS Finder's -psn_... argument is dropped before clap,
unless it comes after `--`, where it is a file name.
- The per-process options for the single-instance hand-off come from
ArgMatches::value_source, in a fixed order. A PerProcess enum names
each (and PHOTOCRAFT_GPU_ADAPTER): id() gives a flag's clap id,
Display the flag or the variable, so single_instance's log line reads
as before. As since #7242, a flag's variable counts as the flag: one
clap reads (PHOTOCRAFT_IN_WINDOW_MENUS when it turns the menus on, an
automation root that isn't blank) is counted here, as clap read it;
env_launch_options keeps #7242's rules for the three read elsewhere
(present mode, frame latency, stroke trace) and returns PerProcess.
- A file name that isn't valid Unicode is still reported in a notice
instead of being opened (#1108); file_names returns
FileNames { openable, unreadable }.
- Windows release builds still have no console: help, version and
usage errors show only when redirected, as since #6435. clap's
Error::exit, like print_stdout, never panics on a closed stream.
The variables are now declared on their flags: PHOTOCRAFT_CONTROL_PORT
on --control, the token variables through ControlArgs (so
security::token_inputs, which nothing else used, is removed), and
PHOTOCRAFT_AUTOMATION_*_ROOT added on the desktop's Command only, so
photocraft-cli still ignores them (#1551).
Behaviour changes, as step 0 of the issue decided:
- A bad or missing --present-mode, --frame-latency or --stroke-trace
value is a usage error (exit 2) instead of a warning. Their
variables stay lenient (logged and ignored); presentation's value
checks serve both, and the flags refuse a value with a ValueError.
- A repeated flag is a usage error, and a value can't start with `-`
unless it is a negative number (write --opt=-value).
- clap reports the first usage error, not all of them. On an unknown
argument clap resolves the pending option value but drops its error,
so `--control nope --safegpu` names `--safegpu`.
- An empty or blank variable counts as unset; before, only an empty
PHOTOCRAFT_CONTROL_PORT did, and an empty token or root was passed
on as given. PHOTOCRAFT_IN_WINDOW_MENUS also reads false, no and
off as off (FalseyValueParser). A variable that isn't valid Unicode
where text is expected is a usage error instead of being ignored.
- With --control (or its variable), a token together with a token
file, from flags or variables, is clap's either/or usage error at
parse time, before the server's own check. Without it no token is
read, so a stale exported pair stops nothing, as before.
- With --control given, PHOTOCRAFT_CONTROL_PORT is not read, so a
bad value there is no longer reported too.
The CPU-compatibility relaunch puts --safe-gpu before the original
arguments, so a `--` among them can't turn it into a file name.
The launch_args unit tests now check clap's errors instead of the error
list. The process tests put a late usage error (--control 0 with the
token pair) where they used a bad port, so stderr still shows how the
other arguments were taken, and cover the variables.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Two process tests of today's behaviour that the clap port keeps:
`--version` prints `photocraft <long version>` and stops reading the
arguments, and PHOTOCRAFT_AUTOMATION_READ_ROOT / _WRITE_ROOT reach the
control server's workspace like their flags. The process tests return
a Run struct instead of tuples, and control_exit_code gains run_with
to set variables.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The hand-written argument loop, its LaunchArgs, USAGE and helpers
(parse_control_port, usage_exit_code, print_stdout) and their unit
tests move unchanged from main.rs into a new launch_args module; only
the module doc, the `mod`/`use` lines and pub(crate) visibility are
new. main.rs keeps the environment defaults and the exits.
This is the step before the clap port, so that one reads as a rewrite
of launch_args.rs rather than a large main.rs deletion. Check with
`git show --color-moved=dimmed-zebra`.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
With the environment fallbacks, clap's conflicts_with on the token pair
made a stale exported PHOTOCRAFT_CONTROL_TOKEN and
PHOTOCRAFT_CONTROL_TOKEN_FILE a usage error for every mcp and serve, though
headless mcp and stdio serve read no token. As on main, the pair is now
refused only by serve on TCP and mcp bridging, still as clap's either/or
usage error naming each half as it was given.
An automation root on the command line with PHOTOCRAFT_CONTROL_PORT set
is a usage error too: the variable would switch mcp to bridging, which
drops the roots, with only a note on stderr.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The desktop app's --control already reads PHOTOCRAFT_CONTROL_PORT, and
the CLI read the token variables at run time. Now every fallback is
declared with `env =` on its flag (clap's `env` workspace feature, no
new crate in Cargo.lock):
- --control-token and --control-token-file in ControlArgs fall back to
PHOTOCRAFT_CONTROL_TOKEN and PHOTOCRAFT_CONTROL_TOKEN_FILE, as
security::token_inputs did at run time. The CLI no longer calls
token_inputs, so the variables are read in one place; the desktop
app still uses it until step 5. Help names the token's variable but
never shows its value (hide_env_values). A token from the
environment together with the other flag is now clap's either/or
usage error (exit 2) rather than the automation crate's (exit 1).
- `serve` takes PHOTOCRAFT_CONTROL_PORT as --port and `mcp` as the
bridge port when the flag is absent, as #2495 proposed. The variable
must be a port: `mcp` refuses 127.0.0.1:7878 or :7878 from it, as the
desktop app's --control would; only --bridge takes host:port.
- The automation roots stay flag-only for the CLI: a
PHOTOCRAFT_AUTOMATION_*_ROOT exported for the desktop app must not
widen headless file access (#1551).
- A flag wins, and an empty or blank variable counts as unset
(blank_env_is_unset; clap would hand the empty value to the parser),
so stdio and headless stay the default.
- A bad value or conflict from a variable is a usage error naming the
variable instead of the flag it stands in for (name_env_vars).
- The port variable changes what the command does, so when it (not the
flag) turns on TCP or bridging, the command says so on stderr and
names the way back (ArgMatches::value_source tells which).
bin() in the tests removes the token and port variables, so an
exported one can't change a test. New tests:
control_port_env_switches_modes_with_a_notice_and_flags_win (ported
from #2495, also checking the bad values) and
control_token_env_vars_are_the_flags_fallbacks.
Co-authored-by: ahmedhosnypro <ahhosnyas@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Step 4 of the issue: the control token flags and the automation roots
move to photocraft-automation's new args module as two clap structs,
which mcp and serve flatten and the desktop app can flatten in step 5
without moving them again. clap is a plain dependency of the crate.
- ControlArgs holds --control-token and --control-token-file;
AutomationArgs holds the roots, and AutomationArgs::workspace opens
them (replacing the CLI's automation_workspace).
- The port value parser is photocraft_automation::args::control_port,
with its unit tests, for serve --port and --bridge now and the
desktop app's --control later. It ignores surrounding spaces and
refuses anything else with a typed NotAPort.
- --bridge parses to Bridge::Port or Bridge::Addr: it takes a bare
port, meaning 127.0.0.1:<port>, or any value containing a ':', which
the bridge client then validates. Anything else is a usage error
(exit 2) instead of a bridge error.
- 127.0.0.1 addresses are built from Ipv4Addr::LOCALHOST.
mcp_bridge_takes_a_port_or_an_address is new: `--bridge nope` is a
usage error, and `--bridge 1` bridges to 127.0.0.1:1, which a tool
call names when nothing listens there.
No new environment behaviour: the token variables are still read at
run time through security::token_inputs, and the roots stay flag-only.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Step 3 of the issue: handlers get values that are already checked, so
a bad one is a clap usage error naming its flag (exit 2) and the
"bad --x" errors (exit 1) are gone.
- --quality is a u8 in 1..=100; --tga-bits, --dpx-bits and
--dds-compression parse to photocraft_io's TgaBits, DpxBits and
DdsCompression through their from_bits/from_name, and --tiff-compression
and --jpeg-format to photocraft_codecs' TiffCompression and JpegFormat,
refusing a bad one with a typed ValueError; serve --port is a u16. export_opts becomes `impl From<&ExportArgs> for ExportOptions`,
which can't fail.
- run's --new and --params parse to serde_json::Value, so JSON that
doesn't parse is a usage error; whether it is an object is still the
command's to say (exit 1). The --cmd/--params pairing keeps using
ArgMatches::indices_of, now over typed Step values.
- Paths are PathBufs (convert, info, run, batch, the control token
file and automation roots), and run hands the OsStrings to clap
instead of refusing non-Unicode up front: a Latin-1 file name works
as a path (#1108), and one given as text is clap's usage error.
load_actions_file takes a &Path, and photocraft_engine's same_file
takes any AsRef<Path>, so convert's #7437 check compares the PathBufs.
- Rules live in the types: run needs exactly one of <FILE> or --new
(ArgGroup "source") and a --cmd or --out ("work"); batch's --actions,
--in and --out are required; --control-token conflicts with
--control-token-file. The runtime checks and their messages are
deleted. The automation crate's own token check stays, as it also
sees PHOTOCRAFT_CONTROL_TOKEN(_FILE).
Tests changed for this: run_errors, quality_outside_1_to_100_is_refused,
dpx_converts_both_ways_with_the_bits_asked_for,
dds_converts_both_ways_with_the_compression_asked_for,
tga_bits_choose_the_targa_depth, jpeg_format_chooses_the_scans
(exit 2, the error names the flag) and
the #1108 test, now
a_non_unicode_path_is_taken_and_non_unicode_text_is_a_usage_error:
convert reads and writes such a file name end to end, while info opens
through the automation layer, whose paths must still be UTF-8, and
refuses it (exit 1). either_or_flags_are_usage_errors is new.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The CLI page said parsing reports malformed flags and JSON. Name what
the step 0 decisions made a usage error: a single-value flag given twice
(--cmd and --params repeat), how to pass a value starting with '-', and
that `version` is only --version/-V. Say why --quality allows negative
numbers.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
run took &[String] and run_os &[OsString], and run_os only checked
each argument was Unicode before calling run. A single run generic over
Into<OsString> (as clap's try_get_matches_from is) does that check once
and serves main's args_os and the tests' &str slices alike.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The behaviour changes decided in step 0 of the issue:
- A repeated single-value flag is a usage error (exit 2) instead of
the last one winning: args_override_self is gone, and with it the
argument scan that found repeats for mcp and serve (#8035). --cmd and
--params stay repeatable.
- A value starting with '-' was already written --out=-image.png or
./-image.png since the clap port (a flag can't be a value since
#8035); this pins it, and that `--filter -1` still works and
`--quality -1` is still refused by its range.
- The bare `version` subcommand is gone; clap's version flag prints
the same `photocraft-cli <long version>` line for --version and now
-V too. `help <subcommand>` stays.
clap's version wants a &'static str, so build_info gains
long_version_static, long_version worked out once in a OnceLock, next
to long_version. The desktop app will need the same in step 5.
flag_value_forms_and_repeats and top_level_help_version_and_no_arguments
now pin the new behaviour; a_value_starting_with_a_dash_is_written_with_equals
is new.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
batch --help printed the "--out that is the --in folder is refused"
sentence twice, in the subcommand's long help and on --in-place. Keep
it on the flag, where -h shows it too.
steps() read --cmd/--params with get_many, which panics on a type
mismatch, against the crate's never-panic rule. Use try_get_many and
treat an error as no values; behaviour is otherwise the same.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Behaviour change from the clap port, not listed in its commit: `help`
followed by an unknown name or extra arguments is a usage error
(exit 2); before, `help …` always printed the whole USAGE with exit 0.
usage_and_unknown_command now pins `help frobnicate` exiting 2 with
the error on stderr.
The --control-token-file help says it is used instead of
--control-token, as the old USAGE showed them as either/or. The
--in-place help carries the "--out equal to --in is refused" sentence,
so `batch -h` shows it and not only `batch --help`.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Replace the hand-rolled parser (USAGE, SUBCOMMANDS, Args::get/has) with
clap derive types in args.rs: a Cli parser, one Args struct per
subcommand, and flattened ExportArgs (convert, run, batch) and
ControlArgs (mcp, serve). The USAGE prose moves into doc comments, so
-h and --help carry it. Handlers keep their bodies; only their argument
type and the lines reading flags change.
Behaviour is kept: the last of a repeated flag wins (args_override_self),
values stay strings checked by the handlers with the same messages and
exit 1, a value flag never takes the next flag as its value, and help
there (`--out --help`) is still help, --version/version are checked
before clap, help goes to stdout with exit 0 and usage errors to stderr
with exit 2. run pairs each --params with the --cmd before it by putting
both back in command-line order with ArgMatches::indices_of.
mcp and serve keep #8035's launch checks: no positional arguments, no
repeated or empty options, --control-token or --control-token-file but
not both, a token flag only with --bridge or --port, and no automation
roots with --bridge. clap gives the first four (a repeat is found by
scanning the arguments while args_override_self stays on); a small
check after parsing gives the last two, with #8035's messages.
Behaviour changes (allowed by the issue's step 2):
- Help and error text is clap's: per-subcommand help instead of the one
USAGE page, "unexpected argument '--fromat' found" instead of
"unknown flag --fromat for batch", and so on.
- A wrong positional count is a usage error (exit 2) instead of a
handler error (exit 1): convert with one or three paths, info with
none, droplet with no inputs, run with two files.
- A positional starting with '-' (other than "-") needs a '--' before
it, and '--' now ends the flags instead of being an unknown flag.
- A value starting with a single '-' is written --out=-image.png (or
./-image.png), as one starting with '--' already was; --quality and
--filter keep negative numbers (allow_negative_numbers).
- `help <subcommand>` prints that subcommand's help.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
invoke returned (code, stdout, stderr) and ok (stdout, stderr), so each
call site had to remember the order. Return Invoke and Printed structs
instead.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Every failure was formatted into a String where it happened, so callers
could only print it. Subcommands now return a CliError, the actions
loaders an ActionsError and the hand-written parser a UsageError; each
variant prints exactly the text the String did, and the engine and
automation errors pass through unchanged.
Also: print_json takes a JsonStyle instead of a bool, the engine's
missing_font_warnings replaces the CLI's copy (same text), batch warns
through warn_all, run drops a second, no-op insert into its reported
font set, and the src tests share one Invoke helper.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
lib.rs held the whole CLI in one 754-line file. Split it into args,
output, document, actions, batch and automation modules so the argument
parser can be replaced with clap in a separate, reviewable change.
Function bodies, doc comments and tests move unchanged. The only edits
are mod/use lines, pub(crate) visibility on items now shared across
modules, and the imports the moved test modules need. The public API
(run, run_os, parse_actions, load_actions, load_actions_file, Steps,
USAGE) is re-exported from lib.rs.
Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Brings in 412 commits from main (cf9ff2a2). Two conflicts:
- crates/ui-egui/src/lib.rs: main added updates_ui::tick right after
prefs_ui::tick, where this branch calls wheel_nav::unsmooth. Both stay,
main's first; the update tick reads no input.
- Cargo.lock: main's, with cargo adding back this branch's one line (the
desktop app's direct windows dependency).
docs/scorecard.md is regenerated; it differs from main only in UI-217-10.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
FreeBSD (and every other OS without a capture backend) compiles the
screen colour picker's stub capture, which never reads monitor placements
or builds images. The build warned about the unused ScreenImage import,
the never-read Placement fields, find_monitor and count. Compile those
only where a capture backend or the tests use them.
Seen in the FreeBSD aarch64 CI build; checked with `cargo check -p
photocraft --all-targets` on FreeBSD 15.1 aarch64 (no warnings) and macOS.
aiongg