OutputFormatter: Do the newline handling centrally

All output formatters are line-based, and they all did their own line
splitting and, if they didn't entirely ignore it, handling of partial
lines.
Instead, we now do all the book-keeping in the base class, and the
subclasses always work with complete lines.

Change-Id: I0b0df7951d0e4f6601f4d912230071784c87b3d3
Reviewed-by: hjk <hjk@qt.io>

src/libs/utils/outputformatter.cpp

@@ -28,6 +28,7 @@
 #include "synchronousprocess.h"
 #include "theme/theme.h"
 
+#include <QPair>
 #include <QPlainTextEdit>
 #include <QTextCursor>
 
@@ -42,6 +43,7 @@ public:
     QTextCharFormat formats[NumberOfFormats];
     QTextCursor cursor;
     AnsiEscapeCodeHandler escapeCodeHandler;
+    QPair<QString, OutputFormat> incompleteLine;
     bool boldFontEnabled = true;
     bool prependCarriageReturn = false;
 };
@@ -118,6 +120,9 @@ QTextCharFormat OutputFormatter::linkFormat(const QTextCharFormat &inputFormat,
 
 void OutputFormatter::clearLastLine()
 {
+    // Note that this approach will fail if the text edit is not read-only and users
+    // have messed with the last line between programmatic inputs.
+    // We live with this risk, as all the alternatives are worse.
     if (!d->cursor.atEnd())
         d->cursor.movePosition(QTextCursor::End);
     d->cursor.movePosition(QTextCursor::StartOfBlock, QTextCursor::KeepAnchor);
@@ -139,6 +144,20 @@ void OutputFormatter::initFormats()
     setBoldFontEnabled(d->boldFontEnabled);
 }
 
+void OutputFormatter::flushIncompleteLine()
+{
+    clearLastLine();
+    doAppendMessage(d->incompleteLine.first, d->incompleteLine.second);
+    d->incompleteLine.first.clear();
+}
+
+void OutputFormatter::dumpIncompleteLine(const QString &line, OutputFormat format)
+{
+    append(line, charFormat(format));
+    d->incompleteLine.first.append(line);
+    d->incompleteLine.second = format;
+}
+
 void OutputFormatter::handleLink(const QString &href)
 {
     Q_UNUSED(href)
@@ -147,7 +166,9 @@ void OutputFormatter::handleLink(const QString &href)
 void OutputFormatter::clear()
 {
     d->prependCarriageReturn = false;
+    d->incompleteLine.first.clear();
     plainTextEdit()->clear();
+    reset();
 }
 
 void OutputFormatter::setBoldFontEnabled(bool enabled)
@@ -160,11 +181,20 @@ void OutputFormatter::setBoldFontEnabled(bool enabled)
 
 void OutputFormatter::flush()
 {
+    if (!d->incompleteLine.first.isEmpty())
+        flushIncompleteLine();
     d->escapeCodeHandler.endFormatScope();
+    reset();
 }
 
 void OutputFormatter::appendMessage(const QString &text, OutputFormat format)
 {
+    // If we have an existing incomplete line and its format is different from this one,
+    // then we consider the two messages unrelated. We re-insert the previous incomplete line,
+    // possibly formatted now, and start from scratch with the new input.
+    if (!d->incompleteLine.first.isEmpty() && d->incompleteLine.second != format)
+        flushIncompleteLine();
+
     QString out = text;
     if (d->prependCarriageReturn) {
         d->prependCarriageReturn = false;
@@ -175,7 +205,35 @@ void OutputFormatter::appendMessage(const QString &text, OutputFormat format)
         d->prependCarriageReturn = true;
         out.chop(1);
     }
-    doAppendMessage(out, format);
+
+    // If the input is a single incomplete line, we do not forward it to the specialized
+    // formatting code, but simply dump it as-is. Once it becomes complete or it needs to
+    // be flushed for other reasons, we remove the unformatted part and re-insert it, this
+    // time with proper formatting.
+    if (!out.contains('\n')) {
+        dumpIncompleteLine(out, format);
+        return;
+    }
+
+    // We have at least one complete line, so let's remove the previously dumped
+    // incomplete line and prepend it to the first line of our new input.
+    if (!d->incompleteLine.first.isEmpty()) {
+        clearLastLine();
+        out.prepend(d->incompleteLine.first);
+        d->incompleteLine.first.clear();
+    }
+
+    // Forward all complete lines to the specialized formatting code, and handle a
+    // potential trailing incomplete line the same way as above.
+    for (int startPos = 0; ;) {
+        const int eolPos = out.indexOf('\n', startPos);
+        if (eolPos == -1) {
+            dumpIncompleteLine(out.mid(startPos), format);
+            break;
+        }
+        doAppendMessage(out.mid(startPos, eolPos - startPos + 1), format);
+        startPos = eolPos + 1;
+    }
 }
 
 } // namespace Utils

src/libs/utils/outputformatter.h

@@ -61,16 +61,24 @@ public:
     static QTextCharFormat linkFormat(const QTextCharFormat &inputFormat, const QString &href);
 
 protected:
+    // text contains at most one line feed character, and if it does occur, it's the last character.
+    // Either way, the input is to be considered "complete" for formatting purposes.
+    virtual void doAppendMessage(const QString &text, OutputFormat format);
+
     virtual void clearLastLine();
+
     QTextCharFormat charFormat(OutputFormat format) const;
     QList<FormattedText> parseAnsi(const QString &text, const QTextCharFormat &format);
-    virtual void doAppendMessage(const QString &text, OutputFormat format);
     QTextCursor &cursor() const;
 
 private:
-    virtual void doAppendMessage(const QString &text, const QTextCharFormat &format);
+    void doAppendMessage(const QString &text, const QTextCharFormat &format);
+    virtual void reset() {}
+
     void append(const QString &text, const QTextCharFormat &format);
     void initFormats();
+    void flushIncompleteLine();
+    void dumpIncompleteLine(const QString &line, OutputFormat format);
 
     Internal::OutputFormatterPrivate *d;
 };

src/plugins/python/pythonrunconfiguration.cpp

@@ -73,50 +73,46 @@ public:
 private:
     void doAppendMessage(const QString &text, OutputFormat format) final
     {
-        const bool isTrace = format == StdErrFormat
+        if (!m_inTraceBack) {
-                          && (text.startsWith("Traceback (most recent call last):")
+            m_inTraceBack = format == StdErrFormat
-                              || text.startsWith("\nTraceback (most recent call last):"));
+                    && text.startsWith("Traceback (most recent call last):");
-
-        if (!isTrace) {
             OutputFormatter::doAppendMessage(text, format);
+        }
+
+        const Core::Id category(PythonErrorTaskCategory);
+        const QRegularExpressionMatch match = filePattern.match(text);
+        if (match.hasMatch()) {
+            QTextCursor tc = plainTextEdit()->textCursor();
+            tc.movePosition(QTextCursor::End, QTextCursor::MoveAnchor);
+            tc.insertText(match.captured(1));
+            tc.insertText(match.captured(2), linkFormat(charFormat(format), match.captured(2)));
+
+            const auto fileName = FilePath::fromString(match.captured(3));
+            const int lineNumber = match.capturedRef(4).toInt();
+            m_tasks.append({Task::Warning, QString(), fileName, lineNumber, category});
             return;
         }
 
-        const QTextCharFormat frm = charFormat(format);
+        if (text.startsWith(' ')) {
-        const Core::Id id(PythonErrorTaskCategory);
+            // Neither traceback start, nor file, nor error message line.
-        QVector<Task> tasks;
+            // Not sure if that can actually happen.
-        const QStringList lines = text.split('\n');
+            if (m_tasks.isEmpty()) {
-        unsigned taskId = unsigned(lines.size());
+                m_tasks.append({Task::Warning, text.trimmed(), {}, -1, category});
-
-        for (const QString &line : lines) {
-            const QRegularExpressionMatch match = filePattern.match(line);
-            if (match.hasMatch()) {
-                QTextCursor tc = plainTextEdit()->textCursor();
-                tc.movePosition(QTextCursor::End, QTextCursor::MoveAnchor);
-                tc.insertText('\n' + match.captured(1));
-                tc.insertText(match.captured(2), linkFormat(frm, match.captured(2)));
-
-                const auto fileName = FilePath::fromString(match.captured(3));
-                const int lineNumber = match.capturedRef(4).toInt();
-                Task task(Task::Warning,
-                                           QString(), fileName, lineNumber, id);
-                task.taskId = --taskId;
-                tasks.append(task);
             } else {
-                if (!tasks.isEmpty()) {
+                Task &task = m_tasks.back();
-                    Task &task = tasks.back();
+                if (!task.description.isEmpty())
-                    if (!task.description.isEmpty())
+                    task.description += ' ';
-                        task.description += ' ';
+                task.description += text.trimmed();
-                    task.description += line.trimmed();
-                }
-                OutputFormatter::doAppendMessage('\n' + line, format);
             }
-        }
+        } else {
-        if (!tasks.isEmpty()) {
+            // The actual exception. This ends the traceback.
-            tasks.back().type = Task::Error;
+            TaskHub::addTask({Task::Error, text, {}, -1, category});
-            for (auto rit = tasks.crbegin(), rend = tasks.crend(); rit != rend; ++rit)
+            for (auto rit = m_tasks.crbegin(), rend = m_tasks.crend(); rit != rend; ++rit)
                 TaskHub::addTask(*rit);
+            m_tasks.clear();
+            m_inTraceBack = false;
         }
+        OutputFormatter::doAppendMessage(text, format);
     }
 
     void handleLink(const QString &href) final
@@ -129,7 +125,15 @@ private:
         Core::EditorManager::openEditorAt(fileName, lineNumber);
     }
 
+    void reset() override
+    {
+        m_inTraceBack = false;
+        m_tasks.clear();
+    }
+
     const QRegularExpression filePattern;
+    QList<Task> m_tasks;
+    bool m_inTraceBack;
 };
 
 ////////////////////////////////////////////////////////////////

src/plugins/qtsupport/qtoutputformatter.cpp

@@ -81,7 +81,6 @@ public:
     const QRegularExpression qtTestFailUnix;
     const QRegularExpression qtTestFailWin;
     QPointer<Project> project;
-    QList<FormattedText> lastLine;
     FileInProjectFinder projectFinder;
     QTextCursor cursor;
 };
@@ -92,20 +91,18 @@ public:
     explicit QtOutputFormatter(Target *target);
     ~QtOutputFormatter() override;
 
-    void doAppendMessage(const QString &text, Utils::OutputFormat format) override;
-    void handleLink(const QString &href) override;
-
 protected:
-    void clearLastLine() override;
     virtual void openEditor(const QString &fileName, int line, int column = -1);
 
 private:
+    void doAppendMessage(const QString &text, Utils::OutputFormat format) override;
+    void handleLink(const QString &href) override;
+
     void updateProjectFileList();
     LinkResult matchLine(const QString &line) const;
     void appendMessagePart(const QString &txt, const QTextCharFormat &fmt);
-    void appendLine(const LinkResult &lr, const QString &line, Utils::OutputFormat format);
     void appendLine(const LinkResult &lr, const QString &line, const QTextCharFormat &format);
-    void doAppendMessage(const QString &text, const QTextCharFormat &format) override;
+    void doAppendMessage(const QString &txt, const QTextCharFormat &format);
 
     QtOutputFormatterPrivate *d;
     friend class QtSupportPlugin; // for testing
@@ -163,6 +160,13 @@ LinkResult QtOutputFormatter::matchLine(const QString &line) const
     return lr;
 }
 
+void QtOutputFormatter::doAppendMessage(const QString &txt, const QTextCharFormat &format)
+{
+    const QList<FormattedText> ansiTextList = parseAnsi(txt, format);
+    for (const FormattedText &output : ansiTextList)
+        appendMessagePart(output.text, output.format);
+}
+
 void QtOutputFormatter::doAppendMessage(const QString &txt, OutputFormat format)
 {
     doAppendMessage(txt, charFormat(format));
@@ -170,61 +174,11 @@ void QtOutputFormatter::doAppendMessage(const QString &txt, OutputFormat format)
 
 void QtOutputFormatter::appendMessagePart(const QString &txt, const QTextCharFormat &fmt)
 {
-    QString deferredText;
+    const LinkResult lr = matchLine(txt);
-
+    if (!lr.href.isEmpty())
-    const int length = txt.length();
+        appendLine(lr, txt, fmt);
-    for (int start = 0, pos = -1; start < length; start = pos + 1) {
+    else
-        bool linkHandled = false;
+        cursor().insertText(txt, fmt);
-        pos = txt.indexOf('\n', start);
-        const QString newPart = txt.mid(start, (pos == -1) ? -1 : pos - start + 1);
-        QString line = newPart;
-        QTextCharFormat format = fmt;
-        if (!d->lastLine.isEmpty()) {
-            line = d->lastLine.last().text + newPart;
-            format = d->lastLine.last().format;
-        }
-
-        LinkResult lr = matchLine(line);
-        if (!lr.href.isEmpty()) {
-            // Found something && line continuation
-            cursor().insertText(deferredText, fmt);
-            deferredText.clear();
-            if (!d->lastLine.isEmpty())
-                clearLastLine();
-            appendLine(lr, line, format);
-            linkHandled = true;
-        } else {
-            // Found nothing, just emit the new part
-            deferredText += newPart;
-        }
-
-        if (pos == -1) {
-            d->lastLine.clear();
-            if (!linkHandled)
-                d->lastLine.append(FormattedText(line, format));
-            break;
-        }
-        d->lastLine.clear(); // Handled line continuation
-    }
-    cursor().insertText(deferredText, fmt);
-}
-
-void QtOutputFormatter::doAppendMessage(const QString &txt, const QTextCharFormat &format)
-{
-    if (!cursor().atEnd())
-        cursor().movePosition(QTextCursor::End);
-    cursor().beginEditBlock();
-
-    const QList<FormattedText> ansiTextList = parseAnsi(txt, format);
-    for (const FormattedText &output : ansiTextList)
-        appendMessagePart(output.text, output.format);
-
-    cursor().endEditBlock();
-}
-
-void QtOutputFormatter::appendLine(const LinkResult &lr, const QString &line, OutputFormat format)
-{
-    appendLine(lr, line, charFormat(format));
 }
 
 void QtOutputFormatter::appendLine(const LinkResult &lr, const QString &line,
@@ -301,13 +255,6 @@ void QtOutputFormatter::handleLink(const QString &href)
     }
 }
 
-void QtOutputFormatter::clearLastLine()
-{
-    OutputFormatter::clearLastLine();
-    if (!d->lastLine.isEmpty())
-        d->lastLine.removeLast();
-}
-
 void QtOutputFormatter::openEditor(const QString &fileName, int line, int column)
 {
     Core::EditorManager::openEditorAt(fileName, line, column);