Handle CR without NL printed in EditorConsole

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,20 +39,21 @@ public class EditorConsole extends JScrollPane {
 
   private static ConsoleOutputStream out;
   private static ConsoleOutputStream err;
+  private int startOfLine = 0;
+  private int insertPosition = 0;
 
-  private static synchronized void init(SimpleAttributeSet outStyle, PrintStream outStream, SimpleAttributeSet errStyle, PrintStream errStream) {
-    if (out != null) {
-      return;
-    }
+  // Regex for linesplitting, see insertString for comments.
+  private static final Pattern newLinePattern = Pattern.compile("([^\r\n]*)([\r\n]*\n)?(\r+)?");
 
-    out = new ConsoleOutputStream(outStyle, outStream);
-    System.setOut(new PrintStream(out, true));
+  public static synchronized void setCurrentEditorConsole(EditorConsole console) {
+    if (out == null) {
+      out = new ConsoleOutputStream(console.stdOutStyle, System.out);
+      System.setOut(new PrintStream(out, true));
 
-    err = new ConsoleOutputStream(errStyle, errStream);
-    System.setErr(new PrintStream(err, true));
-  }
+      err = new ConsoleOutputStream(console.stdErrStyle, System.err);
+      System.setErr(new PrintStream(err, true));
+    }
 
-  public static void setCurrentEditorConsole(EditorConsole console) {
     out.setCurrentEditorConsole(console);
     err.setCurrentEditorConsole(console);
   }
@@ -109,10 +112,9 @@ public EditorConsole(Base base) {
     setPreferredSize(new Dimension(100, (height * lines)));
     setMinimumSize(new Dimension(100, (height * lines)));
 
-    EditorConsole.init(stdOutStyle, System.out, stdErrStyle, System.err);
-
     // Add font size adjustment listeners.
-    base.addEditorFontResizeListeners(consoleTextPane);
+    if (base != null)
+      base.addEditorFontResizeListeners(consoleTextPane);
   }
 
   public void applyPreferences() {
@@ -130,8 +132,10 @@ public void applyPreferences() {
     // Re-insert console text with the new preferences if there were changes.
     // This assumes that the document has single-child paragraphs (default).
     if (!stdOutStyle.isEqual(stdOutStyleOld) || !stdErrStyle.isEqual(stdOutStyleOld)) {
-      out.setAttibutes(stdOutStyle);
-      err.setAttibutes(stdErrStyle);
+      if (out != null)
+        out.setAttibutes(stdOutStyle);
+      if (err != null)
+        err.setAttibutes(stdErrStyle);
 
       int start;
       for (int end = document.getLength() - 1; end >= 0; end = start - 1) {
@@ -164,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?
@@ -179,14 +185,53 @@ 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() {
-    return consoleTextPane.getText().trim();
+    return consoleTextPane.getText();
   }
 
 }

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");
+  }
+}