From 110c66dd329427fb29169fe0389f3ee4cbbd8307 Mon Sep 17 00:00:00 2001 From: Sergey Mishin Date: Sun, 3 Oct 2021 16:48:21 +0000 Subject: [PATCH 1/3] Refactor Clock: generalize multi timezones and single timezone cases After this refactoring: 1. Timezones parses only once on start and the we refer to saved values. All time_zone.isString() checks gone to the constructor. 2. Single timezone case handling as case of multi timezoned logic. 3. Scroll event seems more clear now. 4. Tooltip template parses on start to check if there calendar placeholder or not. To do not calculate calendar_text() if not necessary. --- include/modules/clock.hpp | 11 ++-- src/modules/clock.cpp | 108 ++++++++++++++++++++++++-------------- 2 files changed, 75 insertions(+), 44 deletions(-) diff --git a/include/modules/clock.hpp b/include/modules/clock.hpp index 17752e4d..9f950192 100644 --- a/include/modules/clock.hpp +++ b/include/modules/clock.hpp @@ -17,6 +17,8 @@ struct waybar_time { date::zoned_seconds ztime; }; +const std::string kCalendarPlaceholder = "calendar"; + class Clock : public ALabel { public: Clock(const std::string&, const Json::Value&); @@ -26,18 +28,19 @@ class Clock : public ALabel { private: util::SleeperThread thread_; std::locale locale_; - const date::time_zone* time_zone_; - bool fixed_time_zone_; - int time_zone_idx_; + std::vector time_zones_; + int current_time_zone_idx_; date::year_month_day cached_calendar_ymd_ = date::January/1/0; std::string cached_calendar_text_; + bool is_calendar_in_tooltip_; bool handleScroll(GdkEventScroll* e); auto calendar_text(const waybar_time& wtime) -> std::string; auto weekdays_header(const date::weekday& first_dow, std::ostream& os) -> void; auto first_day_of_week() -> date::weekday; - bool setTimeZone(Json::Value zone_name); + const date::time_zone* current_timezone(); + bool is_timezone_fixed(); }; } // namespace waybar::modules diff --git a/src/modules/clock.cpp b/src/modules/clock.cpp index 7c94c457..d925c2b7 100644 --- a/src/modules/clock.cpp +++ b/src/modules/clock.cpp @@ -14,17 +14,51 @@ using waybar::modules::waybar_time; waybar::modules::Clock::Clock(const std::string& id, const Json::Value& config) - : ALabel(config, "clock", id, "{:%H:%M}", 60, false, false, true), fixed_time_zone_(false) { + : ALabel(config, "clock", id, "{:%H:%M}", 60, false, false, true), + current_time_zone_idx_(0), + is_calendar_in_tooltip_(false) +{ if (config_["timezones"].isArray() && !config_["timezones"].empty()) { - time_zone_idx_ = 0; - setTimeZone(config_["timezones"][time_zone_idx_]); + for (const auto& zone_name: config_["timezones"]) { + if (!zone_name.isString() || zone_name.asString().empty()) { + time_zones_.push_back(nullptr); + continue; + } + time_zones_.push_back( + date::locate_zone( + zone_name.asString() + ) + ); + + } + // If we parse all timezones and no one is good, add nullptr to the tmezones vector, to mark that we need to show localtime + if (!time_zones_.size()) { + time_zones_.push_back(nullptr); + } } else { - setTimeZone(config_["timezone"]); + time_zones_.push_back( + date::locate_zone( + config_["timezone"].asString() + ) + ); } - if (fixed_time_zone_) { + + if (!is_timezone_fixed()) { spdlog::warn("As using a timezone, some format args may be missing as the date library haven't got a release since 2018."); } + // Check if we have to particular placeholder in tooltip format, to know what to calculate on update + if (config_["tooltip-format"].isString()) { + std::string trimmedFormat = config_["tooltip-format"].asString(); + trimmedFormat.erase(std::remove_if(trimmedFormat.begin(), + trimmedFormat.end(), + [](unsigned char x){return std::isspace(x);}), + trimmedFormat.end()); + if (trimmedFormat.find("{" + kCalendarPlaceholder + "}") != std::string::npos) { + is_calendar_in_tooltip_ = true; + } + } + if (config_["locale"].isString()) { locale_ = std::locale(config_["locale"].asString()); } else { @@ -40,53 +74,46 @@ waybar::modules::Clock::Clock(const std::string& id, const Json::Value& config) }; } +const date::time_zone* waybar::modules::Clock::current_timezone() { + return time_zones_[current_time_zone_idx_] ? time_zones_[current_time_zone_idx_] : date::current_zone(); +} + +bool waybar::modules::Clock::is_timezone_fixed() { + return time_zones_[current_time_zone_idx_] != nullptr; +} + auto waybar::modules::Clock::update() -> void { - if (!fixed_time_zone_) { - // Time zone can change. Be sure to pick that. - time_zone_ = date::current_zone(); - } - - auto now = std::chrono::system_clock::now(); + auto time_zone = current_timezone(); + auto now = std::chrono::system_clock::now(); waybar_time wtime = {locale_, - date::make_zoned(time_zone_, date::floor(now))}; - - std::string text; - if (!fixed_time_zone_) { + date::make_zoned(time_zone, date::floor(now))}; + std::string text = ""; + if (!is_timezone_fixed()) { // As date dep is not fully compatible, prefer fmt tzset(); auto localtime = fmt::localtime(std::chrono::system_clock::to_time_t(now)); text = fmt::format(format_, localtime); - label_.set_markup(text); } else { text = fmt::format(format_, wtime); - label_.set_markup(text); } + label_.set_markup(text); if (tooltipEnabled()) { if (config_["tooltip-format"].isString()) { - const auto calendar = calendar_text(wtime); - auto tooltip_format = config_["tooltip-format"].asString(); - auto tooltip_text = fmt::format(tooltip_format, wtime, fmt::arg("calendar", calendar)); - label_.set_tooltip_markup(tooltip_text); - } else { - label_.set_tooltip_markup(text); + std::string calendarText = ""; + if (is_calendar_in_tooltip_) { + calendarText = calendar_text(wtime); + } + auto tooltip_format = config_["tooltip-format"].asString(); + text = fmt::format(tooltip_format, wtime, fmt::arg("calendar", calendarText)); } } + + label_.set_tooltip_markup(text); // Call parent update ALabel::update(); } -bool waybar::modules::Clock::setTimeZone(Json::Value zone_name) { - if (!zone_name.isString() || zone_name.asString().empty()) { - fixed_time_zone_ = false; - return false; - } - - time_zone_ = date::locate_zone(zone_name.asString()); - fixed_time_zone_ = true; - return true; -} - bool waybar::modules::Clock::handleScroll(GdkEventScroll *e) { // defer to user commands if set if (config_["on-scroll-up"].isString() || config_["on-scroll-down"].isString()) { @@ -97,17 +124,18 @@ bool waybar::modules::Clock::handleScroll(GdkEventScroll *e) { if (dir != SCROLL_DIR::UP && dir != SCROLL_DIR::DOWN) { return true; } - if (!config_["timezones"].isArray() || config_["timezones"].empty()) { + if (time_zones_.size() == 1) { return true; } - auto nr_zones = config_["timezones"].size(); + + auto nr_zones = time_zones_.size(); if (dir == SCROLL_DIR::UP) { - size_t new_idx = time_zone_idx_ + 1; - time_zone_idx_ = new_idx == nr_zones ? 0 : new_idx; + size_t new_idx = current_time_zone_idx_ + 1; + current_time_zone_idx_ = new_idx == nr_zones ? 0 : new_idx; } else { - time_zone_idx_ = time_zone_idx_ == 0 ? nr_zones - 1 : time_zone_idx_ - 1; + current_time_zone_idx_ = current_time_zone_idx_ == 0 ? nr_zones - 1 : current_time_zone_idx_ - 1; } - setTimeZone(config_["timezones"][time_zone_idx_]); + update(); return true; } From d8bc6c92bb49f4255dab1c24c88f1e9c1fecd549 Mon Sep 17 00:00:00 2001 From: Sergey Mishin Date: Tue, 5 Oct 2021 09:55:30 +0000 Subject: [PATCH 2/3] Fix style and spelling --- src/modules/clock.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/modules/clock.cpp b/src/modules/clock.cpp index d925c2b7..33028b78 100644 --- a/src/modules/clock.cpp +++ b/src/modules/clock.cpp @@ -31,7 +31,7 @@ waybar::modules::Clock::Clock(const std::string& id, const Json::Value& config) ); } - // If we parse all timezones and no one is good, add nullptr to the tmezones vector, to mark that we need to show localtime + // If all timezones are parsed and no one is good, add nullptr to the timezones vector, to mark that local time should be shown. if (!time_zones_.size()) { time_zones_.push_back(nullptr); } @@ -47,14 +47,14 @@ waybar::modules::Clock::Clock(const std::string& id, const Json::Value& config) spdlog::warn("As using a timezone, some format args may be missing as the date library haven't got a release since 2018."); } - // Check if we have to particular placeholder in tooltip format, to know what to calculate on update + // Check if a particular placeholder is present in the tooltip format, to know what to calculate on update. if (config_["tooltip-format"].isString()) { - std::string trimmedFormat = config_["tooltip-format"].asString(); - trimmedFormat.erase(std::remove_if(trimmedFormat.begin(), - trimmedFormat.end(), + std::string trimmed_format = config_["tooltip-format"].asString(); + trimmed_format.erase(std::remove_if(trimmed_format.begin(), + trimmed_format.end(), [](unsigned char x){return std::isspace(x);}), - trimmedFormat.end()); - if (trimmedFormat.find("{" + kCalendarPlaceholder + "}") != std::string::npos) { + trimmed_format.end()); + if (trimmed_format.find("{" + kCalendarPlaceholder + "}") != std::string::npos) { is_calendar_in_tooltip_ = true; } } @@ -100,12 +100,12 @@ auto waybar::modules::Clock::update() -> void { if (tooltipEnabled()) { if (config_["tooltip-format"].isString()) { - std::string calendarText = ""; + std::string calendar_lines = ""; if (is_calendar_in_tooltip_) { - calendarText = calendar_text(wtime); + calendar_lines = calendar_text(wtime); } auto tooltip_format = config_["tooltip-format"].asString(); - text = fmt::format(tooltip_format, wtime, fmt::arg("calendar", calendarText)); + text = fmt::format(tooltip_format, wtime, fmt::arg("calendar", calendar_lines)); } } From c5e4d2632045a42d620d929a976a0263fbdc047c Mon Sep 17 00:00:00 2001 From: Sergey Mishin Date: Tue, 5 Oct 2021 10:20:06 +0000 Subject: [PATCH 3/3] Fix working without timezone --- src/modules/clock.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/modules/clock.cpp b/src/modules/clock.cpp index 33028b78..7e7d7420 100644 --- a/src/modules/clock.cpp +++ b/src/modules/clock.cpp @@ -29,13 +29,8 @@ waybar::modules::Clock::Clock(const std::string& id, const Json::Value& config) zone_name.asString() ) ); - } - // If all timezones are parsed and no one is good, add nullptr to the timezones vector, to mark that local time should be shown. - if (!time_zones_.size()) { - time_zones_.push_back(nullptr); - } - } else { + } else if (config_["timezone"].isString() && !config_["timezone"].asString().empty()) { time_zones_.push_back( date::locate_zone( config_["timezone"].asString() @@ -43,6 +38,11 @@ waybar::modules::Clock::Clock(const std::string& id, const Json::Value& config) ); } + // If all timezones are parsed and no one is good, add nullptr to the timezones vector, to mark that local time should be shown. + if (!time_zones_.size()) { + time_zones_.push_back(nullptr); + } + if (!is_timezone_fixed()) { spdlog::warn("As using a timezone, some format args may be missing as the date library haven't got a release since 2018."); } @@ -105,7 +105,7 @@ auto waybar::modules::Clock::update() -> void { calendar_lines = calendar_text(wtime); } auto tooltip_format = config_["tooltip-format"].asString(); - text = fmt::format(tooltip_format, wtime, fmt::arg("calendar", calendar_lines)); + text = fmt::format(tooltip_format, wtime, fmt::arg(kCalendarPlaceholder.c_str(), calendar_lines)); } }