Refactor FileChanges to be immutable and update usage

- Make FileChanges immutable and produced only by FileChangesBuilder
- Move push_* methods to private, expose only for_each and special
  members
- Update CMake and tests to use FileChangesBuilder where needed
- Improve documentation to clarify usage and construction
- Remove push_* symbols from public ABI

Author: tarek-y-ismail

include/miral/miral/live_config.h

@@ -30,12 +30,10 @@
 
 namespace miral::live_config
 {
-/// A pimpl'd, ordered batch of file-system change events.
+/// An immutable, ordered batch of file-system change events.
 ///
-/// Callers build the batch with push_unchanged(), push_moved_or_new(),
-/// push_modified(), push_dropped(), and push_renamed(), then pass it to
-/// IniFileWithOverrides::load(). The store consumes the batch by calling
-/// for_each() with one callback per event kind.
+/// Produced internally by FileChangesBuilder and passed to IniFileWithOverrides::load().
+/// The store consumes the batch by calling for_each() with one callback per event kind.
 ///
 /// Events are dispatched in push order, so relative file priority is
 /// preserved across kinds.
@@ -60,6 +58,22 @@ class FileChanges
     using Renamed =
         std::move_only_function<void(std::filesystem::path const&, std::filesystem::path const&, std::istream&)>;
 
+    /// Iterate all events in push order, invoking the matching callback for each.
+    void for_each(
+        Load  unchanged,
+        Load  moved_or_new,
+        Load  modified,
+        Dropped dropped,
+        Renamed renamed) const;
+
+    ~FileChanges();
+
+    FileChanges(FileChanges const&) = default;
+    FileChanges& operator=(FileChanges const&) = default;
+
+private:
+    friend class FileChangesBuilder;
+
     FileChanges();
 
     auto push_unchanged(
@@ -81,19 +95,6 @@ class FileChanges
         std::filesystem::path new_filepath,
         std::function<std::unique_ptr<std::istream>()> stream) -> FileChanges&;
 
-    /// Iterate all events in push order, invoking the matching callback for each.
-    void for_each(
-        Load  unchanged,
-        Load  moved_or_new,
-        Load  modified,
-        Dropped dropped,
-        Renamed renamed) const;
-
-    ~FileChanges() = default;
-    FileChanges(FileChanges const&) = default;
-    FileChanges& operator=(FileChanges const&) = default;
-
-private:
     struct Self;
     std::shared_ptr<Self> self;
 };

src/miral/CMakeLists.txt

@@ -36,6 +36,8 @@ add_library(miral-internal STATIC
                                          window_info_defaults.h
     window_specification_internal.cpp    window_specification_internal.h
     basic_store.cpp                      basic_store.h
+    file_changes_builder.cpp             file_changes_builder.h
+    live_config.cpp                      ${miral_include}/miral/live_config.h
 )
 
 # Already implied by the linker's symbol version script, but can avoid accidents
@@ -148,8 +150,7 @@ add_library(miral-external OBJECT
     mousekeys_config.cpp                ${miral_include}/miral/mousekeys_config.h
     output_filter.cpp                   ${miral_include}/miral/output_filter.h
     simulated_secondary_click.cpp       ${miral_include}/miral/simulated_secondary_click.h
-    live_config.cpp                     ${miral_include}/miral/live_config.h
-    live_config_ini_file.cpp            ${miral_include}/miral/live_config_ini_file.h
+    live_config_ini_file.cpp             ${miral_include}/miral/live_config_ini_file.h
     ini_file_with_overrides.cpp         ${miral_include}/miral/ini_file_with_overrides.h
     hover_click.cpp                     ${miral_include}/miral/hover_click.h
     magnifier.cpp                       ${miral_include}/miral/magnifier.h

src/miral/file_changes_builder.cpp

@@ -0,0 +1,68 @@
+/*
+ * Copyright © Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 or 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "file_changes_builder.h"
+
+namespace mlc = miral::live_config;
+
+mlc::FileChangesBuilder::FileChangesBuilder() :
+    changes_{}
+{
+}
+
+auto mlc::FileChangesBuilder::push_unchanged(
+    std::filesystem::path filepath,
+    std::function<std::unique_ptr<std::istream>()> stream) -> FileChangesBuilder&
+{
+    changes_.push_unchanged(std::move(filepath), std::move(stream));
+    return *this;
+}
+
+auto mlc::FileChangesBuilder::push_moved_or_new(
+    std::filesystem::path filepath,
+    std::function<std::unique_ptr<std::istream>()> stream) -> FileChangesBuilder&
+{
+    changes_.push_moved_or_new(std::move(filepath), std::move(stream));
+    return *this;
+}
+
+auto mlc::FileChangesBuilder::push_modified(
+    std::filesystem::path filepath,
+    std::function<std::unique_ptr<std::istream>()> stream) -> FileChangesBuilder&
+{
+    changes_.push_modified(std::move(filepath), std::move(stream));
+    return *this;
+}
+
+auto mlc::FileChangesBuilder::push_dropped(std::filesystem::path filepath) -> FileChangesBuilder&
+{
+    changes_.push_dropped(std::move(filepath));
+    return *this;
+}
+
+auto mlc::FileChangesBuilder::push_renamed(
+    std::filesystem::path old_filepath,
+    std::filesystem::path new_filepath,
+    std::function<std::unique_ptr<std::istream>()> stream) -> FileChangesBuilder&
+{
+    changes_.push_renamed(std::move(old_filepath), std::move(new_filepath), std::move(stream));
+    return *this;
+}
+
+mlc::FileChangesBuilder::operator mlc::FileChanges const&() const
+{
+    return changes_;
+}

src/miral/file_changes_builder.h

@@ -0,0 +1,59 @@
+/*
+ * Copyright © Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 or 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef MIRAL_FILE_CHANGES_BUILDER_H
+#define MIRAL_FILE_CHANGES_BUILDER_H
+
+#include <miral/live_config.h>
+
+namespace miral::live_config
+{
+/// Internal builder for FileChanges batches.
+/// Only Mir's file-watching infrastructure should construct these;
+/// consumers receive a FileChanges const& and call for_each().
+class FileChangesBuilder
+{
+public:
+    FileChangesBuilder();
+
+    auto push_unchanged(
+        std::filesystem::path filepath,
+        std::function<std::unique_ptr<std::istream>()> stream) -> FileChangesBuilder&;
+
+    auto push_moved_or_new(
+        std::filesystem::path filepath,
+        std::function<std::unique_ptr<std::istream>()> stream) -> FileChangesBuilder&;
+
+    auto push_modified(
+        std::filesystem::path filepath,
+        std::function<std::unique_ptr<std::istream>()> stream) -> FileChangesBuilder&;
+
+    auto push_dropped(std::filesystem::path filepath) -> FileChangesBuilder&;
+
+    auto push_renamed(
+        std::filesystem::path old_filepath,
+        std::filesystem::path new_filepath,
+        std::function<std::unique_ptr<std::istream>()> stream) -> FileChangesBuilder&;
+
+    /// Implicit conversion so a builder can be passed directly to load(FileChanges const&).
+    operator FileChanges const&() const;
+
+private:
+    FileChanges changes_;
+};
+}
+
+#endif // MIRAL_FILE_CHANGES_BUILDER_H

src/miral/live_config.cpp

@@ -111,6 +111,8 @@ miral::live_config::FileChanges::FileChanges() :
 {
 }
 
+miral::live_config::FileChanges::~FileChanges() = default;
+
 auto miral::live_config::FileChanges::push_unchanged(
     std::filesystem::path filepath,
     std::function<std::unique_ptr<std::istream>()> stream) -> FileChanges&

src/miral/symbols.map

@@ -781,11 +781,6 @@ global:
   extern "C++" {
     miral::live_config::FileChanges::FileChanges*;
     miral::live_config::FileChanges::?FileChanges*;
-    miral::live_config::FileChanges::push_unchanged*;
-    miral::live_config::FileChanges::push_moved_or_new*;
-    miral::live_config::FileChanges::push_modified*;
-    miral::live_config::FileChanges::push_dropped*;
-    miral::live_config::FileChanges::push_renamed*;
     miral::live_config::FileChanges::for_each*;
     miral::live_config::IniFileWithOverrides::?IniFileWithOverrides*;
     miral::live_config::IniFileWithOverrides::IniFileWithOverrides*;

tests/miral/CMakeLists.txt

@@ -60,6 +60,7 @@ mir_add_wrapped_executable(miral-test-internal NOINSTALL
     test_hover_click.cpp
     test_locate_pointer.cpp
     basic_store.cpp
+    ini_file_with_overrides.cpp
     ${MIRAL_TEST_SOURCES}
 )
 
@@ -83,7 +84,6 @@ mir_add_wrapped_executable(miral-test NOINSTALL
     config_file.cpp
     live_config.cpp
     live_config_ini_file.cpp
-    ini_file_with_overrides.cpp
     magnifier.cpp
     output_filter.cpp
     sticky_keys.cpp

tests/miral/ini_file_with_overrides.cpp

@@ -15,6 +15,7 @@
  */
 
 #include <miral/ini_file_with_overrides.h>
+#include "file_changes_builder.h"
 
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
@@ -49,7 +50,7 @@ struct IniFileWithOverridesTest : Test
     using FileContents = std::vector<std::pair<std::string, std::filesystem::path>>;
     void load(FileContents contents_and_paths)
     {
-        mlc::FileChanges changes;
+        mlc::FileChangesBuilder changes;
         for (auto const& [contents, path] : contents_and_paths)
             changes.push_modified(
                 path,