Merge pull request #5101 from dotCooCoo/fix/kerning-optical-clears-manual

Type: choosing Optical kerning clears manual pair values, as Auto does

crates/engine/src/cmd/panelcmds.rs

@@ -338,8 +338,8 @@ fn set_format(s: &mut Session, p: &Value) -> Result<Value> {
     let flag = |k: &str| p.get(k).and_then(Value::as_bool);
     let kerning = match p.get("kerning") {
         None | Some(Value::Null) => None,
-        Some(Value::String(a)) if a.eq_ignore_ascii_case("auto") => Some(None),
-        Some(Value::String(a)) if a.eq_ignore_ascii_case("optical") => None,
+        // Auto and Optical both clear manual pair values.
+        Some(Value::String(a)) if a.eq_ignore_ascii_case("auto") || a.eq_ignore_ascii_case("optical") => Some(None),
         Some(v) => Some(Some(v.as_f64().ok_or_else(|| bad(C, "kerning must be a number, \"auto\", or \"optical\""))?.clamp(-1000.0, 10000.0))),
     };
     let optical_kerning = match p.get("kerning") {

crates/engine/src/cmd/textedit.rs

@@ -549,8 +549,8 @@ impl CharChange {
             tracking: num("tracking"),
             kerning: match p.get("kerning") {
                 None | Some(Value::Null) => None,
-                Some(Value::String(v)) if v.eq_ignore_ascii_case("optical") => None,
-                Some(Value::String(v)) if v.eq_ignore_ascii_case("auto") => Some(None),
+                // Auto and Optical both clear manual pair values.
+                Some(Value::String(v)) if v.eq_ignore_ascii_case("optical") || v.eq_ignore_ascii_case("auto") => Some(None),
                 Some(v) => Some(Some(v.as_f64().ok_or_else(|| bad(cmd, "kerning must be a number, \"auto\", or \"optical\""))?)),
             },
             optical_kerning: match p.get("kerning") {

crates/engine/src/tests/textedit.rs

@@ -95,10 +95,10 @@ fn optical_command_manual_pair_undo_redo_and_native_reopen() {
     let result = s.execute("text.setRangeStyle", &json!({"id": id.0, "kerning": "optical"})).unwrap();
     assert_eq!(result["kerning"], "optical");
     assert!(obj(&s, id).runs[0].style.optical_kerning);
-    assert_eq!(obj(&s, id).runs[0].style.kerning, Some(70.0));
+    assert_eq!(obj(&s, id).runs[0].style.kerning, None, "choosing Optical clears the manual value");
     s.execute("text.setRangeStyle", &json!({"id": id.0, "start": 1, "end": 2, "kerning": 0})).unwrap();
     let styled = obj(&s, id);
-    assert_eq!(styled.runs.iter().map(|r| r.style.kerning).collect::<Vec<_>>(), vec![Some(70.0), Some(0.0), Some(70.0)]);
+    assert_eq!(styled.runs.iter().map(|r| r.style.kerning).collect::<Vec<_>>(), vec![None, Some(0.0), None]);
     assert!(styled.runs.iter().all(|r| r.style.optical_kerning));
     let bytes = vectorcraft_format::save_file(&s.doc().unwrap().doc);
     let opened = vectorcraft_format::load(&bytes).unwrap();
@@ -108,14 +108,18 @@ fn optical_command_manual_pair_undo_redo_and_native_reopen() {
         styled.runs.iter().map(|r| (r.style.optical_kerning, r.style.kerning)).collect::<Vec<_>>()
     );
     s.execute("edit.undo", &json!({})).unwrap();
-    assert_eq!(obj(&s, id).runs[0].style.kerning, Some(70.0));
+    assert!(obj(&s, id).runs.iter().all(|r| r.style.kerning.is_none()));
+    s.execute("edit.undo", &json!({})).unwrap();
+    assert_eq!(obj(&s, id).runs[0].style.kerning, Some(70.0), "undoing Optical brings the manual value back");
+    assert!(!obj(&s, id).runs[0].style.optical_kerning);
+    s.execute("edit.redo", &json!({})).unwrap();
     s.execute("edit.redo", &json!({})).unwrap();
     assert_eq!(obj(&s, id).runs[1].style.kerning, Some(0.0));
     assert!(obj(&s, id).runs.iter().all(|r| r.style.optical_kerning));
 }
 
 #[test]
-fn optical_set_format_keeps_existing_pair_override() {
+fn optical_set_format_clears_existing_pair_override() {
     let mut s = session();
     let id = text(&mut s, "III");
     s.execute("text.setRangeStyle", &json!({"id": id.0, "start": 0, "end": 1, "kerning": 40})).unwrap();
@@ -123,7 +127,7 @@ fn optical_set_format_keeps_existing_pair_override() {
     assert_eq!(result["kerning"], "optical");
     let t = obj(&s, id);
     assert!(t.runs.iter().all(|r| r.style.optical_kerning));
-    assert_eq!(t.runs[0].style.kerning, Some(40.0));
+    assert!(t.runs.iter().all(|r| r.style.kerning.is_none()), "choosing Optical clears the manual value");
 }
 
 #[test]

docs/mcp.md

@@ -5067,8 +5067,8 @@ the paragraph's text now starts; when the text before `at` starts no list, nothi
 (the selected type) accept `kerning: "auto" | "optical" | number` (1/1000 em). Auto uses the font's
 pair metrics; Optical spaces horizontal pairs from their glyph shapes. Vertical text uses font
 metrics in either mode. A number sets a manual pair value,
-replacing that pair's automatic correction while tracking remains additive. Choosing Optical keeps
-existing manual pair values; choosing `"auto"` clears them. New text and files without an Optical setting use Auto. The Character
+replacing that pair's automatic correction while tracking remains additive. Choosing `"optical"` or
+`"auto"` clears existing manual pair values in the text it applies to. New text and files without an Optical setting use Auto. The Character
 panel offers both automatic modes.
 
 ## Small Caps, Superscript and Subscript