From 03a52b9c8725992408f0087294ab1bc5be9159e6 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Tue, 29 Oct 2019 17:11:00 +0100 Subject: QmakeProjectManager: Improve renaming functionality This was implemented rather sloppily: The file was removed from all variables, but only added to one. Also, no care was taken to insert the new file name into the same block the old one was removed from. Fixes: QTCREATORBUG-19257 Change-Id: Ib309389ba7647189112d5c7dd7b3e784f921d2c3 Reviewed-by: Joerg Bornemann --- .../qmakeprojectmanager/qmakeparsernodes.cpp | 59 +++++++++++++--------- src/plugins/qmakeprojectmanager/qmakeparsernodes.h | 5 +- src/shared/proparser/prowriter.cpp | 34 ++++++++----- src/shared/proparser/prowriter.h | 21 ++++++-- 4 files changed, 76 insertions(+), 43 deletions(-) diff --git a/src/plugins/qmakeprojectmanager/qmakeparsernodes.cpp b/src/plugins/qmakeprojectmanager/qmakeparsernodes.cpp index fc3be830c7..0ae1c3ae1a 100644 --- a/src/plugins/qmakeprojectmanager/qmakeparsernodes.cpp +++ b/src/plugins/qmakeprojectmanager/qmakeparsernodes.cpp @@ -638,8 +638,7 @@ bool QmakePriFile::canRenameFile(const QString &filePath, const QString &newFile if (changeProFileOptional) return true; - const Utils::MimeType mt = Utils::mimeTypeForFile(newFilePath); - return renameFile(filePath, newFilePath, mt.name(), Change::TestOnly); + return renameFile(filePath, newFilePath, Change::TestOnly); } bool QmakePriFile::renameFile(const QString &filePath, const QString &newFilePath) @@ -648,9 +647,7 @@ bool QmakePriFile::renameFile(const QString &filePath, const QString &newFilePat return false; bool changeProFileOptional = deploysFolder(QFileInfo(filePath).absolutePath()); - const Utils::MimeType mt = Utils::mimeTypeForFile(newFilePath); - - if (renameFile(filePath, newFilePath, mt.name())) + if (renameFile(filePath, newFilePath, Change::Save)) return true; return changeProFileOptional; } @@ -806,10 +803,7 @@ bool QmakePriFile::prepareForChange() return saveModifiedEditors() && ensureWriteableProFile(filePath().toString()); } -bool QmakePriFile::renameFile(const QString &oldName, - const QString &newName, - const QString &mimeType, - Change mode) +bool QmakePriFile::renameFile(const QString &oldName, const QString &newName, Change mode) { if (!prepareForChange()) return false; @@ -822,27 +816,46 @@ bool QmakePriFile::renameFile(const QString &oldName, return false; QDir priFileDir = QDir(m_qmakeProFile->directoryPath().toString()); - QStringList notChanged = ProWriter::removeFiles(includeFile, &lines, priFileDir, - QStringList(oldName), varNamesForRemoving()); + ProWriter::VarLocations removedLocations; + const QStringList notChanged = ProWriter::removeFiles( + includeFile, + &lines, + priFileDir, + QStringList(oldName), + varNamesForRemoving(), + &removedLocations + ); includeFile->deref(); if (!notChanged.isEmpty()) return false; + QTC_ASSERT(!removedLocations.isEmpty(), return false); + + int endLine = lines.count(); + reverseForeach(removedLocations, + [this, &newName, &lines, &endLine](const ProWriter::VarLocation &loc) { + QStringList currentLines = lines.mid(loc.second, endLine - loc.second); + const QString currentContents = currentLines.join('\n'); + + // Reparse necessary due to changed contents. + QMakeParser parser(nullptr, nullptr, nullptr); + ProFile * const proFile = parser.parsedProBlock( + QStringRef(¤tContents), + 0, + filePath().toString(), + 1, + QMakeParser::FullGrammar + ); + QTC_ASSERT(proFile, return); // The file should still be valid after what we did. + + ProWriter::addFiles(proFile, ¤tLines, {newName}, loc.first, continuationIndent()); + lines = lines.mid(0, loc.second) + currentLines + lines.mid(endLine); + endLine = loc.second; + proFile->deref(); + }); - // We need to re-parse here: The file has changed. - QMakeParser parser(nullptr, nullptr, nullptr); - QString contents = lines.join(QLatin1Char('\n')); - includeFile = parser.parsedProBlock(QStringRef(&contents), - 0, filePath().toString(), 1, QMakeParser::FullGrammar); - QTC_ASSERT(includeFile, return false); // The file should still be valid after what we did. - - ProWriter::addFiles(includeFile, &lines, - QStringList(newName), - varNameForAdding(mimeType), - continuationIndent()); if (mode == Change::Save) save(lines); - includeFile->deref(); return true; } diff --git a/src/plugins/qmakeprojectmanager/qmakeparsernodes.h b/src/plugins/qmakeprojectmanager/qmakeparsernodes.h index a6e7fa4ad5..538d1a65e9 100644 --- a/src/plugins/qmakeprojectmanager/qmakeparsernodes.h +++ b/src/plugins/qmakeprojectmanager/qmakeparsernodes.h @@ -192,10 +192,7 @@ protected: }; enum class Change { Save, TestOnly }; - bool renameFile(const QString &oldName, - const QString &newName, - const QString &mimeType, - Change mode = Change::Save); + bool renameFile(const QString &oldName, const QString &newName, Change mode); void changeFiles(const QString &mimeType, const QStringList &filePaths, QStringList *notChanged, diff --git a/src/shared/proparser/prowriter.cpp b/src/shared/proparser/prowriter.cpp index 282080013f..6d79524912 100644 --- a/src/shared/proparser/prowriter.cpp +++ b/src/shared/proparser/prowriter.cpp @@ -415,7 +415,7 @@ void ProWriter::addFiles(ProFile *profile, QStringList *lines, const QStringList } static void findProVariables(const ushort *tokPtr, const QStringList &vars, - QList *proVars, const uint firstLine = 0) + ProWriter::VarLocations &proVars, const uint firstLine = 0) { int lineNo = firstLine; QString tmp; @@ -433,8 +433,11 @@ static void findProVariables(const ushort *tokPtr, const QStringList &vars, tokPtr += blockLen; } } else if (tok == TokAssign || tok == TokAppend || tok == TokAppendUnique) { - if (getLiteral(lastXpr, tokPtr - 1, tmp) && vars.contains(tmp)) - *proVars << lineNo; + if (getLiteral(lastXpr, tokPtr - 1, tmp) && vars.contains(tmp)) { + QString varName = tmp; + varName.detach(); // tmp was constructed via setRawData() + proVars << qMakePair(varName, lineNo); + } skipExpression(++tokPtr, lineNo); } else { lastXpr = skipToken(tok, tokPtr, lineNo); @@ -443,21 +446,21 @@ static void findProVariables(const ushort *tokPtr, const QStringList &vars, } QList ProWriter::removeVarValues(ProFile *profile, QStringList *lines, - const QStringList &values, const QStringList &vars) + const QStringList &values, const QStringList &vars, VarLocations *removedLocations) { QList notChanged; // yeah, this is a bit silly for (int i = 0; i < values.size(); i++) notChanged << i; - QList varLines; - findProVariables(profile->tokPtr(), vars, &varLines); + VarLocations varLocations; + findProVariables(profile->tokPtr(), vars, varLocations); // This code expects proVars to be sorted by the variables' appearance in the file. int delta = 1; - for (int ln : qAsConst(varLines)) { + for (const VarLocation &loc : qAsConst(varLocations)) { bool first = true; - int lineNo = ln - delta; + int lineNo = loc.second - delta; typedef QPair ContPos; QList contPos; while (lineNo < lines->count()) { @@ -507,6 +510,8 @@ QList ProWriter::removeVarValues(ProFile *profile, QStringList *lines, const int pos = values.indexOf(fn); if (pos != -1) { notChanged.removeOne(pos); + if (removedLocations) + *removedLocations << qMakePair(loc.first, loc.second - delta); if (colNo < lineLen) colNo++; else if (varCol) @@ -559,8 +564,13 @@ QList ProWriter::removeVarValues(ProFile *profile, QStringList *lines, return notChanged; } -QStringList ProWriter::removeFiles(ProFile *profile, QStringList *lines, - const QDir &proFileDir, const QStringList &values, const QStringList &vars) +QStringList ProWriter::removeFiles( + ProFile *profile, + QStringList *lines, + const QDir &proFileDir, + const QStringList &values, + const QStringList &vars, + VarLocations *removedLocations) { // This is a tad stupid - basically, it can remove only entries which // the above code added. @@ -569,7 +579,7 @@ QStringList ProWriter::removeFiles(ProFile *profile, QStringList *lines, valuesToFind << proFileDir.relativeFilePath(absoluteFilePath); const QStringList notYetChanged = - Utils::transform(removeVarValues(profile, lines, valuesToFind, vars), + Utils::transform(removeVarValues(profile, lines, valuesToFind, vars, removedLocations), [values](int i) { return values.at(i); }); if (!profile->fileName().endsWith(".pri")) @@ -585,7 +595,7 @@ QStringList ProWriter::removeFiles(ProFile *profile, QStringList *lines, valuesToFind << (prefixPwd + baseDir.relativeFilePath(absoluteFilePath)); const QStringList notChanged = - Utils::transform(removeVarValues(profile, lines, valuesToFind, vars), + Utils::transform(removeVarValues(profile, lines, valuesToFind, vars, removedLocations), [notYetChanged](int i) { return notYetChanged.at(i); }); return notChanged; diff --git a/src/shared/proparser/prowriter.h b/src/shared/proparser/prowriter.h index 31467c0e66..bf19d23bc7 100644 --- a/src/shared/proparser/prowriter.h +++ b/src/shared/proparser/prowriter.h @@ -52,13 +52,26 @@ public: static void putVarValues(ProFile *profile, QStringList *lines, const QStringList &values, const QString &var, PutFlags flags, const QString &scope, const QString &continuationIndent); - static QList removeVarValues(ProFile *profile, QStringList *lines, - const QStringList &values, const QStringList &vars); + + using VarLocation = QPair; + using VarLocations = QList; + static QList removeVarValues( + ProFile *profile, + QStringList *lines, + const QStringList &values, + const QStringList &vars, + VarLocations *removedLocations = nullptr + ); static void addFiles(ProFile *profile, QStringList *lines, const QStringList &filePaths, const QString &var, const QString &continuationIndent); - static QStringList removeFiles(ProFile *profile, QStringList *lines, - const QDir &proFileDir, const QStringList &filePaths, const QStringList &vars); + static QStringList removeFiles( + ProFile *profile, + QStringList *lines, + const QDir &proFileDir, + const QStringList &filePaths, + const QStringList &vars, + VarLocations *removedLocations = nullptr); private: static bool locateVarValues(const ushort *tokPtr, const ushort *tokPtrEnd, -- cgit v1.2.1