Display local file name in case clash warning

Fixes: #8609
owncloud · May 11, 2021 · fa2b144 · fa2b144
1 parent c6ea448
commit fa2b144
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 19 deletions.
diff --git a/changelog/unreleased/8609 b/changelog/unreleased/8609
@@ -0,0 +1,6 @@
+Enhancement: Mention the local file name when a file name clash occurs
+
+We now display the name of the file we detected the clash with.
+
+https://github.com/owncloud/client/issues/8609
+https://github.com/owncloud/client/pull/8630
diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp
@@ -510,7 +510,7 @@ void OwncloudPropagator::setSyncOptions(const SyncOptions &syncOptions)
     _chunkSize = syncOptions._initialChunkSize;
 }
 
-bool OwncloudPropagator::localFileNameClash(const QString &relFile)
+Result<QString, bool> OwncloudPropagator::localFileNameClash(const QString &relFile)
 {
     const QString file(_localDir + relFile);
     OC_ASSERT(!file.isEmpty());
@@ -526,23 +526,21 @@ bool OwncloudPropagator::localFileNameClash(const QString &relFile)
             const QString cName = fileInfo.canonicalFilePath().normalized(QString::NormalizationForm_C);
             if (file != cName && !cName.endsWith(relFile, Qt::CaseSensitive)) {
                 qCWarning(lcPropagator) << "Detected case clash between" << file << "and" << cName;
-                return true;
+                return cName;
             }
         }
 #elif defined(Q_OS_WIN)
         WIN32_FIND_DATA FindFileData;
         HANDLE hFind;
 
         hFind = FindFirstFileW(reinterpret_cast<const wchar_t *>(FileSystem::longWinPath(file).utf16()), &FindFileData);
-        if (hFind == INVALID_HANDLE_VALUE) {
-            // returns false.
-        } else {
-            const QString realFileName = QString::fromWCharArray(FindFileData.cFileName);
+        if (hFind != INVALID_HANDLE_VALUE) {
+            const QString realFileName = _localDir + QString::fromWCharArray(FindFileData.cFileName);
             FindClose(hFind);
 
             if (!file.endsWith(realFileName, Qt::CaseSensitive)) {
                 qCWarning(lcPropagator) << "Detected case clash between" << file << "and" << realFileName;
-                return true;
+                return realFileName;
             }
         }
 #else
@@ -552,7 +550,7 @@ bool OwncloudPropagator::localFileNameClash(const QString &relFile)
         const QString fn = fileInfo.fileName();
         const QStringList list = fileInfo.dir().entryList({ fn });
         if (list.count() > 1 || (list.count() == 1 && list[0] != fn)) {
-            return true;
+            return list[0];
         }
 #endif
     }

diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h
@@ -465,8 +465,9 @@ class OWNCLOUDSYNC_EXPORT OwncloudPropagator : public QObject
 
     /** Check whether a download would clash with an existing file
      * in filesystems that are only case-preserving.
+     * Returns the path of the clashed file
      */
-    bool localFileNameClash(const QString &relfile);
+    Result<QString, bool> localFileNameClash(const QString &relfile);
 
     /** Check whether a file is properly accessible for upload.
      *

diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp
@@ -382,8 +382,8 @@ void PropagateDownloadFile::start()
     if (_item->_type == ItemTypeVirtualFile) {
         qCDebug(lcPropagateDownload) << "creating virtual file" << _item->_file;
         // do a klaas' case clash check.
-        if (propagator()->localFileNameClash(_item->_file)) {
-            done(SyncFileItem::NormalError, tr("File %1 can not be downloaded because of a local file name clash!").arg(QDir::toNativeSeparators(_item->_file)));
+        if (auto clash = propagator()->localFileNameClash(_item->_file)) {
+            done(SyncFileItem::NormalError, tr("File %1 can not be downloaded because of a local file name clash with %2!").arg(QDir::toNativeSeparators(_item->_file), QDir::toNativeSeparators(clash.get())));
             return;
         }
         auto r = vfs->createPlaceholder(*_item);
@@ -460,8 +460,8 @@ void PropagateDownloadFile::startDownload()
         return;
 
     // do a klaas' case clash check.
-    if (propagator()->localFileNameClash(_item->_file)) {
-        done(SyncFileItem::NormalError, tr("File %1 can not be downloaded because of a local file name clash!").arg(QDir::toNativeSeparators(_item->_file)));
+    if (auto clash = propagator()->localFileNameClash(_item->_file)) {
+        done(SyncFileItem::NormalError, tr("File %1 can not be downloaded because of a local file name clash with %2!").arg(QDir::toNativeSeparators(_item->_file), QDir::toNativeSeparators(clash.get())));
         return;
     }
 
@@ -892,8 +892,8 @@ void PropagateDownloadFile::downloadFinished()
 
     // In case of file name clash, report an error
     // This can happen if another parallel download saved a clashing file.
-    if (propagator()->localFileNameClash(_item->_file)) {
-        done(SyncFileItem::NormalError, tr("File %1 cannot be saved because of a local file name clash!").arg(QDir::toNativeSeparators(_item->_file)));
+    if (auto clash = propagator()->localFileNameClash(_item->_file)) {
+        done(SyncFileItem::NormalError, tr("File %1 cannot be saved because of a local file name clash with %2!").arg(QDir::toNativeSeparators(_item->_file), QDir::toNativeSeparators(clash.get())));
         return;
     }
 

diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp
@@ -95,8 +95,8 @@ void PropagateLocalRemove::start()
     const QString filename = propagator()->fullLocalPath(_item->_file);
     qCDebug(lcPropagateLocalRemove) << filename;
 
-    if (propagator()->localFileNameClash(_item->_file)) {
-        done(SyncFileItem::NormalError, tr("Could not remove %1 because of a local file name clash").arg(QDir::toNativeSeparators(filename)));
+    if (auto clash = propagator()->localFileNameClash(_item->_file)) {
+        done(SyncFileItem::NormalError, tr("Could not remove %1 because of a local file name clash with %2!").arg(QDir::toNativeSeparators(filename), QDir::toNativeSeparators(clash.get())));
         return;
     }
 
@@ -156,9 +156,9 @@ void PropagateLocalMkdir::start()
         }
     }
 
-    if (Utility::fsCasePreserving() && propagator()->localFileNameClash(_item->_file)) {
+    if (auto clash = propagator()->localFileNameClash(_item->_file)) {
         qCWarning(lcPropagateLocalMkdir) << "New folder to create locally already exists with different case:" << _item->_file;
-        done(SyncFileItem::NormalError, tr("Attention, possible case sensitivity clash with %1").arg(newDirStr));
+        done(SyncFileItem::NormalError, tr("Attention, possible case sensitivity clash of %1 with %2").arg(QDir::toNativeSeparators(clash.get()), newDirStr));
         return;
     }
     emit propagator()->touchedFile(newDirStr);