Merge pull request #5064 from Siriusuna/fix/mpd-markup-truncation

fix(mpd): truncate before sanitization and improve tooltip UX

include/modules/mpd/mpd.hpp

@@ -33,6 +33,7 @@ class MPD : public ALabel {
   detail::unique_status status_;
   mpd_state state_;
   detail::unique_song song_;
+  std::string ellipsis_;
 
  public:
   MPD(const std::string&, const Json::Value&);
@@ -45,6 +46,10 @@ class MPD : public ALabel {
   void setLabel();
   std::string getStateIcon() const;
   std::string getOptionIcon(const std::string& optionName, bool activated) const;
+  std::string getArtistStr(bool truncated) const;
+  std::string getAlbumArtistStr(bool truncated) const;
+  std::string getAlbumStr(bool truncated) const;
+  std::string getTitleStr(bool truncated) const;
 
   // GUI-side methods
   bool handlePlayPause(GdkEventButton* const&);

include/util/utf8_string.hpp

@@ -0,0 +1,7 @@
+#pragma once
+#include <string>
+
+namespace waybar::util {
+size_t utf8_width(const std::string& str);
+void utf8_truncate(std::string& s, const std::string& ellipsis, size_t max_len);
+}  // namespace waybar::util

meson.build

@@ -186,7 +186,8 @@ src_files = files(
     'src/util/icon_loader.cpp',
     'src/util/regex_collection.cpp',
     'src/util/css_reload_helper.cpp',
-    'src/util/transform_8bit_to_rgba.cpp'
+    'src/util/transform_8bit_to_rgba.cpp',
+    'src/util/utf8_string.cpp'
 )
 
 man_files = files(

src/modules/mpd/mpd.cpp

@@ -6,6 +6,7 @@
 
 #include <system_error>
 #include <util/sanitize_str.hpp>
+#include <util/utf8_string.hpp>
 using namespace waybar::util;
 
 #include "modules/mpd/state.hpp"
@@ -24,7 +25,8 @@ waybar::modules::MPD::MPD(const std::string& id, const Json::Value& config)
       timeout_(config_["timeout"].isUInt() ? config_["timeout"].asUInt() * 1'000 : 30'000),
       connection_(nullptr, &mpd_connection_free),
       status_(nullptr, &mpd_status_free),
-      song_(nullptr, &mpd_song_free) {
+      song_(nullptr, &mpd_song_free),
+      ellipsis_(config_["ellipsis"].isString() ? config_["ellipsis"].asString() : "\u2026") {
   if (!config_["port"].isNull() && !config_["port"].isUInt()) {
     spdlog::warn("{}: `port` configuration should be an unsigned int", module_name_);
   }
@@ -102,7 +104,6 @@ void waybar::modules::MPD::setLabel() {
   label_.get_style_context()->remove_class("disconnected");
 
   auto format = format_;
-  Glib::ustring artist, album_artist, album, title;
   std::string date, filename, uri;
   int song_pos = 0, queue_length = 0, volume = 0;
   std::chrono::seconds elapsedTime, totalTime;
@@ -131,10 +132,6 @@ void waybar::modules::MPD::setLabel() {
 
     stateIcon = getStateIcon();
 
-    artist = sanitize_string(getTag(MPD_TAG_ARTIST));
-    album_artist = sanitize_string(getTag(MPD_TAG_ALBUM_ARTIST));
-    album = sanitize_string(getTag(MPD_TAG_ALBUM));
-    title = sanitize_string(getTag(MPD_TAG_TITLE));
     date = sanitize_string(getTag(MPD_TAG_DATE));
     filename = sanitize_string(getFilename());
     uri = mpd_song_get_uri(song_.get());
@@ -156,17 +153,12 @@ void waybar::modules::MPD::setLabel() {
   std::string repeatIcon = getOptionIcon("repeat", repeatActivated);
   bool singleActivated = mpd_status_get_single(status_.get());
   std::string singleIcon = getOptionIcon("single", singleActivated);
-  if (config_["artist-len"].isInt()) artist = artist.substr(0, config_["artist-len"].asInt());
-  if (config_["album-artist-len"].isInt())
-    album_artist = album_artist.substr(0, config_["album-artist-len"].asInt());
-  if (config_["album-len"].isInt()) album = album.substr(0, config_["album-len"].asInt());
-  if (config_["title-len"].isInt()) title = title.substr(0, config_["title-len"].asInt());
 
   try {
     auto text = fmt::format(
-        fmt::runtime(format), fmt::arg("artist", artist.raw()),
-        fmt::arg("albumArtist", album_artist.raw()), fmt::arg("album", album.raw()),
-        fmt::arg("title", title.raw()), fmt::arg("date", date), fmt::arg("volume", volume),
+        fmt::runtime(format), fmt::arg("artist", getArtistStr(true)),
+        fmt::arg("albumArtist", getAlbumArtistStr(true)), fmt::arg("album", getAlbumStr(true)),
+        fmt::arg("title", getTitleStr(true)), fmt::arg("date", date), fmt::arg("volume", volume),
         fmt::arg("elapsedTime", elapsedTime), fmt::arg("totalTime", totalTime),
         fmt::arg("songPosition", song_pos), fmt::arg("queueLength", queue_length),
         fmt::arg("stateIcon", stateIcon), fmt::arg("consumeIcon", consumeIcon),
@@ -188,9 +180,9 @@ void waybar::modules::MPD::setLabel() {
                                                           : "MPD (connected)";
     try {
       auto tooltip_text = fmt::format(
-          fmt::runtime(tooltip_format), fmt::arg("artist", artist.raw()),
-          fmt::arg("albumArtist", album_artist.raw()), fmt::arg("album", album.raw()),
-          fmt::arg("title", title.raw()), fmt::arg("date", date), fmt::arg("volume", volume),
+          fmt::runtime(tooltip_format), fmt::arg("artist", getArtistStr(false)),
+          fmt::arg("albumArtist", getAlbumArtistStr(false)), fmt::arg("album", getAlbumStr(false)),
+          fmt::arg("title", getTitleStr(false)), fmt::arg("date", date), fmt::arg("volume", volume),
           fmt::arg("elapsedTime", elapsedTime), fmt::arg("totalTime", totalTime),
           fmt::arg("songPosition", song_pos), fmt::arg("queueLength", queue_length),
           fmt::arg("stateIcon", stateIcon), fmt::arg("consumeIcon", consumeIcon),
@@ -243,6 +235,38 @@ std::string waybar::modules::MPD::getOptionIcon(const std::string& optionName,
   }
 }
 
+std::string waybar::modules::MPD::getArtistStr(bool truncated) const {
+  std::string artist = getTag(MPD_TAG_ARTIST);
+  if (truncated && config_["artist-len"].isInt()) {
+    waybar::util::utf8_truncate(artist, ellipsis_, config_["artist-len"].asInt());
+  }
+  return sanitize_string(artist);
+}
+
+std::string waybar::modules::MPD::getAlbumArtistStr(bool truncated) const {
+  std::string album_artist = getTag(MPD_TAG_ALBUM_ARTIST);
+  if (truncated && config_["album-artist-len"].isInt()) {
+    waybar::util::utf8_truncate(album_artist, ellipsis_, config_["album-artist-len"].asInt());
+  }
+  return sanitize_string(album_artist);
+}
+
+std::string waybar::modules::MPD::getAlbumStr(bool truncated) const {
+  std::string album = getTag(MPD_TAG_ALBUM);
+  if (truncated && config_["album-len"].isInt()) {
+    waybar::util::utf8_truncate(album, ellipsis_, config_["album-len"].asInt());
+  }
+  return sanitize_string(album);
+}
+
+std::string waybar::modules::MPD::getTitleStr(bool truncated) const {
+  std::string title = getTag(MPD_TAG_TITLE);
+  if (truncated && config_["title-len"].isInt()) {
+    waybar::util::utf8_truncate(title, ellipsis_, config_["title-len"].asInt());
+  }
+  return sanitize_string(title);
+}
+
 static bool isServerUnavailable(const std::error_code& ec) {
   if (ec.category() == std::system_category()) {
     switch (ec.value()) {

src/modules/mpris/mpris.cpp

@@ -7,6 +7,7 @@
 #include <string>
 
 #include "util/scope_guard.hpp"
+#include "util/utf8_string.hpp"
 
 extern "C" {
 #include <playerctl/playerctl.h>
@@ -202,69 +203,22 @@ auto Mpris::getIconFromJson(const Json::Value& icons, const std::string& key) ->
 // Wide characters count as two, zero-width characters count as zero
 // Modifies str in-place (unless width = std::string::npos)
 // Returns the total width of the string pre-truncating
-size_t utf8_truncate(std::string& str, size_t width = std::string::npos) {
-  if (str.length() == 0) return 0;
-
-  const gchar* trunc_end = nullptr;
-
-  size_t total_width = 0;
-
-  for (gchar *data = str.data(), *end = data + str.size(); data != nullptr;) {
-    gunichar c = g_utf8_get_char_validated(data, end - data);
-    if (c == -1U || c == -2U) {
-      // invalid unicode, treat string as ascii
-      if (width != std::string::npos && str.length() > width) str.resize(width);
-      return str.length();
-    } else if (g_unichar_iswide(c)) {
-      total_width += 2;
-    } else if (!g_unichar_iszerowidth(c) && c != 0xAD) {  // neither zero-width nor soft hyphen
-      total_width += 1;
-    }
-
-    data = g_utf8_find_next_char(data, end);
-    if (width != std::string::npos && total_width <= width && !g_unichar_isspace(c))
-      trunc_end = data;
-  }
-
-  if (trunc_end) str.resize(trunc_end - str.data());
-
-  return total_width;
-}
-
-size_t utf8_width(const std::string& str) { return utf8_truncate(const_cast<std::string&>(str)); }
-
-void truncate(std::string& s, const std::string& ellipsis, size_t max_len) {
-  if (max_len == 0) {
-    s.resize(0);
-    return;
-  }
-  size_t len = utf8_truncate(s, max_len);
-  if (len > max_len) {
-    size_t ellipsis_len = utf8_width(ellipsis);
-    if (max_len >= ellipsis_len) {
-      if (ellipsis_len) utf8_truncate(s, max_len - ellipsis_len);
-      s += ellipsis;
-    } else {
-      s.resize(0);
-    }
-  }
-}
 
 auto Mpris::getArtistStr(const PlayerInfo& info, bool truncated) -> std::string {
   auto artist = info.artist.value_or(std::string());
-  if (truncated && artist_len_ >= 0) truncate(artist, ellipsis_, artist_len_);
+  if (truncated && artist_len_ >= 0) waybar::util::utf8_truncate(artist, ellipsis_, artist_len_);
   return artist;
 }
 
 auto Mpris::getAlbumStr(const PlayerInfo& info, bool truncated) -> std::string {
   auto album = info.album.value_or(std::string());
-  if (truncated && album_len_ >= 0) truncate(album, ellipsis_, album_len_);
+  if (truncated && album_len_ >= 0) waybar::util::utf8_truncate(album, ellipsis_, album_len_);
   return album;
 }
 
 auto Mpris::getTitleStr(const PlayerInfo& info, bool truncated) -> std::string {
   auto title = info.title.value_or(std::string());
-  if (truncated && title_len_ >= 0) truncate(title, ellipsis_, title_len_);
+  if (truncated && title_len_ >= 0) waybar::util::utf8_truncate(title, ellipsis_, title_len_);
   return title;
 }
 
@@ -292,9 +246,9 @@ auto Mpris::getDynamicStr(const PlayerInfo& info, bool truncated, bool html) ->
   // keep position format same as length format
   auto position = getPositionStr(info, truncated && truncate_hours_ && length.length() < 6);
 
-  size_t artistLen = utf8_width(artist);
-  size_t albumLen = utf8_width(album);
-  size_t titleLen = utf8_width(title);
+  size_t artistLen = waybar::util::utf8_width(artist);
+  size_t albumLen = waybar::util::utf8_width(album);
+  size_t titleLen = waybar::util::utf8_width(title);
   size_t lengthLen = length.length();
   size_t posLen = position.length();
 
@@ -313,7 +267,7 @@ auto Mpris::getDynamicStr(const PlayerInfo& info, bool truncated, bool html) ->
     // Since the first element doesn't present a separator and we don't know a priori which one
     // it will be, we add a "virtual separatorLen" to the dynamicLen, since we are adding the
     // separatorLen to all the other lengths.
-    size_t separatorLen = utf8_width(dynamic_separator_);
+    size_t separatorLen = waybar::util::utf8_width(dynamic_separator_);
     size_t dynamicLen = dynamic_len_ + separatorLen;
     if (showArtist) artistLen += separatorLen;
     if (showAlbum) albumLen += separatorLen;
@@ -775,9 +729,12 @@ auto Mpris::update() -> void {
       auto tooltip_text = fmt::format(
           fmt::runtime(tooltipstr), fmt::arg("player", info.name),
           fmt::arg("status", info.status_string),
-          fmt::arg("artist", getArtistStr(info, tooltip_len_limits_)),
-          fmt::arg("title", getTitleStr(info, tooltip_len_limits_)),
-          fmt::arg("album", getAlbumStr(info, tooltip_len_limits_)),
+          fmt::arg("artist",
+                   std::string(Glib::Markup::escape_text(getArtistStr(info, tooltip_len_limits_)))),
+          fmt::arg("title",
+                   std::string(Glib::Markup::escape_text(getTitleStr(info, tooltip_len_limits_)))),
+          fmt::arg("album",
+                   std::string(Glib::Markup::escape_text(getAlbumStr(info, tooltip_len_limits_)))),
           fmt::arg("length", tooltipLength), fmt::arg("position", tooltipPosition),
           fmt::arg("dynamic", getDynamicStr(info, tooltip_len_limits_, false)),
           fmt::arg("player_icon", getIconFromJson(config_["player-icons"], info.name)),

src/util/utf8_string.cpp

@@ -0,0 +1,62 @@
+#include <glib.h>
+
+#include <string>
+#include <util/utf8_string.hpp>
+
+namespace waybar::util {
+
+namespace {
+// Wide characters count as two, zero-width characters count as zero
+// Modifies str in-place (unless width = std::string::npos)
+// Returns the total width of the string pre-truncating
+size_t measure_and_truncate(std::string& str, size_t width = std::string::npos) {
+  if (str.length() == 0) return 0;
+
+  const gchar* trunc_end = nullptr;
+
+  size_t total_width = 0;
+
+  for (gchar *data = str.data(), *end = data + str.size(); data != nullptr;) {
+    gunichar c = g_utf8_get_char_validated(data, end - data);
+    if (c == -1U || c == -2U) {
+      // invalid unicode, treat string as ascii
+      if (width != std::string::npos && str.length() > width) str.resize(width);
+      return str.length();
+    } else if (g_unichar_iswide(c)) {
+      total_width += 2;
+    } else if (!g_unichar_iszerowidth(c) && c != 0xAD) {  // neither zero-width nor soft hyphen
+      total_width += 1;
+    }
+
+    data = g_utf8_find_next_char(data, end);
+    if (width != std::string::npos && total_width <= width && !g_unichar_isspace(c))
+      trunc_end = data;
+  }
+
+  if (trunc_end) str.resize(trunc_end - str.data());
+
+  return total_width;
+}
+}  // namespace
+
+size_t utf8_width(const std::string& str) {
+  return measure_and_truncate(const_cast<std::string&>(str));
+}
+
+void utf8_truncate(std::string& s, const std::string& ellipsis, size_t max_len) {
+  if (max_len == 0) {
+    s.resize(0);
+    return;
+  }
+  size_t len = measure_and_truncate(s, max_len);
+  if (len > max_len) {
+    size_t ellipsis_len = utf8_width(ellipsis);
+    if (max_len >= ellipsis_len) {
+      if (ellipsis_len) measure_and_truncate(s, max_len - ellipsis_len);
+      s += ellipsis;
+    } else {
+      s.resize(0);
+    }
+  }
+}
+}  // namespace waybar::util