From d1bc23e9d80a5f3489b44e96a331d3131bbb60a4 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Tue, 14 Jan 2020 12:02:25 +0000 Subject: [PATCH 1/9] imservice: Add commit_string method --- src/imservice.c | 7 +++++++ src/imservice.rs | 25 ++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/imservice.c b/src/imservice.c index 2a6184c8..e9683da6 100644 --- a/src/imservice.c +++ b/src/imservice.c @@ -49,6 +49,13 @@ void imservice_connect_listeners(struct zwp_input_method_v2 *im, struct imservic zwp_input_method_v2_add_listener(im, &input_method_listener, imservice); } +void +eek_input_method_commit_string(struct zwp_input_method_v2 *zwp_input_method_v2, const char *text) +{ + zwp_input_method_v2_commit_string(zwp_input_method_v2, text); +} + + /// Declared explicitly because _destroy is inline, /// making it unavailable in Rust void imservice_destroy_im(struct zwp_input_method_v2 *im) { diff --git a/src/imservice.rs b/src/imservice.rs index f5989a11..547b494e 100644 --- a/src/imservice.rs +++ b/src/imservice.rs @@ -1,4 +1,5 @@ use std::boxed::Box; +use std::ffi; use std::ffi::CString; use std::num::Wrapping; use std::string::String; @@ -29,6 +30,8 @@ pub mod c { fn imservice_destroy_im(im: *mut c::InputMethod); #[allow(improper_ctypes)] // IMService will never be dereferenced in C pub fn imservice_connect_listeners(im: *mut InputMethod, imservice: *const IMService); + pub fn eek_input_method_commit_string(im: *mut InputMethod, text: *const c_char); + fn eekboard_context_service_set_hint_purpose(state: *const StateManager, hint: u32, purpose: u32); fn server_context_service_show_keyboard(imservice: *const UIManager); fn server_context_service_hide_keyboard(imservice: *const UIManager); @@ -314,7 +317,7 @@ impl Default for IMProtocolState { pub struct IMService { /// Owned reference (still created and destroyed in C) - pub im: *const c::InputMethod, + pub im: *mut c::InputMethod, /// Unowned reference. Be careful, it's shared with C at large state_manager: *const c::StateManager, /// Unowned reference. Be careful, it's shared with C at large @@ -326,6 +329,13 @@ pub struct IMService { serial: Wrapping, } +pub enum SubmitError { + /// The input method had not been activated + NotActive, + /// Submitted text has null bytes + NullBytes(ffi::NulError), +} + impl IMService { pub fn new( im: *mut c::InputMethod, @@ -350,4 +360,17 @@ impl IMService { } imservice } + + pub fn commit_string(&self, text: &str) -> Result<(), SubmitError> { + match self.current.active { + true => { + let text = CString::new(text).map_err(SubmitError::NullBytes)?; + unsafe { + c::eek_input_method_commit_string(self.im, text.as_ptr()) + } + Ok(()) + }, + false => Err(SubmitError::NotActive), + } + } } From 42cb73cd8c305734e9d91830ee77d2d2598ec232 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Tue, 14 Jan 2020 13:13:42 +0000 Subject: [PATCH 2/9] submission: Handle submitting strings --- src/action.rs | 3 +- src/imservice.c | 5 +++ src/imservice.rs | 30 +++++++++++++---- src/keyboard.rs | 14 ++++++++ src/layout.rs | 12 ++----- src/submission.rs | 85 +++++++++++++++++++++++++++++++++++++++++++---- 6 files changed, 126 insertions(+), 23 deletions(-) diff --git a/src/action.rs b/src/action.rs index 97e32dc7..5045007a 100644 --- a/src/action.rs +++ b/src/action.rs @@ -31,7 +31,8 @@ pub enum Action { SetModifier(Modifier), /// Submit some text Submit { - /// Text to submit with input-method + /// Text to submit with input-method. + /// If None, then keys are to be submitted instead. text: Option, /// The key events this symbol submits when submitting text is not possible keys: Vec, diff --git a/src/imservice.c b/src/imservice.c index e9683da6..9a92be67 100644 --- a/src/imservice.c +++ b/src/imservice.c @@ -55,6 +55,11 @@ eek_input_method_commit_string(struct zwp_input_method_v2 *zwp_input_method_v2, zwp_input_method_v2_commit_string(zwp_input_method_v2, text); } +void +eek_input_method_commit(struct zwp_input_method_v2 *zwp_input_method_v2, uint32_t serial) +{ + zwp_input_method_v2_commit(zwp_input_method_v2, serial); +} /// Declared explicitly because _destroy is inline, /// making it unavailable in Rust diff --git a/src/imservice.rs b/src/imservice.rs index 547b494e..2adf9717 100644 --- a/src/imservice.rs +++ b/src/imservice.rs @@ -1,5 +1,9 @@ +/*! Manages zwp_input_method_v2 protocol. + * + * Library module. + */ + use std::boxed::Box; -use std::ffi; use std::ffi::CString; use std::num::Wrapping; use std::string::String; @@ -31,7 +35,7 @@ pub mod c { #[allow(improper_ctypes)] // IMService will never be dereferenced in C pub fn imservice_connect_listeners(im: *mut InputMethod, imservice: *const IMService); pub fn eek_input_method_commit_string(im: *mut InputMethod, text: *const c_char); - + pub fn eek_input_method_commit(im: *mut InputMethod, serial: u32); fn eekboard_context_service_set_hint_purpose(state: *const StateManager, hint: u32, purpose: u32); fn server_context_service_show_keyboard(imservice: *const UIManager); fn server_context_service_hide_keyboard(imservice: *const UIManager); @@ -332,8 +336,6 @@ pub struct IMService { pub enum SubmitError { /// The input method had not been activated NotActive, - /// Submitted text has null bytes - NullBytes(ffi::NulError), } impl IMService { @@ -361,10 +363,9 @@ impl IMService { imservice } - pub fn commit_string(&self, text: &str) -> Result<(), SubmitError> { + pub fn commit_string(&self, text: &CString) -> Result<(), SubmitError> { match self.current.active { true => { - let text = CString::new(text).map_err(SubmitError::NullBytes)?; unsafe { c::eek_input_method_commit_string(self.im, text.as_ptr()) } @@ -373,4 +374,21 @@ impl IMService { false => Err(SubmitError::NotActive), } } + + pub fn commit(&mut self) -> Result<(), SubmitError> { + match self.current.active { + true => { + unsafe { + c::eek_input_method_commit(self.im, self.serial.0) + } + self.serial += Wrapping(1u32); + Ok(()) + }, + false => Err(SubmitError::NotActive), + } + } + + pub fn is_active(&self) -> bool { + self.current.active + } } diff --git a/src/keyboard.rs b/src/keyboard.rs index 79cd36c4..87c26e0d 100644 --- a/src/keyboard.rs +++ b/src/keyboard.rs @@ -1,13 +1,16 @@ /*! State of the emulated keyboard and keys. * Regards the keyboard as if it was composed of switches. */ +use std::cell::RefCell; use std::collections::HashMap; use std::fmt; use std::io; +use std::rc::Rc; use std::string::FromUtf8Error; use ::action::Action; +// Traits use std::io::Write; use std::iter::{ FromIterator, IntoIterator }; @@ -19,6 +22,11 @@ pub enum PressType { pub type KeyCode = u32; +/// When the submitted actions of keys need to be tracked, +/// they need a stable, comparable ID +#[derive(PartialEq)] +pub struct KeyStateId(*const KeyState); + #[derive(Debug, Clone)] pub struct KeyState { pub pressed: PressType, @@ -48,6 +56,12 @@ impl KeyState { ..self } } + + /// KeyStates instances are the unique identifiers of pressed keys, + /// and the actions submitted with them. + pub fn get_id(keystate: &Rc>) -> KeyStateId { + KeyStateId(keystate.as_ptr() as *const KeyState) + } } /// Sorts an iterator by converting it to a Vector and back diff --git a/src/layout.rs b/src/layout.rs index 413e924a..8f8b0055 100644 --- a/src/layout.rs +++ b/src/layout.rs @@ -861,11 +861,7 @@ mod seat { eprintln!("Warning: key {:?} was already pressed", rckey); } let mut key = rckey.borrow_mut(); - submission.virtual_keyboard.switch( - &key.keycodes, - PressType::Pressed, - time, - ); + submission.handle_press(&key, KeyState::get_id(rckey), time); key.pressed = PressType::Pressed; } @@ -893,11 +889,7 @@ mod seat { match action { Action::Submit { text: _, keys: _ } => { unstick_locks(layout).apply(); - submission.virtual_keyboard.switch( - &key.keycodes, - PressType::Released, - time, - ); + submission.handle_release(KeyState::get_id(rckey), time); }, Action::SetView(view) => { try_set_view(layout, view) diff --git a/src/submission.rs b/src/submission.rs index db7e6cc4..01dd979b 100644 --- a/src/submission.rs +++ b/src/submission.rs @@ -16,8 +16,11 @@ * The text-input interface may be enabled and disabled at arbitrary times, * and those events SHOULD NOT cause any lost events. * */ - + +use ::action::Action; +use ::imservice; use ::imservice::IMService; +use ::keyboard::{ KeyCode, KeyState, KeyStateId, PressType }; use ::vkeyboard::VirtualKeyboard; /// Gathers stuff defined in C or called by C @@ -57,6 +60,7 @@ pub mod c { Submission { imservice, virtual_keyboard: VirtualKeyboard(vk), + pressed: Vec::new(), } )) } @@ -92,9 +96,78 @@ pub mod c { #[derive(Clone, Copy)] pub struct Timestamp(pub u32); -pub struct Submission { - // used by C callbacks internally, TODO: make use with virtual keyboard - #[allow(dead_code)] - imservice: Option>, - pub virtual_keyboard: VirtualKeyboard, +enum SubmittedAction { + /// A collection of keycodes that were pressed + VirtualKeyboard(Vec), + IMService, +} + +pub struct Submission { + imservice: Option>, + virtual_keyboard: VirtualKeyboard, + pressed: Vec<(KeyStateId, SubmittedAction)>, +} + +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, + time: Timestamp, + ) { + let key_string = match &key.action { + Action::Submit { text, keys: _ } => text, + _ => { + eprintln!("BUG: Submitted key with action other than Submit"); + return; + }, + }; + + let text_was_committed = match (&mut self.imservice, key_string) { + (Some(imservice), Some(text)) => { + let submit_result = imservice.commit_string(text) + .and_then(|_| imservice.commit()); + match submit_result { + Ok(()) => true, + Err(imservice::SubmitError::NotActive) => false, + } + }, + (_, _) => false, + }; + + let submit_action = match text_was_committed { + true => SubmittedAction::IMService, + false => { + self.virtual_keyboard.switch( + &key.keycodes, + PressType::Pressed, + time, + ); + SubmittedAction::VirtualKeyboard(key.keycodes.clone()) + }, + }; + + self.pressed.push((key_id, submit_action)); + } + + pub fn handle_release(&mut self, key_id: KeyStateId, time: Timestamp) { + let index = self.pressed.iter().position(|(id, _)| *id == key_id); + if let Some(index) = index { + let (_id, action) = self.pressed.remove(index); + match action { + // string already sent, nothing to do + SubmittedAction::IMService => {}, + // no matter if the imservice got activated, + // keys must be released + SubmittedAction::VirtualKeyboard(keycodes) => { + self.virtual_keyboard.switch( + &keycodes, + PressType::Released, + time, + ) + }, + } + }; + } } From 585ed5e97dc8a259fc6ad7d496874284736d01ec Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Thu, 23 Jan 2020 15:39:40 +0000 Subject: [PATCH 3/9] input_method: Use for erasing --- data/keyboards/de.yaml | 2 +- data/keyboards/de_wide.yaml | 2 +- data/keyboards/el.yaml | 2 +- data/keyboards/es.yaml | 2 +- data/keyboards/fi.yaml | 2 +- data/keyboards/it.yaml | 2 +- data/keyboards/jp+kana.yaml | 2 +- data/keyboards/jp+kana_wide.yaml | 2 +- data/keyboards/no.yaml | 2 +- data/keyboards/number.yaml | 2 +- data/keyboards/se.yaml | 2 +- data/keyboards/terminal.yaml | 2 +- data/keyboards/us.yaml | 4 ++-- data/keyboards/us_wide.yaml | 2 +- src/action.rs | 2 ++ src/data.rs | 11 ++++++++- src/imservice.c | 5 ++++ src/imservice.rs | 19 +++++++++++++++ src/keyboard.rs | 41 ++++++++++++++++++++++++-------- src/layout.rs | 4 +++- src/submission.rs | 22 ++++++++++++----- 21 files changed, 101 insertions(+), 33 deletions(-) diff --git a/data/keyboards/de.yaml b/data/keyboards/de.yaml index 66566289..06687ebf 100644 --- a/data/keyboards/de.yaml +++ b/data/keyboards/de.yaml @@ -45,7 +45,7 @@ buttons: BackSpace: outline: "altline" icon: "edit-clear-symbolic" - keysym: "BackSpace" + action: "erase" preferences: action: "show_prefs" outline: "special" diff --git a/data/keyboards/de_wide.yaml b/data/keyboards/de_wide.yaml index 1f638557..dc5dec0f 100644 --- a/data/keyboards/de_wide.yaml +++ b/data/keyboards/de_wide.yaml @@ -45,7 +45,7 @@ buttons: BackSpace: outline: "altline" icon: "edit-clear-symbolic" - keysym: "BackSpace" + action: "erase" preferences: action: "show_prefs" outline: "special" diff --git a/data/keyboards/el.yaml b/data/keyboards/el.yaml index f24b207a..6279f75b 100644 --- a/data/keyboards/el.yaml +++ b/data/keyboards/el.yaml @@ -46,7 +46,7 @@ buttons: BackSpace: outline: "altline" icon: "edit-clear-symbolic" - keysym: "BackSpace" + action: "erase" preferences: action: "show_prefs" outline: "altline" diff --git a/data/keyboards/es.yaml b/data/keyboards/es.yaml index bc7ba73f..7343e7b3 100644 --- a/data/keyboards/es.yaml +++ b/data/keyboards/es.yaml @@ -44,7 +44,7 @@ buttons: BackSpace: outline: "altline" icon: "edit-clear-symbolic" - keysym: "BackSpace" + action: "erase" preferences: action: "show_prefs" outline: "default" diff --git a/data/keyboards/fi.yaml b/data/keyboards/fi.yaml index ec972f47..25493d70 100644 --- a/data/keyboards/fi.yaml +++ b/data/keyboards/fi.yaml @@ -39,7 +39,7 @@ buttons: BackSpace: outline: "altline" icon: "edit-clear-symbolic" - keysym: "BackSpace" + action: "erase" preferences: action: "show_prefs" outline: "altline" diff --git a/data/keyboards/it.yaml b/data/keyboards/it.yaml index 7a05ed46..08930472 100644 --- a/data/keyboards/it.yaml +++ b/data/keyboards/it.yaml @@ -46,7 +46,7 @@ buttons: BackSpace: outline: "altline" icon: "edit-clear-symbolic" - keysym: "BackSpace" + action: "erase" preferences: action: "show_prefs" outline: "default" diff --git a/data/keyboards/jp+kana.yaml b/data/keyboards/jp+kana.yaml index 076a7db1..b129d3d7 100644 --- a/data/keyboards/jp+kana.yaml +++ b/data/keyboards/jp+kana.yaml @@ -195,7 +195,7 @@ buttons: BackSpace: outline: "wide" icon: "edit-clear-symbolic" - keysym: "BackSpace" + action: erase Return: outline: "wide" icon: "key-enter" diff --git a/data/keyboards/jp+kana_wide.yaml b/data/keyboards/jp+kana_wide.yaml index 684cedfc..8f0c536e 100644 --- a/data/keyboards/jp+kana_wide.yaml +++ b/data/keyboards/jp+kana_wide.yaml @@ -195,7 +195,7 @@ buttons: BackSpace: outline: "wide" icon: "edit-clear-symbolic" - keysym: "BackSpace" + action: erase Return: outline: "wide" icon: "key-enter" diff --git a/data/keyboards/no.yaml b/data/keyboards/no.yaml index 27ff21ff..44de74cc 100644 --- a/data/keyboards/no.yaml +++ b/data/keyboards/no.yaml @@ -39,7 +39,7 @@ buttons: BackSpace: outline: "altline" icon: "edit-clear-symbolic" - keysym: "BackSpace" + action: erase preferences: action: "show_prefs" outline: "altline" diff --git a/data/keyboards/number.yaml b/data/keyboards/number.yaml index 3f672cd4..126d17ac 100644 --- a/data/keyboards/number.yaml +++ b/data/keyboards/number.yaml @@ -16,7 +16,7 @@ buttons: BackSpace: outline: "altline" icon: "edit-clear-symbolic" - keysym: "BackSpace" + action: erase space: outline: spaceline text: " " diff --git a/data/keyboards/se.yaml b/data/keyboards/se.yaml index 0a0a127f..b51f49ad 100644 --- a/data/keyboards/se.yaml +++ b/data/keyboards/se.yaml @@ -39,7 +39,7 @@ buttons: BackSpace: outline: "altline" icon: "edit-clear-symbolic" - keysym: "BackSpace" + action: erase preferences: action: "show_prefs" outline: "altline" diff --git a/data/keyboards/terminal.yaml b/data/keyboards/terminal.yaml index 9e8968d0..2b4d4f0e 100644 --- a/data/keyboards/terminal.yaml +++ b/data/keyboards/terminal.yaml @@ -45,7 +45,7 @@ buttons: BackSpace: outline: "altline" icon: "edit-clear-symbolic" - keysym: "BackSpace" + action: erase preferences: action: "show_prefs" outline: "special" diff --git a/data/keyboards/us.yaml b/data/keyboards/us.yaml index d150c480..7a7f4dd6 100644 --- a/data/keyboards/us.yaml +++ b/data/keyboards/us.yaml @@ -39,9 +39,9 @@ buttons: BackSpace: outline: "altline" icon: "edit-clear-symbolic" - keysym: "BackSpace" + action: erase preferences: - action: "show_prefs" + action: show_prefs outline: "special" icon: "keyboard-mode-symbolic" show_numbers: diff --git a/data/keyboards/us_wide.yaml b/data/keyboards/us_wide.yaml index 654db8f8..cf349c00 100644 --- a/data/keyboards/us_wide.yaml +++ b/data/keyboards/us_wide.yaml @@ -39,7 +39,7 @@ buttons: BackSpace: outline: "altline" icon: "edit-clear-symbolic" - keysym: "BackSpace" + action: "erase" preferences: action: "show_prefs" outline: "special" diff --git a/src/action.rs b/src/action.rs index 5045007a..98bdcd83 100644 --- a/src/action.rs +++ b/src/action.rs @@ -37,5 +37,7 @@ pub enum Action { /// The key events this symbol submits when submitting text is not possible keys: Vec, }, + /// Erase a position behind the cursor + Erase, ShowPreferences, } diff --git a/src/data.rs b/src/data.rs index 0fa30751..4798a453 100644 --- a/src/data.rs +++ b/src/data.rs @@ -15,6 +15,7 @@ use std::vec::Vec; use xkbcommon::xkb; +use ::action; use ::keyboard::{ KeyState, PressType, generate_keymap, generate_keycodes, FormattingError @@ -259,6 +260,9 @@ enum Action { SetView(String), #[serde(rename="show_prefs")] ShowPrefs, + /// Remove last character + #[serde(rename="erase")] + Erase, } #[derive(Debug, Clone, Deserialize, PartialEq)] @@ -381,6 +385,10 @@ impl Layout { ) }).collect() }, + action::Action::Erase => vec![ + *keymap.get("BackSpace") + .expect(&format!("BackSpace missing from keymap")), + ], _ => Vec::new(), }; ( @@ -547,6 +555,7 @@ fn create_action( SubmitData::Action( Action::ShowPrefs ) => ::action::Action::ShowPreferences, + SubmitData::Action(Action::Erase) => action::Action::Erase, SubmitData::Keysym(keysym) => ::action::Action::Submit { text: None, keys: vec!(::action::KeySym( @@ -581,7 +590,7 @@ fn create_action( false => format!("U{:04X}", codepoint as u32), }) }).collect(), - } + }, } } diff --git a/src/imservice.c b/src/imservice.c index 9a92be67..b6b04784 100644 --- a/src/imservice.c +++ b/src/imservice.c @@ -55,6 +55,11 @@ eek_input_method_commit_string(struct zwp_input_method_v2 *zwp_input_method_v2, zwp_input_method_v2_commit_string(zwp_input_method_v2, text); } +void +eek_input_method_delete_surrounding_text(struct zwp_input_method_v2 *zwp_input_method_v2, uint32_t before_length, uint32_t after_length) { + zwp_input_method_v2_delete_surrounding_text(zwp_input_method_v2, before_length, after_length); +}; + void eek_input_method_commit(struct zwp_input_method_v2 *zwp_input_method_v2, uint32_t serial) { diff --git a/src/imservice.rs b/src/imservice.rs index 2adf9717..e040157f 100644 --- a/src/imservice.rs +++ b/src/imservice.rs @@ -35,6 +35,7 @@ pub mod c { #[allow(improper_ctypes)] // IMService will never be dereferenced in C pub fn imservice_connect_listeners(im: *mut InputMethod, imservice: *const IMService); pub fn eek_input_method_commit_string(im: *mut InputMethod, text: *const c_char); + pub fn eek_input_method_delete_surrounding_text(im: *mut InputMethod, before: u32, after: u32); pub fn eek_input_method_commit(im: *mut InputMethod, serial: u32); fn eekboard_context_service_set_hint_purpose(state: *const StateManager, hint: u32, purpose: u32); fn server_context_service_show_keyboard(imservice: *const UIManager); @@ -375,6 +376,24 @@ impl IMService { } } + pub fn delete_surrounding_text( + &self, + before: u32, after: u32, + ) -> Result<(), SubmitError> { + match self.current.active { + true => { + unsafe { + c::eek_input_method_delete_surrounding_text( + self.im, + before, after, + ) + } + Ok(()) + }, + false => Err(SubmitError::NotActive), + } + } + pub fn commit(&mut self) -> Result<(), SubmitError> { match self.current.active { true => { diff --git a/src/keyboard.rs b/src/keyboard.rs index 87c26e0d..bae0cd3b 100644 --- a/src/keyboard.rs +++ b/src/keyboard.rs @@ -79,9 +79,10 @@ fn sorted<'a, I: Iterator>( pub fn generate_keycodes<'a, C: IntoIterator>( key_names: C ) -> HashMap { + let special_keysyms = ["BackSpace", "Return"].iter().map(|&s| s); HashMap::from_iter( // sort to remove a source of indeterminism in keycode assignment - sorted(key_names.into_iter()) + sorted(key_names.into_iter().chain(special_keysyms)) .map(|name| String::from(name)) .zip(9..) ) @@ -108,7 +109,10 @@ impl From for FormattingError { } } -/// Generates a de-facto single level keymap. TODO: actually drop second level +/// Generates a de-facto single level keymap. +// TODO: don't rely on keys and their order, +// but rather on what keysyms and keycodes are in use. +// Iterating actions makes it hard to deduplicate keysyms. pub fn generate_keymap( keystates: &HashMap:: ) -> Result { @@ -123,17 +127,32 @@ pub fn generate_keymap( )?; for (name, state) in keystates.iter() { - if let Action::Submit { text: _, keys } = &state.action { - if let 0 = keys.len() { eprintln!("Key {} has no keysyms", name); }; - for (named_keysym, keycode) in keys.iter().zip(&state.keycodes) { + match &state.action { + Action::Submit { text: _, keys } => { + if let 0 = keys.len() { eprintln!("Key {} has no keysyms", name); }; + for (named_keysym, keycode) in keys.iter().zip(&state.keycodes) { + write!( + buf, + " + <{}> = {};", + named_keysym.0, + keycode, + )?; + } + }, + Action::Erase => { + let mut keycodes = state.keycodes.iter(); write!( buf, " - <{}> = {};", - named_keysym.0, - keycode, + = {};", + keycodes.next().expect("Erase key has no keycode"), )?; - } + if let Some(_) = keycodes.next() { + eprintln!("BUG: Erase key has multiple keycodes"); + } + }, + _ => {}, } } @@ -145,7 +164,9 @@ pub fn generate_keymap( xkb_symbols \"squeekboard\" {{ name[Group1] = \"Letters\"; - name[Group2] = \"Numbers/Symbols\";" + name[Group2] = \"Numbers/Symbols\"; + + key {{ [ BackSpace ] }};" )?; for (name, state) in keystates.iter() { diff --git a/src/layout.rs b/src/layout.rs index 8f8b0055..bc3b3a07 100644 --- a/src/layout.rs +++ b/src/layout.rs @@ -887,7 +887,9 @@ mod seat { // process changes match action { - Action::Submit { text: _, keys: _ } => { + Action::Submit { text: _, keys: _ } + | Action::Erase + => { unstick_locks(layout).apply(); submission.handle_release(KeyState::get_id(rckey), time); }, diff --git a/src/submission.rs b/src/submission.rs index 01dd979b..f42f2e03 100644 --- a/src/submission.rs +++ b/src/submission.rs @@ -116,16 +116,18 @@ impl Submission { key: &KeyState, key_id: KeyStateId, time: Timestamp, ) { - let key_string = match &key.action { - Action::Submit { text, keys: _ } => text, + match &key.action { + Action::Submit { text: _, keys: _ } + | Action::Erase + => (), _ => { - eprintln!("BUG: Submitted key with action other than Submit"); + eprintln!("BUG: Submitted key with action other than Submit or Erase"); return; }, }; - let text_was_committed = match (&mut self.imservice, key_string) { - (Some(imservice), Some(text)) => { + let was_committed_as_text = match (&mut self.imservice, &key.action) { + (Some(imservice), Action::Submit { text: Some(text), keys: _ }) => { let submit_result = imservice.commit_string(text) .and_then(|_| imservice.commit()); match submit_result { @@ -133,10 +135,18 @@ impl Submission { Err(imservice::SubmitError::NotActive) => false, } }, + (Some(imservice), Action::Erase) => { + let submit_result = imservice.delete_surrounding_text(1, 0) + .and_then(|_| imservice.commit()); + match submit_result { + Ok(()) => true, + Err(imservice::SubmitError::NotActive) => false, + } + } (_, _) => false, }; - let submit_action = match text_was_committed { + let submit_action = match was_committed_as_text { true => SubmittedAction::IMService, false => { self.virtual_keyboard.switch( From cd252634bd28ff4c910f38b28c4a2c9df4bf191c Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Tue, 28 Jan 2020 12:45:45 +0000 Subject: [PATCH 4/9] logging: Use in merged functions --- src/keyboard.rs | 5 ++++- src/submission.rs | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/keyboard.rs b/src/keyboard.rs index cfb1fa82..46f4bbf9 100644 --- a/src/keyboard.rs +++ b/src/keyboard.rs @@ -155,7 +155,10 @@ pub fn generate_keymap( keycodes.next().expect("Erase key has no keycode"), )?; if let Some(_) = keycodes.next() { - eprintln!("BUG: Erase key has multiple keycodes"); + log_print!( + logging::Level::Bug, + "Erase key has multiple keycodes", + ); } }, _ => {}, diff --git a/src/submission.rs b/src/submission.rs index f42f2e03..7d3e2949 100644 --- a/src/submission.rs +++ b/src/submission.rs @@ -21,6 +21,7 @@ use ::action::Action; use ::imservice; use ::imservice::IMService; use ::keyboard::{ KeyCode, KeyState, KeyStateId, PressType }; +use ::logging; use ::vkeyboard::VirtualKeyboard; /// Gathers stuff defined in C or called by C @@ -121,7 +122,10 @@ impl Submission { | Action::Erase => (), _ => { - eprintln!("BUG: Submitted key with action other than Submit or Erase"); + log_print!( + logging::Level::Bug, + "Submitted key with action other than Submit or Erase", + ); return; }, }; From 97d8dfe4cb918199621c9e581e8f0d10eef2959c Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Sun, 2 Feb 2020 14:45:33 +0000 Subject: [PATCH 5/9] locks: Draw based on current view --- src/drawing.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/drawing.rs b/src/drawing.rs index ead516a7..f5c224a8 100644 --- a/src/drawing.rs +++ b/src/drawing.rs @@ -3,6 +3,7 @@ use cairo; use std::cell::RefCell; +use ::action::Action; use ::keyboard; use ::layout::{ Button, Layout }; use ::layout::c::{ EekGtkKeyboard, Point }; @@ -37,6 +38,7 @@ mod c { ); } + /// Draws all buttons that are not in the base state #[no_mangle] pub extern "C" fn squeek_layout_draw_all_changed( @@ -51,12 +53,19 @@ mod c { for (row_offset, row) in &view.get_rows() { for (x_offset, button) in &row.buttons { let state = RefCell::borrow(&button.state).clone(); - if state.pressed == keyboard::PressType::Pressed || state.locked { + let locked = match state.action { + Action::SetView(name) => name == layout.current_view, + Action::LockView { lock, unlock: _ } => { + lock == layout.current_view + }, + _ => false, + }; + if state.pressed == keyboard::PressType::Pressed || locked { render_button_at_position( renderer, &cr, row_offset + Point { x: *x_offset, y: 0.0 }, button.as_ref(), - state.pressed, state.locked, + state.pressed, locked, ); } } From 500c23beec7b7d308e2850575976b3affe70fe03 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Sun, 2 Feb 2020 15:41:47 +0000 Subject: [PATCH 6/9] locking: Lock keys statelessly Locking is not determined by button state any more, but rather based on the view active at the moment. If pressing/locking a key results in the current view being active, the key is active. If locking a key results in the current view, the unlock view is activated. --- src/action.rs | 16 ++++++++++++++++ src/data.rs | 1 - src/drawing.rs | 9 +-------- src/keyboard.rs | 13 ------------- src/layout.rs | 42 +++++++++++++++++++++++------------------- 5 files changed, 40 insertions(+), 41 deletions(-) diff --git a/src/action.rs b/src/action.rs index 97e32dc7..af24e237 100644 --- a/src/action.rs +++ b/src/action.rs @@ -38,3 +38,19 @@ pub enum Action { }, ShowPreferences, } + +impl Action { + pub fn is_locked(&self, view_name: &str) -> bool { + match self { + Action::LockView { lock, unlock: _ } => lock == view_name, + _ => false, + } + } + pub fn is_active(&self, view_name: &str) -> bool { + match self { + Action::SetView(view) => view == view_name, + Action::LockView { lock, unlock: _ } => lock == view_name, + _ => false, + } + } +} diff --git a/src/data.rs b/src/data.rs index 5b2c4c9d..4df61335 100644 --- a/src/data.rs +++ b/src/data.rs @@ -392,7 +392,6 @@ impl Layout { name.into(), KeyState { pressed: PressType::Released, - locked: false, keycodes, action, } diff --git a/src/drawing.rs b/src/drawing.rs index f5c224a8..95bf8123 100644 --- a/src/drawing.rs +++ b/src/drawing.rs @@ -3,7 +3,6 @@ use cairo; use std::cell::RefCell; -use ::action::Action; use ::keyboard; use ::layout::{ Button, Layout }; use ::layout::c::{ EekGtkKeyboard, Point }; @@ -53,13 +52,7 @@ mod c { for (row_offset, row) in &view.get_rows() { for (x_offset, button) in &row.buttons { let state = RefCell::borrow(&button.state).clone(); - let locked = match state.action { - Action::SetView(name) => name == layout.current_view, - Action::LockView { lock, unlock: _ } => { - lock == layout.current_view - }, - _ => false, - }; + let locked = state.action.is_active(&layout.current_view); if state.pressed == keyboard::PressType::Pressed || locked { render_button_at_position( renderer, &cr, diff --git a/src/keyboard.rs b/src/keyboard.rs index bff1ac16..9c7ec673 100644 --- a/src/keyboard.rs +++ b/src/keyboard.rs @@ -23,7 +23,6 @@ pub type KeyCode = u32; #[derive(Debug, Clone)] pub struct KeyState { pub pressed: PressType, - pub locked: bool, /// A cache of raw keycodes derived from Action::Sumbit given a keymap pub keycodes: Vec, /// Static description of what the key does when pressed or released @@ -31,17 +30,6 @@ pub struct KeyState { } impl KeyState { - #[must_use] - pub fn into_activated(self) -> KeyState { - match self.action { - Action::LockView { lock: _, unlock: _ } => KeyState { - locked: self.locked ^ true, - ..self - }, - _ => self, - } - } - #[must_use] pub fn into_released(self) -> KeyState { KeyState { @@ -195,7 +183,6 @@ mod tests { keys: vec!(KeySym("a".into()), KeySym("c".into())), }, keycodes: vec!(9, 10), - locked: false, pressed: PressType::Released, }, }).unwrap(); diff --git a/src/layout.rs b/src/layout.rs index 671e774a..d1ef6361 100644 --- a/src/layout.rs +++ b/src/layout.rs @@ -619,7 +619,6 @@ pub struct Layout { // When the list tracks actual location, // it becomes possible to place popovers and other UI accurately. pub pressed_keys: HashSet<::util::Pointer>>, - pub locked_keys: HashSet<::util::Pointer>>, } /// A builder structure for picking up layout data from storage @@ -650,7 +649,6 @@ impl Layout { views: data.views, keymap_str: data.keymap_str, pressed_keys: HashSet::new(), - locked_keys: HashSet::new(), margins: data.margins, } } @@ -714,6 +712,23 @@ impl Layout { scale: 1.0, }) } + + pub fn get_locked_keys(&self) -> Vec>> { + let mut out = Vec::new(); + let view = self.get_current_view(); + for (_, row) in &view.get_rows() { + for (_, button) in &row.buttons { + let locked = { + let state = RefCell::borrow(&button.state).clone(); + state.action.is_locked(&self.current_view) + }; + if locked { + out.push(button.state.clone()); + } + } + } + out + } } mod procedures { @@ -838,12 +853,13 @@ mod seat { // This is guaranteed because pressing a lock button unlocks all others. // TODO: Make some broader guarantee about the resulting view, // e.g. by maintaining a stack of stuck keys. + // FIXME: This doesn't record changes in layout.locked_keys #[must_use] fn unstick_locks(layout: &mut Layout) -> ViewChange { let mut new_view = None; - for key in layout.locked_keys.clone() { + for key in layout.get_locked_keys().clone() { let key: &Rc> = key.borrow(); - let mut key = RefCell::borrow_mut(key); + let key = RefCell::borrow(key); match &key.action { Action::LockView { lock: _, unlock: view } => { new_view = Some(view.clone()); @@ -854,7 +870,6 @@ mod seat { a, ), }; - key.locked = false; } ViewChange { @@ -899,10 +914,7 @@ mod seat { // update let key = key.into_released(); - let key = match action { - Action::LockView { lock: _, unlock: _ } => key.into_activated(), - _ => key, - }; + let mut locked = key.action.is_locked(&layout.current_view); // process changes match action { @@ -918,13 +930,12 @@ mod seat { try_set_view(layout, view) }, Action::LockView { lock, unlock } => { - // The button that triggered this will be in the right state - // due to commit at the end. + locked ^= true; unstick_locks(layout) // It doesn't matter what the resulting view should be, // it's getting changed anyway. .choose_view( - match key.locked { + match locked { true => lock.clone(), false => unlock.clone(), } @@ -966,11 +977,6 @@ mod seat { let pointer = ::util::Pointer(rckey.clone()); // Apply state changes layout.pressed_keys.remove(&pointer); - if key.locked { - layout.locked_keys.insert(pointer); - } else { - layout.locked_keys.remove(&pointer); - } // Commit activated button state changes RefCell::replace(rckey, key); } @@ -985,7 +991,6 @@ mod test { pub fn make_state() -> Rc> { Rc::new(RefCell::new(::keyboard::KeyState { pressed: PressType::Released, - locked: false, keycodes: Vec::new(), action: Action::SetView("default".into()), })) @@ -1064,7 +1069,6 @@ mod test { current_view: String::new(), keymap_str: CString::new("").unwrap(), kind: ArrangementKind::Base, - locked_keys: HashSet::new(), pressed_keys: HashSet::new(), // Lots of bottom margin margins: Margins { From 1cbc21ad1101813ea7028a3679e77c141c36c306 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Mon, 3 Feb 2020 14:46:49 +0000 Subject: [PATCH 7/9] variant: Fix double-free gio::Settings::set_value takes over ownership of the Variant sometimes, but in other cases it doesn't. To prevent this being a problem, the custom Variant is made of the type that will never have its ownership taken. This is not necessarily consistent with what gtk-rs authors intended. In practice, the ownership is shared by refcounting, and after the Rust reference is dropped, one taken by Settings survives. --- src/popover.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/popover.rs b/src/popover.rs index 5f0f735f..ce1543ba 100644 --- a/src/popover.rs +++ b/src/popover.rs @@ -91,6 +91,11 @@ 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 + // 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) } } @@ -141,7 +146,7 @@ fn set_layout(kind: String, name: String) { .chain(inputs).collect(); settings.set_value( "sources", - &variants::ArrayPairString(inputs).to_variant() + &variants::ArrayPairString(inputs).to_variant(), ); settings.apply(); } From 2e9b8581e70a11e4f3936703f53d3cc36f1cea66 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Mon, 3 Feb 2020 14:49:54 +0000 Subject: [PATCH 8/9] variant: Fix leak --- eekboard/eekboard-context-service.c | 1 + 1 file changed, 1 insertion(+) diff --git a/eekboard/eekboard-context-service.c b/eekboard/eekboard-context-service.c index f74382be..c636c430 100644 --- a/eekboard/eekboard-context-service.c +++ b/eekboard/eekboard-context-service.c @@ -116,6 +116,7 @@ settings_get_layout(GSettings *settings, char **type, char **layout) GVariant *inputs = g_settings_get_value(settings, "sources"); // current layout is always first g_variant_get_child(inputs, 0, "(ss)", type, layout); + g_variant_unref(inputs); } void From b77051142268fde786110d5cb74c908a097e5b24 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Mon, 3 Feb 2020 14:59:14 +0000 Subject: [PATCH 9/9] keyboard_layout: Fix leak --- eekboard/eekboard-context-service.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/eekboard/eekboard-context-service.c b/eekboard/eekboard-context-service.c index c636c430..cf5e7e4d 100644 --- a/eekboard/eekboard-context-service.c +++ b/eekboard/eekboard-context-service.c @@ -140,9 +140,11 @@ eekboard_context_service_update_layout(EekboardContextService *context, enum squ switch (priv->purpose) { case ZWP_TEXT_INPUT_V3_CONTENT_PURPOSE_NUMBER: case ZWP_TEXT_INPUT_V3_CONTENT_PURPOSE_PHONE: + g_free(keyboard_layout); keyboard_layout = g_strdup("number"); break; case ZWP_TEXT_INPUT_V3_CONTENT_PURPOSE_TERMINAL: + g_free(keyboard_layout); keyboard_layout = g_strdup("terminal"); break; default: