From 866f2958016b7a1da79f8a84f897eea981614459 Mon Sep 17 00:00:00 2001 From: Hendrik Mennen Date: Fri, 16 Oct 2026 10:14:34 +0200 Subject: [PATCH] Fix ArgumentOutOfRangeException in TextModificationService.ColorizeLine ColorizeLine clamped multi-line segments by mutating them. Setting TextSegment.StartOffset moves the segment (keeping its length), so the end offset was pushed past the line end and ChangeLinePart threw. The mutation also permanently corrupted the stored segments. Clamp into local offsets instead, and preserve the end offset when trimming overlapping segments in SetModification. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../TextModificationService.cs | 23 +++++++++-- .../OneWare.Essentials.csproj | 4 ++ .../TextModificationServiceTests.cs | 38 +++++++++++++++++++ 3 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 tests/OneWare.Essentials.UnitTests/TextModificationServiceTests.cs diff --git a/src/OneWare.Essentials/EditorExtensions/TextModificationService.cs b/src/OneWare.Essentials/EditorExtensions/TextModificationService.cs index 28f60c1dc..3765e6ca8 100644 --- a/src/OneWare.Essentials/EditorExtensions/TextModificationService.cs +++ b/src/OneWare.Essentials/EditorExtensions/TextModificationService.cs @@ -45,12 +45,19 @@ public void SetModification(string key, params TextModificationSegment[] segment if (s.StartOffset >= f.StartOffset) { if (s.StartOffset <= f.EndOffset) continue; //Completely overlapped - if (s.EndOffset > f.EndOffset + 1) s.StartOffset = f.EndOffset + 1; + if (s.EndOffset > f.EndOffset + 1) + { + var endOffset = s.EndOffset; + s.StartOffset = f.EndOffset + 1; + s.EndOffset = endOffset; + } } if (s.StartOffset < f.StartOffset) if (s.EndOffset > f.EndOffset - 1) s.EndOffset = f.EndOffset - 1; + + if (s.Length <= 0) continue; } m.Add(s); @@ -67,13 +74,21 @@ protected override void ColorizeLine(DocumentLine line) foreach (var overlap in overlaps) { - if (overlap.EndOffset > line.EndOffset) overlap.EndOffset = line.EndOffset; - if (overlap.StartOffset < line.Offset) overlap.StartOffset = line.Offset; - ChangeLinePart(overlap.StartOffset, overlap.EndOffset, x => ApplyChanges(x, overlap)); + if (!TryClampToLine(overlap, line, out var startOffset, out var endOffset)) continue; + ChangeLinePart(startOffset, endOffset, x => ApplyChanges(x, overlap)); } } } + // Must not mutate the segment: it is shared by all lines it spans, and setting + // TextSegment.StartOffset moves the segment (keeps its length) instead of trimming it. + internal static bool TryClampToLine(ISegment segment, DocumentLine line, out int startOffset, out int endOffset) + { + startOffset = Math.Max(segment.Offset, line.Offset); + endOffset = Math.Min(segment.EndOffset, line.EndOffset); + return endOffset > startOffset; + } + private void ApplyChanges(VisualLineElement element, TextModificationSegment segment) { if (segment.Foreground != null) element.TextRunProperties.SetForegroundBrush(segment.Foreground); diff --git a/src/OneWare.Essentials/OneWare.Essentials.csproj b/src/OneWare.Essentials/OneWare.Essentials.csproj index e9f74ef02..78efcdc2a 100644 --- a/src/OneWare.Essentials/OneWare.Essentials.csproj +++ b/src/OneWare.Essentials/OneWare.Essentials.csproj @@ -40,4 +40,8 @@ + + + + diff --git a/tests/OneWare.Essentials.UnitTests/TextModificationServiceTests.cs b/tests/OneWare.Essentials.UnitTests/TextModificationServiceTests.cs new file mode 100644 index 000000000..3a6b2f94a --- /dev/null +++ b/tests/OneWare.Essentials.UnitTests/TextModificationServiceTests.cs @@ -0,0 +1,38 @@ +using AvaloniaEdit.Document; +using OneWare.Essentials.EditorExtensions; +using Xunit; + +namespace OneWare.Essentials.UnitTests; + +public class TextModificationServiceTests +{ + [Fact] + public void TryClampToLine_MultiLineSegment_StaysWithinEachLineAndDoesNotMutateSegment() + { + var document = new TextDocument { Text = "first\nsecond line\nthird" }; + var segment = new TextModificationSegment(2, document.TextLength - 2); + var originalStart = segment.StartOffset; + var originalEnd = segment.EndOffset; + + foreach (var line in document.Lines) + { + Assert.True(TextModificationService.TryClampToLine(segment, line, out var start, out var end)); + Assert.InRange(start, line.Offset, line.EndOffset); + Assert.InRange(end, line.Offset, line.EndOffset); + Assert.True(end > start); + } + + Assert.Equal(originalStart, segment.StartOffset); + Assert.Equal(originalEnd, segment.EndOffset); + } + + [Fact] + public void TryClampToLine_SegmentOnlyTouchingLineDelimiter_ReturnsFalse() + { + var document = new TextDocument { Text = "ab\ncd" }; + var firstLine = document.GetLineByNumber(1); + var segment = new TextModificationSegment(firstLine.EndOffset, firstLine.EndOffset + 1); + + Assert.False(TextModificationService.TryClampToLine(segment, firstLine, out _, out _)); + } +}