Merge branch 'hotfix/3.6.2'

CHANGELOG.md

@@ -16,6 +16,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `internal/pulseaudio`: `reverse-scroll` option ([`#2664`](https://github.com/polybar/polybar/pull/2664))
 - `custom/script`: Repeat interval for script failure (`interval-fail`) and `exec-if` (`interval-if`) ([`#943`](https://github.com/polybar/polybar/issues/943), [`#2606`](https://github.com/polybar/polybar/issues/2606), [`#2630`](https://github.com/polybar/polybar/pull/2630))
 
+## [3.6.2] - 2022-04-03
+### Fixed
+- `format-offset` being ignored ([`#2643`](https://github.com/polybar/polybar/pull/2643))
+- Negative struts (`margin-bottom`, `margin-top`) being ignored ([`#2642`](https://github.com/polybar/polybar/issues/2642), [`#2644`](https://github.com/polybar/polybar/pull/2644))
+- Positioning in awesomeWM ([`#2651`](https://github.com/polybar/polybar/pull/2651))
+- `internal/xworkspaces`: The module sometimes crashed polybar when windows were closed. ([`#2655`](https://github.com/polybar/polybar/pull/2655))
+- Mouseover error when only one cursor is defined ([`#2656`](https://github.com/polybar/polybar/pull/2656))
+- `custom/script`: Timing inconsistencies ([`#2650`](https://github.com/polybar/polybar/issues/2650), first described at [`#2630`](https://github.com/polybar/polybar/pull/2630))
+
 ## [3.6.1] - 2022-03-05
 ### Build
 - Fixed compiler warning in Clang 13 ([`#2613`](https://github.com/polybar/polybar/pull/2613))
@@ -170,7 +179,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 - Empty color values are no longer treated as invalid and no longer produce an error.
 
-[Unreleased]: https://github.com/polybar/polybar/compare/3.6.1...HEAD
+[Unreleased]: https://github.com/polybar/polybar/compare/3.6.2...HEAD
+[3.6.2]: https://github.com/polybar/polybar/releases/tag/3.6.2
 [3.6.1]: https://github.com/polybar/polybar/releases/tag/3.6.1
 [3.6.0]: https://github.com/polybar/polybar/releases/tag/3.6.0
 [3.5.7]: https://github.com/polybar/polybar/releases/tag/3.5.7

include/adapters/pulseaudio.hpp

@@ -1,12 +1,12 @@
 #pragma once
 
 #include <pulse/pulseaudio.h>
+
 #include <queue>
 
 #include "common.hpp"
-#include "settings.hpp"
 #include "errors.hpp"
-
+#include "settings.hpp"
 #include "utils/math.hpp"
 // fwd
 struct pa_context;
@@ -60,7 +60,7 @@ class pulseaudio {
 
   // used for temporary callback results
   int success{0};
-    pa_cvolume cv;
+  pa_cvolume cv{};
   bool muted{false};
   // default sink name
   static constexpr auto DEFAULT_SINK = "@DEFAULT_SINK@";

include/adapters/script_runner.hpp

@@ -11,9 +11,18 @@ POLYBAR_NS
 
 class script_runner {
  public:
+  struct data {
+    int counter{0};
+    int pid{-1};
+    int exit_status{0};
+    string output;
+  };
+
+  using on_update = std::function<void(const data&)>;
   using interval = std::chrono::duration<double>;
-  script_runner(std::function<void(void)> on_update, const string& exec, const string& exec_if, bool tail,
+
-      interval interval_success, interval interval_fail, const vector<pair<string, string>>& env);
+  script_runner(on_update on_update, const string& exec, const string& exec_if, bool tail, interval interval_success,
+      interval interval_fail, const vector<pair<string, string>>& env);
 
   bool check_condition() const;
   interval process();
@@ -22,16 +31,11 @@ class script_runner {
 
   void stop();
 
-  int get_pid() const;
-  int get_counter() const;
-  int get_exit_status() const;
-
-  string get_output();
-
   bool is_stopping() const;
 
  protected:
   bool set_output(string&&);
+  bool set_exit_status(int);
 
   interval run_tail();
   interval run();
@@ -39,7 +43,7 @@ class script_runner {
  private:
   const logger& m_log;
 
-  const std::function<void(void)> m_on_update;
+  const on_update m_on_update;
 
   const string m_exec;
   const string m_exec_if;
@@ -48,13 +52,8 @@ class script_runner {
   const interval m_interval_fail;
   const vector<pair<string, string>> m_env;
 
-  std::mutex m_output_lock;
+  data m_data;
-  string m_output;
-
-  std::atomic_int m_counter{0};
   std::atomic_bool m_stopping{false};
-  std::atomic_int m_pid{-1};
-  std::atomic_int m_exit_status{0};
 };
 
 POLYBAR_NS_END

include/components/bar.hpp

@@ -31,12 +31,7 @@ namespace tags {
 
 class bar : public xpp::event::sink<evt::button_press, evt::expose, evt::property_notify, evt::enter_notify,
                 evt::leave_notify, evt::motion_notify, evt::destroy_notify, evt::client_message, evt::configure_notify>,
-            public signal_receiver<SIGN_PRIORITY_BAR, signals::ui::dim_window
+            public signal_receiver<SIGN_PRIORITY_BAR, signals::ui::dim_window> {
-#if WITH_XCURSOR
-                ,
-                signals::ui::cursor_change
-#endif
-                > {
  public:
   using make_type = unique_ptr<bar>;
   static make_type make(eventloop::loop&, bool only_initialize_values = false);
@@ -46,7 +41,7 @@ class bar : public xpp::event::sink<evt::button_press, evt::expose, evt::propert
       bool only_initialize_values);
   ~bar();
 
-  const bar_settings settings() const;
+  const bar_settings& settings() const;
 
   void start();
 
@@ -65,6 +60,8 @@ class bar : public xpp::event::sink<evt::button_press, evt::expose, evt::propert
   void reconfigure_wm_hints();
   void broadcast_visibility();
 
+  void map_window();
+
   void trigger_click(mousebtn btn, int pos);
 
   void handle(const evt::client_message& evt) override;
@@ -78,8 +75,14 @@ class bar : public xpp::event::sink<evt::button_press, evt::expose, evt::propert
   void handle(const evt::configure_notify& evt) override;
 
   bool on(const signals::ui::dim_window&) override;
+
 #if WITH_XCURSOR
-  bool on(const signals::ui::cursor_change&) override;
+  /**
+   * Change cursor to the given cursor name.
+   *
+   * The cursor name must be valid (cursor_util::valid)
+   */
+  void change_cursor(const string& name);
 #endif
 
  private:
@@ -96,13 +99,14 @@ class bar : public xpp::event::sink<evt::button_press, evt::expose, evt::propert
 
   bar_settings m_opts{};
 
+  /**
+   * Name of currently active cursor
+   */
+  string m_cursor{};
+
   string m_lastinput{};
   bool m_dblclicks{false};
 
-#if WITH_XCURSOR
-  int m_motion_pos{0};
-#endif
-
   eventloop::TimerHandle& m_leftclick_timer{m_loop.handle<eventloop::TimerHandle>()};
   eventloop::TimerHandle& m_middleclick_timer{m_loop.handle<eventloop::TimerHandle>()};
   eventloop::TimerHandle& m_rightclick_timer{m_loop.handle<eventloop::TimerHandle>()};

include/components/builder.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <map>
+#include <unordered_set>
 
 #include "common.hpp"
 #include "components/types.hpp"
@@ -21,11 +22,9 @@ class builder {
 
   void reset();
   string flush();
-  void append(const string& text);
   void node(const string& str);
   void node(const string& str, int font_index);
   void node(const label_t& label);
-  void node_repeat(const string& str, size_t n);
   void node_repeat(const label_t& label, size_t n);
   void offset(extent_val pixels = ZERO_PX_EXTENT);
   void spacing(spacing_val size);
@@ -33,17 +32,11 @@ class builder {
   void font_close();
   void background(rgba color);
   void background_close();
-  void color(rgba color);
+  void foreground(rgba color);
-  void color_close();
+  void foreground_close();
-  void line_color(const rgba& color);
+  void overline(const rgba& color);
-  void line_color_close();
-  void overline_color(rgba color);
-  void overline_color_close();
-  void underline_color(rgba color);
-  void underline_color_close();
-  void overline(const rgba& color = rgba{});
   void overline_close();
-  void underline(const rgba& color = rgba{});
+  void underline(const rgba& color);
   void underline_close();
   void control(tags::controltag tag);
   void action(mousebtn index, string action);
@@ -52,23 +45,25 @@ class builder {
   void action(mousebtn btn, const modules::module_interface& module, string action, string data, const label_t& label);
   void action_close();
 
-  static string get_spacing_format_string(const spacing_val& space);
+  static string get_spacing_format_string(spacing_val space);
 
  protected:
+  void append(const string& text);
+
+  void overline_color_close();
+  void underline_color_close();
+
   void tag_open(tags::syntaxtag tag, const string& value);
   void tag_open(tags::attribute attr);
   void tag_close(tags::syntaxtag tag);
   void tag_close(tags::attribute attr);
 
  private:
-  const bar_settings m_bar;
+  const bar_settings& m_bar;
   string m_output;
 
   map<tags::syntaxtag, int> m_tags{};
-  map<tags::syntaxtag, string> m_colors{};
+  std::unordered_set<tags::attribute> m_attrs{};
-  map<tags::attribute, bool> m_attrs{};
-
-  int m_fontindex{0};
 };
 
 POLYBAR_NS_END

include/components/types.hpp

@@ -59,6 +59,11 @@ static inline mousebtn mousebtn_get_double(mousebtn btn) {
   }
 }
 
+/**
+ * Order of values for _NET_WM_STRUT_PARTIAL
+ *
+ * https://specifications.freedesktop.org/wm-spec/wm-spec-latest.html#idm45381391268672
+ */
 enum class strut {
   LEFT = 0,
   RIGHT,
@@ -132,13 +137,6 @@ struct percentage_with_offset {
   extent_val offset{ZERO_PX_EXTENT};
 };
 
-struct edge_values {
-  unsigned int left{0U};
-  unsigned int right{0U};
-  unsigned int top{0U};
-  unsigned int bottom{0U};
-};
-
 struct radius {
   double top_left{0.0};
   double top_right{0.0};
@@ -173,7 +171,7 @@ struct bar_settings {
   monitor_t monitor{};
   bool monitor_strict{false};
   bool monitor_exact{true};
-  edge origin{edge::TOP};
+  bool bottom{false};
   struct size size {
     1U, 1U
   };
@@ -185,7 +183,10 @@ struct bar_settings {
   position offset{0, 0};
   side_values padding{ZERO_SPACE, ZERO_SPACE};
   side_values module_margin{ZERO_SPACE, ZERO_SPACE};
-  edge_values strut{0U, 0U, 0U, 0U};
+  struct {
+    int top;
+    int bottom;
+  } strut{0, 0};
 
   rgba background{0xFF000000};
   rgba foreground{0xFFFFFFFF};
@@ -210,8 +211,14 @@ struct bar_settings {
 
   int double_click_interval{400};
 
-  string cursor{};
+  /**
+   * Name of cursor to use for clickable areas
+   */
   string cursor_click{};
+
+  /**
+   * Name of cursor to use for scrollable areas
+   */
   string cursor_scroll{};
 
   vector<action> actions{};

include/events/signal.hpp

@@ -81,9 +81,6 @@ namespace signals {
     struct button_press : public detail::value_signal<button_press, string> {
       using base_type::base_type;
     };
-    struct cursor_change : public detail::value_signal<cursor_change, string> {
-      using base_type::base_type;
-    };
     struct visibility_change : public detail::value_signal<visibility_change, bool> {
       using base_type::base_type;
     };

include/events/signal_fwd.hpp

@@ -28,7 +28,6 @@ namespace signals {
   namespace ui {
     struct changed;
     struct button_press;
-    struct cursor_change;
     struct visibility_change;
     struct dim_window;
     struct request_snapshot;

include/modules/meta/base.hpp

@@ -142,7 +142,7 @@ namespace modules {
   template <class Impl>
   class module : public module_interface {
    public:
-    module(const bar_settings bar, string name);
+    module(const bar_settings& bar, string name);
     ~module() noexcept;
 
     static constexpr auto EVENT_MODULE_TOGGLE = "module_toggle";
@@ -197,7 +197,7 @@ namespace modules {
 
    protected:
     signal_emitter& m_sig;
-    const bar_settings m_bar;
+    const bar_settings& m_bar;
     const logger& m_log;
     const config& m_conf;
 

include/modules/meta/base.inl

@@ -15,7 +15,7 @@ namespace modules {
   // module<Impl> public {{{
 
   template <typename Impl>
-  module<Impl>::module(const bar_settings bar, string name)
+  module<Impl>::module(const bar_settings& bar, string name)
       : m_sig(signal_emitter::make())
       , m_bar(bar)
       , m_log(logger::make())
@@ -271,7 +271,7 @@ namespace modules {
           m_builder->spacing(format->spacing);
         }
 
-        m_builder->append(tag_content);
+        m_builder->node(tag_content);
         has_tags = true;
       }
 
@@ -279,7 +279,7 @@ namespace modules {
     }
 
     if (cursor < value.size()) {
-      m_builder->append(value.substr(cursor));
+      m_builder->node(value.substr(cursor));
     }
 
     return format->decorate(&*m_builder, m_builder->flush());

include/modules/script.hpp

@@ -26,6 +26,8 @@ namespace modules {
     bool check_condition();
 
    private:
+    void handle_runner_update(const script_runner::data&);
+
     static constexpr auto TAG_LABEL = "<label>";
     static constexpr auto TAG_LABEL_FAIL = "<label-fail>";
     static constexpr auto FORMAT_FAIL = "format-fail";
@@ -41,6 +43,10 @@ namespace modules {
 
     label_t m_label;
     label_t m_label_fail;
+
+    int m_exit_status{0};
+    script_runner::data m_data;
+    std::mutex m_data_mutex;
   };
 } // namespace modules
 

include/utils/units.hpp

@@ -27,6 +27,8 @@ namespace units_utils {
   spacing_type parse_spacing_unit(const string& str);
   spacing_val parse_spacing(const string& str);
 
+  extent_val spacing_to_extent(spacing_val spacing);
+
 } // namespace units_utils
 
 POLYBAR_NS_END

include/x11/cursor.hpp

@@ -9,19 +9,21 @@
 #include <xcb/xcb_cursor.h>
 
 #include "common.hpp"
-#include "x11/connection.hpp"
 #include "utils/string.hpp"
+#include "x11/connection.hpp"
 
 POLYBAR_NS
 
 namespace cursor_util {
   static const std::map<string, vector<string>> cursors = {
-    {"pointer", {"pointing_hand", "pointer", "hand", "hand1", "hand2", "e29285e634086352946a0e7090d73106", "9d800788f1b08800ae810202380a0822"}},
+      {"pointer", {"pointing_hand", "pointer", "hand", "hand1", "hand2", "e29285e634086352946a0e7090d73106",
+                      "9d800788f1b08800ae810202380a0822"}},
       {"default", {"left_ptr", "arrow", "dnd-none", "op_left_arrow"}},
-    {"ns-resize", {"size_ver", "sb_v_double_arrow", "v_double_arrow", "n-resize", "s-resize", "col-resize", "top_side", "bottom_side", "base_arrow_up", "base_arrow_down", "based_arrow_down", "based_arrow_up", "00008160000006810000408080010102"}}
+      {"ns-resize", {"size_ver", "sb_v_double_arrow", "v_double_arrow", "n-resize", "s-resize", "col-resize",
-  };
+                        "top_side", "bottom_side", "base_arrow_up", "base_arrow_down", "based_arrow_down",
-  bool valid(string name);
+                        "based_arrow_up", "00008160000006810000408080010102"}}};
-  bool set_cursor(xcb_connection_t *c, xcb_screen_t *screen, xcb_window_t w, string name);
+  bool valid(const string& name);
-}
+  bool set_cursor(xcb_connection_t* c, xcb_screen_t* screen, xcb_window_t w, const string& name);
+} // namespace cursor_util
 
 POLYBAR_NS_END

include/x11/window.hpp

@@ -2,6 +2,7 @@
 
 #include <xcb/xcb.h>
 #include <xcb/xcb_aux.h>
+
 #include <xpp/window.hpp>
 
 #include "common.hpp"
@@ -17,7 +18,7 @@ class window : public xpp::window<connection&> {
 
   window reconfigure_geom(unsigned short int w, unsigned short int h, short int x = 0, short int y = 0);
   window reconfigure_pos(short int x, short int y);
-  window reconfigure_struts(unsigned short int w, unsigned short int h, short int x, bool bottom = false);
+  window reconfigure_struts(uint32_t w, uint32_t strut, uint32_t x, bool bottom = false);
 };
 
 POLYBAR_NS_END

src/adapters/pulseaudio.cpp

@@ -1,4 +1,5 @@
 #include "adapters/pulseaudio.hpp"
+
 #include "components/logger.hpp"
 
 POLYBAR_NS
@@ -6,7 +7,8 @@ POLYBAR_NS
 /**
  * Construct pulseaudio object
  */
-pulseaudio::pulseaudio(const logger& logger, string&& sink_name, bool max_volume) : m_log(logger), spec_s_name(sink_name) {
+pulseaudio::pulseaudio(const logger& logger, string&& sink_name, bool max_volume)
+    : m_log(logger), spec_s_name(sink_name) {
   m_mainloop = pa_threaded_mainloop_new();
   if (!m_mainloop) {
     throw pulseaudio_error("Could not create pulseaudio threaded mainloop.");
@@ -69,14 +71,14 @@ pulseaudio::pulseaudio(const logger& logger, string&& sink_name, bool max_volume
   auto event_types = static_cast<pa_subscription_mask_t>(PA_SUBSCRIPTION_MASK_SINK | PA_SUBSCRIPTION_MASK_SERVER);
   op = pa_context_subscribe(m_context, event_types, simple_callback, this);
   wait_loop(op, m_mainloop);
-  if (!success)
+  if (!success) {
     throw pulseaudio_error("Failed to subscribe to sink.");
+  }
   pa_context_set_subscribe_callback(m_context, subscribe_callback, this);
 
   update_volume(op);
 
   pa_threaded_mainloop_unlock(m_mainloop);
-
 }
 
 /**
@@ -87,7 +89,6 @@ pulseaudio::~pulseaudio() {
   pa_context_disconnect(m_context);
   pa_context_unref(m_context);
   pa_threaded_mainloop_free(m_mainloop);
-
 }
 
 /**
@@ -133,8 +134,9 @@ int pulseaudio::process_events() {
       case evtype::REMOVE:
         o = pa_context_get_sink_info_by_name(m_context, DEFAULT_SINK, sink_info_callback, this);
         wait_loop(o, m_mainloop);
-        if (spec_s_name != s_name)
+        if (spec_s_name != s_name) {
           m_log.notice("pulseaudio: using default sink %s", s_name);
+        }
         break;
       default:
         break;
@@ -287,7 +289,6 @@ void pulseaudio::simple_callback(pa_context *, int success, void *userdata) {
   pa_threaded_mainloop_signal(This->m_mainloop, 0);
 }
 
-
 /**
  * Callback when getting sink info & existence
  */
@@ -321,8 +322,9 @@ void pulseaudio::context_state_callback(pa_context *context, void *userdata) {
 }
 
 inline void pulseaudio::wait_loop(pa_operation* op, pa_threaded_mainloop* loop) {
-  while (pa_operation_get_state(op) != PA_OPERATION_DONE)
+  while (pa_operation_get_state(op) != PA_OPERATION_DONE) {
     pa_threaded_mainloop_wait(loop);
+  }
   pa_operation_unref(op);
 }
 

src/adapters/script_runner.cpp

@@ -11,7 +11,7 @@
 
 POLYBAR_NS
 
-script_runner::script_runner(std::function<void(void)> on_update, const string& exec, const string& exec_if, bool tail,
+script_runner::script_runner(on_update on_update, const string& exec, const string& exec_if, bool tail,
     interval interval_success, interval interval_fail, const vector<pair<string, string>>& env)
     : m_log(logger::make())
     , m_on_update(on_update)
@@ -53,23 +53,6 @@ void script_runner::stop() {
   m_stopping = true;
 }
 
-int script_runner::get_pid() const {
-  return m_pid;
-}
-
-int script_runner::get_counter() const {
-  return m_counter;
-}
-
-int script_runner::get_exit_status() const { 
-  return m_exit_status;
-}
-
-string script_runner::get_output() {
-  std::lock_guard<std::mutex> guard(m_output_lock);
-  return m_output;
-}
-
 bool script_runner::is_stopping() const {
   return m_stopping;
 }
@@ -80,19 +63,28 @@ bool script_runner::is_stopping() const {
  * Returns true if the output changed.
  */
 bool script_runner::set_output(string&& new_output) {
-  std::lock_guard<std::mutex> guard(m_output_lock);
+  if (m_data.output != new_output) {
-
+    m_data.output = std::move(new_output);
-  if (m_output != new_output) {
-    m_output = std::move(new_output);
-    m_on_update();
     return true;
   }
 
   return false;
 }
 
+/**
+ * Updates the current exit status
+ *
+ * Returns true if the exit status changed.
+ */
+bool script_runner::set_exit_status(int new_status) {
+  auto changed = (m_data.exit_status != new_status);
+  m_data.exit_status = new_status;
+
+  return changed;
+}
+
 script_runner::interval script_runner::run() {
-  auto exec = string_util::replace_all(m_exec, "%counter%", to_string(++m_counter));
+  auto exec = string_util::replace_all(m_exec, "%counter%", to_string(++m_data.counter));
   m_log.info("script_runner: Invoking shell command: \"%s\"", exec);
   command<output_policy::REDIRECTED> cmd(m_log, exec);
 
@@ -106,6 +98,8 @@ script_runner::interval script_runner::run() {
   int fd = cmd.get_stdout(PIPE_READ);
   assert(fd != -1);
 
+  bool changed = false;
+
   bool got_output = false;
   while (!m_stopping && cmd.is_running() && !io_util::poll(fd, POLLHUP, 0)) {
     /**
@@ -113,7 +107,7 @@ script_runner::interval script_runner::run() {
      * down, we still need to continue polling.
      */
     if (io_util::poll_read(fd, 25) && !got_output) {
-      set_output(cmd.readline());
+      changed = set_output(cmd.readline());
       got_output = true;
     }
   }
@@ -123,9 +117,13 @@ script_runner::interval script_runner::run() {
     return 0s;
   }
 
-  m_exit_status = cmd.wait();
+  auto exit_status_changed = set_exit_status(cmd.wait());
 
-  if (m_exit_status == 0) {
+  if (changed || exit_status_changed) {
+    m_on_update(m_data);
+  }
+
+  if (m_data.exit_status == 0) {
     return m_interval_success;
   } else {
     return std::max(m_interval_fail, interval{1s});
@@ -133,7 +131,7 @@ script_runner::interval script_runner::run() {
 }
 
 script_runner::interval script_runner::run_tail() {
-  auto exec = string_util::replace_all(m_exec, "%counter%", to_string(++m_counter));
+  auto exec = string_util::replace_all(m_exec, "%counter%", to_string(++m_data.counter));
   m_log.info("script_runner: Invoking shell command: \"%s\"", exec);
   command<output_policy::REDIRECTED> cmd(m_log, exec);
 
@@ -143,15 +141,23 @@ script_runner::interval script_runner::run_tail() {
     throw modules::module_error("Failed to execute command: " + string(err.what()));
   }
 
-  scope_util::on_exit pid_guard([this]() { m_pid = -1; });
+  scope_util::on_exit pid_guard([this]() {
-  m_pid = cmd.get_pid();
+    m_data.pid = -1;
+    m_on_update(m_data);
+  });
+
+  m_data.pid = cmd.get_pid();
 
   int fd = cmd.get_stdout(PIPE_READ);
   assert(fd != -1);
 
   while (!m_stopping && cmd.is_running() && !io_util::poll(fd, POLLHUP, 0)) {
     if (io_util::poll_read(fd, 25)) {
-      set_output(cmd.readline());
+      auto changed = set_output(cmd.readline());
+
+      if (changed) {
+        m_on_update(m_data);
+      }
     }
   }
 

src/components/bar.cpp

@@ -141,17 +141,26 @@ bar::bar(connection& conn, signal_emitter& emitter, const config& config, const
   m_opts.dimvalue = m_conf.get(bs, "dim-value", 1.0);
   m_opts.dimvalue = math_util::cap(m_opts.dimvalue, 0.0, 1.0);
 
-  m_opts.cursor_click = m_conf.get(bs, "cursor-click", ""s);
-  m_opts.cursor_scroll = m_conf.get(bs, "cursor-scroll", ""s);
 #if WITH_XCURSOR
+  m_opts.cursor_click = m_conf.get(bs, "cursor-click", ""s);
   if (!m_opts.cursor_click.empty() && !cursor_util::valid(m_opts.cursor_click)) {
     m_log.warn("Ignoring unsupported cursor-click option '%s'", m_opts.cursor_click);
     m_opts.cursor_click.clear();
   }
+
+  m_opts.cursor_scroll = m_conf.get(bs, "cursor-scroll", ""s);
   if (!m_opts.cursor_scroll.empty() && !cursor_util::valid(m_opts.cursor_scroll)) {
     m_log.warn("Ignoring unsupported cursor-scroll option '%s'", m_opts.cursor_scroll);
     m_opts.cursor_scroll.clear();
   }
+#else
+  if (m_conf.has(bs, "cursor-click")) {
+    m_log.warn("Polybar was not compiled with xcursor support, ignoring cursor-click option");
+  }
+
+  if (m_conf.has(bs, "cursor-scroll")) {
+    m_log.warn("Polybar was not compiled with xcursor support, ignoring cursor-scroll option");
+  }
 #endif
 
   // Build WM_NAME
@@ -191,7 +200,7 @@ bar::bar(connection& conn, signal_emitter& emitter, const config& config, const
 
   // Load configuration values
 
-  m_opts.origin = m_conf.get(bs, "bottom", false) ? edge::BOTTOM : edge::TOP;
+  m_opts.bottom = m_conf.get(bs, "bottom", m_opts.bottom);
   m_opts.spacing = m_conf.get(bs, "spacing", m_opts.spacing);
   m_opts.separator = drawtypes::load_optional_label(m_conf, bs, "separator", "");
   m_opts.locale = m_conf.get(bs, "locale", ""s);
@@ -221,10 +230,8 @@ bar::bar(connection& conn, signal_emitter& emitter, const config& config, const
   // Load values used to adjust the struts atom
   auto margin_top = m_conf.get("global/wm", "margin-top", percentage_with_offset{});
   auto margin_bottom = m_conf.get("global/wm", "margin-bottom", percentage_with_offset{});
-  m_opts.strut.top =
+  m_opts.strut.top = units_utils::percentage_with_offset_to_pixel(margin_top, m_opts.monitor->h, m_opts.dpi_y);
-      units_utils::percentage_with_offset_to_pixel_nonnegative(margin_top, m_opts.monitor->h, m_opts.dpi_y);
+  m_opts.strut.bottom = units_utils::percentage_with_offset_to_pixel(margin_bottom, m_opts.monitor->h, m_opts.dpi_y);
-  m_opts.strut.bottom =
-      units_utils::percentage_with_offset_to_pixel_nonnegative(margin_bottom, m_opts.monitor->h, m_opts.dpi_y);
 
   // Load commands used for fallback click handlers
   vector<action> actions;
@@ -329,7 +336,7 @@ bar::bar(connection& conn, signal_emitter& emitter, const config& config, const
   m_opts.size.h += m_opts.borders[edge::TOP].size;
   m_opts.size.h += m_opts.borders[edge::BOTTOM].size;
 
-  if (m_opts.origin == edge::BOTTOM) {
+  if (m_opts.bottom) {
     m_opts.pos.y = m_opts.monitor->y + m_opts.monitor->h - m_opts.size.h - m_opts.offset.y;
   }
 
@@ -361,7 +368,7 @@ bar::~bar() {
 /**
  * Get the bar settings container
  */
-const bar_settings bar::settings() const {
+const bar_settings& bar::settings() const {
   return m_opts;
 }
 
@@ -454,14 +461,8 @@ void bar::show() {
   try {
     m_log.info("Showing bar window");
     m_sig.emit(visibility_change{true});
-    /**
+    map_window();
-     * First reconfigures the window so that WMs that discard some information
-     * when unmapping have the correct window properties (geometry etc).
-     */
-    reconfigure_window();
-    m_connection.map_window_checked(m_opts.window);
     m_connection.flush();
-    m_visible = true;
     parse(string{m_lastinput}, true);
   } catch (const exception& err) {
     m_log.err("Failed to map bar window (err=%s", err.what());
@@ -556,23 +557,41 @@ void bar::reconfigure_pos() {
  */
 void bar::reconfigure_struts() {
   auto geom = m_connection.get_geometry(m_screen->root());
-  auto w = m_opts.size.w + m_opts.offset.x;
+  int h = m_opts.size.h + m_opts.offset.y;
-  auto h = m_opts.size.h + m_opts.offset.y;
 
-  if (m_opts.origin == edge::BOTTOM) {
+  // Apply user-defined margins
+  if (m_opts.bottom) {
     h += m_opts.strut.top;
   } else {
     h += m_opts.strut.bottom;
   }
 
-  if (m_opts.origin == edge::BOTTOM && m_opts.monitor->y + m_opts.monitor->h < geom->height) {
+  h = std::max(h, 0);
-    h += geom->height - (m_opts.monitor->y + m_opts.monitor->h);
+
-  } else if (m_opts.origin != edge::BOTTOM) {
+  int correction = 0;
-    h += m_opts.monitor->y;
+
+  // Only apply correction if any space is requested
+  if (h > 0) {
+    /*
+     * Strut coordinates have to be relative to root window and not any monitor.
+     * If any monitor is not aligned at the top or bottom
+     */
+    if (m_opts.bottom) {
+      /*
+       * For bottom-algined bars, the correction is the number of pixels between
+       * the root window's bottom edge and the monitor's bottom edge
+       */
+      correction = geom->height - (m_opts.monitor->y + m_opts.monitor->h);
+    } else {
+      // For top-aligned bars, we simply add the monitor's y-position
+      correction = m_opts.monitor->y;
+    }
+
+    correction = std::max(correction, 0);
   }
 
   window win{m_connection, m_opts.window};
-  win.reconfigure_struts(w, h, m_opts.pos.x, m_opts.origin == edge::BOTTOM);
+  win.reconfigure_struts(m_opts.size.w, h + correction, m_opts.pos.x, m_opts.bottom);
 }
 
 /**
@@ -614,6 +633,25 @@ void bar::broadcast_visibility() {
   }
 }
 
+void bar::map_window() {
+  /**
+   * First reconfigures the window so that WMs that discard some information
+   * when unmapping have the correct window properties (geometry etc).
+   */
+  reconfigure_window();
+
+  m_log.trace("bar: Map window");
+  m_connection.map_window_checked(m_opts.window);
+
+  /*
+   * Required by AwesomeWM. AwesomeWM does not seem to respect polybar's position if WM_NORMAL_HINTS are set before
+   * mapping. Additionally updating the window position after mapping seems to fix that.
+   */
+  reconfigure_pos();
+
+  m_visible = true;
+}
+
 void bar::trigger_click(mousebtn btn, int pos) {
   tags::action_t action = m_action_ctxt->has_action(btn, pos);
 
@@ -695,13 +733,13 @@ void bar::handle(const evt::leave_notify&) {
 void bar::handle(const evt::motion_notify& evt) {
   m_log.trace("bar: Detected motion: %i at pos(%i, %i)", evt->detail, evt->event_x, evt->event_y);
 #if WITH_XCURSOR
-  m_motion_pos = evt->event_x;
+  int motion_pos = evt->event_x;
   // scroll cursor is less important than click cursor, so we shouldn't return until we are sure there is no click
   // action
   bool found_scroll = false;
   const auto has_action = [&](const vector<mousebtn>& buttons) -> bool {
     for (auto btn : buttons) {
-      if (m_action_ctxt->has_action(btn, m_motion_pos) != tags::NO_ACTION) {
+      if (m_action_ctxt->has_action(btn, motion_pos) != tags::NO_ACTION) {
         return true;
       }
     }
@@ -709,21 +747,14 @@ void bar::handle(const evt::motion_notify& evt) {
     return false;
   };
 
-  if (has_action({mousebtn::LEFT, mousebtn::MIDDLE, mousebtn::RIGHT, mousebtn::DOUBLE_LEFT, mousebtn::DOUBLE_MIDDLE,
+  if (!m_opts.cursor_click.empty() && has_action({mousebtn::LEFT, mousebtn::MIDDLE, mousebtn::RIGHT,
-          mousebtn::DOUBLE_RIGHT})) {
+                                          mousebtn::DOUBLE_LEFT, mousebtn::DOUBLE_MIDDLE, mousebtn::DOUBLE_RIGHT})) {
-    if (!string_util::compare(m_opts.cursor, m_opts.cursor_click)) {
+    change_cursor(m_opts.cursor_click);
-      m_opts.cursor = m_opts.cursor_click;
-      m_sig.emit(cursor_change{string{m_opts.cursor}});
-    }
-
     return;
   }
 
-  if (has_action({mousebtn::SCROLL_DOWN, mousebtn::SCROLL_UP})) {
+  if (!m_opts.cursor_scroll.empty() && has_action({mousebtn::SCROLL_DOWN, mousebtn::SCROLL_UP})) {
-    if (!string_util::compare(m_opts.cursor, m_opts.cursor_scroll)) {
+    change_cursor(m_opts.cursor_scroll);
-      m_opts.cursor = m_opts.cursor_scroll;
-      m_sig.emit(cursor_change{string{m_opts.cursor}});
-    }
     return;
   }
 
@@ -731,10 +762,7 @@ void bar::handle(const evt::motion_notify& evt) {
     if (!m_opts.cursor_click.empty() &&
         !(action.button == mousebtn::SCROLL_UP || action.button == mousebtn::SCROLL_DOWN ||
             action.button == mousebtn::NONE)) {
-      if (!string_util::compare(m_opts.cursor, m_opts.cursor_click)) {
+      change_cursor(m_opts.cursor_click);
-        m_opts.cursor = m_opts.cursor_click;
-        m_sig.emit(cursor_change{string{m_opts.cursor}});
-      }
       return true;
     } else if (!m_opts.cursor_scroll.empty() &&
                (action.button == mousebtn::SCROLL_UP || action.button == mousebtn::SCROLL_DOWN)) {
@@ -748,23 +776,20 @@ void bar::handle(const evt::motion_notify& evt) {
   for (auto&& action : m_opts.actions) {
     if (!action.command.empty()) {
       m_log.trace("Found matching fallback handler");
-      if (find_click_area(action))
+      if (find_click_area(action)) {
         return;
       }
     }
+  }
+
   if (found_scroll) {
-    if (!string_util::compare(m_opts.cursor, m_opts.cursor_scroll)) {
+    change_cursor(m_opts.cursor_scroll);
-      m_opts.cursor = m_opts.cursor_scroll;
-      m_sig.emit(cursor_change{string{m_opts.cursor}});
-    }
     return;
   }
-  if (!string_util::compare(m_opts.cursor, "default")) {
+
   m_log.trace("No matching cursor area found");
-    m_opts.cursor = "default";
+  change_cursor("default");
-    m_sig.emit(cursor_change{string{m_opts.cursor}});
   return;
-  }
 #endif
 }
 
@@ -867,10 +892,8 @@ void bar::start() {
   m_connection.ensure_event_mask(m_opts.window, XCB_EVENT_MASK_STRUCTURE_NOTIFY);
 
   m_log.info("Bar window: %s", m_connection.id(m_opts.window));
-  reconfigure_window();
 
-  m_log.trace("bar: Map window");
+  map_window();
-  m_connection.map_window_checked(m_opts.window);
 
   // With the mapping, the absolute position of our window may have changed (due to re-parenting for example).
   // Notify all components that depend on the absolute bar position (such as the background manager).
@@ -893,12 +916,17 @@ bool bar::on(const signals::ui::dim_window& sig) {
 }
 
 #if WITH_XCURSOR
-bool bar::on(const signals::ui::cursor_change& sig) {
+void bar::change_cursor(const string& name) {
-  if (!cursor_util::set_cursor(m_connection, m_connection.screen(), m_opts.window, sig.cast())) {
+  // This is already the same cursor, no need to update
-    m_log.warn("Failed to create cursor context");
+  if (m_cursor == name) {
+    return;
+  }
+  m_cursor = name;
+
+  if (!cursor_util::set_cursor(m_connection, m_connection.screen(), m_opts.window, name)) {
+    m_log.warn("Failed to create cursor context for cursor name '%s'", name);
   }
   m_connection.flush();
-  return false;
 }
 #endif
 

src/components/builder.cpp

@@ -31,19 +31,8 @@ void builder::reset() {
   m_tags[syntaxtag::u] = 0;
   m_tags[syntaxtag::P] = 0;
 
-  m_colors.clear();
-  m_colors[syntaxtag::B] = string();
-  m_colors[syntaxtag::F] = string();
-  m_colors[syntaxtag::o] = string();
-  m_colors[syntaxtag::u] = string();
-
   m_attrs.clear();
-  m_attrs[attribute::NONE] = false;
-  m_attrs[attribute::UNDERLINE] = false;
-  m_attrs[attribute::OVERLINE] = false;
-
   m_output.clear();
-  m_fontindex = 1;
 }
 
 /**
@@ -52,33 +41,20 @@ void builder::reset() {
  * This will also close any unclosed tags
  */
 string builder::flush() {
-  if (m_tags[syntaxtag::B]) {
   background_close();
-  }
+  foreground_close();
-  if (m_tags[syntaxtag::F]) {
-    color_close();
-  }
-  if (m_tags[syntaxtag::T]) {
   font_close();
-  }
-  if (m_tags[syntaxtag::o]) {
   overline_color_close();
-  }
-  if (m_tags[syntaxtag::u]) {
   underline_color_close();
-  }
-  if (m_attrs[attribute::UNDERLINE]) {
   underline_close();
-  }
-  if (m_attrs[attribute::OVERLINE]) {
   overline_close();
-  }
 
   while (m_tags[syntaxtag::A]) {
     action_close();
   }
 
-  string output{m_output};
+  string output{};
+  std::swap(m_output, output);
 
   reset();
 
@@ -142,7 +118,7 @@ void builder::node(const label_t& label) {
     background(label->m_background);
   }
   if (label->m_foreground.has_color()) {
-    color(label->m_foreground);
+    foreground(label->m_foreground);
   }
 
   if (label->m_padding.left) {
@@ -159,7 +135,7 @@ void builder::node(const label_t& label) {
     background_close();
   }
   if (label->m_foreground.has_color()) {
-    color_close();
+    foreground_close();
   }
 
   if (label->m_underline.has_color()) {
@@ -174,18 +150,6 @@ void builder::node(const label_t& label) {
   }
 }
 
-/**
- * Repeat text string n times
- */
-void builder::node_repeat(const string& str, size_t n) {
-  string text;
-  text.reserve(str.size() * n);
-  while (n--) {
-    text += str;
-  }
-  node(text);
-}
-
 /**
  * Repeat label contents n times
  */
@@ -196,6 +160,7 @@ void builder::node_repeat(const label_t& label, size_t n) {
   while (n--) {
     text += label_text;
   }
+
   label_t tmp{new label_t::element_type{text}};
   tmp->replace_defined_values(label);
   node(tmp);
@@ -205,7 +170,7 @@ void builder::node_repeat(const label_t& label, size_t n) {
  * Insert tag that will offset the contents by the given extent
  */
 void builder::offset(extent_val extent) {
-  if (extent) {
+  if (!extent) {
     return;
   }
   tag_open(syntaxtag::O, units_utils::extent_to_string(extent));
@@ -233,7 +198,6 @@ void builder::font(int index) {
   if (index == 0) {
     return;
   }
-  m_fontindex = index;
   tag_open(syntaxtag::T, to_string(index));
 }
 
@@ -241,7 +205,6 @@ void builder::font(int index) {
  * Insert tag to reset the font index
  */
 void builder::font_close() {
-  m_fontindex = 1;
   tag_close(syntaxtag::T);
 }
 
@@ -252,7 +215,6 @@ void builder::background(rgba color) {
   color = color.try_apply_alpha_to(m_bar.background);
 
   auto hex = color_util::simplify_hex(color);
-  m_colors[syntaxtag::B] = hex;
   tag_open(syntaxtag::B, hex);
 }
 
@@ -260,88 +222,47 @@ void builder::background(rgba color) {
  * Insert tag to reset the background color
  */
 void builder::background_close() {
-  m_colors[syntaxtag::B].clear();
   tag_close(syntaxtag::B);
 }
 
 /**
  * Insert tag to alter the current foreground color
  */
-void builder::color(rgba color) {
+void builder::foreground(rgba color) {
   color = color.try_apply_alpha_to(m_bar.foreground);
 
   auto hex = color_util::simplify_hex(color);
-  m_colors[syntaxtag::F] = hex;
   tag_open(syntaxtag::F, hex);
 }
 
 /**
  * Insert tag to reset the foreground color
  */
-void builder::color_close() {
+void builder::foreground_close() {
-  m_colors[syntaxtag::F].clear();
   tag_close(syntaxtag::F);
 }
 
-/**
- * Insert tag to alter the current overline/underline color
- */
-void builder::line_color(const rgba& color) {
-  overline_color(color);
-  underline_color(color);
-}
-
-/**
- * Close overline/underline color tag
- */
-void builder::line_color_close() {
-  overline_color_close();
-  underline_color_close();
-}
-
-/**
- * Insert tag to alter the current overline color
- */
-void builder::overline_color(rgba color) {
-  auto hex = color_util::simplify_hex(color);
-  m_colors[syntaxtag::o] = hex;
-  tag_open(syntaxtag::o, hex);
-  tag_open(attribute::OVERLINE);
-}
-
 /**
  * Close underline color tag
  */
 void builder::overline_color_close() {
-  m_colors[syntaxtag::o].clear();
   tag_close(syntaxtag::o);
 }
 
-/**
- * Insert tag to alter the current underline color
- */
-void builder::underline_color(rgba color) {
-  auto hex = color_util::simplify_hex(color);
-  m_colors[syntaxtag::u] = hex;
-  tag_open(syntaxtag::u, hex);
-  tag_open(attribute::UNDERLINE);
-}
-
 /**
  * Close underline color tag
  */
 void builder::underline_color_close() {
   tag_close(syntaxtag::u);
-  m_colors[syntaxtag::u].clear();
 }
 
 /**
- * Insert tag to enable the overline attribute
+ * Insert tag to enable the overline attribute with the given color
  */
 void builder::overline(const rgba& color) {
   if (color.has_color()) {
-    overline_color(color);
+    auto hex = color_util::simplify_hex(color);
-  } else {
+    tag_open(syntaxtag::o, hex);
     tag_open(attribute::OVERLINE);
   }
 }
@@ -354,12 +275,12 @@ void builder::overline_close() {
 }
 
 /**
- * Insert tag to enable the underline attribute
+ * Insert tag to enable the underline attribute with the given color
  */
 void builder::underline(const rgba& color) {
   if (color.has_color()) {
-    underline_color(color);
+    auto hex = color_util::simplify_hex(color);
-  } else {
+    tag_open(syntaxtag::u, hex);
     tag_open(attribute::UNDERLINE);
   }
 }
@@ -381,7 +302,7 @@ void builder::control(controltag tag) {
       str = "R";
       break;
     default:
-      break;
+      throw runtime_error("Invalid controltag: " + to_string(to_integral(tag)));
   }
 
   if (!str.empty()) {
@@ -397,7 +318,7 @@ void builder::control(controltag tag) {
 void builder::action(mousebtn index, string action) {
   if (!action.empty()) {
     action = string_util::replace_all(action, ":", "\\:");
-    tag_open(syntaxtag::A, to_string(static_cast<int>(index)) + ":" + action + ":");
+    tag_open(syntaxtag::A, to_string(to_integral(index)) + ":" + action + ":");
   }
 }
 
@@ -415,7 +336,7 @@ void builder::action(mousebtn index, string action_name, const label_t& label) {
   if (label && *label) {
     action(index, action_name);
     node(label);
-    tag_close(syntaxtag::A);
+    action_close();
   }
 }
 
@@ -477,6 +398,8 @@ void builder::tag_open(syntaxtag tag, const string& value) {
     case syntaxtag::r:
       append("%{r}");
       break;
+    default:
+      throw runtime_error("Invalid tag: " + to_string(to_integral(tag)));
   }
 }
 
@@ -484,21 +407,22 @@ void builder::tag_open(syntaxtag tag, const string& value) {
  * Insert directive to use given attribute unless already set
  */
 void builder::tag_open(attribute attr) {
-  if (m_attrs[attr]) {
+  // Don't emit activation tag if the attribute is already activated
+  if (m_attrs.count(attr) != 0) {
     return;
   }
 
-  m_attrs[attr] = true;
+  m_attrs.insert(attr);
 
   switch (attr) {
-    case attribute::NONE:
-      break;
     case attribute::UNDERLINE:
       append("%{+u}");
       break;
     case attribute::OVERLINE:
       append("%{+o}");
       break;
+    default:
+      throw runtime_error("Invalid attribute: " + to_string(to_integral(attr)));
   }
 }
 
@@ -531,13 +455,8 @@ void builder::tag_close(syntaxtag tag) {
     case syntaxtag::o:
       append("%{o-}");
       break;
-    case syntaxtag::R:
+    default:
-    case syntaxtag::P:
+      throw runtime_error("Cannot close syntaxtag: " + to_string(to_integral(tag)));
-    case syntaxtag::O:
-    case syntaxtag::l:
-    case syntaxtag::c:
-    case syntaxtag::r:
-      break;
   }
 }
 
@@ -545,51 +464,34 @@ void builder::tag_close(syntaxtag tag) {
  * Insert directive to remove given attribute if set
  */
 void builder::tag_close(attribute attr) {
-  if (!m_attrs[attr]) {
+  // Don't close activation tag if it wasn't activated
+  if (m_attrs.erase(attr) == 0) {
     return;
   }
 
-  m_attrs[attr] = false;
-
   switch (attr) {
-    case attribute::NONE:
-      break;
     case attribute::UNDERLINE:
       append("%{-u}");
       break;
     case attribute::OVERLINE:
       append("%{-o}");
       break;
+    default:
+      throw runtime_error("Invalid attribute: " + to_string(to_integral(attr)));
   }
 }
 
-string builder::get_spacing_format_string(const spacing_val& space) {
+string builder::get_spacing_format_string(spacing_val space) {
   float value = space.value;
   if (value == 0) {
     return "";
   }
 
-  string out;
   if (space.type == spacing_type::SPACE) {
-    out += string(value, ' ');
+    return string(value, ' ');
   } else {
-    out += "%{O";
+    return "%{O" + units_utils::extent_to_string(units_utils::spacing_to_extent(space)) + "}";
-
-    switch (space.type) {
-      case spacing_type::POINT:
-        out += to_string(value) + "pt";
-        break;
-      case spacing_type::PIXEL:
-        out += to_string(static_cast<int>(value)) + "px";
-        break;
-      default:
-        break;
   }
-
-    out += '}';
-  }
-
-  return out;
 }
 
 POLYBAR_NS_END

src/components/controller.cpp

@@ -63,6 +63,8 @@ controller::controller(
   if (!created_modules) {
     throw application_error("No modules created");
   }
+
+  m_log.notice("Loaded %zd modules", created_modules);
 }
 
 /**
@@ -178,8 +180,8 @@ void controller::conn_cb() {
       m_log.err("X connection error, terminating... (what: %s)", m_connection.error_str(err.code()));
       stop(false);
     } catch (const exception& err) {
-      m_log.err("Error in X event loop: %s", err.what());
+      // IDs for events are defined in xproto.h
-      stop(false);
+      m_log.err("Error in X event loop while handling event %d: %s", evt->response_type, err.what());
     }
   }
 
@@ -597,6 +599,7 @@ size_t controller::setup_modules(alignment align) {
         throw application_error("Inter-process messaging needs to be enabled");
       }
 
+      m_log.notice("Loading module '%s' of type '%s'", module_name, type);
       auto ptr = make_module(move(type), m_bar->settings(), module_name, m_log);
       module_t module = shared_ptr<modules::module_interface>(ptr);
       ptr = nullptr;
@@ -608,7 +611,7 @@ size_t controller::setup_modules(alignment align) {
     }
   }
 
-  return m_modules.size();
+  return m_blocks[align].size();
 }
 
 /**

src/modules/alsa.cpp

@@ -212,7 +212,7 @@ namespace modules {
       m_builder->action(mousebtn::SCROLL_DOWN, *this, EVENT_DEC, "");
     }
 
-    m_builder->append(output);
+    m_builder->node(output);
 
     return m_builder->flush();
   }

src/modules/backlight.cpp

@@ -98,7 +98,7 @@ namespace modules {
       m_builder->action(mousebtn::SCROLL_DOWN, *this, EVENT_DEC, "");
     }
 
-    m_builder->append(std::move(output));
+    m_builder->node(output);
 
     m_builder->action_close();
     m_builder->action_close();

src/modules/ipc.cpp

@@ -99,7 +99,7 @@ namespace modules {
       }
     }
 
-    m_builder->append(output);
+    m_builder->node(output);
     return m_builder->flush();
   }
 

src/modules/meta/base.cpp

@@ -25,7 +25,7 @@ namespace modules {
       builder->background(bg);
     }
     if (fg.has_color()) {
-      builder->color(fg);
+      builder->foreground(fg);
     }
     if (ul.has_color()) {
       builder->underline(ul);
@@ -46,7 +46,7 @@ namespace modules {
       builder->background(bg);
     }
     if (fg.has_color()) {
-      builder->color(fg);
+      builder->foreground(fg);
     }
     if (ul.has_color()) {
       builder->underline(ul);
@@ -55,7 +55,7 @@ namespace modules {
       builder->overline(ol);
     }
 
-    builder->append(move(output));
+    builder->node(output);
     builder->node(suffix);
 
     if (padding) {
@@ -71,7 +71,7 @@ namespace modules {
       builder->underline_close();
     }
     if (fg.has_color()) {
-      builder->color_close();
+      builder->foreground_close();
     }
     if (bg.has_color()) {
       builder->background_close();

src/modules/pulseaudio.cpp

@@ -59,8 +59,9 @@ namespace modules {
   bool pulseaudio_module::has_event() {
     // Poll for mixer and control events
     try {
-      if (m_pulseaudio->wait())
+      if (m_pulseaudio->wait()) {
         return true;
+      }
     } catch (const pulseaudio_error& e) {
       m_log.err("%s: %s", name(), e.what());
     }
@@ -134,7 +135,7 @@ namespace modules {
       }
     }
 
-    m_builder->append(output);
+    m_builder->node(output);
 
     return m_builder->flush();
   }

src/modules/script.cpp

@@ -12,8 +12,9 @@ namespace modules {
       , m_interval_success(m_conf.get<script_runner::interval>(name(), "interval", m_tail ? 0s : 5s))
       , m_interval_fail(m_conf.get<script_runner::interval>(name(), "interval-fail", m_interval_success))
       , m_interval_if(m_conf.get<script_runner::interval>(name(), "interval-if", m_interval_success))
-      , m_runner([this]() { broadcast(); }, m_conf.get(name(), "exec", ""s), m_conf.get(name(), "exec-if", ""s), m_tail,
+      , m_runner([this](const auto& data) { handle_runner_update(data); }, m_conf.get(name(), "exec", ""s),
-            m_interval_success, m_interval_fail, m_conf.get_with_prefix(name(), "env-")) {
+            m_conf.get(name(), "exec-if", ""s), m_tail, m_interval_success, m_interval_fail,
+            m_conf.get_with_prefix(name(), "env-")) {
     // Load configured click handlers
     m_actions[mousebtn::LEFT] = m_conf.get(name(), "click-left", ""s);
     m_actions[mousebtn::MIDDLE] = m_conf.get(name(), "click-middle", ""s);
@@ -78,29 +79,35 @@ namespace modules {
    * Generate module output
    */
   string script_module::get_format() const {
-    if (m_runner.get_exit_status() != 0 && m_conf.has(name(), FORMAT_FAIL)) {
+    if (m_exit_status != 0 && m_conf.has(name(), FORMAT_FAIL)) {
       return FORMAT_FAIL;
     }
     return DEFAULT_FORMAT;
   }
 
   string script_module::get_output() {
-    auto script_output = m_runner.get_output();
+    auto data = [this] {
-    if (script_output.empty()) {
+      std::lock_guard<std::mutex> lk(m_data_mutex);
+      return m_data;
+    }();
+
+    m_exit_status = data.exit_status;
+
+    if (data.output.empty()) {
       return "";
     }
 
     if (m_label) {
       m_label->reset_tokens();
-      m_label->replace_token("%output%", script_output);
+      m_label->replace_token("%output%", data.output);
     }
 
     if (m_label_fail) {
       m_label_fail->reset_tokens();
-      m_label_fail->replace_token("%output%", script_output);
+      m_label_fail->replace_token("%output%", data.output);
     }
 
-    string cnt{to_string(m_runner.get_counter())};
+    string cnt{to_string(data.counter)};
     string output{module::get_output()};
 
     for (const auto& a : m_actions) {
@@ -114,15 +121,14 @@ namespace modules {
          * The pid token is only for tailed commands.
          * If the command is not specified or running, replacement is unnecessary as well
          */
-        int pid = m_runner.get_pid();
+        if (data.pid != -1) {
-        if (pid != -1) {
+          action_replaced = string_util::replace_all(action_replaced, "%pid%", to_string(data.pid));
-          action_replaced = string_util::replace_all(action_replaced, "%pid%", to_string(pid));
         }
         m_builder->action(btn, action_replaced);
       }
     }
 
-    m_builder->append(output);
+    m_builder->node(output);
 
     return m_builder->flush();
   }
@@ -141,6 +147,15 @@ namespace modules {
 
     return true;
   }
+
+  void script_module::handle_runner_update(const script_runner::data& data) {
+    {
+      std::lock_guard<std::mutex> lk(m_data_mutex);
+      m_data = data;
+    }
+
+    broadcast();
+  }
 } // namespace modules
 
 POLYBAR_NS_END

src/modules/systray.cpp

@@ -45,7 +45,7 @@ namespace modules {
     if (tag == TAG_LABEL_TOGGLE) {
       builder->action(mousebtn::LEFT, *this, EVENT_TOGGLE, "", m_label);
     } else if (tag == TAG_TRAY_CLIENTS && !m_hidden) {
-      builder->append(TRAY_PLACEHOLDER);
+      builder->node(TRAY_PLACEHOLDER);
     } else {
       return false;
     }

src/modules/text.cpp

@@ -47,7 +47,7 @@ namespace modules {
       m_builder->action(mousebtn::SCROLL_DOWN, scroll_down);
     }
 
-    m_builder->append(output);
+    m_builder->node(output);
 
     return m_builder->flush();
   }

src/modules/xbacklight.cpp

@@ -123,7 +123,7 @@ namespace modules {
       m_builder->action(mousebtn::SCROLL_DOWN, *this, EVENT_DEC, "");
     }
 
-    m_builder->append(output);
+    m_builder->node(output);
 
     m_builder->action_close();
     m_builder->action_close();

src/modules/xkeyboard.cpp

@@ -167,10 +167,10 @@ namespace modules {
 
     if (m_keyboard && m_keyboard->size() > 1) {
       m_builder->action(mousebtn::LEFT, *this, EVENT_SWITCH, "");
-      m_builder->append(output);
+      m_builder->node(output);
       m_builder->action_close();
     } else {
-      m_builder->append(output);
+      m_builder->node(output);
     }
 
     return m_builder->flush();

src/modules/xworkspaces.cpp

@@ -135,8 +135,18 @@ namespace modules {
 
     for (auto&& client : newclients) {
       if (m_clients.count(client) == 0) {
+        try {
           // new client: listen for changes (wm_hint or desktop)
           m_connection.ensure_event_mask(client, XCB_EVENT_MASK_PROPERTY_CHANGE);
+        } catch (const xpp::x::error::window& e) {
+          /*
+           * The "new client" may have already disappeared between reading the
+           * client list and setting the event mask.
+           * This is not a severe issue and it will eventually correct itself
+           * when a new _NET_CLIENT_LIST value is set.
+           */
+          m_log.info("%s: New client window no longer exists, ignoring...");
+        }
       }
     }
 
@@ -185,8 +195,8 @@ namespace modules {
      *
      * For WMs that don't support that hint, we store an empty vector
      *
-     * If the length of the vector is less than _NET_NUMBER_OF_DESKTOPS
+     * The vector will be padded/reduced to _NET_NUMBER_OF_DESKTOPS.
-     * all desktops which aren't explicitly assigned a postion will be
+     * All desktops which aren't explicitly assigned a postion will be
      * assigned (0, 0)
      *
      * We use this to map workspaces to viewports, desktop i is at position
@@ -194,16 +204,31 @@ namespace modules {
      */
     vector<position> ws_positions = ewmh_util::get_desktop_viewports();
 
+    auto num_desktops = m_desktop_names.size();
+
     /*
      * Not all desktops were assigned a viewport, add (0, 0) for all missing
      * desktops.
      */
-    if (ws_positions.size() < m_desktop_names.size()) {
+    if (ws_positions.size() < num_desktops) {
-      auto num_insert = m_desktop_names.size() - ws_positions.size();
+      auto num_insert = num_desktops - ws_positions.size();
-      ws_positions.reserve(num_insert);
+      ws_positions.reserve(num_desktops);
       std::fill_n(std::back_inserter(ws_positions), num_insert, position{0, 0});
     }
 
+    /*
+     * If there are too many workspace positions, trim to fit the number of desktops.
+     */
+    if (ws_positions.size() > num_desktops) {
+      ws_positions.erase(ws_positions.begin() + num_desktops, ws_positions.end());
+    }
+
+    /*
+     * There must be as many workspace positions as desktops because the indices
+     * into ws_positions are also used to index into m_desktop_names.
+     */
+    assert(ws_positions.size() == num_desktops);
+
     /*
      * The list of viewports is the set of unique positions in ws_positions
      * Using a set automatically removes duplicates.
@@ -239,6 +264,10 @@ namespace modules {
           return label;
         }();
 
+        /*
+         * Search for all desktops on this viewport and store them in the
+         * desktop list of the viewport.
+         */
         for (size_t i = 0; i < ws_positions.size(); i++) {
           auto&& ws_pos = ws_positions[i];
           if (ws_pos == viewport_pos) {
@@ -322,7 +351,7 @@ namespace modules {
       m_builder->action(mousebtn::SCROLL_UP, *this, m_revscroll ? EVENT_PREV : EVENT_NEXT, "");
     }
 
-    m_builder->append(output);
+    m_builder->node(output);
 
     m_builder->action_close();
     m_builder->action_close();

src/utils/units.cpp

@@ -45,6 +45,9 @@ namespace units_utils {
     return static_cast<int>(math_util::percentage_to_value<double, double>(g_format.percentage, max) + offset_pixel);
   }
 
+  /**
+   * Same as percentage_with_offset_to_pixel but capped below at 0 pixels
+   */
   unsigned percentage_with_offset_to_pixel_nonnegative(percentage_with_offset g_format, double max, double dpi) {
     return std::max<int>(0, percentage_with_offset_to_pixel(g_format, max, dpi));
   }
@@ -135,6 +138,28 @@ namespace units_utils {
     return {type, size_value};
   }
 
+  /**
+   * Creates an extent from a spacing value.
+   *
+   * @param spacing Value to convert, must not be SPACE
+   */
+  extent_val spacing_to_extent(spacing_val spacing) {
+    extent_type t;
+
+    switch (spacing.type) {
+      case spacing_type::POINT:
+        t = extent_type::POINT;
+        break;
+      case spacing_type::PIXEL:
+        t = extent_type::PIXEL;
+        break;
+      default:
+        throw std::runtime_error("spacing_to_extent: Illegal type: " + to_string(to_integral(spacing.type)));
+    }
+
+    return {t, spacing.value};
+  }
+
 } // namespace units_utils
 
 POLYBAR_NS_END

src/x11/cursor.cpp

@@ -1,27 +1,43 @@
 #include "x11/cursor.hpp"
 
+#include "utils/scope.hpp"
+
 POLYBAR_NS
 
 namespace cursor_util {
-  bool valid(string name) {
+  bool valid(const string& name) {
     return (cursors.find(name) != cursors.end());
   }
 
-  bool set_cursor(xcb_connection_t *c, xcb_screen_t *screen, xcb_window_t w, string name) {
+  bool set_cursor(xcb_connection_t* c, xcb_screen_t* screen, xcb_window_t w, const string& name) {
-    xcb_cursor_t cursor = XCB_CURSOR_NONE;
+    if (!valid(name)) {
+      throw std::runtime_error("Tried to set cursor to invalid name: '" + name + "'");
+    }
+
     xcb_cursor_context_t* ctx;
 
     if (xcb_cursor_context_new(c, screen, &ctx) < 0) {
       return false;
     }
-    for (auto&& cursor_name : cursors.at(name)) {
+
+    scope_util::on_exit handler([&] { xcb_cursor_context_free(ctx); });
+
+    xcb_cursor_t cursor = XCB_CURSOR_NONE;
+    for (const auto& cursor_name : cursors.at(name)) {
       cursor = xcb_cursor_load_cursor(ctx, cursor_name.c_str());
-      if (cursor != XCB_CURSOR_NONE)
+      if (cursor != XCB_CURSOR_NONE) {
         break;
       }
+    }
+
+    if (cursor == XCB_CURSOR_NONE) {
+      return false;
+    }
+
     xcb_change_window_attributes(c, w, XCB_CW_CURSOR, &cursor);
-    xcb_cursor_context_free(ctx);
+    xcb_free_cursor(c, cursor);
+
     return true;
   }
-}
+} // namespace cursor_util
 POLYBAR_NS_END

src/x11/icccm.cpp

@@ -1,4 +1,5 @@
 #include "x11/icccm.hpp"
+
 #include "x11/atoms.hpp"
 
 POLYBAR_NS
@@ -40,7 +41,7 @@ namespace icccm_util {
   }
 
   void set_wm_size_hints(xcb_connection_t* c, xcb_window_t w, int x, int y, int width, int height) {
-    xcb_size_hints_t hints;
+    xcb_size_hints_t hints{};
 
     xcb_icccm_size_hints_set_size(&hints, false, width, height);
     xcb_icccm_size_hints_set_min_size(&hints, width, height);
@@ -50,6 +51,6 @@ namespace icccm_util {
 
     xcb_icccm_set_wm_size_hints(c, w, XCB_ATOM_WM_NORMAL_HINTS, &hints);
   }
-}
+} // namespace icccm_util
 
 POLYBAR_NS_END

src/x11/window.cpp

@@ -1,9 +1,10 @@
+#include "x11/window.hpp"
+
 #include "components/types.hpp"
 #include "utils/memory.hpp"
 #include "x11/atoms.hpp"
 #include "x11/connection.hpp"
 #include "x11/extensions/randr.hpp"
-#include "x11/window.hpp"
 
 POLYBAR_NS
 
@@ -45,24 +46,31 @@ window window::reconfigure_pos(short int x, short int y) {
 
 /**
  * Reconfigure the windows ewmh strut
+ *
+ * Ref: https://specifications.freedesktop.org/wm-spec/wm-spec-latest.html#idm45381391268672
+ *
+ * @param w Width of the bar window
+ * @param strut Size of the reserved space. Height of the window, corrected for unaligned monitors
+ * @param x The absolute x-position of the bar window (top-left corner)
+ * @param bottom Whether the bar is at the bottom of the screen
  */
-window window::reconfigure_struts(unsigned short int w, unsigned short int h, short int x, bool bottom) {
+window window::reconfigure_struts(uint32_t w, uint32_t strut, uint32_t x, bool bottom) {
-  unsigned int none{0};
+  std::array<uint32_t, 12> values{};
-  unsigned int values[12]{none};
 
   if (bottom) {
-    values[static_cast<int>(strut::BOTTOM)] = h;
+    values[to_integral(strut::BOTTOM)] = strut;
-    values[static_cast<int>(strut::BOTTOM_START_X)] = x;
+    values[to_integral(strut::BOTTOM_START_X)] = x;
-    values[static_cast<int>(strut::BOTTOM_END_X)] = x + w - 1;
+    values[to_integral(strut::BOTTOM_END_X)] = x + w - 1;
   } else {
-    values[static_cast<int>(strut::TOP)] = h;
+    values[to_integral(strut::TOP)] = strut;
-    values[static_cast<int>(strut::TOP_START_X)] = x;
+    values[to_integral(strut::TOP_START_X)] = x;
-    values[static_cast<int>(strut::TOP_END_X)] = x + w - 1;
+    values[to_integral(strut::TOP_END_X)] = x + w - 1;
   }
 
-  connection().change_property_checked(XCB_PROP_MODE_REPLACE, *this, _NET_WM_STRUT, XCB_ATOM_CARDINAL, 32, 4, values);
   connection().change_property_checked(
-      XCB_PROP_MODE_REPLACE, *this, _NET_WM_STRUT_PARTIAL, XCB_ATOM_CARDINAL, 32, 12, values);
+      XCB_PROP_MODE_REPLACE, *this, _NET_WM_STRUT, XCB_ATOM_CARDINAL, 32, 4, values.data());
+  connection().change_property_checked(
+      XCB_PROP_MODE_REPLACE, *this, _NET_WM_STRUT_PARTIAL, XCB_ATOM_CARDINAL, 32, 12, values.data());
 
   return *this;
 }

tests/CMakeLists.txt

@@ -57,6 +57,7 @@ add_unit_test(utils/string)
 add_unit_test(utils/file)
 add_unit_test(utils/process)
 add_unit_test(utils/units)
+add_unit_test(components/builder)
 add_unit_test(components/command_line)
 add_unit_test(components/config_parser)
 add_unit_test(drawtypes/label)

tests/unit_tests/components/builder.cpp

@@ -0,0 +1,142 @@
+#include "components/builder.hpp"
+
+#include "common/test.hpp"
+
+using namespace polybar;
+
+static const rgba FG = rgba("#00FF00");
+static const rgba BG = rgba("#FF0000");
+
+class BuilderTest : public ::testing::Test {
+ protected:
+  virtual void SetUp() override {
+    opts.foreground = FG;
+    opts.background = BG;
+    opts.spacing = ZERO_SPACE;
+  }
+
+  bar_settings opts{};
+  builder b{opts};
+};
+
+TEST_F(BuilderTest, empty) {
+  EXPECT_EQ("", b.flush());
+}
+
+TEST_F(BuilderTest, text) {
+  b.node("foo");
+  EXPECT_EQ("foo", b.flush());
+}
+
+TEST_F(BuilderTest, textFont) {
+  b.node("foo", 12);
+  EXPECT_EQ("%{T12}foo%{T-}", b.flush());
+}
+
+using offset_test = std::pair<extent_val, string>;
+class OffsetTest : public BuilderTest, public ::testing::WithParamInterface<offset_test> {};
+
+vector<offset_test> offset_test_list = {
+    {ZERO_PX_EXTENT, ""},
+    {{extent_type::POINT, 0}, ""},
+    {{extent_type::PIXEL, 10}, "%{O10px}"},
+    {{extent_type::PIXEL, -10}, "%{O-10px}"},
+    {{extent_type::POINT, 23}, "%{O23pt}"},
+    {{extent_type::POINT, -1}, "%{O-1pt}"},
+};
+
+INSTANTIATE_TEST_SUITE_P(Inst, OffsetTest, ::testing::ValuesIn(offset_test_list));
+
+TEST_P(OffsetTest, correctness) {
+  b.offset(GetParam().first);
+  EXPECT_EQ(GetParam().second, b.flush());
+}
+
+using spacing_test = std::pair<spacing_val, string>;
+class SpacingTest : public BuilderTest, public ::testing::WithParamInterface<spacing_test> {};
+
+vector<spacing_test> spacing_test_list = {
+    {ZERO_SPACE, ""},
+    {{spacing_type::SPACE, 2}, "  "},
+    {{spacing_type::PIXEL, 3}, "%{O3px}"},
+    {{spacing_type::POINT, 4}, "%{O4pt}"},
+};
+
+INSTANTIATE_TEST_SUITE_P(Inst, SpacingTest, ::testing::ValuesIn(spacing_test_list));
+
+TEST_P(SpacingTest, correctness) {
+  b.spacing(GetParam().first);
+  EXPECT_EQ(GetParam().second, b.flush());
+}
+
+TEST_P(SpacingTest, get_spacing_format_string) {
+  b.spacing(GetParam().first);
+  EXPECT_EQ(GetParam().second, b.flush());
+}
+
+TEST_F(BuilderTest, tags) {
+  b.font(10);
+  b.font_close();
+  EXPECT_EQ("%{T10}%{T-}", b.flush());
+
+  b.background(rgba("#f0f0f0"));
+  b.background_close();
+  EXPECT_EQ("%{B#f0f0f0}%{B-}", b.flush());
+
+  b.foreground(rgba("#0f0f0f"));
+  b.foreground_close();
+  EXPECT_EQ("%{F#0f0f0f}%{F-}", b.flush());
+
+  b.overline(rgba("#0e0e0e"));
+  b.overline_close();
+  // Last tag is from auto-closing during flush
+  EXPECT_EQ("%{o#0e0e0e}%{+o}%{-o}%{o-}", b.flush());
+
+  b.underline(rgba("#0d0d0d"));
+  b.underline_close();
+  // Last tag is from auto-closing during flush
+  EXPECT_EQ("%{u#0d0d0d}%{+u}%{-u}%{u-}", b.flush());
+
+  b.control(tags::controltag::R);
+  EXPECT_EQ("%{PR}", b.flush());
+
+  b.action(mousebtn::LEFT, "cmd");
+  b.action_close();
+  EXPECT_EQ("%{A1:cmd:}%{A}", b.flush());
+}
+
+TEST_F(BuilderTest, tagsAutoClose) {
+  b.font(20);
+  EXPECT_EQ("%{T20}%{T-}", b.flush());
+
+  b.background(rgba("#f0f0f0"));
+  EXPECT_EQ("%{B#f0f0f0}%{B-}", b.flush());
+
+  b.foreground(rgba("#0f0f0f"));
+  EXPECT_EQ("%{F#0f0f0f}%{F-}", b.flush());
+
+  b.overline(rgba("#0e0e0e"));
+  EXPECT_EQ("%{o#0e0e0e}%{+o}%{o-}%{-o}", b.flush());
+
+  b.underline(rgba("#0d0d0d"));
+  EXPECT_EQ("%{u#0d0d0d}%{+u}%{u-}%{-u}", b.flush());
+
+  b.action(mousebtn::LEFT, "cmd");
+  EXPECT_EQ("%{A1:cmd:}%{A}", b.flush());
+}
+
+TEST_F(BuilderTest, invalidTags) {
+  EXPECT_THROW(b.control(tags::controltag::NONE), std::runtime_error);
+}
+
+TEST_F(BuilderTest, colorBlending) {
+  b.background(rgba(0x12000000, rgba::type::ALPHA_ONLY));
+  b.background_close();
+
+  EXPECT_EQ("%{B#12ff0000}%{B-}", b.flush());
+
+  b.foreground(rgba(0x34000000, rgba::type::ALPHA_ONLY));
+  b.foreground_close();
+
+  EXPECT_EQ("%{F#3400ff00}%{F-}", b.flush());
+}

version.txt

@@ -1,4 +1,4 @@
 # Polybar version information
 # Update this on every release
 # This is used to create the version string if a git repo is not available
-3.6.1
+3.6.2