WhitespaceCharacterPainter: fix off-by-one error at CR and LF offsets

eclipse-platform · Oct 18, 2024 · ae9e870 · ae9e870
1 parent 929564c
commit ae9e870
Show file tree

Hide file tree

Showing 2 changed files with 97 additions and 0 deletions.
diff --git a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/WhitespaceCharacterPainter.java b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/WhitespaceCharacterPainter.java
@@ -407,6 +407,7 @@ private void drawCharRange(GC gc, int startOffset, int endOffset, int lineOffset
 						if (fShowCarriageReturn) {
 							if (visibleChar.length() > 0 && cache.contains(fTextWidget, lineOffset + textOffset)) {
 								textOffset--;
+								delta--;
 								break;
 							}
 							visibleChar.append(CARRIAGE_RETURN_SIGN);
@@ -420,6 +421,7 @@ private void drawCharRange(GC gc, int startOffset, int endOffset, int lineOffset
 						if (fShowLineFeed) {
 							if (visibleChar.length() > 0 && cache.contains(fTextWidget, lineOffset + textOffset)) {
 								textOffset--;
+								delta--;
 								break;
 							}
 							visibleChar.append(LINE_FEED_SIGN);

diff --git a/...pse.jface.text.tests/src/org/eclipse/jface/text/tests/TestWhitespaceCharacterPainter.java b/...pse.jface.text.tests/src/org/eclipse/jface/text/tests/TestWhitespaceCharacterPainter.java
@@ -11,14 +11,18 @@
  *******************************************************************************/
 package org.eclipse.jface.text.tests;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
@@ -63,6 +67,28 @@ public void after() {
 		shell.dispose();
 	}
 
+	@Test
+	public void multipleSpacesAfterNewLine() throws Exception {
+		List<DrawStringParams> params= collectDrawStringParamsInPaintControl("\n     \n  ", Arrays.asList(6));
+		assertEquals(4, params.size());
+		DrawStringParams first= params.get(0);
+		assertEquals("\u00b6", first.str);
+		DrawStringParams second= params.get(1);
+		assertEquals("\u00b7\u00b7\u00b7\u00b7\u00b7", second.str);
+		assertNotEquals(first.y, second.y); // y pos of first and second line should be different
+	}
+
+	@Test
+	public void multipleSpacesAfterCarriageReturn() throws Exception {
+		List<DrawStringParams> params= collectDrawStringParamsInPaintControl("\r\n     \r\n  ", Arrays.asList(7));
+		assertEquals(4, params.size());
+		DrawStringParams first= params.get(0);
+		assertEquals("\u00a4\u00b6", first.str);
+		DrawStringParams second= params.get(1);
+		assertEquals("\u00b7\u00b7\u00b7\u00b7\u00b7", second.str);
+		assertNotEquals(first.y, second.y); // y pos of first and second line should be different
+	}
+
 	@Test
 	public void glyphMetricsTakenIntoAccount() throws Exception {
 		verifyDrawStringCalledNTimes("first  \nsecond  \nthird  \n", Arrays.asList(6, 15), 5);
@@ -133,6 +159,75 @@ public FontMetrics answer(InvocationOnMock invocation) throws Throwable {
 		verify(ev.gc, times(times)).drawString(anyString(), anyInt(), anyInt(), anyBoolean());
 	}
 
+	private static final record DrawStringParams(String str, int x, int y) {
+	}
+
+	private List<DrawStringParams> collectDrawStringParamsInPaintControl(String source, List<Integer> styleRangeOffsets) {
+		SourceViewer sourceViewer= new SourceViewer(shell, null, SWT.V_SCROLL | SWT.BORDER);
+		sourceViewer.setDocument(new Document(source));
+		StyledText textWidget= sourceViewer.getTextWidget();
+		textWidget.setFont(JFaceResources.getTextFont());
+		WhitespaceCharacterPainter whitespaceCharPainter= new WhitespaceCharacterPainter(sourceViewer, true, true, true, true, true, true, true,
+				true, true, true, true, 100);
+		sourceViewer.addPainter(whitespaceCharPainter);
+		for (Integer offset : styleRangeOffsets) {
+			textWidget.setStyleRange(createStyleRangeWithMetrics(offset));
+		}
+		Event e= new Event();
+		e.widget= textWidget;
+		PaintEvent ev= new PaintEvent(e);
+
+		ev.gc= mock(GC.class);
+		when(ev.gc.getClipping()).thenReturn(new Rectangle(0, 0, 100, 100));
+		when(ev.gc.stringExtent(anyString())).thenAnswer(new Answer<Point>() {
+			@Override
+			public Point answer(InvocationOnMock invocation) throws Throwable {
+				GC gc= new GC(shell);
+				gc.setFont(JFaceResources.getTextFont());
+				Point result= gc.stringExtent(invocation.getArgument(0));
+				gc.dispose();
+				return result;
+			}
+		});
+		when(ev.gc.textExtent(anyString())).thenAnswer(new Answer<Point>() {
+			@Override
+			public Point answer(InvocationOnMock invocation) throws Throwable {
+				GC gc= new GC(shell);
+				gc.setFont(JFaceResources.getTextFont());
+				Point result= gc.textExtent(invocation.getArgument(0));
+				gc.dispose();
+				return result;
+			}
+		});
+		when(ev.gc.getFontMetrics()).thenAnswer(new Answer<FontMetrics>() {
+			@Override
+			public FontMetrics answer(InvocationOnMock invocation) throws Throwable {
+				GC gc= new GC(shell);
+				gc.setFont(JFaceResources.getTextFont());
+				FontMetrics metrics= gc.getFontMetrics();
+				gc.dispose();
+				return metrics;
+			}
+		});
+		List<DrawStringParams> params= new ArrayList<>();
+		doAnswer(new Answer<Void>() {
+			@Override
+			public Void answer(InvocationOnMock invocation) throws Throwable {
+				String str= invocation.getArgument(0, String.class);
+				Integer x= invocation.getArgument(1, Integer.class);
+				Integer y= invocation.getArgument(2, Integer.class);
+				params.add(new DrawStringParams(str, x, y));
+				return null;
+			}
+		}).when(ev.gc).drawString(anyString(), anyInt(), anyInt(), anyBoolean());
+		ev.x= 0;
+		ev.y= 0;
+		ev.width= 100;
+		ev.height= 100;
+		whitespaceCharPainter.paintControl(ev);
+		return params;
+	}
+
 	private StyleRange createStyleRangeWithMetrics(int start) {
 		StyleRange sr= new StyleRange();
 		sr.start= start;