fix: TryAddSegment when a segments prepends a existing Segment

[belucha]

closes #15

This was failing, as the newSegment was extended by the old one, but the Memory still references the original segment!

[Copilot]

Pull request overview

Fixes RawMemory.TryAddSegment so that when a new segment is prepended (and therefore extended to include) an existing segment, the Segments list is updated to reference the combined segment (closing #15).

Changes:

• Replaces the previous Any(...)-based extension check with an indexed loop so the list entry can be updated when newSegment absorbs an existing segment.
• Keeps the existing behavior of rejecting overlaps and sorting only when a truly new segment is added.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[benedekkupper]

Let's simplify the overall logic, and reduce to a single loop, move this check into your for loop please.

[benedekkupper]

@belucha can you carry out these changes?

[belucha]

I initially realy thought about this, but this would fail! A complete check against overlapping is required prior appending!

This code looks cleaner but would fail!

        public bool TryAddSegment(Segment newSegment)
        {
            for (int i = 0; i < Segments.Count; i++)
            {
                var segment = Segments[i];
                if (segment.Overlaps(newSegment))
                {
                    return false;
                }
                if (newSegment.TryAppend(segment))
                {
                    // the new segment now includes the old one, which is essentially a prepend
                    Segments[i] = newSegment;
                    return true;
                }
                if (segment.TryAppend(newSegment))
                {
                    // the old segment now includes the new one
                    return true;
                }
            }
            // no overlaps or extensions, just add it
            Segments.Add(newSegment);
            Segments.Sort();
            return true;
        }

As the new segment might overlap with the next segment, to avoid that we would need to check if the next segment does not overlap too! Which is possible, but would make the whole logik quite unintuitive and error prone, also this is no performance issue at all, I think!

If you really want me to do so -- I will, but it will look not nice! Something like this...

        public bool TryAddSegment(Segment newSegment)
        {
            for (int i = 0; i < Segments.Count; i++)
            {
                var segment = Segments[i];
                if (segment.Overlaps(newSegment))
                {
                    return false;
                }
                var nextSegment = Segments.ElementAtOrDefault(i + 1);
                if (nextSegment!=null && nextSegment.Overlaps(newSegment))
                {
                    return false; 
                }
                if (newSegment.TryAppend(segment))
                {
                    // the new segment now includes the old one, which is essentially a prepend
                    Segments[i] = newSegment;
                    return true;
                }
                if (segment.TryAppend(newSegment))
                {
                    // the old segment now includes the new one
                    return true;
                }
            }
            // no overlaps or extensions, just add it
            Segments.Add(newSegment);
            Segments.Sort();
            return true;
        }

Let me know what to do!

[benedekkupper]

You are right, I overlooked that! What do you think about #17?

[benedekkupper]

Hi @belucha, thanks for your contribution! May I ask how you stumbled upon this bug, being the first to face this issue?

[belucha]

I merged several hex files with e.g. separate version records created separately, then merging somehow failed...
It's that simple!