From 9f828800fbf9005a0f73904f3d58f7b1828513db Mon Sep 17 00:00:00 2001
From: Patrick Ziegler <p.ziegler96@gmail.com>
Date: Sat, 4 Sep 2021 14:36:01 +0200
Subject: [PATCH] fix(xworkspaces): Fix scrolling (#2492)

The scroll handler had two issues:

* It did not respect the order the desktops where displayed in
* It would not wrap around on the first desktop because of an integer
  underflow

Fixes #2491
---
 CHANGELOG.md                |  3 +++
 src/modules/xworkspaces.cpp | 39 +++++++++++++++++++++++++------------
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3ad80fba..db65179c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -132,6 +132,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([`#2371`](https://github.com/polybar/polybar/issues/2371))
 - `polybar -m` used to show both physical outputs and randr monitors, even if the outputs were covered by monitors.
   ([`#2481`](https://github.com/polybar/polybar/issues/2481))
+- `internal/xworkspaces`: Broken scroll-wrapping and order of workspaces when scrolling
+  ([`#2491`](https://github.com/polybar/polybar/issues/2491))
+
 
 ## [3.5.6] - 2021-05-24
 ### Build
diff --git a/src/modules/xworkspaces.cpp b/src/modules/xworkspaces.cpp
index e6c3cee4..1e4e8796 100644
--- a/src/modules/xworkspaces.cpp
+++ b/src/modules/xworkspaces.cpp
@@ -391,27 +391,42 @@ namespace modules {
     focus_direction(false);
   }
 
+  /**
+   * Focuses either the next or previous desktop.
+   *
+   * Will wrap around at the ends and go in the order the desktops are displayed.
+   */
   void xworkspaces_module::focus_direction(bool next) {
     std::lock_guard<std::mutex> lock(m_workspace_mutex);
-    vector<unsigned int> indexes;
+
+    unsigned int current_desktop{ewmh_util::get_current_desktop()};
+    int current_index = -1;
+
+    /*
+     * Desktop indices in the order they are displayed.
+     */
+    vector<unsigned int> indices;
+
     for (auto&& viewport : m_viewports) {
       for (auto&& desktop : viewport->desktops) {
-        indexes.emplace_back(desktop->index);
+
+        if (current_desktop == desktop->index) {
+          current_index = indices.size();
+        }
+
+        indices.emplace_back(desktop->index);
       }
     }
 
-    unsigned new_desktop;
-    unsigned int current_desktop{ewmh_util::get_current_desktop()};
-
-    if (next) {
-      new_desktop = math_util::min<unsigned int>(indexes.back(), current_desktop + 1);
-      new_desktop = new_desktop == current_desktop ? indexes.front() : new_desktop;
-    } else {
-      new_desktop = math_util::max<unsigned int>(indexes.front(), current_desktop - 1);
-      new_desktop = new_desktop == current_desktop ? indexes.back() : new_desktop;
+    if (current_index == -1) {
+      m_log.err("%s: Current desktop (%u) not found in list of desktops", name(), current_desktop);
+      return;
     }
 
-    focus_desktop(new_desktop);
+    int offset = next? 1 : -1;
+
+    int new_index = (current_index + offset + indices.size()) % indices.size();
+    focus_desktop(indices.at(new_index));
   }
 
   void xworkspaces_module::focus_desktop(unsigned new_desktop) {