systemd-failed-units: small tweaks

- Remove unneeded destructor impl
- Rename member variables for consistench with other files
- manpage wording fixes
- updateData(): small logic tweak

include/modules/systemd_failed_units.hpp

@@ -11,18 +11,18 @@ namespace waybar::modules {
 class SystemdFailedUnits : public ALabel {
  public:
   SystemdFailedUnits(const std::string&, const Json::Value&);
-  virtual ~SystemdFailedUnits();
+  virtual ~SystemdFailedUnits() = default;
   auto update() -> void override;
 
  private:
-  bool hide_on_ok;
-  std::string format_ok;
+  bool hide_on_ok_;
+  std::string format_ok_;
 
-  bool update_pending;
-  std::string system_state, user_state, overall_state;
-  uint32_t nr_failed_system, nr_failed_user, nr_failed;
-  std::string last_status;
-  Glib::RefPtr<Gio::DBus::Proxy> system_proxy, user_proxy;
+  bool update_pending_;
+  std::string system_state_, user_state_, overall_state_;
+  uint32_t nr_failed_system_, nr_failed_user_, nr_failed_;
+  std::string last_status_;
+  Glib::RefPtr<Gio::DBus::Proxy> system_props_proxy_, user_props_proxy_;
 
   void notify_cb(const Glib::ustring& sender_name, const Glib::ustring& signal_name,
                  const Glib::VariantContainerBase& arguments);

man/waybar-systemd-failed-units.5.scd

@@ -19,7 +19,7 @@ Addressed by *systemd-failed-units*
 
 *format-ok*: ++
 	typeof: string ++
-	This format is used when there is no failing units.
+	This format is used when there are no failing units.
 
 *user*: ++
 	typeof: bool ++
@@ -34,15 +34,15 @@ Addressed by *systemd-failed-units*
 *hide-on-ok*: ++
 	typeof: bool ++
 	default: *true* ++
-	Option to hide this module when there is no failing units.
+	Option to hide this module when there are no failed units.
 
 *menu*: ++
 	typeof: string ++
-	Action that popups the menu.
+	Action that pops up the menu.
 
 *menu-file*: ++
 	typeof: string ++
-	Location of the menu descriptor file. There need to be an element of type
+	Location of the menu descriptor file. There needs to be an element of type
 	GtkMenu with id *menu*
 
 *menu-actions*: ++
@@ -52,7 +52,7 @@ Addressed by *systemd-failed-units*
 *expand*: ++
 	typeof: bool ++
 	default: false ++
-	Enables this module to consume all left over space dynamically.
+	Enables this module to consume all leftover space dynamically.
 
 # FORMAT REPLACEMENTS
 
@@ -62,11 +62,11 @@ Addressed by *systemd-failed-units*
 
 *{nr_failed}*: Number of total failed units.
 
-*{systemd_state}:* State of the systemd system session
+*{system_state}:* State of the systemd system session.
 
-*{user_state}:* State of the systemd user session
+*{user_state}:* State of the systemd user session.
 
-*{overall_state}:* Overall state of the systemd and user session. ("Ok" or "Degraded")
+*{overall_state}:* Overall state of the systemd and user session. ("ok" or "degraded")
 
 # EXAMPLES
 

src/modules/systemd_failed_units.cpp

@@ -1,10 +1,12 @@
 #include "modules/systemd_failed_units.hpp"
 
+#include <fmt/format.h>
 #include <giomm/dbusproxy.h>
 #include <glibmm/variant.h>
 #include <spdlog/spdlog.h>
 
 #include <cstdint>
+#include <stdexcept>
 
 static const unsigned UPDATE_DEBOUNCE_TIME_MS = 1000;
 
@@ -12,39 +14,41 @@ namespace waybar::modules {
 
 SystemdFailedUnits::SystemdFailedUnits(const std::string& id, const Json::Value& config)
     : ALabel(config, "systemd-failed-units", id, "{nr_failed} failed", 1),
-      hide_on_ok(true),
-      update_pending(false),
-      nr_failed_system(0),
-      nr_failed_user(0),
-      nr_failed(0),
-      last_status() {
+      hide_on_ok_(true),
+      update_pending_(false),
+      nr_failed_system_(0),
+      nr_failed_user_(0),
+      nr_failed_(0),
+      last_status_() {
   if (config["hide-on-ok"].isBool()) {
-    hide_on_ok = config["hide-on-ok"].asBool();
+    hide_on_ok_ = config["hide-on-ok"].asBool();
   }
   if (config["format-ok"].isString()) {
-    format_ok = config["format-ok"].asString();
+    format_ok_ = config["format-ok"].asString();
   } else {
-    format_ok = format_;
+    format_ok_ = format_;
   }
 
   /* Default to enable both "system" and "user". */
   if (!config["system"].isBool() || config["system"].asBool()) {
-    system_proxy = Gio::DBus::Proxy::create_for_bus_sync(
+    system_props_proxy_ = Gio::DBus::Proxy::create_for_bus_sync(
         Gio::DBus::BusType::BUS_TYPE_SYSTEM, "org.freedesktop.systemd1",
         "/org/freedesktop/systemd1", "org.freedesktop.DBus.Properties");
-    if (!system_proxy) {
+    if (!system_props_proxy_) {
       throw std::runtime_error("Unable to connect to systemwide systemd DBus!");
     }
-    system_proxy->signal_signal().connect(sigc::mem_fun(*this, &SystemdFailedUnits::notify_cb));
+    system_props_proxy_->signal_signal().connect(
+        sigc::mem_fun(*this, &SystemdFailedUnits::notify_cb));
   }
   if (!config["user"].isBool() || config["user"].asBool()) {
-    user_proxy = Gio::DBus::Proxy::create_for_bus_sync(
+    user_props_proxy_ = Gio::DBus::Proxy::create_for_bus_sync(
         Gio::DBus::BusType::BUS_TYPE_SESSION, "org.freedesktop.systemd1",
         "/org/freedesktop/systemd1", "org.freedesktop.DBus.Properties");
-    if (!user_proxy) {
+    if (!user_props_proxy_) {
       throw std::runtime_error("Unable to connect to user systemd DBus!");
     }
-    user_proxy->signal_signal().connect(sigc::mem_fun(*this, &SystemdFailedUnits::notify_cb));
+    user_props_proxy_->signal_signal().connect(
+        sigc::mem_fun(*this, &SystemdFailedUnits::notify_cb));
   }
 
   updateData();
@@ -52,16 +56,11 @@ SystemdFailedUnits::SystemdFailedUnits(const std::string& id, const Json::Value&
   dp.emit();
 }
 
-SystemdFailedUnits::~SystemdFailedUnits() {
-  if (system_proxy) system_proxy.reset();
-  if (user_proxy) user_proxy.reset();
-}
-
 auto SystemdFailedUnits::notify_cb(const Glib::ustring& sender_name,
                                    const Glib::ustring& signal_name,
                                    const Glib::VariantContainerBase& arguments) -> void {
-  if (signal_name == "PropertiesChanged" && !update_pending) {
-    update_pending = true;
+  if (signal_name == "PropertiesChanged" && !update_pending_) {
+    update_pending_ = true;
     /* The fail count may fluctuate due to restarting. */
     Glib::signal_timeout().connect_once(sigc::mem_fun(*this, &SystemdFailedUnits::updateData),
                                         UPDATE_DEBOUNCE_TIME_MS);
@@ -88,12 +87,12 @@ void SystemdFailedUnits::RequestSystemState() {
     return "unknown";
   };
 
-  system_state = load("systemwide", system_proxy);
-  user_state = load("user", user_proxy);
-  if (system_state == "running" && user_state == "running")
-    overall_state = "ok";
+  system_state_ = load("systemwide", system_props_proxy_);
+  user_state_ = load("user", user_props_proxy_);
+  if (system_state_ == "running" && user_state_ == "running")
+    overall_state_ = "ok";
   else
-    overall_state = "degraded";
+    overall_state_ = "degraded";
 }
 
 void SystemdFailedUnits::RequestFailedUnits() {
@@ -116,25 +115,31 @@ void SystemdFailedUnits::RequestFailedUnits() {
     return 0;
   };
 
-  nr_failed_system = load("systemwide", system_proxy);
-  nr_failed_user = load("user", user_proxy);
-  nr_failed = nr_failed_system + nr_failed_user;
+  nr_failed_system_ = load("systemwide", system_props_proxy_);
+  nr_failed_user_ = load("user", user_props_proxy_);
+  nr_failed_ = nr_failed_system_ + nr_failed_user_;
 }
 
 void SystemdFailedUnits::updateData() {
-  update_pending = false;
+  update_pending_ = false;
 
   RequestSystemState();
-  if (overall_state == "degraded") RequestFailedUnits();
+  if (overall_state_ == "degraded") {
+    RequestFailedUnits();
+  } else {
+    nr_failed_system_ = 0;
+    nr_failed_user_ = 0;
+    nr_failed_ = 0;
+  }
 
   dp.emit();
 }
 
 auto SystemdFailedUnits::update() -> void {
-  if (last_status == overall_state) return;
+  if (last_status_ == overall_state_) return;
 
   // Hide if needed.
-  if (overall_state == "ok" && hide_on_ok) {
+  if (overall_state_ == "ok" && hide_on_ok_) {
     event_box_.set_visible(false);
     return;
   }
@@ -142,20 +147,20 @@ auto SystemdFailedUnits::update() -> void {
   event_box_.set_visible(true);
 
   // Set state class.
-  if (!last_status.empty() && label_.get_style_context()->has_class(last_status)) {
-    label_.get_style_context()->remove_class(last_status);
+  if (!last_status_.empty() && label_.get_style_context()->has_class(last_status_)) {
+    label_.get_style_context()->remove_class(last_status_);
   }
-  if (!label_.get_style_context()->has_class(overall_state)) {
-    label_.get_style_context()->add_class(overall_state);
+  if (!label_.get_style_context()->has_class(overall_state_)) {
+    label_.get_style_context()->add_class(overall_state_);
   }
 
-  last_status = overall_state;
+  last_status_ = overall_state_;
 
   label_.set_markup(fmt::format(
-      fmt::runtime(nr_failed == 0 ? format_ok : format_), fmt::arg("nr_failed", nr_failed),
-      fmt::arg("nr_failed_system", nr_failed_system), fmt::arg("nr_failed_user", nr_failed_user),
-      fmt::arg("system_state", system_state), fmt::arg("user_state", user_state),
-      fmt::arg("overall_state", overall_state)));
+      fmt::runtime(nr_failed_ == 0 ? format_ok_ : format_), fmt::arg("nr_failed", nr_failed_),
+      fmt::arg("nr_failed_system", nr_failed_system_),
+      fmt::arg("nr_failed_user", nr_failed_user_), fmt::arg("system_state", system_state_),
+      fmt::arg("user_state", user_state_), fmt::arg("overall_state", overall_state_)));
   ALabel::update();
 }