Using Gio::FileMonitor for watching style changes

include/util/css_reload_helper.hpp

@@ -1,13 +1,13 @@
 #pragma once
 
-#include <atomic>
-#include <condition_variable>
 #include <functional>
-#include <mutex>
 #include <string>
-#include <thread>
 #include <vector>
 
+#include "glibmm/refptr.h"
+#include "giomm/file.h"
+#include "giomm/filemonitor.h"
+
 struct pollfd;
 
 namespace waybar {
@@ -15,12 +15,8 @@ class CssReloadHelper {
  public:
   CssReloadHelper(std::string cssFile, std::function<void()> callback);
 
-  ~CssReloadHelper();
-
   virtual void monitorChanges();
 
-  void stop();
-
  protected:
   std::vector<std::string> parseImports(const std::string& cssFile);
 
@@ -38,12 +34,16 @@ class CssReloadHelper {
 
   virtual std::string findPath(const std::string& filename);
 
+  void handleFileChange(
+        Glib::RefPtr<Gio::File> const& file,
+        Glib::RefPtr<Gio::File> const& other_type,
+        Gio::FileMonitorEvent event_type);
+
  private:
   std::string m_cssFile;
+
   std::function<void()> m_callback;
-  std::atomic<bool> m_running = false;
+
-  std::thread m_thread;
+  std::vector<std::tuple<Glib::RefPtr<Gio::FileMonitor>>> m_fileMonitors;
-  std::mutex m_mutex;
-  std::condition_variable m_cv;
 };
 }  // namespace waybar

src/util/css_reload_helper.cpp

@@ -8,8 +8,11 @@
 #include <fstream>
 #include <regex>
 #include <unordered_map>
+#include "glibmm/refptr.h"
+#include "giomm/file.h"
 
 #include "config.hpp"
+
 namespace {
 const std::regex IMPORT_REGEX(R"(@import\s+(?:url\()?(?:"|')([^"')]+)(?:"|')\)?;)");
 }
@@ -17,8 +20,6 @@ const std::regex IMPORT_REGEX(R"(@import\s+(?:url\()?(?:"|')([^"')]+)(?:"|')\)?;
 waybar::CssReloadHelper::CssReloadHelper(std::string cssFile, std::function<void()> callback)
     : m_cssFile(std::move(cssFile)), m_callback(std::move(callback)) {}
 
-waybar::CssReloadHelper::~CssReloadHelper() { stop(); }
-
 std::string waybar::CssReloadHelper::getFileContents(const std::string& filename) {
   if (filename.empty()) {
     return {};
@@ -34,21 +35,56 @@ std::string waybar::CssReloadHelper::getFileContents(const std::string& filename
 
 std::string waybar::CssReloadHelper::findPath(const std::string& filename) {
   // try path and fallback to looking relative to the config
+  std::string result;
   if (std::filesystem::exists(filename)) {
-    return filename;
+    result = filename;
+  } else {
+    result = Config::findConfigPath({filename}).value_or("");
   }
 
-  return Config::findConfigPath({filename}).value_or("");
+  // File monitor does not work with symlinks, so resolve them
+  if (std::filesystem::is_symlink(result)) {
+    result = std::filesystem::read_symlink(result);
+  }
+
+  return result;
 }
 
 void waybar::CssReloadHelper::monitorChanges() {
-  m_thread = std::thread([this] {
+  auto files = parseImports(m_cssFile);
-    m_running = true;
+  for (const auto& file : files) {
-    while (m_running) {
+    auto gioFile = Gio::File::create_for_path(file);
-      auto files = parseImports(m_cssFile);
+    if (!gioFile) {
-      watchFiles(files);
+      spdlog::error("Failed to create file for path: {}", file);
+      continue;
     }
-  });
+
+    auto fileMonitor = gioFile->monitor_file();
+    if (!fileMonitor) {
+      spdlog::error("Failed to create file monitor for path: {}", file);
+      continue;
+    }
+
+    auto connection = fileMonitor->signal_changed().connect(
+        sigc::mem_fun(*this, &CssReloadHelper::handleFileChange));
+
+    if (!connection.connected()) {
+      spdlog::error("Failed to connect to file monitor for path: {}", file);
+      continue;
+    }
+    m_fileMonitors.emplace_back(std::move(fileMonitor));
+  }
+}
+
+void waybar::CssReloadHelper::handleFileChange(Glib::RefPtr<Gio::File> const& file,
+                                               Glib::RefPtr<Gio::File> const& other_type,
+                                               Gio::FileMonitorEvent event_type) {
+  // Multiple events are fired on file changed (attributes, write, changes done hint, etc.), only
+  // fire for one
+  if (event_type == Gio::FileMonitorEvent::FILE_MONITOR_EVENT_CHANGES_DONE_HINT) {
+    spdlog::debug("Reloading style, file changed: {}", file->get_path());
+    m_callback();
+  }
 }
 
 std::vector<std::string> waybar::CssReloadHelper::parseImports(const std::string& cssFile) {
@@ -106,109 +142,3 @@ void waybar::CssReloadHelper::parseImports(const std::string& cssFile,
 
   imports[cssFile] = true;
 }
-
-void waybar::CssReloadHelper::stop() {
-  if (!m_running) {
-    return;
-  }
-
-  m_running = false;
-  m_cv.notify_all();
-  if (m_thread.joinable()) {
-    m_thread.join();
-  }
-}
-
-void waybar::CssReloadHelper::watchFiles(const std::vector<std::string>& files) {
-  auto inotifyFd = inotify_init1(IN_NONBLOCK);
-  if (inotifyFd < 0) {
-    spdlog::error("Failed to initialize inotify: {}", strerror(errno));
-    return;
-  }
-
-  std::vector<int> watchFds;
-  for (const auto& file : files) {
-    auto watchFd = inotify_add_watch(inotifyFd, file.c_str(), IN_MODIFY | IN_MOVED_TO);
-    if (watchFd < 0) {
-      spdlog::error("Failed to add watch for file: {}", file);
-    } else {
-      spdlog::debug("Added watch for file: {}", file);
-    }
-    watchFds.push_back(watchFd);
-  }
-
-  auto pollFd = pollfd{inotifyFd, POLLIN, 0};
-
-  while (true) {
-    if (watch(inotifyFd, &pollFd)) {
-      break;
-    }
-  }
-
-  for (const auto& watchFd : watchFds) {
-    inotify_rm_watch(inotifyFd, watchFd);
-  }
-
-  close(inotifyFd);
-}
-
-bool waybar::CssReloadHelper::watch(int inotifyFd, pollfd* pollFd) {
-  auto pollResult = poll(pollFd, 1, 10);
-  if (pollResult < 0) {
-    spdlog::error("Failed to poll inotify: {}", strerror(errno));
-    return true;
-  }
-
-  if (pollResult == 0) {
-    // check if we should stop
-    if (!m_running) {
-      return true;
-    }
-
-    std::unique_lock<std::mutex> lock(m_mutex);
-    // a condition variable is used to allow the thread to be stopped immediately while still not
-    // spamming poll
-    m_cv.wait_for(lock, std::chrono::milliseconds(250), [this] { return !m_running; });
-
-    // timeout
-    return false;
-  }
-
-  if (static_cast<bool>(pollFd->revents & POLLIN)) {
-    if (handleInotifyEvents(inotifyFd)) {
-      // after the callback is fired we need to re-parse the imports and setup the watches
-      // again in case the import list has changed
-      return true;
-    }
-  }
-
-  return false;
-}
-
-bool waybar::CssReloadHelper::handleInotifyEvents(int inotify_fd) {
-  // inotify event
-  auto buffer = std::array<char, 4096>{};
-  auto readResult = read(inotify_fd, buffer.data(), buffer.size());
-  if (readResult < 0) {
-    spdlog::error("Failed to read inotify event: {}", strerror(errno));
-    return false;
-  }
-
-  auto offset = 0;
-  auto shouldFireCallback = false;
-
-  // read all events on the fd
-  while (offset < readResult) {
-    auto* event = reinterpret_cast<inotify_event*>(buffer.data() + offset);
-    offset += sizeof(inotify_event) + event->len;
-    shouldFireCallback = true;
-  }
-
-  // we only need to fire the callback once
-  if (shouldFireCallback) {
-    m_callback();
-    return true;
-  }
-
-  return false;
-}

test/css_reload_helper.cpp

@@ -124,65 +124,3 @@ TEST_CASE_METHOD(CssReloadHelperTest, "parse_imports", "[util][css_reload_helper
     REQUIRE(files.empty());
   }
 }
-
-TEST_CASE("file_watcher", "[util][css_reload_helper]")
-{
-  SECTION("file does not exist")
-  {
-    std::atomic<int> count;
-    std::string f1 = std::tmpnam(nullptr);
-    waybar::CssReloadHelper helper(f1, [&count](){++count;});
-    helper.monitorChanges();
-
-    std::this_thread::sleep_for(std::chrono::milliseconds(100));
-
-    CHECK(count == 0);
-    helper.stop();
-    std::remove(f1.c_str());
-  }
-
-  SECTION("file exists")
-  {
-    std::atomic<int> count;
-    std::string f1 = std::tmpnam(nullptr);
-    std::ofstream(f1) << "body { color: red; }";
-    waybar::CssReloadHelper helper(f1, [&count](){++count;});
-    helper.monitorChanges();
-
-    std::this_thread::sleep_for(std::chrono::milliseconds(100));
-
-    CHECK(count == 0);
-
-    std::this_thread::sleep_for(std::chrono::milliseconds(100));
-
-    std::ofstream(f1) << "body { color: blue; }";
-    std::this_thread::sleep_for(std::chrono::milliseconds(100));
-    CHECK(count == 1);
-    helper.stop();
-    std::remove(f1.c_str());
-  }
-
-  SECTION("multiple files")
-  {
-    std::atomic<int> count;
-    std::string f1 = std::tmpnam(nullptr);
-    std::string f2 = std::tmpnam(nullptr);
-    std::ofstream(f1) << ("@import '" + f2 + " ';");
-    std::ofstream(f2) << "body { color: red; }";
-    waybar::CssReloadHelper helper(f1, [&count](){++count;});
-    helper.monitorChanges();
-
-    std::this_thread::sleep_for(std::chrono::milliseconds(100));
-
-    CHECK(count == 0);
-
-    std::this_thread::sleep_for(std::chrono::milliseconds(100));
-
-    std::ofstream(f2) << "body { color: blue; }";
-    std::this_thread::sleep_for(std::chrono::milliseconds(100));
-    CHECK(count == 1);
-    helper.stop();
-    std::remove(f1.c_str());
-    std::remove(f2.c_str());
-  }
-}