From 88fefb2060b59b5f7b82946a10c27fdbd4e73fd6 Mon Sep 17 00:00:00 2001 From: Tobias Melcher Date: Fri, 18 Sep 2026 17:39:56 +0200 Subject: [PATCH] Enable syntax coloring for footer minings in unified diff Line-header minings already applied syntax coloring via StyleRange computation. Footer minings (used when the diff sits at the end of a document without a trailing newline) were missing this: they fell back to a plain super.draw() call for the text. They now use the same computeStyleRanges path, transformFontStyleToFont for bold/italic, and the same rendering loop as the header. A shared MouseClickConsumer (via the new IUnifiedDiffCodeMining interface) also gives footer minings the same double-click overlay behaviour the header already had. The word-level detailed-diff highlight rectangles are now measured using the styled font of each run rather than the base font, so the highlight stays correctly aligned when bold or italic text precedes the changed word on the same line. --- .../UnifiedDiffCodeMiningProvider.java | 683 ++++++++++++------ .../ui/UnifiedDiffCodeMiningProviderTest.java | 264 +++++++ 2 files changed, 708 insertions(+), 239 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 c3a48f41c21..3ce35d6f8bf 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 @@ -146,8 +146,9 @@ public CompletableFuture> provideCodeMinings(ITextVi if (this.foldButtonColor != null && !this.foldButtonColor.isDisposed()) { this.foldButtonColor.dispose(); } - this.detailedDiffColor = new Color(interpolate(deletionColor, background, 0.9)); - this.deletionBackgroundColor = new Color(interpolate(deletionColor, background, 0.8)); + // the word-level diff is tinted stronger so it stands out against the band + this.detailedDiffColor = new Color(interpolate(deletionColor, background, 0.8)); + this.deletionBackgroundColor = new Color(interpolate(deletionColor, background, 0.9)); this.foldSeparatorColor = new Color(separatorBackground(background)); this.foldButtonColor = new Color(buttonBackground(background)); lastIsOverlay = isOverlay; @@ -316,13 +317,13 @@ private ICodeMining createMining(IDocument doc, UnifiedDiff diff, int offset, in throws BadLocationException { int end = doc.getLength(); if (offset >= end && !startsLine(doc, end)) { - return new UnifiedDiffFooterCodeMining(doc, this, null, diff, tabWidth, this.deletionBackgroundColor); + return new UnifiedDiffFooterCodeMining(doc, this, diff, tabWidth, this.deletionBackgroundColor, this.detailedDiffColor, tv); } // a position must not reach beyond the document, otherwise the annotation model // silently drops it int start = Math.min(offset, end); return new UnifiedDiffLineHeaderCodeMining(new Position(start, start < end ? 1 : 0), this, diff, tabWidth, - this.detailedDiffColor, this.deletionBackgroundColor, tv); + this.deletionBackgroundColor, this.detailedDiffColor, tv); } private static boolean startsLine(IDocument doc, int offset) throws BadLocationException { @@ -487,21 +488,103 @@ private static void drawChevrons(GC gc, int centerX, int centerY, int height, bo } } - static class UnifiedDiffFooterCodeMining extends DocumentFooterCodeMining { + interface IUnifiedDiffCodeMining { + Rectangle getLastRectangle(); + Color getDeletionBackgroundColor(); + Color getDetailedDiffColor(); + int getTabWidth(); + UnifiedDiff getUnifiedDiff(); + String getLabel(); + } + + static class MouseClickConsumer implements Consumer { + + private final ITextViewer viewer; + private IUnifiedDiffCodeMining mining; + + public MouseClickConsumer(ITextViewer viewer) { + this.viewer = viewer; + } + + public void setCodeMining(IUnifiedDiffCodeMining mining) { + this.mining = mining; + } + + @Override + public void accept(MouseEvent t) { + if (mining == null || viewer == null || mining.getLastRectangle() == null) { + return; + } + StyledText st = viewer.getTextWidget(); + StyledText overlay = new StyledText(st, SWT.NONE); + overlay.setBounds(mining.getLastRectangle()); + overlay.setFont(st.getFont()); + overlay.setBackground(mining.getDeletionBackgroundColor()); + overlay.setLineSpacing(st.getLineSpacing()); + String txt = mining.getLabel().stripTrailing(); + overlay.setText(txt); + overlay.setFocus(); + List backgrounds = createDetailedDiffBackgroundRanges(mining.getUnifiedDiff(), + mining.getTabWidth(), mining.getDetailedDiffColor()); + List foregrounds = computeStyleRanges(viewer, mining.getUnifiedDiff().leftStart, txt); + List ranges = mergeStyleRanges(backgrounds, foregrounds); + overlay.setStyleRanges(ranges.toArray(new StyleRange[] {})); + openOverlay(overlay, viewer); + } + } + + public static class UnifiedDiffFooterCodeMining extends DocumentFooterCodeMining implements IUnifiedDiffCodeMining { private final String unifiedDiffLabel; private final Color deletionBackgroundColor; + private final Color detailedDiffColor; + private final int tabWidth; + private final ITextViewer viewer; private UnifiedDiff diff; + private List styleRanges; + private final HashMap> styledFonts = new HashMap<>(); + private Rectangle lastRectangle; + private Font cachedFont; public UnifiedDiffFooterCodeMining(IDocument document, ICodeMiningProvider provider, - Consumer action, UnifiedDiff diff, int tabWidth, Color deletionBackgroundColor) { - super(document, provider, action); + UnifiedDiff diff, int tabWidth, Color deletionBackgroundColor, Color detailedDiffColor, + ITextViewer viewer) { + super(document, provider, new MouseClickConsumer(viewer)); this.deletionBackgroundColor = deletionBackgroundColor; + this.detailedDiffColor = detailedDiffColor; + this.tabWidth = tabWidth; + this.viewer = viewer; if (diff.mode.equals(UnifiedDiffMode.REPLACE_MODE)) { - this.unifiedDiffLabel = replaceTabWithSpaces(diff.leftStr, tabWidth); + this.unifiedDiffLabel = removeTrailingNewLines(replaceTabWithSpaces(diff.leftStr, tabWidth)); } else { - this.unifiedDiffLabel = replaceTabWithSpaces(diff.rightStr, tabWidth); + this.unifiedDiffLabel = removeTrailingNewLines(replaceTabWithSpaces(diff.rightStr, tabWidth)); } this.diff = diff; + ((MouseClickConsumer) getAction()).setCodeMining(this); + } + + @Override + public Rectangle getLastRectangle() { + return lastRectangle; + } + + @Override + public Color getDeletionBackgroundColor() { + return deletionBackgroundColor; + } + + @Override + public Color getDetailedDiffColor() { + return detailedDiffColor; + } + + @Override + public int getTabWidth() { + return tabWidth; + } + + @Override + public UnifiedDiff getUnifiedDiff() { + return diff; } @Override @@ -509,8 +592,25 @@ public String getLabel() { return this.unifiedDiffLabel; } - public UnifiedDiff getUnifiedDiff() { - return this.diff; + @Override + public void dispose() { + styleRanges = null; + lastRectangle = null; + cachedFont = null; + clearStyledFonts(); + super.dispose(); + } + + private void clearStyledFonts() { + styledFonts.forEach((font, map) -> map.forEach((style, f) -> f.dispose())); + styledFonts.clear(); + } + + private List styleRanges(String label) { + if (styleRanges == null) { + styleRanges = computeStyleRanges(viewer, diff.leftStart, label); + } + return styleRanges; } @Override @@ -520,18 +620,248 @@ public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) { gc.setForeground(c); Font font = textWidget.getFont(); gc.setFont(font); + if (cachedFont != null && (cachedFont.isDisposed() || !cachedFont.equals(font))) { + // font might have been changed in the meantime - drop the derived fonts + // keyed on the old base font so their native handles are not leaked + clearStyledFonts(); + } + cachedFont = font; // first run to get width and height for label // change from https://github.com/eclipse-platform/eclipse.platform.ui/pull/3651 // is required so that background correctly drawn with line spacing > 0 Point result = super.draw(gc, textWidget, color, x, y); + lastRectangle = new Rectangle(x, y, result.x, result.y); // draw background // vs code is drawing the background to the top right of the editor - we do here // the same! gc.fillRectangle(0, y, textWidget.getBounds().width /* result.x */, result.y); - // draw foreground again - result = super.draw(gc, textWidget, color, x, y); + + String label = getLabel(); + List ranges = styleRanges(label); + if (ranges.isEmpty()) { + // no syntax coloring available; fall back to plain rendering + result = super.draw(gc, textWidget, color, x, y); + return result; + } + + // paint the word-level detailed-diff backgrounds first, then draw the + // syntax-colored text transparently on top - same order as the header band + fillDetailedDiffBackgrounds(gc, textWidget, label, ranges, x, y); + gc.setFont(font); + drawStyleRanges(gc, textWidget, ranges, label, styledFonts, x, y, null); return result; } + + /** + * Fills the darker {@code detailedDiffColor} rectangles behind the word-level + * changes. {@link #drawStyleRanges} draws its text transparently and so cannot + * paint a range background itself. + */ + private void fillDetailedDiffBackgrounds(GC gc, StyledText textWidget, String label, List ranges, + int x, int y) { + List backgrounds = createDetailedDiffBackgroundRanges(diff, tabWidth, detailedDiffColor); + if (backgrounds.isEmpty()) { + return; + } + int lineHeight = textWidget.getLineHeight(); + int lineSpacing = textWidget.getLineSpacing(); + gc.setBackground(this.detailedDiffColor); + for (StyleRange bg : backgrounds) { + int pos = bg.start; + int end = bg.start + bg.length; + // a detailed diff may span lines; fill each line segment on its own line + while (pos < end) { + int lineStart = label.lastIndexOf('\n', pos - 1) + 1; + int lineEnd = label.indexOf('\n', pos); + int segmentEnd = lineEnd == -1 ? end : Math.min(end, lineEnd); + int lineIndex = (int) label.substring(0, pos).chars().filter(ch -> ch == '\n').count(); + int startX = styledWidth(gc, label, ranges, lineStart, pos); + int width = styledWidth(gc, label, ranges, pos, segmentEnd); + if (width > 0) { + int rectX = x + startX; + int rectY = y + lineIndex * (lineHeight + lineSpacing); + gc.fillRectangle(rectX, rectY, width, lineHeight); + } + pos = segmentEnd == lineEnd ? segmentEnd + 1 : segmentEnd; + } + } + } + + /** + * Measures {@code label[from, to)}, which must lie within a single line, the + * way {@link #drawStyleRanges} paints it. Measuring in the base font would put + * the highlight left of the changed word whenever bold or italic text precedes + * it on the same line. + */ + private int styledWidth(GC gc, String label, List ranges, int from, int to) { + if (to <= from) { + return 0; + } + Font base = gc.getFont(); + int width = 0; + try { + int cursor = from; + for (StyleRange range : ranges) { + int rangeEnd = range.start + range.length; + if (rangeEnd <= cursor || range.start >= to) { + continue; + } + if (range.start > cursor) { + width += measure(gc, base, label, cursor, range.start); + cursor = range.start; + } + // drawStyleRanges keeps the base font for whitespace-only ranges + boolean blank = label.substring(range.start, Math.min(rangeEnd, label.length())).isBlank(); + StyleRange rangeWithFont = blank ? range : transformFontStyleToFont(styledFonts, base, range); + Font font = rangeWithFont.font != null ? rangeWithFont.font : base; + int segmentEnd = Math.min(rangeEnd, to); + width += measure(gc, font, label, cursor, segmentEnd); + cursor = segmentEnd; + if (cursor >= to) { + break; + } + } + if (cursor < to) { + width += measure(gc, base, label, cursor, to); + } + } finally { + gc.setFont(base); + } + return width; + } + + private int measure(GC gc, Font font, String label, int from, int to) { + gc.setFont(font); + // gc.stringExtent ignores tabs, and drawStyleRanges drops the CR of a CRLF + String text = replaceTabWithSpaces(label.substring(from, to), tabWidth).replace("\r", ""); //$NON-NLS-1$ //$NON-NLS-2$ + return gc.stringExtent(text).x; + } + } + + 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); + int labelLength = replaceTabWithSpaces(trimmedDiffStr, tabWidth).stripTrailing().length(); + for (var detailedDiff : diff.detailedDiffs) { + int detailedDiffStart; + int detailedDiffLength; + String detailedDiffStr; + if (diff.mode.equals(UnifiedDiffMode.REPLACE_MODE)) { + detailedDiffStart = detailedDiff.leftStart; + detailedDiffLength = detailedDiff.leftLength; + detailedDiffStr = detailedDiff.leftStr; + } else { + detailedDiffStart = detailedDiff.rightStart; + detailedDiffLength = detailedDiff.rightLength; + detailedDiffStr = detailedDiff.rightStr; + } + if (detailedDiffStr.trim().length() == 0) { + continue; + } + if (detailedDiffStart + detailedDiffLength >= trimmedDiffStr.length()) { + int delta = diffStr.length() - trimmedDiffStr.length(); + if (detailedDiffLength <= delta) { + continue; + } + detailedDiffLength -= delta; + } + 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) { + StyleRange bgRange = new StyleRange(); + bgRange.start = expandedStart; + bgRange.length = expandedLength; + bgRange.background = detailedDiffColor; + ranges.add(bgRange); + } + } + return ranges; + } + + record ForegroundInfo(int x, int y, String str, Font font, Color background, Color foreground) { + } + + /** + * Draws the syntax-colored label using the given style ranges, advancing the + * cursor position range by range. The {@code onForeground} consumer is called + * for each drawn segment and may be {@code null}; the header mining uses it to + * populate its foreground cache so subsequent repaints skip this path. + */ + static void drawStyleRanges(GC gc, StyledText textWidget, List ranges, String label, + HashMap> styledFonts, int x, int y, + Consumer onForeground) { + Font font = gc.getFont(); + int textWidgetLineHeight = textWidget.getLineHeight(); + int cx = x; + int cy = y; + for (StyleRange range : ranges) { + String sub = label.substring(range.start, range.start + range.length); + if (sub.trim().length() > 0) { + if (range.background != null) { + gc.setBackground(range.background); + } + if (range.foreground != null) { + gc.setForeground(range.foreground); + } + Font currentFont = gc.getFont(); + var rangeWithFont = transformFontStyleToFont(styledFonts, currentFont, range); + if (rangeWithFont.font != null) { + gc.setFont(rangeWithFont.font); + } + String[] lines = sub.split("\n"); //$NON-NLS-1$ + if (lines.length > 1) { + for (int i = 0; i < lines.length; i++) { + String line = lines[i].replace("\r", ""); //$NON-NLS-1$ //$NON-NLS-2$ + gc.drawString(line, cx, cy, true); + if (onForeground != null) { + onForeground.accept(new ForegroundInfo(cx - x, cy - y, line, gc.getFont(), + gc.getBackground(), gc.getForeground())); + } + Point p = gc.stringExtent(line); + if (i < lines.length - 1) { + cy += textWidgetLineHeight + textWidget.getLineSpacing(); + cx = x; + } else { + if (sub.endsWith("\n")) { //$NON-NLS-1$ + cy += textWidgetLineHeight + textWidget.getLineSpacing(); + cx = x; + } else { + cx += p.x; + } + } + } + } else { + gc.drawString(sub, cx, cy, true); + if (onForeground != null) { + onForeground.accept(new ForegroundInfo(cx - x, cy - y, sub, gc.getFont(), + gc.getBackground(), gc.getForeground())); + } + Point p = gc.stringExtent(sub); + if (sub.endsWith("\n")) { //$NON-NLS-1$ + cy += textWidgetLineHeight + textWidget.getLineSpacing(); + cx = x; + } else { + cx += p.x; + } + } + gc.setFont(currentFont); + } else { + int lfCount = 0; + if (sub.contains("\n")) { //$NON-NLS-1$ + lfCount = sub.split("\n", -1).length - 1; //$NON-NLS-1$ + sub = sub.substring(sub.lastIndexOf("\n") + 1); //$NON-NLS-1$ + } + Point p = gc.stringExtent(sub); + if (lfCount > 0) { + cy += lfCount * (textWidgetLineHeight + textWidget.getLineSpacing()); + cx = x; + } + cx += p.x; + } + } + gc.setFont(font); } private static List computeStyleRanges(ITextViewer v, int offset, String source) { @@ -539,8 +869,11 @@ private static List computeStyleRanges(ITextViewer v, int offset, St if (!(v instanceof SourceViewer sv)) { return result; } + IDocument originalDocument = sv.getDocument(); + if (originalDocument == null) { + return result; + } try { - IDocument originalDocument = sv.getDocument(); String prefix = originalDocument.get(0, offset /* diff.leftStart */); IDocument document = new Document(prefix + source); IRegion damage = new Region(prefix.length(), source.length()); @@ -559,7 +892,7 @@ private static List computeStyleRanges(ITextViewer v, int offset, St } } - public static class UnifiedDiffLineHeaderCodeMining extends LineHeaderCodeMining { + public static class UnifiedDiffLineHeaderCodeMining extends LineHeaderCodeMining implements IUnifiedDiffCodeMining { private final String unifiedDiffLabel; private final Color deletionBackgroundColor; private final Color detailedDiffColor; @@ -631,153 +964,34 @@ private static List computeDetailedDiffRanges(UnifiedDiff dif return result; } - private static final class ForegroundInfo { - - final int x; - final int y; - final String str; - final Font font; - final Color background; - final Color foreground; - - public ForegroundInfo(int x, int y, String str, Font font, Color background, Color foreground) { - this.x = x; - this.y = y; - this.str = str; - this.font = font; - this.background = background; - this.foreground = foreground; - } - + @Override + public String getLabel() { + return this.unifiedDiffLabel; } - private static class MouseClickConsumer implements Consumer { - - private final ITextViewer viewer; - private UnifiedDiffLineHeaderCodeMining mining; - - public MouseClickConsumer(ITextViewer viewer) { - this.viewer = viewer; - } - - public void setCodeMining(UnifiedDiffLineHeaderCodeMining mining) { - this.mining = mining; - } - - @Override - public void accept(MouseEvent t) { - if (mining == null || viewer == null || mining.lastRectangle == null) { - return; - } - StyledText st = viewer.getTextWidget(); - StyledText overlay = new StyledText(st, SWT.NONE); - overlay.setBounds(mining.lastRectangle); - overlay.setFont(st.getFont()); - overlay.setBackground(mining.deletionBackgroundColor); - overlay.setLineSpacing(st.getLineSpacing()); - String txt = mining.getLabel().stripTrailing(); - overlay.setText(txt); - overlay.setFocus(); - List backgrounds = createDetailedDiffBackgroundRanges(mining, txt); - List foregrounds = computeStyleRanges(viewer, mining.diff.leftStart, txt); - List ranges = mergeStyleRanges(backgrounds, foregrounds); - overlay.setStyleRanges(ranges.toArray(new StyleRange[] {})); - overlay.addFocusListener(new FocusAdapter() { - @Override - public void focusLost(FocusEvent e) { - overlay.dispose(); - setTextEditorActionsActivated(true); - } - }); - overlay.addKeyListener(new KeyAdapter() { - @Override - public void keyPressed(KeyEvent e) { - if (e.keyCode == SWT.ESC) { - overlay.dispose(); - setTextEditorActionsActivated(true); - } - e.doit = false; - } - }); - setTextEditorActionsActivated(false); - } + @Override + public UnifiedDiff getUnifiedDiff() { + return this.diff; + } - private List createDetailedDiffBackgroundRanges(UnifiedDiffLineHeaderCodeMining miningParam, - String txt) { - List ranges = new ArrayList<>(); - String diffStr = miningParam.diff.mode.equals(UnifiedDiffMode.REPLACE_MODE) ? miningParam.diff.leftStr - : miningParam.diff.rightStr; - String trimmedDiffStr = removeTrailingNewLines(diffStr); - for (var detailedDiff : miningParam.diff.detailedDiffs) { - int detailedDiffStart; - int detailedDiffLength; - String detailedDiffStr; - if (miningParam.diff.mode.equals(UnifiedDiffMode.REPLACE_MODE)) { - detailedDiffStart = detailedDiff.leftStart; - detailedDiffLength = detailedDiff.leftLength; - detailedDiffStr = detailedDiff.leftStr; - } else { - detailedDiffStart = detailedDiff.rightStart; - detailedDiffLength = detailedDiff.rightLength; - detailedDiffStr = detailedDiff.rightStr; - } - if (detailedDiffStr.trim().length() == 0) { - continue; - } - if (detailedDiffStart + detailedDiffLength >= trimmedDiffStr.length()) { - int delta = diffStr.length() - trimmedDiffStr.length(); - if (detailedDiffLength <= delta) { - continue; - } - detailedDiffLength -= delta; - } - int expandedStart = mapOffsetToTabExpanded(diffStr, detailedDiffStart, miningParam.tabWidth); - int expandedEnd = mapOffsetToTabExpanded(diffStr, detailedDiffStart + detailedDiffLength, - miningParam.tabWidth); - int expandedLength = expandedEnd - expandedStart; - if (expandedStart >= 0 && expandedLength > 0 && expandedStart + expandedLength <= txt.length()) { - StyleRange bgRange = new StyleRange(); - bgRange.start = expandedStart; - bgRange.length = expandedLength; - bgRange.background = miningParam.detailedDiffColor; - ranges.add(bgRange); - } - } - return ranges; - } + @Override + public Rectangle getLastRectangle() { + return lastRectangle; + } - private void setTextEditorActionsActivated(boolean state) { - IEditorPart part = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage() - .getActiveEditor(); - if (part instanceof MultiPageEditorPart multiPageEditorPart) { - Object page = multiPageEditorPart.getSelectedPage(); - if (page instanceof IEditorPart editorPart) { - part = editorPart; - } - } - if (!(part instanceof AbstractTextEditor) || part.getSite().getWorkbenchWindow().isClosing()) { - return; - } - if (UnifiedDiffManager.isViewerInPart(part, viewer)) { - try { - Method method = AbstractTextEditor.class.getDeclaredMethod("setActionActivation", //$NON-NLS-1$ - boolean.class); - method.setAccessible(true); - method.invoke(part, Boolean.valueOf(state)); - } catch (IllegalArgumentException | ReflectiveOperationException ex) { - error(ex); - } - } - } + @Override + public Color getDeletionBackgroundColor() { + return deletionBackgroundColor; } @Override - public String getLabel() { - return this.unifiedDiffLabel; + public Color getDetailedDiffColor() { + return detailedDiffColor; } - public UnifiedDiff getUnifiedDiff() { - return this.diff; + @Override + public int getTabWidth() { + return tabWidth; } /** @@ -847,22 +1061,22 @@ public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) { } // foregrounds for (var f : foregrounds) { - if (y + f.y < 0) { + if (y + f.y() < 0) { continue; } - if (f.font == null) { + if (f.font() == null) { gc.setFont(cachedFont); - } else if (!f.font.isDisposed()) { - gc.setFont(f.font); + } else if (!f.font().isDisposed()) { + gc.setFont(f.font()); } else { cleanCachedData(); cachedFont = font; fontIsDisposed = true; break; } - gc.setBackground(f.background); - gc.setForeground(f.foreground); - gc.drawString(f.str, x + f.x, y + f.y, true); + gc.setBackground(f.background()); + gc.setForeground(f.foreground()); + gc.drawString(f.str(), x + f.x(), y + f.y(), true); } if (!fontIsDisposed) { lastRectangle = new Rectangle(x, y, lastRectangle.width, lastRectangle.height); @@ -1002,70 +1216,7 @@ public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) { } foregrounds = new ArrayList<>(); gc.setFont(cachedFont); - int textWidgetLineHeight = textWidget.getLineHeight(); - int cx = x; - int cy = y; - for (StyleRange range : ranges) { - String sub = label.substring(range.start, range.start + range.length); - if (sub.trim().length() > 0) { - if (range.background != null) { - gc.setBackground(range.background); - } - if (range.foreground != null) { - gc.setForeground(range.foreground); - } - Font currentFont = gc.getFont(); - var rangeWithFont = transformFontStyleToFont(currentFont, range); - if (rangeWithFont.font != null) { - gc.setFont(rangeWithFont.font); - } - String[] lines = sub.split("\n"); //$NON-NLS-1$ - if (lines.length > 1) { - for (int i = 0; i < lines.length; i++) { - String line = lines[i].replace("\r", ""); //$NON-NLS-1$ //$NON-NLS-2$ - gc.drawString(line, cx, cy, true); - foregrounds.add(new ForegroundInfo(cx - x, cy - y, line, gc.getFont(), gc.getBackground(), - gc.getForeground())); - Point p = gc.stringExtent(line); - if (i < lines.length - 1) { - cy += textWidgetLineHeight + textWidget.getLineSpacing(); - cx = x; - } else { - if (sub.endsWith("\n")) { //$NON-NLS-1$ - cy += textWidgetLineHeight + textWidget.getLineSpacing(); - cx = x; - } else { - cx += p.x; - } - } - } - } else { - gc.drawString(sub, cx, cy, true); - foregrounds.add(new ForegroundInfo(cx - x, cy - y, sub, gc.getFont(), gc.getBackground(), - gc.getForeground())); - Point p = gc.stringExtent(sub); - if (sub.endsWith("\n")) { //$NON-NLS-1$ - cy += textWidgetLineHeight + textWidget.getLineSpacing(); - cx = x; - } else { - cx += p.x; - } - } - gc.setFont(currentFont); - } else { - int lfCount = 0; - if (sub.contains("\n")) { //$NON-NLS-1$ - lfCount = sub.split("\n", -1).length - 1; //$NON-NLS-1$ - sub = sub.substring(sub.lastIndexOf("\n") + 1); //$NON-NLS-1$ - } - Point p = gc.stringExtent(sub); - if (lfCount > 0) { - cy += lfCount * (textWidgetLineHeight + textWidget.getLineSpacing()); - cx = x; - } - cx += p.x; - } - } + drawStyleRanges(gc, textWidget, ranges, label, styledFonts, x, y, foregrounds::add); return result; } @@ -1168,21 +1319,7 @@ private boolean isLastForCurrentOffset(List ranges, int i, int offse } private StyleRange transformFontStyleToFont(Font baseFont, StyleRange styleRange) { - // as per the StyleRange contract, only consider fontStyle if font is not - // already set - if (styleRange.font == null && styleRange.fontStyle > 0) { - StyleRange newRange = (StyleRange) styleRange.clone(); - newRange.font = styledFonts.computeIfAbsent(baseFont, f -> new HashMap<>()) - .computeIfAbsent(Integer.valueOf(styleRange.fontStyle), s -> { - FontData[] fontDatas = baseFont.getFontData(); - for (FontData fontData : fontDatas) { - fontData.setStyle(styleRange.fontStyle); - } - return new Font(baseFont.getDevice(), fontDatas); - }); - return newRange; - } - return styleRange; + return UnifiedDiffCodeMiningProvider.transformFontStyleToFont(styledFonts, baseFont, styleRange); } private int getOffsetAtLine(String str, int off) { @@ -1216,6 +1353,74 @@ private int getYForLine(int line, int y, GC gc, StyledText textWidget) { } } + static void openOverlay(StyledText overlay, ITextViewer viewer) { + overlay.addFocusListener(new FocusAdapter() { + @Override + public void focusLost(FocusEvent e) { + overlay.dispose(); + setTextEditorActionsActivated(viewer, true); + } + }); + overlay.addKeyListener(new KeyAdapter() { + @Override + public void keyPressed(KeyEvent e) { + if (e.keyCode == SWT.ESC) { + overlay.dispose(); + setTextEditorActionsActivated(viewer, true); + } + e.doit = false; + } + }); + setTextEditorActionsActivated(viewer, false); + } + + static void setTextEditorActionsActivated(ITextViewer viewer, boolean state) { + IEditorPart part = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage().getActiveEditor(); + if (part instanceof MultiPageEditorPart multiPageEditorPart) { + Object page = multiPageEditorPart.getSelectedPage(); + if (page instanceof IEditorPart editorPart) { + part = editorPart; + } + } + if (!(part instanceof AbstractTextEditor) || part.getSite().getWorkbenchWindow().isClosing()) { + return; + } + if (UnifiedDiffManager.isViewerInPart(part, viewer)) { + try { + Method method = AbstractTextEditor.class.getDeclaredMethod("setActionActivation", //$NON-NLS-1$ + boolean.class); + method.setAccessible(true); + method.invoke(part, Boolean.valueOf(state)); + } catch (IllegalArgumentException | ReflectiveOperationException ex) { + error(ex); + } + } + } + + /** + * Returns a {@link StyleRange} whose {@code font} carries the range's font + * style. Fonts are cached in the caller-owned {@code styledFonts} map so the + * cache lifetime stays tied to the owning mining instance. + */ + static StyleRange transformFontStyleToFont(Map> styledFonts, Font baseFont, + StyleRange styleRange) { + // as per the StyleRange contract, only consider fontStyle if font is not + // already set + if (styleRange.font == null && styleRange.fontStyle > 0) { + StyleRange newRange = (StyleRange) styleRange.clone(); + newRange.font = styledFonts.computeIfAbsent(baseFont, f -> new HashMap<>()) + .computeIfAbsent(Integer.valueOf(styleRange.fontStyle), s -> { + FontData[] fontDatas = baseFont.getFontData(); + for (FontData fontData : fontDatas) { + fontData.setStyle(styleRange.fontStyle); + } + return new Font(baseFont.getDevice(), fontDatas); + }); + return newRange; + } + return styleRange; + } + // from inner class ColorPalette in TextMergeViewer static RGB interpolate(RGB fg, RGB bg, double scale) { if (fg != null && bg != null) { diff --git a/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffCodeMiningProviderTest.java b/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffCodeMiningProviderTest.java index 6505ec9b0db..3b088d8add0 100644 --- a/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffCodeMiningProviderTest.java +++ b/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffCodeMiningProviderTest.java @@ -19,6 +19,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assumptions.assumeTrue; import java.util.ArrayList; import java.util.Iterator; @@ -33,6 +34,7 @@ import org.eclipse.compare.unifieddiff.UnifiedDiffMode; import org.eclipse.compare.unifieddiff.internal.UnifiedDiffCodeMiningProvider; import org.eclipse.compare.unifieddiff.internal.UnifiedDiffCodeMiningProvider.FoldedRegionCodeMining; +import org.eclipse.compare.unifieddiff.internal.UnifiedDiffCodeMiningProvider.UnifiedDiffFooterCodeMining; import org.eclipse.compare.unifieddiff.internal.UnifiedDiffCodeMiningProvider.UnifiedDiffLineHeaderCodeMining; import org.eclipse.compare.unifieddiff.internal.UnifiedDiffManager; import org.eclipse.compare.unifieddiff.internal.UnifiedDiffManager.UnifiedDiff; @@ -44,17 +46,35 @@ import org.eclipse.jface.text.IDocument; import org.eclipse.jface.text.ITextViewer; import org.eclipse.jface.text.Position; +import org.eclipse.jface.text.TextAttribute; import org.eclipse.jface.text.codemining.ICodeMining; import org.eclipse.jface.text.codemining.ICodeMiningProvider; +import org.eclipse.jface.text.presentation.IPresentationReconciler; +import org.eclipse.jface.text.presentation.PresentationReconciler; +import org.eclipse.jface.text.rules.DefaultDamagerRepairer; +import org.eclipse.jface.text.rules.IRule; +import org.eclipse.jface.text.rules.IWordDetector; +import org.eclipse.jface.text.rules.RuleBasedScanner; +import org.eclipse.jface.text.rules.Token; +import org.eclipse.jface.text.rules.WordRule; import org.eclipse.jface.text.source.Annotation; import org.eclipse.jface.text.source.AnnotationModel; import org.eclipse.jface.text.source.AnnotationPainter; import org.eclipse.jface.text.source.IAnnotationModel; +import org.eclipse.jface.text.source.ISourceViewer; +import org.eclipse.jface.text.source.SourceViewerConfiguration; import org.eclipse.jface.text.source.inlined.AbstractInlinedAnnotation; import org.eclipse.jface.text.source.projection.ProjectionAnnotation; import org.eclipse.jface.text.source.projection.ProjectionAnnotationModel; import org.eclipse.jface.text.source.projection.ProjectionViewer; import org.eclipse.swt.SWT; +import org.eclipse.swt.graphics.Color; +import org.eclipse.swt.graphics.Font; +import org.eclipse.swt.graphics.FontData; +import org.eclipse.swt.graphics.GC; +import org.eclipse.swt.graphics.Image; +import org.eclipse.swt.graphics.ImageData; +import org.eclipse.swt.graphics.RGB; import org.eclipse.swt.widgets.Display; import org.eclipse.swt.widgets.Shell; import org.eclipse.text.undo.DocumentUndoManagerRegistry; @@ -90,6 +110,7 @@ public void setUp() { document = numberedLines(60); model = new AnnotationModel(); viewer = new ProjectionViewer(shell, null, null, false, SWT.V_SCROLL); + viewer.configure(syntaxColoringConfiguration()); viewer.setDocument(document, model); viewer.enableProjection(); DocumentUndoManagerRegistry.connect(document); @@ -362,8 +383,234 @@ public void testReopeningNextToAnAsynchronousProviderKeepsEveryMining() throws E waitForAttachedMinings(diffs, allRegions().size()); } + /** + * When the document does not end with a newline, the diff at the end cannot + * anchor a line-header mining (there is no following line to indent). A footer + * mining must be created instead. + */ + @Test + public void testFooterMiningIsCreatedWhenDocumentHasNoTrailingNewline() throws Exception { + switchToDocument(new Document("line 0\nline 1 changed")); + + IStatus status = UnifiedDiffManager.open(viewer, document, model, null, "line 0\nline 1\n", MODE, null, null, + null, true, CONTEXT_LINES); + assertTrue(status.isOK(), "open() should succeed: " + status); + + List minings = provide(); + List footers = new ArrayList<>(); + for (ICodeMining mining : minings) { + if (mining instanceof UnifiedDiffFooterCodeMining footer) { + footers.add(footer); + } + } + assertThat(footers).as("a footer mining is used for the diff at the end of a document without trailing newline") + .isNotEmpty(); + } + + /** + * A word-level footer highlight following bold text must start after the bold + * text, not where the base font would put it. Inspects the pixels + * {@link UnifiedDiffFooterCodeMining#draw(GC, org.eclipse.swt.custom.StyledText, Color, int, int)} + * paints. + */ + @Test + public void testFooterMiningDetailedDiffUsesStyledTextPosition() throws Exception { + // several bold keywords, so the bold/regular difference clearly exceeds a space + String keywords = "public public public public public public public public"; + switchToDocument(new Document("line 0\n" + keywords)); + + IStatus status = UnifiedDiffManager.open(viewer, document, model, null, "line 0\n" + keywords + " changed\n", + MODE, null, null, null, true, CONTEXT_LINES); + assertTrue(status.isOK(), "open() should succeed: " + status); + + List minings = provide(); + UnifiedDiffFooterCodeMining footer = null; + for (ICodeMining mining : minings) { + if (mining instanceof UnifiedDiffFooterCodeMining f) { + footer = f; + } + } + assertNotNull(footer, "a footer mining must be present"); + + Color deletionColor = new Color(display, 11, 22, 33); + Color detailedDiffColor = new Color(display, 44, 55, 66); + UnifiedDiffFooterCodeMining paintingFooter = new UnifiedDiffFooterCodeMining(document, provider, + footer.getUnifiedDiff(), 4, deletionColor, detailedDiffColor, viewer); + Image image = new Image(display, 1200, 100); + GC gc = new GC(image); + Font boldFont = null; + try { + paintingFooter.draw(gc, viewer.getTextWidget(), null, 0, 0); + + Font baseFont = viewer.getTextWidget().getFont(); + FontData[] boldData = baseFont.getFontData(); + for (FontData fontData : boldData) { + fontData.setStyle(SWT.BOLD); + } + boldFont = new Font(display, boldData); + // stringExtent measures at the display zoom, getImageData() at 100% + int zoom = display.getPrimaryMonitor().getZoom(); + gc.setFont(baseFont); + int baseWidth = gc.stringExtent(keywords + " ").x * 100 / zoom; + gc.setFont(boldFont); + int boldWidth = gc.stringExtent(keywords).x * 100 / zoom; + assumeTrue(boldWidth > baseWidth, "the viewer font renders bold no wider than regular, " + + "so this test cannot tell the two measurements apart"); + + int firstDetailedColumn = firstColumnWith(image, detailedDiffColor); + assertThat(firstDetailedColumn).as("draw() must paint the word-level detailed-diff background") + .isGreaterThanOrEqualTo(0); + assertThat(firstDetailedColumn) + .as("the highlight must be positioned with the bold font, not the narrower base font") + .isGreaterThan(baseWidth); + } finally { + gc.dispose(); + image.dispose(); + paintingFooter.dispose(); + if (boldFont != null) { + boldFont.dispose(); + } + deletionColor.dispose(); + detailedDiffColor.dispose(); + } + } + + /** + * A detailed diff spanning several lines must restart each line's word-level + * highlight at the line start instead of accumulating the width of the + * preceding lines, which would make the highlight cascade to the right. + */ + @Test + public void testFooterMiningMultiLineDetailedDiffResetsPerLine() throws Exception { + switchToDocument(new Document("line 0\nx")); + + String target = "line 0\nalpha alpha\nbravo bravo\ncarol carol\n"; + IStatus status = UnifiedDiffManager.open(viewer, document, model, null, target, MODE, null, null, null, true, + CONTEXT_LINES); + assertTrue(status.isOK(), "open() should succeed: " + status); + + List minings = provide(); + UnifiedDiffFooterCodeMining footer = null; + for (ICodeMining mining : minings) { + if (mining instanceof UnifiedDiffFooterCodeMining f) { + footer = f; + } + } + assertNotNull(footer, "a footer mining must be present"); + + Color deletionColor = new Color(display, 11, 22, 33); + Color detailedDiffColor = new Color(display, 44, 55, 66); + UnifiedDiffFooterCodeMining paintingFooter = new UnifiedDiffFooterCodeMining(document, provider, + footer.getUnifiedDiff(), 4, deletionColor, detailedDiffColor, viewer); + Image image = new Image(display, 400, 200); + GC gc = new GC(image); + try { + paintingFooter.draw(gc, viewer.getTextWidget(), null, 0, 0); + + int lineHeight = viewer.getTextWidget().getLineHeight(); + int lineSpacing = viewer.getTextWidget().getLineSpacing(); + // the line metrics are at the display zoom, so read the pixels at that zoom + int zoom = display.getPrimaryMonitor().getZoom(); + ImageData data = image.getImageData(zoom); + RGB detailedRgb = detailedDiffColor.getRGB(); + + List paintedLastColumns = new ArrayList<>(); + for (int lineIndex = 0; lineIndex < 8; lineIndex++) { + int yTop = lineIndex * (lineHeight + lineSpacing); + int lastColumn = lastColumnWithInBand(data, detailedRgb, yTop, yTop + lineHeight); + if (lastColumn >= 0) { + paintedLastColumns.add(lastColumn); + } + } + assertThat(paintedLastColumns).as("the multi-line detailed-diff background must span several lines") + .hasSizeGreaterThanOrEqualTo(2); + // cascading would widen each line by about a full line width + int min = paintedLastColumns.stream().mapToInt(Integer::intValue).min().getAsInt(); + int max = paintedLastColumns.stream().mapToInt(Integer::intValue).max().getAsInt(); + assertThat(max).as("later lines must not cascade to the right").isLessThanOrEqualTo(min * 2); + } finally { + gc.dispose(); + image.dispose(); + paintingFooter.dispose(); + deletionColor.dispose(); + detailedDiffColor.dispose(); + } + } + + /** + * Returns the rightmost column in {@code [yTop, yBottom)} holding + * {@code target}, or {@code -1}. Matches with a small tolerance against + * anti-aliasing. + */ + private static int lastColumnWithInBand(ImageData data, RGB target, int yTop, int yBottom) { + int bottom = Math.min(yBottom, data.height); + int last = -1; + for (int x = 0; x < data.width; x++) { + for (int y = Math.max(0, yTop); y < bottom; y++) { + RGB rgb = data.palette.getRGB(data.getPixel(x, y)); + if (Math.abs(rgb.red - target.red) <= 2 && Math.abs(rgb.green - target.green) <= 2 + && Math.abs(rgb.blue - target.blue) <= 2) { + last = x; + break; + } + } + } + return last; + } + + /** + * Returns the leftmost column holding {@code color}, or {@code -1}. Matches + * with a small tolerance against anti-aliasing. + */ + private static int firstColumnWith(Image image, Color color) { + ImageData data = image.getImageData(); + RGB target = color.getRGB(); + for (int x = 0; x < data.width; x++) { + for (int y = 0; y < data.height; y++) { + RGB rgb = data.palette.getRGB(data.getPixel(x, y)); + if (Math.abs(rgb.red - target.red) <= 2 && Math.abs(rgb.green - target.green) <= 2 + && Math.abs(rgb.blue - target.blue) <= 2) { + return x; + } + } + } + return -1; + } + // ------------------------------------------------------------------ helpers + /** + * A test configuration that colors all text and renders {@code public} in bold. + */ + private static SourceViewerConfiguration syntaxColoringConfiguration() { + return new SourceViewerConfiguration() { + @Override + public IPresentationReconciler getPresentationReconciler(ISourceViewer sourceViewer) { + PresentationReconciler reconciler = new PresentationReconciler(); + RuleBasedScanner scanner = new RuleBasedScanner(); + Color fg = Display.getCurrent().getSystemColor(SWT.COLOR_DARK_BLUE); + scanner.setDefaultReturnToken(new Token(new TextAttribute(fg))); + WordRule publicKeyword = new WordRule(new IWordDetector() { + @Override + public boolean isWordStart(char character) { + return Character.isJavaIdentifierStart(character); + } + + @Override + public boolean isWordPart(char character) { + return Character.isJavaIdentifierPart(character); + } + }); + publicKeyword.addWord("public", new Token(new TextAttribute(fg, null, SWT.BOLD))); + scanner.setRules(new IRule[] { publicKeyword }); + DefaultDamagerRepairer dr = new DefaultDamagerRepairer(scanner); + reconciler.setDamager(dr, IDocument.DEFAULT_CONTENT_TYPE); + reconciler.setRepairer(dr, IDocument.DEFAULT_CONTENT_TYPE); + return reconciler; + } + }; + } + /** A provider that answers only when the test lets it, like a slow editor. */ private final class PendingProvider implements ICodeMiningProvider { @@ -422,6 +669,8 @@ private void assertMinings(List minings, List diffs) { for (ICodeMining mining : minings) { if (mining instanceof UnifiedDiffLineHeaderCodeMining overlay) { shown.add(overlay.getUnifiedDiff()); + } else if (mining instanceof UnifiedDiffFooterCodeMining footer) { + shown.add(footer.getUnifiedDiff()); } else if (mining instanceof FoldedRegionCodeMining expander) { expanders.add(Integer.valueOf(expander.getPosition().getOffset())); } else { @@ -474,6 +723,8 @@ private void waitForAttachedMinings(List diffs, int regions) { for (ICodeMining mining : attachedMinings()) { if (mining instanceof UnifiedDiffLineHeaderCodeMining overlay) { shown.add(overlay.getUnifiedDiff()); + } else if (mining instanceof UnifiedDiffFooterCodeMining footer) { + shown.add(footer.getUnifiedDiff()); } else if (mining instanceof FoldedRegionCodeMining) { expanders++; } @@ -571,4 +822,17 @@ private static IDocument numberedLines(int count) { } return new Document(content.toString()); } + + /** + * Replaces the viewer's document mid-test. Disconnects the undo manager from + * the old document, connects it to the new one, and re-wires the viewer. + */ + private void switchToDocument(IDocument newDocument) { + DocumentUndoManagerRegistry.disconnect(document); + document = newDocument; + model = new AnnotationModel(); + viewer.setDocument(document, model); + DocumentUndoManagerRegistry.connect(document); + installCodeMinings(provider); + } }