Some code cleanup

codebook-config/src/lib.rs

@@ -33,7 +33,7 @@ pub struct ConfigSettings {
 impl Default for ConfigSettings {
     fn default() -> Self {
         Self {
-            dictionaries: vec!["en_us".to_string()],
+            dictionaries: vec![],
             words: Vec::new(),
             flag_words: Vec::new(),
             ignore_paths: Vec::new(),
@@ -133,23 +133,21 @@ impl CodebookConfig {
     }
 
     /// Add a word to the allowlist and save the configuration
-    pub fn add_word(&self, word: &str) -> Result<()> {
+    pub fn add_word(&self, word: &str) -> Result<bool> {
         {
             let word = word.to_lowercase();
             let settings = &mut self.settings.write().unwrap();
             // Check if word already exists
             if settings.words.contains(&word.to_string()) {
-                return Ok(());
+                return Ok(false);
             }
 
             // Add the word
             settings.words.push(word.to_string());
             // Sort for consistency
             settings.words.sort();
         }
-        // Save the changes
-        self.save()?;
-        Ok(())
+        Ok(true)
     }
 
     /// Save the configuration to its file
@@ -159,8 +157,7 @@ impl CodebookConfig {
             None => return Ok(()),
         };
 
-        let content = toml::to_string_pretty(&*self.settings.read().unwrap())
-            .context("Failed to serialize config")?;
+        let content = self.as_toml()?;
 
         fs::write(config_path, content).with_context(|| {
             format!("Failed to write config to file: {}", config_path.display())
@@ -169,6 +166,11 @@ impl CodebookConfig {
         Ok(())
     }
 
+    pub fn as_toml(&self) -> Result<String> {
+        toml::to_string_pretty(&*self.settings.read().unwrap())
+            .context("Failed to serialize config")
+    }
+
     /// Create a new configuration file if one doesn't exist
     pub fn create_if_not_exists(directory: Option<&PathBuf>) -> Result<Self> {
         let current_dir = env::current_dir().context("Failed to get current directory")?;

codebook-lsp/src/file_cache.rs

@@ -1,4 +1,7 @@
-use std::num::NonZero;
+use std::{
+    num::NonZero,
+    sync::{Arc, RwLock},
+};
 
 use lru::LruCache;
 use tower_lsp::lsp_types::{TextDocumentItem, Url};
@@ -35,33 +38,37 @@ impl TextDocumentCacheItem {
 
 #[derive(Debug)]
 pub struct TextDocumentCache {
-    documents: LruCache<String, TextDocumentCacheItem>,
+    documents: Arc<RwLock<LruCache<String, TextDocumentCacheItem>>>,
 }
 
 impl TextDocumentCache {
     pub fn new() -> Self {
         Self {
-            documents: LruCache::new(NonZero::new(1000).unwrap()),
+            documents: Arc::new(RwLock::new(LruCache::new(NonZero::new(1000).unwrap()))),
         }
     }
 
-    pub fn get(&mut self, uri: &str) -> Option<&TextDocumentCacheItem> {
-        self.documents.get(uri)
+    pub fn get(&self, uri: &str) -> Option<TextDocumentCacheItem> {
+        self.documents.write().unwrap().get(uri).cloned()
     }
 
-    pub fn insert(&mut self, document: &TextDocumentItem) {
+    pub fn insert(&self, document: &TextDocumentItem) {
         let document = TextDocumentCacheItem::new(
             &document.uri,
             Some(document.version),
             Some(&document.language_id),
             Some(&document.text),
         );
-        self.documents.put(document.uri.to_string(), document);
+        self.documents
+            .write()
+            .unwrap()
+            .put(document.uri.to_string(), document);
     }
 
-    pub fn update(&mut self, uri: &Url, text: &str) {
+    pub fn update(&self, uri: &Url, text: &str) {
         let key = uri.to_string();
-        let item = self.documents.get(&key);
+        let mut cache = self.documents.write().unwrap();
+        let item = cache.get(&key);
         match item {
             Some(item) => {
                 let new_item = TextDocumentCacheItem::new(
@@ -70,20 +77,25 @@ impl TextDocumentCache {
                     item.language_id.as_deref(),
                     Some(text),
                 );
-                self.documents.put(key, new_item);
+                cache.put(key, new_item);
             }
             None => {
                 let item = TextDocumentCacheItem::new(uri, None, None, Some(text));
-                self.documents.put(key, item);
+                cache.put(key, item);
             }
         }
     }
 
-    pub fn remove(&mut self, uri: &Url) {
-        self.documents.pop(uri.as_str());
+    pub fn remove(&self, uri: &Url) {
+        self.documents.write().unwrap().pop(uri.as_str());
     }
 
-    pub fn iter(&self) -> impl Iterator<Item = &TextDocumentCacheItem> {
-        self.documents.iter().map(|(_, item)| item)
+    pub fn cached_urls(&self) -> Vec<Url> {
+        self.documents
+            .read()
+            .unwrap()
+            .iter()
+            .map(|(_, v)| v.uri.clone())
+            .collect()
     }
 }

codebook-lsp/src/lsp.rs

@@ -1,6 +1,6 @@
 use std::collections::HashMap;
 use std::path::PathBuf;
-use std::sync::{Arc, RwLock};
+use std::sync::Arc;
 
 use codebook::dictionary::{SpellCheckResult, TextRange};
 use serde_json::Value;
@@ -12,7 +12,7 @@ use codebook::Codebook;
 use codebook_config::CodebookConfig;
 use log::{debug, info};
 
-use crate::file_cache::{TextDocumentCache, TextDocumentCacheItem};
+use crate::file_cache::TextDocumentCache;
 
 const SOURCE_NAME: &str = "Codebook";
 
@@ -21,7 +21,7 @@ pub struct Backend {
     pub client: Client,
     pub codebook: Codebook,
     pub config: Arc<CodebookConfig>,
-    pub document_cache: Arc<RwLock<TextDocumentCache>>,
+    pub document_cache: TextDocumentCache,
 }
 
 enum CodebookCommand {
@@ -80,12 +80,12 @@ impl LanguageServer for Backend {
     }
 
     async fn did_open(&self, params: DidOpenTextDocumentParams) {
-        self.insert_cache(&params.text_document);
+        self.document_cache.insert(&params.text_document);
         self.spell_check(&params.text_document.uri).await;
     }
 
     async fn did_close(&self, params: DidCloseTextDocumentParams) {
-        self.delete_cache(&params.text_document.uri);
+        self.document_cache.remove(&params.text_document.uri);
         // Clear diagnostics when a file is closed.
         self.client
             .publish_diagnostics(params.text_document.uri, vec![], None)
@@ -94,14 +94,17 @@ impl LanguageServer for Backend {
 
     async fn did_save(&self, params: DidSaveTextDocumentParams) {
         if let Some(text) = params.text {
-            self.update_cache(&params.text_document.uri, &text);
+            self.document_cache.update(&params.text_document.uri, &text);
             self.spell_check(&params.text_document.uri).await;
         }
     }
 
     async fn code_action(&self, params: CodeActionParams) -> RpcResult<Option<CodeActionResponse>> {
         let mut actions: Vec<CodeActionOrCommand> = vec![];
-        let doc = match self.get_cache(&params.text_document.uri) {
+        let doc = match self
+            .document_cache
+            .get(&params.text_document.uri.to_string())
+        {
             Some(doc) => doc,
             None => return Ok(None),
         };
@@ -150,17 +153,14 @@ impl LanguageServer for Backend {
     async fn execute_command(&self, params: ExecuteCommandParams) -> RpcResult<Option<Value>> {
         match CodebookCommand::from(params.command.as_str()) {
             CodebookCommand::AddWord => {
-                for args in params.arguments {
-                    if let Some(word) = args.as_str() {
-                        match self.config.add_word(word) {
-                            Ok(_) => {
-                                self.recheck_all().await;
-                            }
-                            Err(e) => {
-                                log::error!("Failed to add word: {}", e);
-                            }
-                        }
-                    }
+                let words = params
+                    .arguments
+                    .iter()
+                    .filter_map(|arg| arg.as_str().map(|s| s.to_string()));
+                let updated = self.add_words(words);
+                if updated {
+                    let _ = self.config.save();
+                    self.recheck_all().await;
                 }
                 Ok(None)
             }
@@ -178,7 +178,7 @@ impl Backend {
             client,
             codebook,
             config: Arc::clone(&config_arc),
-            document_cache: Arc::new(RwLock::new(TextDocumentCache::new())),
+            document_cache: TextDocumentCache::new(),
         }
     }
     fn make_diagnostic(&self, result: &SpellCheckResult, range: &TextRange) -> Diagnostic {
@@ -205,6 +205,24 @@ impl Backend {
         }
     }
 
+    fn add_words(&self, words: impl Iterator<Item = String>) -> bool {
+        let mut should_save = false;
+        for word in words {
+            match self.config.add_word(&word) {
+                Ok(true) => {
+                    should_save = true;
+                }
+                Ok(false) => {
+                    log::info!("Word '{}' already exists in dictionary.", word);
+                }
+                Err(e) => {
+                    log::error!("Failed to add word: {}", e);
+                }
+            }
+        }
+        should_save
+    }
+
     fn make_suggestion(&self, suggestion: &str, range: &Range, uri: &Url) -> CodeAction {
         let title = format!("Replace with {}", suggestion);
         let mut map = HashMap::new();
@@ -232,34 +250,8 @@ impl Backend {
         }
     }
 
-    fn insert_cache(&self, doc: &TextDocumentItem) {
-        let mut cache = self.document_cache.write().unwrap();
-        cache.insert(doc);
-    }
-
-    fn delete_cache(&self, uri: &Url) {
-        let mut cache = self.document_cache.write().unwrap();
-        cache.remove(uri);
-    }
-
-    fn get_cache(&self, uri: &Url) -> Option<TextDocumentCacheItem> {
-        self.document_cache
-            .write()
-            .unwrap()
-            .get(&uri.to_string())
-            .cloned()
-    }
-
-    fn update_cache(&self, uri: &Url, text: &str) {
-        let mut cache = self.document_cache.write().unwrap();
-        cache.update(uri, text);
-    }
-
     async fn recheck_all(&self) {
-        let urls = {
-            let cache = self.document_cache.read().unwrap();
-            cache.iter().map(|doc| doc.uri.clone()).collect::<Vec<_>>()
-        };
+        let urls = self.document_cache.cached_urls();
         debug!("Rechecking documents: {:?}", urls);
         for url in urls {
             self.publish_spellcheck_diagnostics(&url).await;
@@ -275,7 +267,18 @@ impl Backend {
             }
         };
 
-        if did_reload {
+        let did_config_change = uri
+            .to_file_path()
+            .map(|path| {
+                self.config
+                    .config_path
+                    .as_ref()
+                    .map(|config_path| path == *config_path)
+                    .unwrap_or(false)
+            })
+            .unwrap_or(false);
+
+        if did_reload || did_config_change {
             self.recheck_all().await;
         } else {
             self.publish_spellcheck_diagnostics(uri).await;
@@ -284,7 +287,7 @@ impl Backend {
 
     /// Helper method to publish diagnostics for spell-checking.
     async fn publish_spellcheck_diagnostics(&self, uri: &Url) {
-        let doc = match self.get_cache(uri) {
+        let doc = match self.document_cache.get(&uri.to_string()) {
             Some(doc) => doc,
             None => return,
         };