Skip to content

Commit 1bf6417

Browse files
aportaleaportale
committed
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 <[email protected]>
1 parent 29bfece commit 1bf6417

File tree

4 files changed

+52
-50
lines changed

4 files changed

+52
-50
lines changed

‎src/libs/utils/fileutils.cpp

Lines changed: 43 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -305,49 +305,50 @@ FileUtils::CopyAskingForOverwrite::CopyAskingForOverwrite(QWidget *dialogParent,
305305
, m_postOperation(postOperation)
306306
{}
307307

308-
bool FileUtils::CopyAskingForOverwrite::operator()(const FilePath &src,
309-
const FilePath &dest,
310-
QString *error)
311-
{
312-
bool copyFile = true;
313-
if (dest.exists()) {
314-
if (m_skipAll)
315-
copyFile = false;
316-
else if (!m_overwriteAll) {
317-
const int res = QMessageBox::question(
318-
m_parent,
319-
Tr::tr("Overwrite File?"),
320-
Tr::tr("Overwrite existing file \"%1\"?").arg(dest.toUserOutput()),
321-
QMessageBox::Yes | QMessageBox::YesToAll | QMessageBox::No | QMessageBox::NoToAll
322-
| QMessageBox::Cancel);
323-
if (res == QMessageBox::Cancel) {
324-
return false;
325-
} else if (res == QMessageBox::No) {
326-
copyFile = false;
327-
} else if (res == QMessageBox::NoToAll) {
328-
m_skipAll = true;
308+
FileUtils::CopyHelper FileUtils::CopyAskingForOverwrite::operator()()
309+
{
310+
CopyHelper helperFunction = [this](const FilePath &src, const FilePath &dest, QString *error) {
311+
bool copyFile = true;
312+
if (dest.exists()) {
313+
if (m_skipAll)
329314
copyFile = false;
330-
} else if (res == QMessageBox::YesToAll) {
331-
m_overwriteAll = true;
315+
else if (!m_overwriteAll) {
316+
const int res = QMessageBox::question(
317+
m_parent,
318+
Tr::tr("Overwrite File?"),
319+
Tr::tr("Overwrite existing file \"%1\"?").arg(dest.toUserOutput()),
320+
QMessageBox::Yes | QMessageBox::YesToAll | QMessageBox::No
321+
| QMessageBox::NoToAll | QMessageBox::Cancel);
322+
if (res == QMessageBox::Cancel) {
323+
return false;
324+
} else if (res == QMessageBox::No) {
325+
copyFile = false;
326+
} else if (res == QMessageBox::NoToAll) {
327+
m_skipAll = true;
328+
copyFile = false;
329+
} else if (res == QMessageBox::YesToAll) {
330+
m_overwriteAll = true;
331+
}
332+
if (copyFile)
333+
dest.removeFile();
332334
}
333-
if (copyFile)
334-
dest.removeFile();
335335
}
336-
}
337-
if (copyFile) {
338-
dest.parentDir().ensureWritableDir();
339-
if (!src.copyFile(dest)) {
340-
if (error) {
341-
*error = Tr::tr("Could not copy file \"%1\" to \"%2\".")
342-
.arg(src.toUserOutput(), dest.toUserOutput());
336+
if (copyFile) {
337+
dest.parentDir().ensureWritableDir();
338+
if (!src.copyFile(dest)) {
339+
if (error) {
340+
*error = Tr::tr("Could not copy file \"%1\" to \"%2\".")
341+
.arg(src.toUserOutput(), dest.toUserOutput());
342+
}
343+
return false;
343344
}
344-
return false;
345+
if (m_postOperation)
346+
m_postOperation(dest);
345347
}
346-
if (m_postOperation)
347-
m_postOperation(dest);
348-
}
349-
m_files.append(dest.absoluteFilePath());
350-
return true;
348+
m_files.append(dest.absoluteFilePath());
349+
return true;
350+
};
351+
return helperFunction;
351352
}
352353

353354
FilePaths FileUtils::CopyAskingForOverwrite::files() const
@@ -694,11 +695,10 @@ FilePathInfo FileUtils::filePathInfoFromTriple(const QString &infos, int modeBas
694695
return {size, flags, dt};
695696
}
696697

697-
bool FileUtils::copyRecursively(
698-
const FilePath &srcFilePath,
699-
const FilePath &tgtFilePath,
700-
QString *error,
701-
std::function<bool(const FilePath &, const FilePath &, QString *)> copyHelper)
698+
bool FileUtils::copyRecursively(const FilePath &srcFilePath,
699+
const FilePath &tgtFilePath,
700+
QString *error,
701+
CopyHelper copyHelper)
702702
{
703703
if (srcFilePath.isDir()) {
704704
if (!tgtFilePath.ensureWritableDir()) {

‎src/libs/utils/fileutils.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,14 @@ struct QTCREATOR_UTILS_EXPORT RunResult
4343
class QTCREATOR_UTILS_EXPORT FileUtils
4444
{
4545
public:
46+
using CopyHelper = std::function<bool(const FilePath &, const FilePath &, QString *)>;
4647
#ifdef QT_GUI_LIB
4748
class QTCREATOR_UTILS_EXPORT CopyAskingForOverwrite
4849
{
4950
public:
5051
CopyAskingForOverwrite(QWidget *dialogParent,
5152
const std::function<void(FilePath)> &postOperation = {});
52-
bool operator()(const FilePath &src, const FilePath &dest, QString *error);
53+
CopyHelper operator()();
5354
FilePaths files() const;
5455

5556
private:
@@ -65,7 +66,7 @@ class QTCREATOR_UTILS_EXPORT FileUtils
6566
const FilePath &srcFilePath,
6667
const FilePath &tgtFilePath,
6768
QString *error,
68-
std::function<bool(const FilePath &, const FilePath &, QString *)> helper);
69+
CopyHelper helper);
6970

7071
static bool copyIfDifferent(const FilePath &srcFilePath,
7172
const FilePath &tgtFilePath);

‎src/plugins/android/createandroidmanifestwizard.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -281,17 +281,17 @@ void CreateAndroidManifestWizard::createAndroidTemplateFiles()
281281
FileUtils::copyRecursively(version->prefix() / "src/android/java/AndroidManifest.xml",
282282
m_directory / "AndroidManifest.xml",
283283
nullptr,
284-
copy);
284+
copy());
285285
} else {
286286
FileUtils::copyRecursively(version->prefix() / "src/android/templates",
287287
m_directory,
288288
nullptr,
289-
copy);
289+
copy());
290290

291291
if (m_copyGradle) {
292292
FilePath gradlePath = version->prefix() / "src/3rdparty/gradle";
293293
QTC_ASSERT(gradlePath.exists(), return);
294-
FileUtils::copyRecursively(gradlePath, m_directory, nullptr, copy);
294+
FileUtils::copyRecursively(gradlePath, m_directory, nullptr, copy());
295295
}
296296
}
297297

‎src/plugins/coreplugin/plugininstallwizard.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -425,11 +425,12 @@ bool executePluginInstallWizard(const FilePath &archive)
425425
return copyPluginFile(data.sourcePath, installPath);
426426
} else {
427427
QString error;
428+
FileUtils::CopyAskingForOverwrite copy(ICore::dialogParent(),
429+
postCopyOperation());
428430
if (!FileUtils::copyRecursively(data.extractedPath,
429431
installPath,
430432
&error,
431-
FileUtils::CopyAskingForOverwrite(ICore::dialogParent(),
432-
postCopyOperation()))) {
433+
copy())) {
433434
QMessageBox::warning(ICore::dialogParent(),
434435
Tr::tr("Failed to Copy Plugin Files"),
435436
error);

0 commit comments

Comments
 (0)