Merge remote-tracking branch 'upstream/master' into scaling

.gitlab-ci.yml

@@ -84,3 +84,13 @@ test:
   script:
     - apt-get -y build-dep .
     - ninja -C _build test
+
+check_release:
+  <<: *tags
+  stage: test
+  only:
+    refs:
+      - master
+  script:
+    - apt-get install git python3
+    - (head -n 1 ./debian/changelog && git tag) | ./debian/check_release.py

Cargo.lock

@@ -2,10 +2,10 @@
 # It is not intended for manual editing.
 [[package]]
 name = "aho-corasick"
-version = "0.7.7"
+version = "0.7.8"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "memchr 2.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "memchr 2.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -254,7 +254,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
 name = "memchr"
-version = "2.3.0"
+version = "2.3.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
@@ -317,8 +317,8 @@ name = "regex"
 version = "1.1.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "aho-corasick 0.7.7 (registry+https://github.com/rust-lang/crates.io-index)",
+ "aho-corasick 0.7.8 (registry+https://github.com/rust-lang/crates.io-index)",
- "memchr 2.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "memchr 2.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "regex-syntax 0.6.14 (registry+https://github.com/rust-lang/crates.io-index)",
  "thread_local 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "utf8-ranges 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -457,7 +457,7 @@ dependencies = [
 ]
 
 [metadata]
-"checksum aho-corasick 0.7.7 (registry+https://github.com/rust-lang/crates.io-index)" = "5f56c476256dc249def911d6f7580b5fc7e875895b5d7ee88f5d602208035744"
+"checksum aho-corasick 0.7.8 (registry+https://github.com/rust-lang/crates.io-index)" = "743ad5a418686aad3b87fd14c43badd828cf26e214a00f92a384291cf22e1811"
 "checksum atk-sys 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "c7017e53393e713212aed7aea336b6553be4927f58c37070a56c2fe3d107e489"
 "checksum bitflags 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)" = "228047a76f468627ca71776ecdebd732a3423081fcf5125585bcd7c49886ce12"
 "checksum cairo-rs 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "dd940f0d609699e343ef71c4af5f66423afbf30d666f796dabd8fd15229cf5b6"
@@ -481,7 +481,7 @@ dependencies = [
 "checksum libc 0.2.66 (registry+https://github.com/rust-lang/crates.io-index)" = "d515b1f41455adea1313a4a2ac8a8a477634fbae63cc6100e3aebb207ce61558"
 "checksum linked-hash-map 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)" = "ae91b68aebc4ddb91978b11a1b02ddd8602a05ec19002801c5666000e05e0f83"
 "checksum maplit 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "3e2e65a1a2e43cfcb47a895c4c8b10d1f4a61097f9f254f183aee60cad9c651d"
-"checksum memchr 2.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "3197e20c7edb283f87c071ddfc7a2cca8f8e0b888c242959846a6fce03c72223"
+"checksum memchr 2.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "53445de381a1f436797497c61d851644d0e8e88e6140f22872ad33a704933978"
 "checksum memmap 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "6585fd95e7bb50d6cc31e20d4cf9afb4e2ba16c5846fc76793f11218da9c475b"
 "checksum pango 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "4c2cb169402a3eb1ba034a7cc7d95b8b1c106e9be5ba4be79a5a93dc1a2795f4"
 "checksum pango-sys 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "d6eb49268e69dd0c1da5d3001a61aac08e2e9d2bfbe4ae4b19b9963c998f6453"

README.md

@@ -56,4 +56,4 @@ $ src/squeekboard
 Developing
 ----------
 
-See `HACKING.md`
+See [`docs/hacking.md`](docs/hacking.md) for this copy, or the [official documentation](https://developer.puri.sm/projects/squeekboard/) for the current release.

debian/changelog

@@ -1,3 +1,40 @@
+squeekboard (1.9.0) amber-phone; urgency=medium
+
+  [ Dorota Czaplejewicz ]
+  * imservice: Add commit_string method
+  * submission: Handle submitting strings
+  * input_method: Use for erasing
+  * logging: Use in merged functions
+  * translations: Remove redundant ones
+  * translations: Translate builtin layouts
+  * greek: Rename to gr which is used by gnome settings
+
+  [ Sebastian Krzyszkowiak ]
+  * layouts: Add Polish layouts
+
+  [ Dorota Czaplejewicz ]
+  * locks: Draw based on current view
+  * locking: Lock keys statelessly
+  * layouts: Better accented uppercase in PL
+  * emoji: Add more choices
+  * row: Eliminate angle
+  * layout: Center views relative to each other and the layout bounds
+  * drawing: Generalized foreach_visible_button
+  * variant: Fix double-free
+  * variant: Fix leak
+  * keyboard_layout: Fix leak
+  * layout: Improve scoping of locked variable
+  * terminal: Make */ easier to reach
+
+  [ Sebastian Krzyszkowiak ]
+  * layouts: terminal: Use altline outline for dot key
+
+  [ Dorota Czaplejewicz ]
+  * text input: Disable erasing
+  * cargo: Update deps
+
+ -- Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>  Wed, 19 Feb 2020 14:32:39 +0000
+
 squeekboard (1.8.1) amber-phone; urgency=medium
 
   [ Dorota Czaplejewicz ]

debian/check_release.py

@@ -0,0 +1,10 @@
+#!/usr/bin/env python3
+
+"""Checks tag before release.
+Feed it the first changelog line, and then all available tags.
+"""
+
+import re, sys
+tag = "v" + re.findall("\\((.*)\\)", input())[0]
+if tag not in map(str.strip, sys.stdin.readlines()):
+    raise Exception("Changelog's current version doesn't have a tag. Push the tag!")

doc/hacking.md

@@ -3,10 +3,42 @@ Hacking
 
 This document describes the standards for modifying and maintaining the *squeekboard* project.
 
+Principles
+----------
+
+The project was built upon some guiding principles, which should be respected primarily by the maintainers, but also by contributors to avoid needlessly rejected changes.
+
+The overarching principle of *squeekboard* is to empower users.
+
+Software is primarily meant to solve problems of its users. Often in the quest to make software better, a hard distinction is made between the developer, who becomes the creator, and the user, who takes the role of the consumer, without direct influence on the software they use.
+This project aims to give users the power to make the software work for them by blurring the lines between users and developers.
+
+Nonwithstanding its current state, *squeekboard* must be structured in a way that provides users a gradual way to gain more experience and power to adjust it. It must be easy, in order of importance:
+
+- to use the software,
+- to modify its resources,
+- to change its behaviour,
+- to contribute upstream.
+
+To give an idea of what it means in practice, those are some examples of what has been important for *squeekboard* so far:
+
+- being quick and useable,
+- allowing local overrides of resources and config,
+- storing resources and config as editable, standard files,
+- having complete, up to date documentation of interfaces,
+- having an easy process of sending contributions,
+- adapting to to user's settings and constrains without overriding them,
+- avoiding compiling whenever possible,
+- making it easy to build,
+- having code that is [simple and obvious](https://www.python.org/dev/peps/pep-0020/),
+- having an easy process of testing and accepting contributions.
+
+You may notice that they are ordered roughly from "user-focused" to "maintainer-focused". While good properties are desired, sometimes they conflict, and maintainers should give additional weight to those benefitting the user compared to those benefitting regular contributors.
+
 Sending patches
 ---------------
 
-By submitting a change to this project, you agree to license it under the [GPL license version 3](./COPYING), or any later version. You also certify that your contribution fulfills the [Developer's Certificate of Origin 1.1](./dco.txt).
+By submitting a change to this project, you agree to license it under the [GPL license version 3](https://source.puri.sm/Librem5/squeekboard/blob/master/COPYING), or any later version. You also certify that your contribution fulfills the [Developer's Certificate of Origin 1.1](https://source.puri.sm/Librem5/squeekboard/blob/master/dco.txt).
 
 Development environment
 -----------------------
@@ -24,8 +56,7 @@ sudo apt-get -y install build-essential
 sudo apt-get -y build-dep .
 ```
 
-For an explicit list of dependencies check the `Build-Depends` entry in the
+For an explicit list of dependencies check the `Build-Depends` entry in the [`debian/control`](https://source.puri.sm/Librem5/squeekboard/blob/master/debian/control) file.
-[`debian/control`](./debian/control) file.
 
 Testing
 -------

doc/index.md

@@ -5,17 +5,23 @@ Contents
 --------
 
 * [Tutorial](tutorial.md)
+* [Contributing](hacking.md)
 
 Introduction
 ------------
 
-Squeekboard is the on-screen keyboard for the Librem 5 phone. For more information, look at the [README](https://source.puri.sm/Librem5/squeekboard/blob/master/README.md).
+Squeekboard is the on-screen keyboard for the Librem 5 phone. For information about building, look at the [README](https://source.puri.sm/Librem5/squeekboard/blob/master/README.md).
 
 Layouts
 -------
 
-Squeekboard allows user-provided keyboard layouts. They can be created without recompiling the keyboard code. The [tutorial](/tutorial.md) explains the process in detail.
+Squeekboard allows user-provided keyboard layouts. They can be created without recompiling the keyboard code. The [tutorial](tutorial.md) explains the process in detail.
 
 Layouts are created using a text-based format, based on YAML.
 
 TODO: Provide a description of the format.
+
+Contributions
+-------------
+
+Anyone is free to modify *squeekboard*. See the [contributing document](hacking.md).

doc/tutorial.md

@@ -31,7 +31,7 @@ So at least I will try to start writing a short how-to here and edit this post a
 **Running squeekboard**
 
 * Follow these instructions to run squeekboard: [https://source.puri.sm/Librem5/squeekboard/blob/master/README.md#running ](https://source.puri.sm/Librem5/squeekboard/blob/master/README.md#running)
-* Additionally take a look at https://source.puri.sm/Librem5/squeekboard/blob/master/HACKING.md#testing
+* Additionally take a look at the contribution document for [testing info](HACKING.md#testing)
 * You can either test it locally on your Linux system or use the [QEMU Librem 5 image ](https://developer.puri.sm/Librem5/Development_Environment/Boards/emulators.html)
 * To test squeekboard locally, you need phoc. Either compile that from the sources as well or use the CI repository ci.puri.sm for Debian based systems:
   `deb [arch=amd64] http://ci.puri.sm/ scratch librem5`

eek/eek-gtk-keyboard.c

@@ -99,7 +99,7 @@ eek_gtk_keyboard_real_draw (GtkWidget *self,
                                        gtk_widget_get_scale_factor (self));
     }
 
-    eek_renderer_render_keyboard (priv->renderer, cr, priv->keyboard);
+    eek_renderer_render_keyboard (priv->renderer, priv->submission, cr, priv->keyboard);
     return FALSE;
 }
 

eek/eek-renderer.c

@@ -191,8 +191,10 @@ render_button_label (cairo_t     *cr,
     g_object_unref (layout);
 }
 
+// FIXME: Pass just the active modifiers instead of entire submission
 void
 eek_renderer_render_keyboard (EekRenderer *self,
+                              struct submission *submission,
                                    cairo_t     *cr,
                               LevelKeyboard *keyboard)
 {
@@ -210,7 +212,7 @@ eek_renderer_render_keyboard (EekRenderer *self,
     cairo_scale (cr, self->widget_to_layout.scale, self->widget_to_layout.scale);
 
     squeek_draw_layout_base_view(keyboard->layout, self, cr);
-    squeek_layout_draw_all_changed(keyboard->layout, self, cr);
+    squeek_layout_draw_all_changed(keyboard->layout, self, cr, submission);
     cairo_restore (cr);
 }
 

eek/eek-renderer.h

@@ -25,6 +25,7 @@
 #include <pango/pangocairo.h>
 
 #include "eek-types.h"
+#include "src/submission.h"
 
 struct squeek_layout;
 
@@ -63,7 +64,7 @@ cairo_surface_t *eek_renderer_get_icon_surface(const gchar     *icon_name,
                                                 gint             size,
                                                 gint             scale);
 
-void             eek_renderer_render_keyboard  (EekRenderer     *renderer,
+void             eek_renderer_render_keyboard  (EekRenderer     *renderer, struct submission *submission,
                                                 cairo_t         *cr, LevelKeyboard *keyboard);
 void
 eek_renderer_free (EekRenderer        *self);

eekboard/eekboard-context-service.c

@@ -101,8 +101,14 @@ static void
 settings_get_layout(GSettings *settings, char **type, char **layout)
 {
     GVariant *inputs = g_settings_get_value(settings, "sources");
+    if (g_variant_n_children(inputs) == 0) {
+        g_warning("No system layout present");
+        *type = NULL;
+        *layout = NULL;
+    } else {
         // current layout is always first
         g_variant_get_child(inputs, 0, "(ss)", type, layout);
+    }
     g_variant_unref(inputs);
 }
 

squeekboard.doap

@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<Project xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#" xmlns:rdfs="http://www.w3.org/2000/01/rdf-schema#" xmlns="http://usefulinc.com/ns/doap#" xmlns:foaf="http://xmlns.com/foaf/0.1/" xmlns:admin="http://webns.net/mvcb/">
+ <name>squeekboard</name>
+ <shortdesc>A Wayland virtual keyboard</shortdesc>
+ <description>A virtual keyboard supporting Wayland, built primarily for the Librem 5 phone.</description>
+ <homepage rdf:resource="https://source.puri.sm/Librem5/squeekboard" />
+ <bug-database rdf:resource="https://source.puri.sm/Librem5/squeekboard/issues" />
+ <os>Linux</os>
+ <license rdf:resource="http://usefulinc.com/doap/licenses/gpl" />
+ <maintainer>
+  <foaf:Person>
+     <foaf:name>Dorota Czaplejewicz</foaf:name>
+     <foaf:mbox rdf:resource="mailto:dorota.czaplejewicz@puri.sm" />
+  </foaf:Person>
+ </maintainer>
+ 
+</Project>

src/action.rs

@@ -10,8 +10,11 @@ pub struct KeySym(pub String);
 type View = String;
 
 /// Use to send modified keypresses
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub enum Modifier {
+    /// Control and Alt are the only modifiers
+    /// which doesn't interfere with levels,
+    /// so it's simple to implement as levels are deprecated in squeekboard.
     Control,
     Alt,
 }
@@ -27,8 +30,8 @@ pub enum Action {
         /// When unlocked by pressing it or emitting a key
         unlock: View,
     },
-    /// Set this modifier TODO: release?
+    /// Hold this modifier for as long as the button is pressed
-    SetModifier(Modifier),
+    ApplyModifier(Modifier),
     /// Submit some text
     Submit {
         /// Text to submit with input-method.

src/data.rs

@@ -240,14 +240,20 @@ type ButtonIds = String;
 #[derive(Debug, Default, Deserialize, PartialEq)]
 #[serde(deny_unknown_fields)]
 struct ButtonMeta {
-    /// Special action to perform on activation. Conflicts with keysym, text.
+    // TODO: structure (action, keysym, text, modifier) as an enum
+    // to detect conflicts and missing values at compile time
+    /// Special action to perform on activation.
+    /// Conflicts with keysym, text, modifier.
     action: Option<Action>,
     /// The name of the XKB keysym to emit on activation.
-    /// Conflicts with action, text
+    /// Conflicts with action, text, modifier.
     keysym: Option<String>,
     /// The text to submit on activation. Will be derived from ID if not present
-    /// Conflicts with action, keysym
+    /// Conflicts with action, keysym, modifier.
     text: Option<String>,
+    /// The modifier to apply while the key is locked
+    /// Conflicts with action, keysym, text
+    modifier: Option<Modifier>,
     /// If not present, will be derived from text or the button ID
     label: Option<String>,
     /// Conflicts with label
@@ -270,6 +276,20 @@ enum Action {
     Erase,
 }
 
+#[derive(Debug, Clone, PartialEq, Deserialize)]
+#[serde(deny_unknown_fields)]
+enum Modifier {
+    Control,
+    Shift,
+    Lock,
+    #[serde(alias="Mod1")]
+    Alt,
+    Mod2,
+    Mod3,
+    Mod4,
+    Mod5,
+}
+
 #[derive(Debug, Clone, Deserialize, PartialEq)]
 #[serde(deny_unknown_fields)]
 struct Outline {
@@ -510,22 +530,27 @@ fn create_action<H: logging::Handler>(
         Action(Action),
         Text(String),
         Keysym(String),
+        Modifier(Modifier),
     };
     
     let submission = match (
         &symbol_meta.action,
         &symbol_meta.keysym,
-        &symbol_meta.text
+        &symbol_meta.text,
+        &symbol_meta.modifier,
     ) {
-        (Some(action), None, None) => SubmitData::Action(action.clone()),
+        (Some(action), None, None, None) => SubmitData::Action(action.clone()),
-        (None, Some(keysym), None) => SubmitData::Keysym(keysym.clone()),
+        (None, Some(keysym), None, None) => SubmitData::Keysym(keysym.clone()),
-        (None, None, Some(text)) => SubmitData::Text(text.clone()),
+        (None, None, Some(text), None) => SubmitData::Text(text.clone()),
-        (None, None, None) => SubmitData::Text(name.into()),
+        (None, None, None, Some(modifier)) => {
+            SubmitData::Modifier(modifier.clone())
+        },
+        (None, None, None, None) => SubmitData::Text(name.into()),
         _ => {
             warning_handler.handle(
                 logging::Level::Warning,
                 &format!(
-                    "Button {} has more than one of (action, keysym, text)",
+                    "Button {} has more than one of (action, keysym, text, modifier)",
                     name,
                 ),
             );
@@ -614,6 +639,26 @@ fn create_action<H: logging::Handler>(
                 })
             }).collect(),
         },
+        SubmitData::Modifier(modifier) => match modifier {
+            Modifier::Control => action::Action::ApplyModifier(
+                action::Modifier::Control,
+            ),
+            Modifier::Alt => action::Action::ApplyModifier(
+                action::Modifier::Alt,
+            ),
+            unsupported_modifier => {
+                warning_handler.handle(
+                    logging::Level::Bug,
+                    &format!(
+                        "Modifier {:?} unsupported", unsupported_modifier,
+                    ),
+                );
+                action::Action::Submit {
+                    text: None,
+                    keys: Vec::new(),
+                }
+            },
+        },
     }
 }
 
@@ -711,6 +756,7 @@ mod tests {
                         keysym: None,
                         action: None,
                         text: None,
+                        modifier: None,
                         label: Some("test".into()),
                         outline: None,
                     }
@@ -852,6 +898,7 @@ mod tests {
                         keysym: None,
                         text: None,
                         action: None,
+                        modifier: None,
                         label: Some("test".into()),
                         outline: None,
                     }

src/drawing.rs

@@ -3,9 +3,11 @@
 use cairo;
 use std::cell::RefCell;
 
+use ::action::Action;
 use ::keyboard;
 use ::layout::{ Button, Layout };
 use ::layout::c::{ EekGtkKeyboard, Point };
+use ::submission::Submission;
 
 use glib::translate::FromGlibPtrNone;
 use gtk::WidgetExt;
@@ -44,13 +46,21 @@ mod c {
         layout: *mut Layout,
         renderer: EekRenderer,
         cr: *mut cairo_sys::cairo_t,
+        submission: *const Submission,
     ) {
         let layout = unsafe { &mut *layout };
+        let submission = unsafe { &*submission };
         let cr = unsafe { cairo::Context::from_raw_none(cr) };
+        let active_modifiers = submission.get_active_modifiers();
 
         layout.foreach_visible_button(|offset, button| {
             let state = RefCell::borrow(&button.state).clone();
-            let locked = state.action.is_active(&layout.current_view);
+            let active_mod = match &state.action {
+                Action::ApplyModifier(m) => active_modifiers.contains(m),
+                _ => false,
+            };
+            let locked = state.action.is_active(&layout.current_view)
+                | active_mod;
             if state.pressed == keyboard::PressType::Pressed || locked {
                 render_button_at_position(
                     renderer, &cr,

src/keyboard.rs

@@ -23,6 +23,24 @@ pub enum PressType {
 
 pub type KeyCode = u32;
 
+bitflags!{
+    /// Map to `virtual_keyboard.modifiers` modifiers values
+    /// From https://www.x.org/releases/current/doc/kbproto/xkbproto.html#Keyboard_State
+    pub struct Modifiers: u8 {
+        const SHIFT = 0x1;
+        const LOCK = 0x2;
+        const CONTROL = 0x4;
+        /// Alt
+        const MOD1 = 0x8;
+        const MOD2 = 0x10;
+        const MOD3 = 0x20;
+        /// Meta
+        const MOD4 = 0x40;
+        /// AltGr
+        const MOD5 = 0x80;
+    }
+}
+
 /// When the submitted actions of keys need to be tracked,
 /// they need a stable, comparable ID
 #[derive(PartialEq)]
@@ -31,7 +49,7 @@ pub struct KeyStateId(*const KeyState);
 #[derive(Debug, Clone)]
 pub struct KeyState {
     pub pressed: PressType,
-    /// A cache of raw keycodes derived from Action::Sumbit given a keymap
+    /// A cache of raw keycodes derived from Action::Submit given a keymap
     pub keycodes: Vec<KeyCode>,
     /// Static description of what the key does when pressed or released
     pub action: Action,
@@ -46,6 +64,14 @@ impl KeyState {
         }
     }
 
+    #[must_use]
+    pub fn into_pressed(self) -> KeyState {
+        KeyState {
+            pressed: PressType::Pressed,
+            ..self
+        }
+    }
+
     /// KeyStates instances are the unique identifiers of pressed keys,
     /// and the actions submitted with them.
     pub fn get_id(keystate: &Rc<RefCell<KeyState>>) -> KeyStateId {

src/layout.h

@@ -7,6 +7,7 @@
 #include "eek/eek-gtk-keyboard.h"
 #include "eek/eek-renderer.h"
 #include "eek/eek-types.h"
+#include "src/submission.h"
 #include "virtual-keyboard-unstable-v1-client-protocol.h"
 #include "text-input-unstable-v3-client-protocol.h"
 
@@ -62,6 +63,6 @@ void squeek_layout_drag(struct squeek_layout *layout,
                         struct transformation widget_to_layout,
                         uint32_t timestamp, EekboardContextService *manager,
                         EekGtkKeyboard *ui_keyboard);
-void squeek_layout_draw_all_changed(struct squeek_layout *layout, EekRenderer* renderer, cairo_t     *cr);
+void squeek_layout_draw_all_changed(struct squeek_layout *layout, EekRenderer* renderer, cairo_t     *cr, struct submission *submission);
 void squeek_draw_layout_base_view(struct squeek_layout *layout, EekRenderer* renderer, cairo_t     *cr);
 #endif

src/layout.rs

@@ -26,10 +26,10 @@ use std::vec::Vec;
 
 use ::action::Action;
 use ::drawing;
-use ::keyboard::{ KeyState, PressType };
+use ::keyboard::KeyState;
 use ::logging;
 use ::manager;
-use ::submission::{ Submission, Timestamp };
+use ::submission::{ Submission, SubmitData, Timestamp };
 use ::util::find_max_double;
 
 // Traits
@@ -249,7 +249,7 @@ pub mod c {
         unsafe { Box::from_raw(layout) };
     }
 
-    /// Entry points for more complex procedures and algoithms which span multiple modules
+    /// Entry points for more complex procedures and algorithms which span multiple modules
     pub mod procedures {
         use super::*;
 
@@ -916,9 +916,38 @@ mod seat {
                 "Key {:?} was already pressed", rckey,
             );
         }
-        let mut key = rckey.borrow_mut();
+        let key: KeyState = {
-        submission.handle_press(&key, KeyState::get_id(rckey), time);
+            RefCell::borrow(rckey).clone()
-        key.pressed = PressType::Pressed;
+        };
+        let action = key.action.clone();
+        match action {
+            Action::Submit {
+                text: Some(text),
+                keys: _,
+            } => submission.handle_press(
+                KeyState::get_id(rckey),
+                SubmitData::Text(&text),
+                &key.keycodes,
+                time,
+            ),
+            Action::Submit {
+                text: None,
+                keys: _,
+            } => submission.handle_press(
+                KeyState::get_id(rckey),
+                SubmitData::Keycodes,
+                &key.keycodes,
+                time,
+            ),
+            Action::Erase => submission.handle_press(
+                KeyState::get_id(rckey),
+                SubmitData::Erase,
+                &key.keycodes,
+                time,
+            ),
+            _ => {},
+        };
+        RefCell::replace(rckey, key.into_pressed());
     }
 
     pub fn handle_release_key(
@@ -961,6 +990,18 @@ mod seat {
                     )
                     .apply()
             },
+            Action::ApplyModifier(modifier) => {
+                // FIXME: key id is unneeded with stateless locks
+                let key_id = KeyState::get_id(rckey);
+                let gets_locked = !submission.is_modifier_active(modifier.clone());
+                match gets_locked {
+                    true => submission.handle_add_modifier(
+                        key_id,
+                        modifier, time,
+                    ),
+                    false => submission.handle_drop_modifier(key_id, time),
+                }
+            }
             // only show when UI is present
             Action::ShowPreferences => if let Some(ui) = &ui {
                 // only show when layout manager is available
@@ -987,10 +1028,6 @@ mod seat {
                     }
                 }
             },
-            Action::SetModifier(_) => log_print!(
-                logging::Level::Bug,
-                "Modifiers unsupported",
-            ),
         };
 
         let pointer = ::util::Pointer(rckey.clone());
@@ -1006,6 +1043,7 @@ mod test {
     use super::*;
 
     use std::ffi::CString;
+    use ::keyboard::PressType;
 
     pub fn make_state() -> Rc<RefCell<::keyboard::KeyState>> {
         Rc::new(RefCell::new(::keyboard::KeyState {

src/popover.rs

@@ -29,6 +29,7 @@ mod variants {
 
     use glib::ToVariant;
     use glib::translate::FromGlibPtrFull;
+    use glib::translate::FromGlibPtrNone;
     use glib::translate::ToGlibPtr;
 
     /// Unpacks tuple & array variants
@@ -91,12 +92,7 @@ mod variants {
             unsafe {
                 let ret = glib_sys::g_variant_builder_end(builder);
                 glib_sys::g_variant_builder_unref(builder);
-                // HACK: This is to prevent C taking ownership
+                glib::Variant::from_glib_none(ret)
-                // of "floating" Variants,
-                // where Rust gets to keep a stale reference
-                // and crash when trying to drop it.
-                glib_sys::g_variant_ref_sink(ret);
-                glib::Variant::from_glib_full(ret)
             }
         }
     }

src/submission.rs

@@ -17,13 +17,18 @@
  * and those events SHOULD NOT cause any lost events.
  * */
 
-use ::action::Action;
+use std::collections::HashSet;
+use std::ffi::CString;
+use ::action::Modifier;
 use ::imservice;
 use ::imservice::IMService;
-use ::keyboard::{ KeyCode, KeyState, KeyStateId, PressType };
+use ::keyboard::{ KeyCode, KeyStateId, Modifiers, PressType };
-use ::logging;
+use ::util::vec_remove;
 use ::vkeyboard::VirtualKeyboard;
 
+// traits
+use std::iter::FromIterator;
+
 /// Gathers stuff defined in C or called by C
 pub mod c {
     use super::*;
@@ -60,6 +65,7 @@ pub mod c {
         Box::<Submission>::into_raw(Box::new(
             Submission {
                 imservice,
+                modifiers_active: Vec::new(),
                 virtual_keyboard: VirtualKeyboard(vk),
                 pressed: Vec::new(),
             }
@@ -106,50 +112,60 @@ enum SubmittedAction {
 pub struct Submission {
     imservice: Option<Box<IMService>>,
     virtual_keyboard: VirtualKeyboard,
+    modifiers_active: Vec<(KeyStateId, Modifier)>,
     pressed: Vec<(KeyStateId, SubmittedAction)>,
 }
 
+pub enum SubmitData<'a> {
+    Text(&'a CString),
+    Erase,
+    Keycodes,
+}
+
 impl Submission {
     /// Sends a submit text event if possible;
     /// otherwise sends key press and makes a note of it
     pub fn handle_press(
         &mut self,
-        key: &KeyState, key_id: KeyStateId,
+        key_id: KeyStateId,
+        data: SubmitData,
+        keycodes: &Vec<KeyCode>,
         time: Timestamp,
     ) {
-        match &key.action {
+        let mods_are_on = !self.modifiers_active.is_empty();
-            Action::Submit { text: _, keys: _ }
+
-                | Action::Erase
+        let was_committed_as_text = match (&mut self.imservice, mods_are_on) {
-            => (),
+            (Some(imservice), false) => {
-            _ => {
+                enum Outcome {
-                log_print!(
+                    Submitted(Result<(), imservice::SubmitError>),
-                    logging::Level::Bug,
+                    NotSubmitted,
-                    "Submitted key with action other than Submit or Erase",
-                );
-                return;
-            },
                 };
 
-        let was_committed_as_text = match (&mut self.imservice, &key.action) {
+                let submit_outcome = match data {
-            (Some(imservice), Action::Submit { text: Some(text), keys: _ }) => {
+                    SubmitData::Text(text) => {
-                let submit_result = imservice.commit_string(text)
+                        Outcome::Submitted(imservice.commit_string(text))
-                    .and_then(|_| imservice.commit());
-                match submit_result {
-                    Ok(()) => true,
-                    Err(imservice::SubmitError::NotActive) => false,
-                }
                     },
+                    SubmitData::Erase => {
                         /* Delete_surrounding_text takes byte offsets,
                          * so cannot work without get_surrounding_text.
                          * This is a bug in the protocol.
-            (Some(imservice), Action::Erase) => {
+                         */
-                let submit_result = imservice.delete_surrounding_text(1, 0)
+                        // imservice.delete_surrounding_text(1, 0),
-                    .and_then(|_| imservice.commit());
+                        Outcome::NotSubmitted
-                match submit_result {
+                    },
+                    SubmitData::Keycodes => Outcome::NotSubmitted,
+                };
+
+                match submit_outcome {
+                    Outcome::Submitted(result) => {
+                        match result.and_then(|()| imservice.commit()) {
                             Ok(()) => true,
                             Err(imservice::SubmitError::NotActive) => false,
                         }
-            }*/
+                    },
+                    Outcome::NotSubmitted => false,
+                }
+            },
             (_, _) => false,
         };
 
@@ -157,11 +173,11 @@ impl Submission {
             true => SubmittedAction::IMService,
             false => {
                 self.virtual_keyboard.switch(
-                    &key.keycodes,
+                    keycodes,
                     PressType::Pressed,
                     time,
                 );
-                SubmittedAction::VirtualKeyboard(key.keycodes.clone())
+                SubmittedAction::VirtualKeyboard(keycodes.clone())
             },
         };
         
@@ -187,4 +203,44 @@ impl Submission {
             }
         };
     }
+    
+    pub fn handle_add_modifier(
+        &mut self,
+        key_id: KeyStateId,
+        modifier: Modifier, _time: Timestamp,
+    ) {
+        self.modifiers_active.push((key_id, modifier));
+        self.update_modifiers();
+    }
+
+    pub fn handle_drop_modifier(
+        &mut self,
+        key_id: KeyStateId,
+        _time: Timestamp,
+    ) {
+        vec_remove(&mut self.modifiers_active, |(id, _)| *id == key_id);
+        self.update_modifiers();
+    }
+
+    fn update_modifiers(&mut self) {
+        let raw_modifiers = self.modifiers_active.iter()
+            .map(|(_id, m)| match m {
+                Modifier::Control => Modifiers::CONTROL,
+                Modifier::Alt => Modifiers::MOD1,
+            })
+            .fold(Modifiers::empty(), |m, n| m | n);
+        self.virtual_keyboard.set_modifiers_state(raw_modifiers);
+    }
+
+    pub fn is_modifier_active(&self, modifier: Modifier) -> bool {
+        self.modifiers_active.iter()
+            .position(|(_id, m)| *m == modifier)
+            .is_some()
+    }
+
+    pub fn get_active_modifiers(&self) -> HashSet<Modifier> {
+        HashSet::from_iter(
+            self.modifiers_active.iter().map(|(_id, m)| m.clone())
+        )
+    }
 }

src/util.rs

@@ -197,6 +197,12 @@ pub trait WarningHandler {
     fn handle(&mut self, warning: &str);
 }
 
+/// Removes the first matcing item
+pub fn vec_remove<T, F: FnMut(&T) -> bool>(v: &mut Vec<T>, pred: F) -> Option<T> {
+    let idx = v.iter().position(pred);
+    idx.map(|idx| v.remove(idx))
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/vkeyboard.rs

@@ -1,6 +1,6 @@
 /*! Managing the events belonging to virtual-keyboard interface. */
 
-use ::keyboard::{ KeyCode, PressType };
+use ::keyboard::{ KeyCode, Modifiers, PressType };
 use ::layout::c::LevelKeyboard;
 use ::submission::Timestamp;
 
@@ -26,6 +26,11 @@ pub mod c {
             virtual_keyboard: ZwpVirtualKeyboardV1,
             keyboard: LevelKeyboard,
         );
+        
+        pub fn eek_virtual_keyboard_set_modifiers(
+            virtual_keyboard: ZwpVirtualKeyboardV1,
+            modifiers: u32,
+        );
     }
 }
 
@@ -33,7 +38,7 @@ pub mod c {
 pub struct VirtualKeyboard(pub c::ZwpVirtualKeyboardV1);
 
 impl VirtualKeyboard {
-    // TODO: split out keyboard state management
+    // TODO: error out if keymap not set
     pub fn switch(
         &self,
         keycodes: &Vec<KeyCode>,
@@ -68,12 +73,16 @@ impl VirtualKeyboard {
         }
     }
     
+    pub fn set_modifiers_state(&self, modifiers: Modifiers) {
+        let modifiers = modifiers.bits() as u32;
+        unsafe {
+            c::eek_virtual_keyboard_set_modifiers(self.0, modifiers);
+        }
+    }
+    
     pub fn update_keymap(&self, keyboard: LevelKeyboard) {
         unsafe {
-            c::eek_virtual_keyboard_update_keymap(
+            c::eek_virtual_keyboard_update_keymap(self.0, keyboard);
-                self.0,
-                keyboard,
-            );
         }
     }
 }

src/wayland.c

@@ -20,6 +20,12 @@ void eek_virtual_keyboard_update_keymap(struct zwp_virtual_keyboard_v1 *zwp_virt
         keyboard->keymap_fd, keyboard->keymap_len);
 }
 
+void
+eek_virtual_keyboard_set_modifiers(struct zwp_virtual_keyboard_v1 *zwp_virtual_keyboard_v1, uint32_t mods_depressed) {
+    zwp_virtual_keyboard_v1_modifiers(zwp_virtual_keyboard_v1,
+                                      mods_depressed, 0, 0, 0);
+}
+
 int squeek_output_add_listener(struct wl_output *wl_output,
                                 const struct wl_output_listener *listener, void *data) {
     return wl_output_add_listener(wl_output, listener, data);