Merge pull request #150 from Kethku/structured-settings

Structured settings

src/bridge/mod.rs

@@ -15,10 +15,10 @@ use log::{info, error, trace};
 
 pub use events::*;
 pub use keybindings::*;
+use crate::settings::*;
 pub use ui_commands::UiCommand;
 use handler::NeovimHandler;
 use crate::error_handling::ResultPanicExplanation;
-use crate::settings::SETTINGS;
 use crate::INITIAL_DIMENSIONS;
 
 

src/main.rs

@@ -1,12 +1,15 @@
 #![cfg_attr(not(debug_assertions), windows_subsystem = "windows")]
 
+#[macro_use]
+mod settings;
+
 mod bridge;
 mod editor;
 mod window;
 mod renderer;
 mod error_handling;
 mod redraw_scheduler;
-mod settings;
+
 
 #[macro_use] extern crate derive_new;
 #[macro_use] extern crate rust_embed;
@@ -20,6 +23,9 @@ use window::ui_loop;
 pub const INITIAL_DIMENSIONS: (u64, u64) = (100, 50);
 
 fn main() {
+    window::initialize_settings();
+    redraw_scheduler::initialize_settings();
+
     initialize(&BRIDGE);
     ui_loop();
 }

src/redraw_scheduler.rs

@@ -4,12 +4,32 @@ use std::time::Instant;
 
 use log::trace;
 
-use crate::settings::SETTINGS;
+use crate::settings::*;
 
 lazy_static! {
     pub static ref REDRAW_SCHEDULER: RedrawScheduler = RedrawScheduler::new();
 }
 
+#[derive(Clone)]
+struct RedrawSettings {
+    extra_buffer_frames: u64,
+}
+
+pub fn initialize_settings() {
+    
+    let buffer_frames = if SETTINGS.neovim_arguments.contains(&String::from("--extraBufferFrames")) {
+        60
+    }else{
+        1
+    };
+
+    SETTINGS.set(&RedrawSettings {
+        extra_buffer_frames: buffer_frames,
+    });
+
+    register_nvim_setting!("extra_buffer_frames", RedrawSettings::extra_buffer_frames);
+}
+
 pub struct RedrawScheduler {
     frames_queued: AtomicU16,
     scheduled_frame: Mutex<Option<Instant>>
@@ -37,8 +57,8 @@ impl RedrawScheduler {
 
     pub fn queue_next_frame(&self) {
         trace!("Next frame queued");
-        let buffer_frames = SETTINGS.get("extra_buffer_frames").read_u16();
-        self.frames_queued.store(buffer_frames, Ordering::Relaxed);
+        let buffer_frames = SETTINGS.get::<RedrawSettings>().extra_buffer_frames;
+        self.frames_queued.store(buffer_frames as u16, Ordering::Relaxed);
     }
 
     pub fn should_draw(&self) -> bool {

src/settings.rs

@@ -1,13 +1,14 @@
 use std::collections::HashMap;
 use std::convert::TryInto;
+use std::any::{Any, TypeId};
 
-use rmpv::Value;
+pub use rmpv::Value;
 use nvim_rs::Neovim;
 use nvim_rs::compat::tokio::Compat;
 use flexi_logger::{Logger, Criterion, Naming, Cleanup};
 use tokio::process::ChildStdin;
-use parking_lot::Mutex;
-use log::warn;
+use parking_lot::RwLock;
+use log::{error,warn};
 
 use crate::error_handling::ResultPanicExplanation;
 
@@ -15,120 +16,179 @@ lazy_static! {
     pub static ref SETTINGS: Settings = Settings::new();
 }
 
-#[derive(Debug)]
-pub enum Setting {
-    Bool(bool),
-    U16(u16),
-    String(String)
+// Trait to allow for conversion from rmpv::Value to any other data type.
+// Note: Feel free to implement this trait for custom types in each subsystem.
+// The reverse conversion (MyType->Value) can be performed by implementing `From<MyType> for Value`
+pub trait FromValue {
+    fn from_value(&mut self, value: Value);
 }
 
-impl Setting {
-    fn new_bool(value: bool) -> Setting {
-        Setting::Bool(value)
+// FromValue implementations for most typical types
+impl FromValue for f32 {
+    fn from_value(&mut self, value: Value) {
+        if value.is_f32() {
+            *self = value.as_f64().unwrap() as f32; 
+        }else{
+            error!("Setting expected an f32, but received {:?}", value);
+        }
+
     }
+}
 
-    pub fn read_bool(&self) -> bool {
-        if let Setting::Bool(value) = self {
-            *value
-        } else {
-            panic!("Could not read setting as bool");
+impl FromValue for u64 {
+    fn from_value(&mut self, value: Value) {
+        if value.is_u64() {
+            *self = value.as_u64().unwrap(); 
+        }else{
+            error!("Setting expected a u64, but received {:?}", value);
         }
-    }
 
-    fn new_u16(value: u16) -> Setting {
-        Setting::U16(value)
     }
+}
 
-    pub fn read_u16(&self) -> u16 {
-        if let Setting::U16(value) = self {
-            *value
-        } else {
-            panic!("Could not read setting as u16");
+impl FromValue for u32 {
+    fn from_value(&mut self, value: Value) {
+        if value.is_u64() {
+            *self = value.as_u64().unwrap() as u32; 
+        }else{
+            error!("Setting expected a u32, but received {:?}", value);
         }
     }
+}
 
-    fn new_string(value: String) -> Setting {
-        Setting::String(value)
+impl FromValue for i32 {
+    fn from_value(&mut self, value: Value) {
+        if value.is_i64() {
+            *self = value.as_i64().unwrap() as i32; 
+        }else{
+            error!("Setting expected an i32, but received {:?}", value);
+        }
     }
+}
 
-    pub fn read_string(&self) -> String {
-        if let Setting::String(value) = self {
-            value.clone()
-        } else {
-            panic!("Could not read setting as string");
+impl FromValue for String {
+    fn from_value(&mut self, value: Value) {
+        if value.is_str() {
+            *self = String::from(value.as_str().unwrap());
+        }else{
+            error!("Setting expected a string, but received {:?}", value);
         }
+
     }
+}
 
-    fn parse(&mut self, value: Value) {
-        match self {
-            Setting::Bool(internal_bool) => {
-                if let Ok(value) = value.try_into() {
-                    let intermediate: u64 = value;
-                    *internal_bool = intermediate != 0;
-                }
-            },
-            Setting::U16(internal_u16) => {
-                if let Ok(value) = value.try_into() {
-                    let intermediate: u64 = value;
-                    *internal_u16 = intermediate as u16;
-                }
-            },
-            Setting::String(internal_string) => {
-                if let Ok(value) = value.try_into() {
-                    let intermediate: String = value;
-                    *internal_string = intermediate;
-                }
-            }
+impl FromValue for bool {
+    fn from_value(&mut self, value: Value) {
+        if value.is_bool() {
+            *self = value.as_bool().unwrap();
+        }else{
+            error!("Setting expected a string, but received {:?}", value);
         }
     }
+}
 
-    fn unparse(&self) -> Value {
-        match self {
-            Setting::Bool(internal_bool) => {
-                let value = if *internal_bool {
-                    1
-                } else {
-                    0
-                };
-                Value::from(value)
-            },
-            Setting::U16(internal_u16) => Value::from(*internal_u16),
-            Setting::String(internal_string) => Value::from(internal_string.as_str()),
+// Macro to register settings changed handlers.
+// Note: Invocations to this macro must happen before the call to Settings::read_initial_values.
+#[macro_export]
+macro_rules! register_nvim_setting {
+    ($vim_setting_name: expr, $type_name:ident :: $field_name: ident) => {{
+        // The update func sets a new value for a setting
+        fn update_func(value: Value) {
+            let mut s = SETTINGS.get::<$type_name>();
+            s.$field_name.from_value(value);
+            SETTINGS.set(&s);
         }
-    }
 
-    fn clone(&self) -> Setting {
-        match self {
-            Setting::Bool(_) => Setting::new_bool(self.read_bool()),
-            Setting::U16(_) => Setting::new_u16(self.read_u16()),
-            Setting::String(_) => Setting::new_string(self.read_string()),
+        // The reader func retrieves the current value for a setting
+        fn reader_func() -> Value {
+            let s = SETTINGS.get::<$type_name>();
+            s.$field_name.into()
         }
-    }
+
+        SETTINGS.set_setting_handlers($vim_setting_name, update_func, reader_func);
+    }};
 }
 
+// Function types to handle settings updates
+type UpdateHandlerFunc = fn(Value);
+type ReaderFunc = fn()->Value;
+
+
+// The Settings struct acts as a global container where each of Neovide's subsystems can store
+// their own settings. It will also coordinate updates between Neovide and nvim to make sure the
+// settings remain consistent on both sides.
+// Note: As right now we're only sending new setting values to Neovide during the
+// read_initial_values call, after that point we should not modify the contents of the Settings
+// struct except when prompted by an update event from nvim. Otherwise, the settings in Neovide and
+// nvim will get out of sync.
 pub struct Settings {
     pub neovim_arguments: Vec<String>,
-    pub settings: Mutex<HashMap<String, Setting>>
+    settings: RwLock<HashMap<TypeId, Box<dyn Any + Send + Sync>>>,
+    listeners: RwLock<HashMap<String, UpdateHandlerFunc>>,
+    readers: RwLock<HashMap<String, ReaderFunc>>,
 }
 
 impl Settings {
+
+    fn new() -> Settings {
+        let neovim_arguments = std::env::args().filter(|arg| {
+            if arg == "--log" {
+                Logger::with_str("neovide")
+                    .log_to_file()
+                    .rotate(Criterion::Size(10_000_000), Naming::Timestamps, Cleanup::KeepLogFiles(1))
+                    .start()
+                    .expect("Could not start logger");
+                false
+            } else {
+                true
+            }
+        }).collect::<Vec<String>>();
+
+        Settings{
+            neovim_arguments,
+            settings: RwLock::new(HashMap::new()),
+            listeners: RwLock::new(HashMap::new()),
+            readers: RwLock::new(HashMap::new()),
+        }
+    }
+
+    pub fn set_setting_handlers(&self, property_name: &str, update_func: UpdateHandlerFunc, reader_func: ReaderFunc) {
+        self.listeners.write().insert(String::from(property_name), update_func);
+        self.readers.write().insert(String::from(property_name), reader_func);
+    }
+    
+    pub fn set<T: Clone + Send + Sync + 'static >(&self, t: &T) {
+        let type_id : TypeId = TypeId::of::<T>();
+        let t : T = (*t).clone();
+        self.settings.write().insert(type_id, Box::new(t));
+    }
+
+    pub fn get<'a, T: Clone + Send + Sync + 'static>(&'a self) -> T {
+        let read_lock = self.settings.read();
+        let boxed = &read_lock.get(&TypeId::of::<T>()).expect("Trying to retrieve a settings object that doesn't exist");
+        let value: &T = boxed.downcast_ref::<T>().expect("Attempted to extract a settings object of the wrong type");
+        (*value).clone()
+    }
+
     pub async fn read_initial_values(&self, nvim: &Neovim<Compat<ChildStdin>>) {
-        let keys : Vec<String> = self.settings.lock().keys().cloned().collect();
+        let keys : Vec<String> = self.listeners.read().keys().cloned().collect();
         for name in keys {
             let variable_name = format!("neovide_{}", name.to_string());
             match nvim.get_var(&variable_name).await {
-                Ok(value) => self.settings.lock().get_mut(&name).unwrap().parse(value),
+                Ok(value) => {
+                    self.listeners.read().get(&name).unwrap()(value);
+                },
                 Err(error) => {
                     warn!("Initial value load failed for {}: {}", name, error);
-                    let setting = self.get(&name);
-                    nvim.set_var(&variable_name, setting.unparse()).await.ok();
+                    let setting = self.readers.read().get(&name).unwrap()();
+                    nvim.set_var(&variable_name, setting).await.ok();
                 }
             }
         }
     }
 
     pub async fn setup_changed_listeners(&self, nvim: &Neovim<Compat<ChildStdin>>) {
-        let keys : Vec<String> = self.settings.lock().keys().cloned().collect();
+        let keys : Vec<String> = self.listeners.read().keys().cloned().collect();
         for name in keys {
             let vimscript = format!(
                 concat!(
@@ -151,44 +211,6 @@ impl Settings {
         let name: Result<String, _>= name.try_into();
         let name = name.unwrap();
 
-        self.settings.lock().get_mut(&name).unwrap().parse(value);
-    }
-
-    pub fn get(&self, name: &str) -> Setting {
-        let settings = self.settings.lock();
-        let setting = settings.get(name).expect(&format!("Could not find option {}", name));
-        setting.clone()
-    }
-
-    pub fn new() -> Settings {
-        let mut no_idle = false;
-        let mut buffer_frames = 1;
-
-        let neovim_arguments = std::env::args().filter(|arg| {
-            if arg == "--log" {
-                Logger::with_str("neovide")
-                    .log_to_file()
-                    .rotate(Criterion::Size(10_000_000), Naming::Timestamps, Cleanup::KeepLogFiles(1))
-                    .start()
-                    .expect("Could not start logger");
-                false
-            } else if arg == "--noIdle" {
-                no_idle = true;
-                false
-            } else if arg == "--extraBufferFrames" {
-                buffer_frames = 60;
-                false
-            } else {
-                true
-            }
-        }).collect::<Vec<String>>();
-
-        let mut settings = HashMap::new();
-
-        settings.insert("no_idle".to_string(),  Setting::new_bool(no_idle));
-        settings.insert("extra_buffer_frames".to_string(), Setting::new_u16(buffer_frames));
-        settings.insert("refresh_rate".to_string(), Setting::new_u16(60));
-
-        Settings { neovim_arguments, settings: Mutex::new(settings) }
+        self.listeners.read().get(&name).unwrap()(value);
     }
 }

src/window.rs

@@ -10,11 +10,11 @@ use skulpin::sdl2::event::Event;
 use skulpin::sdl2::keyboard::{Mod, Keycode};
 use skulpin::{RendererBuilder, Renderer as SkulpinRenderer, PresentMode, CoordinateSystem, dpis};
 
+use crate::settings::*;
 use crate::bridge::{parse_keycode, append_modifiers, BRIDGE, UiCommand};
 use crate::renderer::Renderer;
 use crate::redraw_scheduler::REDRAW_SCHEDULER;
 use crate::editor::EDITOR;
-use crate::settings::SETTINGS;
 use crate::INITIAL_DIMENSIONS;
 
 #[derive(RustEmbed)]
@@ -236,7 +236,7 @@ impl WindowWrapper {
         }
 
         debug!("Render Triggered");
-        if REDRAW_SCHEDULER.should_draw() || SETTINGS.get("no_idle").read_bool() {
+        if REDRAW_SCHEDULER.should_draw() || SETTINGS.get::<WindowSettings>().no_idle {
             let renderer = &mut self.renderer;
             if self.skulpin_renderer.draw(&self.window, |canvas, coordinate_system_helper| {
                 if renderer.draw(canvas, coordinate_system_helper) {
@@ -251,6 +251,25 @@ impl WindowWrapper {
     }
 }
 
+#[derive(Clone)]
+struct WindowSettings {
+    refresh_rate: u64,
+    no_idle: bool,
+}
+
+pub fn initialize_settings() {
+    
+    let no_idle = SETTINGS.neovim_arguments.contains(&String::from("--noIdle"));
+
+    SETTINGS.set(&WindowSettings {
+        refresh_rate: 60,
+        no_idle,
+    });
+    
+    register_nvim_setting!("refresh_rate", WindowSettings::refresh_rate);
+    register_nvim_setting!("no_idle", WindowSettings::no_idle);
+}
+
 pub fn ui_loop() {
     let mut window = WindowWrapper::new();
 
@@ -280,7 +299,7 @@ pub fn ui_loop() {
         }
 
         let elapsed = frame_start.elapsed();
-        let refresh_rate = SETTINGS.get("refresh_rate").read_u16() as f32;
+        let refresh_rate = SETTINGS.get::<WindowSettings>().refresh_rate as f32;
         let frame_length = Duration::from_secs_f32(1.0 / refresh_rate);
         if elapsed < frame_length {
             sleep(frame_length - elapsed);