From 013beb245bc8ec73f0fdc86e852a24cbbe6de5d3 Mon Sep 17 00:00:00 2001 From: Henrik Friedrichsen Date: Sun, 18 Oct 2020 13:09:45 +0200 Subject: [PATCH] refactor: pass globally mutable config reference Before, copies of the configuration were passed over. This change also causes configuration reloads to affect the entire application and is easier to maintain but introduces some RwLock overhead. --- src/album.rs | 2 +- src/artist.rs | 2 +- src/commands.rs | 17 +++++++++------ src/config.rs | 42 ++++++++++++++++++++++++++++++++----- src/library.rs | 10 +++------ src/main.rs | 28 +++++++++++-------------- src/playlist.rs | 2 +- src/queue.rs | 7 +++++-- src/show.rs | 2 +- src/spotify.rs | 41 ++++++++++++++++++++++-------------- src/theme.rs | 55 ++++++++++++++++++++++++++++--------------------- src/track.rs | 4 ++-- 12 files changed, 132 insertions(+), 80 deletions(-) diff --git a/src/album.rs b/src/album.rs index 4401bb5..10b7323 100644 --- a/src/album.rs +++ b/src/album.rs @@ -182,7 +182,7 @@ impl ListItem for Album { fn display_right(&self, library: Arc) -> String { let saved = if library.is_saved_album(self) { - if library.use_nerdfont { + if library.cfg.values().use_nerdfont.unwrap_or(false) { "\u{f62b} " } else { "✓ " diff --git a/src/artist.rs b/src/artist.rs index c6ccdde..daa6076 100644 --- a/src/artist.rs +++ b/src/artist.rs @@ -154,7 +154,7 @@ impl ListItem for Artist { fn display_right(&self, library: Arc) -> String { let followed = if library.is_followed_artist(self) { - if library.use_nerdfont { + if library.cfg.values().use_nerdfont.unwrap_or(false) { "\u{f62b} " } else { "✓ " diff --git a/src/commands.rs b/src/commands.rs index ce64da9..877f181 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -32,6 +32,7 @@ pub struct CommandManager { spotify: Arc, queue: Arc, library: Arc, + config: Arc, } impl CommandManager { @@ -39,18 +40,21 @@ impl CommandManager { spotify: Arc, queue: Arc, library: Arc, - config: &Config, + config: Arc, ) -> CommandManager { + let bindings = RefCell::new(Self::get_bindings(config.clone())); CommandManager { aliases: HashMap::new(), - bindings: RefCell::new(Self::get_bindings(config)), + bindings, spotify, queue, library, + config, } } - pub fn get_bindings(config: &Config) -> HashMap { + pub fn get_bindings(config: Arc) -> HashMap { + let config = config.values(); let mut kb = if config.default_keybindings.unwrap_or(true) { Self::default_keybindings() } else { @@ -161,15 +165,16 @@ impl CommandManager { Ok(None) } Command::ReloadConfig => { - let cfg = crate::config::load()?; + self.config.reload(); // update theme - let theme = crate::theme::load(&cfg); + let theme = self.config.build_theme(); s.set_theme(theme); // update bindings self.unregister_keybindings(s); - self.bindings.replace(Self::get_bindings(&cfg)); + self.bindings + .replace(Self::get_bindings(self.config.clone())); self.register_keybindings(s); Ok(None) } diff --git a/src/config.rs b/src/config.rs index 6db1036..82bcbee 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,14 +1,15 @@ use std::collections::HashMap; -use std::fs; use std::path::{Path, PathBuf}; -use std::sync::RwLock; +use std::sync::{RwLock, RwLockReadGuard}; +use std::{fs, process}; +use cursive::theme::Theme; use platform_dirs::AppDirs; pub const CLIENT_ID: &str = "d420a117a32841c2b3474932e49fb54b"; #[derive(Clone, Serialize, Deserialize, Debug, Default)] -pub struct Config { +pub struct ConfigValues { pub default_keybindings: Option, pub keybindings: Option>, pub theme: Option, @@ -57,9 +58,40 @@ lazy_static! { pub static ref BASE_PATH: RwLock> = RwLock::new(None); } -pub fn load() -> Result { +pub struct Config { + values: RwLock, +} + +impl Config { + pub fn new() -> Self { + let values = load().unwrap_or_else(|e| { + eprintln!("could not load config: {}", e); + process::exit(1); + }); + + Self { + values: RwLock::new(values), + } + } + + pub fn values(&self) -> RwLockReadGuard { + self.values.read().expect("can't readlock config values") + } + + pub fn build_theme(&self) -> Theme { + let theme = &self.values().theme; + crate::theme::load(theme) + } + + pub fn reload(&self) { + let cfg = load().expect("could not reload config"); + *self.values.write().expect("can't writelock config values") = cfg + } +} + +fn load() -> Result { let path = config_path("config.toml"); - load_or_generate_default(path, |_| Ok(Config::default()), false) + load_or_generate_default(path, |_| Ok(ConfigValues::default()), false) } fn proj_dirs() -> AppDirs { diff --git a/src/library.rs b/src/library.rs index 6f2bd8e..3f17eb4 100644 --- a/src/library.rs +++ b/src/library.rs @@ -36,11 +36,11 @@ pub struct Library { user_id: Option, ev: EventManager, spotify: Arc, - pub use_nerdfont: bool, + pub cfg: Arc, } impl Library { - pub fn new(ev: &EventManager, spotify: Arc, use_nerdfont: bool) -> Self { + pub fn new(ev: &EventManager, spotify: Arc, cfg: Arc) -> Self { let user_id = spotify.current_user().map(|u| u.id); let library = Self { @@ -53,7 +53,7 @@ impl Library { user_id, ev: ev.clone(), spotify, - use_nerdfont, + cfg, }; library.update_library(); @@ -66,10 +66,6 @@ impl Library { .expect("could not readlock listview content") } - pub fn config(&self) -> &Config { - &self.spotify.cfg - } - fn load_cache(&self, cache_path: PathBuf, store: Arc>>) { if let Ok(contents) = std::fs::read_to_string(&cache_path) { debug!("loading cache from {}", cache_path.display()); diff --git a/src/main.rs b/src/main.rs index 4668752..958b2ba 100644 --- a/src/main.rs +++ b/src/main.rs @@ -77,6 +77,7 @@ mod mpris; use crate::command::{Command, JumpMode}; use crate::commands::CommandManager; +use crate::config::Config; use crate::events::{Event, EventManager}; use crate::library::Library; use crate::spotify::PlayerEvent; @@ -189,10 +190,7 @@ fn main() { // Things here may cause the process to abort; we must do them before creating curses windows // otherwise the error message will not be seen by a user - let cfg: crate::config::Config = config::load().unwrap_or_else(|e| { - eprintln!("{}", e); - process::exit(1); - }); + let cfg: Arc = Arc::new(Config::new()); let cache = Cache::new(config::cache_path("librespot"), true); let mut credentials = { @@ -218,9 +216,8 @@ fn main() { credentials = credentials_prompt(reset, Some(error_msg)); } - let theme = theme::load(&cfg); - let mut cursive = Cursive::default(); + let theme = cfg.build_theme(); cursive.set_theme(theme.clone()); let event_manager = EventManager::new(cursive.cb_sink().clone()); @@ -228,22 +225,18 @@ fn main() { let spotify = Arc::new(spotify::Spotify::new( event_manager.clone(), credentials, - &cfg, + cfg.clone(), )); - let queue = Arc::new(queue::Queue::new(spotify.clone())); + let queue = Arc::new(queue::Queue::new(spotify.clone(), cfg.clone())); #[cfg(feature = "mpris")] let mpris_manager = Arc::new(mpris::MprisManager::new(spotify.clone(), queue.clone())); - let library = Arc::new(Library::new( - &event_manager, - spotify.clone(), - cfg.use_nerdfont.unwrap_or(false), - )); + let library = Arc::new(Library::new(&event_manager, spotify.clone(), cfg.clone())); let mut cmd_manager = - CommandManager::new(spotify.clone(), queue.clone(), library.clone(), &cfg); + CommandManager::new(spotify.clone(), queue.clone(), library.clone(), cfg.clone()); cmd_manager.register_all(); cmd_manager.register_keybindings(&mut cursive); @@ -262,8 +255,11 @@ fn main() { let queueview = ui::queue::QueueView::new(queue.clone(), library.clone()); - let status = - ui::statusbar::StatusBar::new(queue.clone(), library, cfg.use_nerdfont.unwrap_or(false)); + let status = ui::statusbar::StatusBar::new( + queue.clone(), + library, + cfg.values().use_nerdfont.unwrap_or(false), + ); let mut layout = ui::layout::Layout::new(status, &event_manager, theme) .view("search", search.with_name("search"), "Search") diff --git a/src/playlist.rs b/src/playlist.rs index b4de90d..714473d 100644 --- a/src/playlist.rs +++ b/src/playlist.rs @@ -134,7 +134,7 @@ impl ListItem for Playlist { fn display_right(&self, library: Arc) -> String { let saved = if library.is_saved_playlist(self) { - if library.use_nerdfont { + if library.cfg.values().use_nerdfont.unwrap_or(false) { "\u{f62b} " } else { "✓ " diff --git a/src/queue.rs b/src/queue.rs index 4137b89..21f0ba9 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -7,6 +7,7 @@ use notify_rust::Notification; use rand::prelude::*; use strum_macros::Display; +use crate::config::Config; use crate::playable::Playable; use crate::spotify::Spotify; @@ -23,16 +24,18 @@ pub struct Queue { current_track: RwLock>, repeat: RwLock, spotify: Arc, + cfg: Arc, } impl Queue { - pub fn new(spotify: Arc) -> Queue { + pub fn new(spotify: Arc, cfg: Arc) -> Queue { let q = Queue { queue: Arc::new(RwLock::new(Vec::new())), spotify, current_track: RwLock::new(None), repeat: RwLock::new(RepeatSetting::None), random_order: RwLock::new(None), + cfg, }; q.set_repeat(q.spotify.repeat); q.set_shuffle(q.spotify.shuffle); @@ -248,7 +251,7 @@ impl Queue { let mut current = self.current_track.write().unwrap(); current.replace(index); self.spotify.update_track(); - if self.spotify.cfg.notify.unwrap_or(false) { + if self.cfg.values().notify.unwrap_or(false) { #[cfg(feature = "notify")] if let Err(e) = Notification::new().summary(&track.to_string()).show() { error!("error showing notification: {:?}", e); diff --git a/src/show.rs b/src/show.rs index e22ac7a..8d14969 100644 --- a/src/show.rs +++ b/src/show.rs @@ -98,7 +98,7 @@ impl ListItem for Show { fn display_right(&self, library: Arc) -> String { let saved = if library.is_saved_show(self) { - if library.use_nerdfont { + if library.cfg.values().use_nerdfont.unwrap_or(false) { "\u{f62b} " } else { "✓ " diff --git a/src/spotify.rs b/src/spotify.rs index c230240..16611e6 100644 --- a/src/spotify.rs +++ b/src/spotify.rs @@ -48,7 +48,7 @@ use core::task::Poll; use std::pin::Pin; use std::str::FromStr; use std::sync::atomic::{AtomicU16, Ordering}; -use std::sync::RwLock; +use std::sync::{Arc, RwLock}; use std::thread; use std::time::{Duration, SystemTime}; use std::{env, io}; @@ -84,7 +84,7 @@ pub enum PlayerEvent { pub struct Spotify { events: EventManager, credentials: Credentials, - pub cfg: config::Config, + cfg: Arc, status: RwLock, api: RwLock, elapsed: RwLock>, @@ -246,15 +246,19 @@ impl futures::Future for Worker { } impl Spotify { - pub fn new(events: EventManager, credentials: Credentials, cfg: &config::Config) -> Spotify { - let volume = match &cfg.saved_state { + pub fn new( + events: EventManager, + credentials: Credentials, + cfg: Arc, + ) -> Spotify { + let volume = match &cfg.values().saved_state { Some(state) => match state.volume { Some(vol) => ((std::cmp::min(vol, 100) as f32) / 100.0 * 65535_f32).ceil() as u16, None => 0xFFFF as u16, }, None => 0xFFFF as u16, }; - let repeat = match &cfg.saved_state { + let repeat = match &cfg.values().saved_state { Some(state) => match &state.repeat { Some(s) => match s.as_str() { "track" => queue::RepeatSetting::RepeatTrack, @@ -265,7 +269,7 @@ impl Spotify { }, _ => queue::RepeatSetting::None, }; - let shuffle = match &cfg.saved_state { + let shuffle = match &cfg.values().saved_state { Some(state) => matches!(&state.shuffle, Some(true)), None => false, }; @@ -273,7 +277,7 @@ impl Spotify { let mut spotify = Spotify { events, credentials, - cfg: cfg.clone(), + cfg, status: RwLock::new(PlayerEvent::Stopped), api: RwLock::new(SpotifyAPI::default()), elapsed: RwLock::new(None), @@ -306,7 +310,14 @@ impl Spotify { let volume = self.volume(); let credentials = self.credentials.clone(); thread::spawn(move || { - Self::worker(events, Box::pin(rx), cfg, credentials, user_tx, volume) + Self::worker( + events, + Box::pin(rx), + cfg.clone(), + credentials, + user_tx, + volume, + ) }); } @@ -349,7 +360,7 @@ impl Spotify { let session_config = Self::session_config(); let cache = Cache::new( config::cache_path("librespot"), - cfg.audio_cache.unwrap_or(true), + cfg.values().audio_cache.unwrap_or(true), ); let handle = core.handle(); debug!("opening spotify session"); @@ -391,12 +402,12 @@ impl Spotify { fn worker( events: EventManager, commands: Pin>>, - cfg: config::Config, + cfg: Arc, credentials: Credentials, user_tx: Option>, volume: u16, ) { - let bitrate_str = cfg.bitrate.unwrap_or(320).to_string(); + let bitrate_str = cfg.values().bitrate.unwrap_or(320).to_string(); let bitrate = Bitrate::from_str(&bitrate_str); if bitrate.is_err() { error!("invalid bitrate, will use 320 instead") @@ -405,8 +416,8 @@ impl Spotify { let player_config = PlayerConfig { gapless: false, bitrate: bitrate.unwrap_or(Bitrate::Bitrate320), - normalisation: cfg.volnorm.unwrap_or(false), - normalisation_pregain: cfg.volnorm_pregain.unwrap_or(0.0), + normalisation: cfg.values().volnorm.unwrap_or(false), + normalisation_pregain: cfg.values().volnorm_pregain.unwrap_or(0.0), }; let mut core = Core::new().unwrap(); @@ -419,12 +430,12 @@ impl Spotify { let mixer = create_mixer(None); mixer.set_volume(volume); - let backend = audio_backend::find(cfg.backend.clone()).unwrap(); + let backend = audio_backend::find(cfg.values().backend.clone()).unwrap(); let (player, player_events) = Player::new( player_config, session.clone(), mixer.get_audio_filter(), - move || (backend)(cfg.backend_device), + move || (backend)(cfg.values().backend_device.clone()), ); let worker = Worker::new( diff --git a/src/theme.rs b/src/theme.rs index 9804dce..3516f4d 100644 --- a/src/theme.rs +++ b/src/theme.rs @@ -3,11 +3,11 @@ use cursive::theme::Color::*; use cursive::theme::PaletteColor::*; use cursive::theme::*; -use crate::config::Config; +use crate::config::ConfigTheme; macro_rules! load_color { - ( $cfg: expr, $member: ident, $default: expr ) => { - $cfg.theme + ( $theme: expr, $member: ident, $default: expr ) => { + $theme .as_ref() .and_then(|t| t.$member.clone()) .map(|c| Color::parse(c.as_ref()).expect(&format!("Failed to parse color \"{}\"", c))) @@ -15,41 +15,50 @@ macro_rules! load_color { }; } -pub fn load(cfg: &Config) -> Theme { +pub fn load(theme_cfg: &Option) -> Theme { let mut palette = Palette::default(); let borders = BorderStyle::Simple; - palette[Background] = load_color!(cfg, background, TerminalDefault); - palette[View] = load_color!(cfg, background, TerminalDefault); - palette[Primary] = load_color!(cfg, primary, TerminalDefault); - palette[Secondary] = load_color!(cfg, secondary, Dark(Blue)); - palette[TitlePrimary] = load_color!(cfg, title, Dark(Red)); - palette[Tertiary] = load_color!(cfg, highlight, TerminalDefault); - palette[Highlight] = load_color!(cfg, highlight_bg, Dark(Red)); - palette.set_color("playing", load_color!(cfg, playing, Dark(Blue))); + palette[Background] = load_color!(theme_cfg, background, TerminalDefault); + palette[View] = load_color!(theme_cfg, background, TerminalDefault); + palette[Primary] = load_color!(theme_cfg, primary, TerminalDefault); + palette[Secondary] = load_color!(theme_cfg, secondary, Dark(Blue)); + palette[TitlePrimary] = load_color!(theme_cfg, title, Dark(Red)); + palette[Tertiary] = load_color!(theme_cfg, highlight, TerminalDefault); + palette[Highlight] = load_color!(theme_cfg, highlight_bg, Dark(Red)); + palette.set_color("playing", load_color!(theme_cfg, playing, Dark(Blue))); palette.set_color( "playing_selected", - load_color!(cfg, playing_selected, Light(Blue)), + load_color!(theme_cfg, playing_selected, Light(Blue)), ); - palette.set_color("playing_bg", load_color!(cfg, playing_bg, TerminalDefault)); - palette.set_color("error", load_color!(cfg, error, TerminalDefault)); - palette.set_color("error_bg", load_color!(cfg, error_bg, Dark(Red))); + palette.set_color( + "playing_bg", + load_color!(theme_cfg, playing_bg, TerminalDefault), + ); + palette.set_color("error", load_color!(theme_cfg, error, TerminalDefault)); + palette.set_color("error_bg", load_color!(theme_cfg, error_bg, Dark(Red))); palette.set_color( "statusbar_progress", - load_color!(cfg, statusbar_progress, Dark(Blue)), + load_color!(theme_cfg, statusbar_progress, Dark(Blue)), ); palette.set_color( "statusbar_progress_bg", - load_color!(cfg, statusbar_progress_bg, Light(Black)), + load_color!(theme_cfg, statusbar_progress_bg, Light(Black)), ); - palette.set_color("statusbar", load_color!(cfg, statusbar, Dark(Yellow))); + palette.set_color("statusbar", load_color!(theme_cfg, statusbar, Dark(Yellow))); palette.set_color( "statusbar_bg", - load_color!(cfg, statusbar_bg, TerminalDefault), + load_color!(theme_cfg, statusbar_bg, TerminalDefault), + ); + palette.set_color("cmdline", load_color!(theme_cfg, cmdline, TerminalDefault)); + palette.set_color( + "cmdline_bg", + load_color!(theme_cfg, cmdline_bg, TerminalDefault), + ); + palette.set_color( + "search_match", + load_color!(theme_cfg, search_match, Light(Red)), ); - palette.set_color("cmdline", load_color!(cfg, cmdline, TerminalDefault)); - palette.set_color("cmdline_bg", load_color!(cfg, cmdline_bg, TerminalDefault)); - palette.set_color("search_match", load_color!(cfg, search_match, Light(Red))); Theme { shadow: false, diff --git a/src/track.rs b/src/track.rs index 5676f55..266b914 100644 --- a/src/track.rs +++ b/src/track.rs @@ -154,7 +154,7 @@ impl ListItem for Track { } fn display_center(&self, library: Arc) -> String { - if library.config().album_column.unwrap_or(true) { + if library.cfg.values().album_column.unwrap_or(true) { self.album.to_string() } else { "".to_string() @@ -163,7 +163,7 @@ impl ListItem for Track { fn display_right(&self, library: Arc) -> String { let saved = if library.is_saved_track(&Playable::Track(self.clone())) { - if library.use_nerdfont { + if library.cfg.values().use_nerdfont.unwrap_or(false) { "\u{f62b} " } else { "✓ "