dwm: Reorganize module

Remove EVENT_SCROLL_{UP_DOWN} since it is not being used.

Change EVENT_PREFIX to include dash and event names to be just the name without
the prefix, for simplified parsing. Use check_send_cmd function to parse the
cmd, using the event name as the IPC command name, and the section after the
event name to be the argument. The format of a cmd would be "dwm-<event
name>-<arg>", so the cmd can easily be translated into an IPC command.

Call member functions for all dwmipc events for better organization and to avoid
cluttering the constructor. The dwmipc event functions are now just assigned to
a lambda that calls a member function.

Add update_tag_labels function for updating the tag labels based on their state
since this code is repetetive.

Add update_title_labels function since that code is also somewhat repetetive.

Move reconnect code to reconnect_dwm for better organization.

Use pointers to m_monitors array elements instead of holding onto indices, since
most of the time, a member of the Monitor element will need to be accessed.
These variables should always hold a valid address starting in the constructor,
so checks for nullptr should not be necessary. A monitor will always be active
and the bar will always be mapped onto a monitor.

Add some comments where needed.

Reorganize the constructor into a more logical format

Only subscribe to events if their labels are included in the default format.

Follow clang-tidy warnings and use trailing return types.

Move m_ipc->get_monitors to update_monitor_ref since in most cases where the
monitor references would need to be updated using geometry, m_ipc->get_monitors
would need to be called.

include/modules/dwm.hpp

@@ -48,31 +48,39 @@ namespace modules {
     static constexpr const char* TAG_LABEL_LAYOUT{"<label-layout>"};
     static constexpr const char* TAG_LABEL_TITLE{"<label-title>"};
 
-    static constexpr const char* EVENT_PREFIX{"dwm"};
+    static constexpr const char* EVENT_PREFIX{"dwm-"};
-    static constexpr const char* EVENT_LCLICK{"dwm-view-"};
+    static constexpr const char* EVENT_LCLICK{"view"};
-    static constexpr const char* EVENT_RCLICK{"dwm-toggleview-"};
+    static constexpr const char* EVENT_RCLICK{"toggleview"};
-    static constexpr const char* EVENT_SCROLL_UP{"dwm-tagnext"};
+
-    static constexpr const char* EVENT_SCROLL_DOWN{"dwm-tagprev"};
+    void on_layout_change(const dwmipc::LayoutChangeEvent& ev);
+    void on_monitor_focus_change(const dwmipc::MonitorFocusChangeEvent& ev);
+    void on_tag_change(const dwmipc::TagChangeEvent& ev);
+    void on_client_focus_change(const dwmipc::ClientFocusChangeEvent& ev);
+    void on_focused_title_change(const dwmipc::FocusedTitleChangeEvent& ev);
+
+    void update_monitor_ref();
+    void update_tag_labels();
+    void update_title_label(unsigned int client_id);
 
     auto get_state(tag_mask_t bit_mask) const -> state_t;
-    void update_monitor_ref();
+    auto check_send_cmd(string&& cmd, const string& ev_name) -> bool;
+    auto reconnect_dwm() -> bool;
 
     bool m_click{true};
     bool m_pin_tags{false};
 
+    const dwmipc::Monitor* m_active_mon = nullptr;
+    const dwmipc::Monitor* m_bar_mon = nullptr;
+    unsigned int m_focused_client_id = 0;
+
     label_t m_layout_label;
     label_t m_seperator_label;
     label_t m_title_label;
 
+    unique_ptr<dwmipc::Connection> m_ipc;
     shared_ptr<std::vector<dwmipc::Monitor>> m_monitors;
-    unsigned int m_active_mon_num = 0;
-    unsigned int m_bar_mon = 0;
-    unsigned int m_focused_client_id = 0;
-
     std::unordered_map<state_t, label_t> m_state_labels;
     vector<tag_t> m_tags;
-
-    unique_ptr<dwmipc::Connection> m_ipc;
   };
 }  // namespace modules
 

src/modules/dwm.cpp

@@ -17,6 +17,7 @@ namespace modules {
   dwm_module::dwm_module(const bar_settings& bar, string name_) : event_module<dwm_module>(bar, move(name_)) {
     string socket_path = env_util::get("DWM_SOCKET");
     if (socket_path.empty()) {
+      // Defined by cmake
       socket_path = DWM_SOCKET_PATH;
     }
 
@@ -27,11 +28,9 @@ namespace modules {
     m_ipc = factory_util::unique<dwmipc::Connection>(socket_path);
 
     // Load configuration
-    m_click = m_conf.get(name(), "enable-click", m_click);
-    m_pin_tags = m_conf.get(name(), "pin-tags", m_pin_tags);
-
     m_formatter->add(DEFAULT_FORMAT, DEFAULT_FORMAT_TAGS, {TAG_LABEL_STATE, TAG_LABEL_LAYOUT, TAG_LABEL_TITLE});
 
+    // Populate m_state_labels map with labels and their states
     if (m_formatter->has(TAG_LABEL_STATE)) {
       m_state_labels.insert(
           std::make_pair(state_t::FOCUSED, load_optional_label(m_conf, name(), "label-focused", DEFAULT_TAG_LABEL)));
@@ -41,39 +40,29 @@ namespace modules {
           std::make_pair(state_t::VISIBLE, load_optional_label(m_conf, name(), "label-visible", DEFAULT_TAG_LABEL)));
       m_state_labels.insert(
           std::make_pair(state_t::URGENT, load_optional_label(m_conf, name(), "label-urgent", DEFAULT_TAG_LABEL)));
+      // Dummy label for unoccupied tags
       m_state_labels.insert(
           std::make_pair(state_t::NONE, load_optional_label(m_conf, name(), "label-none", DEFAULT_TAG_LABEL)));
     }
 
+    m_seperator_label = load_optional_label(m_conf, name(), "label-separator", "");
+
     if (m_formatter->has(TAG_LABEL_LAYOUT)) {
       m_layout_label = load_optional_label(m_conf, name(), "label-layout", "%layout%");
     }
 
     if (m_formatter->has(TAG_LABEL_TITLE)) {
       m_title_label = load_optional_label(m_conf, name(), "label-title", "%title%");
-      m_title_label->replace_token("%title%", "");
     }
 
-    m_seperator_label = load_optional_label(m_conf, name(), "label-separator", "");
+    m_click = m_conf.get(name(), "enable-click", m_click);
+    m_pin_tags = m_conf.get(name(), "pin-tags", m_pin_tags);
 
     try {
-      m_monitors = m_ipc->get_monitors();
-      std::sort(m_monitors->begin(), m_monitors->end(),
-          [](dwmipc::Monitor& m1, dwmipc::Monitor& m2) { return m1.num < m2.num; });
       update_monitor_ref();
 
-      auto selected_client = m_monitors->at(m_bar_mon).clients.selected;
+      // Initialize tags array
-      std::shared_ptr<dwmipc::Client> c;
-      if (selected_client != 0) {
-        c = m_ipc->get_client(selected_client);
-      }
-
-      m_title_label->reset_tokens();
-      m_title_label->replace_token("%title%", c->name);
-
       auto tags = m_ipc->get_tags();
-      // m_tags.resize(tags->size());
-
       for (dwmipc::Tag& t : *tags) {
         auto state = get_state(t.bit_mask);
         auto label = m_state_labels.at(state)->clone();
@@ -81,69 +70,38 @@ namespace modules {
         m_tags.emplace_back(t.tag_name, t.bit_mask, state, move(label));
       }
 
+      // This event is only needed to update the layout label
       if (m_layout_label) {
         auto layouts = m_ipc->get_layouts();
-        m_layout_label->replace_token("%layout%", m_monitors->at(m_bar_mon).layout.symbol.cur);
+        // Initialize layout symbol
-        m_ipc->on_layout_change = [this](const dwmipc::LayoutChangeEvent& ev) {
+        m_layout_label->replace_token("%layout%", m_bar_mon->layout.symbol.cur);
-          if (ev.monitor_num == m_bar_mon) {
+        m_ipc->on_layout_change = [this](const dwmipc::LayoutChangeEvent& ev) { on_layout_change(ev); };
-            m_layout_label->reset_tokens();
+        m_ipc->subscribe(dwmipc::Event::LAYOUT_CHANGE);
-            m_layout_label->replace_token("%layout%", ev.new_symbol);
-          }
-        };
       }
 
-      m_ipc->on_monitor_focus_change = [this](const dwmipc::MonitorFocusChangeEvent& ev) {
+      // These events are only necessary to update the focused window title
-        m_active_mon_num = ev.new_mon_num;
+      if (m_title_label) {
-
+        update_title_label(m_active_mon->clients.selected);
-        for (auto& t : m_tags) {
-          t.state = get_state(t.bit_mask);
-          t.label = m_state_labels.at(t.state)->clone();
-          t.label->reset_tokens();
-          t.label->replace_token("%name%", t.name);
-        }
-      };
-
-      m_ipc->on_tag_change = [this](const dwmipc::TagChangeEvent& ev) {
-        auto& mon = m_monitors->at(ev.monitor_num);
-        mon.tag_state = ev.new_state;
-
-        if (ev.monitor_num == m_bar_mon) {
-          for (auto& t : m_tags) {
-            t.state = get_state(t.bit_mask);
-            t.label = m_state_labels.at(t.state)->clone();
-            t.label->reset_tokens();
-            t.label->replace_token("%name%", t.name);
-          }
-        }
-      };
-
         m_ipc->on_client_focus_change = [this](const dwmipc::ClientFocusChangeEvent& ev) {
-        if (ev.monitor_num != m_bar_mon) {
+          this->on_client_focus_change(ev);
-          return;
-        }
-
-        m_focused_client_id = ev.new_win_id;
-        m_title_label->reset_tokens();
-        if (m_focused_client_id != 0) {
-          auto focused_client = m_ipc->get_client(m_focused_client_id);
-          m_title_label->replace_token("%title%", focused_client->name);
-        } else {
-          m_title_label->replace_token("%title%", "");
-        }
         };
+        m_ipc->subscribe(dwmipc::Event::CLIENT_FOCUS_CHANGE);
 
         m_ipc->on_focused_title_change = [this](const dwmipc::FocusedTitleChangeEvent& ev) {
-        if (ev.monitor_num == m_bar_mon && ev.client_window_id == m_focused_client_id) {
+          this->on_focused_title_change(ev);
-          m_title_label->reset_tokens();
-          m_title_label->replace_token("%title%", ev.new_name);
-        }
         };
-
-      m_ipc->subscribe(dwmipc::Event::LAYOUT_CHANGE);
-      m_ipc->subscribe(dwmipc::Event::CLIENT_FOCUS_CHANGE);
-      m_ipc->subscribe(dwmipc::Event::TAG_CHANGE);
-      m_ipc->subscribe(dwmipc::Event::MONITOR_FOCUS_CHANGE);
         m_ipc->subscribe(dwmipc::Event::FOCUSED_TITLE_CHANGE);
+      }
+
+      // This event is for keeping track of the currently focused monitor
+      m_ipc->on_monitor_focus_change = [this](const dwmipc::MonitorFocusChangeEvent& ev) {
+        this->on_monitor_focus_change(ev);
+      };
+      m_ipc->subscribe(dwmipc::Event::MONITOR_FOCUS_CHANGE);
+
+      // This event is for keeping track of the tag states
+      m_ipc->on_tag_change = [this](const dwmipc::TagChangeEvent& ev) { this->on_tag_change(ev); };
+      m_ipc->subscribe(dwmipc::Event::TAG_CHANGE);
     } catch (const dwmipc::IPCError& err) {
       throw module_error(err.what());
     }
@@ -159,35 +117,24 @@ namespace modules {
     event_module::stop();
   }
 
-  bool dwm_module::has_event() {
+  auto dwm_module::has_event() -> bool {
     try {
       return m_ipc->handle_event();
     } catch (const dwmipc::SocketClosedError& err) {
       m_log.err("%s: Disconnected from socket: %s", name(), err.what());
-      try {
+      return reconnect_dwm();
-        if (!m_ipc->is_main_socket_connected()) {
-          m_log.info("%s: Attempting to reconnect to main socket", name());
-          m_ipc->connect_main_socket();
-          m_log.info("%s: Successfully reconnected to main socket", name());
-        } else if (!m_ipc->is_event_socket_connected()) {
-          m_log.info("%s: Attempting to reconnect event socket", name());
-          m_ipc->connect_event_socket();
-          m_log.info("%s: Successfully reconnected event to socket", name());
-        }
-      } catch (const exception& err) {
-        m_log.err("%s: Failed to reconnect to socket: %s", name(), err.what());
-      }
     } catch (const exception& err) {
       m_log.err("%s: Failed to handle event (reason: %s)", name(), err.what());
     }
     return false;
   }
 
-  bool dwm_module::update() {
+  auto dwm_module::update() -> bool {
+    // All updates are handled in has_event
     return true;
   }
 
-  bool dwm_module::build(builder* builder, const string& tag) const {
+  auto dwm_module::build(builder* builder, const string& tag) const -> bool {
     if (tag == TAG_LABEL_LAYOUT) {
       builder->node(m_layout_label);
     } else if (tag == TAG_LABEL_TITLE) {
@@ -195,7 +142,9 @@ namespace modules {
     } else if (tag == TAG_LABEL_STATE) {
       bool first = true;
       for (const auto& tag : m_tags) {
+        // Print tags without state only if m_pin_tags
         if ((tag.state == state_t::NONE && m_pin_tags) || (!m_pin_tags && tag.state != state_t::NONE)) {
+          // Don't insert separator before first tag
           if (first) {
             first = false;
           } else if (*m_seperator_label) {
@@ -219,36 +168,36 @@ namespace modules {
     return true;
   }
 
-  bool dwm_module::input(string&& cmd) {
+  auto dwm_module::check_send_cmd(string&& cmd, const string& ev_name) -> bool {
+    // cmd = <EVENT_PREFIX>-<ev_name>-<arg>
+    // +1 : Erase '-'
+    cmd.erase(0, strlen(EVENT_PREFIX) + 1);
+
+    // cmd = <ev_name>-<arg>
+    if (cmd.compare(0, ev_name.size(), ev_name) == 0) {
+      // Erase '<ev_name>-'
+      cmd.erase(0, ev_name.size() + 1);
+      m_log.info("%s: Sending workspace view command to ipc handler", name());
+
+      try {
+        m_ipc->run_command(ev_name, stoul(cmd));
+        return true;
+      } catch (const dwmipc::IPCError& err) {
+        throw module_error(err.what());
+      }
+    }
+    return false;
+  }
+
+  auto dwm_module::input(string&& cmd) -> bool {
     if (cmd.find(EVENT_PREFIX) != 0) {
       return false;
     }
 
-    if (cmd.compare(0, strlen(EVENT_LCLICK), EVENT_LCLICK) == 0) {
+    return check_send_cmd(move(cmd), EVENT_LCLICK) || check_send_cmd(move(cmd), EVENT_RCLICK);
-      cmd.erase(0, strlen(EVENT_LCLICK));
-      m_log.info("%s: Sending workspace view command to ipc handler", name());
-
-      try {
-        m_ipc->run_command("view", stoul(cmd));
-        return true;
-      } catch (const dwmipc::IPCError& err) {
-        throw module_error(err.what());
-      }
-    } else if (cmd.compare(0, strlen(EVENT_RCLICK), EVENT_RCLICK) == 0) {
-      cmd.erase(0, strlen(EVENT_RCLICK));
-      m_log.info("%s: Sending workspace toggleview command to ipc handler", name());
-
-      try {
-        m_ipc->run_command("toggleview", stoul(cmd));
-        return true;
-      } catch (const dwmipc::IPCError& err) {
-        throw module_error(err.what());
-      }
-    }
-    return true;
   }
 
-  dwm_module::state_t dwm_module::get_state(tag_mask_t bit_mask) const {
+  auto dwm_module::get_state(tag_mask_t bit_mask) const -> state_t {
     // Tag selected > occupied > urgent
     // Monitor selected - Tag selected FOCUSED
     // Monitor unselected - Tag selected UNFOCUSED
@@ -256,8 +205,8 @@ namespace modules {
     // Tag unselected - Tag occupied - Tag urgent URGENT
     // Tag unselected - Tag unoccupied PIN?
 
-    auto tag_state = m_monitors->at(m_bar_mon).tag_state;
+    auto tag_state = m_bar_mon->tag_state;
-    bool is_mon_active = m_bar_mon == m_active_mon_num;
+    bool is_mon_active = m_bar_mon == m_active_mon;
 
     if (is_mon_active && tag_state.selected & bit_mask) {
       // Tag selected on selected monitor
@@ -277,19 +226,101 @@ namespace modules {
   }
 
   void dwm_module::update_monitor_ref() {
+    m_monitors = m_ipc->get_monitors();
+    // Sort by increasing monitor index
+    std::sort(m_monitors->begin(), m_monitors->end(),
+        [](dwmipc::Monitor& m1, dwmipc::Monitor& m2) { return m1.num < m2.num; });
+
     for (const dwmipc::Monitor& m : *m_monitors) {
       const auto& geom = m.monitor_geom;
       const auto& bmon = *m_bar.monitor;
+      // Compare geometry
       if (geom.x == bmon.x && geom.y == bmon.y && geom.width == bmon.w && geom.height == bmon.h) {
-        m_bar_mon = m.num;
+        m_bar_mon = &m;
       }
 
       if (m.is_selected) {
-        m_active_mon_num = m.num;
+        m_active_mon = &m;
       }
     }
   }
 
+  void dwm_module::update_tag_labels() {
+    for (auto& t : m_tags) {
+      t.state = get_state(t.bit_mask);
+      t.label = m_state_labels.at(t.state)->clone();
+      t.label->reset_tokens();
+      t.label->replace_token("%name%", t.name);
+    }
+  }
+
+  void dwm_module::update_title_label(unsigned int client_id) {
+    std::string new_title;
+    if (client_id != 0) {
+      try {
+        new_title = m_ipc->get_client(client_id)->name;
+      } catch (const dwmipc::IPCError &err) {
+        throw module_error(err.what());
+      }
+    }
+    m_title_label->reset_tokens();
+    m_title_label->replace_token("%title%", new_title);
+  }
+
+  auto dwm_module::reconnect_dwm() -> bool {
+    try {
+      if (!m_ipc->is_main_socket_connected()) {
+        m_log.info("%s: Attempting to reconnect to main socket", name());
+        m_ipc->connect_main_socket();
+        m_log.info("%s: Successfully reconnected to main socket", name());
+      }
+      if (!m_ipc->is_event_socket_connected()) {
+        m_log.info("%s: Attempting to reconnect event socket", name());
+        m_ipc->connect_event_socket();
+        m_log.info("%s: Successfully reconnected event to socket", name());
+      }
+      return true;
+    } catch (const exception& err) {
+      m_log.err("%s: Failed to reconnect to socket: %s", name(), err.what());
+    }
+    return false;
+  }
+
+  void dwm_module::on_layout_change(const dwmipc::LayoutChangeEvent& ev) {
+    if (ev.monitor_num == m_bar_mon->num) {
+      m_layout_label->reset_tokens();
+      m_layout_label->replace_token("%layout%", ev.new_symbol);
+    }
+  }
+
+  void dwm_module::on_monitor_focus_change(const dwmipc::MonitorFocusChangeEvent& ev) {
+    m_active_mon = &m_monitors->at(ev.new_mon_num);
+    update_tag_labels();
+  }
+
+  void dwm_module::on_tag_change(const dwmipc::TagChangeEvent& ev) {
+    auto& mon = m_monitors->at(ev.monitor_num);
+    mon.tag_state = ev.new_state;
+
+    if (ev.monitor_num == m_bar_mon->num) {
+      update_tag_labels();
+    }
+  }
+
+  void dwm_module::on_focused_title_change(const dwmipc::FocusedTitleChangeEvent& ev) {
+    if (ev.monitor_num == m_bar_mon->num && ev.client_window_id == m_focused_client_id) {
+      m_title_label->reset_tokens();
+      m_title_label->replace_token("%title%", ev.new_name);
+    }
+  }
+
+  void dwm_module::on_client_focus_change(const dwmipc::ClientFocusChangeEvent& ev) {
+    if (ev.monitor_num == m_bar_mon->num) {
+      m_focused_client_id = ev.new_win_id;
+      update_title_label(ev.new_win_id);
+    }
+  }
+
 }  // namespace modules
 
 POLYBAR_NS_END