From a99e8696fc720e2dd8050163d36cbf076fc83eba Mon Sep 17 00:00:00 2001 From: ljkx <177671075+ljkx@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:59:20 +0100 Subject: [PATCH] CLI --metadata: qualify the docs per format, test text and ICC (#9466) Review follow-up on #9975. The help text and the CLI book page said the XMP and free-form text are kept in every flat export and the ICC profile, EXIF and resolution "either way". Not every format holds them: JPEG and WebP have no place for free-form text, TIFF writes only its named text tags, and an Indexed Color document's palette PNG carries no XMP, text, ICC or EXIF at all, only its resolution. The texts (and the parity-doc revision row) now say the XMP and text are kept where the format holds them and that the option leaves everything else as it is without it. The test fixture gains a free-form text entry and a Display P3 profile (from photocraft-cms, now a dev-dependency; no profile file committed), so it checks the text is gone with `none` and kept by a default PNG, and that the default export keeps a real ICC profile that `none` matches, instead of comparing None with None. --- Cargo.lock | 1 + apps/photocraft-cli/Cargo.toml | 2 ++ apps/photocraft-cli/src/args.rs | 6 +++--- apps/photocraft-cli/tests/it/cli.rs | 20 ++++++++++++++------ book/src/automation/cli.md | 17 +++++++++++------ docs/file-format-parity.md | 2 +- 6 files changed, 32 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ab0417b79b..0fab1b2a97 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5102,6 +5102,7 @@ dependencies = [ "clap_complete", "clap_mangen", "photocraft-automation", + "photocraft-cms", "photocraft-codecs", "photocraft-engine", "photocraft-format", diff --git a/apps/photocraft-cli/Cargo.toml b/apps/photocraft-cli/Cargo.toml index 94e04b518a..19ef8bfa92 100644 --- a/apps/photocraft-cli/Cargo.toml +++ b/apps/photocraft-cli/Cargo.toml @@ -34,6 +34,8 @@ thiserror = { workspace = true } tokio = { version = "1", features = ["rt-multi-thread", "macros"] } [dev-dependencies] +# A real ICC profile for the --metadata test (#9466). +photocraft-cms = { workspace = true } # Synthetic camera raws (#7789). photocraft-raw = { workspace = true, features = ["testgen"] } # Shell completions and man pages (examples/man-and-completions.rs, for the packages); not in diff --git a/apps/photocraft-cli/src/args.rs b/apps/photocraft-cli/src/args.rs index a5de7bbc68..4ba08fbaae 100644 --- a/apps/photocraft-cli/src/args.rs +++ b/apps/photocraft-cli/src/args.rs @@ -79,9 +79,9 @@ pub(crate) struct ExportArgs { #[arg(long)] pub(crate) pcraft_full_composite: bool, /// The document metadata a flat export embeds: all (the default) keeps the document's XMP - /// packet and free-form text, as Save As does; none leaves them out, as Export As's Metadata: - /// None. The ICC profile, EXIF and resolution are kept either way, and PSD, PSB and .pcraft - /// always keep the XMP + /// packet and free-form text where the format holds them, as Save As does; none leaves them + /// out, as Export As's Metadata: None. Nothing else changes: the ICC profile, EXIF and + /// resolution are written as they are without it. PSD, PSB and .pcraft always keep the XMP #[arg(long, value_name = "none|all", value_parser = metadata)] pub(crate) metadata: Option, } diff --git a/apps/photocraft-cli/tests/it/cli.rs b/apps/photocraft-cli/tests/it/cli.rs index caad46edc1..cffeed0644 100644 --- a/apps/photocraft-cli/tests/it/cli.rs +++ b/apps/photocraft-cli/tests/it/cli.rs @@ -758,10 +758,11 @@ fn jpeg_format_chooses_the_scans() { } } -/// #9466: `--metadata none` leaves the document's XMP packet out of a flat export from `convert`, -/// `run --out` and `batch`, as Export As's Metadata: None does, and changes nothing else (pixels, -/// ICC profile, resolution). The default, and `all`, keep it as before, and a PSD or `.pcraft` -/// keeps it either way. A value it doesn't name is a usage error and nothing is written. +/// #9466: `--metadata none` leaves the document's XMP packet and free-form text out of a flat +/// export from `convert`, `run --out` and `batch`, as Export As's Metadata: None does, and changes +/// nothing else (pixels, ICC profile, resolution). The default, and `all`, keep them as before (the +/// text where the format holds it, as PNG does), and a PSD or `.pcraft` keeps the XMP either way. +/// A value it doesn't name is a usage error and nothing is written. #[test] fn metadata_none_leaves_out_the_document_xmp() { const XMP: &str = "\ @@ -776,10 +777,14 @@ fn metadata_none_leaves_out_the_document_xmp() { let mut img = photocraft_codecs::Image::from_u8(8, 6, photocraft_codecs::ChannelLayout::Rgba, data).unwrap(); img.meta.xmp = Some(XMP.into()); img.meta.dpi = Some((300.0, 300.0)); + img.meta.text = vec![("Comment".into(), "free-form-note".into())]; + // Display P3 rather than sRGB, so no export can pass for keeping it by writing its default. + img.icc = Some(photocraft_cms::Builtin::DisplayP3.profile().to_bytes().to_vec()); std::fs::write(&a, photocraft_codecs::encode(&img, photocraft_codecs::Format::Png, &Default::default()).unwrap()).unwrap(); let ancestors = |p: &Path| String::from_utf8_lossy(&std::fs::read(p).unwrap()).contains("DocumentAncestors"); let decoded = |p: &Path| photocraft_codecs::decode(&std::fs::read(p).unwrap()).unwrap(); - assert!(ancestors(&a), "the input carries the packet"); + let note = |p: &Path| decoded(p).meta.text.iter().any(|(_, v)| v == "free-form-note"); + assert!(ancestors(&a) && note(&a), "the input carries the packet and the text"); let invert = r#"[{"command":"image.adjustments.invert"}]"#; for ext in ["png", "webp", "jpg", "tif"] { let mut converted_files = Vec::new(); @@ -795,12 +800,15 @@ fn metadata_none_leaves_out_the_document_xmp() { let batched = d.join(format!("batch-{name}")).join(format!("a.{ext}")); for out in [&converted, &ran, &batched] { assert_eq!(ancestors(out), flag != Some("none"), "{name}: {}", out.display()); + // PNG holds free-form text; JPEG, WebP and TIFF (but for its named tags) don't. + assert_eq!(note(out), ext == "png" && flag != Some("none"), "{name} text: {}", out.display()); } converted_files.push(converted); } - // Only the XMP goes: the pixels, profile and resolution are the default export's. + // Only the XMP and text go: the pixels, profile and resolution are the default export's. let (default, none) = (decoded(&converted_files[0]), decoded(&converted_files[2])); assert_eq!(none.data(), default.data(), "{ext}: pixels"); + assert!(default.icc.is_some(), "{ext}: the default export keeps the ICC profile"); assert_eq!(none.icc, default.icc, "{ext}: ICC profile"); assert_eq!(none.meta.dpi, default.meta.dpi, "{ext}: resolution"); assert!(none.meta.dpi.is_some_and(|(x, y)| (x - 300.0).abs() < 0.5 && (y - 300.0).abs() < 0.5), "{ext}: {:?}", none.meta.dpi); diff --git a/book/src/automation/cli.md b/book/src/automation/cli.md index 2f8c3628d7..c7278455ef 100644 --- a/book/src/automation/cli.md +++ b/book/src/automation/cli.md @@ -37,14 +37,19 @@ the operation. To list or replace the missing families intentionally, use ### Document metadata -`convert`, `run --out` and `batch` keep the document's XMP packet and -free-form text in a flat export (PNG, JPEG, WebP, TIFF, …), as Save As does. -`--metadata none` leaves them out, as Export As's Metadata: None does: a -Photoshop file's XMP can list every placed document's id +`convert`, `run --out` and `batch` keep the document's XMP packet in a flat +export whose format holds one (PNG, JPEG, WebP, TIFF, …), and its free-form +text where the format holds that too (PNG text chunks, TIFF's named text tags +such as ImageDescription; JPEG and WebP have no place for it), as Save As +does. `--metadata none` leaves them out, as Export As's Metadata: None does: +a Photoshop file's XMP can list every placed document's id (`DocumentAncestors`) and the text of every type layer, and can be far larger than the image. `--metadata all` is the default. The option changes nothing -else: the ICC profile, EXIF and resolution are written either way, and PSD, -PSB and `.pcraft` saves always keep the XMP. It is not a full metadata scrub. +else: the ICC profile, EXIF and resolution are written as they are without +it, where the format holds them. An Indexed Color document saved as a palette +PNG carries no XMP, text, ICC profile or EXIF with either value, only its +resolution. PSD, PSB and `.pcraft` saves always keep the XMP. It is not a full +metadata scrub. ```sh photocraft-cli batch --actions resize.json --in masters/ --out web/ \ diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index c586cb0b60..3fcbc6eb84 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -151,7 +151,7 @@ The original reference entry retains its source date and historical totals; curr | Date | Change | Summary | |---|---|---| -| 2026-11-03 | minor | CLI `--metadata` (`none` or `all`) for `convert`, `run --out` and `batch`: `none` leaves the document's XMP packet and free-form text out of flat exports (PNG, JPEG, WebP, TIFF, …) through Export As's Metadata: None path; `all`, which keeps them as Save As does, stays the default. ICC, EXIF and resolution are written either way; PSD, PSB and `.pcraft` always keep the XMP (#9466) | +| 2026-11-03 | minor | CLI `--metadata` (`none` or `all`) for `convert`, `run --out` and `batch`: `none` leaves the document's XMP packet and free-form text out of flat exports (PNG, JPEG, WebP, TIFF, …) through Export As's Metadata: None path; `all`, which keeps them as Save As does, stays the default. The ICC profile, EXIF and resolution are written as they are without the option (a palette PNG carries only its resolution, either way); PSD, PSB and `.pcraft` always keep the XMP (#9466) | | 2026-11-01 | minor | `.atn`: event reference [atn.md](atn.md): the 117 supported events, and measured keys, defaults, ranges and enum values for 101 events not mapped yet | | 2026-10-31 | minor | `.atn` Group B: four Render events load, play and save; 120 supported and 97 unmapped. Supported-table columns corrected against the event registry; the original 117/101 reference entry remains historical. | | 2026-10-31 | minor | `.atn`: Pinch (`Pnch`), Polar Coordinates (`Plr `), Spherize (`Sphr`), Twirl (`Twrl`), Wave (`Wave`) and ZigZag (`ZgZg`) map to `filter.distort.*` with every key in recorded order; out-of-range values clamped and missing keys given Photoshop's defaults on import; a Wave scaled below 100 % stays a Photoshop step (the command has no scale); Shear still kept verbatim (#10605) |