Skip to content

Commit 950ca8a

Browse files
Display.withCrLf() is producing wrong line breaks on Windows
Replacing all line breaks on windows with CrLf using regex, which is much safer. Fixes #1557
1 parent d7febc4 commit 950ca8a

File tree

2 files changed

+41
-84
lines changed

2 files changed

+41
-84
lines changed

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Display.java

Lines changed: 5 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -5152,85 +5152,16 @@ String wrapText (String text, long handle, int width) {
51525152
}
51535153

51545154
static String withCrLf (String string) {
5155-
5156-
/* If the string is empty, return the string. */
5157-
int length = string.length ();
5158-
if (length == 0) return string;
5159-
5160-
/*
5161-
* Check for an LF or CR/LF and assume the rest of
5162-
* the string is formated that way. This will not
5163-
* work if the string contains mixed delimiters.
5164-
*/
5165-
int i = string.indexOf ('\n', 0);
5166-
if (i == -1) return string;
5167-
if (i > 0 && string.charAt (i - 1) == '\r') {
5155+
if (string == null) {
51685156
return string;
51695157
}
5170-
5171-
/*
5172-
* The string is formatted with LF. Compute the
5173-
* number of lines and the size of the buffer
5174-
* needed to hold the result
5175-
*/
5176-
i++;
5177-
int count = 1;
5178-
while (i < length) {
5179-
if ((i = string.indexOf ('\n', i)) == -1) break;
5180-
count++; i++;
5181-
}
5182-
count += length;
5183-
5184-
/* Create a new string with the CR/LF line terminator. */
5185-
i = 0;
5186-
StringBuilder result = new StringBuilder (count);
5187-
while (i < length) {
5188-
int j = string.indexOf ('\n', i);
5189-
if (j == -1) j = length;
5190-
result.append (string.substring (i, j));
5191-
if ((i = j) < length) {
5192-
result.append ("\r\n"); //$NON-NLS-1$
5193-
i++;
5194-
}
5195-
}
5196-
return result.toString ();
5158+
// Replace \r\n, \r, or \n with \r\n
5159+
return string.replaceAll("(\r\n|\r|\n)", "\r\n");
51975160
}
51985161

51995162
static char [] withCrLf (char [] string) {
5200-
/* If the string is empty, return the string. */
5201-
int length = string.length;
5202-
if (length == 0) return string;
5203-
5204-
/*
5205-
* Check for an LF or CR/LF and assume the rest of
5206-
* the string is formated that way. This will not
5207-
* work if the string contains mixed delimiters.
5208-
* Also, compute the number of lines.
5209-
*/
5210-
int count = 0;
5211-
for (int i = 0; i < string.length; i++) {
5212-
if (string [i] == '\n') {
5213-
count++;
5214-
if (count == 1 && i > 0 && string [i - 1] == '\r') return string;
5215-
}
5216-
}
5217-
if (count == 0) return string;
5218-
5219-
/*
5220-
* The string is formatted with LF.
5221-
*/
5222-
count += length;
5223-
5224-
/* Create a new string with the CR/LF line terminator. */
5225-
char [] result = new char [count];
5226-
for (int i = 0, j = 0; i < length && j < count; i++) {
5227-
if (string [i] == '\n') {
5228-
result [j++] = '\r';
5229-
}
5230-
result [j++] = string [i];
5231-
}
5232-
5233-
return result;
5163+
String withCrLf = withCrLf(new String(string));
5164+
return withCrLf.toCharArray();
52345165
}
52355166

52365167
static boolean isActivateShellOnForceFocus() {

tests/org.eclipse.swt.tests.win32/JUnit Tests/org/eclipse/swt/tests/win32/widgets/Test_org_eclipse_swt_widgets_Display.java

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,26 +13,52 @@
1313
*******************************************************************************/
1414
package org.eclipse.swt.tests.win32.widgets;
1515

16+
import static org.junit.Assert.assertEquals;
17+
1618
import java.lang.reflect.InvocationTargetException;
1719
import java.lang.reflect.Method;
1820

21+
import org.eclipse.swt.SWT;
1922
import org.eclipse.swt.widgets.Display;
23+
import org.eclipse.swt.widgets.Shell;
24+
import org.eclipse.swt.widgets.Text;
25+
import org.junit.After;
26+
import org.junit.Before;
2027
import org.junit.Test;
2128

2229
public class Test_org_eclipse_swt_widgets_Display {
2330

31+
private Display display;
32+
private Shell shell;
33+
34+
@Before
35+
public void setup() {
36+
display = new Display();
37+
shell = new Shell(display);
38+
}
39+
40+
@After
41+
public void teardown() {
42+
shell.dispose();
43+
display.dispose();
44+
}
45+
2446
@Test
2547
public void test_isXMouseActive() throws NoSuchMethodException, SecurityException, IllegalAccessException,
2648
IllegalArgumentException, InvocationTargetException {
27-
Display display = new Display();
28-
try {
29-
Method method = display.getClass().getDeclaredMethod("isXMouseActive");
30-
method.setAccessible(true);
31-
32-
boolean xMouseActive = (boolean) method.invoke(display);
33-
System.out.println("org.eclipse.swt.widgets.Display.isXMouseActive(): " + xMouseActive);
34-
} finally {
35-
display.dispose();
36-
}
49+
Method method = display.getClass().getDeclaredMethod("isXMouseActive");
50+
method.setAccessible(true);
51+
52+
boolean xMouseActive = (boolean) method.invoke(display);
53+
System.out.println("org.eclipse.swt.widgets.Display.isXMouseActive(): " + xMouseActive);
54+
}
55+
56+
@Test
57+
public void test_mixedLfAndCrfl() {
58+
//Use text control for testing since Display.withCrLf() is package private
59+
Text text = new Text(shell, SWT.None);
60+
61+
text.setText("First Line \n second line \r\n third line");
62+
assertEquals("First Line \r\n second line \r\n third line", text.getText());
3763
}
3864
}

0 commit comments

Comments
 (0)