From 65af79af8405e1ce0f7cb5bed272b143d615602f Mon Sep 17 00:00:00 2001
From: Eike Ziller <eike.ziller@qt.io>
Date: Wed, 20 Dec 2023 11:59:54 +0100
Subject: [PATCH] ExternalTool: Fix output handling for General Messages

The handling of lines was wrong and only happened to work if the output
was delivered in one batch. Otherwise it would add line breaks - and if
the tool output was delivered not in batches of lines, that could break
in the middle of output lines.

Change to code to use the Process API for handling lines of output.
That also makes the codec handling in ExternalTool unnecessary. The
corresponding members can be removed in the next binary incompatible
release.

Fixes: QTCREATORBUG-29977
Change-Id: I9d37e5e374a43f34a28418272692cca17599c8b4
Reviewed-by: Jarek Kobus <jaroslaw.kobus@qt.io>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
---
 src/plugins/coreplugin/externaltool.cpp | 29 +++++++++++--------------
 src/plugins/coreplugin/externaltool.h   |  5 +++--
 2 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/src/plugins/coreplugin/externaltool.cpp b/src/plugins/coreplugin/externaltool.cpp
index c65cfe80882..7328e024dba 100644
--- a/src/plugins/coreplugin/externaltool.cpp
+++ b/src/plugins/coreplugin/externaltool.cpp
@@ -623,10 +623,8 @@ void ExternalToolRunner::run()
     }
     m_process = new Process(this);
     connect(m_process, &Process::done, this, &ExternalToolRunner::done);
-    connect(m_process, &Process::readyReadStandardOutput,
-            this, &ExternalToolRunner::readStandardOutput);
-    connect(m_process, &Process::readyReadStandardError,
-            this, &ExternalToolRunner::readStandardError);
+    m_process->setStdOutLineCallback([this](const QString &s) { readStandardOutput(s); });
+    m_process->setStdErrLineCallback([this](const QString &s) { readStandardError(s); });
     if (!m_resolvedWorkingDirectory.isEmpty())
         m_process->setWorkingDirectory(m_resolvedWorkingDirectory);
     const CommandLine cmd{m_resolvedExecutable, m_resolvedArguments, CommandLine::Raw};
@@ -665,30 +663,29 @@ void ExternalToolRunner::done()
     deleteLater();
 }
 
-void ExternalToolRunner::readStandardOutput()
+static QString stripNewline(const QString &output)
+{
+    if (output.endsWith('\n'))
+        return output.chopped(1);
+    return output;
+}
+
+void ExternalToolRunner::readStandardOutput(const QString &output)
 {
     if (m_tool->outputHandling() == ExternalTool::Ignore)
         return;
-    const QByteArray data = m_process->readAllRawStandardOutput();
-    const QString output = m_outputCodec->toUnicode(data.constData(),
-                                                    data.length(),
-                                                    &m_outputCodecState);
     if (m_tool->outputHandling() == ExternalTool::ShowInPane)
-        MessageManager::writeSilently(output);
+        MessageManager::writeSilently(stripNewline(output));
     else if (m_tool->outputHandling() == ExternalTool::ReplaceSelection)
         m_processOutput.append(output);
 }
 
-void ExternalToolRunner::readStandardError()
+void ExternalToolRunner::readStandardError(const QString &output)
 {
     if (m_tool->errorHandling() == ExternalTool::Ignore)
         return;
-    const QByteArray data = m_process->readAllRawStandardError();
-    const QString output = m_outputCodec->toUnicode(data.constData(),
-                                                    data.length(),
-                                                    &m_errorCodecState);
     if (m_tool->errorHandling() == ExternalTool::ShowInPane)
-        MessageManager::writeSilently(output);
+        MessageManager::writeSilently(stripNewline(output));
     else if (m_tool->errorHandling() == ExternalTool::ReplaceSelection)
         m_processOutput.append(output);
 }
diff --git a/src/plugins/coreplugin/externaltool.h b/src/plugins/coreplugin/externaltool.h
index 15b420f1808..2d26291fc1d 100644
--- a/src/plugins/coreplugin/externaltool.h
+++ b/src/plugins/coreplugin/externaltool.h
@@ -115,8 +115,8 @@ public:
 
 private:
     void done();
-    void readStandardOutput();
-    void readStandardError();
+    void readStandardOutput(const QString &output);
+    void readStandardError(const QString &output);
 
     void run();
     bool resolve();
@@ -128,6 +128,7 @@ private:
     Utils::FilePath m_resolvedWorkingDirectory;
     Utils::Environment m_resolvedEnvironment;
     Utils::Process *m_process;
+    // TODO remove codec handling, that is done by Process now
     QTextCodec *m_outputCodec;
     QTextCodec::ConverterState m_outputCodecState;
     QTextCodec::ConverterState m_errorCodecState;