From 9bd1789e2fed6439354e0caa9367b521cb741bab Mon Sep 17 00:00:00 2001 From: Marcus Tillmanns Date: Thu, 12 Sep 2024 07:15:16 +0200 Subject: [PATCH] Utils: Let FilePath::renameFile return an error Change-Id: I186ee762c072de78e589dba7b595db60b02379c1 Reviewed-by: hjk --- .../gocmdbridge/client/bridgedfileaccess.cpp | 18 ++++++++---- .../gocmdbridge/client/bridgedfileaccess.h | 3 +- src/libs/utils/devicefileaccess.cpp | 29 ++++++++++++++----- src/libs/utils/devicefileaccess.h | 6 ++-- src/libs/utils/filepath.cpp | 2 +- src/libs/utils/filepath.h | 2 +- src/libs/utils/fsengine/fsenginehandler.cpp | 10 +++++-- src/plugins/coreplugin/fileutils.cpp | 2 +- .../languageclient/languageclientutils.cpp | 2 +- 9 files changed, 50 insertions(+), 24 deletions(-) diff --git a/src/libs/gocmdbridge/client/bridgedfileaccess.cpp b/src/libs/gocmdbridge/client/bridgedfileaccess.cpp index 01ccf3029af..dcd17b23e30 100644 --- a/src/libs/gocmdbridge/client/bridgedfileaccess.cpp +++ b/src/libs/gocmdbridge/client/bridgedfileaccess.cpp @@ -544,17 +544,23 @@ expected_str FileAccess::copyFile(const Utils::FilePath &filePath, } } -bool FileAccess::renameFile(const Utils::FilePath &filePath, const Utils::FilePath &target) const +expected_str FileAccess::renameFile( + const Utils::FilePath &filePath, const Utils::FilePath &target) const { try { - auto f = m_client->renameFile(filePath.nativePath(), target.nativePath()); - QTC_ASSERT_EXPECTED(f, return false); + Utils::expected_str> f + = m_client->renameFile(filePath.nativePath(), target.nativePath()); + if (!f) + return make_unexpected(f.error()); f->waitForFinished(); - return true; + if (!f) + return make_unexpected(f.error()); } catch (const std::exception &e) { - qCWarning(faLog) << "Error renaming file:" << e.what(); - return false; + return make_unexpected( + Tr::tr("Error renaming file: %1").arg(QString::fromLocal8Bit(e.what()))); } + + return {}; } Environment FileAccess::deviceEnvironment() const diff --git a/src/libs/gocmdbridge/client/bridgedfileaccess.h b/src/libs/gocmdbridge/client/bridgedfileaccess.h index bf76d8191e5..2d6ffb72278 100644 --- a/src/libs/gocmdbridge/client/bridgedfileaccess.h +++ b/src/libs/gocmdbridge/client/bridgedfileaccess.h @@ -78,7 +78,8 @@ protected: Utils::expected_str copyFile(const Utils::FilePath &filePath, const Utils::FilePath &target) const override; - bool renameFile(const Utils::FilePath &filePath, const Utils::FilePath &target) const override; + Utils::expected_str renameFile( + const Utils::FilePath &filePath, const Utils::FilePath &target) const override; Utils::expected_str createTempFile(const Utils::FilePath &filePath) override; diff --git a/src/libs/utils/devicefileaccess.cpp b/src/libs/utils/devicefileaccess.cpp index f264a30e4cd..299623adcce 100644 --- a/src/libs/utils/devicefileaccess.cpp +++ b/src/libs/utils/devicefileaccess.cpp @@ -275,12 +275,13 @@ expected_str DeviceFileAccess::copyRecursively(const FilePath &src, #endif } -bool DeviceFileAccess::renameFile(const FilePath &filePath, const FilePath &target) const +expected_str DeviceFileAccess::renameFile(const FilePath &filePath, const FilePath &target) const { Q_UNUSED(filePath) Q_UNUSED(target) QTC_CHECK(false); - return false; + return make_unexpected( + Tr::tr("renameFile is not implemented for \"%1\".").arg(filePath.toUserOutput())); } FilePath DeviceFileAccess::symLinkTarget(const FilePath &filePath) const @@ -786,9 +787,16 @@ expected_str DesktopDeviceFileAccess::copyFile(const FilePath &filePath, .arg(filePath.toUserOutput(), target.toUserOutput(), srcFile.errorString())); } -bool DesktopDeviceFileAccess::renameFile(const FilePath &filePath, const FilePath &target) const +expected_str DesktopDeviceFileAccess::renameFile( + const FilePath &filePath, const FilePath &target) const { - return QFile::rename(filePath.path(), target.path()); + QFile f(filePath.path()); + + if (f.rename(target.path())) + return {}; + return make_unexpected( + Tr::tr("Failed to rename file \"%1\" to \"%2\": %3") + .arg(filePath.toUserOutput(), target.toUserOutput(), f.errorString())); } FilePathInfo DesktopDeviceFileAccess::filePathInfo(const FilePath &filePath) const @@ -1181,12 +1189,19 @@ expected_str UnixDeviceFileAccess::copyFile(const FilePath &filePath, return {}; } -bool UnixDeviceFileAccess::renameFile(const FilePath &filePath, const FilePath &target) const +expected_str UnixDeviceFileAccess::renameFile(const FilePath &filePath, const FilePath &target) const { if (disconnected()) - return false; + return make_unexpected_disconnected(); - return runInShellSuccess({"mv", {filePath.path(), target.path()}, OsType::OsTypeLinux}); + auto result = runInShell({"mv", {filePath.path(), target.path()}, OsType::OsTypeLinux}); + if (result.exitCode != 0) { + return make_unexpected(Tr::tr("Failed to rename file \"%1\" to \"%2\": %3") + .arg(filePath.toUserOutput(), + target.toUserOutput(), + QString::fromUtf8(result.stdErr))); + } + return {}; } FilePath UnixDeviceFileAccess::symLinkTarget(const FilePath &filePath) const diff --git a/src/libs/utils/devicefileaccess.h b/src/libs/utils/devicefileaccess.h index c94e3b2081d..97aa014ab21 100644 --- a/src/libs/utils/devicefileaccess.h +++ b/src/libs/utils/devicefileaccess.h @@ -47,7 +47,7 @@ protected: virtual expected_str copyFile(const FilePath &filePath, const FilePath &target) const; virtual expected_str copyRecursively(const FilePath &filePath, const FilePath &target) const; - virtual bool renameFile(const FilePath &filePath, const FilePath &target) const; + virtual expected_str renameFile(const FilePath &filePath, const FilePath &target) const; virtual FilePath symLinkTarget(const FilePath &filePath) const; virtual FilePathInfo filePathInfo(const FilePath &filePath) const; @@ -104,7 +104,7 @@ protected: expected_str removeFile(const FilePath &filePath) const override; bool removeRecursively(const FilePath &filePath, QString *error) const override; expected_str copyFile(const FilePath &filePath, const FilePath &target) const override; - bool renameFile(const FilePath &filePath, const FilePath &target) const override; + expected_str renameFile(const FilePath &filePath, const FilePath &target) const override; FilePath symLinkTarget(const FilePath &filePath) const override; FilePathInfo filePathInfo(const FilePath &filePath) const override; @@ -162,7 +162,7 @@ protected: expected_str removeFile(const FilePath &filePath) const override; bool removeRecursively(const FilePath &filePath, QString *error) const override; expected_str copyFile(const FilePath &filePath, const FilePath &target) const override; - bool renameFile(const FilePath &filePath, const FilePath &target) const override; + expected_str renameFile(const FilePath &filePath, const FilePath &target) const override; FilePathInfo filePathInfo(const FilePath &filePath) const override; FilePath symLinkTarget(const FilePath &filePath) const override; diff --git a/src/libs/utils/filepath.cpp b/src/libs/utils/filepath.cpp index fd1ddc8667b..9aef4ee5adf 100644 --- a/src/libs/utils/filepath.cpp +++ b/src/libs/utils/filepath.cpp @@ -2022,7 +2022,7 @@ expected_str FilePath::copyFile(const FilePath &target) const return fileAccess()->copyFile(*this, target); } -bool FilePath::renameFile(const FilePath &target) const +expected_str FilePath::renameFile(const FilePath &target) const { return fileAccess()->renameFile(*this, target); } diff --git a/src/libs/utils/filepath.h b/src/libs/utils/filepath.h index 4233265d654..a14c525e950 100644 --- a/src/libs/utils/filepath.h +++ b/src/libs/utils/filepath.h @@ -167,7 +167,7 @@ public: bool removeRecursively(QString *error = nullptr) const; expected_str copyRecursively(const FilePath &target) const; expected_str copyFile(const FilePath &target) const; - bool renameFile(const FilePath &target) const; + expected_str renameFile(const FilePath &target) const; qint64 fileSize() const; qint64 bytesAvailable() const; bool createDir() const; diff --git a/src/libs/utils/fsengine/fsenginehandler.cpp b/src/libs/utils/fsengine/fsenginehandler.cpp index 3667e52b92e..ba4235e6c3d 100644 --- a/src/libs/utils/fsengine/fsenginehandler.cpp +++ b/src/libs/utils/fsengine/fsenginehandler.cpp @@ -216,13 +216,17 @@ bool FSEngineImpl::remove() bool FSEngineImpl::copy(const QString &newName) { expected_str result = m_filePath.copyFile(FilePath::fromString(newName)); - QTC_ASSERT_EXPECTED(result, return false); - return true; + if (!result) + setError(QFile::CopyError, result.error()); + return result.has_value(); } bool FSEngineImpl::rename(const QString &newName) { - return m_filePath.renameFile(FilePath::fromString(newName)); + auto result = m_filePath.renameFile(FilePath::fromString(newName)); + if (!result) + setError(QFile::RenameError, result.error()); + return result.has_value(); } bool FSEngineImpl::renameOverwrite(const QString &newName) diff --git a/src/plugins/coreplugin/fileutils.cpp b/src/plugins/coreplugin/fileutils.cpp index e05375393da..b5f719f4a22 100644 --- a/src/plugins/coreplugin/fileutils.cpp +++ b/src/plugins/coreplugin/fileutils.cpp @@ -187,7 +187,7 @@ bool FileUtils::renameFile(const FilePath &orgFilePath, const FilePath &newFileP if (vc && vc->supportsOperation(IVersionControl::MoveOperation)) result = vc->vcsMove(orgFilePath, newFilePath); if (!result) // The moving via vcs failed or the vcs does not support moving, fall back - result = orgFilePath.renameFile(newFilePath); + result = orgFilePath.renameFile(newFilePath).has_value(); if (result) { DocumentManager::renamedFile(orgFilePath, newFilePath); updateHeaderFileGuardIfApplicable(orgFilePath, newFilePath, handleGuards); diff --git a/src/plugins/languageclient/languageclientutils.cpp b/src/plugins/languageclient/languageclientutils.cpp index 5f8e832d17e..67a008ab074 100644 --- a/src/plugins/languageclient/languageclientutils.cpp +++ b/src/plugins/languageclient/languageclientutils.cpp @@ -381,7 +381,7 @@ bool applyDocumentChange(const Client *client, const DocumentChange &change) } } } - return oldPath.renameFile(newPath); + return oldPath.renameFile(newPath).has_value(); } else if (const auto deleteOperation = std::get_if(&change)) { const FilePath filePath = deleteOperation->uri().toFilePath(client->hostPathMapper()); if (const std::optional options = deleteOperation->options()) {