From a93f3c55e7adc0092a5210aa20cdc78a6e7a93fe Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Mon, 9 Dec 2019 13:38:38 +0000 Subject: [PATCH 1/2] translations: Use gnome-desktop's xkb info database for layout names --- debian/control | 1 + src/locale.rs | 75 +++++++++++++++++++++++++++++++++++++++++++++++-- src/meson.build | 1 + src/popover.rs | 63 ++++++++++++++++++++++++++++++++--------- 4 files changed, 124 insertions(+), 16 deletions(-) diff --git a/debian/control b/debian/control index 22e3cc5c..02a477af 100644 --- a/debian/control +++ b/debian/control @@ -9,6 +9,7 @@ Build-Depends: ninja-build, pkg-config, libglib2.0-dev, + libgnome-desktop-3-dev, libgtk-3-dev, libcroco3-dev, librust-bitflags-1-dev (>= 1.0), diff --git a/src/locale.rs b/src/locale.rs index 4368fc15..de5df977 100644 --- a/src/locale.rs +++ b/src/locale.rs @@ -1,24 +1,93 @@ -/*! Locale-specific functions */ +/*! Locale-specific functions. */ use std::cmp; -use std::ffi::CString; +use std::ffi::{ CStr, CString }; +use std::os::raw::c_char; +use std::ptr; +use std::str::Utf8Error; mod c { - use std::os::raw::c_char; + use super::*; + use std::os::raw::c_void; #[allow(non_camel_case_types)] pub type c_int = i32; + #[derive(Clone, Copy)] + #[repr(C)] + pub struct GnomeXkbInfo(*const c_void); + #[no_mangle] extern "C" { // from libc pub fn strcoll(cs: *const c_char, ct: *const c_char) -> c_int; + // from gnome-desktop3 + pub fn gnome_xkb_info_new() -> GnomeXkbInfo; + pub fn gnome_xkb_info_get_layout_info ( + info: GnomeXkbInfo, + id: *const c_char, + display_name: *mut *const c_char, + short_name: *const *const c_char, + xkb_layout: *const *const c_char, + xkb_variant: *const *const c_char + ) -> c_int; + pub fn g_object_unref(o: GnomeXkbInfo); + } +} + +#[derive(Debug)] +pub enum Error { + StringConversion(Utf8Error), + NoInfo, +} + +pub struct XkbInfo(c::GnomeXkbInfo); + +impl XkbInfo { + pub fn new() -> XkbInfo { + XkbInfo(unsafe { c::gnome_xkb_info_new() }) + } + pub fn get_display_name(&self, id: &str) -> Result { + let id = cstring_safe(id); + let id_ref = id.as_ptr(); + let mut display_name: *const c_char = ptr::null(); + let found = unsafe { + c::gnome_xkb_info_get_layout_info( + self.0, + id_ref, + &mut display_name as *mut *const c_char, + ptr::null(), ptr::null(), ptr::null(), + ) + }; + if found != 0 && !display_name.is_null() { + let display_name = unsafe { CStr::from_ptr(display_name) }; + display_name.to_str() + .map(str::to_string) + .map_err(Error::StringConversion) + } else { + Err(Error::NoInfo) + } + } +} + +impl Drop for XkbInfo { + fn drop(&mut self) { + unsafe { c::g_object_unref(self.0) } } } #[derive(Clone, Debug, PartialEq)] pub struct Translation<'a>(pub &'a str); +impl<'a> Translation<'a> { + pub fn to_owned(&'a self) -> OwnedTranslation { + OwnedTranslation(self.0.to_owned()) + } +} + +#[derive(Clone, Debug, PartialEq)] +pub struct OwnedTranslation(pub String); + fn cstring_safe(s: &str) -> CString { CString::new(s) .unwrap_or(CString::new("").unwrap()) diff --git a/src/meson.build b/src/meson.build index e3a90935..9763f647 100644 --- a/src/meson.build +++ b/src/meson.build @@ -39,6 +39,7 @@ cc = meson.get_compiler('c') deps = [ # dependency('glib-2.0', version: '>=2.26.0'), dependency('gio-2.0', version: '>=2.26.0'), + dependency('gnome-desktop-3.0', version: '>=3.0'), dependency('gtk+-3.0', version: '>=3.0'), dependency('libcroco-0.6'), dependency('wayland-client', version: '>=1.14'), diff --git a/src/popover.rs b/src/popover.rs index 2319fc6d..ee99c1c8 100644 --- a/src/popover.rs +++ b/src/popover.rs @@ -4,7 +4,8 @@ use gio; use gtk; use std::ffi::CString; use ::layout::c::{ Bounds, EekGtkKeyboard }; -use ::locale::{ Translation, compare_current_locale }; +use ::locale; +use ::locale::{ OwnedTranslation, Translation, compare_current_locale }; use ::locale_config::system_locale; use ::manager; use ::resources; @@ -94,7 +95,7 @@ mod variants { } } -fn make_menu_builder(inputs: Vec<(&str, Translation)>) -> gtk::Builder { +fn make_menu_builder(inputs: Vec<(&str, OwnedTranslation)>) -> gtk::Builder { let mut xml: Vec = Vec::new(); writeln!( xml, @@ -227,37 +228,73 @@ pub fn show( ) .and_then(|lang| resources::get_layout_names(lang.as_str())); + // The actual translation procedure attempts to take all xkb names + // from gnome-desktop's xkb info. + // Remaining names are translated using the internal database, + // which is only available if the locale is set. + // The result is a rather ugly and verbose translation procedure... + enum Status { + /// xkb names should get all translated here + Translated(OwnedTranslation), + /// Builtin names need builtin translations + Remaining(String), + } + + let xkb_translator = locale::XkbInfo::new(); + let translated_names = all_layouts.iter() - .map(LayoutId::get_name); - let translated_names: Vec = match translations { + .map(|id| match id { + LayoutId::System { name, kind: _ } => { + xkb_translator.get_display_name(name) + .map(|s| Status::Translated(OwnedTranslation(s))) + .unwrap_or_else(|e| { + eprintln!( + "No display name for xkb layout {}: {:?}", + name, + e, + ); + Status::Remaining(name.clone()) + }) + }, + LayoutId::Local(name) => Status::Remaining(name.clone()), + }); + + let translated_names: Vec = match translations { Some(translations) => { translated_names - .map(move |name| { - translations.get(name) - .map(|translation| translation.clone()) - .unwrap_or(Translation(name)) + .map(|status| match status { + Status::Remaining(name) => { + translations.get(name.as_str()) + .unwrap_or(&Translation(name.as_str())) + .to_owned() + }, + Status::Translated(t) => t, }) .collect() }, None => { - translated_names.map(|name| Translation(name)) + translated_names + .map(|status| match status { + Status::Remaining(name) => OwnedTranslation(name), + Status::Translated(t) => t, + }) .collect() }, }; // sorted collection of human and machine names - let mut human_names: Vec<(Translation, LayoutId)> = translated_names + let mut human_names: Vec<(OwnedTranslation, LayoutId)> = translated_names .into_iter() .zip(all_layouts.clone().into_iter()) .collect(); human_names.sort_unstable_by(|(tr_a, _), (tr_b, _)| { - compare_current_locale(tr_a.0, tr_b.0) + compare_current_locale(&tr_a.0, &tr_b.0) }); // GVariant doesn't natively support `enum`s, // so the `choices` vector will serve as a lookup table. - let choices_with_translations: Vec<(String, (Translation, LayoutId))> + let choices_with_translations: Vec<(String, (OwnedTranslation, LayoutId))> = human_names.into_iter() .enumerate() .map(|(i, human_entry)| {( @@ -268,7 +305,7 @@ pub fn show( let builder = make_menu_builder( choices_with_translations.iter() - .map(|(id, (translation, _))| (id.as_str(), translation.clone())) + .map(|(id, (translation, _))| (id.as_str(), (*translation).clone())) .collect() ); From 0bfd846139edd895ccdc3e5daa07acf5cd4a2ce9 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Wed, 11 Dec 2019 15:20:29 +0000 Subject: [PATCH 2/2] translations: Make the code cleaner --- src/locale.rs | 8 ++- src/logging.rs | 14 ++++- src/popover.rs | 140 +++++++++++++++++++++++++++---------------------- src/style.rs | 9 ++-- src/util.rs | 5 ++ 5 files changed, 108 insertions(+), 68 deletions(-) diff --git a/src/locale.rs b/src/locale.rs index de5df977..6b87f68c 100644 --- a/src/locale.rs +++ b/src/locale.rs @@ -1,4 +1,10 @@ -/*! Locale-specific functions. */ +/*! Locale-specific functions. + * + * This file is intended as a library: + * it must pass errors upwards + * and panicking is allowed only when + * this code encounters an internal inconsistency. + */ use std::cmp; use std::ffi::{ CStr, CString }; diff --git a/src/logging.rs b/src/logging.rs index 9848783e..7beb9ff1 100644 --- a/src/logging.rs +++ b/src/logging.rs @@ -70,12 +70,12 @@ pub enum Level { /// Approach 2. pub trait Warn { type Value; - fn ok_warn(self, msg: &str) -> Option; + fn or_warn(self, msg: &str) -> Option; } impl Warn for Result { type Value = T; - fn ok_warn(self, msg: &str) -> Option { + fn or_warn(self, msg: &str) -> Option { self.map_err(|e| { eprintln!("{}: {}", msg, e); e @@ -83,6 +83,16 @@ impl Warn for Result { } } +impl Warn for Option { + type Value = T; + fn or_warn(self, msg: &str) -> Option { + self.or_else(|| { + eprintln!("{}", msg); + None + }) + } +} + /// A mutable handler for text warnings. /// Approach 3. pub trait WarningHandler { diff --git a/src/popover.rs b/src/popover.rs index ee99c1c8..c0b53086 100644 --- a/src/popover.rs +++ b/src/popover.rs @@ -18,6 +18,7 @@ use glib::variant::ToVariant; use gtk::PopoverExt; use gtk::WidgetExt; use std::io::Write; +use ::logging::Warn; mod variants { use glib; @@ -195,6 +196,82 @@ fn get_current_layout( } } +/// Translates all provided layout names according to current locale, +/// for the purpose of display (i.e. errors will be caught and reported) +fn translate_layout_names(layouts: &Vec) -> Vec { + // This procedure is rather ugly... + // Xkb lookup *must not* be applied to non-system layouts, + // so both translators can't be merged into one lookup table, + // therefore must be done in two steps. + // `XkbInfo` being temporary also means + // that its return values must be copied, + // forcing the use of `OwnedTranslation`. + enum Status { + /// xkb names should get all translated here + Translated(OwnedTranslation), + /// Builtin names need builtin translations + Remaining(String), + } + + // Attempt to take all xkb names from gnome-desktop's xkb info. + let xkb_translator = locale::XkbInfo::new(); + + let translated_names = layouts.iter() + .map(|id| match id { + LayoutId::System { name, kind: _ } => { + xkb_translator.get_display_name(name) + .map(|s| Status::Translated(OwnedTranslation(s))) + .unwrap_or_else(|e| { + eprintln!( + "No display name for xkb layout {}: {:?}", + name, + e, + ); + Status::Remaining(name.clone()) + }) + }, + LayoutId::Local(name) => Status::Remaining(name.clone()), + }); + + // Non-xkb layouts and weird xkb layouts + // still need to be looked up in the internal database. + let builtin_translations = system_locale() + .map(|locale| + locale.tags_for("messages") + .next().unwrap() // guaranteed to exist + .as_ref() + .to_owned() + ) + .or_warn("No locale detected") + .and_then(|lang| { + resources::get_layout_names(lang.as_str()) + .or_warn(&format!("No translations for locale {}", lang)) + }); + + match builtin_translations { + Some(translations) => { + translated_names + .map(|status| match status { + Status::Remaining(name) => { + translations.get(name.as_str()) + .unwrap_or(&Translation(name.as_str())) + .to_owned() + }, + Status::Translated(t) => t, + }) + .collect() + }, + None => { + translated_names + .map(|status| match status { + Status::Remaining(name) => OwnedTranslation(name), + Status::Translated(t) => t, + }) + .collect() + }, + } +} + pub fn show( window: EekGtkKeyboard, position: Bounds, @@ -219,69 +296,8 @@ pub fn show( .chain(overlay_layouts) .collect(); - let translations = system_locale() - .map(|locale| - locale.tags_for("messages") - .next().unwrap() // guaranteed to exist - .as_ref() - .to_owned() - ) - .and_then(|lang| resources::get_layout_names(lang.as_str())); - - // The actual translation procedure attempts to take all xkb names - // from gnome-desktop's xkb info. - // Remaining names are translated using the internal database, - // which is only available if the locale is set. - // The result is a rather ugly and verbose translation procedure... - enum Status { - /// xkb names should get all translated here - Translated(OwnedTranslation), - /// Builtin names need builtin translations - Remaining(String), - } + let translated_names = translate_layout_names(&all_layouts); - let xkb_translator = locale::XkbInfo::new(); - - let translated_names = all_layouts.iter() - .map(|id| match id { - LayoutId::System { name, kind: _ } => { - xkb_translator.get_display_name(name) - .map(|s| Status::Translated(OwnedTranslation(s))) - .unwrap_or_else(|e| { - eprintln!( - "No display name for xkb layout {}: {:?}", - name, - e, - ); - Status::Remaining(name.clone()) - }) - }, - LayoutId::Local(name) => Status::Remaining(name.clone()), - }); - - let translated_names: Vec = match translations { - Some(translations) => { - translated_names - .map(|status| match status { - Status::Remaining(name) => { - translations.get(name.as_str()) - .unwrap_or(&Translation(name.as_str())) - .to_owned() - }, - Status::Translated(t) => t, - }) - .collect() - }, - None => { - translated_names - .map(|status| match status { - Status::Remaining(name) => OwnedTranslation(name), - Status::Translated(t) => t, - }) - .collect() - }, - }; - // sorted collection of human and machine names let mut human_names: Vec<(OwnedTranslation, LayoutId)> = translated_names .into_iter() diff --git a/src/style.rs b/src/style.rs index b12e756c..381dd045 100644 --- a/src/style.rs +++ b/src/style.rs @@ -16,7 +16,7 @@ * License along with this library. If not, see .Free */ -/*! CSS data loading */ +/*! CSS data loading. */ use std::env; @@ -83,6 +83,7 @@ fn get_theme_name(settings: >k::Settings) -> GtkTheme { .map_err(|e| { match &e { env::VarError::NotPresent => {}, + // maybe TODO: forward this warning? e => eprintln!("GTK_THEME variable invalid: {}", e), }; e @@ -93,13 +94,15 @@ fn get_theme_name(settings: >k::Settings) -> GtkTheme { None => GtkTheme { name: { settings.get_property("gtk-theme-name") - .ok_warn("No theme name") + // maybe TODO: is this worth a warning? + .or_warn("No theme name") .and_then(|value| value.get::()) .unwrap_or(DEFAULT_THEME_NAME.into()) }, variant: { settings.get_property("gtk-application-prefer-dark-theme") - .ok_warn("No settings key") + // maybe TODO: is this worth a warning? + .or_warn("No settings key") .and_then(|value| value.get::()) .and_then(|dark_preferred| match dark_preferred { true => Some("dark".into()), diff --git a/src/util.rs b/src/util.rs index 55a4373f..d58a3e77 100644 --- a/src/util.rs +++ b/src/util.rs @@ -190,6 +190,11 @@ impl Borrow> for Pointer { } } +pub trait WarningHandler { + /// Handle a warning + fn handle(&mut self, warning: &str); +} + #[cfg(test)] mod tests { use super::*;