refactor(config): Cleanup and minor tweaks

include/components/bar.hpp

@@ -1,7 +1,6 @@
 #pragma once
 
 #include "common.hpp"
-#include "components/config.hpp"
 #include "components/screen.hpp"
 #include "components/types.hpp"
 #include "errors.hpp"

include/components/config.hpp

@@ -31,16 +31,14 @@ class config {
 
   explicit config(const logger& logger, const xresource_manager& xrm, string&& path = "", string&& bar = "");
 
-  void load(string file, string barname);
   string filepath() const;
-  string bar_section() const;
+  string section() const;
-  vector<string> defined_bars() const;
+
   void warn_deprecated(const string& section, const string& key, string replacement) const;
 
   /**
    * Returns true if a given parameter exists
    */
-  template <typename T>
   bool has(const string& section, const string& key) const {
     auto it = m_sections.find(section);
     return it != m_sections.end() && it->second.find(key) != it->second.end();
@@ -51,7 +49,7 @@ class config {
    */
   template <typename T>
   T get(const string& key) const {
-    return get<T>(bar_section(), key);
+    return get<T>(section(), key);
   }
 
   /**
@@ -100,7 +98,7 @@ class config {
    */
   template <typename T>
   vector<T> get_list(const string& key) const {
-    return get_list<T>(bar_section(), key);
+    return get_list<T>(section(), key);
   }
 
   /**
@@ -152,8 +150,38 @@ class config {
       return vec;
   }
 
+  /**
+   * Attempt to load value using the deprecated key name. If successful show a
+   * warning message. If it fails load the value using the new key and given
+   * fallback value
+   */
+  template <typename T>
+  T deprecated(const string& section, const string& old, const string& newkey, const T& fallback) const {
+    try {
+      T value{get<T>(section, old)};
+      warn_deprecated(section, old, newkey);
+      return value;
+    } catch (const key_error& err) {
+      return get<T>(section, newkey, fallback);
+    }
+  }
+
+  /**
+   * @see deprecated<T>
+   */
+  template <typename T>
+  T deprecated_list(const string& section, const string& old, const string& newkey, const vector<T>& fallback) const {
+    try {
+      vector<T> value{get_list<T>(section, old)};
+      warn_deprecated(section, old, newkey);
+      return value;
+    } catch (const key_error& err) {
+      return get_list<T>(section, newkey, fallback);
+    }
+  }
+
  protected:
-  void read();
+  void parse_file();
   void copy_inherited();
 
   template <typename T>
@@ -206,8 +234,8 @@ class config {
       m_logger.warn("${BAR.key} is deprecated. Use ${root.key} instead");
     }
 
-    section = string_util::replace(section, "BAR", bar_section(), 0, 3);
+    section = string_util::replace(section, "BAR", this->section(), 0, 3);
-    section = string_util::replace(section, "root", bar_section(), 0, 4);
+    section = string_util::replace(section, "root", this->section(), 0, 4);
     section = string_util::replace(section, "self", current_section, 0, 4);
 
     optional<T> result{opt<T>(section, key)};
@@ -263,7 +291,7 @@ class config {
   const logger& m_logger;
   const xresource_manager& m_xrm;
   string m_file;
-  string m_current_bar;
+  string m_barname;
   sectionmap_t m_sections;
 };
 

include/modules/unsupported.hpp

@@ -24,7 +24,7 @@ namespace modules {
 #define DEFINE_UNSUPPORTED_MODULE(MODULE_NAME, MODULE_TYPE)                             \
   class MODULE_NAME : public module_interface {                                         \
    public:                                                                              \
-    MODULE_NAME(const bar_settings, const logger&, const config&, string) {             \
+    MODULE_NAME(const bar_settings, string) {             \
       throw application_error("No built-in support for '" + string{MODULE_TYPE} + "'"); \
     }                                                                                   \
     string name() const {                                                               \

src/components/bar.cpp

@@ -3,9 +3,7 @@
 #include <string>
 
 #include "components/bar.hpp"
-#include "components/command_line.hpp"
 #include "components/config.hpp"
-#include "components/eventloop.hpp"
 #include "components/parser.hpp"
 #include "components/renderer.hpp"
 #include "components/screen.hpp"
@@ -60,7 +58,7 @@ bar::bar(connection& conn, signal_emitter& emitter, const config& config, const
     , m_log(logger)
     , m_screen(move(screen))
     , m_tray(move(tray_manager)) {
-  string bs{m_conf.bar_section()};
+  string bs{m_conf.section()};
 
   // Get available RandR outputs
   auto monitor_name = m_conf.get<string>(bs, "monitor", "");
@@ -163,10 +161,10 @@ bar::bar(connection& conn, signal_emitter& emitter, const config& config, const
   m_opts.borders[edge::RIGHT].color = color::parse(m_conf.get<string>(bs, "border-right-color", bcolor));
 
   // Load geometry values
-  auto w = m_conf.get<string>(m_conf.bar_section(), "width", "100%");
+  auto w = m_conf.get<string>(m_conf.section(), "width", "100%");
-  auto h = m_conf.get<string>(m_conf.bar_section(), "height", "24");
+  auto h = m_conf.get<string>(m_conf.section(), "height", "24");
-  auto offsetx = m_conf.get<string>(m_conf.bar_section(), "offset-x", "");
+  auto offsetx = m_conf.get<string>(m_conf.section(), "offset-x", "");
-  auto offsety = m_conf.get<string>(m_conf.bar_section(), "offset-y", "");
+  auto offsety = m_conf.get<string>(m_conf.section(), "offset-y", "");
 
   if ((m_opts.size.w = atoi(w.c_str())) && w.find('%') != string::npos) {
     m_opts.size.w = math_util::percentage_to_value<int>(m_opts.size.w, m_opts.monitor->w);
@@ -211,7 +209,7 @@ bar::bar(connection& conn, signal_emitter& emitter, const config& config, const
   m_opts.center.x += m_opts.borders[edge::LEFT].size;
 
   m_log.trace("bar: Create renderer");
-  auto fonts = m_conf.get_list<string>(m_conf.bar_section(), "font", {});
+  auto fonts = m_conf.get_list<string>(m_conf.section(), "font", {});
   m_renderer = renderer::make(m_opts, move(fonts));
 
   m_log.trace("bar: Attaching sink to registry");
@@ -312,7 +310,7 @@ void bar::restack_window() {
   string wm_restack;
 
   try {
-    wm_restack = m_conf.get<string>(m_conf.bar_section(), "wm-restack");
+    wm_restack = m_conf.get<string>(m_conf.section(), "wm-restack");
   } catch (const key_error& err) {
     return;
   }

src/components/config.cpp

@@ -1,4 +1,5 @@
 #include <algorithm>
+#include <chrono>
 #include <fstream>
 #include <istream>
 #include <utility>
@@ -7,59 +8,45 @@
 #include "utils/env.hpp"
 #include "utils/factory.hpp"
 #include "utils/file.hpp"
+#include "utils/string.hpp"
 
 POLYBAR_NS
 
+namespace chrono = std::chrono;
+
 /**
  * Create instance
  */
 config::make_type config::make(string path, string bar) {
   return static_cast<config::make_type>(*factory_util::singleton<std::remove_reference_t<config::make_type>>(
-      *factory_util::singleton<const config>(logger::make(), xresource_manager::make(), move(path), move(bar))));
+      logger::make(), xresource_manager::make(), move(path), move(bar)));
 }
 
 /**
  * Construct config object
  */
 config::config(const logger& logger, const xresource_manager& xrm, string&& path, string&& bar)
-    : m_logger(logger), m_xrm(xrm) {
+    : m_logger(logger), m_xrm(xrm), m_file(forward<decltype(path)>(path)), m_barname(forward<decltype(bar)>(bar)) {
-  if (!path.empty() && !bar.empty()) {
+  if (!file_util::exists(m_file)) {
-    load(forward<decltype(path)>(path), forward<decltype(bar)>(bar));
+    throw application_error("Could not find config file: " + m_file);
-  }
-}
-
-/**
- * Load configuration and validate bar section
- *
- * This is done outside the constructor due to boost::di noexcept
- */
-void config::load(string file, string barname) {
-  m_file = file;
-  m_current_bar = move(barname);
-
-  if (!file_util::exists(file)) {
-    throw application_error("Could not find config file: " + file);
   }
 
-  // Read values
+  parse_file();
-  read();
 
-  auto bars = defined_bars();
+  bool found_bar{false};
-  if (std::find(bars.begin(), bars.end(), m_current_bar) == bars.end()) {
+  for (auto&& p : m_sections) {
-    throw application_error("Undefined bar: " + m_current_bar);
+    if (p.first == section()) {
+      found_bar = true;
+      break;
+    }
   }
 
-  if (env_util::has("XDG_CONFIG_HOME")) {
+  if (!found_bar) {
-    file = string_util::replace(file, env_util::get("XDG_CONFIG_HOME"), "$XDG_CONFIG_HOME");
+    throw application_error("Undefined bar: " + m_barname);
-  }
-  if (env_util::has("HOME")) {
-    file = string_util::replace(file, env_util::get("HOME"), "~");
   }
 
-  m_logger.trace("config: Loaded %s", file);
+  m_logger.trace("config: Loaded %s", m_file);
-  m_logger.trace("config: Current bar section: [%s]", bar_section());
+  m_logger.trace("config: Current bar section: [%s]", section());
-
-  copy_inherited();
 }
 
 /**
@@ -72,23 +59,8 @@ string config::filepath() const {
 /**
  * Get the section name of the bar in use
  */
-string config::bar_section() const {
+string config::section() const {
-  return "bar/" + m_current_bar;
+  return "bar/" + m_barname;
-}
-
-/**
- * Get a list of defined bar sections in the current config
- */
-vector<string> config::defined_bars() const {
-  vector<string> bars;
-
-  for (auto&& p : m_sections) {
-    if (p.first.compare(0, 4, "bar/") == 0) {
-      bars.emplace_back(p.first.substr(4));
-    }
-  }
-
-  return bars;
 }
 
 /**
@@ -102,13 +74,20 @@ void config::warn_deprecated(const string& section, const string& key, string re
   }
 }
 
-void config::read() {
+/**
+ * Parse key/value pairs from the configuration file
+ */
+void config::parse_file() {
   std::ifstream in(m_file.c_str());
   string line;
   string section;
+  uint32_t lineno{0};
 
   while (std::getline(in, line)) {
-    if (line.empty()) {
+    lineno++;
+
+    // Ignore empty lines and comments
+    if (line.empty() || line[0] == ';' || line[0] == '#') {
       continue;
     }
 
@@ -128,8 +107,15 @@ void config::read() {
     string key{string_util::trim(line.substr(0, equal_pos), ' ')};
     string value{string_util::trim(string_util::trim(line.substr(equal_pos + 1), ' '), '"')};
 
-    m_sections[section][key] = move(value);
+    auto it = m_sections[section].find(key);
+    if (it != m_sections[section].end()) {
+      throw key_error("Duplicate key name \"" + key + "\" defined on line " + to_string(lineno));
+    }
+
+    m_sections[section].emplace_hint(it, move(key), move(value));
   }
+
+  copy_inherited();
 }
 
 /**
@@ -198,27 +184,39 @@ short config::convert(string&& value) const {
 
 template <>
 bool config::convert(string&& value) const {
-  return std::atoi(value.c_str()) != 0;
+  string lower{string_util::lower(forward<string>(value))};
+
+  if (lower.compare(0, 4, "true") == 0) {
+    return true;
+  } else if (lower.compare(0, 3, "yes") == 0) {
+    return true;
+  } else if (lower.compare(0, 2, "on") == 0) {
+    return true;
+  } else if (lower.compare(0, 1, "1") == 0) {
+    return true;
+  } else {
+    return false;
+  }
 }
 
 template <>
 float config::convert(string&& value) const {
-  return std::atof(value.c_str());
+  return std::strtof(value.c_str(), nullptr);
 }
 
 template <>
 double config::convert(string&& value) const {
-  return std::atof(value.c_str());
+  return std::strtod(value.c_str(), nullptr);
 }
 
 template <>
 long config::convert(string&& value) const {
-  return std::atol(value.c_str());
+  return std::strtol(value.c_str(), nullptr, 10);
 }
 
 template <>
 long long config::convert(string&& value) const {
-  return std::atoll(value.c_str());
+  return std::strtoll(value.c_str(), nullptr, 10);
 }
 
 template <>
@@ -267,4 +265,14 @@ unsigned long long config::convert(string&& value) const {
   return std::strtoull(value.c_str(), nullptr, 10);
 }
 
+template <>
+chrono::seconds config::convert(string&& value) const {
+  return chrono::seconds{convert<chrono::seconds::rep>(forward<string>(value))};
+}
+
+template <>
+chrono::milliseconds config::convert(string&& value) const {
+  return chrono::milliseconds{convert<chrono::milliseconds::rep>(forward<string>(value))};
+}
+
 POLYBAR_NS_END

src/components/controller.cpp

@@ -96,6 +96,7 @@ controller::~controller() {
 void controller::setup() {
   string bs{m_conf.bar_section()};
 
+  string bs{m_conf.section()};
   m_log.trace("controller: Setup user-defined modules");
 
   for (int i = 0; i < 3; i++) {

src/main.cpp

@@ -109,25 +109,24 @@ int main(int argc, char** argv) {
     // Dump requested data
     //==================================================
     if (cli->has("dump")) {
-      std::cout << conf.get<string>(conf.bar_section(), cli->get("dump")) << std::endl;
+      std::cout << conf.get<string>(conf.section(), cli->get("dump")) << std::endl;
       throw exit_success{};
     }
 
     //==================================================
     // Create controller and run application
     //==================================================
-    unique_ptr<controller> ctrl;
     string path_confwatch;
     bool enable_ipc{false};
 
     if (!cli->has("print-wmname")) {
-      enable_ipc = conf.get<bool>(conf.bar_section(), "enable-ipc", false);
+      enable_ipc = conf.get<bool>(conf.section(), "enable-ipc", false);
     }
     if (!cli->has("print-wmname") && cli->has("reload")) {
       path_confwatch = conf.filepath();
     }
 
-    ctrl = controller::make(move(path_confwatch), enable_ipc, cli->has("stdout"));
+    unique_ptr<controller> ctrl{controller::make(move(path_confwatch), move(enable_ipc), cli->has("stdout"))};
 
     if (cli->has("print-wmname")) {
       std::cout << ctrl->opts().wmname << std::endl;

src/x11/tray_manager.cpp

@@ -62,7 +62,7 @@ tray_manager::~tray_manager() {
 
 void tray_manager::setup(const bar_settings& bar_opts) {
   auto conf = config::make();
-  auto bs = conf.bar_section();
+  auto bs = conf.section();
   auto tray_position = conf.get<string>(bs, "tray-position", "");
 
   if (tray_position == "left") {