mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-14 19:27:01 -04:00
Merge PR #287
This commit is contained in:
8 files changed
+358
-6
No files matched your search
@@ -661,7 +661,7 @@ fn render(args: &[String]) -> Result<(), String> {
|
||||
for (k, v) in opts {
|
||||
p[k] = v;
|
||||
}
|
||||
let mut o = ExportOptions::from_json(&p);
|
||||
let mut o = ExportOptions::from_params(&p).map_err(|e| e.to_string())?;
|
||||
if p.get("format").is_none() {
|
||||
let ext = Path::new(&output).extension().map(|e| e.to_string_lossy().to_string()).unwrap_or_default();
|
||||
o.format = ExportFormat::parse(&ext)
|
||||
|
||||
@@ -54,6 +54,8 @@ fn save(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
if let Some(o) = params.as_object_mut() {
|
||||
o.retain(|k, _| !NOT_SETTINGS.contains(&k.as_str()));
|
||||
}
|
||||
// a preset that app.export would refuse is refused here, where the typo is on screen
|
||||
crate::export::ExportOptions::validate(ID, ¶ms)?;
|
||||
let preset = ExportPreset { name: name.to_string(), params };
|
||||
match s.export_presets.iter_mut().find(|x| x.name.eq_ignore_ascii_case(name)) {
|
||||
Some(x) => *x = preset,
|
||||
|
||||
@@ -562,12 +562,248 @@ impl Default for ExportOptions {
|
||||
}
|
||||
}
|
||||
|
||||
/// The `app.export` params that are not export options: which photos, where to, a preset to start
|
||||
/// from, and the desktop app's background flag.
|
||||
pub const TARGET_PARAMS: &[&str] = &["id", "ids", "path", "dir", "preset", "background"];
|
||||
|
||||
/// The export option params [`ExportOptions::from_json`] reads (and [`ExportOptions::to_json`] writes).
|
||||
pub const OPTION_PARAMS: &[&str] = &[
|
||||
"format",
|
||||
"quality",
|
||||
"resize",
|
||||
"longEdge",
|
||||
"shortEdge",
|
||||
"width",
|
||||
"height",
|
||||
"megapixels",
|
||||
"percent",
|
||||
"dontEnlarge",
|
||||
"ppi",
|
||||
"limitKb",
|
||||
"sharpen",
|
||||
"sharpenAmount",
|
||||
"naming",
|
||||
"startNumber",
|
||||
"subfolder",
|
||||
"conflict",
|
||||
"tiffCompression",
|
||||
"dngCompression",
|
||||
"metadata",
|
||||
"removeLocation",
|
||||
"watermark",
|
||||
"colorSpace",
|
||||
"bitDepth",
|
||||
];
|
||||
|
||||
/// The keys of a `watermark` object ([`Watermark`], camelCase).
|
||||
pub const WATERMARK_PARAMS: &[&str] = &["text", "vertical", "size", "opacity", "anchor", "inset", "color", "shadow", "image", "imageWidth"];
|
||||
|
||||
/// Text height as a fraction of the short edge: the range the renderer draws without clamping.
|
||||
pub const WATERMARK_SIZE_RANGE: (f32, f32) = (0.005, 0.5);
|
||||
/// Margin as a fraction of the short edge.
|
||||
pub const WATERMARK_INSET_RANGE: (f32, f32) = (0.0, 0.4);
|
||||
/// A graphic's width as a fraction of the photo's width.
|
||||
pub const WATERMARK_IMAGE_WIDTH_RANGE: (f32, f32) = (0.01, 1.0);
|
||||
|
||||
/// Levenshtein distance, for "did you mean" on a misspelled key (both inputs are short).
|
||||
fn edit_distance(a: &str, b: &str) -> usize {
|
||||
let (a, b): (Vec<char>, Vec<char>) = (a.chars().collect(), b.chars().collect());
|
||||
let mut prev: Vec<usize> = (0..=b.len()).collect();
|
||||
for (i, ca) in a.iter().enumerate() {
|
||||
let mut cur = vec![i + 1; b.len() + 1];
|
||||
for (j, cb) in b.iter().enumerate() {
|
||||
let sub = prev.get(j).copied().unwrap_or(0) + usize::from(ca != cb);
|
||||
let del = prev.get(j + 1).copied().unwrap_or(0) + 1;
|
||||
let ins = cur.get(j).copied().unwrap_or(0) + 1;
|
||||
if let Some(c) = cur.get_mut(j + 1) {
|
||||
*c = sub.min(del).min(ins);
|
||||
}
|
||||
}
|
||||
prev = cur;
|
||||
}
|
||||
prev.last().copied().unwrap_or(0)
|
||||
}
|
||||
|
||||
/// `unknown parameter \`k\``, naming the closest known key when one is near.
|
||||
fn unknown_key(what: &str, k: &str, known: &[&str]) -> String {
|
||||
let near = known.iter().map(|n| (edit_distance(&k.to_ascii_lowercase(), &n.to_ascii_lowercase()), *n)).filter(|(d, _)| *d <= 2).min();
|
||||
match near {
|
||||
Some((_, n)) => format!("unknown {what} `{k}` (did you mean `{n}`?)"),
|
||||
None => format!("unknown {what} `{k}` (one of {})", known.join(", ")),
|
||||
}
|
||||
}
|
||||
|
||||
/// A JSON integer (`92`; never `92.5` or `"92"`), as [`ExportOptions::from_json`] reads them.
|
||||
fn whole(v: &serde_json::Value) -> Option<i64> {
|
||||
v.as_i64()
|
||||
}
|
||||
|
||||
impl ExportOptions {
|
||||
/// Strict [`ExportOptions::from_json`] for a command that is about to export (issue #181): an
|
||||
/// unknown parameter or a value that cannot be read as what it is for is an error, never a
|
||||
/// silent default. `app.export` targets (`id`, `ids`, `path`, `dir`, `preset`, `background`)
|
||||
/// are accepted and left to the caller.
|
||||
pub fn from_params(p: &serde_json::Value) -> crate::Result<Self> {
|
||||
Self::validate("app.export", p)?;
|
||||
Ok(Self::from_json(p))
|
||||
}
|
||||
|
||||
/// Check `app.export` params for `cmd` (the command named in the error): every key must be an
|
||||
/// export option ([`OPTION_PARAMS`]) or a target ([`TARGET_PARAMS`]), every value must be the
|
||||
/// kind of thing its key takes, and a `watermark` object must hold only [`WATERMARK_PARAMS`]
|
||||
/// with its sizes in range (issue #183). `null` means "not given", as a dropped key does.
|
||||
pub fn validate(cmd: &str, p: &serde_json::Value) -> crate::Result<()> {
|
||||
use serde_json::Value;
|
||||
let bad = |msg: String| crate::cmd::bad(cmd, msg);
|
||||
let Some(obj) = p.as_object() else { return Err(bad("params must be an object".into())) };
|
||||
let known: Vec<&str> = OPTION_PARAMS.iter().chain(TARGET_PARAMS).copied().collect();
|
||||
let int = |k: &str, v: &Value, lo: i64, hi: i64| -> crate::Result<i64> {
|
||||
whole(v).filter(|i| (lo..=hi).contains(i)).ok_or_else(|| bad(format!("`{k}` must be an integer {lo}..{hi}")))
|
||||
};
|
||||
let num = |k: &str, v: &Value, lo: f64, hi: f64| -> crate::Result<f64> {
|
||||
v.as_f64().filter(|f| f.is_finite() && (lo..=hi).contains(f)).ok_or_else(|| bad(format!("`{k}` must be a number {lo}..{hi}")))
|
||||
};
|
||||
fn string<'a>(cmd: &str, k: &str, v: &'a Value) -> crate::Result<&'a str> {
|
||||
v.as_str().ok_or_else(|| crate::cmd::bad(cmd, format!("`{k}` must be a string")))
|
||||
}
|
||||
let boolean = |k: &str, v: &Value| -> crate::Result<bool> { v.as_bool().ok_or_else(|| bad(format!("`{k}` must be true or false"))) };
|
||||
let one_of = |k: &str, v: &Value, options: &[&str]| -> crate::Result<()> {
|
||||
let s = string(cmd, k, v)?;
|
||||
if options.iter().any(|o| o.eq_ignore_ascii_case(s)) { Ok(()) } else { Err(bad(format!("`{k}` must be one of {}", options.join("|")))) }
|
||||
};
|
||||
for (k, v) in obj {
|
||||
if v.is_null() {
|
||||
continue;
|
||||
}
|
||||
match k.as_str() {
|
||||
"format" => {
|
||||
if ExportFormat::parse(string(cmd, k, v)?).is_none() {
|
||||
return Err(bad(format!("`{k}` must be one of jpeg|png|tiff|webp|avif|dng|original")));
|
||||
}
|
||||
}
|
||||
"quality" => {
|
||||
int(k, v, 1, 100)?;
|
||||
}
|
||||
"ppi" => {
|
||||
int(k, v, 1, u16::MAX as i64)?;
|
||||
}
|
||||
"limitKb" => {
|
||||
int(k, v, 0, u32::MAX as i64)?;
|
||||
}
|
||||
"startNumber" => {
|
||||
int(k, v, 0, 999_999)?;
|
||||
}
|
||||
"bitDepth" => {
|
||||
if !matches!(whole(v), Some(8 | 10 | 16 | 32)) {
|
||||
return Err(bad(format!("`{k}` must be 8, 10, 16 or 32")));
|
||||
}
|
||||
}
|
||||
"longEdge" | "shortEdge" | "width" | "height" | "megapixels" | "percent" => {
|
||||
num(k, v, 0.0, 1.0e9)?;
|
||||
}
|
||||
"resize" => {
|
||||
let Some(r) = v.as_object() else { return Err(bad(format!("`{k}` must be an object {{mode, value, height?, dontEnlarge?}}"))) };
|
||||
if let Some(x) = r.keys().find(|x| !matches!(x.as_str(), "mode" | "value" | "height" | "dontEnlarge")) {
|
||||
return Err(bad(unknown_key("resize key", x, &["mode", "value", "height", "dontEnlarge"])));
|
||||
}
|
||||
serde_json::from_value::<Resize>(v.clone()).map_err(|e| bad(format!("`{k}`: {e}")))?;
|
||||
}
|
||||
"dontEnlarge" | "removeLocation" | "background" => {
|
||||
boolean(k, v)?;
|
||||
}
|
||||
"naming" | "subfolder" | "path" | "dir" | "preset" => {
|
||||
string(cmd, k, v)?;
|
||||
}
|
||||
"sharpen" => one_of(k, v, &["none", "screen", "matte", "glossy"])?,
|
||||
"sharpenAmount" => one_of(k, v, &["low", "standard", "high"])?,
|
||||
"conflict" => one_of(k, v, &["unique", "overwrite", "skip"])?,
|
||||
"tiffCompression" => one_of(k, v, &["none", "lzw", "zip", "deflate"])?,
|
||||
"dngCompression" => {
|
||||
serde_json::from_value::<DngCompression>(v.clone())
|
||||
.map_err(|_| bad(format!("`{k}` must be one of lossless|deflate|uncompressed")))?;
|
||||
}
|
||||
"metadata" => one_of(k, v, &["all", "allExceptCamera", "copyright", "none"])?,
|
||||
"colorSpace" => {
|
||||
if OutputSpace::parse(string(cmd, k, v)?).is_none() {
|
||||
return Err(bad(format!("`{k}` must be one of srgb|displayP3|adobeRgb|proPhoto|rec2020")));
|
||||
}
|
||||
}
|
||||
"watermark" => Self::validate_watermark(cmd, v)?,
|
||||
"id" => {
|
||||
int(k, v, 0, i64::MAX)?;
|
||||
}
|
||||
"ids" => {
|
||||
let ok = v.as_array().is_some_and(|a| a.iter().all(|x| x.is_u64()));
|
||||
if !ok {
|
||||
return Err(bad(format!("`{k}` must be an array of photo ids")));
|
||||
}
|
||||
}
|
||||
_ => return Err(bad(unknown_key("parameter", k, &known))),
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// A `watermark` value: text, or an object of [`WATERMARK_PARAMS`] (issue #183).
|
||||
fn validate_watermark(cmd: &str, v: &serde_json::Value) -> crate::Result<()> {
|
||||
use serde_json::Value;
|
||||
let bad = |msg: String| crate::cmd::bad(cmd, msg);
|
||||
let obj = match v {
|
||||
Value::String(_) => return Ok(()),
|
||||
Value::Object(o) => o,
|
||||
_ => {
|
||||
return Err(bad(
|
||||
"`watermark` must be text or an object {text, vertical, size, opacity, anchor, inset, color, shadow, image, imageWidth}".into(),
|
||||
));
|
||||
}
|
||||
};
|
||||
let frac = |k: &str, v: &Value, (lo, hi): (f32, f32), unit: &str| -> crate::Result<()> {
|
||||
match v.as_f64() {
|
||||
Some(f) if f.is_finite() && (f64::from(lo)..=f64::from(hi)).contains(&f) => Ok(()),
|
||||
_ => Err(bad(format!("`watermark.{k}` must be a number {lo}..{hi} ({unit})"))),
|
||||
}
|
||||
};
|
||||
for (k, v) in obj {
|
||||
if v.is_null() {
|
||||
continue;
|
||||
}
|
||||
match k.as_str() {
|
||||
"text" | "image" => {
|
||||
v.as_str().ok_or_else(|| bad(format!("`watermark.{k}` must be a string")))?;
|
||||
}
|
||||
"vertical" | "shadow" => {
|
||||
v.as_bool().ok_or_else(|| bad(format!("`watermark.{k}` must be true or false")))?;
|
||||
}
|
||||
"size" => frac(k, v, WATERMARK_SIZE_RANGE, "fraction of the short edge")?,
|
||||
"opacity" => frac(k, v, (0.0, 1.0), "0 = invisible, 1 = solid")?,
|
||||
"inset" => frac(k, v, WATERMARK_INSET_RANGE, "fraction of the short edge")?,
|
||||
"imageWidth" => frac(k, v, WATERMARK_IMAGE_WIDTH_RANGE, "fraction of the photo's width")?,
|
||||
"anchor" => {
|
||||
serde_json::from_value::<Anchor>(v.clone()).map_err(|_| {
|
||||
bad("`watermark.anchor` must be one of topLeft|top|topRight|left|center|right|bottomLeft|bottom|bottomRight".into())
|
||||
})?;
|
||||
}
|
||||
"color" => {
|
||||
let ok = v.as_array().is_some_and(|a| a.len() == 3 && a.iter().all(|c| matches!(whole(c), Some(0..=255))));
|
||||
if !ok {
|
||||
return Err(bad("`watermark.color` must be [r, g, b] with 0..255 each (sRGB)".into()));
|
||||
}
|
||||
}
|
||||
_ => return Err(bad(unknown_key("watermark key", k, WATERMARK_PARAMS))),
|
||||
}
|
||||
}
|
||||
serde_json::from_value::<Watermark>(v.clone()).map(|_| ()).map_err(|e| bad(format!("`watermark`: {e}")))
|
||||
}
|
||||
|
||||
/// Read options from command params (`format`, `quality`, `limitKb`, `sharpen`, `sharpenAmount`,
|
||||
/// `naming`, `metadata`, `removeLocation`, `watermark`, `colorSpace`, `bitDepth`, `ppi`) and the
|
||||
/// size: one of `longEdge`, `shortEdge`, `width`, `height` (both = fit inside W × H),
|
||||
/// `megapixels`, `percent` (absent or 0 = full size), or a [`Resize`] object as `resize`; plus
|
||||
/// `dontEnlarge` (default true).
|
||||
///
|
||||
/// Lenient: anything unknown or unreadable falls back to the default, which suits stored
|
||||
/// prefs and dialog prefill. A command that is about to export uses
|
||||
/// [`ExportOptions::from_params`], which validates first.
|
||||
pub fn from_json(p: &serde_json::Value) -> Self {
|
||||
use serde_json::Value;
|
||||
let d = Self::default();
|
||||
@@ -1720,4 +1956,99 @@ mod tests {
|
||||
Err(e) => assert!(e.contains("not available"), "{e}"),
|
||||
}
|
||||
}
|
||||
|
||||
/// Issue #181: a command about to export refuses what `from_json` would silently default —
|
||||
/// unknown keys (naming the nearest known one) and values of the wrong kind or range.
|
||||
#[test]
|
||||
fn export_params_are_validated() {
|
||||
use serde_json::json;
|
||||
let err = |p: serde_json::Value| ExportOptions::from_params(&p).err().map(|e| e.to_string()).unwrap_or_default();
|
||||
let e = err(json!({"longEdgee": 400}));
|
||||
assert!(e.starts_with("invalid parameters for `app.export`: unknown parameter `longEdgee` (did you mean `longEdge`?)"), "{e}");
|
||||
assert!(err(json!({"qualty": 5})).contains("did you mean `quality`"));
|
||||
assert!(err(json!({"longEdge": "banana"})).contains("`longEdge` must be a number"));
|
||||
assert!(err(json!({"quality": 0})).contains("`quality` must be an integer 1..100"));
|
||||
assert!(err(json!({"quality": "5"})).contains("`quality` must be an integer"));
|
||||
assert!(err(json!({"quality": 92.5})).contains("`quality` must be an integer"));
|
||||
assert!(err(json!({"sharpen": "lots"})).contains("`sharpen` must be one of none|screen|matte|glossy"));
|
||||
assert!(err(json!({"format": "bmp"})).contains("`format` must be one of"));
|
||||
assert!(err(json!({"bitDepth": 12})).contains("`bitDepth` must be 8, 10, 16 or 32"));
|
||||
assert!(err(json!({"colorSpace": "cmyk"})).contains("`colorSpace` must be one of"));
|
||||
assert!(err(json!({"dontEnlarge": "no"})).contains("`dontEnlarge` must be true or false"));
|
||||
assert!(err(json!({"resize": {"mode": "longEdge", "valeu": 10}})).contains("did you mean `value`"));
|
||||
assert!(err(json!({"resize": {"mode": "diagonal", "value": 10}})).contains("`resize`:"));
|
||||
assert!(err(json!({"ids": [1, "x"]})).contains("`ids` must be an array of photo ids"));
|
||||
assert!(err(json!("jpeg")).contains("params must be an object"));
|
||||
assert!(
|
||||
ExportOptions::validate("export.savePreset", &json!({"qualty": 5}))
|
||||
.unwrap_err()
|
||||
.to_string()
|
||||
.starts_with("invalid parameters for `export.savePreset`")
|
||||
);
|
||||
// everything the presets, the dialog, Export with Previous and `render --opt` send passes
|
||||
for p in builtin_presets() {
|
||||
ExportOptions::from_params(&p.params).unwrap_or_else(|e| panic!("{}: {e}", p.name));
|
||||
}
|
||||
let full = ExportOptions {
|
||||
resize: Some(Resize::default()),
|
||||
limit_kb: Some(500),
|
||||
watermark: Some(Watermark { text: "©".into(), ..Default::default() }),
|
||||
bit_depth: Some(16),
|
||||
..Default::default()
|
||||
};
|
||||
let mut j = full.to_json();
|
||||
for (k, v) in [
|
||||
("dir", json!("/tmp/out")),
|
||||
("background", json!(true)),
|
||||
("preset", json!("x")),
|
||||
("ids", json!([1, 2])),
|
||||
("id", json!(1)),
|
||||
("path", json!("a.jpg")),
|
||||
] {
|
||||
j[k] = v;
|
||||
}
|
||||
assert_eq!(ExportOptions::from_params(&j).unwrap(), full);
|
||||
assert_eq!(ExportOptions::from_params(&ExportOptions::default().to_json()).unwrap(), ExportOptions::default());
|
||||
// null is "not given", as a dropped key is
|
||||
ExportOptions::from_params(&json!({"quality": 92, "longEdge": null, "watermark": null})).unwrap();
|
||||
}
|
||||
|
||||
/// Issue #183: the `watermark` object's keys, kinds and ranges are checked (and documented in
|
||||
/// the `app.export` param string), instead of unknown keys passing and sizes clamping silently.
|
||||
#[test]
|
||||
fn watermark_params_are_validated() {
|
||||
use serde_json::json;
|
||||
let err = |w: serde_json::Value| ExportOptions::from_params(&json!({"watermark": w})).err().map(|e| e.to_string()).unwrap_or_default();
|
||||
assert!(
|
||||
err(json!(5))
|
||||
.contains("`watermark` must be text or an object {text, vertical, size, opacity, anchor, inset, color, shadow, image, imageWidth}")
|
||||
);
|
||||
assert!(
|
||||
err(json!({"text": "x", "bogus": 1}))
|
||||
.contains("unknown watermark key `bogus` (one of text, vertical, size, opacity, anchor, inset, color, shadow, image, imageWidth)")
|
||||
);
|
||||
assert!(err(json!({"text": "x", "sizee": 0.1})).contains("did you mean `size`"));
|
||||
for size in [json!(3), json!(0.5001), json!(0), json!("big")] {
|
||||
let e = err(json!({"text": "x", "size": size}));
|
||||
assert!(e.contains("`watermark.size` must be a number 0.005..0.5 (fraction of the short edge)"), "{size}: {e}");
|
||||
}
|
||||
assert!(err(json!({"text": "x", "opacity": 1.5})).contains("`watermark.opacity` must be a number 0..1"));
|
||||
assert!(err(json!({"text": "x", "inset": -0.1})).contains("`watermark.inset` must be a number 0..0.4"));
|
||||
assert!(err(json!({"image": "logo.png", "imageWidth": 2})).contains("`watermark.imageWidth` must be a number 0.01..1"));
|
||||
assert!(err(json!({"text": "x", "anchor": "middle"})).contains("`watermark.anchor` must be one of"));
|
||||
assert!(err(json!({"text": "x", "color": [255, 255]})).contains("`watermark.color` must be [r, g, b]"));
|
||||
assert!(err(json!({"text": "x", "color": [255, 255, 256]})).contains("`watermark.color`"));
|
||||
assert!(err(json!({"text": "x", "vertical": "yes"})).contains("`watermark.vertical` must be true or false"));
|
||||
assert!(err(json!({"text": 7})).contains("`watermark.text` must be a string"));
|
||||
// the documented shapes pass and read as before
|
||||
let o = ExportOptions::from_params(&json!({"watermark": "© me"})).unwrap();
|
||||
assert_eq!(o.watermark.map(|w| w.text).as_deref(), Some("© me"));
|
||||
let w = json!({"text": "日本語", "vertical": true, "size": 0.1, "opacity": 0.5, "anchor": "topLeft", "inset": 0.05, "color": [0, 0, 0], "shadow": false, "image": "", "imageWidth": 0.3});
|
||||
let wm = ExportOptions::from_params(&json!({"watermark": w})).unwrap().watermark.unwrap();
|
||||
assert!((wm.size - 0.1).abs() < 1e-6 && wm.vertical && wm.anchor == Anchor::TopLeft && wm.color == [0, 0, 0] && !wm.shadow);
|
||||
assert_eq!(ExportOptions::from_params(&json!({"watermark": {"text": "x", "size": 0.5}})).unwrap().watermark.map(|w| w.size), Some(0.5));
|
||||
// the Export dialog's sliders (1–15 % size, 5–100 % opacity, 2–100 % width) stay inside the accepted ranges
|
||||
assert!(WATERMARK_SIZE_RANGE.0 <= 0.01 && WATERMARK_SIZE_RANGE.1 >= 0.15);
|
||||
assert!(WATERMARK_IMAGE_WIDTH_RANGE.0 <= 0.02 && WATERMARK_IMAGE_WIDTH_RANGE.1 >= 1.0);
|
||||
}
|
||||
}
|
||||
@@ -61,7 +61,7 @@ impl Headless {
|
||||
fn export(&mut self, p: &Value) -> Result<Value, String> {
|
||||
use lightcraft_engine::export::{Destination, ExportFormat, ExportOptions, Resize, export_batch};
|
||||
let p = &self.session.export_params(p)?;
|
||||
let mut opts = ExportOptions::from_json(p);
|
||||
let mut opts = ExportOptions::from_params(p).map_err(|e| e.to_string())?;
|
||||
if !ExportOptions::has_size_param(p) {
|
||||
opts.resize = Some(Resize::long_edge(3000));
|
||||
}
|
||||
@@ -107,7 +107,7 @@ impl Backend for Headless {
|
||||
"engine.commands" => {
|
||||
let mut v: Vec<Value> = self.session.commands().into_iter().map(|c| serde_json::to_value(c).unwrap_or_default()).collect();
|
||||
v.push(json!({"id": "app.export", "label": "Export Now", "menu": [], "shortcut": null,
|
||||
"params": "{path?: output file (.jpg/.png/.tif/.webp/.avif/.dng) | dir?, ids?, format?: jpeg|png|tiff|webp|avif|dng|original, longEdge?|shortEdge?|width?|height?|megapixels?|percent? (default longEdge 3000; longEdge 0 = full size), dontEnlarge?, ppi?, quality?, limitKb?, colorSpace?, bitDepth?, sharpen?, metadata?, watermark?, naming?}",
|
||||
"params": "{path?: output file (.jpg/.png/.tif/.webp/.avif/.dng) | dir?, ids?, preset?, format?: jpeg|png|tiff|webp|avif|dng|original, longEdge?|shortEdge?|width?|height?|megapixels?|percent? (default longEdge 3000; longEdge 0 = full size), dontEnlarge?, ppi?, quality?: 1..100, limitKb?, colorSpace?: srgb|displayP3|adobeRgb|proPhoto|rec2020, bitDepth?: 8|10|16|32, sharpen?: none|screen|matte|glossy, sharpenAmount?: low|standard|high, metadata?: all|allExceptCamera|copyright|none, removeLocation?, naming?, startNumber?, subfolder?, conflict?: unique|overwrite|skip, tiffCompression?: none|lzw|zip, dngCompression?: lossless|deflate|uncompressed, watermark?: text | {text?, vertical? (upright columns, right to left), size? (text height, 0.005..0.5 of the short edge; default 0.035), opacity? (0..1; 0.7), anchor?: topLeft|top|topRight|left|center|right|bottomLeft|bottom|bottomRight, inset? (margin, 0..0.4 of the short edge; 0.025), color? [r,g,b] sRGB, shadow?, image? (graphic drawn instead of the text), imageWidth? (0.01..1 of the photo's width; 0.2)}} — an unknown parameter, an out-of-range watermark size or a value of the wrong kind is an error, not a default",
|
||||
"enabled": self.session.active().is_some()}));
|
||||
Ok(Value::Array(v))
|
||||
}
|
||||
|
||||
@@ -120,6 +120,25 @@ fn path_writes_never_replace_an_original() {
|
||||
let _ = std::fs::remove_dir_all(&base);
|
||||
}
|
||||
|
||||
/// Issue #181: a misspelled or unreadable `app.export` param is an error through the headless
|
||||
/// backend (what `lightcraft-cli run` and the MCP `export` tool use), not a silent default.
|
||||
#[test]
|
||||
fn export_refuses_unknown_params_and_bad_values() {
|
||||
let mut b = Headless::demo();
|
||||
let out = "/nonexistent-lc-test/x.jpg";
|
||||
let e = b.call("app.export", json!({"path": out, "longEdgee": 400})).unwrap_err();
|
||||
assert!(e.contains("unknown parameter `longEdgee` (did you mean `longEdge`?)"), "{e}");
|
||||
let e = b.call("app.export", json!({"path": out, "longEdge": "banana"})).unwrap_err();
|
||||
assert!(e.contains("`longEdge` must be a number"), "{e}");
|
||||
let e = b.call("app.export", json!({"path": out, "watermark": {"text": "x", "size": 3}})).unwrap_err();
|
||||
assert!(e.contains("`watermark.size` must be a number 0.005..0.5"), "{e}");
|
||||
// `export` tool and preset expansion go through the same check
|
||||
let r = call_tool(&mut b, "export", &json!({"path": out, "quality": 101}));
|
||||
assert!(r.is_error, "{r:?}");
|
||||
let e = b.session.execute("export.savePreset", &json!({"name": "Typo", "params": {"qualty": 5}})).unwrap_err().to_string();
|
||||
assert!(e.contains("did you mean `quality`"), "{e}");
|
||||
}
|
||||
|
||||
/// The `import` helper moves: renamed into the folder template, the source removed.
|
||||
#[test]
|
||||
fn import_tool_moves_with_a_folder_template() {
|
||||
|
||||
@@ -205,7 +205,7 @@ pub fn helper_tools(has_ui: bool) -> Vec<Value> {
|
||||
"removeLocation": {"type": "boolean"},
|
||||
"colorSpace": {"type": "string", "enum": ["srgb", "displayP3", "adobeRgb", "proPhoto", "rec2020"], "description": "Output colour space (default sRGB; AVIF is always sRGB). adobeRgb = Adobe RGB (1998) compatible; the embedded ICC profile is generated from the published primaries"},
|
||||
"bitDepth": {"type": "integer", "enum": [8, 10, 16, 32], "description": "Bits per channel: PNG 8|16 (default 8), TIFF 8|16|32 (default 16; 32 = linear float with a linear profile), AVIF 8|10; JPEG/WebP are 8-bit"},
|
||||
"watermark": {"description": "Text, or {text, vertical (boolean; defaults to false), size (fraction of short edge), opacity, anchor (topLeft|top|topRight|left|center|right|bottomLeft|bottom|bottomRight), inset, color [r,g,b], shadow}"}
|
||||
"watermark": {"description": "Text, or {text, vertical (boolean; defaults to false: upright columns right to left), size (text height as a fraction of the short edge, 0.005..0.5; default 0.035), opacity (0..1; default 0.7), anchor (topLeft|top|topRight|left|center|right|bottomLeft|bottom|bottomRight), inset (margin as a fraction of the short edge, 0..0.4; default 0.025), color [r,g,b] (sRGB 0..255), shadow (boolean), image (path of a graphic drawn instead of the text), imageWidth (fraction of the photo's width, 0.01..1; default 0.2)}. Unknown keys and out-of-range sizes are errors"}
|
||||
}),
|
||||
&[],
|
||||
),
|
||||
|
||||
@@ -352,7 +352,7 @@ pub fn default_export_dir() -> String {
|
||||
pub fn export_active(app: &mut LightcraftApp, p: &Value) -> Result<Value, String> {
|
||||
use lightcraft_engine::export::{Destination, ExportOptions, export_batch};
|
||||
let p = &app.session.export_params(p)?;
|
||||
let mut opts = ExportOptions::from_json(p);
|
||||
let mut opts = ExportOptions::from_params(p).map_err(|e| e.to_string())?;
|
||||
if let (Some(path), None) = (p.get("path").and_then(Value::as_str), p.get("format")) {
|
||||
let ext = path.rsplit_once('.').map_or("", |(_, e)| e);
|
||||
opts.format = lightcraft_engine::export::ExportFormat::parse(ext).unwrap_or(opts.format);
|
||||
|
||||
+1
-1
@@ -434,7 +434,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins.
|
||||
| LR-EXP-HDR | HDR output | P2 | ⬜ | | |
|
||||
| LR-EXP-SHARPEN | Output sharpening | P1 | ✅ | `cmd:app.export` (`sharpen`, `sharpenAmount`) | |
|
||||
| LR-EXP-METADATA | Metadata policy | P1 | ✅ | `cmd:app.export` (`metadata`, `removeLocation`) | |
|
||||
| LR-EXP-WATERMARK | Watermark | P1 | ✅ | `cmd:app.export` (`watermark`: text or `image` + `imageWidth`), `crates/engine/src/export.rs` (`Watermark`) | text (size, colour, shadow) or a graphic with transparency (width as % of the photo, converted to the output colour space); position, inset, opacity |
|
||||
| LR-EXP-WATERMARK | Watermark | P1 | ✅ | `cmd:app.export` (`watermark`: text, or `{text, vertical, size, opacity, anchor, inset, color, shadow, image, imageWidth}`), `crates/engine/src/export.rs` (`Watermark`, `WATERMARK_PARAMS`) | text (`size` = height as a fraction of the short edge, 0.005..0.5, default 0.035; `color` sRGB `[r,g,b]`; `shadow`) or a graphic with transparency (`image` path, `imageWidth` = fraction of the photo's width, 0.01..1, converted to the output colour space); `anchor`, `inset` (fraction of the short edge, 0..0.4), `opacity` 0..1. Unknown keys and out-of-range values are errors (issue #183) |
|
||||
| LR-EXP-NAMING | File naming | P1 | ✅ | `cmd:app.export` (`naming`, `startNumber`), `crates/engine/src/rename.rs` (`expand_tokens`) | one template language for export, Rename Photos and import renaming: `{name}`, `{num}` (number at the end of the name), `{seq}` / `{seq:N}`, `{date}` / `{date:%Y-%m-%d}`, `{folder}`, `{camera}`, `{lens}`, `{iso}`, `{rating}`, `{title}`, `{creator}`, `{ext}`; free-form template rather than a list of named schemes |
|
||||
| LR-EXP-LOCATION | Destination folder | P0 | ✅ | `cmd:app.export` (`dir`, `subfolder`, `conflict`: unique / overwrite / skip), `cmd:export.checkTarget`, `crates/ui-egui/src/panels/dialogs.rs` (Choose… folder picker), `crates/engine/src/originals.rs`, `crates/catalog/src/safe_file.rs` | folder picker on the desktop; existing files get `-2`, `-3`… by default; the policy covers a file and its sidecars together; a catalogued original (or its sidecar) is never written over, whatever the policy or an exact `path` (export, `ui.render`, MCP `render_photo`, merge `previewPath`, `lightcraft-cli render`); files are written to a temp file and renamed into place (issue #93; not synced file by file — exports can be made again, issue #134) |
|
||||
| LR-EXP-PREVIOUS | Export with previous settings | P0 | ✅ | `cmd:app.exportPrevious`, `cmd:dialog.export` | last options persist in prefs.json; dialog prefilled; no named export presets yet |
|
||||
|
||||
Reference in new issue
Block a user