From 7163bf93dbc33cab8a7d16e65b7447a2d2f19f94 Mon Sep 17 00:00:00 2001 From: Caio Pontes Date: Sun, 25 Oct 2026 23:15:33 -0300 Subject: [PATCH 1/3] Fix #3837: unit fields read a decimal comma as a decimal point Typing an offset of `0,1` in Offset Path, in a document in millimeters, put the field back to the value it had (1 mm), so it looked as if values under 1 mm were refused. The length parser only takes a point as the decimal separator, and a field drops text it can't read. The shared numeric fields (unit_field, bounded_field and opt_field) now read a comma as the decimal separator when the text has no point, so `0,1 mm`, `1,5+0,25` and `12,5%` work in every dialog and panel that uses them. Text with a point keeps its commas, so `1,000.5` is still refused. The engine, MCP and control channel keep the strict format. --- crates/ui-egui/src/dialogs/tests_modal.rs | 23 ++++++++++++ crates/ui-egui/src/widgets.rs | 45 ++++++++++++++++++++--- 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/dialogs/tests_modal.rs b/crates/ui-egui/src/dialogs/tests_modal.rs index 602afe09b..3a0075757 100644 --- a/crates/ui-egui/src/dialogs/tests_modal.rs +++ b/crates/ui-egui/src/dialogs/tests_modal.rs @@ -703,3 +703,26 @@ fn printer_mark_labels_are_distinct_from_language_names() { assert_eq!(save_pdf::choice_label(cs, "language", "Japanese"), crate::i18n::tr(cs, "Japanese")); assert_eq!(save_pdf::choice_label(crate::i18n::Lang::EN, "marks.kind", "Japanese"), "Japanese"); } + +/// In a document in millimeters, an offset typed with a decimal comma (`0,1`) is taken as 0.1 mm +/// instead of going back to the value the field had, and Offset Path opens on it again (#3837). +#[test] +fn offset_path_takes_a_decimal_comma() { + let (ctx, mut app) = (ctx(), app_with_selection()); + app.run("document.setUnits", json!({"units": "Millimeters"})).unwrap(); + app.run("select.all", json!({})).unwrap(); + crate::menus::invoke(&mut app, "object.path.offsetPath", json!({})); + settle(&ctx, &mut app); + // The dialog focuses its first field, the offset: type as a user does, then Tab commits it. + frame(&ctx, &mut app, vec![Event::Text("0,1".into())]); + frame(&ctx, &mut app, vec![Event::Key { key: egui::Key::Tab, physical_key: None, pressed: true, repeat: false, modifiers: Default::default() }]); + let texts = frame(&ctx, &mut app, vec![]); + let mm = vectorcraft_doc::Unit::Millimeters.to_pt(0.1); + let offset = app.ui.dialog.as_ref().unwrap().f64("offset", 0.0); + assert!((offset - mm).abs() < 1e-9, "0.1 mm: {offset} pt"); + assert!(texts.iter().any(|(t, _)| t == "0.1 mm"), "the field shows 0.1 mm: {texts:?}"); + confirm(&mut app).unwrap(); + crate::menus::invoke(&mut app, "object.path.offsetPath", json!({})); + let offset = app.ui.dialog.as_ref().unwrap().f64("offset", 0.0); + assert!((offset - mm).abs() < 1e-9, "reopened on 0.1 mm: {offset} pt"); +} diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 8fc3443cf..8ea710809 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -343,6 +343,13 @@ pub(crate) fn typed_unit(ctx: &egui::Context, unit: Unit) -> Unit { if bare_points && unit == Unit::Picas { Unit::Points } else { unit } } +/// `text` typed in a numeric field, with a decimal comma read as a point: `0,1 mm` is `0.1 mm` +/// (#3837). Text with a point keeps its commas, so `1,000.5` is still refused rather than read +/// with two decimal separators. +fn decimal_point(text: &str) -> std::borrow::Cow<'_, str> { + if text.contains(',') && !text.contains('.') { text.replace(',', ".").into() } else { text.into() } +} + /// A recessed numeric field showing `value` (points) in `unit`. Returns the new value (points) /// when the user commits (Enter / focus loss / Shift+Enter, which keeps the focus outside dialogs). /// Supports unit suffixes and arithmetic; a number without a unit is in `unit` ([`typed_unit`]: in @@ -387,14 +394,15 @@ fn unit_field( let (mut buf, commit, resp, rect) = recessed_text(ui, id, &shown, width, 1, label, name); select_all_on_focus(ui, &resp, &buf); // ↑/↓ and the wheel step in the field's unit; a drag on its label scrubs it. - let read = typed_unit(ui.ctx(), unit); + let typed = typed_unit(ui.ctx(), unit); + let read = |b: &str| typed.parse(&decimal_point(b)); let bounds = unit.from_pt(*bounds.start())..=unit.from_pt(*bounds.end()); - let stepped = step_value(ui, &resp, &mut buf, &bounds, (|b: &str| Some(unit.from_pt(read.parse(b)?)), |v| unit.format(unit.to_pt(v))), None); + let stepped = step_value(ui, &resp, &mut buf, &bounds, (|b: &str| Some(unit.from_pt(read(b)?)), |v| unit.format(unit.to_pt(v))), None); let scrubbed = scrub::field(ui, id, rect, &buf, value.map(|v| unit.from_pt(v)), STEP_DECIMALS, &bounds); let commit = commit && buf != shown; ui.data_mut(|d| d.insert_temp(id, buf.clone())); if commit { - read.parse(&buf).or_else(|| if share_of_value { share_of(&buf, value?) } else { None }) + read(&buf).or_else(|| if share_of_value { share_of(&decimal_point(&buf), value?) } else { None }) } else { stepped.or(scrubbed).map(|v| unit.to_pt(v)) } @@ -933,6 +941,7 @@ fn bounded_field( // The suffix (and %, °) may follow any operand: `45*2°`, `50% / 2`. let read = |buf: &str| { let suffix = suffix.trim(); + let buf = decimal_point(buf); let bare = if suffix.is_empty() { buf.to_string() } else { buf.replace(suffix, "") }; vectorcraft_doc::parse_number(&bare.replace(['%', '°'], "")) }; @@ -2589,7 +2598,14 @@ pub fn opt_field_named(ui: &mut Ui, id: impl std::hash::Hash, name: Option<&str> select_all_on_focus(ui, &resp, &buf); // ↑/↓ and the wheel step a value (an empty field stays empty); a drag on its label scrubs it. let lengths = 0.0..=f64::INFINITY; - let stepped = step_value(ui, &resp, &mut buf, &lengths, (|b: &str| Some(unit.from_pt(unit.parse(b)?)), |v| unit.number(unit.to_pt(v))), None); + let stepped = step_value( + ui, + &resp, + &mut buf, + &lengths, + (|b: &str| Some(unit.from_pt(unit.parse(&decimal_point(b))?)), |v| unit.number(unit.to_pt(v))), + None, + ); let scrubbed = scrub::field(ui, id, framed.response.rect, &buf, value.map(|v| unit.from_pt(v)), STEP_DECIMALS, &lengths); let focused = ui.memory(|m| m.has_focus(id)); ui.data_mut(|d| { @@ -2598,7 +2614,7 @@ pub fn opt_field_named(ui: &mut Ui, id: impl std::hash::Hash, name: Option<&str> }); if (resp.lost_focus() || kept) && buf != shown { let s = buf.trim(); - if s.is_empty() { Some(None) } else { typed_unit(ui.ctx(), unit).parse(s).map(Some) } + if s.is_empty() { Some(None) } else { typed_unit(ui.ctx(), unit).parse(&decimal_point(s)).map(Some) } } else { stepped.or(scrubbed).map(|v| Some(unit.to_pt(v))) } @@ -3224,6 +3240,25 @@ mod step_to_multiple_tests { } } +#[cfg(test)] +mod decimal_point_tests { + use super::*; + + /// A decimal comma reads as a point, in a length, with arithmetic and in a percentage; text + /// with a point keeps its commas and is refused as before (#3837). + #[test] + fn a_decimal_comma_reads_as_a_point() { + let mm = |s: &str| Unit::Millimeters.parse(&decimal_point(s)).map(|pt| Unit::Millimeters.from_pt(pt)); + for (text, want) in [("0,1", 0.1), ("0,1 mm", 0.1), ("0,1mm", 0.1), ("-0,5", -0.5), ("1,5+0,25", 1.75), ("0.1", 0.1)] { + let got = mm(text).unwrap_or(f64::NAN); + assert!((got - want).abs() < 1e-9, "{text}: {got}"); + } + assert_eq!(mm("1,000.5"), None, "a point and a comma"); + assert_eq!(mm("1,2,3"), None, "two commas"); + assert_eq!(share_of(&decimal_point("12,5%"), 200.0), Some(25.0)); + } +} + #[cfg(test)] mod menu_room_tests { use super::*; From 326f7809b0d166d50d0e00eb0c2f59f98084833e Mon Sep 17 00:00:00 2001 From: Caio Pontes Date: Sun, 25 Oct 2026 23:15:33 -0300 Subject: [PATCH 2/3] Contributors: add Caio Pontes --- contributors/people.toml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/contributors/people.toml b/contributors/people.toml index f8c651239..67130e9e0 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -159,3 +159,8 @@ display_name = "Aditya Sisodiya" real_name = "Angelo Bernardini" display_name = "Angelo Bernardini" sync_github_name = false + +[people.DCaioP] +real_name = "Caio Pontes Magalhães" +display_name = "Caio Pontes" +sync_github_name = false From aaf574b8450561d62be6f317d29e9423b580497b Mon Sep 17 00:00:00 2001 From: Caio Pontes Date: Mon, 26 Oct 2026 02:16:53 -0300 Subject: [PATCH 3/3] Fix #3837: a thousands comma is refused, not read as a decimal Review: reading `1,000` as 1 would give someone who writes decimals with a point a value a thousand times too small, with no sign that anything went wrong; refusing it, as before, is the safer answer. A number that reads as thousands (one to three digits, a comma, exactly three digits, then at most a unit: `1,000`, `12,500 mm`) now keeps its comma, so the field refuses it. `0,125` is still a decimal (a lone 0 never groups thousands), as are `1,25`, `1,0000` and `1,500+2`. --- crates/ui-egui/src/widgets.rs | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 8ea710809..ec1b20592 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -345,9 +345,24 @@ pub(crate) fn typed_unit(ctx: &egui::Context, unit: Unit) -> Unit { /// `text` typed in a numeric field, with a decimal comma read as a point: `0,1 mm` is `0.1 mm` /// (#3837). Text with a point keeps its commas, so `1,000.5` is still refused rather than read -/// with two decimal separators. +/// with two decimal separators, and so does a number that reads as thousands ([`thousands`]). fn decimal_point(text: &str) -> std::borrow::Cow<'_, str> { - if text.contains(',') && !text.contains('.') { text.replace(',', ".").into() } else { text.into() } + if text.contains(',') && !text.contains('.') && !thousands(text) { text.replace(',', ".").into() } else { text.into() } +} + +/// Is `text` a number written with a thousands comma, as `1,000` or `12,500 mm`: one to three +/// digits (not a lone 0), a comma, exactly three digits, then at most a unit? Read as a decimal it +/// would be a thousand times too small, so the field refuses it instead (`0,125` and `1,25` are +/// decimals). +fn thousands(text: &str) -> bool { + let number = text.trim().trim_start_matches('-'); + let Some((whole, rest)) = number.split_once(',') else { return false }; + let (part, unit) = rest.split_at(rest.find(|c: char| !c.is_ascii_digit()).unwrap_or(rest.len())); + (1..=3).contains(&whole.len()) + && whole.bytes().all(|b| b.is_ascii_digit()) + && !whole.starts_with('0') + && part.len() == 3 + && unit.trim().chars().all(char::is_alphabetic) } /// A recessed numeric field showing `value` (points) in `unit`. Returns the new value (points) @@ -3245,7 +3260,8 @@ mod decimal_point_tests { use super::*; /// A decimal comma reads as a point, in a length, with arithmetic and in a percentage; text - /// with a point keeps its commas and is refused as before (#3837). + /// with a point keeps its commas and is refused as before, and so does a thousands comma + /// (#3837). #[test] fn a_decimal_comma_reads_as_a_point() { let mm = |s: &str| Unit::Millimeters.parse(&decimal_point(s)).map(|pt| Unit::Millimeters.from_pt(pt)); @@ -3255,6 +3271,13 @@ mod decimal_point_tests { } assert_eq!(mm("1,000.5"), None, "a point and a comma"); assert_eq!(mm("1,2,3"), None, "two commas"); + for thousand in ["1,000", "12,500 mm", "-1,000", "999,999mm"] { + assert_eq!(mm(thousand), None, "{thousand} reads as thousands: refused, not a thousand times too small"); + } + for (text, want) in [("0,125", 0.125), ("1,25", 1.25), ("1,0000", 1.0), ("1,500+2", 3.5)] { + let got = mm(text).unwrap_or(f64::NAN); + assert!((got - want).abs() < 1e-9, "{text} is a decimal: {got}"); + } assert_eq!(share_of(&decimal_point("12,5%"), 200.0), Some(25.0)); } }