Gui: Migrate old macro dir if needed

src/Gui/Dialogs/DlgVersionMigrator.cpp

@@ -38,6 +38,7 @@
 #include <string>
 #include <ranges>
 #include <cstdlib>
+#include <filesystem>
 
 #include "DlgVersionMigrator.h"
 #include "SplitButton.h"
@@ -225,57 +226,112 @@ Q_SIGNALS:
     void cancelled();
 };
 
-class PathMigrationWorker: public QObject
+PathMigrationWorker::PathMigrationWorker(std::string configDir, std::string userAppDir, int major, int minor)
+    : _configDir(std::move(configDir))
+    , _userAppDir(std::move(userAppDir))
+    , _major(major)
+    , _minor(minor)
+{}
+
+void PathMigrationWorker::run()
 {
-    Q_OBJECT
-
-public:
-    void run()
-    {
-        try {
-            App::GetApplication().GetUserParameter().SaveDocument();
-            App::Application::directories()->migrateAllPaths(
-                {App::Application::directories()->getUserAppDataDir(),
-                 App::Application::directories()->getUserConfigPath()}
-            );
-
-            // In addition to migrating the actual files, there is a parameter that might be
-            // recording userMacroDir() which must be updated if it stores the old default
-            auto macroDir = App::GetApplication()
-                                .GetParameterGroupByPath("User parameter:BaseApp/Preferences/Macro")
-                                ->GetASCII("MacroPath", App::Application::getUserMacroDir().c_str());
-            std::filesystem::path chosenPath(macroDir);
-            std::filesystem::path userMacroDir(App::Application::getUserMacroDir());
-            if (chosenPath == userMacroDir) {
-                App::GetApplication()
-                    .GetParameterGroupByPath("User parameter:BaseApp/Preferences/Macro")
-                    ->RemoveASCII("MacroPath");
-            }
-
-            Q_EMIT(complete());
-        }
-        catch (const Base::Exception& e) {
-            Base::Console().error("Error migrating configuration data: %s\n", e.what());
-            Q_EMIT(failed());
-        }
-        catch (const std::exception& e) {
-            Base::Console().error("Unrecognized error migrating configuration data: %s\n", e.what());
-            Q_EMIT(failed());
-        }
-        catch (...) {
-            Base::Console().error("Error migrating configuration data\n");
-            Q_EMIT(failed());
-        }
-        Q_EMIT(finished());
+    try {
+        App::GetApplication().GetUserParameter().SaveDocument();
+        App::Application::directories()->migrateAllPaths({_userAppDir, _configDir});
+        replaceOccurrencesInPreferences();
+        Q_EMIT(complete());
     }
+    catch (const Base::Exception& e) {
+        Base::Console().error("Error migrating configuration data: %s\n", e.what());
+        Q_EMIT(failed());
+    }
+    catch (const std::exception& e) {
+        Base::Console().error("Unrecognized error migrating configuration data: %s\n", e.what());
+        Q_EMIT(failed());
+    }
+    catch (...) {
+        Base::Console().error("Error migrating configuration data\n");
+        Q_EMIT(failed());
+    }
+    Q_EMIT(finished());
+}
 
-Q_SIGNALS:
-    void finished();
+void PathMigrationWorker::replaceOccurrencesInPreferences()
+{
+    std::filesystem::path prefPath = locateNewPreferences();
+    std::map<std::string, std::string> replacements = {
+        {_configDir, generateNewUserAppPathString(_configDir)},
+        {_userAppDir, generateNewUserAppPathString(_userAppDir)}
+    };
 
-    void complete();
+    try {
+        std::ifstream prefFile(prefPath);
+        std::string contents(
+            (std::istreambuf_iterator<char>(prefFile)),
+            std::istreambuf_iterator<char>()
+        );
 
-    void failed();
-};
+        for (const auto& [oldString, newString] : replacements) {
+            replaceInContents(contents, oldString, newString);
+        }
+
+        std::ofstream newPrefFile(prefPath);
+        newPrefFile << contents;
+    }
+    catch (const std::exception& e) {
+        Base::Console().error("Error reading preferences file: %s\n", e.what());
+    }
+}
+
+std::filesystem::path PathMigrationWorker::locateNewPreferences() const
+{
+    std::filesystem::path path(_configDir);
+    if (path.filename().empty()) {
+        // Handle the case where the path was constructed from a std::string with a trailing /
+        path = path.parent_path();
+    }
+    fs::path newPath;
+
+    if (App::Application::directories()->isVersionedPath(path)) {
+        newPath = path.parent_path()
+            / App::ApplicationDirectories::versionStringForPath(_major, _minor);
+    }
+    else {
+        newPath = path / App::ApplicationDirectories::versionStringForPath(_major, _minor);
+    }
+    newPath /= "user.cfg";
+    return newPath;
+}
+
+std::string PathMigrationWorker::generateNewUserAppPathString(const std::string& oldPath) const
+{
+    std::filesystem::path newPath = Base::FileInfo::stringToPath(oldPath);
+    if (App::Application::directories()->isVersionedPath(newPath)) {
+        newPath = newPath.parent_path();
+    }
+    newPath /= App::ApplicationDirectories::versionStringForPath(_major, _minor);
+    std::string result = Base::FileInfo::pathToString(newPath);
+    if (oldPath.back() == std::filesystem::path::preferred_separator) {
+        result += std::filesystem::path::preferred_separator;
+    }
+    return result;
+}
+
+void PathMigrationWorker::replaceInContents(
+    std::string& contents,
+    const std::string& oldString,
+    const std::string& newString
+)
+{
+    if (oldString.empty()) {
+        return;
+    }
+    std::size_t pos = 0;
+    while ((pos = contents.find(oldString, pos)) != std::string::npos) {
+        contents.replace(pos, oldString.length(), newString);
+        pos += newString.length();
+    }
+}
 
 void DlgVersionMigrator::calculateMigrationSize()
 {
@@ -324,7 +380,12 @@ void DlgVersionMigrator::migrate()
 {
     hide();
     auto* workerThread = new QThread(mainWindow);
-    auto* worker = new PathMigrationWorker();
+    auto* worker = new PathMigrationWorker(
+        App::Application::getUserConfigPath(),
+        App::Application::getUserAppDataDir(),
+        std::stoi(App::Application::Config()["BuildVersionMajor"]),
+        std::stoi(App::Application::Config()["BuildVersionMinor"])
+    );
     worker->moveToThread(workerThread);
     connect(workerThread, &QThread::started, worker, &PathMigrationWorker::run);
     connect(worker, &PathMigrationWorker::finished, workerThread, &QThread::quit);

src/Gui/Dialogs/DlgVersionMigrator.h

@@ -66,6 +66,65 @@ private:
     void restart(const QString& message);
 };
 
+
+class GuiExport PathMigrationWorker: public QObject
+{
+    Q_OBJECT
+
+public:
+    PathMigrationWorker(std::string configDir, std::string userAppDir, int major, int minor);
+    void run();
+
+Q_SIGNALS:
+    void finished();
+    void complete();
+    void failed();
+
+protected:
+    /**
+     * @brief Find any occurrence of the original config and userAppDir paths in the new copy of the
+     * config file and replace them with updated versions.
+     */
+    void replaceOccurrencesInPreferences();
+
+    /**
+     * @brief Locate the new user config file
+     *
+     * After it's been moved, this method figures out the path to the new user.cfg file. It does not
+     * verify the existence of the file, just determines where it *should* be.
+     *
+     * @return The path to the new version of user.cfg.
+     */
+    std::filesystem::path locateNewPreferences() const;
+
+    /**
+     * @brief Given an old path, figure out what the new versioned one would be
+     *
+     * @param oldPath  The old path
+     * @return An equivalent new versioned path
+     */
+    std::string generateNewUserAppPathString(const std::string& oldPath) const;
+
+    /**
+     * @brief Replace all occurrences of oldString with newString, modifying contents in place.
+     *
+     * @param[inout] contents The string to do the replacement in
+     * @param[in] oldString The string to search for
+     * @param[in] newString The new string to put in place of oldString
+     */
+    static void replaceInContents(
+        std::string& contents,
+        const std::string& oldString,
+        const std::string& newString
+    );
+
+private:
+    std::string _configDir;
+    std::string _userAppDir;
+    int _major;
+    int _minor;
+};
+
 }  // namespace Dialog
 }  // namespace Gui
 

src/Gui/StartupProcess.cpp

@@ -58,6 +58,7 @@
 #include "FreeCADStyle.h"
 
 #include <App/Application.h>
+#include <App/ApplicationDirectories.h>
 #include <Base/Console.h>
 
 
@@ -589,6 +590,42 @@ void StartupPostProcess::checkParameters()
             "Continue with an empty configuration that won't be saved.\n"
         );
     }
+
+    // Prior to the release of v1.1, MacroPath was stored in the config file, even if it was just
+    // set to the default value. However, for a short time during the development of v1.1, when
+    // that directory was migrated, the config value was not updated. This code block corrects for
+    // that oversight by detecting when the path is set to the old default, and updates it to the
+    // new one -- but only once, so that if the user does manually set the path to the old default
+    // intentionally after this is run, it doesn't undo that action.
+    auto macroPrefs = App::GetApplication().GetParameterGroupByPath(
+        "User parameter:BaseApp/Preferences/Macro"
+    );
+    auto v11MacroLocationChecked = macroPrefs->GetBool("MacroPathCheckedForMigrationTov1-1", false);
+    if (!v11MacroLocationChecked) {
+        std::filesystem::path newDefaultPath {App::Application::getUserMacroDir()};
+        if (newDefaultPath.filename().empty()) {
+            newDefaultPath = newDefaultPath.parent_path();
+        }
+        int major = std::stoi(App::Application::Config()["BuildVersionMajor"]);
+        int minor = std::stoi(App::Application::Config()["BuildVersionMinor"]);
+        auto versionString = App::ApplicationDirectories::versionStringForPath(major, minor);
+        if (newDefaultPath.filename() == "Macro"
+            && (newDefaultPath.parent_path().filename() == versionString)) {
+            std::filesystem::path oldDefaultPath {newDefaultPath.parent_path().parent_path() / "Macro"};
+            std::filesystem::path macroDir
+                = macroPrefs->GetASCII("MacroPath", newDefaultPath.string().c_str());
+            if (macroDir.filename().empty()) {
+                macroDir = macroDir.parent_path();
+            }
+            if (macroDir == oldDefaultPath) {
+                Base::Console().warning(
+                    "Removing 'MacroPath' parameter in order to default to the new versioned path\n"
+                );
+                macroPrefs->RemoveASCII("MacroPath");
+            }
+        }
+        macroPrefs->SetBool("MacroPathCheckedForMigrationTov1-1", true);
+    }
 }
 
 void StartupPostProcess::checkVersionMigration() const

tests/src/Gui/CMakeLists.txt

@@ -1,5 +1,7 @@
 # SPDX-License-Identifier: LGPL-2.1-or-later
 
+add_subdirectory(Dialogs)
+
 # Standard C++ GTest tests
 add_executable(Gui_tests_run
         Assistant.cpp

tests/src/Gui/Dialogs/CMakeLists.txt

@@ -0,0 +1 @@
+setup_qt_test(DlgVersionMigrator)

tests/src/Gui/Dialogs/DlgVersionMigrator.cpp

@@ -0,0 +1,265 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+// SPDX-FileNotice: Part of the FreeCAD project.
+
+/******************************************************************************
+ *                                                                            *
+ *   © 2026 The FreeCAD Project Association AISBL                             *
+ *                                                                            *
+ *   FreeCAD is free software: you can redistribute it and/or modify          *
+ *   it under the terms of the GNU Lesser General Public License as           *
+ *   published by the Free Software Foundation, either version 2.1            *
+ *   of the License, or (at your option) any later version.                   *
+ *                                                                            *
+ *   FreeCAD is distributed in the hope that it will be useful,               *
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty              *
+ *   of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.                  *
+ *   See the GNU Lesser General Public License for more details.              *
+ *                                                                            *
+ *   You should have received a copy of the GNU Lesser General Public         *
+ *   License along with FreeCAD. If not, see https://www.gnu.org/licenses     *
+ *                                                                            *
+ ******************************************************************************/
+
+#include <QDebug>
+#include <QTest>
+#include <fstream>
+#include <iostream>
+
+#include <App/Application.h>
+
+#include "Gui/Dialogs/DlgVersionMigrator.h"
+
+#include "App/ApplicationDirectories.h"
+
+#include <src/App/InitApplication.h>
+
+
+class TestablePathMigrationWorker: public Gui::Dialog::PathMigrationWorker
+{
+    using Gui::Dialog::PathMigrationWorker::PathMigrationWorker;
+
+public:
+    TestablePathMigrationWorker(std::string userConfigDir, std::string userAppDir, int major, int minor)
+        : PathMigrationWorker(userConfigDir, userAppDir, major, minor)
+        , testConfigDir(userConfigDir)
+        , testUserAppDir(userAppDir) {};
+
+    void testableReplaceOccurrencesInPreferences()
+    {
+        replaceOccurrencesInPreferences();
+    }
+
+    std::filesystem::path testableLocateNewPreferences()
+    {
+        return locateNewPreferences();
+    }
+
+    std::string testableGenerateNewUserAppPathString(const std::string& oldPath)
+    {
+        return generateNewUserAppPathString(oldPath);
+    }
+
+    static void testableReplaceInContents(
+        std::string& contents,
+        const std::string& oldString,
+        const std::string& newString
+    )
+    {
+        replaceInContents(contents, oldString, newString);
+    }
+
+    std::string getConfigDir()
+    {
+        return testConfigDir;
+    }
+
+    std::string getUserAppDir()
+    {
+        return testUserAppDir;
+    }
+
+private:
+    std::string testConfigDir;
+    std::string testUserAppDir;
+};
+
+class testPathMigrationWorker final: public QObject
+{
+    Q_OBJECT
+
+public:
+    testPathMigrationWorker()
+    {
+        tests::initApplication();
+    }
+
+private Q_SLOTS:
+
+    void init()  // NOLINT
+    {}
+
+    void cleanup()  // NOLINT
+    {}
+
+    void replaceInContents_data()  // NOLINT
+    {
+        QTest::addColumn<QString>("contents");
+        QTest::addColumn<QString>("oldString");
+        QTest::addColumn<QString>("newString");
+        QTest::addColumn<QString>("expected");
+
+        // No-op / no matches
+        QTest::newRow("empty-contents") << "" << "a" << "b" << "";
+        QTest::newRow("no-match") << "abcdef" << "x" << "y" << "abcdef";
+
+        // Single match
+        QTest::newRow("single-match-middle") << "abcXYZdef" << "XYZ" << "Q" << "abcQdef";
+        QTest::newRow("single-match-begin") << "XYZdef" << "XYZ" << "Q" << "Qdef";
+        QTest::newRow("single-match-end") << "abcXYZ" << "XYZ" << "Q" << "abcQ";
+
+        // Multiple matches
+        QTest::newRow("multiple-separated") << "a1a2a3" << "a" << "b" << "b1b2b3";
+        QTest::newRow("multiple-words") << "foo bar foo" << "foo" << "x" << "x bar x";
+
+        // Adjacent / overlapping-looking cases (should replace non-overlapping occurrences)
+        QTest::newRow("adjacent") << "aaaa" << "aa" << "b" << "bb";  // "aa" + "aa"
+        QTest::newRow("pattern-repeats") << "ababab" << "ab" << "x" << "xxx";
+
+        // oldString longer/shorter than newString
+        QTest::newRow("shorten") << "abc123abc" << "abc" << "a" << "a123a";
+        QTest::newRow("expand") << "a-b-a-b" << "a" << "LONG" << "LONG-b-LONG-b";
+
+        // oldString == newString
+        QTest::newRow("old-equals-new") << "same same" << "same" << "same" << "same same";
+
+        // newString contains oldString (avoid infinite loop)
+        // Expected behavior for a correct implementation: only original matches are replaced.
+        QTest::newRow("new-contains-old") << "aa" << "a" << "aa" << "aaaa";
+
+        // oldString empty: treat empty oldString as no-op.
+        QTest::newRow("empty-oldString-noop") << "abc" << "" << "X" << "abc";
+
+        // newString empty: delete occurrences
+        QTest::newRow("delete-occurrences") << "bananas" << "na" << "" << "bas";
+    }
+
+    void replaceInContents()  // NOLINT
+    {
+        QFETCH(QString, contents);
+        QFETCH(QString, oldString);
+        QFETCH(QString, newString);
+        QFETCH(QString, expected);
+
+        std::string c = contents.toStdString();
+        const std::string oldS = oldString.toStdString();
+        const std::string newS = newString.toStdString();
+
+        TestablePathMigrationWorker::testableReplaceInContents(c, oldS, newS);
+
+        QCOMPARE(QString::fromStdString(c), expected);
+    }
+
+    void replaceInContents_idempotent_when_old_not_present()  // NOLINT
+    {
+        std::string c = "no matches here";
+        const std::string oldS = "ZZZ";
+        const std::string newS = "YYY";
+
+        TestablePathMigrationWorker::testableReplaceInContents(c, oldS, newS);
+        const std::string once = c;
+
+        // run twice: should stay identical
+        TestablePathMigrationWorker::testableReplaceInContents(c, oldS, newS);
+        QCOMPARE(QString::fromStdString(c), QString::fromStdString(once));
+    }
+
+    std::unique_ptr<TestablePathMigrationWorker> makeWorker(int major, int minor)
+    {
+        std::string userConfigDir = Base::FileInfo::pathToString(
+            std::filesystem::temp_directory_path() / "Config"
+        );
+        std::string userAppDataDir = Base::FileInfo::pathToString(
+            std::filesystem::temp_directory_path() / "AppData"
+        );
+        return std::make_unique<TestablePathMigrationWorker>(userConfigDir, userAppDataDir, major, minor);
+    }
+
+    void generateNewUserAppPathString_no_version_now()
+    {
+        auto worker = makeWorker(1, 1);
+        std::filesystem::path testPath = std::filesystem::temp_directory_path() / "foo" / "bar";
+        std::string oldPath = Base::FileInfo::pathToString(testPath);
+        std::string newPath = worker->testableGenerateNewUserAppPathString(oldPath);
+
+
+        std::string expectedAddition = App::ApplicationDirectories::versionStringForPath(1, 1);
+
+        std::string expectedPath = Base::FileInfo::pathToString(testPath / expectedAddition);
+        QCOMPARE(newPath, expectedPath);
+    }
+
+    void generateNewUserAppPathString_version_in_current()
+    {
+        auto worker = makeWorker(1, 1);
+        std::filesystem::path testPath = std::filesystem::temp_directory_path() / "foo" / "bar"
+            / "v1-0";
+        std::string oldPath = Base::FileInfo::pathToString(testPath);
+        std::string newPath = worker->testableGenerateNewUserAppPathString(oldPath);
+
+        std::string expectedAddition = App::ApplicationDirectories::versionStringForPath(1, 1);
+
+        std::string expectedPath = Base::FileInfo::pathToString(
+            testPath.parent_path() / expectedAddition
+        );
+        QCOMPARE(newPath, expectedPath);
+    }
+
+    void locateNewPreferences()
+    {
+        auto worker = makeWorker(1, 1);
+        std::string configDir = worker->getConfigDir();
+        std::filesystem::path expectedNewPreferences = Base::FileInfo::stringToPath(configDir);
+        expectedNewPreferences = expectedNewPreferences / "v1-1" / "user.cfg";
+        std::string expectedNewPreferencesString = Base::FileInfo::pathToString(expectedNewPreferences);
+        std::string actualNewPreferencesString = Base::FileInfo::pathToString(
+            worker->testableLocateNewPreferences()
+        );
+        QCOMPARE(actualNewPreferencesString, expectedNewPreferencesString);
+    }
+
+    static void _writePreferencesTestData(std::ostream& stream, const std::string& pathToInject)
+    {
+        stream << "<FCParamGroup Name=\"Macro\">\n"
+               << "  <FCBool Name=\"LocalEnvironment\" Value=\"1\"/>\n"
+               << "  <FCText Name=\"MacroPath\">" << pathToInject << "</FCText>";
+    }
+
+    void replaceOccurrencesInPreferences()
+    {
+        auto worker = makeWorker(1, 1);
+        std::string configDir = worker->getConfigDir();
+        std::string userAppDir = worker->getUserAppDir();
+        std::filesystem::path newPrefs = Base::FileInfo::stringToPath(configDir);
+        newPrefs = newPrefs / "v1-1" / "user.cfg";
+        std::filesystem::create_directories(newPrefs.parent_path());
+        std::ofstream prefs(newPrefs);
+        _writePreferencesTestData(prefs, userAppDir);
+        prefs.close();
+        worker->testableReplaceOccurrencesInPreferences();
+        std::ifstream loadedPrefs(newPrefs, std::ios::in | std::ios::binary);
+        std::string modifiedPrefsData {
+            std::istreambuf_iterator<char>(loadedPrefs),
+            std::istreambuf_iterator<char>()
+        };
+        loadedPrefs.close();
+        auto expectedNewDir = Base::FileInfo::pathToString(
+            Base::FileInfo::stringToPath(userAppDir) / "v1-1"
+        );
+        Q_ASSERT(modifiedPrefsData.find(expectedNewDir) != std::string::npos);
+    }
+};
+
+
+QTEST_MAIN(testPathMigrationWorker)
+
+#include "DlgVersionMigrator.moc"