mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-27 11:37:13 -06:00
Issue #181: saved presets and Export with Previous drop keys this version doesn't know
#287 made app.export refuse unknown params. Stored settings (a user export preset, prefs.json lastExport) written by another version are not the caller's typo, and refusing them would break that preset or Export with Previous for good after an update: their unknown keys (also inside watermark) are dropped before the strict check. What the caller passes is still checked strictly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01912WaK6Rmew3wxZuPyX7E8
This commit is contained in:
@@ -23,7 +23,8 @@ impl Session {
|
||||
.into_iter()
|
||||
.find(|(x, _)| x.name.eq_ignore_ascii_case(name.trim()))
|
||||
.ok_or_else(|| format!("unknown export preset `{name}` (see export.presets)"))?;
|
||||
let mut out = preset.params;
|
||||
// a preset saved by another version may hold keys this one doesn't know: not the caller's typo
|
||||
let mut out = crate::export::ExportOptions::known_keys_only(&preset.params);
|
||||
if let (Some(o), Some(own)) = (out.as_object_mut(), p.as_object()) {
|
||||
for (k, v) in own.iter().filter(|(k, _)| *k != "preset") {
|
||||
o.insert(k.clone(), v.clone());
|
||||
|
||||
@@ -688,6 +688,22 @@ fn whole(v: &serde_json::Value) -> Option<i64> {
|
||||
}
|
||||
|
||||
impl ExportOptions {
|
||||
/// Stored export settings (a saved preset, `prefs.json ▸ lastExport`) with the keys an older or
|
||||
/// newer version may have written and this one doesn't know dropped, also inside `watermark`.
|
||||
/// [`ExportOptions::validate`] is strict about what a caller types; settings saved on disk are
|
||||
/// not the caller's typo, and refusing them would make a preset or Export with Previous fail
|
||||
/// for good after an update.
|
||||
pub fn known_keys_only(p: &serde_json::Value) -> serde_json::Value {
|
||||
let mut p = p.clone();
|
||||
if let Some(o) = p.as_object_mut() {
|
||||
o.retain(|k, _| OPTION_PARAMS.contains(&k.as_str()) || TARGET_PARAMS.contains(&k.as_str()));
|
||||
if let Some(w) = o.get_mut("watermark").and_then(serde_json::Value::as_object_mut) {
|
||||
w.retain(|k, _| WATERMARK_PARAMS.contains(&k.as_str()));
|
||||
}
|
||||
}
|
||||
p
|
||||
}
|
||||
|
||||
/// 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`)
|
||||
|
||||
@@ -850,3 +850,20 @@ fn selecting_an_unknown_photo_is_an_error_and_keeps_the_selection() {
|
||||
// an empty selection is still allowed
|
||||
assert_eq!(s.execute("library.select", &json!({"ids": []})).unwrap()["selected"], 0);
|
||||
}
|
||||
|
||||
/// A preset saved by another version may hold keys this one doesn't know: exporting with it keeps
|
||||
/// working (the stale keys are dropped), while a typo the caller makes is still refused (#181).
|
||||
#[test]
|
||||
fn saved_presets_with_unknown_keys_still_export() {
|
||||
use crate::export::{ExportOptions, ExportPreset};
|
||||
let mut s = demo();
|
||||
s.export_presets.push(ExportPreset {
|
||||
name: "Old".into(),
|
||||
params: json!({"format": "png", "retiredOption": 3, "watermark": {"text": "©", "retiredKey": true}}),
|
||||
});
|
||||
let p = s.export_params(&json!({"preset": "old", "quality": 80})).unwrap();
|
||||
let o = ExportOptions::from_params(&p).unwrap();
|
||||
assert_eq!(o.format, crate::export::ExportFormat::Png);
|
||||
assert_eq!(o.watermark.map(|w| w.text), Some("©".to_string()));
|
||||
assert!(ExportOptions::from_params(&s.export_params(&json!({"preset": "old", "qualty": 80})).unwrap()).is_err());
|
||||
}
|
||||
|
||||
@@ -1240,7 +1240,7 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option<Re
|
||||
crate::links::open(app, url)
|
||||
}
|
||||
"app.exportPrevious" => match app.session.last_export.clone() {
|
||||
Some(prev) => crate::control::export_active(app, &prev),
|
||||
Some(prev) => crate::control::export_active(app, &lightcraft_engine::export::ExportOptions::known_keys_only(&prev)),
|
||||
None => Err("nothing exported yet — use Export…".into()),
|
||||
},
|
||||
_ => return None,
|
||||
|
||||
Reference in New Issue
Block a user