From 2f9399326633cc8c70edfac4196592bc2cd1d83d Mon Sep 17 00:00:00 2001 From: yaraslau Date: Thu, 24 Sep 2026 22:22:16 +0200 Subject: [PATCH 1/3] forms: a display unit and decimals for a plain double member (FieldMeta::unit / ::decimals) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A DTO whose readings are plain `double`s -- the common shape for an app that already has its own persistence types -- had no way to tell a renderer "kg/m³, three decimals". That knowledge lived only in `Quantity`'s type, so a host rendering such a form got a bare text field with no unit and no entry precision, and a slot registered for it was handed neither. `FieldMeta` gains two presentation members, applied by the one `annotateBasicMemberProperty` implementation and therefore at every nesting depth: - `unit` is emitted as the property's `ExtUnits` (`{"unitAscii": unit, "unitUnicode": unit}`), the key a `Quantity` already carries. Nothing on the reading side changes: DynamicForm's unit suffix, `SlotRegistry.byUnit` and a view's `v-columns` entry all read `ExtUnits` off the property node already. - `decimals` (`std::optional`) is emitted as the new `x-displayDecimals`, deliberately **not** `x-decimalPlaces`: the latter wins over `"type": "number"` and switches the property to the exact `{num,den,dp}` encoding, which a `double` member cannot decode. Both are ignored on a `Quantity` member (its unit and declared precision are part of its type, and a second declaration could only disagree) and a `decimals` above `kMaxDecimalPlaces` is ignored, as a non-positive `multipleOf` is. `withUnit()` / `withDecimals()` join the fluent builders. DynamicForm reads `x-displayDecimals` only for a `"number"` with no `x-decimalPlaces`. It keeps the JSON-number encoding, refuses an entry with more fraction digits than declared (the rule a `Quantity` entry already follows: refused, never rounded), spells the placeholder from it, and puts `decimals` / `decimalsDeclared` / `displayDecimals` on the field descriptor a slot receives. Tests: tests/test_forms_display_unit.cpp (8 cases: emission, omission, the Quantity exclusion, the kMax guard, a std::vector element, the builders, and an untouched wire) and src/qt/forms/tests/tst_DynamicFormDisplayUnit.qml (11 cases, each asserting the submitted body). Mutation-checked: commenting out the emission reddens 4 of the 8 C++ cases; disabling the renderer's entry limit reddens the 2 QML cases that pin it. Full suites on MSVC 14.51 / Qt 6.11.1: morph_tests 1584 cases green, forms_qml_logic 328 green. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 14 ++ docs/spec/forms/forms.md | 59 ++++- include/morph/forms/forms.hpp | 75 ++++++- src/qt/forms/qml/DynamicForm.qml | 27 ++- .../tests/tst_DynamicFormDisplayUnit.qml | 192 ++++++++++++++++ tests/CMakeLists.txt | 1 + tests/test_forms_display_unit.cpp | 208 ++++++++++++++++++ 7 files changed, 570 insertions(+), 6 deletions(-) create mode 100644 src/qt/forms/tests/tst_DynamicFormDisplayUnit.qml create mode 100644 tests/test_forms_display_unit.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 711190e4f..55a88a8a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -128,6 +128,20 @@ API surface). `resolve()` takes it as an optional sixth argument, consulted after unit and before type. See `docs/spec/forms/forms.md`, "Theming / component-override registry" (fixes #812). +- **`FieldMeta::unit` / `FieldMeta::decimals` — a display unit and precision + for a plain `double`/`float`/integral member.** A DTO holding lab readings as + plain `double`s had no way to tell a renderer "kg/m³, three decimals": that + knowledge lived only in `Quantity`'s type. `unit` is emitted as the + property's `ExtUnits` (the key a `Quantity` already carries, so the shipped + renderer's unit suffix, `SlotRegistry.byUnit` and view columns all see it + unchanged); `decimals` is emitted as the new `x-displayDecimals`, **not** + `x-decimalPlaces`, because the latter switches a property to the exact + `{num,den,dp}` encoding a `double` cannot decode. `DynamicForm` keeps the + JSON-number encoding, refuses an entry with more fraction digits than + declared (as it does for a `Quantity`), spells the placeholder from it, and + hands slots `field.decimals` / `field.decimalsDeclared`. Both keys are + ignored on a `Quantity` member and apply at any nesting depth. See + `docs/spec/forms/forms.md`, "Display unit and decimals for a plain member". - **A locale numeric entry accepts an explicit `+`.** `morph::render::normalizeLocaleNumber` had no notion of a positive sign: a diff --git a/docs/spec/forms/forms.md b/docs/spec/forms/forms.md index 66eb2bbfc..d3f179721 100644 --- a/docs/spec/forms/forms.md +++ b/docs/spec/forms/forms.md @@ -402,6 +402,8 @@ struct FieldMeta { std::optional minimum{}; // disengaged = no floor std::optional maximum{}; // disengaged = no ceiling std::optional multipleOf{}; // disengaged = any value + std::string_view unit{}; // "" = no display unit (plain members only) + std::optional decimals{}; // disengaged = no display precision }; struct RecordMeasurement { @@ -567,6 +569,47 @@ A per-*instance* `x-minimum`/`x-maximum` written by composes with a compiled bound rather than replacing it: the renderer checks both, so an instance range narrows the declared one and never widens it. +### Display unit and decimals for a plain member — `unit` / `decimals` + +A `Quantity` carries its unit and its precision in its type. A DTO whose +numeric members are plain `double`s has neither, so `FieldMeta` declares them: + +```cpp +struct RecordDensity { + double density = 0.0; + double temperature = 0.0; + + static constexpr std::array fieldMetadata{ + FieldMeta{.field = "density", .unit = "kg/m³", .decimals = math::DecimalPlaces{3}}, + FieldMeta{.field = "temperature"}.withUnit("°C"), + }; +}; +``` + +- **`unit` is emitted as `ExtUnits`**, the key a `Quantity`'s unit already + travels in, with the one string in both `unitAscii` and `unitUnicode`. Every + reader of a unit therefore finds a plain member's where it finds a + `Quantity`'s: the shipped renderer's unit suffix, `SlotRegistry.byUnit`, and + a view's `v-columns` entry (`views.hpp` copies `ExtUnits` off the property + node). It is presentation only: nothing converts through it, and it never + reaches the payload. +- **`decimals` is emitted as `x-displayDecimals`, deliberately not + `x-decimalPlaces`.** `x-decimalPlaces` hands a property the exact + `{num,den,dp}` encoding (see [Plain number fields](#plain-number-fields--type-number): + a declared precision wins over the `"number"` type), and a `double` member + cannot decode that object. `x-displayDecimals` keeps the JSON-number encoding + and only tells the renderer how many fraction digits to show and accept. It + is read only for a `"number"` property with no `x-decimalPlaces`. +- **Both are ignored on a `Quantity` member**, whose unit and declared + precision are part of its type and already emitted; a second declaration + could only disagree with the first. A `decimals` above `kMaxDecimalPlaces` + is ignored too, as a non-positive `multipleOf` is. +- **Both apply at any depth**, like the rest of `FieldMeta`: a `std::vector` + element's own `fieldMetadata` stamps the row type's properties. + +Neither is checked server-side. Like `placeholder`, they are presentation; +the one gate that follows from `decimals` is the renderer's entry limit below. + ### Field metadata is not a security control `x-readonly` and `x-hidden` are presentation only. The field still travels in @@ -589,8 +632,10 @@ member of the action at all. | `minimum` | property node (sibling of `$ref`) | number | Inclusive lower bound on the field's value, from `FieldMeta::minimum`. Omitted when not declared. Standard JSON-Schema vocabulary, not an `x-*` key — see [Per-field scalar bounds](#per-field-scalar-bounds--minimum--maximum--multipleof). | | `maximum` | property node (sibling of `$ref`) | number | Inclusive upper bound, from `FieldMeta::maximum`. Omitted when not declared. | | `multipleOf` | property node (sibling of `$ref`) | number | The field's value must be an exact integer multiple of this, from `FieldMeta::multipleOf`. `1` is how "whole number" is spelled. Omitted when not declared, or when the declared value is not strictly positive. | +| `ExtUnits` | property node (sibling of `$ref`) | object | A plain member's display unit, from `FieldMeta::unit`, as `{"unitAscii": unit, "unitUnicode": unit}` — the shape a `Quantity` carries. Omitted when empty, and never emitted for a `Quantity` member. See [Display unit and decimals](#display-unit-and-decimals-for-a-plain-member--unit--decimals). | +| `x-displayDecimals` | property node (sibling of `$ref`) | non-negative integer | A plain number's display and entry precision, from `FieldMeta::decimals`. Omitted when disengaged, above `kMaxDecimalPlaces`, or on a `Quantity` member. | -All ten keys are additive and non-breaking, extending the renderer-contract +All twelve keys are additive and non-breaking, extending the renderer-contract table below without renaming or retyping any existing key, per this program's versioning stance (see "Design principle" above). A renderer that ignores them falls back to today's behavior exactly: it shows @@ -788,7 +833,8 @@ below) `DynamicForm.qml`'s `resolveProp` does exactly this dual read. | `x-maximum` | property node (sibling of `$ref`) | object `{num,den,dp}` | Inclusive upper bound, same source and shape as `x-minimum`. | | `x-instanceConstraints` | top-level (object) | array of strings | Wire field names whose keys were written from *instance* data rather than derived from the compiled action type. Present only on a decorated schema. A renderer needing to know whether an `x-decimalPlaces`/`x-minimum`/`x-maximum` is instance-sourced checks membership here rather than guessing. | | `format` | `Timestamp` property (or its `$def`) | string, value `"date-time"` | Standard JSON-Schema vocabulary (stamped by glaze, not by morph). The renderer shows a date-time input; the wire value is the ISO-8601 string `Timestamp` serialises to. No `x-*` extension is used for timestamps. | -| `ExtUnits` | `$def` of the `Quantity`'s unit type (reached via the property's `$ref`) | object | Glaze-stamped block describing the field's **canonical** unit. Two fields: `unitAscii` (the stable ascii id, e.g. `"kg_per_m3"` — sourced from `UnitMeta::id`) and `unitUnicode` (the human display text, e.g. `"kg/m³"` — from `UnitMeta::display`). This is the unit a payload value is always denominated in, and the reference point the `num`/`den` of every `x-unitAlternatives` entry converts *to*. A renderer resolves the property's `$ref` into `$defs` to read `ExtUnits.unitAscii`/`unitUnicode` (it is **not** on the property node next to the `x-*` keys) to label the field and anchor the unit selector. | +| `x-displayDecimals` | property node (sibling of `$ref`) | non-negative integer | A plain `"number"` member's display and entry precision, from `FieldMeta::decimals`. Read only when the property has no `x-decimalPlaces`; the value keeps its JSON-number encoding, and an entry with more fraction digits is refused. See [Display unit and decimals](#display-unit-and-decimals-for-a-plain-member--unit--decimals). | +| `ExtUnits` | `$def` of the `Quantity`'s unit type (reached via the property's `$ref`) — or, for a plain member declaring `FieldMeta::unit`, the property node | object | Glaze-stamped block describing the field's **canonical** unit. Two fields: `unitAscii` (the stable ascii id, e.g. `"kg_per_m3"` — sourced from `UnitMeta::id`) and `unitUnicode` (the human display text, e.g. `"kg/m³"` — from `UnitMeta::display`). This is the unit a payload value is always denominated in, and the reference point the `num`/`den` of every `x-unitAlternatives` entry converts *to*. A renderer resolves the property's `$ref` into `$defs` to read `ExtUnits.unitAscii`/`unitUnicode` (it is **not** on the property node next to the `x-*` keys) to label the field and anchor the unit selector. On a plain member it is the `FieldMeta::unit` display text in both subfields, and there is no unit selector. | | `x-layout` | top-level (object) | object | The form's group structure: `{ "groups": [ { "title": string, "kind": "section"\|"tab"\|"accordion", "fields": [wire-key,…] }, … ] }`, in `A::formLayout` declaration order. Emitted only when the action declares `formLayout`. The renderer builds the named containers in array order and places each field in its group; fields absent from every group go in a trailing default group. | | `x-group` | property node (sibling of `$ref`) | string | The title of the group this field belongs to. Omitted for a field in the implicit default group, or when `x-layout` is absent. | | `x-section` | property node (sibling of `$ref`) | non-negative integer | The 0-based index of this field's group in `x-layout.groups`. Omitted under the same conditions as `x-group`. | @@ -973,6 +1019,15 @@ number: | an integral member | `"type": "integer"` (+ `x-exactMinimum`/`x-exactMaximum` past 2^53) | bare integer, gated on the [exact string bounds](#exact-numeric-bounds--x-exactminimum--x-exactmaximum) | | `double` / `float` | `"type": "number"` | bare JSON number, as above | +**`x-displayDecimals` is an entry limit.** When a plain number declares one +(`FieldMeta::decimals`, see [Display unit and decimals](#display-unit-and-decimals-for-a-plain-member--unit--decimals)), +an entry with more fraction digits than that has no literal, exactly as an +over-precise `Quantity` entry has none: it is refused, never rounded, because a +rounded value is one the user did not type. The encoding stays a bare JSON +number. The field descriptor carries the count as `decimals` (with +`decimalsDeclared` telling a slot whether `0` was declared or merely +defaulted), and the placeholder spells it (`"0.000"`). + A generated `Quantity` property is therefore not `"number"` at all. The order still matters, because a *decorated* schema can put `x-decimalPlaces` on a property whose type is `"number"` — a precise field spelled the plain way. The diff --git a/include/morph/forms/forms.hpp b/include/morph/forms/forms.hpp index fa4f29e17..e9abb7f37 100644 --- a/include/morph/forms/forms.hpp +++ b/include/morph/forms/forms.hpp @@ -78,6 +78,11 @@ /// declaration in C++, so an action's `validate()` enforces exactly what /// the client was served. Bounds are per *field*, unlike /// `UnitTraits::bounds`, which is per unit. +/// - **`ExtUnits` / `x-displayDecimals`** — for a *non-`Quantity`* field whose +/// `FieldMeta` declares `unit` / `decimals`: the display unit (in the same +/// `ExtUnits` shape a `Quantity` carries) and the fraction digits a renderer +/// shows and accepts. Presentation only — the member keeps its plain JSON +/// encoding. /// - **`x-computed` / `x-readonly`** — for a member listed as the destination /// of an action's `computedFields` declaration: the field is derived from /// sibling inputs (named in `x-computed.inputs`) and must not be rendered as @@ -188,7 +193,8 @@ namespace morph::forms { /// @brief Per-field presentation overrides and scalar bounds: label, help, -/// placeholder, read-only, hidden, `minimum`/`maximum`/`multipleOf` +/// placeholder, read-only, hidden, `minimum`/`maximum`/`multipleOf`, +/// and a plain member's display `unit`/`decimals` /// (docs/spec/forms/forms.md, "Field metadata"). /// /// An action opts in with a `static constexpr std::array` @@ -198,7 +204,8 @@ namespace morph::forms { /// than `field` defaults to "not declared": an empty `label`/`help`/ /// `placeholder` means "infer the title, omit the rest"; `readOnly`/`hidden` /// default to `false`; a disengaged `minimum`/`maximum`/`multipleOf` emits -/// nothing and checks nothing. `mergeSchemaExtras` looks up the entry (if any) +/// nothing and checks nothing; an empty `unit` and a disengaged `decimals` +/// emit nothing. `mergeSchemaExtras` looks up the entry (if any) /// matching each reflected member by wire key and patches the property node; /// an entry naming a field that does not exist on the action is ignored. /// @@ -275,6 +282,27 @@ struct FieldMeta { /// exactly the same set of values as its magnitude. std::optional<::morph::math::Rational> multipleOf{}; + /// @brief Display unit for a member whose C++ type carries none (a plain + /// `double`, `float` or integral member); emitted as the property's + /// `ExtUnits` (`{"unitAscii": unit, "unitUnicode": unit}`), the key + /// a `Quantity` already carries. Empty emits nothing. + /// + /// Presentation only: the unit never travels in the payload and nothing + /// converts through it. **Ignored on a `Quantity` member**, whose unit is + /// part of its type and already emitted. + std::string_view unit{}; + + /// @brief Display and entry precision for a plain `double`/`float` member; + /// emitted as `x-displayDecimals`. Disengaged emits nothing. + /// + /// Deliberately not `x-decimalPlaces`: that key hands a property the exact + /// `{num,den,dp}` encoding, which a `double` cannot decode. This one keeps + /// the plain JSON-number encoding and only tells a renderer how many + /// fraction digits to show and accept. **Ignored on a `Quantity` member** + /// (its `x-decimalPlaces` is authoritative) and when it exceeds + /// `morph::math::kMaxDecimalPlaces`. + std::optional<::morph::math::DecimalPlaces> decimals{}; + /// @brief Returns a copy with `placeholder` set to @p text. /// @param text The placeholder hint. /// @return The updated descriptor. @@ -327,6 +355,24 @@ struct FieldMeta { copy.multipleOf = step; return copy; } + + /// @brief Returns a copy with `unit` set to @p text. + /// @param text The display unit, e.g. `"kg/m³"`. + /// @return The updated descriptor. + [[nodiscard]] constexpr FieldMeta withUnit(std::string_view text) const noexcept { + FieldMeta copy = *this; + copy.unit = text; + return copy; + } + + /// @brief Returns a copy with `decimals` set to @p places. + /// @param places Fraction digits to display and accept. + /// @return The updated descriptor. + [[nodiscard]] constexpr FieldMeta withDecimals(::morph::math::DecimalPlaces places) const noexcept { + FieldMeta copy = *this; + copy.decimals = places; + return copy; + } }; /// @brief Concept: a field type with an internal empty state (`Quantity`, @@ -2174,6 +2220,26 @@ inline void annotateDeclaredBounds(glz::generic_u64& property, const FieldMeta& } } +/// @brief Stamps @p meta's display `unit` (as `ExtUnits`) and `decimals` (as +/// `x-displayDecimals`) onto @p property, for a non-`Quantity` member. +/// +/// `ExtUnits` is the key a `Quantity` already carries, so every reader of a +/// unit -- a renderer's suffix label, `SlotRegistry.byUnit`, a view column -- +/// finds a plain member's unit where it finds a `Quantity`'s. +/// @param property Property node to annotate in place. +/// @param meta The field's declared metadata. +inline void annotateDisplayUnit(glz::generic_u64& property, const FieldMeta& meta) { + if (!meta.unit.empty()) { + glz::generic_u64 units{}; + units["unitAscii"] = std::string{meta.unit}; + units["unitUnicode"] = std::string{meta.unit}; + property["ExtUnits"] = std::move(units); + } + if (meta.decimals.has_value() && meta.decimals->value <= ::morph::math::kMaxDecimalPlaces) { + property["x-displayDecimals"] = std::uint64_t{meta.decimals->value}; + } +} + /// @brief Whether @p value satisfies every bound @p meta declares. /// /// The comparisons run on the exact `math::Rational`, never on a `double`, so @@ -2261,6 +2327,11 @@ void annotateBasicMemberProperty(glz::generic_u64& property, std::string_view na property["x-i18nKey"] = std::string{fieldMeta->i18nKey}; } annotateDeclaredBounds(property, *fieldMeta); + // A Quantity's unit and precision are part of its type and emitted + // below; a FieldMeta restating them could only disagree. + if constexpr (!units::isQuantity) { + annotateDisplayUnit(property, *fieldMeta); + } } if constexpr (units::isQuantity) { diff --git a/src/qt/forms/qml/DynamicForm.qml b/src/qt/forms/qml/DynamicForm.qml index b7fc70a14..b00941d29 100644 --- a/src/qt/forms/qml/DynamicForm.qml +++ b/src/qt/forms/qml/DynamicForm.qml @@ -15,6 +15,9 @@ // a genuine JSON array literal, e.g. "a, b" -> ["a","b"] // type: "number" -> plain text field encoding a JSON number, for a member // with no x-decimalPlaces (a bare double/float) +// x-displayDecimals -> a plain number's display/entry precision: at most +// that many fraction digits are accepted, and the +// JSON-number encoding is kept (FieldMeta::decimals) // x-submitMode: "explicit" -> suppresses auto-submit-on-validity; renders // an explicit Submit button (enabled only while ready) // instead -- see "Explicit submit mode" below @@ -478,6 +481,11 @@ Frame { const p = resolveProp(raw) const types = jsonTypes(p) const dp = opt(raw["x-decimalPlaces"], p["x-decimalPlaces"]) + // A plain number's display precision (FieldMeta::decimals). + // Only a "number" with no x-decimalPlaces reads it: the + // declared precision of a Quantity is the one that encodes. + const displayDp = (dp === undefined && types.indexOf("number") !== -1) + ? opt(raw["x-displayDecimals"], p["x-displayDecimals"]) : undefined const optionsAction = opt(raw["x-optionsAction"], p["x-optionsAction"]) // A closed set stated by the schema itself. Read // from `raw`, not the collapsed `p`: resolveProp keeps only @@ -558,7 +566,13 @@ Frame { dependsOn: opt(raw["x-optionsDependsOn"], opt(p["x-optionsDependsOn"], [])), isDateTime: p.format === "date-time", isQuantity: dp !== undefined, - decimals: opt(dp, 0), + // Fraction digits: a Quantity's declared precision, else a + // plain number's x-displayDecimals, else 0. + // `decimalsDeclared` tells a slot which of "0" and "none + // declared" it is looking at. + decimals: opt(dp, opt(displayDp, 0)), + decimalsDeclared: dp !== undefined || displayDp !== undefined, + displayDecimals: displayDp, isInteger: types.indexOf("integer") !== -1, // "number" -- a bare `double`/`float`. The plain text // field draws it, but the JSON *number* encoding is its @@ -1412,6 +1426,11 @@ Frame { // fraction, stray sign, letters) is refused rather than encoded. if (canonicalNumber === null || !/^-?\d+(\.\d+)?$/.test(canonicalNumber)) return null + // A declared display precision is an entry limit, as a Quantity's + // is: more fraction digits are refused rather than rounded away. + if (f.displayDecimals !== undefined + && (canonicalNumber.split(".")[1] || "").length > f.displayDecimals) + return null const numberValue = parseFloat(canonicalNumber) // The declared range, which for a plain member is the one glaze // stamps on the type itself: a `float` field carries ±3.4e38, so @@ -1889,7 +1908,11 @@ Frame { ? fieldColumn.modelData.placeholder : (fieldColumn.modelData.isQuantity ? "0." + "0".repeat(Math.max(1, fieldColumn.modelData.decimals)) - : (fieldColumn.modelData.isInteger ? "0" : "")) + : (fieldColumn.modelData.isInteger ? "0" + : (fieldColumn.modelData.displayDecimals !== undefined + ? (fieldColumn.modelData.displayDecimals > 0 + ? "0." + "0".repeat(fieldColumn.modelData.displayDecimals) : "0") + : ""))) inputMethodHints: (fieldColumn.modelData.isQuantity || fieldColumn.modelData.isInteger || fieldColumn.modelData.isNumber) ? Qt.ImhFormattedNumbersOnly : Qt.ImhNone diff --git a/src/qt/forms/tests/tst_DynamicFormDisplayUnit.qml b/src/qt/forms/tests/tst_DynamicFormDisplayUnit.qml new file mode 100644 index 000000000..68329ca41 --- /dev/null +++ b/src/qt/forms/tests/tst_DynamicFormDisplayUnit.qml @@ -0,0 +1,192 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// A plain number's display unit and decimals -- `ExtUnits` and +// `x-displayDecimals` on a `"number"` property, what `FieldMeta::unit` / +// `::decimals` stamp there (tests/test_forms_display_unit.cpp pins the C++ +// half against the same key names). +// +// The unit is read exactly where a Quantity's is, so the suffix label and the +// `byUnit` slot tier see it with no code of their own. The decimals are the +// new part: an entry limit that keeps the plain JSON-number encoding, which is +// why every case asserts the submitted body and not only `ready`. + +import QtQuick +import QtQuick.Controls +import QtTest +import MorphForms + +TestCase { + id: testCase + name: "DynamicFormDisplayUnit" + visible: true + + // `struct { double density; double temperature; }` with + // FieldMeta{.unit = "kg/m³", .decimals = 3} / FieldMeta{.unit = "°C"}. + property var readingSchema: ({ + "$defs": { + "double": { type: "number", minimum: -1.7976931348623157e+308, maximum: 1.7976931348623157e+308 } + }, + properties: { + density: { + "$ref": "#/$defs/double", "x-order": 0, title: "Density", + ExtUnits: { unitAscii: "kg/m³", unitUnicode: "kg/m³" }, "x-displayDecimals": 3 + }, + temperature: { + "$ref": "#/$defs/double", "x-order": 1, title: "Temperature", + ExtUnits: { unitAscii: "°C", unitUnicode: "°C" } + } + }, + required: ["density"] + }) + + // A declared precision beside a display precision: the first encodes, so + // the second is not read at all. + property var precisionSchema: ({ + properties: { + reading: { type: "number", "x-decimalPlaces": 1, "x-displayDecimals": 3, "x-order": 0 } + }, + required: ["reading"] + }) + + // A display precision of zero: whole numbers only, still a JSON number. + property var wholeSchema: ({ + properties: { count: { type: "number", "x-displayDecimals": 0, "x-order": 0 } }, + required: ["count"] + }) + + Component { + id: readingForm + DynamicForm { actionType: "T_Reading"; schema: testCase.readingSchema; controller: null } + } + + Component { + id: precisionForm + DynamicForm { actionType: "T_Precision"; schema: testCase.precisionSchema; controller: null } + } + + Component { + id: wholeForm + DynamicForm { actionType: "T_Whole"; schema: testCase.wholeSchema; controller: null } + } + + // A host slot registered for the unit: what it is handed is the contract. + Component { + id: unitSlot + TextField { + objectName: "unitSlot" + property var field + property var setValue + onTextChanged: if (setValue) setValue(text) + } + } + + Component { + id: slotRegistryComponent + SlotRegistry {} + } + + function typeInto(form, field, text) { + findChild(form, field).text = text + } + + function descriptor(form, name) { + return form.fieldByName[name] + } + + // Depth-first search for a visible Label showing exactly `text`. + function visibleLabel(item, text) { + if (!item) + return null + if (item instanceof Label && item.text === text && item.visible) + return item + const kids = item.children || [] + for (let i = 0; i < kids.length; ++i) { + const found = visibleLabel(kids[i], text) + if (found) + return found + } + return null + } + + function test_the_descriptor_carries_the_unit_and_the_decimals() { + const form = createTemporaryObject(readingForm, testCase) + const density = descriptor(form, "density") + compare(density.unit, "kg/m³") + compare(density.unitAscii, "kg/m³") + compare(density.decimals, 3) + compare(density.decimalsDeclared, true) + compare(density.isNumber, true) + compare(density.isQuantity, false) + + const temperature = descriptor(form, "temperature") + compare(temperature.unit, "°C") + compare(temperature.decimals, 0) + compare(temperature.decimalsDeclared, false) + } + + function test_the_unit_is_shown_beside_the_control() { + const form = createTemporaryObject(readingForm, testCase) + verify(visibleLabel(form, "kg/m³") !== null) + verify(visibleLabel(form, "°C") !== null) + } + + function test_the_placeholder_shows_the_declared_decimals() { + const form = createTemporaryObject(readingForm, testCase) + compare(findChild(form, "field_density").placeholderText, "0.000") + compare(findChild(form, "field_temperature").placeholderText, "") + } + + function test_an_entry_within_the_decimals_is_a_json_number() { + const form = createTemporaryObject(readingForm, testCase) + typeInto(form, "field_density", "2.505") + compare(form.ready, true) + compare(form.previewLine, '{"density":2.505}') + } + + function test_more_decimals_than_declared_are_refused_not_rounded() { + const form = createTemporaryObject(readingForm, testCase) + typeInto(form, "field_density", "2.5051") + compare(form.ready, false) + compare(form.previewLine, "") + } + + function test_a_unit_without_decimals_leaves_the_fraction_unlimited() { + const form = createTemporaryObject(readingForm, testCase) + typeInto(form, "field_density", "1") + typeInto(form, "field_temperature", "20.123456") + compare(form.ready, true) + compare(form.previewLine, '{"density":1,"temperature":20.123456}') + } + + function test_zero_decimals_accepts_whole_numbers_only() { + const form = createTemporaryObject(wholeForm, testCase) + compare(findChild(form, "field_count").placeholderText, "0") + typeInto(form, "field_count", "12") + compare(form.previewLine, '{"count":12}') + typeInto(form, "field_count", "12.5") + compare(form.ready, false) + } + + function test_a_declared_precision_still_wins() { + const form = createTemporaryObject(precisionForm, testCase) + const reading = descriptor(form, "reading") + compare(reading.isQuantity, true) + compare(reading.decimals, 1) + typeInto(form, "field_reading", "3.5") + compare(form.previewLine, '{"reading":{"num":35,"den":10,"dp":1}}') + } + + function test_a_unit_slot_receives_the_unit_and_the_decimals() { + const registry = createTemporaryObject(slotRegistryComponent, testCase) + registry.byUnit("kg/m³", unitSlot) + const form = createTemporaryObject(readingForm, testCase, { slotRegistry: registry }) + const slot = findChild(form, "unitSlot") + verify(slot !== null) + compare(slot.field.name, "density") + compare(slot.field.unit, "kg/m³") + compare(slot.field.decimals, 3) + slot.text = "1.25" + compare(form.ready, true) + compare(form.previewLine, '{"density":1.25}') + } +} diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index d7e480de2..fb9a115b5 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -113,6 +113,7 @@ add_executable(morph_tests test_forms_rule_corpus.cpp test_forms_layout.cpp test_forms_field_bounds.cpp + test_forms_display_unit.cpp test_forms_instance_constraints.cpp test_widget_hints.cpp test_datetime.cpp diff --git a/tests/test_forms_display_unit.cpp b/tests/test_forms_display_unit.cpp new file mode 100644 index 000000000..081d11535 --- /dev/null +++ b/tests/test_forms_display_unit.cpp @@ -0,0 +1,208 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// A plain member's display unit and decimals: `FieldMeta::unit` / `::decimals` +// +// A DTO whose numeric members are plain `double`s has no type to carry a unit +// or a precision, so a renderer had nothing to show beside the control and no +// fraction-digit count to accept. These tests pin that the declaration reaches +// the served schema -- the unit as `ExtUnits`, the key a `Quantity` already +// carries, and the precision as `x-displayDecimals`, deliberately *not* +// `x-decimalPlaces`, which would hand the property the `{num,den,dp}` encoding +// a `double` cannot decode. +// +// src/qt/forms/tests/tst_DynamicFormDisplayUnit.qml pins the renderer half +// against the same key names. + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using morph::math::DecimalPlaces; + +// File-scope (not anonymous-namespaced): glaze's reflection needs a type with +// linkage. Same suppression, for the same reason, as +// tests/test_forms_field_bounds.cpp. +// NOLINTBEGIN(misc-use-internal-linkage) + +enum class FDUUnit : std::uint8_t { kg }; + +template <> +struct morph::units::UnitTraits { + static constexpr morph::units::UnitMeta meta(FDUUnit /*unit*/) noexcept { + return {.id = "kg", .display = "kg", .defaultDecimals = 3}; + } +}; + +using FDUMass = morph::units::Quantity; + +/// The motivating shape: lab readings held as plain `double`s. +struct FDUReadingAction { + double density = 0.0; + double temperature = 0.0; + std::int64_t specimens = 0; + double ratio = 0.0; + + static constexpr std::array fieldMetadata{ + morph::forms::FieldMeta{.field = "density", .unit = "kg/m³", .decimals = DecimalPlaces{3}}, + morph::forms::FieldMeta{.field = "temperature", .unit = "°C"}, + morph::forms::FieldMeta{.field = "specimens", .unit = "pcs"}, + }; +}; + +/// A `Quantity` already states both; a `FieldMeta` restating them is ignored. +struct FDUQuantityAction { + FDUMass mass; + + static constexpr std::array fieldMetadata{ + morph::forms::FieldMeta{.field = "mass", .unit = "lb", .decimals = DecimalPlaces{1}}, + }; +}; + +/// A precision past `kMaxDecimalPlaces` has no meaning and is not emitted. +struct FDUOverPreciseAction { + double value = 0.0; + + static constexpr std::array fieldMetadata{ + morph::forms::FieldMeta{.field = "value", .decimals = DecimalPlaces{morph::math::kMaxDecimalPlaces + 1}}, + }; +}; + +/// The same declaration one level down: the element of a repeated aggregate. +struct FDURow { + double sieve = 0.0; + double passing = 0.0; + + static constexpr std::array fieldMetadata{ + morph::forms::FieldMeta{.field = "sieve", .unit = "mm", .decimals = DecimalPlaces{1}}, + morph::forms::FieldMeta{.field = "passing", .unit = "%", .decimals = DecimalPlaces{2}}, + }; +}; + +struct FDUGradingAction { + std::vector rows; +}; + +/// The fluent builders, which must produce what the literal produces. +struct FDUFluentAction { + double density = 0.0; + + static constexpr std::array fieldMetadata{ + morph::forms::FieldMeta{.field = "density"}.withUnit("kg/m³").withDecimals(DecimalPlaces{3}), + }; +}; + +// NOLINTEND(misc-use-internal-linkage) + +namespace { + +glz::generic_u64 schemaDom(std::string const& schema) { + glz::generic_u64 dom{}; + REQUIRE_FALSE(schema.empty()); + REQUIRE_FALSE(glz::read_json(dom, schema)); + return dom; +} + +// The object schema a property's `items` (or the property itself) resolves +// to: a `$ref` into `$defs`, or the node inlined. +const glz::generic_u64& resolveObject(const glz::generic_u64& dom, const glz::generic_u64& node) { + if (node.contains("$ref")) { + constexpr std::string_view kPrefix = "#/$defs/"; + std::string const ref = node["$ref"].get(); + REQUIRE(ref.starts_with(kPrefix)); + return dom["$defs"][ref.substr(kPrefix.size())]; + } + return node; +} + +} // namespace + +TEST_CASE("Forms::FieldMeta::DisplayUnitAndDecimalsOnAPlainDouble", "[forms][field_meta][display_unit]") { + auto const dom = schemaDom(morph::forms::schemaJson()); + auto const& density = dom["properties"]["density"]; + + REQUIRE(density.contains("ExtUnits")); + CHECK(density["ExtUnits"]["unitAscii"].get() == "kg/m³"); + CHECK(density["ExtUnits"]["unitUnicode"].get() == "kg/m³"); + REQUIRE(density.contains("x-displayDecimals")); + CHECK(density["x-displayDecimals"].as() == 3); + // The exact-decimal key would re-type the member on the wire. + CHECK_FALSE(density.contains("x-decimalPlaces")); +} + +TEST_CASE("Forms::FieldMeta::UnitWithoutDecimalsEmitsOnlyTheUnit", "[forms][field_meta][display_unit]") { + auto const dom = schemaDom(morph::forms::schemaJson()); + auto const& temperature = dom["properties"]["temperature"]; + REQUIRE(temperature.contains("ExtUnits")); + CHECK(temperature["ExtUnits"]["unitAscii"].get() == "°C"); + CHECK_FALSE(temperature.contains("x-displayDecimals")); + + // An integral member takes a unit as readily as a floating one. + auto const& specimens = dom["properties"]["specimens"]; + REQUIRE(specimens.contains("ExtUnits")); + CHECK(specimens["ExtUnits"]["unitAscii"].get() == "pcs"); +} + +TEST_CASE("Forms::FieldMeta::UndeclaredDisplayUnitEmitsNothing", "[forms][field_meta][display_unit]") { + auto const dom = schemaDom(morph::forms::schemaJson()); + auto const& ratio = dom["properties"]["ratio"]; + CHECK_FALSE(ratio.contains("ExtUnits")); + CHECK_FALSE(ratio.contains("x-displayDecimals")); +} + +TEST_CASE("Forms::FieldMeta::DisplayUnitIsIgnoredOnAQuantity", "[forms][field_meta][display_unit]") { + auto const schema = morph::forms::schemaJson(); + auto const dom = schemaDom(schema); + auto const& mass = dom["properties"]["mass"]; + + CHECK(mass["x-decimalPlaces"].as() == 3); + CHECK_FALSE(mass.contains("x-displayDecimals")); + // The type's own unit is the only one anywhere in the schema. + CHECK_FALSE(schema.contains(R"("lb")")); + CHECK(resolveObject(dom, mass)["ExtUnits"]["unitAscii"].get() == "kg"); +} + +TEST_CASE("Forms::FieldMeta::DecimalsPastTheMaximumAreIgnored", "[forms][field_meta][display_unit]") { + auto const dom = schemaDom(morph::forms::schemaJson()); + CHECK_FALSE(dom["properties"]["value"].contains("x-displayDecimals")); +} + +TEST_CASE("Forms::FieldMeta::DisplayUnitReachesARepeatedAggregatesElement", "[forms][field_meta][display_unit]") { + auto const dom = schemaDom(morph::forms::schemaJson()); + auto const& items = dom["properties"]["rows"]["items"]; + auto const& row = resolveObject(dom, items); + + CHECK(row["properties"]["sieve"]["ExtUnits"]["unitAscii"].get() == "mm"); + CHECK(row["properties"]["sieve"]["x-displayDecimals"].as() == 1); + CHECK(row["properties"]["passing"]["ExtUnits"]["unitAscii"].get() == "%"); + CHECK(row["properties"]["passing"]["x-displayDecimals"].as() == 2); +} + +TEST_CASE("Forms::FieldMeta::DisplayUnitBuildersMatchTheLiteral", "[forms][field_meta][display_unit]") { + auto const fluent = schemaDom(morph::forms::schemaJson()); + auto const literal = schemaDom(morph::forms::schemaJson()); + CHECK(fluent["properties"]["density"]["ExtUnits"]["unitAscii"].get() == + literal["properties"]["density"]["ExtUnits"]["unitAscii"].get()); + CHECK(fluent["properties"]["density"]["x-displayDecimals"].as() == + literal["properties"]["density"]["x-displayDecimals"].as()); +} + +TEST_CASE("Forms::FieldMeta::DisplayUnitLeavesTheWireUntouched", "[forms][field_meta][display_unit]") { + FDUReadingAction reading{}; + reading.density = 2.5; + std::string json{}; + REQUIRE_FALSE(glz::write_json(reading, json)); + CHECK(json.contains(R"("density":2.5)")); + CHECK_FALSE(json.contains("kg/m")); + + FDUReadingAction decoded{}; + REQUIRE_FALSE(glz::read_json(decoded, R"({"density":1.25,"temperature":20,"specimens":3,"ratio":0.5})")); + std::string roundTripped{}; + REQUIRE_FALSE(glz::write_json(decoded, roundTripped)); + CHECK(roundTripped.contains(R"("density":1.25)")); +} From 693af4931cb438d6f16b1ca3172e2f7fd450721a Mon Sep 17 00:00:00 2001 From: yaraslau Date: Thu, 24 Sep 2026 22:58:26 +0200 Subject: [PATCH 2/3] forms: clear the clang-tidy findings on the display-unit change - annotateDisplayUnit becomes a template that is a no-op for a Quantity, so annotateBasicMemberProperty gains a plain call and no cognitive-complexity increment on a changed line; its operator[] writes sit in the same NOLINTBEGIN block emitDeclaredBound uses for the glaze DOM. - FieldMeta::unit / ::decimals keep their {} like every sibling member, with the redundant-member-init suppression saying why. - The test's $ref helper returns a copy and takes (dom, name, items), so it neither returns a reference to a parameter nor takes two swappable DOM references; schemaJson's const& result is bound by reference. Co-Authored-By: Claude Opus 5.5 (1M context) --- include/morph/forms/forms.hpp | 41 +++++++++++++++++++------------ tests/test_forms_display_unit.cpp | 18 ++++++++------ 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/include/morph/forms/forms.hpp b/include/morph/forms/forms.hpp index e9abb7f37..4209f66fb 100644 --- a/include/morph/forms/forms.hpp +++ b/include/morph/forms/forms.hpp @@ -290,6 +290,9 @@ struct FieldMeta { /// Presentation only: the unit never travels in the payload and nothing /// converts through it. **Ignored on a `Quantity` member**, whose unit is /// part of its type and already emitted. + // Spelled like every sibling's default, which a consumer's + // -Wmissing-field-initializers reads as "has a default". + // NOLINTNEXTLINE(readability-redundant-member-init) std::string_view unit{}; /// @brief Display and entry precision for a plain `double`/`float` member; @@ -301,6 +304,7 @@ struct FieldMeta { /// fraction digits to show and accept. **Ignored on a `Quantity` member** /// (its `x-decimalPlaces` is authoritative) and when it exceeds /// `morph::math::kMaxDecimalPlaces`. + // NOLINTNEXTLINE(readability-redundant-member-init) -- as `unit` above std::optional<::morph::math::DecimalPlaces> decimals{}; /// @brief Returns a copy with `placeholder` set to @p text. @@ -2221,24 +2225,33 @@ inline void annotateDeclaredBounds(glz::generic_u64& property, const FieldMeta& } /// @brief Stamps @p meta's display `unit` (as `ExtUnits`) and `decimals` (as -/// `x-displayDecimals`) onto @p property, for a non-`Quantity` member. +/// `x-displayDecimals`) onto @p property, unless @p Member is a +/// `Quantity`. /// /// `ExtUnits` is the key a `Quantity` already carries, so every reader of a /// unit -- a renderer's suffix label, `SlotRegistry.byUnit`, a view column -- -/// finds a plain member's unit where it finds a `Quantity`'s. +/// finds a plain member's unit where it finds a `Quantity`'s. A `Quantity`'s +/// own unit and precision are part of its type; a `FieldMeta` restating them +/// could only disagree, so for one this is a no-op. +/// @tparam Member The static type of the member being annotated. /// @param property Property node to annotate in place. /// @param meta The field's declared metadata. -inline void annotateDisplayUnit(glz::generic_u64& property, const FieldMeta& meta) { - if (!meta.unit.empty()) { - glz::generic_u64 units{}; - units["unitAscii"] = std::string{meta.unit}; - units["unitUnicode"] = std::string{meta.unit}; - property["ExtUnits"] = std::move(units); - } - if (meta.decimals.has_value() && meta.decimals->value <= ::morph::math::kMaxDecimalPlaces) { - property["x-displayDecimals"] = std::uint64_t{meta.decimals->value}; +// NOLINTBEGIN(cppcoreguidelines-pro-bounds-avoid-unchecked-container-access) -- glaze DOM requires operator[] +template +void annotateDisplayUnit(glz::generic_u64& property, const FieldMeta& meta) { + if constexpr (!units::isQuantity) { + if (!meta.unit.empty()) { + glz::generic_u64 units{}; + units["unitAscii"] = std::string{meta.unit}; + units["unitUnicode"] = std::string{meta.unit}; + property["ExtUnits"] = std::move(units); + } + if (meta.decimals.has_value() && meta.decimals->value <= ::morph::math::kMaxDecimalPlaces) { + property["x-displayDecimals"] = std::uint64_t{meta.decimals->value}; + } } } +// NOLINTEND(cppcoreguidelines-pro-bounds-avoid-unchecked-container-access) /// @brief Whether @p value satisfies every bound @p meta declares. /// @@ -2327,11 +2340,7 @@ void annotateBasicMemberProperty(glz::generic_u64& property, std::string_view na property["x-i18nKey"] = std::string{fieldMeta->i18nKey}; } annotateDeclaredBounds(property, *fieldMeta); - // A Quantity's unit and precision are part of its type and emitted - // below; a FieldMeta restating them could only disagree. - if constexpr (!units::isQuantity) { - annotateDisplayUnit(property, *fieldMeta); - } + annotateDisplayUnit(property, *fieldMeta); } if constexpr (units::isQuantity) { diff --git a/tests/test_forms_display_unit.cpp b/tests/test_forms_display_unit.cpp index 081d11535..11fe69385 100644 --- a/tests/test_forms_display_unit.cpp +++ b/tests/test_forms_display_unit.cpp @@ -108,9 +108,14 @@ glz::generic_u64 schemaDom(std::string const& schema) { return dom; } -// The object schema a property's `items` (or the property itself) resolves -// to: a `$ref` into `$defs`, or the node inlined. -const glz::generic_u64& resolveObject(const glz::generic_u64& dom, const glz::generic_u64& node) { +// Property @p name of the action's schema -- or, with @p items, the element +// schema of that array property -- with a `$ref` into `$defs` resolved. A +// copy, so nothing returned refers into a caller's temporary. +glz::generic_u64 resolvedProperty(const glz::generic_u64& dom, std::string const& name, bool items) { + glz::generic_u64 node = dom["properties"][name]; + if (items) { + node = glz::generic_u64{node["items"]}; + } if (node.contains("$ref")) { constexpr std::string_view kPrefix = "#/$defs/"; std::string const ref = node["$ref"].get(); @@ -156,7 +161,7 @@ TEST_CASE("Forms::FieldMeta::UndeclaredDisplayUnitEmitsNothing", "[forms][field_ } TEST_CASE("Forms::FieldMeta::DisplayUnitIsIgnoredOnAQuantity", "[forms][field_meta][display_unit]") { - auto const schema = morph::forms::schemaJson(); + auto const& schema = morph::forms::schemaJson(); auto const dom = schemaDom(schema); auto const& mass = dom["properties"]["mass"]; @@ -164,7 +169,7 @@ TEST_CASE("Forms::FieldMeta::DisplayUnitIsIgnoredOnAQuantity", "[forms][field_me CHECK_FALSE(mass.contains("x-displayDecimals")); // The type's own unit is the only one anywhere in the schema. CHECK_FALSE(schema.contains(R"("lb")")); - CHECK(resolveObject(dom, mass)["ExtUnits"]["unitAscii"].get() == "kg"); + CHECK(resolvedProperty(dom, "mass", false)["ExtUnits"]["unitAscii"].get() == "kg"); } TEST_CASE("Forms::FieldMeta::DecimalsPastTheMaximumAreIgnored", "[forms][field_meta][display_unit]") { @@ -174,8 +179,7 @@ TEST_CASE("Forms::FieldMeta::DecimalsPastTheMaximumAreIgnored", "[forms][field_m TEST_CASE("Forms::FieldMeta::DisplayUnitReachesARepeatedAggregatesElement", "[forms][field_meta][display_unit]") { auto const dom = schemaDom(morph::forms::schemaJson()); - auto const& items = dom["properties"]["rows"]["items"]; - auto const& row = resolveObject(dom, items); + auto const row = resolvedProperty(dom, "rows", true); CHECK(row["properties"]["sieve"]["ExtUnits"]["unitAscii"].get() == "mm"); CHECK(row["properties"]["sieve"]["x-displayDecimals"].as() == 1); From 16811b3057fe5753e57439efeaeddb811a0f2361 Mon Sep 17 00:00:00 2001 From: yaraslau Date: Thu, 24 Sep 2026 23:58:18 +0200 Subject: [PATCH 3/3] tests(forms): copy-initialise the resolved node -- generic_u64{x} is an array of x under Clang/GCC The clang-tidy follow-up rewrote the display-unit test's $ref helper and spelled one copy as `glz::generic_u64{node["items"]}`. That is list-initialisation: MSVC picks the copy constructor, Clang and GCC pick the initializer_list one and build a one-element array, so every lookup below it threw `std::get: wrong index for variant`. CI reported it on gcc-debug, clang-tsan, clang-ubsan, Qt6 WebSockets and clangcl-debug; reproduced locally with clang-cl (4 of 46 assertions failing) and green after this change. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_forms_display_unit.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/test_forms_display_unit.cpp b/tests/test_forms_display_unit.cpp index 11fe69385..1635f76c8 100644 --- a/tests/test_forms_display_unit.cpp +++ b/tests/test_forms_display_unit.cpp @@ -112,10 +112,11 @@ glz::generic_u64 schemaDom(std::string const& schema) { // schema of that array property -- with a `$ref` into `$defs` resolved. A // copy, so nothing returned refers into a caller's temporary. glz::generic_u64 resolvedProperty(const glz::generic_u64& dom, std::string const& name, bool items) { - glz::generic_u64 node = dom["properties"][name]; - if (items) { - node = glz::generic_u64{node["items"]}; - } + // Copy-initialised, never brace-initialised: `generic_u64{x}` is + // list-initialisation, which under GCC/Clang builds a one-element array + // holding `x` rather than a copy of it. + const glz::generic_u64& property = dom["properties"][name]; + glz::generic_u64 node = items ? property["items"] : property; if (node.contains("$ref")) { constexpr std::string_view kPrefix = "#/$defs/"; std::string const ref = node["$ref"].get();