Address PR comments.
authorCamila <hello@camila.codes>
Mon, 23 Jan 2023 16:30:59 +0000 (16:30 +0000)
committerMatthieu Gallien <matthieu_gallien@yahoo.fr>
Mon, 30 Jan 2023 08:44:17 +0000 (09:44 +0100)
Signed-off-by: Camila <hello@camila.codes>
src/gui/application.cpp
src/libsync/configfile.cpp
src/libsync/configfile.h

index a6a2c281b665784899826e571e2355e2b6094594..b20a60117301eaea8d9c690143b4b66dcb969664 100644 (file)
@@ -145,29 +145,28 @@ bool Application::configVersionMigration()
 
     // We want to message the user either for destructive changes,
     // or if we're ignoring something and the client version changed.
-    auto warningMessage = !deleteKeys.isEmpty() || (!ignoreKeys.isEmpty() && versionChanged);
+    const auto showWarning = !deleteKeys.isEmpty() || (!ignoreKeys.isEmpty() && versionChanged);
 
-    if (!versionChanged && !warningMessage) {
+    if (!versionChanged && !showWarning) {
         return true;
     }
 
     // back up all old config file
     QStringList backupFilesList;
-    QDir directory(configFile.configPath());
-    const auto anyConfigFileList = directory.entryInfoList({"*.cfg"}, QDir::Files);
-    for (const auto &file : anyConfigFileList) {
-        const auto fileName = file.fileName();
-        backupFilesList.append(configFile.backup(fileName));
-        if (file.baseName() != APPLICATION_CONFIG_NAME) {
-            const auto newConfig = configFile.configFile();
-            const auto oldConfig = file.filePath();
-            if (!QFile::rename(oldConfig, newConfig)) {
-                qCWarning(lcApplication) << "Failed to rename configuration file from" << oldConfig << "to" << newConfig;
+    QDir configDir(configFile.configPath());
+    const auto anyConfigFileNameList = configDir.entryInfoList({"*.cfg"}, QDir::Files);
+    for (const auto &oldConfig : anyConfigFileNameList) {
+        const auto oldConfigFileName = oldConfig.fileName();
+        const auto newConfigFileName = configFile.configFile();
+        backupFilesList.append(configFile.backup(oldConfigFileName));
+        if (oldConfigFileName != newConfigFileName) {
+            if (!QFile::rename(oldConfig.filePath(), newConfigFileName)) {
+                qCWarning(lcApplication) << "Failed to rename configuration file from" << oldConfigFileName << "to" << newConfigFileName;
             }
         }
     }
 
-    if (warningMessage || backupFilesList.count() > 0) {
+    if (showWarning || backupFilesList.count() > 0) {
         QString boldMessage;
         if (!deleteKeys.isEmpty()) {
             boldMessage = tr("Continuing will mean <b>deleting these settings</b>.");
index 631551a5e23143b8dfec23048ec41bc8d85d1ee0..529dabf77c9369086366e4b5a26735ea780afe14 100644 (file)
@@ -430,9 +430,9 @@ QString ConfigFile::excludeFileFromSystem()
     return fi.absoluteFilePath();
 }
 
-QString ConfigFile::backup(const QString fileName) const
+QString ConfigFile::backup(const QString &fileName) const
 {
-    QString baseFile = configPath() + fileName;
+    const QString baseFilePath = configPath() + fileName;
     auto versionString = clientVersionString();
 
     if (!versionString.isEmpty()) {
@@ -441,18 +441,16 @@ QString ConfigFile::backup(const QString fileName) const
 
     QString backupFile =
         QString("%1.backup_%2%3")
-            .arg(baseFile)
+            .arg(baseFilePath)
             .arg(QDateTime::currentDateTime().toString("yyyyMMdd_HHmmss"))
             .arg(versionString);
 
     // If this exact file already exists it's most likely that a backup was
     // already done. (two backup calls directly after each other, potentially
     // even with source alterations in between!)
-    QFile f(baseFile);
-
-    // QFile does not overwrite it
-    if(!f.copy(backupFile)) {
-        qCWarning(lcConfigFile) << "Failed to create a backup of the config file" << baseFile;
+    // QFile does not overwrite backupFile
+    if(!QFile::copy(baseFilePath, backupFile)) {
+        qCWarning(lcConfigFile) << "Failed to create a backup of the config file" << baseFilePath;
     }
 
     return backupFile;
index c0f1267369e97058f1751f7989581886ec731da3..a0c7f8b6df0e105a6ba2e12cd8175ff273d9ca42 100644 (file)
@@ -53,7 +53,7 @@ public:
      *
      * Returns the path of the new backup.
      */
-    [[nodiscard]] QString backup(const QString fileName) const;
+    [[nodiscard]] QString backup(const QString &fileName) const;
 
     bool exists();