Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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()
{
Expand Down Expand Up @@ -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");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<ViewDiagnostic>
Expand All @@ -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"));
Expand All @@ -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));
Expand Down Expand Up @@ -99,6 +107,28 @@ public void Deserialize_WrongFormatVersion_Throws()
Throws.TypeOf<InvalidDataException>());
}

[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<InvalidDataException>(),
"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<InvalidDataException>(),
"a null entry must fail the load, not surface at compose time");
}

[Test]
public void RoundTrip_PreservesAddNode_WithParentIndexAndIdentity()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public static ViewDefinitionModel Apply(ViewDefinitionModel shipped, ViewDefinit
var setLabel = new Dictionary<string, string>(StringComparer.Ordinal);
var hide = new HashSet<string>(StringComparer.Ordinal);
var reorder = new Dictionary<string, IReadOnlyList<string>>(StringComparer.Ordinal);
var setWritingSystems = new Dictionary<string, IReadOnlyList<string>>(StringComparer.Ordinal);
var addByParent = new Dictionary<string, List<ViewOverrideOperation>>(StringComparer.Ordinal);
var duplicateByParent = new Dictionary<string, List<ViewOverrideOperation>>(StringComparer.Ordinal);

Expand All @@ -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;
Expand All @@ -64,7 +70,8 @@ public static ViewDefinitionModel Apply(ViewDefinitionModel shipped, ViewDefinit
var diagnostics = new List<ViewDiagnostic>(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);

Expand All @@ -81,6 +88,7 @@ private sealed class ApplyContext
private readonly Dictionary<string, string> _setLabel;
private readonly HashSet<string> _hide;
private readonly Dictionary<string, IReadOnlyList<string>> _reorder;
private readonly Dictionary<string, IReadOnlyList<string>> _setWritingSystems;
private readonly Dictionary<string, List<ViewOverrideOperation>> _addByParent;
private readonly Dictionary<string, List<ViewOverrideOperation>> _duplicateByParent;
private readonly Dictionary<string, ViewNode> _baseById;
Expand All @@ -92,6 +100,7 @@ public ApplyContext(
Dictionary<string, string> setLabel,
HashSet<string> hide,
Dictionary<string, IReadOnlyList<string>> reorder,
Dictionary<string, IReadOnlyList<string>> setWritingSystems,
Dictionary<string, List<ViewOverrideOperation>> addByParent,
Dictionary<string, List<ViewOverrideOperation>> duplicateByParent,
Dictionary<string, ViewNode> baseById,
Expand All @@ -101,6 +110,7 @@ public ApplyContext(
_setLabel = setLabel;
_hide = hide;
_reorder = reorder;
_setWritingSystems = setWritingSystems;
_addByParent = addByParent;
_duplicateByParent = duplicateByParent;
_baseById = baseById;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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<ViewNode> 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<ViewNode> children, IReadOnlyList<string> 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).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ public enum ViewOverrideOperationKind
/// <summary>Reorder a node's children (same child set, different order).</summary>
ReorderChildren,

/// <summary>
/// Restrict a field to a subset of its writing systems, in display order.
/// </summary>
SetVisibleWritingSystems,

/// <summary>A node present in the shipped definition that the override removed/hid.</summary>
HideNode,

Expand Down Expand Up @@ -53,13 +58,15 @@ public ViewOverrideOperation(
string field = null,
string editor = null,
string sourceStableId = null,
string writingSystem = null)
string writingSystem = null,
IReadOnlyList<string> writingSystems = null)
{
Kind = kind;
StableId = stableId ?? throw new ArgumentNullException(nameof(stableId));
Visibility = visibility;
Label = label;
ChildOrder = childOrder ?? (IReadOnlyList<string>)Array.Empty<string>();
WritingSystems = writingSystems ?? (IReadOnlyList<string>)Array.Empty<string>();
ParentStableId = parentStableId;
Index = index;
NodeKind = nodeKind;
Expand All @@ -83,6 +90,11 @@ public ViewOverrideOperation(
/// <summary>New child order (StableIds) for <see cref="ViewOverrideOperationKind.ReorderChildren"/>.</summary>
public IReadOnlyList<string> ChildOrder { get; }

/// <summary>
/// The writing systems a field is restricted to, in display order.
/// </summary>
public IReadOnlyList<string> WritingSystems { get; }

/// <summary>For <see cref="ViewOverrideOperationKind.AddNode"/>: the parent the new node is inserted under.</summary>
public string ParentStableId { get; }

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -170,9 +184,10 @@ public ViewDefinitionOverride(
/// the diff keys on <see cref="ViewNode.StableId"/>
/// -- 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.
/// </summary>
public static class ViewDefinitionOverrideDiffer
{
Expand Down Expand Up @@ -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);
}

Expand All @@ -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,
Expand Down Expand Up @@ -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<string> shipped, IReadOnlyList<string> overridden)
=> (shipped ?? Array.Empty<string>()).SequenceEqual(
overridden ?? Array.Empty<string>(), StringComparer.OrdinalIgnoreCase);

private static int CompareOperations(ViewOverrideOperation a, ViewOverrideOperation b)
{
var byId = string.CompareOrdinal(a.StableId, b.StableId);
Expand Down
Loading
Loading