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 143a2a68eec..0e545cd993b 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 @@ -359,6 +359,8 @@ public static class UnifiedDiffLineHeaderCodeMining extends LineHeaderCodeMining private final int tabWidth; private ITextViewer viewer; + private final List detailedDiffRanges; + private Rectangle lastRectangle; private List backgrounds; private List foregrounds; @@ -380,9 +382,47 @@ public UnifiedDiffLineHeaderCodeMining(Position position, ICodeMiningProvider pr this.diff = diff; this.tabWidth = tabWidth; this.viewer = viewer; + this.detailedDiffRanges = computeDetailedDiffRanges(diff); ((MouseClickConsumer) getAction()).setCodeMining(this); } + /** + * Line range of a detailed diff, resolved once so painting does not scan + * the hunk text. + */ + private record DetailedDiffRange(String diffStr, int start, int length, int fromLine, int toLine) { + } + + private static List computeDetailedDiffRanges(UnifiedDiff diff) { + boolean useRight = diff.mode.equals(UnifiedDiffMode.OVERLAY_MODE) + || diff.mode.equals(UnifiedDiffMode.OVERLAY_READ_ONLY_MODE) + || diff.mode.equals(UnifiedDiffMode.REVERT_MODE); + String fullDiffStr = useRight ? diff.rightStr : diff.leftStr; + String diffStr = removeTrailingNewLines(fullDiffStr); + int diffStrDelta = fullDiffStr.length() - diffStr.length(); + List result = new ArrayList<>(); + for (var detailedDiff : diff.detailedDiffs) { + String detailedDiffStr = useRight ? detailedDiff.rightStr : detailedDiff.leftStr; + int detailedDiffStart = useRight ? detailedDiff.rightStart : detailedDiff.leftStart; + int detailedDiffLength = useRight ? detailedDiff.rightLength : detailedDiff.leftLength; + if (detailedDiffStr.trim().length() == 0) { + continue; + } + if (detailedDiffStart + detailedDiffLength >= diffStr.length()) { + if (detailedDiffLength <= diffStrDelta) { + continue; + } + detailedDiffLength -= diffStrDelta; + } + // String#split drops trailing empty strings, so it must not be used to + // count lines: a prefix ending with \n starts the next line + int fromLine = countLines(diffStr, detailedDiffStart); + int toLine = countLines(diffStr, detailedDiffStart + detailedDiffLength); + result.add(new DetailedDiffRange(diffStr, detailedDiffStart, detailedDiffLength, fromLine, toLine)); + } + return result; + } + private static final class ForegroundInfo { final int x; @@ -636,37 +676,16 @@ public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) { // draw darker background for detailed diff gc.setBackground(this.detailedDiffColor); backgrounds = new ArrayList<>(); - for (var detailedDiff : this.diff.detailedDiffs) { - String diffStr = this.diff.leftStr; - String detailedDiffStr = detailedDiff.leftStr; - int detailedDiffStart = detailedDiff.leftStart; - int detailedDiffLength = detailedDiff.leftLength; - if (diff.mode.equals(UnifiedDiffMode.OVERLAY_MODE) - || diff.mode.equals(UnifiedDiffMode.OVERLAY_READ_ONLY_MODE) - || diff.mode.equals(UnifiedDiffMode.REVERT_MODE)) { - diffStr = this.diff.rightStr; - detailedDiffStr = detailedDiff.rightStr; - detailedDiffStart = detailedDiff.rightStart; - detailedDiffLength = detailedDiff.rightLength; - } - if (detailedDiffStr.trim().length() == 0) { - continue; - } - int diffStrLength = diffStr.length(); - diffStr = removeTrailingNewLines(diffStr); - if (detailedDiffStart + detailedDiffLength >= diffStr.length()) { - int diffStrDelta = diffStrLength - diffStr.length(); - if (detailedDiffLength <= diffStrDelta) { - continue; - } - detailedDiffLength -= diffStrDelta; - } + String[] diffLines = null; + Document diffStrDoc = null; + for (var range : this.detailedDiffRanges) { + String diffStr = range.diffStr(); + int detailedDiffStart = range.start(); + int detailedDiffLength = range.length(); + int fromLine = range.fromLine(); + int toLine = range.toLine(); try { var rangeInfo = new RangeInfo(-1, -1, null); - // String#split drops trailing empty strings, so it must not be used to - // count lines: a prefix ending with \n starts the next line - int fromLine = countLines(diffStr.substring(0, detailedDiffStart)); - int toLine = countLines(diffStr.substring(0, detailedDiffStart + detailedDiffLength)); if (fromLine == toLine) { int starty = getYForLine(fromLine - 1, y, gc, textWidget); Point start = getPositionForOffset(textWidget, gc, detailedDiffStart, diffStr, ranges, @@ -685,11 +704,13 @@ public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) { } } else { // mark first line until end - String[] lines = diffStr.split("\n"); //$NON-NLS-1$ - String firstLine = lines[fromLine - 1]; + if (diffLines == null) { + diffLines = diffStr.split("\n"); //$NON-NLS-1$ + diffStrDoc = new Document(diffStr); + } + String firstLine = diffLines[fromLine - 1]; int starty = getYForLine(fromLine - 1, y, gc, textWidget); int idx = getOffsetAtLine(diffStr, detailedDiffStart); - var diffStrDoc = new Document(diffStr); int fromLineOffset; try { fromLineOffset = diffStrDoc.getLineOffset(fromLine - 1); diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java index cb5a6012edd..21fee40f26e 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java @@ -1316,6 +1316,10 @@ void dispose() { @Override public void paintControl(PaintEvent e) { Rectangle bounds = this.w.getBounds(); + // Only the damaged lines: getTextBounds lays out every line it is asked for, + // so touching every hunk would make each repaint O(document). + int firstDamagedLine = this.w.getLineIndex(e.y); + int lastDamagedLine = this.w.getLineIndex(e.y + e.height); Iterator it = this.model.getAnnotationIterator(); while (it.hasNext()) { Annotation anno = it.next(); @@ -1338,8 +1342,11 @@ public void paintControl(PaintEvent e) { } posLength = pos.length; } - int fromLine = this.w.getLineAtOffset(posOffset); - int toLine = this.w.getLineAtOffset(posOffset + posLength); + int fromLine = Math.max(this.w.getLineAtOffset(posOffset), firstDamagedLine); + int toLine = Math.min(this.w.getLineAtOffset(posOffset + posLength), lastDamagedLine + 1); + if (fromLine >= toLine) { + continue; + } e.gc.setBackground(this.additionBackgroundColor); for (int lineNr = fromLine; lineNr < toLine; lineNr++) { String line = this.w.getLine(lineNr); diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffText.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffText.java index c46e4279524..eb12952fb74 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffText.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffText.java @@ -36,8 +36,16 @@ public static int countLines(String str) { if (str == null) { return 0; } + return countLines(str, str.length()); + } + + /** + * Returns the number of lines of the first {@code end} characters of the + * given string, like {@link #countLines(String)} on that prefix. + */ + public static int countLines(String str, int end) { int result = 1; - for (int i = 0; i < str.length(); i++) { + for (int i = 0; i < end; i++) { if (str.charAt(i) == '\n') { result++; } 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 9b97d70bc44..7cf8bef93c4 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 @@ -63,6 +63,20 @@ public void testCountLinesForDetailedDiffStartOffsets() { () -> assertEquals(2, countLines("first\r\nsecond"), "CRLF is one delimiter")); } + /** + * The prefix overload counts the same lines as the copied substring would. + */ + @Test + public void testCountLinesOfPrefixMatchesSubstring() { + String diff = "first\nsecond\r\nthird\n"; + assertAll( // + () -> assertEquals(1, countLines(diff, 0), "empty prefix is one line"), // + () -> assertEquals(1, countLines(diff, 5), "prefix ending before the delimiter"), // + () -> assertEquals(2, countLines(diff, 6), "prefix ending after the delimiter"), // + () -> assertEquals(3, countLines(diff, 14), "CRLF is one delimiter"), // + () -> assertEquals(countLines(diff), countLines(diff, diff.length()), "whole string")); + } + // ------------------------------------------------------------ tab expansion @Test