Refactor Seat::add_keyboard

With the new XkbConfig struct, it is now easier to just use xkbcommon's
default configuration, by moving the xkbcommon specific options into a
struct with according documentation.

Additionally, anvil now uses xkbcommon's defaults with all backends
(previously, the winit backend had a hardcoded french keyboard layout).

anvil/src/udev.rs

@@ -29,7 +29,7 @@ use smithay::drm::result::Error as DrmError;
 use smithay::input::Libinput;
 use smithay::wayland::compositor::CompositorToken;
 use smithay::wayland::output::{Mode, Output, PhysicalProperties};
-use smithay::wayland::seat::Seat;
+use smithay::wayland::seat::{Seat, XkbConfig};
 use smithay::wayland::shm::init_shm_global;
 use smithay::wayland_server::calloop::EventLoop;
 use smithay::wayland_server::protocol::wl_output;
@@ -97,7 +97,7 @@ pub fn run_udev(mut display: Display, mut event_loop: EventLoop<()>, log: Logger
 
     let pointer = w_seat.add_pointer();
     let keyboard = w_seat
-        .add_keyboard("", "", "", None, 1000, 500)
+        .add_keyboard(XkbConfig::default(), 1000, 500)
         .expect("Failed to initialize the keyboard");
 
     let (output, _output_global) = Output::new(

anvil/src/winit.rs

@@ -8,7 +8,7 @@ use smithay::backend::graphics::egl::EGLGraphicsBackend;
 use smithay::backend::input::InputBackend;
 use smithay::backend::winit;
 use smithay::wayland::output::{Mode, Output, PhysicalProperties};
-use smithay::wayland::seat::Seat;
+use smithay::wayland::seat::{Seat, XkbConfig};
 use smithay::wayland::shm::init_shm_global;
 use smithay::wayland_server::calloop::EventLoop;
 use smithay::wayland_server::protocol::wl_output;
@@ -53,7 +53,7 @@ pub fn run_winit(display: &mut Display, event_loop: &mut EventLoop<()>, log: Log
 
     let pointer = seat.add_pointer();
     let keyboard = seat
-        .add_keyboard("", "fr", "oss", None, 1000, 500)
+        .add_keyboard(XkbConfig::default(), 1000, 500)
         .expect("Failed to initialize the keyboard");
 
     let (output, _) = Output::new(

src/wayland/seat/keyboard.rs

@@ -1,4 +1,5 @@
 use backend::input::KeyState;
+use std::default::Default;
 use std::io::{Error as IoError, Write};
 use std::os::unix::io::AsRawFd;
 use std::sync::{Arc, Mutex};
@@ -57,6 +58,47 @@ impl ModifiersState {
     }
 }
 
+/// Configuration for xkbcommon.
+///
+/// For the fields that are not set ("" or None, as set in the `Default` impl), xkbcommon will use
+/// the values from the environment variables `XKB_DEFAULT_RULES`, `XKB_DEFAULT_MODEL`,
+/// `XKB_DEFAULT_LAYOUT`, `XKB_DEFAULT_VARIANT` and `XKB_DEFAULT_OPTIONS`.
+///
+/// For details, see the [documentation at xkbcommon.org][docs].
+///
+/// [docs]: https://xkbcommon.org/doc/current/structxkb__rule__names.html
+#[derive(Clone, Debug)]
+pub struct XkbConfig<'a> {
+    /// The rules file to use.
+    ///
+    /// The rules file describes how to interpret the values of the model, layout, variant and
+    /// options fields.
+    pub rules: &'a str,
+    /// The keyboard model by which to interpret keycodes and LEDs.
+    pub model: &'a str,
+    /// A comma separated list of layouts (languages) to include in the keymap.
+    pub layout: &'a str,
+    /// A comma separated list of variants, one per layout, which may modify or augment the
+    /// respective layout in various ways.
+    pub variant: &'a str,
+    /// A comma separated list of options, through which the user specifies non-layout related
+    /// preferences, like which key combinations are used for switching layouts, or which key is the
+    /// Compose key.
+    pub options: Option<String>,
+}
+
+impl<'a> Default for XkbConfig<'a> {
+    fn default() -> Self {
+        Self {
+            rules: "",
+            model: "",
+            layout: "",
+            variant: "",
+            options: None,
+        }
+    }
+}
+
 struct KbdInternal {
     known_kbds: Vec<Resource<WlKeyboard>>,
     focus: Option<Resource<WlSurface>>,
@@ -73,15 +115,7 @@ struct KbdInternal {
 unsafe impl Send for KbdInternal {}
 
 impl KbdInternal {
-    fn new(
+    fn new(xkb_config: XkbConfig, repeat_rate: i32, repeat_delay: i32) -> Result<KbdInternal, ()> {
-        rules: &str,
-        model: &str,
-        layout: &str,
-        variant: &str,
-        options: Option<String>,
-        repeat_rate: i32,
-        repeat_delay: i32,
-    ) -> Result<KbdInternal, ()> {
         // we create a new contex for each keyboard because libxkbcommon is actually NOT threadsafe
         // so confining it inside the KbdInternal allows us to use Rusts mutability rules to make
         // sure nothing goes wrong.
@@ -91,11 +125,11 @@ impl KbdInternal {
         let context = xkb::Context::new(xkb::CONTEXT_NO_FLAGS);
         let keymap = xkb::Keymap::new_from_names(
             &context,
-            &rules,
+            &xkb_config.rules,
-            &model,
+            &xkb_config.model,
-            &layout,
+            &xkb_config.layout,
-            &variant,
+            &xkb_config.variant,
-            options,
+            xkb_config.options,
             xkb::KEYMAP_COMPILE_NO_FLAGS,
         ).ok_or(())?;
         let state = xkb::State::new(&keymap);
@@ -182,25 +216,20 @@ pub enum Error {
 
 /// Create a keyboard handler from a set of RMLVO rules
 pub(crate) fn create_keyboard_handler(
-    rules: &str,
+    xkb_config: XkbConfig,
-    model: &str,
-    layout: &str,
-    variant: &str,
-    options: Option<String>,
     repeat_delay: i32,
     repeat_rate: i32,
     logger: &::slog::Logger,
 ) -> Result<KeyboardHandle, Error> {
     let log = logger.new(o!("smithay_module" => "xkbcommon_handler"));
     info!(log, "Initializing a xkbcommon handler with keymap query";
-        "rules" => rules, "model" => model, "layout" => layout, "variant" => variant,
+        "rules" => xkb_config.rules, "model" => xkb_config.model, "layout" => xkb_config.layout,
-        "options" => &options
+        "variant" => xkb_config.variant, "options" => &xkb_config.options
     );
-    let internal = KbdInternal::new(rules, model, layout, variant, options, repeat_rate, repeat_delay)
+    let internal = KbdInternal::new(xkb_config, repeat_rate, repeat_delay).map_err(|_| {
-        .map_err(|_| {
+        debug!(log, "Loading keymap failed");
-            debug!(log, "Loading keymap failed");
+        Error::BadKeymap
-            Error::BadKeymap
+    })?;
-        })?;
 
     info!(log, "Loaded Keymap"; "name" => internal.keymap.layouts().next());
 

src/wayland/seat/mod.rs

@@ -59,7 +59,9 @@ use std::sync::{Arc, Mutex};
 mod keyboard;
 mod pointer;
 
-pub use self::keyboard::{keysyms, Error as KeyboardError, KeyboardHandle, Keysym, ModifiersState};
+pub use self::keyboard::{
+    keysyms, Error as KeyboardError, KeyboardHandle, Keysym, ModifiersState, XkbConfig,
+};
 pub use self::pointer::{PointerAxisHandle, PointerHandle};
 use wayland_server::protocol::wl_seat;
 use wayland_server::{Display, Global, NewResource, Resource};
@@ -187,26 +189,34 @@ impl Seat {
     /// Calling this method on a seat that already has a keyboard capability
     /// will overwrite it, and will be seen by the clients as if the
     /// keyboard was unplugged and a new one was plugged.
+    ///
+    /// # Examples
+    ///
+    /// ```no_run
+    /// # extern crate smithay;
+    /// # use smithay::wayland::seat::{Seat, XkbConfig};
+    /// # let mut seat: Seat = unimplemented!();
+    /// let keyboard = seat
+    ///     .add_keyboard(
+    ///         XkbConfig {
+    ///             layout: "de",
+    ///             variant: "nodeadkeys",
+    ///             ..XkbConfig::default()
+    ///         },
+    ///         1000,
+    ///         500,
+    ///     )
+    ///     .expect("Failed to initialize the keyboard");
+    /// ```
     pub fn add_keyboard(
         &mut self,
-        model: &str,
+        xkb_config: keyboard::XkbConfig,
-        layout: &str,
-        variant: &str,
-        options: Option<String>,
         repeat_delay: i32,
         repeat_rate: i32,
     ) -> Result<KeyboardHandle, KeyboardError> {
         let mut inner = self.inner.lock().unwrap();
-        let keyboard = self::keyboard::create_keyboard_handler(
+        let keyboard =
-            "evdev", // we need this one
+            self::keyboard::create_keyboard_handler(xkb_config, repeat_delay, repeat_rate, &inner.log)?;
-            model,
-            layout,
-            variant,
-            options,
-            repeat_delay,
-            repeat_rate,
-            &inner.log,
-        )?;
         if inner.keyboard.is_some() {
             // there is already a keyboard, remove it and notify the clients
             // of the change