UI: Improves texts displayed to the user when picking the wrong location for syncing

src/common/vfs.cpp

@@ -1,4 +1,4 @@
 /*
  * Copyright (C) by Dominik Schmidt <[email protected]>
  *
  * This library is free software; you can redistribute it and/or
@@ -16,7 +16,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
 #include "config.h"
 #include "vfs.h"
 #include "plugin.h"
 #include "version.h"
@@ -71,16 +71,16 @@
 #ifdef Q_OS_WIN
     if (mode == Mode::WindowsCfApi) {
         const auto info = QFileInfo(path);
-        if (QDir(info.canonicalFilePath()).isRoot()) {
-            return tr("The Virtual filesystem feature does not support a drive as sync root");
+        if (QDir(info.canonicalPath()).isRoot()) {
+            return tr("Please choose a different location. %1 is a drive. It doesn't support virtual files.").arg(path);
         }
-        const auto fs = FileSystem::fileSystemForPath(info.absoluteFilePath());
-        if (fs != QLatin1String("NTFS")) {
-            return tr("The Virtual filesystem feature requires a NTFS file system, %1 is using %2").arg(path, fs);
+        if (const auto fileSystemForPath = FileSystem::fileSystemForPath(info.absoluteFilePath());
+            fileSystemForPath != QLatin1String("NTFS")) {
+            return tr("Please choose a different location. %1 isn't a NTFS file system. It doesn't support virtual files.").arg(path);
         }
         const auto type = GetDriveTypeW(reinterpret_cast<const wchar_t *>(QDir::toNativeSeparators(info.absoluteFilePath().mid(0, 3)).utf16()));
         if (type == DRIVE_REMOTE) {
-            return tr("The Virtual filesystem feature is not supported on network drives");
+            return tr("Please choose a different location. %1 is a network drive. It doesn't support virtual files.").arg(path);
         }
     }
 #else

src/gui/folder.cpp

@@ -13,7 +13,7 @@
  * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
  * for more details.
  */
 #include "common/syncjournaldb.h"
 #include "config.h"
 
 #include "account.h"
@@ -196,12 +196,12 @@
     } else {
         QString error;
         // Check directory again
         if (!FileSystem::fileExists(_definition.localPath, fi)) {
-            error = tr("Local folder %1 does not exist.").arg(_definition.localPath);
+            error = tr("Please choose a different location. The folder %1 doesn't exist.").arg(_definition.localPath);
         } else if (!fi.isDir()) {
-            error = tr("%1 should be a folder but is not.").arg(_definition.localPath);
+            error = tr("Please choose a different location. %1 isn't a valid folder.").arg(_definition.localPath);
         } else if (!fi.isReadable()) {
-            error = tr("%1 is not readable.").arg(_definition.localPath);
+            error = tr("Please choose a different location. %1 isn't a readable folder.").arg(_definition.localPath);
         }
         if (!error.isEmpty()) {
             _syncResult.appendErrorString(error);

src/gui/folderman.cpp

@@ -1,4 +1,4 @@
 /*
  * Copyright (C) by Klaas Freitag <[email protected]>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -1799,39 +1799,41 @@
 static QString checkPathValidityRecursive(const QString &path)
 {
     if (path.isEmpty()) {
-        return FolderMan::tr("No valid folder selected!");
+        return FolderMan::tr("Please choose a different location. The selected folder isn't valid.");
     }
 
 #ifdef Q_OS_WIN
     Utility::NtfsPermissionLookupRAII ntfs_perm;
 #endif
     const QFileInfo selFile(path);
     if (numberOfSyncJournals(selFile.filePath()) != 0) {
-        return FolderMan::tr("The folder %1 is used in a folder sync connection!").arg(QDir::toNativeSeparators(selFile.filePath()));
+        return FolderMan::tr("Please choose a different location. %1 is already being used as a sync folder.").arg(QDir::toNativeSeparators(selFile.filePath()));
     }
 
     if (!FileSystem::fileExists(path)) {
         QString parentPath = selFile.dir().path();
-        if (parentPath != path)
+        if (parentPath != path) {
             return checkPathValidityRecursive(parentPath);
-        return FolderMan::tr("The selected path does not exist!");
+        }
+
+        return FolderMan::tr("Please choose a different location. The path %1 doesn't exist.").arg(QDir::toNativeSeparators(selFile.filePath()));
     }
 
     if (!FileSystem::isDir(path)) {
-        return FolderMan::tr("The selected path is not a folder!");
+        return FolderMan::tr("Please choose a different location. The path %1 isn't a folder.").arg(QDir::toNativeSeparators(selFile.filePath()));
     }
 
     #ifdef Q_OS_WIN
     if (!FileSystem::isWritable(path)) {
         // isWritable() doesn't cover all NTFS permissions
         // try creating and removing a test file, and make sure it is excluded from sync
         if (!Utility::canCreateFileInPath(path)) {
-            return FolderMan::tr("You have no permission to write to the selected folder!");
+            return FolderMan::tr("Please choose a different location. You don't have enough permissions to write to %1.", "folder location").arg(QDir::toNativeSeparators(selFile.filePath()));
         }
     }
     #else
     if (!FileSystem::isWritable(path)) {
-        return FolderMan::tr("You have no permission to write to the selected folder!");
+        return FolderMan::tr("Please choose a different location. You don't have enough permissions to write to %1.", "folder location").arg(QDir::toNativeSeparators(selFile.filePath()));
     }
     #endif
     return {};
@@ -1884,17 +1886,15 @@
         bool differentPaths = QString::compare(folderDir, userDir, cs) != 0;
         if (differentPaths && folderDir.startsWith(userDir, cs)) {
             result.first = FolderMan::PathValidityResult::ErrorContainsFolder;
-            result.second = tr("The local folder %1 already contains a folder used in a folder sync connection. "
-                              "Please pick another one!")
+            result.second = tr("Please choose a different location. %1 is already being used as a sync folder.")
                                 .arg(QDir::toNativeSeparators(path));
             return result;
         }
 
         if (differentPaths && userDir.startsWith(folderDir, cs)) {
             result.first = FolderMan::PathValidityResult::ErrorContainedInFolder;
-            result.second = tr("The local folder %1 is already contained in a folder used in a folder sync connection. "
-                      "Please pick another one!")
-                .arg(QDir::toNativeSeparators(path));
+            result.second = tr("Please choose a different location. %1 is already contained in a folder used as a sync folder.")
+                                .arg(QDir::toNativeSeparators(path));
             return result;
         }
 
@@ -1908,8 +1908,9 @@
 
             if (serverUrl == folderUrl) {
                 result.first = FolderMan::PathValidityResult::ErrorNonEmptyFolder;
-                result.second = tr("There is already a sync from the server to this local folder. "
-                          "Please pick another local folder!");
+                result.second = tr("Please choose a different location. %1 is already being used as a sync folder for %2.", "folder location, server url")
+                                    .arg(QDir::toNativeSeparators(path),
+                                         serverUrl.toString());
                 return result;
             }
         }

src/gui/folderwizard.cpp

@@ -1,4 +1,4 @@
 /*
  * Copyright (C) by Duncan Mac-Vicar P. <[email protected]>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -61,18 +61,18 @@
 
 QString FormatWarningsWizardPage::formatWarnings(const QStringList &warnings) const
 {
-    QString ret;
+    QString formattedWarning;
     if (warnings.count() == 1) {
-        ret = tr("<b>Warning:</b> %1").arg(warnings.first());
+        formattedWarning = warnings.first();
     } else if (warnings.count() > 1) {
-        ret = tr("<b>Warning:</b>") + " <ul>";
-        Q_FOREACH (QString warning, warnings) {
-            ret += QString::fromLatin1("<li>%1</li>").arg(warning);
+        formattedWarning = "<ul>";
+        for (const auto &warning : warnings) {
+            formattedWarning += QString::fromLatin1("<li>%1</li>").arg(warning);
         }
-        ret += "</ul>";
+        formattedWarning += "</ul>";
     }
 
-    return ret;
+    return formattedWarning;
 }
 
 FolderWizardLocalPath::FolderWizardLocalPath(const AccountPtr &account)
@@ -484,34 +484,39 @@
 
 bool FolderWizardRemotePath::isComplete() const
 {
-    if (!_ui.folderTreeWidget->currentItem())
+    if (!_ui.folderTreeWidget->currentItem()) {
         return false;
+    }
 
-    QStringList warnStrings;
-    QString dir = _ui.folderTreeWidget->currentItem()->data(0, Qt::UserRole).toString();
-    if (!dir.startsWith(QLatin1Char('/'))) {
-        dir.prepend(QLatin1Char('/'));
+    auto targetPath = _ui.folderTreeWidget->currentItem()->data(0, Qt::UserRole).toString();
+    if (!targetPath.startsWith(QLatin1Char('/'))) {
+        targetPath.prepend(QLatin1Char('/'));
     }
-    wizard()->setProperty("targetPath", dir);
+    wizard()->setProperty("targetPath", targetPath);
 
-    Folder::Map map = FolderMan::instance()->map();
-    Folder::Map::const_iterator i = map.constBegin();
-    for (i = map.constBegin(); i != map.constEnd(); i++) {
-        auto *f = static_cast<Folder *>(i.value());
-        if (f->accountState()->account() != _account) {
+    for (const auto folder : std::as_const(FolderMan::instance()->map())) {
+        if (folder->accountState()->account() != _account) {
             continue;
         }
-        QString curDir = f->remotePathTrailingSlash();
-        if (QDir::cleanPath(dir) == QDir::cleanPath(curDir)) {
-            warnStrings.append(tr("This folder is already being synced."));
-        } else if (dir.startsWith(curDir)) {
-            warnStrings.append(tr("You are already syncing <i>%1</i>, which is a parent folder of <i>%2</i>.").arg(Utility::escape(curDir), Utility::escape(dir)));
-        } else if (curDir.startsWith(dir)) {
-            warnStrings.append(tr("You are already syncing <i>%1</i>, which is a subfolder of <i>%2</i>.").arg(Utility::escape(curDir), Utility::escape(dir)));
+
+        const auto remoteDir = folder->remotePathTrailingSlash();
+        const auto localDir = folder->cleanPath();
+        if (QDir::cleanPath(targetPath) == QDir::cleanPath(remoteDir)) {
+            showWarn(tr("Please choose a different location. %1 is already being synced to %2.").arg(Utility::escape(remoteDir), Utility::escape(localDir)));
+            break;
+        }
+
+        if (targetPath.startsWith(remoteDir)) {
+            showWarn(tr("Please choose a different location. %1 is already being synced to %2.").arg(Utility::escape(targetPath), Utility::escape(localDir)));
+            break;
+        }
+
+        if (remoteDir.startsWith(targetPath)) {
+            showWarn(tr("Please choose a different location. %1 is already being synced to %2.").arg(Utility::escape(remoteDir), Utility::escape(localDir)));
+            break;
         }
     }
 
-    showWarn(formatWarnings(warnStrings));
     return true;
 }
 
@@ -625,7 +630,10 @@
     if (useVirtualFiles) {
         const auto availability = Vfs::checkAvailability(wizard()->field(QStringLiteral("sourceFolder")).toString(), mode);
         if (!availability) {
-            auto msg = new QMessageBox(QMessageBox::Warning, tr("Virtual files are not available for the selected folder"), availability.error(), QMessageBox::Ok, this);
+            auto msg = new QMessageBox(QMessageBox::Warning,
+                                       tr("Virtual files are not supported at the selected location"),
+                                       availability.error(),
+                                       QMessageBox::Ok, this);
             msg->setAttribute(Qt::WA_DeleteOnClose);
             msg->open();
             return false;

src/gui/wizard/owncloudadvancedsetuppage.cpp

@@ -1,4 +1,4 @@
 /*
  * Copyright (C) by Klaas Freitag <[email protected]>
  * Copyright (C) by Krzesimir Nowak <[email protected]>
  *
@@ -13,7 +13,7 @@
  * for more details.
  */
 
 #include <QDir>
 #include <QFileDialog>
 #include <QUrl>
 #include <QTimer>
@@ -395,7 +395,10 @@
     if (useVirtualFileSync()) {
         const auto availability = Vfs::checkAvailability(localFolder(), bestAvailableVfsMode());
         if (!availability) {
-            auto msg = new QMessageBox(QMessageBox::Warning, tr("Virtual files are not available for the selected folder"), availability.error(), QMessageBox::Ok, this);
+            auto msg = new QMessageBox(QMessageBox::Warning,
+                                       tr("Virtual files are not supported at the selected location"),
+                                       availability.error(),
+                                       QMessageBox::Ok, this);
             msg->setAttribute(Qt::WA_DeleteOnClose);
             msg->open();
             return false;

test/testfolderman.cpp

@@ -1,11 +1,11 @@
 /*
  *    This software is in the public domain, furnished "as is", without technical
  *    support, and with no warranty, express or implied, as to its usefulness for
  *    any purpose.
  *
  */
 
 #include <qglobal.h>
 #include <QTemporaryDir>
 #include <QtTest>
 
@@ -313,8 +313,8 @@
         // The following both fail because they are already sync folders even if for another account
         QUrl url3("http://anotherexample.org");
         url3.setUserName("dummy");
-        QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1", url3).second, QString("The folder %1 is used in a folder sync connection!").arg(QDir::toNativeSeparators(dirPath + "/sub/ownCloud1")));
-        QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2", url3).second, QString("The folder %1 is used in a folder sync connection!").arg(QDir::toNativeSeparators(dirPath + "/ownCloud2")));
+        QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1", url3).second, QString("Please choose a different location. %1 is already being used as a sync folder.").arg(QDir::toNativeSeparators(dirPath + "/sub/ownCloud1")));
+        QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2", url3).second, QString("Please choose a different location. %1 is already being used as a sync folder.").arg(QDir::toNativeSeparators(dirPath + "/ownCloud2")));
 
         QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath).second.isNull());
         QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/folder").second.isNull());
@@ -337,7 +337,7 @@
         // link 3 points to an existing sync folder. To make it fail, the account must be the same
         QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3", url2).second.isNull());
         // while with a different account, this is fine
-        QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/link3", url3).second, QString("The folder %1 is used in a folder sync connection!").arg(dirPath + "/link3"));
+        QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/link3", url3).second, QString("Please choose a different location. %1 is already being used as a sync folder.").arg(dirPath + "/link3"));
 
         QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link4").second.isNull());
         QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3/folder").second.isNull());