Repository navigation
Find tied containers' descendants regardless of insertion order #557
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -227,12 +227,49 @@ public bool FindDepthFirst(TOffset offset, Direction dir, out Annotation<TOffset | |
| if (Find(offset, dir, out result)) | ||
| return true; | ||
|
|
||
| if (!result.IsLeaf && result.Range.Contains(offset, dir)) | ||
| return result.Children.FindDepthFirst(offset, dir, out result); | ||
| // Several containers can share the same range while only one of them owns the children that lead to the | ||
| // offset, so every one of them has to be tried, not just the one Find happened to return. | ||
| Annotation<TOffset> candidate = result; | ||
| foreach (Annotation<TOffset> tied in GetSameRange(candidate)) | ||
| { | ||
| if ( | ||
| !tied.IsLeaf | ||
| && tied.Range.Contains(offset, dir) | ||
| && tied.Children.FindDepthFirst(offset, dir, out Annotation<TOffset> inner) | ||
| ) | ||
| { | ||
| result = inner; | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| if (!candidate.IsLeaf && candidate.Range.Contains(offset, dir)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI: F2. On a miss, |
||
| return candidate.Children.FindDepthFirst(offset, dir, out result); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI: F1. On a miss, |
||
|
|
||
| return false; | ||
| } | ||
|
|
||
| private IEnumerable<Annotation<TOffset>> GetSameRange(Annotation<TOffset> ann) | ||
| { | ||
| yield return ann; | ||
| if (ann == Begin || ann == End) | ||
| yield break; | ||
|
|
||
| for ( | ||
| Annotation<TOffset> cur = ann.Prev; | ||
| cur != Begin && cur.Range.CompareTo(ann.Range) == 0; | ||
| cur = cur.Prev | ||
| ) | ||
| { | ||
| yield return cur; | ||
| } | ||
|
|
||
| for (Annotation<TOffset> cur = ann.Next; cur != End && cur.Range.CompareTo(ann.Range) == 0; cur = cur.Next) | ||
| { | ||
| yield return cur; | ||
| } | ||
| } | ||
|
|
||
| public IEnumerable<Annotation<TOffset>> GetNodes(TOffset start, TOffset end) | ||
| { | ||
| return GetNodes(Range<TOffset>.Create(start, end)); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| using System.Reflection; | ||
| using NUnit.Framework; | ||
| using SIL.Machine.DataStructures; | ||
| using SIL.Machine.FeatureModel; | ||
|
|
||
| namespace SIL.Machine.Annotations; | ||
|
|
||
| [TestFixture] | ||
| public class AnnotationTiedContainerTests | ||
| { | ||
| private const int ContainerCount = 4; | ||
|
|
||
| private static Shape CreateShape(int seed, out ShapeNode[] nodes) | ||
| { | ||
| var annotations = new AnnotationList<ShapeNode>(); | ||
| typeof(BidirList<Annotation<ShapeNode>>) | ||
| .GetField("_rand", BindingFlags.Instance | BindingFlags.NonPublic)! | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI: F4. Because |
||
| .SetValue(annotations, new Random(seed)); | ||
| var shape = new Shape(begin => new ShapeNode(FeatureStruct.New().Value), annotations); | ||
| nodes = new ShapeNode[3]; | ||
| for (int i = 0; i < nodes.Length; i++) | ||
| nodes[i] = shape.Add(FeatureStruct.New().Value); | ||
| return shape; | ||
| } | ||
|
|
||
| private static Annotation<ShapeNode> NewContainer(ShapeNode[] nodes) | ||
| { | ||
| return new Annotation<ShapeNode>(Range<ShapeNode>.Create(nodes[0], nodes[2]), FeatureStruct.New().Value); | ||
| } | ||
|
|
||
| // Only the tied container at ownerIndex owns the segments, so the one Find returns varies with insertion order. | ||
| [Test] | ||
| public void FindDepthFirst_TiedContainers_FindsSegmentInOwner( | ||
| [Values(0, 1, 2, 3, 4, 5)] int seed, | ||
| [Range(0, ContainerCount - 1)] int ownerIndex, | ||
| [Values] Direction dir | ||
| ) | ||
| { | ||
| Shape shape = CreateShape(seed, out ShapeNode[] nodes); | ||
| for (int i = 0; i < ContainerCount; i++) | ||
| { | ||
| Annotation<ShapeNode> container = NewContainer(nodes); | ||
| if (i == ownerIndex) | ||
| container.Children.AddRange(nodes.Select(n => n.Annotation)); | ||
| shape.Annotations.Add(container, false); | ||
| } | ||
|
|
||
| Assert.That(shape.Annotations.FindDepthFirst(nodes[1], dir, out Annotation<ShapeNode> result), Is.True); | ||
| Assert.That(result, Is.SameAs(nodes[1].Annotation)); | ||
| } | ||
|
|
||
| // The owner is itself a container of tied containers, only one of which owns the segment annotations. | ||
| [Test] | ||
| public void FindDepthFirst_NestedTiedContainers_FindsSegmentInInnermostOwner( | ||
| [Values(0, 1, 2, 3, 4, 5)] int seed, | ||
| [Range(0, ContainerCount - 1)] int outerOwnerIndex, | ||
| [Range(0, ContainerCount - 1)] int innerOwnerIndex, | ||
| [Values] Direction dir | ||
| ) | ||
| { | ||
| Shape shape = CreateShape(seed, out ShapeNode[] nodes); | ||
| for (int i = 0; i < ContainerCount; i++) | ||
| { | ||
| Annotation<ShapeNode> outer = NewContainer(nodes); | ||
| if (i == outerOwnerIndex) | ||
| { | ||
| for (int j = 0; j < ContainerCount; j++) | ||
| { | ||
| Annotation<ShapeNode> inner = NewContainer(nodes); | ||
| if (j == innerOwnerIndex) | ||
| inner.Children.AddRange(nodes.Select(n => n.Annotation)); | ||
| outer.Children.Add(inner, false); | ||
| } | ||
| } | ||
| shape.Annotations.Add(outer, false); | ||
| } | ||
|
|
||
| Assert.That(shape.Annotations.FindDepthFirst(nodes[1], dir, out Annotation<ShapeNode> result), Is.True); | ||
| Assert.That(result, Is.SameAs(nodes[1].Annotation)); | ||
| } | ||
|
|
||
| // Tied containers that own only the other segments must not hide, or stand in for, an annotation that is not | ||
| // inside any of them. | ||
| [Test] | ||
| public void FindDepthFirst_TiedContainersNotOwningTheOffset_ReturnsTheUnownedAnnotation( | ||
| [Values(0, 1, 2, 3, 4, 5)] int seed, | ||
| [Values] Direction dir | ||
| ) | ||
| { | ||
| Shape shape = CreateShape(seed, out ShapeNode[] nodes); | ||
| for (int i = 0; i < ContainerCount; i++) | ||
| { | ||
| Annotation<ShapeNode> container = NewContainer(nodes); | ||
| if (i == 0) | ||
| container.Children.AddRange(new[] { nodes[0].Annotation, nodes[2].Annotation }); | ||
| shape.Annotations.Add(container, false); | ||
| } | ||
|
|
||
| Assert.That(shape.Annotations.FindDepthFirst(nodes[1], dir, out Annotation<ShapeNode> result), Is.True); | ||
| Assert.That(result, Is.SameAs(nodes[1].Annotation)); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: F3. Only containers with exactly the same range are tried, so an earlier sibling with a wider range that owns the offset is still missed. Example: A[n0,n3] owns n0..n3 and B[n0,n2] is a leaf, both added with
subsume: false. The list order is A, B,Find(n1)returns B, andFindDepthFirst(n1)returns false. This is pre-existing and deterministic. Traced, not run.