Skip to content

Commit

Permalink
Handle CR without NL printed in EditorConsole
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
matthijskooijman authored and facchinm committed May 12, 2020
1 parent fa267da commit 6d3ad18
Show file tree
Hide file tree
Showing 2 changed files with 207 additions and 4 deletions.
56 changes: 52 additions & 4 deletions app/src/processing/app/EditorConsole.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import javax.swing.text.*;
import java.awt.*;
import java.io.PrintStream;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static processing.app.Theme.scale;

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

private static ConsoleOutputStream out;
private static ConsoleOutputStream err;
private int startOfLine = 0;
private int insertPosition = 0;

// Regex for linesplitting, see insertString for comments.
private static final Pattern newLinePattern = Pattern.compile("([^\r\n]*)([\r\n]*\n)?(\r+)?");

public static synchronized void setCurrentEditorConsole(EditorConsole console) {
if (out == null) {
Expand Down Expand Up @@ -161,6 +168,8 @@ public void applyPreferences() {
public void clear() {
try {
document.remove(0, document.getLength());
startOfLine = 0;
insertPosition = 0;
} catch (BadLocationException e) {
// ignore the error otherwise this will cause an infinite loop
// maybe not a good idea in the long run?
Expand All @@ -176,10 +185,49 @@ public boolean isEmpty() {
return document.getLength() == 0;
}

public void insertString(String line, SimpleAttributeSet attributes) throws BadLocationException {
line = line.replace("\r\n", "\n").replace("\r", "\n");
int offset = document.getLength();
document.insertString(offset, line, attributes);
public void insertString(String str, SimpleAttributeSet attributes) throws BadLocationException {
// Separate the string into content, newlines and lone carriage
// returns.
//
// Doing so allows lone CRs to move the insertPosition back to the
// start of the line to allow overwriting the most recent line (e.g.
// for a progress bar). Any CR or NL that are immediately followed
// by another NL are bunched together for efficiency, since these
// can just be inserted into the document directly and still be
// correct.
//
// The regex is written so it will necessarily match any string
// completely if applied repeatedly. This is important because any
// part not matched would be silently dropped.
Matcher m = newLinePattern.matcher(str);

while (m.find()) {
String content = m.group(1);
String newlines = m.group(2);
String crs = m.group(3);

// Replace (or append if at end of the document) the content first
int replaceLength = Math.min(content.length(), document.getLength() - insertPosition);
document.replace(insertPosition, replaceLength, content, attributes);
insertPosition += content.length();

// Then insert any newlines, but always at the end of the document
// e.g. if insertPosition is halfway a line, do not delete
// anything, just add the newline(s) at the end).
if (newlines != null) {
document.insertString(document.getLength(), newlines, attributes);
insertPosition = document.getLength();
startOfLine = insertPosition;
}

// Then, for any CRs not followed by newlines, move insertPosition
// to the start of the line. Note that if a newline follows before
// any content in the next call to insertString, it will be added
// at the end of the document anyway, as expected.
if (crs != null) {
insertPosition = startOfLine;
}
}
}

public String getText() {
Expand Down
155 changes: 155 additions & 0 deletions app/test/processing/app/EditorConsoleTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
/*
* This file is part of Arduino.
*
* Copyright 2020 Arduino LLC (http://www.arduino.cc/)
*
* Arduino is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*
* As a special exception, you may use this file as part of a free software
* library without restriction. Specifically, if other files instantiate
* templates or use macros or inline functions from this file, or you compile
* this file and link it with other files to produce an executable, this
* file does not by itself cause the resulting executable to be covered by
* the GNU General Public License. This exception does not however
* invalidate any other reasons why the executable file might be covered by
* the GNU General Public License.
*/

package processing.app;

import static org.junit.Assert.assertEquals;

import org.junit.Before;
import org.junit.Test;

public class EditorConsoleTest extends AbstractWithPreferencesTest {
private EditorConsole console;

@Before
public void createConsole() {
console = new EditorConsole(null);
}

public String escapeString(String input) {
// This escapes backslashes, newlines and carriage returns, to get
// more readable assertion failures.
return input.replace("\\", "\\\\").replace("\n", "\\n").replace("\r", "\\r");
}

public void assertOutput(String output) {
assertEquals(escapeString(output), escapeString(console.getText()));
}

@Test
public void testHelloWorld() throws Exception {
console.insertString("Hello, world!", null);

assertOutput("Hello, world!");
}

@Test
public void testCrNlHandling() throws Exception {
// Do some basic tests with \r\n
console.insertString("abc\r\ndef", null);
assertOutput("abc\r\ndef");

console.insertString("xyz", null);
assertOutput("abc\r\ndefxyz");

console.insertString("000\r\n123", null);
assertOutput("abc\r\ndefxyz000\r\n123");

console.insertString("\r\n", null);
assertOutput("abc\r\ndefxyz000\r\n123\r\n");
}

@Test
public void testNlHandling() throws Exception {
// Basic tests, but with just \n
console.insertString("abc\ndef", null);
assertOutput("abc\ndef");

console.insertString("xyz", null);
assertOutput("abc\ndefxyz");

console.insertString("000\n123", null);
assertOutput("abc\ndefxyz000\n123");

console.insertString("\n", null);
assertOutput("abc\ndefxyz000\n123\n");
}

@Test
public void testCrHandling() throws Exception {
// Then test that single \r clears the current line
console.clear();
console.insertString("abc\rdef", null);
assertOutput("def");

// A single \r at the end is not added to the document
console.insertString("\r", null);
assertOutput("def");

// Nor are multiple \r at the end
console.insertString("\r\r\r", null);
assertOutput("def");

// But it does clear the line on the next write
console.insertString("123", null);
assertOutput("123");

// Same when combined with some data
console.insertString("\r456\r\r", null);
assertOutput("456");

console.insertString("000", null);
assertOutput("000");

// Then add a newline so preceding data is kept
console.insertString("\r\nxxx\r", null);
assertOutput("000\r\nxxx");

// But data after the newline is removed
console.insertString("yyy", null);
assertOutput("000\r\nyyy");

// When a \r\n is split across inserts, it becomes a lone \n
console.insertString("\r", null);
assertOutput("000\r\nyyy");
console.insertString("\n", null);
assertOutput("000\r\nyyy\n");
}

@Test
public void testCrPartialOverwrite() throws Exception {
console.insertString("abcdef\r", null);
assertOutput("abcdef");

console.insertString("123", null);
assertOutput("123def");

console.insertString("4", null);
assertOutput("1234ef");

console.insertString("\r\n56", null);
assertOutput("1234ef\r\n56");
}

@Test
public void testTogether() throws Exception {
console.insertString("abc\n123456\rdef\rx\r\nyyy\nzzz\r999", null);
assertOutput("abc\nxef456\r\nyyy\n999");
}
}

0 comments on commit 6d3ad18

Please sign in to comment.