From 6d3ad183abbb597c7f5c7e67f26dcf97df03530c Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Sat, 28 Mar 2020 15:00:31 +0100 Subject: [PATCH] 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). --- app/src/processing/app/EditorConsole.java | 56 ++++++- .../processing/app/EditorConsoleTest.java | 155 ++++++++++++++++++ 2 files changed, 207 insertions(+), 4 deletions(-) create mode 100644 app/test/processing/app/EditorConsoleTest.java diff --git a/app/src/processing/app/EditorConsole.java b/app/src/processing/app/EditorConsole.java index 3f8a59f3b2a..3942908a1da 100644 --- a/app/src/processing/app/EditorConsole.java +++ b/app/src/processing/app/EditorConsole.java @@ -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; @@ -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) { @@ -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? @@ -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() { diff --git a/app/test/processing/app/EditorConsoleTest.java b/app/test/processing/app/EditorConsoleTest.java new file mode 100644 index 00000000000..308523ce6ef --- /dev/null +++ b/app/test/processing/app/EditorConsoleTest.java @@ -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"); + } +}