From b2e8252785737db5d2ba3e440499a485d9e86772 Mon Sep 17 00:00:00 2001 From: David Knaack Date: Sun, 24 Jan 2021 21:19:22 +0100 Subject: [PATCH] refactor(git_status): simplify git status with once-cell (#2150) I simplified the code in the git status module by moving everything from RwLock<_> to OnceCell<_>. I think this should also get rid of any remaining race conditions that remained after #1777. --- src/modules/git_status.rs | 95 +++++++++++---------------------------- 1 file changed, 27 insertions(+), 68 deletions(-) diff --git a/src/modules/git_status.rs b/src/modules/git_status.rs index c555ce904be1..22ee628b6805 100644 --- a/src/modules/git_status.rs +++ b/src/modules/git_status.rs @@ -1,4 +1,5 @@ use git2::{Repository, Status}; +use once_cell::sync::OnceCell; use super::{Context, Module, RootModuleConfig}; @@ -6,7 +7,7 @@ use crate::configs::git_status::GitStatusConfig; use crate::context::Repo; use crate::formatter::StringFormatter; use crate::segment::Segment; -use std::sync::{Arc, RwLock}; +use std::sync::Arc; const ALL_STATUS_FORMAT: &str = "$conflicted$stashed$deleted$renamed$modified$staged$untracked"; @@ -108,18 +109,18 @@ pub fn module<'a>(context: &'a Context) -> Option> { struct GitStatusInfo<'a> { repo: &'a Repo, - ahead_behind: RwLock>>, - repo_status: RwLock>>, - stashed_count: RwLock>>, + ahead_behind: OnceCell>, + repo_status: OnceCell>, + stashed_count: OnceCell>, } impl<'a> GitStatusInfo<'a> { pub fn load(repo: &'a Repo) -> Self { Self { repo, - ahead_behind: RwLock::new(None), - repo_status: RwLock::new(None), - stashed_count: RwLock::new(None), + ahead_behind: OnceCell::new(), + repo_status: OnceCell::new(), + stashed_count: OnceCell::new(), } } @@ -137,89 +138,47 @@ impl<'a> GitStatusInfo<'a> { Repository::open(repo_root).ok() } - pub fn get_ahead_behind(&self) -> Option<(usize, usize)> { - { - let data = self.ahead_behind.read().unwrap(); - if let Some(result) = data.as_ref() { - return match result.as_ref() { - Ok(ahead_behind) => Some(*ahead_behind), - Err(error) => { - log::debug!("get_ahead_behind: {}", error); - None - } - }; - }; - } - - { - let mut data = self.ahead_behind.write().unwrap(); + pub fn get_ahead_behind(&self) -> &Option<(usize, usize)> { + self.ahead_behind.get_or_init(|| { let repo = self.get_repository()?; let branch_name = self.get_branch_name(); - *data = Some(get_ahead_behind(&repo, &branch_name)); - match data.as_ref().unwrap() { - Ok(ahead_behind) => Some(*ahead_behind), + + match get_ahead_behind(&repo, &branch_name) { + Ok(ahead_behind) => Some(ahead_behind), Err(error) => { log::debug!("get_ahead_behind: {}", error); None } } - } + }) } - pub fn get_repo_status(&self) -> Option { - { - let data = self.repo_status.read().unwrap(); - if let Some(result) = data.as_ref() { - return match result.as_ref() { - Ok(repo_status) => Some(*repo_status), - Err(error) => { - log::debug!("get_repo_status: {}", error); - None - } - }; - }; - } - - { - let mut data = self.repo_status.write().unwrap(); + pub fn get_repo_status(&self) -> &Option { + self.repo_status.get_or_init(|| { let mut repo = self.get_repository()?; - *data = Some(get_repo_status(&mut repo)); - match data.as_ref().unwrap() { - Ok(repo_status) => Some(*repo_status), + + match get_repo_status(&mut repo) { + Ok(repo_status) => Some(repo_status), Err(error) => { - log::debug!(" get_repo_status: {}", error); + log::debug!("get_repo_status: {}", error); None } } - } + }) } - pub fn get_stashed(&self) -> Option { - { - let data = self.stashed_count.read().unwrap(); - if let Some(result) = data.as_ref() { - return match result.as_ref() { - Ok(stashed_count) => Some(*stashed_count), - Err(error) => { - log::debug!("get_stashed_count: {}", error); - None - } - }; - }; - } - - { - let mut data = self.stashed_count.write().unwrap(); + pub fn get_stashed(&self) -> &Option { + self.stashed_count.get_or_init(|| { let mut repo = self.get_repository()?; - *data = Some(get_stashed_count(&mut repo)); - match data.as_ref().unwrap() { - Ok(stashed_count) => Some(*stashed_count), + + match get_stashed_count(&mut repo) { + Ok(stashed_count) => Some(stashed_count), Err(error) => { log::debug!("get_stashed_count: {}", error); None } } - } + }) } pub fn get_conflicted(&self) -> Option {