Fix issue where structure tag could get stuck at the wrong location

[CyrusNajmabadi]

Partial fix of #73799

[CyrusNajmabadi]

@olegtk @AmadeusW @veler can you point me to where your code for IStructureTag structure tags is? I have a feelling you are not using .EdgeInclusive as your span tracking mode for them, causing them to not move to the desired location as the user edits at the start of a structure tag.

If that's the case, we'll likely want you to change that to fix #73799 completely.

[ToddGrun]

can you point me to where your code for IStructureTag structure tags is

Is this it?

[CyrusNajmabadi]

Thanks. Given:

            guideLineSpanStart = guideLineSpanStart != null ?
                MappingPointSnapshot.Create(
                    targetSnapshot,
                    new SnapshotPoint(tag.Snapshot, guideLineSpanStart.Value),
                    PointTrackingMode.Positive, null).GetPoint(targetSnapshot, PositionAffinity.Predecessor) :
                    null;

This should likely move to PointTrackingMode.Negative @olegtk

[ToddGrun]

HeaderSpan

Is this just about the guideline behavior? If so, would GuideLineSpan.Start be appropriate to use? FYI, I believe you could also use a SnapshotSpan and SpanTrackingMode.Inclusive

[CyrusNajmabadi]

We can't use guidelinespan as we don't set it. it's just:

 public Span? GuideLineSpan => null;

we defer to the editor to determine that span.

[ToddGrun]

I still feel like mapping a span is better than two calls to mapping points.

var previousStructureStart = new SnapshotSpan(previousSnapshot, previousStructureTag.HeaderSpan.Start, 0);

if (!previousStructureStart.TranslateTo(latestSnapshot, SpanTrackingMode.EdgeInclusive).IsEmpty)

[ToddGrun]

To clarify, I was thinking of something like the following (with the comment needing to be altered)

    protected sealed override bool TagEquals(IContainerStructureTag latestTag, IContainerStructureTag previousTag)
    {
        if (latestTag is not StructureTag latestStructureTag || previousTag is not StructureTag previousStructureTag)
        {
            Contract.Fail("Tags were the wrong type");
            return latestTag == previousTag;
        }

        // If these tags are the same object in memory, then they are obviously equal
        if (latestTag == previousTag)
            return true;

        var latestSnapshot = latestStructureTag.Snapshot;
        var previousSnapshot = previousStructureTag.Snapshot;

        var previousStructureStart = new SnapshotSpan(previousSnapshot, previousStructureTag.HeaderSpan.Start, 0);

        if (previousStructureStart.TranslateTo(latestSnapshot, SpanTrackingMode.EdgeInclusive).IsEmpty)
        {
            // We can't know that how we think this block moved is actually how the editor actually moved it.
            // Specifically, the tracking mode is an implementation detail.  As such, we don't want to reuse this tag as
            // its stale data (as mapped by the editor) may not be where we'd expect the new block's data to be.  This
            // can happen when the user types right at the start of a structure tag, causing it to move inwards
            // undesirably.

            return latestTag.Equals(previousTag);
        }

        return false;
    }

[CyrusNajmabadi]

@ToddGrun ptal.

[CyrusNajmabadi]

@JoeRobich @akhera99 ptal.

[ToddGrun]

[CyrusNajmabadi]

Tlaked offline. Going to take the code as is as i personally think it is clearer, and this is not a hot path.