From c86669a1a16ef41db3d77a36d69ec6dd97dc5c8c Mon Sep 17 00:00:00 2001 From: Tobias Melcher Date: Wed, 30 Sep 2026 14:27:53 +0200 Subject: [PATCH] Trim detailed-diff highlight ending in trailing spaces The label is drawn with its trailing spaces stripped, so a detailed diff reaching into that region overshot the space-stripped labelLength and was dropped completely instead of being trimmed. The length is now clamped to labelLength via clampDetailedDiffLength, so the highlight is trimmed to the visible label. Reviewed with the help of Claude Code. --- .../UnifiedDiffCodeMiningProvider.java | 5 +-- .../team/tests/ui/UnifiedDiffTextTest.java | 34 +++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java index 60bdf744b3f..ef6b46aa38f 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java @@ -739,7 +739,7 @@ private int measure(GC gc, Font font, String label, int from, int to) { } } - static List createDetailedDiffBackgroundRanges(UnifiedDiff diff, int tabWidth, Color detailedDiffColor) { + public static List createDetailedDiffBackgroundRanges(UnifiedDiff diff, int tabWidth, Color detailedDiffColor) { List ranges = new ArrayList<>(); String diffStr = diff.mode.equals(UnifiedDiffMode.REPLACE_MODE) ? diff.leftStr : diff.rightStr; String trimmedDiffStr = removeTrailingNewLines(diffStr); @@ -767,7 +767,8 @@ static List createDetailedDiffBackgroundRanges(UnifiedDiff diff, int int expandedStart = mapOffsetToTabExpanded(diffStr, detailedDiffStart, tabWidth); int expandedEnd = mapOffsetToTabExpanded(diffStr, detailedDiffStart + detailedDiffLength, tabWidth); int expandedLength = expandedEnd - expandedStart; - if (expandedStart >= 0 && expandedLength > 0 && expandedStart + expandedLength <= labelLength) { + expandedLength = clampDetailedDiffLength(expandedStart, expandedLength, labelLength); + if (expandedStart >= 0 && expandedLength > 0) { StyleRange bgRange = new StyleRange(); bgRange.start = expandedStart; bgRange.length = expandedLength; diff --git a/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffTextTest.java b/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffTextTest.java index d4adbb19ab5..b448af69b7d 100644 --- a/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffTextTest.java +++ b/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffTextTest.java @@ -24,9 +24,12 @@ import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.eclipse.compare.unifieddiff.UnifiedDiffMode; +import org.eclipse.compare.unifieddiff.internal.UnifiedDiffCodeMiningProvider; import org.eclipse.compare.unifieddiff.internal.UnifiedDiffCodeMiningProvider.UnifiedDiffLineHeaderCodeMining; import org.eclipse.compare.unifieddiff.internal.UnifiedDiffCodeMiningProvider.UnifiedDiffLineHeaderCodeMining.RangeInfo; import org.eclipse.compare.unifieddiff.internal.UnifiedDiffManager.UnifiedDiff; @@ -283,6 +286,22 @@ public void testClampDetailedDiffLengthNoOvershoot() { assertEquals(3, clampDetailedDiffLength(5, 3, 20)); } + // --------------------------------- createDetailedDiffBackgroundRanges + + /** A detailed diff reaching into trailing spaces is trimmed to the visible label, not dropped. */ + @Test + public void testDetailedDiffRunningIntoTrailingSpacesIsTrimmedNotDropped() { + String diffStr = "foo bar "; + UnifiedDiff diff = replaceDiff(diffStr, detailedDiff(diffStr, 4, 6)); + + List ranges = UnifiedDiffCodeMiningProvider.createDetailedDiffBackgroundRanges(diff, 4, + BACKGROUND_1); + + assertEquals(1, ranges.size(), "the highlight must survive, trimmed to the visible label"); + assertEquals(4, ranges.get(0).start, "highlight starts at 'bar'"); + assertEquals(3, ranges.get(0).length, "highlight is trimmed to 'bar', dropping the trailing spaces"); + } + // ------------------------------------------- getPositionForOffset @Test @@ -312,6 +331,21 @@ public void testGetPositionForOffsetResetsXAfterNewlineAtRangeStart() throws Exc // ------------------------------------------------------------------ helpers + private static UnifiedDiff replaceDiff(String diffStr, UnifiedDiff... detailedDiffs) { + Document doc = new Document(diffStr); + UnifiedDiff diff = new UnifiedDiff(doc, 0, diffStr.length(), diffStr, doc, 0, diffStr.length(), diffStr, + new ArrayList<>(), UnifiedDiffMode.REPLACE_MODE); + Collections.addAll(diff.detailedDiffs, detailedDiffs); + return diff; + } + + private static UnifiedDiff detailedDiff(String diffStr, int start, int length) { + Document doc = new Document(diffStr); + String sub = diffStr.substring(start, start + length); + return new UnifiedDiff(doc, start, start + length, sub, doc, start, start + length, sub, List.of(), + UnifiedDiffMode.REPLACE_MODE); + } + /** * The merged ranges are handed to {@code StyledText#setStyleRanges}, which * rejects overlapping ranges. They must therefore tile the foregrounds exactly: