Integration: write a shift tool switch change made before the first save (#5878)

When the file on disk is already version 2 (another instance saved the
migrated "on"), the save base took this instance's current value, so
turning the switch off here before the first save matched the base and
was never written; the file kept "on". The base now holds the migrated
value (on), so only a change made here since loading is written.

Test: turning_the_shift_tool_switch_off_is_saved_over_another_instances_migration
(fails with the previous base: true kept instead of false).

Co-authored-by: Higgs32584 <55243596+Higgs32584@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
David Mann
2026-10-22 21:57:43 +02:00
co-authored by Higgs32584 Claude Opus 5.5
parent 6bb15d2278
commit dc180cd891
+27 -5
View File
@@ -618,7 +618,7 @@ fn write_prefs(app: &mut PhotocraftApp, ours: &Value) -> Result<(), String> {
let stored = app.services.load_prefs.as_mut().and_then(|load| load()).and_then(|t| serde_json::from_str::<Value>(&t).ok());
let text = match (stored, &app.prefs_rt.saved_value) {
(Some(mut theirs @ Value::Object(_)), Some(base)) => {
let base = &without_stale_migration(base, ours, &theirs);
let base = &without_stale_migration(base, &theirs);
merge_changes(base, ours, &mut theirs);
serde_json::to_string_pretty(&theirs)
}
@@ -630,14 +630,17 @@ fn write_prefs(app: &mut PhotocraftApp, ours: &Value) -> Result<(), String> {
/// The save base without the shift tool switch migration (#5647) once storage already holds a
/// version 2 file: another instance wrote it, so its Use Shift Key for Tool Switch is a choice,
/// not the old default this instance migrated, and must not be overwritten by the migration.
fn without_stale_migration(base: &Value, ours: &Value, theirs: &Value) -> Value {
/// not the old default this instance migrated. The base then holds the migrated value (on) instead
/// of the old file's off, so only a change made here since loading is written over that choice.
fn without_stale_migration(base: &Value, theirs: &Value) -> Value {
let mut base = base.clone();
let new_file = |v: &Value| v.get("version").and_then(Value::as_u64).is_some_and(|v| v >= 2);
if new_file(theirs) && !new_file(&base) {
let key = "useShiftKeyForToolSwitch";
if let (Some(o), Some(tools)) = (ours.pointer("/tools").and_then(|t| t.get(key)), base.get_mut("tools").and_then(Value::as_object_mut)) {
tools.insert(key.into(), o.clone());
if let Some(tools) = base.get_mut("tools").and_then(Value::as_object_mut)
&& tools.get(key) == Some(&Value::Bool(false))
{
tools.insert(key.into(), Value::Bool(true));
}
}
base
@@ -3312,6 +3315,25 @@ mod tests {
assert_eq!(v["version"], json!(prefs::PREFS_VERSION));
}
#[test]
fn turning_the_shift_tool_switch_off_is_saved_over_another_instances_migration() {
// Two instances migrate the same version 1 file. B saves version 2 with the migrated "on";
// the user then turns the switch off in A, which must be written.
let saved = json!({"version": 1, "tools": {"useShiftKeyForToolSwitch": false}});
let store = Arc::new(Mutex::new(Some(saved.to_string())));
let ctx = egui::Context::default();
let (mut a, mut b) = (app_on(&store, Arc::default()), app_on(&store, Arc::default()));
tick(&mut a, &ctx);
tick(&mut b, &ctx);
b.run("prefs.set", json!({"values": {"performance.historyStates": 12}})).unwrap();
tick(&mut b, &ctx);
assert_eq!(stored(&store)["tools"]["useShiftKeyForToolSwitch"], true);
a.run("prefs.set", json!({"values": {"tools.useShiftKeyForToolSwitch": false}})).unwrap();
tick(&mut a, &ctx);
let v = stored(&store);
assert_eq!((v["tools"]["useShiftKeyForToolSwitch"].as_bool(), v["performance"]["historyStates"].as_u64()), (Some(false), Some(12)));
}
#[test]
fn a_newer_files_version_is_kept_on_save() {
let saved = json!({"version": prefs::PREFS_VERSION + 1, "tools": {"useShiftKeyForToolSwitch": false}});