refactor(sway/language): misc. C++ style refactorings

include/modules/sway/language.hpp

@@ -5,6 +5,8 @@
 
 #include <map>
 #include <string>
+#include <type_traits>
+#include <vector>
 
 #include "ALabel.hpp"
 #include "bar.hpp"
@@ -21,7 +23,12 @@ class Language : public ALabel, public sigc::trackable {
   auto update() -> void override;
 
  private:
-  enum class DispayedShortFlag { None = 0, ShortName = 1, ShortDescription = 1 << 1 };
+  enum VisibleFields {
+    None = 0,
+    ShortName = 1,
+    ShortDescription = 1 << 1,
+    Variant = 1 << 2,
+  };
 
   struct Layout {
     std::string full_name;
@@ -29,6 +36,11 @@ class Language : public ALabel, public sigc::trackable {
     std::string variant;
     std::string short_description;
     std::string country_flag() const;
+
+    Layout() = default;
+    Layout(rxkb_layout*);
+
+    void addShortNameSuffix(std::string_view suffix);
   };
 
   class XKBContext {
@@ -40,7 +52,7 @@ class Language : public ALabel, public sigc::trackable {
    private:
     rxkb_context* context_ = nullptr;
     rxkb_layout* xkb_layout_ = nullptr;
-    Layout* layout_ = nullptr;
+    std::unique_ptr<Layout> layout_;
     std::map<std::string, rxkb_layout*> base_layouts_by_name_;
   };
 
@@ -57,8 +69,7 @@ class Language : public ALabel, public sigc::trackable {
   std::string tooltip_format_ = "";
   std::map<std::string, Layout> layouts_map_;
   bool hide_single_;
-  bool is_variant_displayed;
+  std::underlying_type_t<VisibleFields> visible_fields = VisibleFields::None;
-  std::byte displayed_short_flag = static_cast<std::byte>(DispayedShortFlag::None);
 
   util::JsonParser parser_;
   std::mutex mutex_;

src/modules/sway/language.cpp

@@ -20,12 +20,14 @@ const std::string Language::XKB_ACTIVE_LAYOUT_NAME_KEY = "xkb_active_layout_name
 Language::Language(const std::string& id, const Json::Value& config)
     : ALabel(config, "language", id, "{}", 0, true) {
   hide_single_ = config["hide-single-layout"].isBool() && config["hide-single-layout"].asBool();
-  is_variant_displayed = format_.find("{variant}") != std::string::npos;
   if (format_.find("{}") != std::string::npos || format_.find("{short}") != std::string::npos) {
-    displayed_short_flag |= static_cast<std::byte>(DispayedShortFlag::ShortName);
+    visible_fields |= VisibleFields::ShortName;
   }
   if (format_.find("{shortDescription}") != std::string::npos) {
-    displayed_short_flag |= static_cast<std::byte>(DispayedShortFlag::ShortDescription);
+    visible_fields |= VisibleFields::ShortDescription;
+  }
+  if (format_.find("{variant}") != std::string::npos) {
+    visible_fields |= VisibleFields::Variant;
   }
   if (config.isMember("tooltip-format")) {
     tooltip_format_ = config["tooltip-format"].asString();
@@ -106,7 +108,7 @@ auto Language::update() -> void {
       fmt::arg("variant", layout_.variant), fmt::arg("flag", layout_.country_flag())));
   label_.set_markup(display_layout);
   if (tooltipEnabled()) {
-    if (tooltip_format_ != "") {
+    if (!tooltip_format_.empty()) {
       auto tooltip_display_layout = trim(
           fmt::format(fmt::runtime(tooltip_format_), fmt::arg("short", layout_.short_name),
                       fmt::arg("shortDescription", layout_.short_description),
@@ -131,51 +133,43 @@ auto Language::set_current_layout(std::string current_layout) -> void {
 }
 
 auto Language::init_layouts_map(const std::vector<std::string>& used_layouts) -> void {
-  std::map<std::string, std::vector<Layout*>> found_by_short_names;
+  // First layout entry with this short name
+  std::map<std::string_view, Layout&> layout_by_short_names;
+  // Current number of layout entries with this short name
+  std::map<std::string_view, int> count_by_short_names;
   XKBContext xkb_context;
-  auto layout = xkb_context.next_layout();
+
+  bool want_unique_names = ((visible_fields & VisibleFields::Variant) == 0) &&
+                           ((visible_fields & ~VisibleFields::Variant) != 0);
+
+  auto* layout = xkb_context.next_layout();
   for (; layout != nullptr; layout = xkb_context.next_layout()) {
     if (std::find(used_layouts.begin(), used_layouts.end(), layout->full_name) ==
         used_layouts.end()) {
       continue;
     }
 
-    if (!is_variant_displayed) {
+    auto [it, added] = layouts_map_.emplace(layout->full_name, *layout);
-      auto short_name = layout->short_name;
+    if (!added) continue;
-      if (found_by_short_names.count(short_name) > 0) {
+
-        found_by_short_names[short_name].push_back(layout);
+    if (want_unique_names) {
-      } else {
+      auto prev = layout_by_short_names.emplace(it->second.short_name, it->second).first;
-        found_by_short_names[short_name] = {layout};
+      switch (int number = ++count_by_short_names[it->second.short_name]; number) {
+        case 1:
+          break;
+        case 2:
+          // First duplicate appeared, add suffix to the original entry
+          prev->second.addShortNameSuffix("1");
+          G_GNUC_FALLTHROUGH;
+        default:
+          it->second.addShortNameSuffix(std::to_string(number));
+          break;
       }
     }
 
-    layouts_map_.emplace(layout->full_name, *layout);
+    spdlog::debug("Language: new layout '{}' short='{}' variant='{}' shortDescription='{}'",
-  }
+                  it->second.full_name, it->second.short_name, it->second.variant,
-
+                  it->second.short_description);
-  if (is_variant_displayed || found_by_short_names.size() == 0) {
-    return;
-  }
-
-  std::map<std::string, int> short_name_to_number_map;
-  for (const auto& used_layout_name : used_layouts) {
-    auto found = layouts_map_.find(used_layout_name);
-    if (found == layouts_map_.end()) continue;
-    auto used_layout = &found->second;
-    auto layouts_with_same_name_list = found_by_short_names[used_layout->short_name];
-    if (layouts_with_same_name_list.size() < 2) {
-      continue;
-    }
-
-    if (short_name_to_number_map.count(used_layout->short_name) == 0) {
-      short_name_to_number_map[used_layout->short_name] = 1;
-    }
-
-    if (displayed_short_flag != static_cast<std::byte>(0)) {
-      int& number = short_name_to_number_map[used_layout->short_name];
-      used_layout->short_name = used_layout->short_name + std::to_string(number);
-      used_layout->short_description = used_layout->short_description + std::to_string(number);
-      ++number;
-    }
   }
 }
 
@@ -195,28 +189,31 @@ auto Language::XKBContext::next_layout() -> Layout* {
     return nullptr;
   }
 
-  auto description = std::string(rxkb_layout_get_description(xkb_layout_));
+  layout_ = std::make_unique<Layout>(xkb_layout_);
-  auto name = std::string(rxkb_layout_get_name(xkb_layout_));
+
-  auto variant_ = rxkb_layout_get_variant(xkb_layout_);
+  if (!layout_->short_description.empty()) {
-  std::string variant = variant_ == nullptr ? "" : std::string(variant_);
+    base_layouts_by_name_.emplace(layout_->short_name, xkb_layout_);
-  auto short_description_ = rxkb_layout_get_brief(xkb_layout_);
+  } else if (auto base = base_layouts_by_name_.find(layout_->short_name);
-  std::string short_description;
+             base != base_layouts_by_name_.end()) {
-  if (short_description_ != nullptr) {
+    layout_->short_description = rxkb_layout_get_brief(base->second);
-    short_description = std::string(short_description_);
-    base_layouts_by_name_.emplace(name, xkb_layout_);
-  } else {
-    auto base_layout = base_layouts_by_name_[name];
-    short_description =
-        base_layout == nullptr ? "" : std::string(rxkb_layout_get_brief(base_layout));
-  }
-  delete layout_;
-  layout_ = new Layout{description, name, variant, short_description};
-  return layout_;
   }
 
-Language::XKBContext::~XKBContext() {
+  return layout_.get();
-  rxkb_context_unref(context_);
+}
-  delete layout_;
+
+Language::XKBContext::~XKBContext() { rxkb_context_unref(context_); }
+
+Language::Layout::Layout(rxkb_layout* xkb_layout) {
+  short_name = rxkb_layout_get_name(xkb_layout);
+  full_name = rxkb_layout_get_description(xkb_layout);
+
+  if (const auto* value = rxkb_layout_get_variant(xkb_layout)) {
+    variant = value;
+  }
+
+  if (const auto* value = rxkb_layout_get_brief(xkb_layout)) {
+    short_description = value;
+  }
 }
 
 std::string Language::Layout::country_flag() const {
@@ -229,4 +226,12 @@ std::string Language::Layout::country_flag() const {
   if (result[7] < 0xa6 || result[7] > 0xbf) return "";
   return std::string{reinterpret_cast<char*>(result)};
 }
+
+void Language::Layout::addShortNameSuffix(std::string_view suffix) {
+  short_name += suffix;
+  if (!short_description.empty()) {
+    short_description += suffix;
+  }
+}
+
 }  // namespace waybar::modules::sway