Repository: ultrabug/py3status

Issue:
keyboard_layout: keyboard shortcut stop working
Problem
--------------------

Switching keyboard layout causes layout switching shortcut to stop working (This might related to #1742)


Analysis
---------------------

[`_set_active`](https://github.com/ultrabug/py3status/blob/65a609261095f7502fc2814e9a6feb478845a69c/py3status/modules/keyboard_layout.py#L159-L163) switch layout by replacing all layout in `XkbLayout` with the target layout, This causes switching layout shortcut to not work since there's only a single layout left.

Currently, It's not possible to choose layout without replacing `XkbLayout` through `setxkbmap`,
Fortunately, `xkblayout-state` provides a proper method to switch layout.

Will submit PR later.

Discussion:

Patch:
diff --git a/py3status/constants.py b/py3status/constants.py
index 1b61ce98..78bddd93 100644
--- a/py3status/constants.py
+++ b/py3status/constants.py
@@ -61,6 +61,8 @@ ERROR_CONFIG = """
 }
 """
 
+COLOR_NAMES_EXCLUDED = ["good", "bad", "degraded", "separator", "threshold", "None"]
+
 COLOR_NAMES = {
     "aliceblue": "#f0f8ff",
     "antiquewhite": "#faebd7",
diff --git a/py3status/formatter.py b/py3status/formatter.py
index 62992315..bbbd3eea 100644
--- a/py3status/formatter.py
+++ b/py3status/formatter.py
@@ -6,6 +6,7 @@ from math import ceil
 from numbers import Number
 
 from py3status.composite import Composite
+from py3status.constants import COLOR_NAMES, COLOR_NAMES_EXCLUDED
 
 try:
     from urllib.parse import parse_qsl
@@ -52,6 +53,25 @@ class Formatter:
             self.format_string_cache[format_string] = tokens
         return self.format_string_cache[format_string]
 
+    def get_color_names(self, format_string):
+        """
+        Parses the format_string and returns a set of color names.
+        """
+        names = set()
+        # Tokenize the format string and process them
+        for token in self.tokens(format_string):
+            if token.group("command"):
+                name = dict(parse_qsl(token.group("command"))).get("color")
+                if (
+                    not name
+                    or name in COLOR_NAMES_EXCLUDED
+                    or name in COLOR_NAMES
+                    or name[0] == "#"
+                ):
+                    continue
+                names.add(name)
+        return names
+
     def get_placeholders(self, format_string):
         """
         Parses the format_string and returns a set of placeholders.
diff --git a/py3status/modules/keyboard_layout.py b/py3status/modules/keyboard_layout.py
index bf545189..1d2c83e7 100644
--- a/py3status/modules/keyboard_layout.py
+++ b/py3status/modules/keyboard_layout.py
@@ -18,7 +18,7 @@ Color options:
 Requires:
     xkblayout-state:
         or
-    setxkbmap: and `xset` (works for the first two predefined layouts.)
+    setxkbmap: and `xset` (works for the first two predefined layouts. overrides `XkbLayout` when switching layout.)
 
 Examples:
 ```
@@ -68,11 +68,14 @@ class Py3status:
 
     def post_config_hook(self):
         self.colors = getattr(self, "colors", None)  # old config
-        try:
-            self._xkblayout()
-            self._command = self._xkblayout
-        except self.py3.CommandError:
-            self._command = self._setxkbmap
+
+        avail_command = {
+            "xkblayout-state": (self._set_xkblayout, self._get_xkblayout),
+            "setxkbmap": (self._set_setxkbmap, self._get_setxkbmap),
+        }
+        self._set_command, self._get_command = avail_command[
+            self.py3.check_commands(avail_command.keys())
+        ]
 
         if not self.layouts:
             self.layouts = []
@@ -91,7 +94,7 @@ class Py3status:
         }
 
     def keyboard_layout(self):
-        layout, variant = self._command()
+        layout, variant = self._get_command()
         # If the current layout is not in our layouts list we need to add it
         if layout not in self._layouts:
             self._layouts = [layout] + self.layouts
@@ -131,7 +134,7 @@ class Py3status:
 
         return response
 
-    def _xkblayout(self):
+    def _get_xkblayout(self):
         layout, variant = [
             x.strip()
             for x in self.py3.command_output(
@@ -140,7 +143,7 @@ class Py3status:
         ]
         return layout, variant
 
-    def _setxkbmap(self):
+    def _get_setxkbmap(self):
         # this method works only for the first two predefined layouts.
         out = self.py3.command_output(["setxkbmap", "-query"])
         layouts = re.match(LAYOUTS_RE, out).group(1).split(",")
@@ -156,11 +159,24 @@ class Py3status:
         led_mask = re.match(LEDMASK_RE, xset_output).groups(0)[0]
         return layouts[int(led_mask)], ""
 
+    def _set_setxkbmap(self):
+        layout = self._layouts[self._active]
+        # Note: This will override user-defined layout, keyboard shortcut won't work
+        self.py3.command_run("setxkbmap -layout {}".format(layout))
+
+    def _set_xkblayout(self):
+        layout = self._layouts[self._active]
+        layout_pos = (
+            self.py3.command_output(["xkblayout-state", "print", "%S"])
+            .split()
+            .index(layout)
+        )
+        self.py3.command_run("xkblayout-state set {}".format(layout_pos))
+
     def _set_active(self, delta):
         self._active += delta
         self._active = self._active % len(self._layouts)
-        layout = self._layouts[self._active]
-        self.py3.command_run("setxkbmap -layout {}".format(layout))
+        self._set_command()
 
     def on_click(self, event):
         button = event["button"]
diff --git a/py3status/py3.py b/py3status/py3.py
index 0441fc17..1c30fa1a 100644
--- a/py3status/py3.py
+++ b/py3status/py3.py
@@ -730,15 +730,7 @@ class Py3:
             format_strings = [format_strings]
         names = set()
         for string in format_strings:
-            for color in string.replace("&", " ").split("color=")[1::1]:
-                color = color.split()[0]
-                if "#" in color:
-                    continue
-                if color in ["good", "bad", "degraded", "None", "threshold"]:
-                    continue
-                if color in COLOR_NAMES:
-                    continue
-                names.add(color)
+            names.update(self._formatter.get_color_names(string))
         return list(names)
 
     def get_placeholders_list(self, format_string, match=None):


Test patch:
diff --git a/tests/test_formatter.py b/tests/test_formatter.py
index beadf23a..8882ddbe 100644
--- a/tests/test_formatter.py
+++ b/tests/test_formatter.py
@@ -165,6 +165,20 @@ def update_placeholders(test_dict):
         pytest.fail("Results not as expected")
 
 
+def get_color_names(test_dict):
+    __tracebackhide__ = True
+
+    result = f.get_color_names(test_dict["format"])
+    expected = test_dict.get("expected")
+
+    if result != expected:
+        print("Format\n{}\n".format(test_dict["format"]))
+        print("Expected\n{}".format(pformat(expected)))
+        print("Got\n{}".format(pformat(result)))
+    if result != expected:
+        pytest.fail("Results not as expected")
+
+
 def test_1():
     run_formatter({"format": u"hello ☂", "expected": u"hello ☂"})
 
@@ -1359,6 +1373,42 @@ def test_update_placeholders_5():
     )
 
 
+def test_get_color_names_1():
+    get_color_names(
+        {
+            "format": r"\?color=red \?color=#0f0 green \?color=#0000ff blue",
+            "expected": set(),
+        }
+    )
+
+
+def test_get_color_names_2():
+    get_color_names(
+        {
+            "format": r"[\?color=tobias funke|\?color=bluemangroup troupe]",
+            "expected": {"tobias", "bluemangroup"},
+        }
+    )
+
+
+def test_get_color_names_3():
+    get_color_names(
+        {
+            "format": r"[\?color=good bonsai tree][\?color=None Saibot]",
+            "expected": set(),
+        }
+    )
+
+
+def test_get_color_names_4():
+    get_color_names(
+        {
+            "format": r"\?color=rebeccapurple \?color=rebecca \?color=purple",
+            "expected": {"rebecca"},
+        }
+    )
+
+
 if __name__ == "__main__":
     # run tests
     import sys