Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

code minings drawn more consistently always via drawAsLeftOf1stCharacter #2243

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.jface.text
Bundle-Version: 3.25.300.qualifier
Bundle-Version: 3.26.0.qualifier
Bundle-Vendor: %providerName
Bundle-Localization: plugin
Export-Package:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ public class CodeMiningLineContentAnnotation extends LineContentAnnotation imple
*/
private IProgressMonitor fMonitor;

private final boolean afterPosition;

/**
* Code mining annotation constructor.
*
Expand All @@ -72,6 +74,21 @@ public CodeMiningLineContentAnnotation(Position position, ISourceViewer viewer)
fResolvedMinings= null;
fMinings= new ArrayList<>();
fBounds= new ArrayList<>();
afterPosition= false;
}

/**
* Code mining annotation constructor.
*
* @param position the position
* @param viewer the viewer
*/
public CodeMiningLineContentAnnotation(Position position, ISourceViewer viewer, boolean afterPosition) {
super(position, viewer);
fResolvedMinings= null;
fMinings= new ArrayList<>();
fBounds= new ArrayList<>();
this.afterPosition= afterPosition;
}

@Override
Expand Down Expand Up @@ -183,4 +200,8 @@ public Consumer<MouseEvent> getAction(MouseEvent e) {
public boolean isInVisibleLines() {
return super.isInVisibleLines();
}

public final boolean isAfterPosition() {
return afterPosition;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.eclipse.jface.text.Position;
import org.eclipse.jface.text.codemining.ICodeMining;
import org.eclipse.jface.text.codemining.ICodeMiningProvider;
import org.eclipse.jface.text.codemining.LineContentCodeMining;
import org.eclipse.jface.text.codemining.LineHeaderCodeMining;
import org.eclipse.jface.text.source.ISourceViewer;
import org.eclipse.jface.text.source.inlined.AbstractInlinedAnnotation;
Expand Down Expand Up @@ -251,12 +252,17 @@ private void renderCodeMinings(Map<Position, List<ICodeMining>> groups, ISourceV

Position pos= new Position(g.getKey().offset, g.getKey().length);
List<ICodeMining> minings= g.getValue();
boolean inLineHeader= !minings.isEmpty() ? (minings.get(0) instanceof LineHeaderCodeMining) : true;
ICodeMining first= minings.get(0);
boolean inLineHeader= !minings.isEmpty() ? (first instanceof LineHeaderCodeMining) : true;
// Try to find existing annotation
AbstractInlinedAnnotation ann= fInlinedAnnotationSupport.findExistingAnnotation(pos);
if (ann == null) {
// The annotation doesn't exists, create it.
ann= inLineHeader ? new CodeMiningLineHeaderAnnotation(pos, viewer) : new CodeMiningLineContentAnnotation(pos, viewer);
boolean afterPosition= false;
if (first instanceof LineContentCodeMining m) {
afterPosition= m.isAfterPosition();
}
ann= inLineHeader ? new CodeMiningLineHeaderAnnotation(pos, viewer) : new CodeMiningLineContentAnnotation(pos, viewer, afterPosition);
} else if (ann instanceof ICodeMiningAnnotation && ((ICodeMiningAnnotation) ann).isInVisibleLines()) {
// annotation is in visible lines
annotationsToRedraw.add((ICodeMiningAnnotation) ann);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
*******************************************************************************/
package org.eclipse.jface.text;

import java.util.HashSet;
import java.util.Set;
import java.util.HashMap;
import java.util.Map;

import org.eclipse.swt.SWT;
import org.eclipse.swt.custom.StyleRange;
Expand Down Expand Up @@ -405,6 +405,10 @@ private void drawCharRange(GC gc, int startOffset, int endOffset, int lineOffset
break;
case '\r':
if (fShowCarriageReturn) {
if (visibleChar.length() > 0 && cache.contains(fTextWidget, lineOffset + textOffset)) {
textOffset--;
break;
}
visibleChar.append(CARRIAGE_RETURN_SIGN);
}
if (textOffset >= endOffsetInLine - 1 || lineText.charAt(textOffset + 1) != '\n') {
Expand All @@ -414,6 +418,10 @@ private void drawCharRange(GC gc, int startOffset, int endOffset, int lineOffset
continue;
case '\n':
if (fShowLineFeed) {
if (visibleChar.length() > 0 && cache.contains(fTextWidget, lineOffset + textOffset)) {
textOffset--;
break;
}
visibleChar.append(LINE_FEED_SIGN);
}
eol= true;
Expand All @@ -439,7 +447,7 @@ private void drawCharRange(GC gc, int startOffset, int endOffset, int lineOffset
fg= styleRange.foreground;
}
}
draw(gc, widgetOffset, visibleChar.toString(), fg);
draw(gc, widgetOffset, visibleChar.toString(), fg, cache);
}
visibleChar.delete(0, visibleChar.length());
}
Expand Down Expand Up @@ -492,40 +500,57 @@ private void redrawAll() {
* @param s the string to be drawn
* @param fg the foreground color
*/
private void draw(GC gc, int offset, String s, Color fg) {
private void draw(GC gc, int offset, String s, Color fg,StyleRangeWithMetricsOffsets cache) {
// Compute baseline delta (see https://bugs.eclipse.org/bugs/show_bug.cgi?id=165640)
int baseline= fTextWidget.getBaseline(offset);
FontMetrics fontMetrics= gc.getFontMetrics();
int fontBaseline= fontMetrics.getAscent() + fontMetrics.getLeading();
int baslineDelta= baseline - fontBaseline;

Point pos= fTextWidget.getLocationAtOffset(offset);
StyleRange styleRange= cache.get(fTextWidget, offset);
if (styleRange != null && styleRange.metrics != null) { // code mining at \r or \n character - line break character should be drawn at end of code mining
String charBeforeOffset= " "; //$NON-NLS-1$
if (offset > 0) {
charBeforeOffset= fTextWidget.getText(offset - 1, offset - 1);
}
Point extCharBeforeOffset= gc.textExtent(charBeforeOffset);
pos.x= pos.x + styleRange.metrics.width - extCharBeforeOffset.x;
}
gc.setForeground(fg);
gc.drawString(s, pos.x, pos.y + baslineDelta, true);
}

private static class StyleRangeWithMetricsOffsets {
private Set<Integer> offsets= null;
private Map<Integer, StyleRange> offsets= null;

public boolean contains(StyledText st, int offset) {
if (offsets == null) {
fillSet(st);
fillMap(st);
}
if (offsets.contains(offset)) {
if (offsets.containsKey(offset)) {
return true;
}
return false;
}

private void fillSet(StyledText st) {
offsets= new HashSet<>();
public StyleRange get(StyledText st, int offset) {
if (offsets == null) {
fillMap(st);
}
StyleRange styleRange= offsets.get(offset);
return styleRange;
}

private void fillMap(StyledText st) {
offsets= new HashMap<>();
StyleRange[] ranges= st.getStyleRanges();
if (ranges == null) {
return;
}
for (StyleRange range : ranges) {
if (range != null && range.metrics != null) {
offsets.add(range.start);
offsets.put(range.start, range);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
*/
public abstract class LineContentCodeMining extends AbstractCodeMining {

private final boolean afterPosition;

/**
* CodeMining constructor to locate the code mining in a given position.
*
Expand All @@ -36,6 +38,20 @@ public LineContentCodeMining(Position position, ICodeMiningProvider provider) {
this(position, provider, null);
}

/**
* CodeMining constructor to locate the code mining in a given position.
*
* @param position the position where the mining must be drawn.
* @param afterPosition if true code mining is treated as suffix code mining where cursor and
* selection is not including the mining
* @param provider the owner codemining provider which creates this mining.
*
* @since 3.26
*/
public LineContentCodeMining(Position position, boolean afterPosition, ICodeMiningProvider provider) {
this(position, afterPosition, provider, null);
}

/**
* CodeMining constructor to locate the code mining in a given position.
*
Expand All @@ -44,7 +60,33 @@ public LineContentCodeMining(Position position, ICodeMiningProvider provider) {
* @param action the action to execute when mining is clicked and null otherwise.
*/
public LineContentCodeMining(Position position, ICodeMiningProvider provider, Consumer<MouseEvent> action) {
this(position, false, provider, action);
}

/**
* CodeMining constructor to locate the code mining in a given position.
*
* @param position the position where the mining must be drawn.
* @param provider the owner codemining provider which creates this mining.
* @param action the action to execute when mining is clicked and null otherwise.
* @param afterPosition if true code mining is treated as suffix code mining where cursor and
* selection is not including the mining
*
* @since 3.26
*/
public LineContentCodeMining(Position position, boolean afterPosition, ICodeMiningProvider provider, Consumer<MouseEvent> action) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is new API, this will likely require some @since 3.26 and a version increase in the MANIFEST.

super(position, provider, action);
this.afterPosition= afterPosition;
}

/**
* indicates if code mining should be rendered after given position; cursor and selection does
* not include the code mining if set to true.
*
* @since 3.26
*/
public boolean isAfterPosition() {
return afterPosition;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import org.eclipse.swt.graphics.Point;
import org.eclipse.swt.graphics.Rectangle;

import org.eclipse.jface.internal.text.codemining.CodeMiningLineContentAnnotation;

import org.eclipse.jface.text.ITextViewer;
import org.eclipse.jface.text.source.Annotation;
import org.eclipse.jface.text.source.AnnotationPainter.IDrawingStrategy;
Expand Down Expand Up @@ -202,6 +204,12 @@ private static void draw(LineHeaderAnnotation annotation, GC gc, StyledText text
*/
private static void draw(LineContentAnnotation annotation, GC gc, StyledText textWidget, int widgetOffset, int length,
Color color) {
if (annotation instanceof CodeMiningLineContentAnnotation a) {
if (a.isAfterPosition()) {
drawAsLeftOf1stCharacter(annotation, gc, textWidget, widgetOffset, length, color);
return;
}
}
if (annotation.isEmptyLine(widgetOffset, textWidget)) {
drawAfterLine(annotation, gc, textWidget, widgetOffset, length, color);
} else if (LineContentAnnotation.drawRightToPreviousChar(widgetOffset, textWidget)) {
Expand Down Expand Up @@ -254,9 +262,18 @@ protected static void drawAsLeftOf1stCharacter(LineContentAnnotation annotation,

// Compute the location of the annotation
Rectangle bounds= textWidget.getTextBounds(widgetOffset, widgetOffset);
int x= bounds.x + (isEndOfLine ? bounds.width * 2 : 0);
int y= bounds.y;

int x;
if (isEndOfLine) {
// getTextBounds at offset with char '\r' or '\n' returns incorrect x position, use getLocationAtOffset instead
x= textWidget.getLocationAtOffset(widgetOffset).x;
} else {
x= bounds.x;
}
int y= bounds.y;
if (isAfterPosition(annotation)) {
isEndOfLine= false;
}
// When line text has line header annotation, there is a space on the top, adjust the y by using char height
y+= bounds.height - textWidget.getLineHeight();

Expand All @@ -275,14 +292,18 @@ protected static void drawAsLeftOf1stCharacter(LineContentAnnotation annotation,
// Get size of the character where GlyphMetrics width is added
Point charBounds= gc.stringExtent(hostCharacter);
int charWidth= charBounds.x;

if (charWidth == 0 && ("\r".equals(hostCharacter) || "\n".equals(hostCharacter))) { //$NON-NLS-1$ //$NON-NLS-2$
// charWidth is 0 for '\r' on font Consolas, but not on other fonts, why?
charWidth= gc.stringExtent(" ").x; //$NON-NLS-1$
}
// FIXME: remove this code when we need not redraw the character (see https://bugs.eclipse.org/bugs/show_bug.cgi?id=531769)
// START TO REMOVE
annotation.setRedrawnCharacterWidth(charWidth);
// END TO REMOVE

// Annotation takes place, add GlyphMetrics width to the style
StyleRange newStyle= annotation.updateStyle(style, gc.getFontMetrics(), textWidget.getData() instanceof ITextViewer viewer ? viewer : annotation.getViewer());
StyleRange newStyle= annotation.updateStyle(style, gc.getFontMetrics(), textWidget.getData() instanceof ITextViewer viewer ? viewer : annotation.getViewer(),
isAfterPosition(annotation));
if (newStyle != null) {
textWidget.setStyleRange(newStyle);
return;
Expand Down Expand Up @@ -328,6 +349,13 @@ protected static void drawAsLeftOf1stCharacter(LineContentAnnotation annotation,
}
}

private static boolean isAfterPosition(LineContentAnnotation annotation) {
if (annotation instanceof CodeMiningLineContentAnnotation a) {
return a.isAfterPosition();
}
return false;
}

protected static void drawAsRightOfPreviousCharacter(LineContentAnnotation annotation, GC gc, StyledText textWidget, int widgetOffset, int length, Color color) {
StyleRange style= null;
try {
Expand Down Expand Up @@ -365,7 +393,7 @@ protected static void drawAsRightOfPreviousCharacter(LineContentAnnotation annot
// END TO REMOVE

// Annotation takes place, add GlyphMetrics width to the style
StyleRange newStyle= annotation.updateStyle(style, gc.getFontMetrics(), InlinedAnnotationSupport.getSupport(textWidget).getViewer());
StyleRange newStyle= annotation.updateStyle(style, gc.getFontMetrics(), InlinedAnnotationSupport.getSupport(textWidget).getViewer(), isAfterPosition(annotation));
if (newStyle != null) {
textWidget.setStyleRange(newStyle);
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@

import org.eclipse.core.runtime.Assert;

import org.eclipse.jface.internal.text.codemining.CodeMiningLineContentAnnotation;

import org.eclipse.jface.text.BadLocationException;
import org.eclipse.jface.text.DocumentEvent;
import org.eclipse.jface.text.IDocument;
Expand Down Expand Up @@ -107,7 +109,7 @@ public void applyTextPresentation(TextPresentation textPresentation) {
.forEachRemaining(annotation -> {
if (annotation instanceof LineContentAnnotation) {
LineContentAnnotation ann= (LineContentAnnotation) annotation;
StyleRange style= ann.updateStyle(null, fFontMetrics, fViewer);
StyleRange style= ann.updateStyle(null, fFontMetrics, fViewer, isAfterPosition(ann));
if (style != null) {
if (fViewer instanceof ITextViewerExtension5 projectionViewer) {
IRegion annotationRegion= projectionViewer.widgetRange2ModelRange(new Region(style.start, style.length));
Expand All @@ -119,6 +121,13 @@ public void applyTextPresentation(TextPresentation textPresentation) {
}
});
}

private static boolean isAfterPosition(LineContentAnnotation annotation) {
if (annotation instanceof CodeMiningLineContentAnnotation a) {
return a.isAfterPosition();
}
return false;
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,16 @@ boolean contains(int x, int y) {
* @return the style to apply with GlyphMetrics width only if needed. It uses widget position,
* not model position.
*/
StyleRange updateStyle(StyleRange style, FontMetrics fontMetrics, ITextViewer viewer) {
StyleRange updateStyle(StyleRange style, FontMetrics fontMetrics, ITextViewer viewer, boolean afterPosition) {
Position widgetPosition= computeWidgetPosition(viewer);
if (widgetPosition == null) {
return null;
}
StyledText textWidget = viewer.getTextWidget();
boolean usePreviousChar= drawRightToPreviousChar(widgetPosition.getOffset(), textWidget);
boolean usePreviousChar= false;
if (!afterPosition) {
usePreviousChar= drawRightToPreviousChar(widgetPosition.getOffset(), textWidget);
}
if (width == 0 || getRedrawnCharacterWidth() == 0) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: CodeMinig Examples
Bundle-SymbolicName: org.eclipse.jface.text.examples
Bundle-Version: 1.2.0.qualifier
Bundle-Version: 1.2.100.qualifier
Bundle-RequiredExecutionEnvironment: JavaSE-17
Bundle-Vendor: Eclipse.org
Require-Bundle: org.eclipse.jface.text,
Expand Down
Loading
Loading