Merge pull request #1637 from ogayot/kb-multi-layout

keyboard: fix use of multi-layout keyboard layout

subiquity/common/types.py

@@ -156,6 +156,12 @@ AnyStep = Union[StepPressKey, StepKeyPresent, StepResult]
 
 @attr.s(auto_attribs=True)
 class KeyboardSetting:
+    # This data structure represents a subset of the XKB options.
+    # As explained in the XKB configuration guide, XkbLayout and
+    # XkbVariant can hold multiple comma-separated values.
+    # http://www.xfree86.org/current/XKB-Config2.html#4
+    # Ideally, we would internally represent a keyboard setting as a
+    # toggle + a list of [layout, variant].
     layout: str
     variant: str = ''
     toggle: Optional[str] = None

subiquity/models/keyboard.py

@@ -43,6 +43,15 @@ BACKSPACE="guess"
 """
 
 
+class InconsistentMultiLayoutError(ValueError):
+    """ Exception to raise when a multi layout has a different number of
+    layouts and variants. """
+    def __init__(self, layouts: str, variants: str) -> None:
+        super().__init__(
+                f'inconsistent multi-layout: layouts="{layouts}"'
+                f' variants="{variants}"')
+
+
 def from_config_file(config_file):
     with open(config_file) as fp:
         content = fp.read()
@@ -91,13 +100,21 @@ class KeyboardModel:
         self._setting = value
 
     def validate_setting(self, setting: KeyboardSetting) -> None:
-        kbd_layout = self.keyboard_list.layout_map.get(setting.layout)
+        layout_tokens = setting.layout.split(",")
+        variant_tokens = setting.variant.split(",")
+
+        if len(layout_tokens) != len(variant_tokens):
+            raise InconsistentMultiLayoutError(
+                    layouts=setting.layout, variants=setting.variant)
+
+        for layout, variant in zip(layout_tokens, variant_tokens):
+            kbd_layout = self.keyboard_list.layout_map.get(layout)
             if kbd_layout is None:
-            raise ValueError(f'Unknown keyboard layout "{setting.layout}"')
-        if not any(variant.code == setting.variant
-                   for variant in kbd_layout.variants):
-            raise ValueError(f'Unknown keyboard variant "{setting.variant}" '
-                             f'for layout "{setting.layout}"')
+                raise ValueError(f'Unknown keyboard layout "{layout}"')
+            if not any(kbd_variant.code == variant
+                       for kbd_variant in kbd_layout.variants):
+                raise ValueError(f'Unknown keyboard variant "{variant}" '
+                                 f'for layout "{layout}"')
 
     def render_config_file(self):
         options = ""

subiquity/models/tests/test_keyboard.py

@@ -18,7 +18,10 @@ from subiquitycore.tests.parameterized import parameterized
 from subiquitycore.tests import SubiTestCase
 
 from subiquity.common.types import KeyboardSetting
-from subiquity.models.keyboard import KeyboardModel
+from subiquity.models.keyboard import (
+    InconsistentMultiLayoutError,
+    KeyboardModel,
+)
 
 
 class TestKeyboardModel(SubiTestCase):
@@ -45,6 +48,25 @@ class TestKeyboardModel(SubiTestCase):
             self.model.setting = val
         self.assertEqual(initial, self.model.setting)
 
+    def testMultiLayout(self):
+        val = KeyboardSetting(layout='us,ara', variant=',')
+        self.model.setting = val
+        self.assertEqual(self.model.setting, val)
+
+    def testInconsistentMultiLayout(self):
+        initial = self.model.setting
+        val = KeyboardSetting(layout='us,ara', variant='')
+        with self.assertRaises(InconsistentMultiLayoutError):
+            self.model.setting = val
+        self.assertEqual(self.model.setting, initial)
+
+    def testInvalidMultiLayout(self):
+        initial = self.model.setting
+        val = KeyboardSetting(layout='us,ara', variant='zz,')
+        with self.assertRaises(ValueError):
+            self.model.setting = val
+        self.assertEqual(self.model.setting, initial)
+
     @parameterized.expand([
         ['ast_ES.UTF-8', 'es', 'ast'],
         ['de_DE.UTF-8', 'de', ''],

subiquity/server/controllers/keyboard.py

@@ -14,7 +14,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import logging
-from typing import Dict, Optional, Sequence
+from typing import Dict, Optional, Sequence, Tuple
 import os
 import pwd
 
@@ -48,7 +48,7 @@ standard_non_latin_layouts = set(
 default_desktop_user = 'ubuntu'
 
 
-def latinizable(layout_code, variant_code):
+def latinizable(layout_code, variant_code) -> Optional[Tuple[str, str]]:
     """
     If this setting does not allow the typing of latin characters,
     return a setting that can be switched to one that can.