Skip to content

Commit 6d3ad18

Browse files
matthijskooijmanfacchinm
matthijskooijman
authored and
facchinm
committed
Handle CR without NL printed in EditorConsole
Previously, any CR without NL was treated just like a NL. For tools that used single CRs to update a progress bar (such as dfu-util), this would end up printing the subsequent versions of the progress bar below each other, instead of updating a single line as intended. Additionally, since ConsoleOutputStream only scrolled the view on \n, these updates would end up outside of the main view, making the upload progress quite unclear. This commit makes EditorConsole support lone CRs by resetting the insert position to the start of the current line, so subsequent writes overwrite existing content. If subsequent lines are shorter than an earlier line, only part of the earlier line will be overwritten (this mimics what terminal emulators do).
1 parent fa267da commit 6d3ad18

File tree

2 files changed

+207
-4
lines changed

2 files changed

+207
-4
lines changed

‎app/src/processing/app/EditorConsole.java

+52-4
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import javax.swing.text.*;
2828
import java.awt.*;
2929
import java.io.PrintStream;
30+
import java.util.regex.Matcher;
31+
import java.util.regex.Pattern;
3032

3133
import static processing.app.Theme.scale;
3234

@@ -37,6 +39,11 @@ public class EditorConsole extends JScrollPane {
3739

3840
private static ConsoleOutputStream out;
3941
private static ConsoleOutputStream err;
42+
private int startOfLine = 0;
43+
private int insertPosition = 0;
44+
45+
// Regex for linesplitting, see insertString for comments.
46+
private static final Pattern newLinePattern = Pattern.compile("([^\r\n]*)([\r\n]*\n)?(\r+)?");
4047

4148
public static synchronized void setCurrentEditorConsole(EditorConsole console) {
4249
if (out == null) {
@@ -161,6 +168,8 @@ public void applyPreferences() {
161168
public void clear() {
162169
try {
163170
document.remove(0, document.getLength());
171+
startOfLine = 0;
172+
insertPosition = 0;
164173
} catch (BadLocationException e) {
165174
// ignore the error otherwise this will cause an infinite loop
166175
// maybe not a good idea in the long run?
@@ -176,10 +185,49 @@ public boolean isEmpty() {
176185
return document.getLength() == 0;
177186
}
178187

179-
public void insertString(String line, SimpleAttributeSet attributes) throws BadLocationException {
180-
line = line.replace("\r\n", "\n").replace("\r", "\n");
181-
int offset = document.getLength();
182-
document.insertString(offset, line, attributes);
188+
public void insertString(String str, SimpleAttributeSet attributes) throws BadLocationException {
189+
// Separate the string into content, newlines and lone carriage
190+
// returns.
191+
//
192+
// Doing so allows lone CRs to move the insertPosition back to the
193+
// start of the line to allow overwriting the most recent line (e.g.
194+
// for a progress bar). Any CR or NL that are immediately followed
195+
// by another NL are bunched together for efficiency, since these
196+
// can just be inserted into the document directly and still be
197+
// correct.
198+
//
199+
// The regex is written so it will necessarily match any string
200+
// completely if applied repeatedly. This is important because any
201+
// part not matched would be silently dropped.
202+
Matcher m = newLinePattern.matcher(str);
203+
204+
while (m.find()) {
205+
String content = m.group(1);
206+
String newlines = m.group(2);
207+
String crs = m.group(3);
208+
209+
// Replace (or append if at end of the document) the content first
210+
int replaceLength = Math.min(content.length(), document.getLength() - insertPosition);
211+
document.replace(insertPosition, replaceLength, content, attributes);
212+
insertPosition += content.length();
213+
214+
// Then insert any newlines, but always at the end of the document
215+
// e.g. if insertPosition is halfway a line, do not delete
216+
// anything, just add the newline(s) at the end).
217+
if (newlines != null) {
218+
document.insertString(document.getLength(), newlines, attributes);
219+
insertPosition = document.getLength();
220+
startOfLine = insertPosition;
221+
}
222+
223+
// Then, for any CRs not followed by newlines, move insertPosition
224+
// to the start of the line. Note that if a newline follows before
225+
// any content in the next call to insertString, it will be added
226+
// at the end of the document anyway, as expected.
227+
if (crs != null) {
228+
insertPosition = startOfLine;
229+
}
230+
}
183231
}
184232

185233
public String getText() {

‎app/test/processing/app/EditorConsoleTest.java

+155
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
/*
2+
* This file is part of Arduino.
3+
*
4+
* Copyright 2020 Arduino LLC (http://www.arduino.cc/)
5+
*
6+
* Arduino is free software; you can redistribute it and/or modify
7+
* it under the terms of the GNU General Public License as published by
8+
* the Free Software Foundation; either version 2 of the License, or
9+
* (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14+
* GNU General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU General Public License
17+
* along with this program; if not, write to the Free Software
18+
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
19+
*
20+
* As a special exception, you may use this file as part of a free software
21+
* library without restriction. Specifically, if other files instantiate
22+
* templates or use macros or inline functions from this file, or you compile
23+
* this file and link it with other files to produce an executable, this
24+
* file does not by itself cause the resulting executable to be covered by
25+
* the GNU General Public License. This exception does not however
26+
* invalidate any other reasons why the executable file might be covered by
27+
* the GNU General Public License.
28+
*/
29+
30+
package processing.app;
31+
32+
import static org.junit.Assert.assertEquals;
33+
34+
import org.junit.Before;
35+
import org.junit.Test;
36+
37+
public class EditorConsoleTest extends AbstractWithPreferencesTest {
38+
private EditorConsole console;
39+
40+
@Before
41+
public void createConsole() {
42+
console = new EditorConsole(null);
43+
}
44+
45+
public String escapeString(String input) {
46+
// This escapes backslashes, newlines and carriage returns, to get
47+
// more readable assertion failures.
48+
return input.replace("\\", "\\\\").replace("\n", "\\n").replace("\r", "\\r");
49+
}
50+
51+
public void assertOutput(String output) {
52+
assertEquals(escapeString(output), escapeString(console.getText()));
53+
}
54+
55+
@Test
56+
public void testHelloWorld() throws Exception {
57+
console.insertString("Hello, world!", null);
58+
59+
assertOutput("Hello, world!");
60+
}
61+
62+
@Test
63+
public void testCrNlHandling() throws Exception {
64+
// Do some basic tests with \r\n
65+
console.insertString("abc\r\ndef", null);
66+
assertOutput("abc\r\ndef");
67+
68+
console.insertString("xyz", null);
69+
assertOutput("abc\r\ndefxyz");
70+
71+
console.insertString("000\r\n123", null);
72+
assertOutput("abc\r\ndefxyz000\r\n123");
73+
74+
console.insertString("\r\n", null);
75+
assertOutput("abc\r\ndefxyz000\r\n123\r\n");
76+
}
77+
78+
@Test
79+
public void testNlHandling() throws Exception {
80+
// Basic tests, but with just \n
81+
console.insertString("abc\ndef", null);
82+
assertOutput("abc\ndef");
83+
84+
console.insertString("xyz", null);
85+
assertOutput("abc\ndefxyz");
86+
87+
console.insertString("000\n123", null);
88+
assertOutput("abc\ndefxyz000\n123");
89+
90+
console.insertString("\n", null);
91+
assertOutput("abc\ndefxyz000\n123\n");
92+
}
93+
94+
@Test
95+
public void testCrHandling() throws Exception {
96+
// Then test that single \r clears the current line
97+
console.clear();
98+
console.insertString("abc\rdef", null);
99+
assertOutput("def");
100+
101+
// A single \r at the end is not added to the document
102+
console.insertString("\r", null);
103+
assertOutput("def");
104+
105+
// Nor are multiple \r at the end
106+
console.insertString("\r\r\r", null);
107+
assertOutput("def");
108+
109+
// But it does clear the line on the next write
110+
console.insertString("123", null);
111+
assertOutput("123");
112+
113+
// Same when combined with some data
114+
console.insertString("\r456\r\r", null);
115+
assertOutput("456");
116+
117+
console.insertString("000", null);
118+
assertOutput("000");
119+
120+
// Then add a newline so preceding data is kept
121+
console.insertString("\r\nxxx\r", null);
122+
assertOutput("000\r\nxxx");
123+
124+
// But data after the newline is removed
125+
console.insertString("yyy", null);
126+
assertOutput("000\r\nyyy");
127+
128+
// When a \r\n is split across inserts, it becomes a lone \n
129+
console.insertString("\r", null);
130+
assertOutput("000\r\nyyy");
131+
console.insertString("\n", null);
132+
assertOutput("000\r\nyyy\n");
133+
}
134+
135+
@Test
136+
public void testCrPartialOverwrite() throws Exception {
137+
console.insertString("abcdef\r", null);
138+
assertOutput("abcdef");
139+
140+
console.insertString("123", null);
141+
assertOutput("123def");
142+
143+
console.insertString("4", null);
144+
assertOutput("1234ef");
145+
146+
console.insertString("\r\n56", null);
147+
assertOutput("1234ef\r\n56");
148+
}
149+
150+
@Test
151+
public void testTogether() throws Exception {
152+
console.insertString("abc\n123456\rdef\rx\r\nyyy\nzzz\r999", null);
153+
assertOutput("abc\nxef456\r\nyyy\n999");
154+
}
155+
}

0 commit comments

Comments
 (0)