From 9d6d84047cf3f1c2acd9d98a9dd4636423c2233d Mon Sep 17 00:00:00 2001 From: mark-sil <83427558+mark-sil@users.noreply.github.com> Date: Thu, 20 Aug 2026 15:20:59 -0400 Subject: [PATCH] LT-22691: Add the SetVisibleWritingSystems override operation Store a per-field writing-system restriction in the project view override. Note: Nothing writes this operation at runtime yet. Co-Authored-By: Claude Fable 5 --- .../ViewDefinitionOverrideApplierTests.cs | 101 ++++++++++++++++++ ...ewDefinitionOverrideJsonSerializerTests.cs | 32 +++++- .../ViewDefinitionOverrideApplier.cs | 27 +++-- .../ViewDefinitionOverrideDiffer.cs | 48 ++++++++- .../ViewDefinitionOverrideJsonSerializer.cs | 21 +++- 5 files changed, 215 insertions(+), 14 deletions(-) diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionOverrideApplierTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionOverrideApplierTests.cs index 2011ef3afe..36dd1f4363 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionOverrideApplierTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionOverrideApplierTests.cs @@ -25,6 +25,12 @@ private static ViewNode GroupNode(string id, string label, params ViewNode[] chi EditorClassification.GroupingNone, null, ViewVisibility.Always, ViewExpansion.Expanded, false, null, children); + private static ViewNode WsFieldNode(string id, string[] visibleWritingSystems) + => new ViewNode(id, ViewNodeKind.Field, "A", null, "Form", "multistring", + EditorClassification.Known, "all vernacular", ViewVisibility.Always, + ViewExpansion.NotApplicable, false, null, null, + visibleWritingSystems: visibleWritingSystems); + private static ViewDefinitionModel Model(params ViewNode[] roots) => new ViewDefinitionModel("LexEntry", "detail", "jtview", roots, null); @@ -72,6 +78,45 @@ public void Apply_PreservesNodeFieldsNoOperationTouches() "an enum option list survives the rebuild"); } + // The writing-system subset a user picks for one field, recorded against its stable id. + [Test] + public void Apply_SetVisibleWritingSystems_RestrictsThatFieldOnly() + { + var shipped = Model(GroupNode("g", "Group", + WsFieldNode("g/a", new[] { "fr", "seh", "aka" }), + FieldNode("g/b", "B"))); + var patch = new ViewDefinitionOverride("LexEntry", "detail", "jtview", + new[] + { + new ViewOverrideOperation(ViewOverrideOperationKind.SetVisibleWritingSystems, "g/a", + writingSystems: new[] { "fr" }) + }, null); + + var applied = ViewDefinitionOverrideApplier.Apply(shipped, patch); + + Assert.That(applied.Roots[0].Children[0].VisibleWritingSystems, Is.EqualTo(new[] { "fr" }), + "the patched field is restricted to the chosen writing systems"); + Assert.That(applied.Roots[0].Children[1].VisibleWritingSystems, Is.Null, + "a field the patch does not name is untouched"); + } + + [Test] + public void Apply_SetVisibleWritingSystems_PreservesTheChosenOrder() + { + var shipped = Model(GroupNode("g", "Group", WsFieldNode("g/a", new[] { "fr", "seh" }))); + var patch = new ViewDefinitionOverride("LexEntry", "detail", "jtview", + new[] + { + new ViewOverrideOperation(ViewOverrideOperationKind.SetVisibleWritingSystems, "g/a", + writingSystems: new[] { "seh", "fr" }) + }, null); + + var applied = ViewDefinitionOverrideApplier.Apply(shipped, patch); + + Assert.That(applied.Roots[0].Children[0].VisibleWritingSystems, + Is.EqualTo(new[] { "seh", "fr" }), "display order is the user's order, not the shipped one"); + } + [Test] public void Apply_AddNode_InsertsAtParentIndex() { @@ -171,5 +216,61 @@ public void RoundTrip_DiffThenApply_ReproducesCustomized_Reorder() Assert.That(applied.ToSnapshot(), Is.EqualTo(customized.ToSnapshot())); } + + // VisibleWritingSystems is outside ToSnapshot(), so these round trips assert it directly. + [Test] + public void RoundTrip_DiffThenApply_ReproducesCustomized_WritingSystemRestriction() + { + var shipped = Model(GroupNode("g", "Group", WsFieldNode("g/a", new[] { "fr", "seh" }))); + var customized = Model(GroupNode("g", "Group", WsFieldNode("g/a", new[] { "seh" }))); + + var patch = ViewDefinitionOverrideDiffer.Diff(shipped, customized); + var applied = ViewDefinitionOverrideApplier.Apply(shipped, patch); + + Assert.That(applied.Roots[0].Children[0].VisibleWritingSystems, + Is.EqualTo(new[] { "seh" }), "the differ captures the restriction and apply replays it"); + } + + [Test] + public void RoundTrip_DiffThenApply_ClearedRestrictionComesBackNull() + { + var shipped = Model(GroupNode("g", "Group", WsFieldNode("g/a", new[] { "fr", "seh" }))); + var customized = Model(GroupNode("g", "Group", WsFieldNode("g/a", null))); + + var patch = ViewDefinitionOverrideDiffer.Diff(shipped, customized); + var applied = ViewDefinitionOverrideApplier.Apply(shipped, patch); + + Assert.That(applied.Roots[0].Children[0].VisibleWritingSystems, Is.Null, + "clearing normalizes to null, the model's own \"unrestricted\""); + } + + [Test] + public void Diff_CaseOnlyWritingSystemDifference_EmitsNoOp() + { + var shipped = Model(GroupNode("g", "Group", WsFieldNode("g/a", new[] { "fr-FR" }))); + var customized = Model(GroupNode("g", "Group", WsFieldNode("g/a", new[] { "fr-fr" }))); + + var patch = ViewDefinitionOverrideDiffer.Diff(shipped, customized); + + Assert.That(patch.Operations, Is.Empty, + "the composer matches tags case-insensitively, so these render identically"); + } + + [Test] + public void Diff_AddedNodeWithWsRestriction_ReportsDiagnostic() + { + var shipped = Model(GroupNode("g", "Group", FieldNode("g/a", "A"))); + var customized = Model(GroupNode("g", "Group", FieldNode("g/a", "A"), + WsFieldNode("g/new", new[] { "fr" }))); + + var patch = ViewDefinitionOverrideDiffer.Diff(shipped, customized); + + Assert.That(patch.Operations.Single(o => + o.Kind == ViewOverrideOperationKind.AddNode).StableId, + Is.EqualTo("g/new"), "the added node itself is still representable"); + Assert.That(patch.Diagnostics.Any( + d => d.Code == "override-added-ws-restriction-dropped"), Is.True, + "the dropped restriction is reported, never silent"); + } } } diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionOverrideJsonSerializerTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionOverrideJsonSerializerTests.cs index edf6b48a0c..6f58e1ef1c 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionOverrideJsonSerializerTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionOverrideJsonSerializerTests.cs @@ -25,6 +25,8 @@ private static ViewDefinitionOverride SampleWithAllOpKinds() new ViewOverrideOperation(ViewOverrideOperationKind.SetLabel, "b", label: "Headword"), new ViewOverrideOperation(ViewOverrideOperationKind.ReorderChildren, "g", childOrder: new[] { "g/b", "g/a" }), + new ViewOverrideOperation(ViewOverrideOperationKind.SetVisibleWritingSystems, "w", + writingSystems: new[] { "seh", "fr" }), new ViewOverrideOperation(ViewOverrideOperationKind.HideNode, "c") }; var diags = new List @@ -47,7 +49,7 @@ public void RoundTrip_PreservesHeaderOperationsAndDiagnostics() Assert.That(restored.LayoutName, Is.EqualTo("detail")); Assert.That(restored.LayoutType, Is.EqualTo("jtview")); - Assert.That(restored.Operations.Count, Is.EqualTo(4)); + Assert.That(restored.Operations.Count, Is.EqualTo(5)); var vis = restored.Operations.Single(o => o.Kind == ViewOverrideOperationKind.SetVisibility); Assert.That(vis.StableId, Is.EqualTo("a")); @@ -64,6 +66,12 @@ public void RoundTrip_PreservesHeaderOperationsAndDiagnostics() var hide = restored.Operations.Single(o => o.Kind == ViewOverrideOperationKind.HideNode); Assert.That(hide.StableId, Is.EqualTo("c")); + var writingSystems = restored.Operations + .Single(o => o.Kind == ViewOverrideOperationKind.SetVisibleWritingSystems); + Assert.That(writingSystems.StableId, Is.EqualTo("w")); + Assert.That(writingSystems.WritingSystems, Is.EqualTo(new[] { "seh", "fr" }), + "the chosen writing systems round-trip in display order"); + Assert.That(restored.Diagnostics.Count, Is.EqualTo(1)); Assert.That(restored.Diagnostics[0].Code, Is.EqualTo("override-added-node")); Assert.That(restored.Diagnostics[0].Severity, Is.EqualTo(ViewDiagnosticSeverity.Info)); @@ -99,6 +107,28 @@ public void Deserialize_WrongFormatVersion_Throws() Throws.TypeOf()); } + [Test] + public void Deserialize_NullWritingSystemEntry_Throws() + { + const string json = "{ \"formatVersion\": 1, \"class\": \"LexEntry\", \"name\": \"detail\"," + + " \"type\": \"jtview\", \"operations\": [ { \"op\": \"setVisibleWritingSystems\"," + + " \"id\": \"w\", \"writingSystems\": [ \"seh\", null ] } ] }"; + Assert.That(() => ViewDefinitionOverrideJsonSerializer.Deserialize(json), + Throws.TypeOf(), + "a null entry must fail the load, not flow into the node model"); + } + + [Test] + public void Deserialize_NullChildOrderEntry_Throws() + { + const string json = "{ \"formatVersion\": 1, \"class\": \"LexEntry\", \"name\": \"detail\"," + + " \"type\": \"jtview\", \"operations\": [ { \"op\": \"reorderChildren\"," + + " \"id\": \"g\", \"childOrder\": [ \"g/a\", null ] } ] }"; + Assert.That(() => ViewDefinitionOverrideJsonSerializer.Deserialize(json), + Throws.TypeOf(), + "a null entry must fail the load, not surface at compose time"); + } + [Test] public void RoundTrip_PreservesAddNode_WithParentIndexAndIdentity() { diff --git a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideApplier.cs b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideApplier.cs index 3e3174fd66..e8ad1f756e 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideApplier.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideApplier.cs @@ -30,6 +30,7 @@ public static ViewDefinitionModel Apply(ViewDefinitionModel shipped, ViewDefinit var setLabel = new Dictionary(StringComparer.Ordinal); var hide = new HashSet(StringComparer.Ordinal); var reorder = new Dictionary>(StringComparer.Ordinal); + var setWritingSystems = new Dictionary>(StringComparer.Ordinal); var addByParent = new Dictionary>(StringComparer.Ordinal); var duplicateByParent = new Dictionary>(StringComparer.Ordinal); @@ -49,6 +50,11 @@ public static ViewDefinitionModel Apply(ViewDefinitionModel shipped, ViewDefinit case ViewOverrideOperationKind.ReorderChildren: reorder[op.StableId] = op.ChildOrder; break; + case ViewOverrideOperationKind.SetVisibleWritingSystems: + // An empty op clears the restriction; null is the model's "unrestricted". + setWritingSystems[op.StableId] = + op.WritingSystems.Count > 0 ? op.WritingSystems : null; + break; case ViewOverrideOperationKind.AddNode: AppendByParent(addByParent, op); break; @@ -64,7 +70,8 @@ public static ViewDefinitionModel Apply(ViewDefinitionModel shipped, ViewDefinit var diagnostics = new List(shipped.Diagnostics); var baseById = FlattenBase(shipped.Roots); var context = new ApplyContext( - setVisibility, setLabel, hide, reorder, addByParent, duplicateByParent, baseById, diagnostics); + setVisibility, setLabel, hide, reorder, setWritingSystems, addByParent, duplicateByParent, + baseById, diagnostics); var newRoots = context.RebuildChildren(RootParentKey, shipped.Roots); @@ -81,6 +88,7 @@ private sealed class ApplyContext private readonly Dictionary _setLabel; private readonly HashSet _hide; private readonly Dictionary> _reorder; + private readonly Dictionary> _setWritingSystems; private readonly Dictionary> _addByParent; private readonly Dictionary> _duplicateByParent; private readonly Dictionary _baseById; @@ -92,6 +100,7 @@ public ApplyContext( Dictionary setLabel, HashSet hide, Dictionary> reorder, + Dictionary> setWritingSystems, Dictionary> addByParent, Dictionary> duplicateByParent, Dictionary baseById, @@ -101,6 +110,7 @@ public ApplyContext( _setLabel = setLabel; _hide = hide; _reorder = reorder; + _setWritingSystems = setWritingSystems; _addByParent = addByParent; _duplicateByParent = duplicateByParent; _baseById = baseById; @@ -154,7 +164,10 @@ private ViewNode RebuildNode(ViewNode node) var visibility = _setVisibility.TryGetValue(node.StableId, out var v) ? v : node.Visibility; var label = _setLabel.TryGetValue(node.StableId, out var l) ? l : node.Label; var children = RebuildChildren(node.StableId, node.Children); - return CloneWith(node, visibility, label, children); + var writingSystems = _setWritingSystems.TryGetValue(node.StableId, out var w) + ? w + : node.VisibleWritingSystems; + return CloneWith(node, visibility, label, children, writingSystems); } private ViewNode CreateAddedNode(ViewOverrideOperation addOp) @@ -286,17 +299,17 @@ void Visit(ViewNode node) return map; } - // Reconstruct an immutable node with overridden visibility/label/children, copying every - // other field. Every trailing optional constructor argument must be passed, or that - // field is stripped. - private static ViewNode CloneWith(ViewNode n, ViewVisibility visibility, string label, IReadOnlyList children) + // Reconstruct an immutable node with the overridden fields, copying every other one. + // Every trailing optional constructor argument must be passed, or that field is stripped. + private static ViewNode CloneWith(ViewNode n, ViewVisibility visibility, string label, + IReadOnlyList children, IReadOnlyList visibleWritingSystems) => new ViewNode( n.StableId, n.Kind, label, n.Abbreviation, n.Field, n.RawEditor, n.EditorClassification, n.WritingSystem, visibility, n.Expansion, n.Indented, n.TargetLayout, children, n.LocalizationKey, n.AutomationId, n.Routing, n.BoldEmphasis, n.FontScalePercent, n.MenuId, n.ContextMenuId, n.HotlinksId, n.GhostField, n.GhostWs, n.GhostClass, n.GhostLabel, n.ForVariant, n.CustomEditorClass, n.CustomEditorAssembly, n.GhostInitMethod, n.Condition, - n.ChooserLinks, n.EnumStringList, n.VisibleWritingSystems, n.ToggleValue); + n.ChooserLinks, n.EnumStringList, visibleWritingSystems, n.ToggleValue); // Copy a (leaf) node under a new StableId; AutomationId is dropped so the duplicate gets a fresh, // non-colliding identity (the renderer derives one from the new StableId by convention). diff --git a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideDiffer.cs b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideDiffer.cs index d0bf1c5f59..7c57427068 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideDiffer.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideDiffer.cs @@ -24,6 +24,11 @@ public enum ViewOverrideOperationKind /// Reorder a node's children (same child set, different order). ReorderChildren, + /// + /// Restrict a field to a subset of its writing systems, in display order. + /// + SetVisibleWritingSystems, + /// A node present in the shipped definition that the override removed/hid. HideNode, @@ -53,13 +58,15 @@ public ViewOverrideOperation( string field = null, string editor = null, string sourceStableId = null, - string writingSystem = null) + string writingSystem = null, + IReadOnlyList writingSystems = null) { Kind = kind; StableId = stableId ?? throw new ArgumentNullException(nameof(stableId)); Visibility = visibility; Label = label; ChildOrder = childOrder ?? (IReadOnlyList)Array.Empty(); + WritingSystems = writingSystems ?? (IReadOnlyList)Array.Empty(); ParentStableId = parentStableId; Index = index; NodeKind = nodeKind; @@ -83,6 +90,11 @@ public ViewOverrideOperation( /// New child order (StableIds) for . public IReadOnlyList ChildOrder { get; } + /// + /// The writing systems a field is restricted to, in display order. + /// + public IReadOnlyList WritingSystems { get; } + /// For : the parent the new node is inserted under. public string ParentStableId { get; } @@ -115,6 +127,8 @@ public override string ToString() return $"setLabel {StableId} -> {Label}"; case ViewOverrideOperationKind.ReorderChildren: return $"reorderChildren {StableId} -> [{string.Join(",", ChildOrder)}]"; + case ViewOverrideOperationKind.SetVisibleWritingSystems: + return $"setVisibleWritingSystems {StableId} -> [{string.Join(",", WritingSystems)}]"; case ViewOverrideOperationKind.HideNode: return $"hideNode {StableId}"; case ViewOverrideOperationKind.AddNode: @@ -170,9 +184,10 @@ public ViewDefinitionOverride( /// the diff keys on /// -- the identity scheme the semantic baselines already use -- instead of a second one. /// - /// Representable edits (visibility, label, child reorder, node hidden) become operations; everything - /// else (added nodes, changed binding/editor/kind) becomes an explicit diagnostic. Output is - /// deterministic: operations and diagnostics are ordered by StableId then kind. + /// Representable edits (visibility, label, child reorder, writing-system restriction, node + /// hidden) become operations; everything else (added nodes, changed binding/editor/kind) + /// becomes an explicit diagnostic. Output is deterministic: operations and diagnostics are + /// ordered by StableId then kind. /// public static class ViewDefinitionOverrideDiffer { @@ -239,6 +254,14 @@ public static ViewDefinitionOverride Diff(ViewDefinitionModel shipped, ViewDefin stableId, label: overriddenNode.Label)); } + if (!WritingSystemsEqual(shippedNode.VisibleWritingSystems, + overriddenNode.VisibleWritingSystems)) + { + operations.Add(new ViewOverrideOperation( + ViewOverrideOperationKind.SetVisibleWritingSystems, stableId, + writingSystems: overriddenNode.VisibleWritingSystems)); + } + AppendReorderIfNeeded(operations, stableId, shippedNode, overriddenNode); } @@ -257,6 +280,17 @@ public static ViewDefinitionOverride Diff(ViewDefinitionModel shipped, ViewDefin parentStableId: place.ParentId, index: place.Index, nodeKind: added.Kind, field: added.Field, editor: added.RawEditor, writingSystem: added.WritingSystem)); + + // An AddNode op cannot carry a writing-system restriction; report the drop + // rather than lose it silently. + if (added.VisibleWritingSystems != null && added.VisibleWritingSystems.Count > 0) + { + diagnostics.Add(new ViewDiagnostic(ViewDiagnosticSeverity.Warning, + "override-added-ws-restriction-dropped", + $"added node '{stableId}' restricts its writing systems; the restriction" + + " is not representable on an AddNode op", + stableId)); + } } AppendReorderIfNeeded(operations, RootParentKey, @@ -297,6 +331,12 @@ private static void AppendReorderIfNeeded( key, childOrder: overriddenOrder)); } + // Null and empty both mean "no restriction"; order matters (it is the display order). + // Tags compare case-insensitively, matching how the composer resolves them. + private static bool WritingSystemsEqual(IReadOnlyList shipped, IReadOnlyList overridden) + => (shipped ?? Array.Empty()).SequenceEqual( + overridden ?? Array.Empty(), StringComparer.OrdinalIgnoreCase); + private static int CompareOperations(ViewOverrideOperation a, ViewOverrideOperation b) { var byId = string.CompareOrdinal(a.StableId, b.StableId); diff --git a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideJsonSerializer.cs b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideJsonSerializer.cs index 018a6cfdf6..b64ee49add 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideJsonSerializer.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideJsonSerializer.cs @@ -26,6 +26,7 @@ public static class ViewDefinitionOverrideJsonSerializer { ViewOverrideOperationKind.SetVisibility, "setVisibility" }, { ViewOverrideOperationKind.SetLabel, "setLabel" }, { ViewOverrideOperationKind.ReorderChildren, "reorderChildren" }, + { ViewOverrideOperationKind.SetVisibleWritingSystems, "setVisibleWritingSystems" }, { ViewOverrideOperationKind.HideNode, "hideNode" }, { ViewOverrideOperationKind.AddNode, "addNode" }, { ViewOverrideOperationKind.DuplicateNode, "duplicateNode" } @@ -94,6 +95,9 @@ private static JObject WriteOperation(ViewOverrideOperation op) case ViewOverrideOperationKind.ReorderChildren: o["childOrder"] = new JArray(op.ChildOrder); break; + case ViewOverrideOperationKind.SetVisibleWritingSystems: + o["writingSystems"] = new JArray(op.WritingSystems); + break; case ViewOverrideOperationKind.HideNode: break; case ViewOverrideOperationKind.AddNode: @@ -132,8 +136,11 @@ private static ViewOverrideOperation ReadOperation(JToken token) case ViewOverrideOperationKind.SetLabel: return new ViewOverrideOperation(kind, stableId, label: (string)o["label"]); case ViewOverrideOperationKind.ReorderChildren: - var order = ((JArray)o["childOrder"] ?? new JArray()).Select(t => (string)t).ToList(); - return new ViewOverrideOperation(kind, stableId, childOrder: order); + return new ViewOverrideOperation(kind, stableId, + childOrder: ReadStringList(o, "childOrder")); + case ViewOverrideOperationKind.SetVisibleWritingSystems: + return new ViewOverrideOperation(kind, stableId, + writingSystems: ReadStringList(o, "writingSystems")); case ViewOverrideOperationKind.AddNode: var addKindText = (string)o["nodeKind"]; var addKind = addKindText == null @@ -173,6 +180,16 @@ private static ViewDiagnostic ReadDiagnostic(JToken token) return new ViewDiagnostic(severity, (string)o["code"], (string)o["message"], (string)o["path"]); } + // Reads a string-array field. To prevent a null/empty entry from flowing + // into the node model, fail the load with InvalidDataException. + private static List ReadStringList(JObject o, string field) + { + var list = ((JArray)o[field] ?? new JArray()).Select(t => (string)t).ToList(); + if (list.Any(string.IsNullOrEmpty)) + throw new InvalidDataException($"Null or empty {field} entry in override patch."); + return list; + } + // Parses an enum value from committed JSON, turning a null/garbage token into a controlled // InvalidDataException (the load path catches it) rather than a raw ArgumentException/NRE. private static TEnum ParseEnum(string text, string field) where TEnum : struct