ProjectExplorer: Let IOutputParser handle newlines

This makes IOutputParser structurally more similar to
Utils::OutputFormatter, which makes it simpler to explore possibilities
of somehow uniting these two related classes.

Task-number: QTCREATORBUG-22665
Change-Id: Ibb12ab6c8c785d863b9a921102a929864d0a5251
Reviewed-by: hjk <hjk@qt.io>

src/plugins/cmakeprojectmanager/cmakeprocess.cpp

@@ -201,7 +201,7 @@ void CMakeProcess::processStandardError()
 
     static QString rest;
     rest = lineSplit(rest, m_process->readAllStandardError(), [this](const QString &s) {
-        m_parser->stdError(s);
+        m_parser->handleStderr(s);
         Core::MessageManager::write(s);
     });
 }

src/plugins/projectexplorer/abstractprocessstep.cpp

@@ -45,6 +45,7 @@
 #include <QDir>
 #include <QHash>
 #include <QPair>
+#include <QTextDecoder>
 #include <QUrl>
 
 #include <algorithm>
@@ -108,15 +109,11 @@ public:
     std::unique_ptr<IOutputParser> m_outputParserChain;
     ProcessParameters m_param;
     Utils::FileInProjectFinder m_fileFinder;
-    QByteArray deferredText;
     bool m_ignoreReturnValue = false;
     bool m_skipFlush = false;
     bool m_lowPriority = false;
-
+    std::unique_ptr<QTextDecoder> stdoutStream;
-    void readData(void (AbstractProcessStep::*func)(const QString &), bool isUtf8 = false);
+    std::unique_ptr<QTextDecoder> stderrStream;
-    void processLine(const QByteArray &data,
-                     void (AbstractProcessStep::*func)(const QString &),
-                     bool isUtf8 = false);
 };
 
 AbstractProcessStep::AbstractProcessStep(BuildStepList *bsl, Core::Id id) :
@@ -224,6 +221,10 @@ void AbstractProcessStep::doRun()
         return;
     }
 
+    d->stdoutStream = std::make_unique<QTextDecoder>(buildEnvironment().hasKey("VSLANG")
+            ? QTextCodec::codecForName("UTF-8") : QTextCodec::codecForLocale());
+    d->stderrStream = std::make_unique<QTextDecoder>(QTextCodec::codecForLocale());
+
     d->m_process.reset(new Utils::QtcProcess());
     d->m_process->setUseCtrlCStub(Utils::HostOsInfo::isWindowsHost());
     d->m_process->setWorkingDirectory(wd.absolutePath());
@@ -347,40 +348,7 @@ void AbstractProcessStep::processReadyReadStdOutput()
 {
     if (!d->m_process)
         return;
-    d->m_process->setReadChannel(QProcess::StandardOutput);
+    stdOutput(d->stdoutStream->toUnicode(d->m_process->readAllStandardOutput()));
-    const bool utf8Output = buildEnvironment().hasKey("VSLANG");
-    d->readData(&AbstractProcessStep::stdOutput, utf8Output);
-}
-
-void AbstractProcessStep::Private::readData(void (AbstractProcessStep::*func)(const QString &),
-                                            bool isUtf8)
-{
-    while (m_process->bytesAvailable()) {
-        const bool hasLine = m_process->canReadLine();
-        const QByteArray data = hasLine ? m_process->readLine() : m_process->readAll();
-        int startPos = 0;
-        int crPos = -1;
-        while ((crPos = data.indexOf('\r', startPos)) >= 0)  {
-            if (data.size() > crPos + 1 && data.at(crPos + 1) == '\n')
-                break;
-            processLine(data.mid(startPos, crPos - startPos + 1), func, isUtf8);
-            startPos = crPos + 1;
-        }
-        if (hasLine)
-            processLine(data.mid(startPos), func, isUtf8);
-        else if (startPos < data.count())
-            deferredText += data.mid(startPos);
-    }
-}
-
-void AbstractProcessStep::Private::processLine(const QByteArray &data,
-                                               void (AbstractProcessStep::*func)(const QString &),
-                                               bool isUtf8)
-{
-    const QByteArray text = deferredText + data;
-    deferredText.clear();
-    const QString line = isUtf8 ? QString::fromUtf8(text) : QString::fromLocal8Bit(text);
-    (q->*func)(line);
 }
 
 /*!
@@ -389,19 +357,18 @@ void AbstractProcessStep::Private::processLine(const QByteArray &data,
     The default implementation adds the line to the application output window.
 */
 
-void AbstractProcessStep::stdOutput(const QString &line)
+void AbstractProcessStep::stdOutput(const QString &output)
 {
     if (d->m_outputParserChain)
-        d->m_outputParserChain->stdOutput(line);
+        d->m_outputParserChain->handleStdout(output);
-    emit addOutput(line, BuildStep::OutputFormat::Stdout, BuildStep::DontAppendNewline);
+    emit addOutput(output, BuildStep::OutputFormat::Stdout, BuildStep::DontAppendNewline);
 }
 
 void AbstractProcessStep::processReadyReadStdError()
 {
     if (!d->m_process)
         return;
-    d->m_process->setReadChannel(QProcess::StandardError);
+    stdError(d->stderrStream->toUnicode(d->m_process->readAllStandardError()));
-    d->readData(&AbstractProcessStep::stdError);
 }
 
 /*!
@@ -410,11 +377,11 @@ void AbstractProcessStep::processReadyReadStdError()
     The default implementation adds the line to the application output window.
 */
 
-void AbstractProcessStep::stdError(const QString &line)
+void AbstractProcessStep::stdError(const QString &output)
 {
     if (d->m_outputParserChain)
-        d->m_outputParserChain->stdError(line);
+        d->m_outputParserChain->handleStderr(output);
-    emit addOutput(line, BuildStep::OutputFormat::Stderr, BuildStep::DontAppendNewline);
+    emit addOutput(output, BuildStep::OutputFormat::Stderr, BuildStep::DontAppendNewline);
 }
 
 void AbstractProcessStep::finish(bool success)
@@ -432,7 +399,7 @@ void AbstractProcessStep::taskAdded(const Task &task, int linkedOutputLines, int
     // flush out any pending tasks before proceeding:
     if (!d->m_skipFlush && d->m_outputParserChain) {
         d->m_skipFlush = true;
-        d->m_outputParserChain->flush();
+        d->m_outputParserChain->flushTasks();
         d->m_skipFlush = false;
     }
 
@@ -463,15 +430,10 @@ void AbstractProcessStep::slotProcessFinished(int, QProcess::ExitStatus)
     QProcess *process = d->m_process.get();
     if (!process) // Happens when the process was canceled and handed over to the Reaper.
         process = qobject_cast<QProcess *>(sender()); // The process was canceled!
-
+    if (process) {
-    const QString stdErrLine = process ? QString::fromLocal8Bit(process->readAllStandardError()) : QString();
+        stdError(d->stderrStream->toUnicode(process->readAllStandardError()));
-    for (const QString &l : stdErrLine.split('\n'))
+        stdOutput(d->stdoutStream->toUnicode(process->readAllStandardOutput()));
-        stdError(l);
+    }
-
-    const QString stdOutLine = process ? QString::fromLocal8Bit(process->readAllStandardOutput()) : QString();
-    for (const QString &l : stdOutLine.split('\n'))
-        stdOutput(l);
-
     cleanUp(process);
 }
 

src/plugins/projectexplorer/abstractprocessstep.h

@@ -64,8 +64,8 @@ protected:
     virtual void processFinished(int exitCode, QProcess::ExitStatus status);
     virtual void processStartupFailed();
     virtual bool processSucceeded(int exitCode, QProcess::ExitStatus status);
-    virtual void stdOutput(const QString &line);
+    virtual void stdOutput(const QString &output);
-    virtual void stdError(const QString &line);
+    virtual void stdError(const QString &output);
 
     void doCancel() override;
 

src/plugins/projectexplorer/ioutputparser.cpp

@@ -26,6 +26,8 @@
 #include "ioutputparser.h"
 #include "task.h"
 
+#include <utils/synchronousprocess.h>
+
 /*!
     \class ProjectExplorer::IOutputParser
 
@@ -42,13 +44,6 @@
     ownership.
 */
 
-/*!
-    \fn IOutputParser *ProjectExplorer::IOutputParser::takeOutputParserChain()
-
-    Removes the appended outputparser chain from this parser, transferring
-           ownership of the parser chain to the caller.
-*/
-
 /*!
     \fn IOutputParser *ProjectExplorer::IOutputParser::childParser() const
 
@@ -89,18 +84,6 @@
    Should be emitted for each task seen in the output.
 */
 
-/*!
-   \fn void ProjectExplorer::IOutputParser::outputAdded(const QString &string, ProjectExplorer::BuildStep::OutputFormat format)
-
-   Subparsers have their addOutput signal connected to this slot.
-*/
-
-/*!
-   \fn void ProjectExplorer::IOutputParser::outputAdded(const QString &string, ProjectExplorer::BuildStep::OutputFormat format)
-
-   This function can be overwritten to change the string.
-*/
-
 /*!
    \fn void ProjectExplorer::IOutputParser::taskAdded(const ProjectExplorer::Task &task)
 
@@ -123,50 +106,116 @@
 
 namespace ProjectExplorer {
 
+class OutputChannelState
+{
+public:
+    using LineHandler = void (IOutputParser::*)(const QString &line);
+
+    OutputChannelState(IOutputParser *parser, LineHandler lineHandler)
+        : parser(parser), lineHandler(lineHandler) {}
+
+    void handleData(const QString &newData)
+    {
+        pendingData += newData;
+        pendingData = Utils::SynchronousProcess::normalizeNewlines(pendingData);
+        while (true) {
+            const int eolPos = pendingData.indexOf('\n');
+            if (eolPos == -1)
+                break;
+            const QString line = pendingData.left(eolPos + 1);
+            pendingData.remove(0, eolPos + 1);
+            (parser->*lineHandler)(line);
+        }
+    }
+
+    void flush()
+    {
+        if (!pendingData.isEmpty()) {
+            (parser->*lineHandler)(pendingData);
+            pendingData.clear();
+        }
+    }
+
+    IOutputParser * const parser;
+    LineHandler lineHandler;
+    QString pendingData;
+};
+
+class IOutputParser::IOutputParserPrivate
+{
+public:
+    IOutputParserPrivate(IOutputParser *parser)
+        : stdoutState(parser, &IOutputParser::stdOutput),
+          stderrState(parser, &IOutputParser::stdError)
+    {}
+
+    IOutputParser *childParser = nullptr;
+    Utils::FilePath workingDir;
+    OutputChannelState stdoutState;
+    OutputChannelState stderrState;
+};
+
+IOutputParser::IOutputParser() : d(new IOutputParserPrivate(this))
+{
+}
+
 IOutputParser::~IOutputParser()
 {
-    delete m_parser;
+    delete d->childParser;
+    delete d;
+}
+
+void IOutputParser::handleStdout(const QString &data)
+{
+    d->stdoutState.handleData(data);
+}
+
+void IOutputParser::handleStderr(const QString &data)
+{
+    d->stderrState.handleData(data);
 }
 
 void IOutputParser::appendOutputParser(IOutputParser *parser)
 {
     if (!parser)
         return;
-    if (m_parser) {
+    if (d->childParser) {
-        m_parser->appendOutputParser(parser);
+        d->childParser->appendOutputParser(parser);
         return;
     }
 
-    m_parser = parser;
+    d->childParser = parser;
     connect(parser, &IOutputParser::addTask, this, &IOutputParser::taskAdded);
 }
 
 IOutputParser *IOutputParser::childParser() const
 {
-    return m_parser;
+    return d->childParser;
 }
 
 void IOutputParser::setChildParser(IOutputParser *parser)
 {
-    if (m_parser != parser)
+    if (d->childParser != parser)
-        delete m_parser;
+        delete d->childParser;
-    m_parser = parser;
+    d->childParser = parser;
     if (parser)
         connect(parser, &IOutputParser::addTask, this, &IOutputParser::taskAdded);
 }
 
 void IOutputParser::stdOutput(const QString &line)
 {
-    if (m_parser)
+    if (d->childParser)
-        m_parser->stdOutput(line);
+        d->childParser->stdOutput(line);
 }
 
 void IOutputParser::stdError(const QString &line)
 {
-    if (m_parser)
+    if (d->childParser)
-        m_parser->stdError(line);
+        d->childParser->stdError(line);
 }
 
+Utils::FilePath IOutputParser::workingDirectory() const { return d->workingDir; }
+
 void IOutputParser::taskAdded(const Task &task, int linkedOutputLines, int skipLines)
 {
     emit addTask(task, linkedOutputLines, skipLines);
@@ -177,21 +226,29 @@ void IOutputParser::doFlush()
 
 bool IOutputParser::hasFatalErrors() const
 {
-    return m_parser && m_parser->hasFatalErrors();
+    return d->childParser && d->childParser->hasFatalErrors();
 }
 
 void IOutputParser::setWorkingDirectory(const Utils::FilePath &fn)
 {
-    m_workingDir = fn;
+    d->workingDir = fn;
-    if (m_parser)
+    if (d->childParser)
-        m_parser->setWorkingDirectory(fn);
+        d->childParser->setWorkingDirectory(fn);
 }
 
 void IOutputParser::flush()
+{
+    flushTasks();
+    d->stdoutState.flush();
+    d->stderrState.flush();
+    flushTasks();
+}
+
+void IOutputParser::flushTasks()
 {
     doFlush();
-    if (m_parser)
+    if (d->childParser)
-        m_parser->flush();
+        d->childParser->flushTasks();
 }
 
 QString IOutputParser::rightTrimmed(const QString &in)

src/plugins/projectexplorer/ioutputparser.h

@@ -38,22 +38,23 @@ class PROJECTEXPLORER_EXPORT IOutputParser : public QObject
 {
     Q_OBJECT
 public:
-    IOutputParser() = default;
+    IOutputParser();
     ~IOutputParser() override;
 
+    void handleStdout(const QString &data);
+    void handleStderr(const QString &data);
+
     void appendOutputParser(IOutputParser *parser);
 
     IOutputParser *childParser() const;
     void setChildParser(IOutputParser *parser);
 
-    virtual void stdOutput(const QString &line);
-    virtual void stdError(const QString &line);
-
     virtual bool hasFatalErrors() const;
 
     void setWorkingDirectory(const Utils::FilePath &fn);
 
-    void flush(); // flush out pending tasks
+    void flush(); // flush pending tasks & output
+    void flushTasks(); // flush pending tasks only
 
     static QString rightTrimmed(const QString &in);
 
@@ -63,13 +64,16 @@ signals:
     void addTask(const ProjectExplorer::Task &task, int linkedOutputLines = 0, int skipLines = 0);
 
 protected:
-    Utils::FilePath workingDirectory() const { return m_workingDir; }
+    virtual void stdOutput(const QString &line);
+    virtual void stdError(const QString &line);
+
+    Utils::FilePath workingDirectory() const;
 
 private:
     virtual void doFlush();
 
-    IOutputParser *m_parser = nullptr;
+    class IOutputParserPrivate;
-    Utils::FilePath m_workingDir;
+    IOutputParserPrivate * const d;
 };
 
 } // namespace ProjectExplorer

src/plugins/projectexplorer/outputparser_test.cpp

@@ -53,14 +53,11 @@ void OutputParserTester::testParsing(const QString &lines,
     reset();
     Q_ASSERT(childParser());
 
-    const QStringList inputLines = lines.split('\n');
+    if (inputChannel == STDOUT)
-    for (const QString &input : inputLines) {
+        handleStdout(lines + '\n');
-        if (inputChannel == STDOUT)
+    else
-            childParser()->stdOutput(input + '\n');
+        handleStderr(lines + '\n');
-        else
+    flush();
-            childParser()->stdError(input + '\n');
-    }
-    childParser()->flush();
 
     // delete the parser(s) to test
     emit aboutToDeleteParser();

src/plugins/projectexplorer/parseissuesdialog.cpp

@@ -140,9 +140,9 @@ static void parse(QFutureInterface<void> &future, const QString &output,
 {
     const QStringList lines = output.split('\n');
     future.setProgressRange(0, lines.count());
-    const auto parserFunc = isStderr ? &IOutputParser::stdError : &IOutputParser::stdOutput;
+    const auto parserFunc = isStderr ? &IOutputParser::handleStderr : &IOutputParser::handleStdout;
     for (const QString &line : lines) {
-        (parser.get()->*parserFunc)(line);
+        (parser.get()->*parserFunc)(line + '\n');
         future.setProgressValue(future.progressValue() + 1);
         if (future.isCanceled())
             return;

src/plugins/qbsprojectmanager/qbsbuildstep.cpp

@@ -383,11 +383,11 @@ void QbsBuildStep::handleProcessResult(
     emit addOutput(executable.toUserOutput() + ' '  + QtcProcess::joinArgs(arguments),
                    OutputFormat::Stdout);
     for (const QString &line : stdErr) {
-        m_parser->stdError(line);
+        m_parser->handleStderr(line + '\n');
         emit addOutput(line, OutputFormat::Stderr);
     }
     for (const QString &line : stdOut) {
-        m_parser->stdOutput(line);
+        m_parser->handleStdout(line + '\n');
         emit addOutput(line, OutputFormat::Stdout);
     }
     m_parser->flush();