refactor(script): Split non-/tail logic

Split the script module into one tailing output module and
one exec once module.

Refs #281

include/modules/cmdscript.hpp

@@ -0,0 +1,17 @@
+#pragma once
+
+#include "modules/script.hpp"
+
+POLYBAR_NS
+
+namespace modules {
+  class cmdscript_module : virtual public script_module {
+   public:
+    explicit cmdscript_module(const bar_settings&, string);
+
+   protected:
+    void process();
+  };
+}
+
+POLYBAR_NS_END

include/modules/meta/base.inl

@@ -78,7 +78,7 @@ namespace modules {
   template <typename Impl>
   string module<Impl>::contents() {
     if (m_changed) {
-      m_log.info("Rebuilding cache for '%s'...", name());
+      m_log.info("%s: Rebuilding cache", name());
       m_cache = CAST_MOD(Impl)->get_output();
       m_changed = false;
     }

include/modules/meta/factory.hpp

@@ -5,6 +5,7 @@
 #include "modules/backlight.hpp"
 #include "modules/battery.hpp"
 #include "modules/bspwm.hpp"
+#include "modules/cmdscript.hpp"
 #include "modules/counter.hpp"
 #include "modules/cpu.hpp"
 #include "modules/date.hpp"
@@ -14,6 +15,7 @@
 #include "modules/menu.hpp"
 #include "modules/meta/base.hpp"
 #include "modules/script.hpp"
+#include "modules/tailscript.hpp"
 #include "modules/temperature.hpp"
 #include "modules/text.hpp"
 #include "modules/xbacklight.hpp"
@@ -46,52 +48,55 @@ POLYBAR_NS
 using namespace modules;
 
 namespace {
-  template <typename... Args>
+  module_interface* make_module(string&& name, const bar_settings& bar, string module_name) {
-  module_interface* make_module(string&& name, Args&&... args) {
     if (name == "internal/counter") {
-      return new counter_module(forward<Args>(args)...);
+      return new counter_module(bar, move(module_name));
     } else if (name == "internal/backlight") {
-      return new backlight_module(forward<Args>(args)...);
+      return new backlight_module(bar, move(module_name));
     } else if (name == "internal/battery") {
-      return new battery_module(forward<Args>(args)...);
+      return new battery_module(bar, move(module_name));
     } else if (name == "internal/bspwm") {
-      return new bspwm_module(forward<Args>(args)...);
+      return new bspwm_module(bar, move(module_name));
     } else if (name == "internal/cpu") {
-      return new cpu_module(forward<Args>(args)...);
+      return new cpu_module(bar, move(module_name));
     } else if (name == "internal/date") {
-      return new date_module(forward<Args>(args)...);
+      return new date_module(bar, move(module_name));
     } else if (name == "internal/github") {
-      return new github_module(forward<Args>(args)...);
+      return new github_module(bar, move(module_name));
     } else if (name == "internal/fs") {
-      return new fs_module(forward<Args>(args)...);
+      return new fs_module(bar, move(module_name));
     } else if (name == "internal/memory") {
-      return new memory_module(forward<Args>(args)...);
+      return new memory_module(bar, move(module_name));
     } else if (name == "internal/i3") {
-      return new i3_module(forward<Args>(args)...);
+      return new i3_module(bar, move(module_name));
     } else if (name == "internal/mpd") {
-      return new mpd_module(forward<Args>(args)...);
+      return new mpd_module(bar, move(module_name));
     } else if (name == "internal/volume") {
-      return new volume_module(forward<Args>(args)...);
+      return new volume_module(bar, move(module_name));
     } else if (name == "internal/network") {
-      return new network_module(forward<Args>(args)...);
+      return new network_module(bar, move(module_name));
     } else if (name == "internal/temperature") {
-      return new temperature_module(forward<Args>(args)...);
+      return new temperature_module(bar, move(module_name));
     } else if (name == "internal/xbacklight") {
-      return new xbacklight_module(forward<Args>(args)...);
+      return new xbacklight_module(bar, move(module_name));
     } else if (name == "internal/xkeyboard") {
-      return new xkeyboard_module(forward<Args>(args)...);
+      return new xkeyboard_module(bar, move(module_name));
     } else if (name == "internal/xwindow") {
-      return new xwindow_module(forward<Args>(args)...);
+      return new xwindow_module(bar, move(module_name));
     } else if (name == "internal/xworkspaces") {
-      return new xworkspaces_module(forward<Args>(args)...);
+      return new xworkspaces_module(bar, move(module_name));
     } else if (name == "custom/text") {
-      return new text_module(forward<Args>(args)...);
+      return new text_module(bar, move(module_name));
     } else if (name == "custom/script") {
-      return new script_module(forward<Args>(args)...);
+      if (config::make().get<bool>("module/" + module_name, "tail", false)) {
+        return new tailscript_module(bar, move(module_name));
+      } else {
+        return new cmdscript_module(bar, move(module_name));
+      }
     } else if (name == "custom/menu") {
-      return new menu_module(forward<Args>(args)...);
+      return new menu_module(bar, move(module_name));
     } else if (name == "custom/ipc") {
-      return new ipc_module(forward<Args>(args)...);
+      return new ipc_module(bar, move(module_name));
     } else {
       throw application_error("Unknown module: " + name);
     }

include/modules/script.hpp

@@ -1,41 +1,32 @@
 #pragma once
 
-#include <chrono>
+#include "modules/meta/base.hpp"
-
-#include "modules/meta/event_module.hpp"
 #include "utils/command.hpp"
+#include "utils/io.hpp"
 
 POLYBAR_NS
 
-namespace chrono = std::chrono;
-
-#define OUTPUT_ACTION(BUTTON)     \
-  if (!m_actions[BUTTON].empty()) \
-  m_builder->cmd(BUTTON, string_util::replace_all(m_actions[BUTTON], "%counter%", counter_str))
-
 namespace modules {
-  /**
+  class script_module : public module<script_module> {
-   * TODO: Split into timed-/streaming modules
-   */
-  class script_module : public event_module<script_module> {
    public:
     explicit script_module(const bar_settings&, string);
+    virtual ~script_module() {}
+
+    virtual void start();
+    virtual void stop();
 
-    void stop();
-    void idle();
-    bool has_event();
-    bool update();
     string get_output();
     bool build(builder* builder, const string& tag) const;
 
    protected:
+    virtual void process() = 0;
+
     static constexpr const char* TAG_OUTPUT{"<output>"};
     static constexpr const char* TAG_LABEL{"<label>"};
 
     unique_ptr<command> m_command;
 
     string m_exec;
-    bool m_tail{false};
     chrono::duration<double> m_interval{0};
     map<mousebtn, string> m_actions;
 
@@ -48,6 +39,8 @@ namespace modules {
     size_t m_maxlen{0};
     // @deprecated
     bool m_ellipsis{true};
+
+    bool m_stopping{false};
   };
 }
 

include/modules/tailscript.hpp

@@ -0,0 +1,17 @@
+#pragma once
+
+#include "modules/script.hpp"
+
+POLYBAR_NS
+
+namespace modules {
+  class tailscript_module : virtual public script_module {
+   public:
+    explicit tailscript_module(const bar_settings&, string);
+
+   protected:
+    void process();
+  };
+}
+
+POLYBAR_NS_END

include/utils/file.hpp

@@ -28,7 +28,7 @@ class file_ptr {
 class file_descriptor {
  public:
   explicit file_descriptor(const string& path, int flags = 0);
-  explicit file_descriptor(int fd);
+  explicit file_descriptor(int fd, bool autoclose = true);
   ~file_descriptor();
 
   file_descriptor& operator=(const int);
@@ -44,13 +44,18 @@ class file_descriptor {
 
  private:
   int m_fd{-1};
+  bool m_autoclose{true};
 };
 
 class fd_streambuf : public std::streambuf {
  public:
   using traits_type = std::streambuf::traits_type;
 
-  explicit fd_streambuf(int fd);
+  template <typename... Args>
+  explicit fd_streambuf(Args&&... args) : m_fd(forward<Args>(args)...) {
+    setg(m_in, m_in, m_in);
+    setp(m_out, m_out + sizeof(m_in));
+  }
   ~fd_streambuf();
 
   explicit operator int();
@@ -75,7 +80,8 @@ class fd_stream : public StreamType {
  public:
   using type = fd_stream<StreamType>;
 
-  explicit fd_stream(int fd) : m_buf(fd) {
+  template <typename... Args>
+  explicit fd_stream(Args&&... args) : m_buf(forward<Args>(args)...) {
     StreamType::rdbuf(&m_buf);
   }
 
@@ -95,7 +101,7 @@ namespace file_util {
   bool exists(const string& filename);
   string pick(const vector<string>& filenames);
   string contents(const string& filename);
-  bool is_fifo(string filename);
+  bool is_fifo(const string& filename);
 
   template <typename... Args>
   decltype(auto) make_file_descriptor(Args&&... args) {

include/utils/io.hpp

@@ -5,12 +5,10 @@
 POLYBAR_NS
 
 namespace io_util {
-  string read(int read_fd, int bytes_to_read, int& bytes_read_loc, int& status_loc);
+  string read(int read_fd, size_t bytes_to_read);
-  string read(int read_fd, int bytes_to_read = -1);
-  string readline(int read_fd, int& bytes_read);
   string readline(int read_fd);
 
-  size_t write(int write_fd, const string& data);
+  size_t write(int write_fd, size_t bytes_to_write, const string& data);
   size_t writeline(int write_fd, const string& data);
 
   void tail(int read_fd, const function<void(string)>& callback);

src/main.cpp

@@ -13,6 +13,7 @@
 #include "utils/env.hpp"
 #include "utils/file.hpp"
 #include "utils/inotify.hpp"
+#include "utils/io.hpp"
 #include "utils/process.hpp"
 #include "x11/connection.hpp"
 #include "x11/tray_manager.hpp"
@@ -20,6 +21,7 @@
 using namespace polybar;
 
 int main(int argc, char** argv) {
+  // std::cout << x << std::Endl;
   // clang-format off
   const command_line::options opts{
       command_line::option{"-h", "--help", "Show help options"},

src/modules/cmdscript.cpp

@@ -0,0 +1,38 @@
+#include "modules/cmdscript.hpp"
+#include "drawtypes/label.hpp"
+
+#include "modules/meta/base.inl"
+
+POLYBAR_NS
+
+namespace modules {
+  cmdscript_module::cmdscript_module(const bar_settings& bar, string name_) : script_module(bar, move(name_)) {
+    m_interval = m_conf.get<decltype(m_interval)>(name(), "interval", 5s);
+  }
+
+  void cmdscript_module::process() {
+    if (!m_updatelock.try_lock()) {
+      return;
+    }
+
+    try {
+      std::unique_lock<mutex> guard(m_updatelock, std::adopt_lock);
+      auto exec = string_util::replace_all(m_exec, "%counter%", to_string(++m_counter));
+      m_log.info("%s: Invoking shell command: \"%s\"", name(), exec);
+      m_command = command_util::make_command(exec);
+      m_command->exec(true);
+    } catch (const exception& err) {
+      m_log.err("%s: %s", name(), err.what());
+      throw module_error("Failed to execute command, stopping module...");
+    }
+
+    if ((m_output = m_command->readline()) != m_prev) {
+      broadcast();
+      m_prev = m_output;
+    }
+
+    sleep(std::max(m_command->get_exit_status() == 0 ? m_interval : 1s, m_interval));
+  }
+}
+
+POLYBAR_NS_END

src/modules/script.cpp

@@ -8,12 +8,11 @@ POLYBAR_NS
 namespace modules {
   template class module<script_module>;
 
-  script_module::script_module(const bar_settings& bar, string name_) : event_module<script_module>(bar, move(name_)) {
+  script_module::script_module(const bar_settings& bar, string name_) : module<script_module>(bar, move(name_)) {
     m_exec = m_conf.get(name(), "exec", m_exec);
-    m_tail = m_conf.get(name(), "tail", m_tail);
     m_maxlen = m_conf.get(name(), "maxlen", m_maxlen);
     m_ellipsis = m_conf.get(name(), "ellipsis", m_ellipsis);
-    m_interval = m_conf.get<decltype(m_interval)>(name(), "interval", m_tail ? 0s : 5s);
+    m_interval = m_conf.get<decltype(m_interval)>(name(), "interval", 5s);
 
     m_conf.warn_deprecated(
         name(), "maxlen", "\"format = <label>\" and \"label = %output:0:" + to_string(m_maxlen) + "%\"");
@@ -33,70 +32,32 @@ namespace modules {
     }
   }
 
-  void script_module::stop() {
+  void script_module::start() {
-    m_updatelock.unlock();
+    m_mainthread = thread([this] {
-    event_module::stop();
+      try {
+        while (running() && !m_stopping) {
+          this->process();
+        }
+      } catch (const exception& err) {
+        halt(err.what());
+      }
+    });
+  }
 
-    if (m_command && m_command->is_running()) {
+  void script_module::stop() {
+    std::lock_guard<mutex> guard(m_updatelock, std::adopt_lock);
+
+    m_stopping = true;
+    this->wakeup();
+
+    if (m_command) {
+      if (m_command->is_running()) {
         m_log.warn("%s: Stopping shell command", name());
+      }
       m_command->terminate();
     }
-  }
 
-  void script_module::idle() {
+    this->module::stop();
-    if (!m_tail) {
-      sleep(m_interval);
-    } else if (!m_command || !m_command->is_running()) {
-      sleep(m_interval);
-    }
-  }
-
-  bool script_module::has_event() {
-    if (!m_tail) {
-      return true;
-    } else if (!m_command || !m_command->is_running()) {
-      try {
-        string exec{string_util::replace_all(m_exec, "%counter%", to_string(++m_counter))};
-        m_log.info("%s: Invoking shell command: \"%s\"", name(), exec);
-        m_command = command_util::make_command(move(exec));
-        m_command->exec(false);
-      } catch (const std::exception& err) {
-        m_log.err("%s: %s", name(), err.what());
-        throw module_error("Failed to execute tail command, stopping module...");
-      }
-    }
-
-    if (!m_command || (m_output = m_command->readline()) == m_prev) {
-      return false;
-    }
-
-    m_prev = m_output;
-
-    return true;
-  }
-
-  bool script_module::update() {
-    if (m_tail) {
-      return true;
-    }
-
-    try {
-      auto exec = string_util::replace_all(m_exec, "%counter%", to_string(++m_counter));
-      m_log.info("%s: Executing \"%s\"", name(), exec);
-      m_command = command_util::make_command(exec);
-      m_command->exec();
-      m_command->tail([&](string output) { m_output = output; });
-    } catch (const std::exception& err) {
-      m_log.err("%s: %s", name(), err.what());
-      throw module_error("Failed to execute command, stopping module...");
-    }
-
-    if (m_output == m_prev) {
-      return false;
-    }
-
-    m_prev = m_output;
-    return true;
   }
 
   string script_module::get_output() {
@@ -117,11 +78,23 @@ namespace modules {
     auto counter_str = to_string(m_counter);
     string output{module::get_output()};
 
-    OUTPUT_ACTION(mousebtn::LEFT);
+    if (!m_actions[mousebtn::LEFT].empty()) {
-    OUTPUT_ACTION(mousebtn::MIDDLE);
+      m_builder->cmd(mousebtn::LEFT, string_util::replace_all(m_actions[mousebtn::LEFT], "%counter%", counter_str));
-    OUTPUT_ACTION(mousebtn::RIGHT);
+    }
-    OUTPUT_ACTION(mousebtn::SCROLL_UP);
+    if (!m_actions[mousebtn::MIDDLE].empty()) {
-    OUTPUT_ACTION(mousebtn::SCROLL_DOWN);
+      m_builder->cmd(mousebtn::MIDDLE, string_util::replace_all(m_actions[mousebtn::MIDDLE], "%counter%", counter_str));
+    }
+    if (!m_actions[mousebtn::RIGHT].empty()) {
+      m_builder->cmd(mousebtn::RIGHT, string_util::replace_all(m_actions[mousebtn::RIGHT], "%counter%", counter_str));
+    }
+    if (!m_actions[mousebtn::SCROLL_UP].empty()) {
+      m_builder->cmd(
+          mousebtn::SCROLL_UP, string_util::replace_all(m_actions[mousebtn::SCROLL_UP], "%counter%", counter_str));
+    }
+    if (!m_actions[mousebtn::SCROLL_DOWN].empty()) {
+      m_builder->cmd(
+          mousebtn::SCROLL_DOWN, string_util::replace_all(m_actions[mousebtn::SCROLL_DOWN], "%counter%", counter_str));
+    }
 
     m_builder->append(output);
 

src/modules/tailscript.cpp

@@ -0,0 +1,50 @@
+#include "modules/tailscript.hpp"
+#include "drawtypes/label.hpp"
+
+#include "modules/meta/base.inl"
+
+POLYBAR_NS
+
+namespace modules {
+  tailscript_module::tailscript_module(const bar_settings& bar, string name_) : script_module(bar, move(name_)) {
+    m_interval = m_conf.get<decltype(m_interval)>(name(), "interval", 0s);
+  }
+
+  void tailscript_module::process() {
+    if (!m_updatelock.try_lock()) {
+      return;
+    }
+
+    std::unique_lock<mutex> guard(m_updatelock, std::adopt_lock);
+
+    if (!m_command || !m_command->is_running()) {
+      string exec{string_util::replace_all(m_exec, "%counter%", to_string(++m_counter))};
+      m_log.info("%s: Invoking shell command: \"%s\"", name(), exec);
+      m_command = command_util::make_command(exec);
+
+      try {
+        m_command->exec(false);
+      } catch (const exception& err) {
+        m_log.err("%s: %s", name(), err.what());
+        throw module_error("Failed to execute command, stopping module...");
+      }
+    }
+
+    if (io_util::poll(m_command->get_stdout(PIPE_READ), POLLIN, 0)) {
+      if ((m_output = m_command->readline()) != m_prev) {
+        m_prev = m_output;
+        broadcast();
+      }
+    }
+
+    guard.unlock();
+
+    if (m_command && !m_command->is_running()) {
+      sleep(std::max(m_command->get_exit_status() == 0 ? m_interval : 1s, m_interval));
+    } else {
+      sleep(m_interval);
+    }
+  }
+}
+
+POLYBAR_NS_END

src/utils/file.cpp

@@ -51,18 +51,22 @@ file_descriptor::file_descriptor(const string& path, int flags) {
   }
 }
 
-file_descriptor::file_descriptor(int fd) : m_fd(fd) {
+file_descriptor::file_descriptor(int fd, bool autoclose) : m_fd(fd), m_autoclose(autoclose) {
   if (m_fd != -1 && !*this) {
     throw system_error("Given file descriptor (" + to_string(m_fd) + ") is not valid");
   }
 }
 
 file_descriptor::~file_descriptor() {
+  if (m_autoclose) {
     close();
+  }
 }
 
 file_descriptor& file_descriptor::operator=(const int fd) {
+  if (m_autoclose) {
     close();
+  }
   m_fd = fd;
   return *this;
 }
@@ -96,10 +100,6 @@ void file_descriptor::close() {
 // }}}
 // implementation of file_streambuf {{{
 
-fd_streambuf::fd_streambuf(int fd) : m_fd(-1) {
-  open(fd);
-}
-
 fd_streambuf::~fd_streambuf() {
   close();
 }
@@ -186,9 +186,10 @@ namespace file_util {
    */
   string contents(const string& filename) {
     try {
-      std::ifstream ifs(filename);
+      string contents;
-      return string((std::istreambuf_iterator<char>(ifs)), (std::istreambuf_iterator<char>()));
+      std::getline(std::ifstream(filename, std::ifstream::in), contents);
-    } catch (std::ios_base::failure& e) {
+      return contents;
+    } catch (const std::ifstream::failure& e) {
       return "";
     }
   }
@@ -196,12 +197,9 @@ namespace file_util {
   /**
    * Checks if the given file is a named pipe
    */
-  bool is_fifo(string filename) {
+  bool is_fifo(const string& filename) {
-    auto fileptr = factory_util::unique<file_ptr>(filename);
+    struct stat buffer {};
-    int fd = fileno(*fileptr);
+    return stat(filename.c_str(), &buffer) == 0 && S_ISFIFO(buffer.st_mode);
-    struct stat statbuf {};
-    fstat(fd, &statbuf);
-    return S_ISFIFO(statbuf.st_mode);
   }
 }
 

src/utils/io.cpp

@@ -3,91 +3,52 @@
 #include <unistd.h>
 #include <cstdio>
 #include <cstdlib>
+#include <iomanip>
 
 #include "errors.hpp"
+#include "utils/file.hpp"
 #include "utils/io.hpp"
 #include "utils/string.hpp"
 
 POLYBAR_NS
 
 namespace io_util {
-  string read(int read_fd, int bytes_to_read, int& bytes_read_loc, int& status_loc) {
+  string read(int read_fd, size_t bytes_to_read) {
-    char buffer[BUFSIZ - 1];
+    fd_stream<std::istream> in(read_fd, false);
-
+    char buffer[BUFSIZ];
-    if (bytes_to_read == -1) {
+    in.getline(buffer, bytes_to_read);
-      bytes_to_read = sizeof(buffer);
+    string out{buffer};
-    }
+    return out;
-
-    status_loc = 0;
-
-    if ((bytes_read_loc = ::read(read_fd, &buffer, bytes_to_read)) == -1) {
-      throw system_error("Error trying to read from fd");
-    } else if (bytes_read_loc == 0) {
-      status_loc = -1;
-    } else {
-      buffer[bytes_read_loc] = '\0';
-    }
-
-    return {buffer};
-  }
-
-  string read(int read_fd, int bytes_to_read) {
-    int bytes_read = 0;
-    int status = 0;
-    return read(read_fd, bytes_to_read, bytes_read, status);
-  }
-
-  string readline(int read_fd, int& bytes_read) {
-    stringstream buffer;
-    char char_;
-    int bytes = 0;
-    bytes_read = 0;
-
-    while ((bytes = ::read(read_fd, &char_, 1)) > 0) {
-      if (bytes <= 0) {
-        break;
-      }
-      if (char_ == '\n' || char_ == '\x00') {
-        break;
-      }
-      bytes_read += bytes;
-      buffer << char_;
-    }
-
-    if (bytes_read <= 0) {
-      return "";
-    }
-    return string_util::strip_trailing_newline(buffer.str());
   }
 
   string readline(int read_fd) {
-    int bytes_read;
+    fd_stream<std::istream> in(read_fd, false);
-    return readline(read_fd, bytes_read);
+    string out;
+    std::getline(in, out);
+    return out;
   }
 
-  size_t write(int write_fd, const string& data) {
+  size_t write(int write_fd, size_t bytes_to_write, const string& data) {
-    return ::write(write_fd, data.c_str(), strlen(data.c_str()));
+    fd_stream<std::ostream> out(write_fd, false);
+    out.write(data.c_str(), bytes_to_write).flush();
+    return out.good() ? data.size() : 0;
   }
 
   size_t writeline(int write_fd, const string& data) {
-    if (data.length() == 0) {
+    fd_stream<std::ostream> out(write_fd, false);
-      return -1;
+    if (data[data.size() - 1] != '\n') {
-    }
+      out << data << std::endl;
-    if (data.substr(data.length() - 1, 1) != "\n") {
-      return io_util::write(write_fd, data + "\n");
     } else {
-      return io_util::write(write_fd, data);
+      out << data << std::flush;
     }
+    return out.good() ? data.size() : 0;
   }
 
   void tail(int read_fd, const function<void(string)>& callback) {
-    int bytes_read;
+    string line;
-    while (true) {
+    fd_stream<std::istream> in(read_fd, false);
-      auto line = io_util::readline(read_fd, bytes_read);
+    while (std::getline(in, line)) {
-      if (bytes_read <= 0) {
+      callback(move(line));
-        break;
-      }
-      callback(line);
     }
   }
 
@@ -99,9 +60,7 @@ namespace io_util {
     struct pollfd fds[1];
     fds[0].fd = fd;
     fds[0].events = events;
-
     ::poll(fds, 1, timeout_ms);
-
     return fds[0].revents & events;
   }
 
@@ -114,9 +73,7 @@ namespace io_util {
   }
 
   bool interrupt_read(int write_fd) {
-    char end[1] = {'\n'};
+    return writeline(write_fd, "") > 0;
-    size_t bytes = ::write(write_fd, end, 1);
-    return bytes > 0;
   }
 
   void set_block(int fd) {