From cddaecfe216bccc970c225cc83b532c2c420cae8 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Tue, 31 Mar 2020 11:55:48 +0200 Subject: ProjectExplorer: Clean up IOutputParser interface - Remove unneeded/unused functions. - De-virtualize where possible. In particular, after untangling a number of self-referential redirections, it became apparent that the outputAdded() infrastructure was entirely unused. Change-Id: I51e1beed008df2727b42494b087efa476342397e Reviewed-by: hjk --- .../cmakeprojectmanager/servermodereader.cpp | 2 - .../projectexplorer/abstractprocessstep.cpp | 1 - src/plugins/projectexplorer/customparser.cpp | 12 ++---- src/plugins/projectexplorer/customparser.h | 4 -- src/plugins/projectexplorer/gnumakeparser.cpp | 21 +++++----- src/plugins/projectexplorer/gnumakeparser.h | 2 - src/plugins/projectexplorer/ioutputparser.cpp | 37 +++-------------- src/plugins/projectexplorer/ioutputparser.h | 17 ++++---- src/plugins/projectexplorer/makestep.cpp | 2 +- src/plugins/projectexplorer/outputparser_test.cpp | 46 +++------------------- src/plugins/projectexplorer/outputparser_test.h | 8 +--- src/plugins/qbsprojectmanager/qbsbuildstep.cpp | 6 +-- src/plugins/qbsprojectmanager/qbsparser.cpp | 15 ++----- src/plugins/qbsprojectmanager/qbsparser.h | 5 --- src/plugins/qmakeprojectmanager/qmakestep.cpp | 2 +- 15 files changed, 39 insertions(+), 141 deletions(-) (limited to 'src/plugins') diff --git a/src/plugins/cmakeprojectmanager/servermodereader.cpp b/src/plugins/cmakeprojectmanager/servermodereader.cpp index 7445d75dde..ae826895f1 100644 --- a/src/plugins/cmakeprojectmanager/servermodereader.cpp +++ b/src/plugins/cmakeprojectmanager/servermodereader.cpp @@ -73,8 +73,6 @@ const int MAX_PROGRESS = 1400; ServerModeReader::ServerModeReader() { - connect(&m_parser, &CMakeParser::addOutput, - this, [](const QString &m) { Core::MessageManager::write(m); }); connect(&m_parser, &CMakeParser::addTask, this, [this](const Task &t) { Task editable(t); if (!editable.file.isEmpty()) { diff --git a/src/plugins/projectexplorer/abstractprocessstep.cpp b/src/plugins/projectexplorer/abstractprocessstep.cpp index d88e8dea7d..c4d8ae8dc4 100644 --- a/src/plugins/projectexplorer/abstractprocessstep.cpp +++ b/src/plugins/projectexplorer/abstractprocessstep.cpp @@ -142,7 +142,6 @@ void AbstractProcessStep::setOutputParser(IOutputParser *parser) d->m_outputParserChain.reset(new AnsiFilterParser); d->m_outputParserChain->appendOutputParser(parser); - connect(d->m_outputParserChain.get(), &IOutputParser::addOutput, this, &AbstractProcessStep::outputAdded); connect(d->m_outputParserChain.get(), &IOutputParser::addTask, this, &AbstractProcessStep::taskAdded); } diff --git a/src/plugins/projectexplorer/customparser.cpp b/src/plugins/projectexplorer/customparser.cpp index afb73b4cbc..22ad09c307 100644 --- a/src/plugins/projectexplorer/customparser.cpp +++ b/src/plugins/projectexplorer/customparser.cpp @@ -134,11 +134,6 @@ void CustomParser::stdOutput(const QString &line) IOutputParser::stdOutput(line); } -void CustomParser::setWorkingDirectory(const QString &workingDirectory) -{ - m_workingDirectory = workingDirectory; -} - void CustomParser::setSettings(const CustomParserSettings &settings) { m_error = settings.error; @@ -152,10 +147,9 @@ Core::Id CustomParser::id() FilePath CustomParser::absoluteFilePath(const QString &filePath) const { - if (m_workingDirectory.isEmpty()) + if (workingDirectory().isEmpty()) return FilePath::fromUserInput(filePath); - - return FilePath::fromString(m_workingDirectory).resolvePath(filePath); + return workingDirectory().resolvePath(filePath); } bool CustomParser::hasMatch(const QString &line, CustomParserExpression::CustomParserChannel channel, @@ -481,7 +475,7 @@ void ProjectExplorerPlugin::testCustomOutputParsers() CustomParser *parser = new CustomParser; parser->setSettings(settings); - parser->setWorkingDirectory(workDir); + parser->setWorkingDirectory(FilePath::fromString(workDir)); OutputParserTester testbench; testbench.appendOutputParser(parser); diff --git a/src/plugins/projectexplorer/customparser.h b/src/plugins/projectexplorer/customparser.h index dafb0e8da1..6435bc13fa 100644 --- a/src/plugins/projectexplorer/customparser.h +++ b/src/plugins/projectexplorer/customparser.h @@ -89,8 +89,6 @@ public: void stdError(const QString &line) override; void stdOutput(const QString &line) override; - void setWorkingDirectory(const QString &workingDirectory) override; - void setSettings(const CustomParserSettings &settings); static Core::Id id(); @@ -103,8 +101,6 @@ private: CustomParserExpression m_error; CustomParserExpression m_warning; - - QString m_workingDirectory; }; } // namespace ProjectExplorer diff --git a/src/plugins/projectexplorer/gnumakeparser.cpp b/src/plugins/projectexplorer/gnumakeparser.cpp index 506f0cfe2d..c145344531 100644 --- a/src/plugins/projectexplorer/gnumakeparser.cpp +++ b/src/plugins/projectexplorer/gnumakeparser.cpp @@ -56,12 +56,6 @@ GnuMakeParser::GnuMakeParser() QTC_CHECK(m_errorInMakefile.isValid()); } -void GnuMakeParser::setWorkingDirectory(const QString &workingDirectory) -{ - addDirectory(workingDirectory); - IOutputParser::setWorkingDirectory(workingDirectory); -} - bool GnuMakeParser::hasFatalErrors() const { return (m_fatalErrorCount > 0) || IOutputParser::hasFatalErrors(); @@ -180,7 +174,7 @@ void GnuMakeParser::taskAdded(const Task &task, int linkedLines, int skippedLine if (!filePath.isEmpty() && !QDir::isAbsolutePath(filePath)) { QFileInfoList possibleFiles; - foreach (const QString &dir, m_directories) { + foreach (const QString &dir, searchDirectories()) { QFileInfo candidate(dir + QLatin1Char('/') + filePath); if (candidate.exists() && !possibleFiles.contains(candidate)) { @@ -197,6 +191,14 @@ void GnuMakeParser::taskAdded(const Task &task, int linkedLines, int skippedLine IOutputParser::taskAdded(editable, linkedLines, skippedLines); } +QStringList GnuMakeParser::searchDirectories() const +{ + QStringList dirs = m_directories; + if (!workingDirectory().isEmpty()) + dirs << workingDirectory().toString(); + return dirs; +} + } // ProjectExplorer #ifdef WITH_TESTS @@ -210,11 +212,6 @@ void GnuMakeParser::taskAdded(const Task &task, int linkedLines, int skippedLine namespace ProjectExplorer { -QStringList GnuMakeParser::searchDirectories() const -{ - return m_directories; -} - GnuMakeParserTester::GnuMakeParserTester(GnuMakeParser *p, QObject *parent) : QObject(parent), parser(p) diff --git a/src/plugins/projectexplorer/gnumakeparser.h b/src/plugins/projectexplorer/gnumakeparser.h index e509a7881a..c2d126f30b 100644 --- a/src/plugins/projectexplorer/gnumakeparser.h +++ b/src/plugins/projectexplorer/gnumakeparser.h @@ -42,8 +42,6 @@ public: void stdOutput(const QString &line) override; void stdError(const QString &line) override; - void setWorkingDirectory(const QString &workingDirectory) override; - QStringList searchDirectories() const; bool hasFatalErrors() const override; diff --git a/src/plugins/projectexplorer/ioutputparser.cpp b/src/plugins/projectexplorer/ioutputparser.cpp index 8dd6d99264..0b8c832975 100644 --- a/src/plugins/projectexplorer/ioutputparser.cpp +++ b/src/plugins/projectexplorer/ioutputparser.cpp @@ -138,19 +138,7 @@ void IOutputParser::appendOutputParser(IOutputParser *parser) } m_parser = parser; - connect(parser, &IOutputParser::addOutput, - this, &IOutputParser::outputAdded, Qt::DirectConnection); - connect(parser, &IOutputParser::addTask, - this, &IOutputParser::taskAdded, Qt::DirectConnection); -} - -IOutputParser *IOutputParser::takeOutputParserChain() -{ - IOutputParser *parser = m_parser; - disconnect(parser, &IOutputParser::addOutput, this, &IOutputParser::outputAdded); - disconnect(parser, &IOutputParser::addTask, this, &IOutputParser::taskAdded); - m_parser = nullptr; - return parser; + connect(parser, &IOutputParser::addTask, this, &IOutputParser::taskAdded); } IOutputParser *IOutputParser::childParser() const @@ -163,12 +151,8 @@ void IOutputParser::setChildParser(IOutputParser *parser) if (m_parser != parser) delete m_parser; m_parser = parser; - if (parser) { - connect(parser, &IOutputParser::addOutput, - this, &IOutputParser::outputAdded, Qt::DirectConnection); - connect(parser, &IOutputParser::addTask, - this, &IOutputParser::taskAdded, Qt::DirectConnection); - } + if (parser) + connect(parser, &IOutputParser::addTask, this, &IOutputParser::taskAdded); } void IOutputParser::stdOutput(const QString &line) @@ -183,11 +167,6 @@ void IOutputParser::stdError(const QString &line) m_parser->stdError(line); } -void IOutputParser::outputAdded(const QString &string, BuildStep::OutputFormat format) -{ - emit addOutput(string, format); -} - void IOutputParser::taskAdded(const Task &task, int linkedOutputLines, int skipLines) { emit addTask(task, linkedOutputLines, skipLines); @@ -201,15 +180,11 @@ bool IOutputParser::hasFatalErrors() const return m_parser && m_parser->hasFatalErrors(); } -void IOutputParser::setWorkingDirectory(const QString &workingDirectory) -{ - if (m_parser) - m_parser->setWorkingDirectory(workingDirectory); -} - void IOutputParser::setWorkingDirectory(const Utils::FilePath &fn) { - setWorkingDirectory(fn.toString()); + m_workingDir = fn; + if (m_parser) + m_parser->setWorkingDirectory(fn); } void IOutputParser::flush() diff --git a/src/plugins/projectexplorer/ioutputparser.h b/src/plugins/projectexplorer/ioutputparser.h index cd65378fe2..b27ad25a54 100644 --- a/src/plugins/projectexplorer/ioutputparser.h +++ b/src/plugins/projectexplorer/ioutputparser.h @@ -28,7 +28,7 @@ #include "projectexplorer_export.h" #include "buildstep.h" -namespace Utils { class FilePath; } +#include namespace ProjectExplorer { class Task; @@ -41,9 +41,7 @@ public: IOutputParser() = default; ~IOutputParser() override; - virtual void appendOutputParser(IOutputParser *parser); - - IOutputParser *takeOutputParserChain(); + void appendOutputParser(IOutputParser *parser); IOutputParser *childParser() const; void setChildParser(IOutputParser *parser); @@ -52,25 +50,26 @@ public: virtual void stdError(const QString &line); virtual bool hasFatalErrors() const; - virtual void setWorkingDirectory(const QString &workingDirectory); + void setWorkingDirectory(const Utils::FilePath &fn); void flush(); // flush out pending tasks static QString rightTrimmed(const QString &in); + virtual void taskAdded(const ProjectExplorer::Task &task, int linkedOutputLines = 0, int skipLines = 0); + signals: - void addOutput(const QString &string, ProjectExplorer::BuildStep::OutputFormat format); void addTask(const ProjectExplorer::Task &task, int linkedOutputLines = 0, int skipLines = 0); -public slots: - virtual void outputAdded(const QString &string, ProjectExplorer::BuildStep::OutputFormat format); - virtual void taskAdded(const ProjectExplorer::Task &task, int linkedOutputLines = 0, int skipLines = 0); +protected: + Utils::FilePath workingDirectory() const { return m_workingDir; } private: virtual void doFlush(); IOutputParser *m_parser = nullptr; + Utils::FilePath m_workingDir; }; } // namespace ProjectExplorer diff --git a/src/plugins/projectexplorer/makestep.cpp b/src/plugins/projectexplorer/makestep.cpp index ea327fe303..807cb14f31 100644 --- a/src/plugins/projectexplorer/makestep.cpp +++ b/src/plugins/projectexplorer/makestep.cpp @@ -106,7 +106,7 @@ bool MakeStep::init() IOutputParser *parser = target()->kit()->createOutputParser(); if (parser) appendOutputParser(parser); - outputParser()->setWorkingDirectory(pp->effectiveWorkingDirectory().toString()); + outputParser()->setWorkingDirectory(pp->effectiveWorkingDirectory()); return AbstractProcessStep::init(); } diff --git a/src/plugins/projectexplorer/outputparser_test.cpp b/src/plugins/projectexplorer/outputparser_test.cpp index 17834ff5c8..5c477dd42f 100644 --- a/src/plugins/projectexplorer/outputparser_test.cpp +++ b/src/plugins/projectexplorer/outputparser_test.cpp @@ -38,8 +38,6 @@ static inline QByteArray msgFileComparisonFail(const Utils::FilePath &f1, const return result.toLocal8Bit(); } -OutputParserTester::OutputParserTester() = default; - // test functions: void OutputParserTester::testParsing(const QString &lines, Channel inputChannel, @@ -48,6 +46,10 @@ void OutputParserTester::testParsing(const QString &lines, const QString &childStdErrLines, const QString &outputLines) { + if (!m_terminator) { + m_terminator = new TestTerminator(this); + appendOutputParser(m_terminator); + } reset(); Q_ASSERT(childParser()); @@ -60,18 +62,8 @@ void OutputParserTester::testParsing(const QString &lines, } childParser()->flush(); - // first disconnect ourselves from the end of the parser chain again - IOutputParser *parser = this; - while ( (parser = parser->childParser()) ) { - if (parser->childParser() == this) { - childParser()->takeOutputParserChain(); - break; - } - } - parser = nullptr; + // delete the parser(s) to test emit aboutToDeleteParser(); - - // then delete the parser(s) to test setChildParser(nullptr); QCOMPARE(m_receivedOutput, outputLines); @@ -110,39 +102,11 @@ void OutputParserTester::testTaskMangling(const Task &input, } } -void OutputParserTester::testOutputMangling(const QString &input, - const QString &output) -{ - reset(); - - childParser()->outputAdded(input, BuildStep::OutputFormat::Stdout); - - QCOMPARE(m_receivedOutput, output); - QVERIFY(m_receivedStdErrChildLine.isNull()); - QVERIFY(m_receivedStdOutChildLine.isNull()); - QVERIFY(m_receivedTasks.isEmpty()); -} - void OutputParserTester::setDebugEnabled(bool debug) { m_debug = debug; } -void OutputParserTester::appendOutputParser(IOutputParser *parser) -{ - Q_ASSERT(!childParser()); - parser->appendOutputParser(new TestTerminator(this)); - IOutputParser::appendOutputParser(parser); -} - -void OutputParserTester::outputAdded(const QString &line, BuildStep::OutputFormat format) -{ - Q_UNUSED(format) - if (!m_receivedOutput.isEmpty()) - m_receivedOutput.append('\n'); - m_receivedOutput.append(line); -} - void OutputParserTester::taskAdded(const Task &task, int linkedLines, int skipLines) { Q_UNUSED(linkedLines) diff --git a/src/plugins/projectexplorer/outputparser_test.h b/src/plugins/projectexplorer/outputparser_test.h index 43920d0ebf..dec7e974d5 100644 --- a/src/plugins/projectexplorer/outputparser_test.h +++ b/src/plugins/projectexplorer/outputparser_test.h @@ -46,8 +46,6 @@ public: STDERR }; - OutputParserTester(); - // test functions: void testParsing(const QString &lines, Channel inputChannel, Tasks tasks, @@ -56,18 +54,13 @@ public: const QString &outputLines); void testTaskMangling(const Task &input, const Task &output); - void testOutputMangling(const QString &input, - const QString &output); void setDebugEnabled(bool); - void appendOutputParser(IOutputParser *parser) override; - signals: void aboutToDeleteParser(); private: - void outputAdded(const QString &string, ProjectExplorer::BuildStep::OutputFormat format) override; void taskAdded(const ProjectExplorer::Task &task, int linkedLines, int skipLines) override; void reset(); @@ -78,6 +71,7 @@ private: QString m_receivedStdOutChildLine; Tasks m_receivedTasks; QString m_receivedOutput; + TestTerminator *m_terminator = nullptr; friend class TestTerminator; }; diff --git a/src/plugins/qbsprojectmanager/qbsbuildstep.cpp b/src/plugins/qbsprojectmanager/qbsbuildstep.cpp index a47c1c972f..11b42b890f 100644 --- a/src/plugins/qbsprojectmanager/qbsbuildstep.cpp +++ b/src/plugins/qbsprojectmanager/qbsbuildstep.cpp @@ -179,10 +179,6 @@ bool QbsBuildStep::init() m_activeFileTags = bc->activeFileTags(); m_products = bc->products(); - connect(m_parser, &ProjectExplorer::IOutputParser::addOutput, - this, [this](const QString &string, ProjectExplorer::BuildStep::OutputFormat format) { - emit addOutput(string, format); - }); connect(m_parser, &ProjectExplorer::IOutputParser::addTask, this, &QbsBuildStep::addTask); return true; @@ -383,7 +379,7 @@ void QbsBuildStep::handleProcessResult( if (success && !hasOutput) return; - m_parser->setWorkingDirectory(workingDir.toString()); + m_parser->setWorkingDirectory(workingDir); emit addOutput(executable.toUserOutput() + ' ' + QtcProcess::joinArgs(arguments), OutputFormat::Stdout); for (const QString &line : stdErr) { diff --git a/src/plugins/qbsprojectmanager/qbsparser.cpp b/src/plugins/qbsprojectmanager/qbsparser.cpp index d6b2339dbb..abbe7600ed 100644 --- a/src/plugins/qbsprojectmanager/qbsparser.cpp +++ b/src/plugins/qbsprojectmanager/qbsparser.cpp @@ -29,6 +29,7 @@ #include +#include #include namespace QbsProjectManager { @@ -39,21 +40,13 @@ QbsParser::QbsParser() setObjectName(QLatin1String("QbsParser")); } -void QbsParser::setWorkingDirectory(const QString &workingDirectory) -{ - m_workingDirectory = QDir(workingDirectory); - IOutputParser::setWorkingDirectory(workingDirectory); -} - +// TODO: Is this really needed? qbs never emits relative paths... void QbsParser::taskAdded(const ProjectExplorer::Task &task, int linkedLines, int skipLines) { ProjectExplorer::Task editable(task); - - QString filePath = task.file.toString(); - + const QString filePath = task.file.toString(); if (!filePath.isEmpty()) - editable.file = Utils::FilePath::fromUserInput(m_workingDirectory.absoluteFilePath(filePath)); - + editable.file = workingDirectory().pathAppended(filePath); IOutputParser::taskAdded(editable, linkedLines, skipLines); } diff --git a/src/plugins/qbsprojectmanager/qbsparser.h b/src/plugins/qbsprojectmanager/qbsparser.h index 44bfb133ba..c5814dae97 100644 --- a/src/plugins/qbsprojectmanager/qbsparser.h +++ b/src/plugins/qbsprojectmanager/qbsparser.h @@ -29,8 +29,6 @@ #include -#include - namespace QbsProjectManager { namespace Internal { @@ -42,10 +40,7 @@ public: explicit QbsParser(); private: - void setWorkingDirectory(const QString &workingDirectory) override; void taskAdded(const ProjectExplorer::Task &task, int linkedLines, int skipLines) override; - - QDir m_workingDirectory; }; } // namespace Internal diff --git a/src/plugins/qmakeprojectmanager/qmakestep.cpp b/src/plugins/qmakeprojectmanager/qmakestep.cpp index 2851b114a6..320a786af6 100644 --- a/src/plugins/qmakeprojectmanager/qmakestep.cpp +++ b/src/plugins/qmakeprojectmanager/qmakestep.cpp @@ -338,7 +338,7 @@ void QMakeStep::runNextCommand() case State::RUN_MAKE_QMAKE_ALL: { auto *parser = new GnuMakeParser; - parser->setWorkingDirectory(processParameters()->workingDirectory().toString()); + parser->setWorkingDirectory(processParameters()->workingDirectory()); setOutputParser(parser); m_nextState = State::POST_PROCESS; startOneCommand(m_makeCommand); -- cgit v1.2.3