Merge pull request #6482 from dotCooCoo/fix/prefs-reset-keeps-document-units

Preferences: Reset Preferences leaves the open document's units alone

crates/engine/src/cmd/prefscmds.rs

@@ -5,7 +5,7 @@
 
 use serde_json::{Map, Value, json};
 
-use super::{CommandSpec, EngineError, Result, Session, always, bad, cmd, str_param};
+use super::{CommandSpec, EngineError, Result, Session, always, bad, cmd, str_param, switch_or};
 use crate::Prefs;
 use crate::units::Measure;
 use vectorcraft_doc::{GridPrefs, Unit};
@@ -763,7 +763,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Set Preference",
             [],
             None,
-            "{key, value} or {values: {key: value, …}} set preferences (validated; see prefs.list; lengths in pt or strings with a unit; unitsGeneral also sets the active document's units, one undo step; the eyedropper group takes a partial object, as eyedropper.setOptions; a new recoveryFolder must be a folder that can be listed and written in, and a new pluginsFolder a folder that can be listed: otherwise an error, and nothing changes; an empty value is not checked)",
+            "{key, value} or {values: {key: value, …}, documentUnits?: bool=true} set preferences (validated; see prefs.list; lengths in pt or strings with a unit; unitsGeneral also sets the active document's units, one undo step, unless documentUnits is false; the eyedropper group takes a partial object, as eyedropper.setOptions; a new recoveryFolder must be a folder that can be listed and written in, and a new pluginsFolder a folder that can be listed: otherwise an error, and nothing changes; an empty value is not checked)",
             always,
             set
         ),
@@ -789,6 +789,7 @@ fn set(s: &mut Session, p: &Value) -> Result<Value> {
     if values.is_empty() {
         return Err(bad("prefs.set", "give {key, value} or {values}"));
     }
+    let document_units = switch_or(p, "documentUnits", true, "prefs.set")?;
     let mut next = s.prefs.clone();
     next.set_values(&values).map_err(|e| bad("prefs.set", e))?;
     if let Some(e) = s.prefs.keep_folders(&mut next).into_iter().next() {
@@ -797,7 +798,7 @@ fn set(s: &mut Session, p: &Value) -> Result<Value> {
     s.prefs.check_folders(&values).map_err(EngineError::Other)?;
     s.apply_prefs(next);
     // Units ▸ General is also the open document's units (as Document Setup sets them).
-    if values.contains_key("unitsGeneral") && s.active().is_some() {
+    if document_units && values.contains_key("unitsGeneral") && s.active().is_some() {
         s.set_document_units(s.default_units())?;
     }
     let all = s.prefs.to_json();

crates/engine/src/tests/units.rs

@@ -36,6 +36,19 @@ fn preferences_general_sets_the_open_documents_units_as_one_undo_step() {
     assert_eq!(s.prefs.units_general, "millimeters", "undo is the document's; the preference stays");
 }
 
+#[test]
+fn preferences_general_with_document_units_false_sets_only_the_preference() {
+    let mut s = session();
+    s.execute("document.setUnits", &json!({"units": "Millimeters"})).unwrap();
+    let undo = s.doc().unwrap().history.undo.len();
+    s.execute("prefs.set", &json!({"values": {"unitsGeneral": "inches"}, "documentUnits": false})).unwrap();
+    assert_eq!((units(&s), s.prefs.units_general.as_str()), (Unit::Millimeters, "inches"));
+    assert_eq!(s.doc().unwrap().history.undo.len(), undo);
+    let e = s.execute("prefs.set", &json!({"key": "unitsGeneral", "value": "points", "documentUnits": "no"})).unwrap_err();
+    assert!(e.to_string().contains("`documentUnits` must be true or false"), "{e}");
+    assert_eq!((units(&s), s.prefs.units_general.as_str()), (Unit::Millimeters, "inches"), "nothing changes on error");
+}
+
 #[test]
 fn new_documents_start_in_the_preference_and_document_setup_only_changes_the_document() {
     let mut s = Session::new();

crates/ui-egui/src/dialogs.rs

@@ -504,6 +504,9 @@ pub fn set_field(d: &mut Dialog, field: String, value: Value) {
     if d.kind == layer_options::KIND {
         layer_options::note_set(d, &field, &value);
     }
+    if d.kind == "preferences" {
+        crate::prefs_dialog::note_set(d, &field, &value);
+    }
     let dxf = (d.kind == dxf_options::KIND).then(|| field.clone());
     d.fields.insert(field, value);
     if let Some(field) = dxf {

crates/ui-egui/src/prefs_dialog.rs

@@ -19,6 +19,9 @@ pub const UI_FIELDS: &[(&str, &str, &str)] = &[
     ("__taskBar", "General", "Enable Contextual Task Bar"),
 ];
 
+/// The Units ▸ General value Reset Preferences put in the dialog, while it is still the field's value.
+const RESET_UNITS: &str = "__resetUnits";
+
 /// Whether User Interface shows VectorCraft Title Bar on Every Desktop (`appTitleBar`): Linux and the
 /// BSDs, where the desktop may draw the window's frame. macOS and Windows never read it.
 const APP_TITLE_BAR_SHOWN: bool = cfg!(not(any(target_arch = "wasm32", target_os = "macos", target_os = "windows")));
@@ -194,12 +197,21 @@ fn found_paint(ui: &mut egui::Ui, d: &mut Dialog, mark: Option<(egui::layers::Sh
     }
 }
 
+/// `field` of the Preferences dialog `d` is about to be `value` (a frame's edit or `ui.dialog.set`):
+/// choosing another General unit after Reset Preferences makes OK set the document's units again.
+pub(crate) fn note_set(d: &mut Dialog, field: &str, value: &Value) {
+    if field == "unitsGeneral" && d.fields.get(RESET_UNITS).is_some_and(|v| v != value) {
+        d.fields.remove(RESET_UNITS);
+    }
+}
+
 /// OK: validate and apply every preference in the dialog (nothing changes on error).
 pub fn confirm(app: &mut VectorcraftApp) -> Result<Value, String> {
     let Some(d) = app.ui.dialog.clone() else { return Err("no dialog open".into()) };
     let values: Map<String, Value> = d.fields.iter().filter(|(k, _)| !k.starts_with("__")).map(|(k, v)| (k.clone(), v.clone())).collect();
     let touch_ws = values.get("touchWorkspace").and_then(Value::as_bool);
-    app.run("prefs.set", json!({ "values": values }))?;
+    let document_units = d.fields.get(RESET_UNITS).is_none_or(|v| values.get("unitsGeneral") != Some(v));
+    app.run("prefs.set", json!({ "values": values, "documentUnits": document_units }))?;
     if touch_ws.is_some() {
         crate::workspaces::sync_touch_workspace(app, app.touch_screen);
     }
@@ -418,6 +430,11 @@ pub fn show(app: &mut VectorcraftApp, ctx: &egui::Context) {
         for (k, v) in Prefs::default().to_json().as_object().cloned().unwrap_or_default() {
             d.fields.insert(k, v);
         }
+        // OK resets the Units ▸ General preference and leaves the open document's units alone.
+        d.fields.insert(RESET_UNITS.into(), d.fields.get("unitsGeneral").cloned().unwrap_or(Value::Null));
+    }
+    if let Some(v) = d.fields.get("unitsGeneral").cloned() {
+        note_set(&mut d, "unitsGeneral", &v);
     }
     // The setting a search went to stays highlighted until another category is shown.
     let found = d.str("__found");

crates/ui-egui/src/prefs_dialog/tests.rs

@@ -273,6 +273,102 @@ fn preferences_ok_changes_the_guide_lock_only_with_the_option() {
     assert!(!a.session.guides_locked(), "the option went off");
 }
 
+/// Click the Preferences dialog's button `label` with the pointer.
+fn click_button(app: &mut VectorcraftApp, label: &str) {
+    let ctx = egui::Context::default();
+    crate::theme::install_fonts(&ctx);
+    let frame = |app: &mut VectorcraftApp, events: Vec<egui::Event>| {
+        // A window takes a sizing frame and fades in: each frame is a quarter second on.
+        let time = ctx.input(|i| i.time) + 0.25;
+        let screen_rect = Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1200.0, 900.0)));
+        let raw = egui::RawInput { screen_rect, events, time: Some(time), ..Default::default() };
+        let mut out = ctx.run_ui(raw, |ui| show(app, ui.ctx()));
+        out.textures_delta.clear();
+        fn walk(s: &egui::Shape, v: &mut Vec<(String, egui::Rect)>) {
+            match s {
+                egui::Shape::Text(t) => v.push((t.galley.text().to_string(), egui::Rect::from_min_size(t.pos, t.galley.size()))),
+                egui::Shape::Vec(s) => s.iter().for_each(|s| walk(s, v)),
+                _ => {}
+            }
+        }
+        let mut v = vec![];
+        out.shapes.iter().for_each(|c| walk(&c.shape, &mut v));
+        v
+    };
+    frame(app, vec![]);
+    let texts = frame(app, vec![]);
+    let at = texts.iter().find(|(t, _)| t == label).map(|(_, r)| r.center()).unwrap_or_else(|| panic!("no `{label}` in {texts:?}"));
+    let press = |pressed| egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() };
+    frame(app, vec![egui::Event::PointerMoved(at)]);
+    frame(app, vec![press(true)]);
+    frame(app, vec![press(false)]);
+}
+
+/// Reset Preferences then OK resets the preferences, General included, and leaves the open
+/// document's units, with no undo step in the document.
+#[test]
+fn reset_preferences_then_ok_leaves_the_documents_units() {
+    let mut a = app();
+    a.run("file.new", json!({"width": 400, "height": 400})).unwrap();
+    a.run("document.setUnits", json!({"units": "Millimeters"})).unwrap();
+    a.run("prefs.set", json!({"key": "keyboardIncrement", "value": 5})).unwrap();
+    let undo = a.session.active().unwrap().history.undo.len();
+    open(&mut a, Some("General"));
+    click_button(&mut a, "Reset Preferences");
+    let d = a.ui.dialog.as_ref().expect("Reset leaves the dialog open");
+    assert_eq!(d.fields["keyboardIncrement"], Prefs::default().to_json()["keyboardIncrement"], "the working copy holds the defaults");
+    assert_eq!(d.fields["unitsGeneral"], json!("points"));
+    confirm(&mut a).unwrap();
+    assert_eq!(a.session.prefs.keyboard_increment, Prefs::default().keyboard_increment);
+    assert_eq!(a.session.prefs.units_general, Prefs::default().units_general);
+    let st = a.session.active().unwrap();
+    assert_eq!(st.doc.units, vectorcraft_doc::Unit::Millimeters);
+    assert_eq!(st.history.undo.len(), undo, "no undo step");
+}
+
+/// After Reset Preferences, choosing a General unit in the dialog still sets the document's units
+/// on OK, Points included once another unit was chosen in between. With no document open, Reset
+/// Preferences resets General to its default.
+#[test]
+fn reset_preferences_keeps_an_edit_of_general_units() {
+    let mut a = app();
+    a.run("file.new", json!({"width": 400, "height": 400})).unwrap();
+    a.run("document.setUnits", json!({"units": "Millimeters"})).unwrap();
+    open(&mut a, Some("General"));
+    click_button(&mut a, "Reset Preferences");
+    a.ui.dialog.as_mut().unwrap().fields.insert("unitsGeneral".into(), json!("inches"));
+    confirm(&mut a).unwrap();
+    assert_eq!(a.session.active().unwrap().doc.units, vectorcraft_doc::Unit::Inches);
+    assert_eq!(a.session.prefs.units_general, "inches");
+
+    // Between frames, as `ui.dialog.set` sets fields.
+    a.run("document.setUnits", json!({"units": "Millimeters"})).unwrap();
+    open(&mut a, Some("General"));
+    click_button(&mut a, "Reset Preferences");
+    for u in ["inches", "points"] {
+        crate::dialogs::set_field(a.ui.dialog.as_mut().unwrap(), "unitsGeneral".into(), json!(u));
+    }
+    confirm(&mut a).unwrap();
+    assert_eq!(a.session.active().unwrap().doc.units, vectorcraft_doc::Unit::Points);
+
+    // Through a frame of the dialog.
+    a.run("document.setUnits", json!({"units": "Millimeters"})).unwrap();
+    open(&mut a, Some("General"));
+    click_button(&mut a, "Reset Preferences");
+    a.ui.dialog.as_mut().unwrap().fields.insert("unitsGeneral".into(), json!("inches"));
+    click_button(&mut a, "General");
+    a.ui.dialog.as_mut().unwrap().fields.insert("unitsGeneral".into(), json!("points"));
+    confirm(&mut a).unwrap();
+    assert_eq!(a.session.active().unwrap().doc.units, vectorcraft_doc::Unit::Points);
+
+    let mut b = app();
+    b.run("prefs.set", json!({"key": "unitsGeneral", "value": "Millimeters"})).unwrap();
+    open(&mut b, Some("General"));
+    click_button(&mut b, "Reset Preferences");
+    confirm(&mut b).unwrap();
+    assert_eq!(b.session.prefs.units_general, Prefs::default().units_general);
+}
+
 #[test]
 fn old_pref_files_keep_their_brightness() {
     let mut a = app();

docs/mcp.md

@@ -4500,7 +4500,8 @@ four: General (rulers, positions and sizes, the Info panel, dialog distances, ca
 (weights, dashes), Type (font size, leading, baseline shift) and East Asian Type (no field uses it yet). General is the active document's units
 (`document.inspect` → `units`): `document.setUnits {units}` (Document Setup) sets it for that document, and
 `prefs.set {key: "unitsGeneral", value}` sets it for the active document too (one undo step) and is the units
-`file.new` starts in when it gets no `units`. Stroke, Type and East Asian Type are the preferences `unitsStroke`,
+`file.new` starts in when it gets no `units`. With `documentUnits: false`, `prefs.set` sets only the preference and
+leaves the document's units as they are; Reset Preferences followed by OK in the Preferences dialog does this. Stroke, Type and East Asian Type are the preferences `unitsStroke`,
 `unitsType` and `unitsAsianType`. Only `unitsAsianType` also takes `q` (Q, `Q`) and `ha` (Ha, `Ha`), each a quarter of
 a millimetre (0.709 pt); the other preferences, `document.setUnits` and the rulers do not list them, and it is Points
 by default. A length typed with a unit