From 1bf641795f9b8923c87c12f86d502550ba5f6bb3 Mon Sep 17 00:00:00 2001 From: Alessandro Portale Date: Tue, 2 Jul 2024 17:28:05 +0200 Subject: [PATCH] Utils: Fix FileUtils::copyRecursively with user interaction CopyAskingForOverwrite got passed around as value, thus it got copied. The messagebox results were stored into the fields of such copies, and therefore had not the expected values across the recursion. This change wraps the copy helper function into a lambda and the lambda is passed around instead of the object. It may still be a bit much decoupling architecture for a single use-case, but it works now (for me). Fixes: QTCREATORBUG-31174 Change-Id: I5444d18d7bf4ef59ab879e31e5233eadf1c935e4 Reviewed-by: Marcus Tillmanns --- src/libs/utils/fileutils.cpp | 88 +++++++++---------- src/libs/utils/fileutils.h | 5 +- .../android/createandroidmanifestwizard.cpp | 6 +- .../coreplugin/plugininstallwizard.cpp | 5 +- 4 files changed, 53 insertions(+), 51 deletions(-) diff --git a/src/libs/utils/fileutils.cpp b/src/libs/utils/fileutils.cpp index d82353e2468..defc39ede64 100644 --- a/src/libs/utils/fileutils.cpp +++ b/src/libs/utils/fileutils.cpp @@ -305,49 +305,50 @@ FileUtils::CopyAskingForOverwrite::CopyAskingForOverwrite(QWidget *dialogParent, , m_postOperation(postOperation) {} -bool FileUtils::CopyAskingForOverwrite::operator()(const FilePath &src, - const FilePath &dest, - QString *error) +FileUtils::CopyHelper FileUtils::CopyAskingForOverwrite::operator()() { - bool copyFile = true; - if (dest.exists()) { - if (m_skipAll) - copyFile = false; - else if (!m_overwriteAll) { - const int res = QMessageBox::question( - m_parent, - Tr::tr("Overwrite File?"), - Tr::tr("Overwrite existing file \"%1\"?").arg(dest.toUserOutput()), - QMessageBox::Yes | QMessageBox::YesToAll | QMessageBox::No | QMessageBox::NoToAll - | QMessageBox::Cancel); - if (res == QMessageBox::Cancel) { + CopyHelper helperFunction = [this](const FilePath &src, const FilePath &dest, QString *error) { + bool copyFile = true; + if (dest.exists()) { + if (m_skipAll) + copyFile = false; + else if (!m_overwriteAll) { + const int res = QMessageBox::question( + m_parent, + Tr::tr("Overwrite File?"), + Tr::tr("Overwrite existing file \"%1\"?").arg(dest.toUserOutput()), + QMessageBox::Yes | QMessageBox::YesToAll | QMessageBox::No + | QMessageBox::NoToAll | QMessageBox::Cancel); + if (res == QMessageBox::Cancel) { + return false; + } else if (res == QMessageBox::No) { + copyFile = false; + } else if (res == QMessageBox::NoToAll) { + m_skipAll = true; + copyFile = false; + } else if (res == QMessageBox::YesToAll) { + m_overwriteAll = true; + } + if (copyFile) + dest.removeFile(); + } + } + if (copyFile) { + dest.parentDir().ensureWritableDir(); + if (!src.copyFile(dest)) { + if (error) { + *error = Tr::tr("Could not copy file \"%1\" to \"%2\".") + .arg(src.toUserOutput(), dest.toUserOutput()); + } return false; - } else if (res == QMessageBox::No) { - copyFile = false; - } else if (res == QMessageBox::NoToAll) { - m_skipAll = true; - copyFile = false; - } else if (res == QMessageBox::YesToAll) { - m_overwriteAll = true; } - if (copyFile) - dest.removeFile(); + if (m_postOperation) + m_postOperation(dest); } - } - if (copyFile) { - dest.parentDir().ensureWritableDir(); - if (!src.copyFile(dest)) { - if (error) { - *error = Tr::tr("Could not copy file \"%1\" to \"%2\".") - .arg(src.toUserOutput(), dest.toUserOutput()); - } - return false; - } - if (m_postOperation) - m_postOperation(dest); - } - m_files.append(dest.absoluteFilePath()); - return true; + m_files.append(dest.absoluteFilePath()); + return true; + }; + return helperFunction; } FilePaths FileUtils::CopyAskingForOverwrite::files() const @@ -694,11 +695,10 @@ FilePathInfo FileUtils::filePathInfoFromTriple(const QString &infos, int modeBas return {size, flags, dt}; } -bool FileUtils::copyRecursively( - const FilePath &srcFilePath, - const FilePath &tgtFilePath, - QString *error, - std::function copyHelper) +bool FileUtils::copyRecursively(const FilePath &srcFilePath, + const FilePath &tgtFilePath, + QString *error, + CopyHelper copyHelper) { if (srcFilePath.isDir()) { if (!tgtFilePath.ensureWritableDir()) { diff --git a/src/libs/utils/fileutils.h b/src/libs/utils/fileutils.h index bdf7a7e3bab..32e7da429f4 100644 --- a/src/libs/utils/fileutils.h +++ b/src/libs/utils/fileutils.h @@ -43,13 +43,14 @@ struct QTCREATOR_UTILS_EXPORT RunResult class QTCREATOR_UTILS_EXPORT FileUtils { public: + using CopyHelper = std::function; #ifdef QT_GUI_LIB class QTCREATOR_UTILS_EXPORT CopyAskingForOverwrite { public: CopyAskingForOverwrite(QWidget *dialogParent, const std::function &postOperation = {}); - bool operator()(const FilePath &src, const FilePath &dest, QString *error); + CopyHelper operator()(); FilePaths files() const; private: @@ -65,7 +66,7 @@ public: const FilePath &srcFilePath, const FilePath &tgtFilePath, QString *error, - std::function helper); + CopyHelper helper); static bool copyIfDifferent(const FilePath &srcFilePath, const FilePath &tgtFilePath); diff --git a/src/plugins/android/createandroidmanifestwizard.cpp b/src/plugins/android/createandroidmanifestwizard.cpp index 18a7872e08d..c446ddd55b4 100644 --- a/src/plugins/android/createandroidmanifestwizard.cpp +++ b/src/plugins/android/createandroidmanifestwizard.cpp @@ -281,17 +281,17 @@ void CreateAndroidManifestWizard::createAndroidTemplateFiles() FileUtils::copyRecursively(version->prefix() / "src/android/java/AndroidManifest.xml", m_directory / "AndroidManifest.xml", nullptr, - copy); + copy()); } else { FileUtils::copyRecursively(version->prefix() / "src/android/templates", m_directory, nullptr, - copy); + copy()); if (m_copyGradle) { FilePath gradlePath = version->prefix() / "src/3rdparty/gradle"; QTC_ASSERT(gradlePath.exists(), return); - FileUtils::copyRecursively(gradlePath, m_directory, nullptr, copy); + FileUtils::copyRecursively(gradlePath, m_directory, nullptr, copy()); } } diff --git a/src/plugins/coreplugin/plugininstallwizard.cpp b/src/plugins/coreplugin/plugininstallwizard.cpp index e9f91ffbb8e..84c90c43043 100644 --- a/src/plugins/coreplugin/plugininstallwizard.cpp +++ b/src/plugins/coreplugin/plugininstallwizard.cpp @@ -425,11 +425,12 @@ bool executePluginInstallWizard(const FilePath &archive) return copyPluginFile(data.sourcePath, installPath); } else { QString error; + FileUtils::CopyAskingForOverwrite copy(ICore::dialogParent(), + postCopyOperation()); if (!FileUtils::copyRecursively(data.extractedPath, installPath, &error, - FileUtils::CopyAskingForOverwrite(ICore::dialogParent(), - postCopyOperation()))) { + copy())) { QMessageBox::warning(ICore::dialogParent(), Tr::tr("Failed to Copy Plugin Files"), error);