From fe869e5dc7a8cd6c2c2e3945816bd890956eef3a Mon Sep 17 00:00:00 2001 From: kyfanc Date: Tue, 13 Feb 2024 18:58:53 +0800 Subject: [PATCH] fix lsp config reload (#9415) `syn_loader` was replaced rather than interior value being replace, old value was still being referenced and not updated after `:config-refresh`. By using `ArcSwap` like for `config`, each `.load()` call will return the most updated value. Co-authored-by: kyfan --- Cargo.lock | 1 + helix-core/src/syntax.rs | 30 +++++++++++++++++++++++------- helix-core/tests/indent.rs | 10 ++++++++-- helix-lsp/Cargo.toml | 1 + helix-lsp/src/lib.rs | 9 +++++---- helix-term/src/application.rs | 7 ++++--- helix-term/src/ui/lsp.rs | 9 +++++++-- helix-term/src/ui/markdown.rs | 8 +++++--- helix-term/src/ui/mod.rs | 4 ++-- helix-term/src/ui/picker.rs | 11 +++++++---- helix-term/src/ui/prompt.rs | 9 +++++++-- helix-view/src/document.rs | 20 ++++++++++++-------- helix-view/src/editor.rs | 11 +++++++---- 13 files changed, 89 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6131871d7..6f38f0034 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1316,6 +1316,7 @@ name = "helix-lsp" version = "23.10.0" dependencies = [ "anyhow", + "arc-swap", "futures-executor", "futures-util", "globset", diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index 5d45deaf4..a9344448f 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -1000,7 +1000,7 @@ thread_local! { pub struct Syntax { layers: HopSlotMap, root: LayerId, - loader: Arc, + loader: Arc>, } fn byte_range_to_str(range: std::ops::Range, source: RopeSlice) -> Cow { @@ -1011,7 +1011,7 @@ impl Syntax { pub fn new( source: RopeSlice, config: Arc, - loader: Arc, + loader: Arc>, ) -> Option { let root_layer = LanguageLayer { tree: None, @@ -1055,9 +1055,10 @@ impl Syntax { let mut queue = VecDeque::new(); queue.push_back(self.root); - let scopes = self.loader.scopes.load(); + let loader = self.loader.load(); + let scopes = loader.scopes.load(); let injection_callback = |language: &InjectionLanguageMarker| { - self.loader + loader .language_configuration_for_injection_string(language) .and_then(|language_config| language_config.highlight_config(&scopes)) }; @@ -2663,7 +2664,12 @@ mod test { let mut cursor = QueryCursor::new(); let config = HighlightConfiguration::new(language, "", "", "").unwrap(); - let syntax = Syntax::new(source.slice(..), Arc::new(config), Arc::new(loader)).unwrap(); + let syntax = Syntax::new( + source.slice(..), + Arc::new(config), + Arc::new(ArcSwap::from_pointee(loader)), + ) + .unwrap(); let root = syntax.tree().root_node(); let mut test = |capture, range| { @@ -2738,7 +2744,12 @@ mod test { fn main() {} ", ); - let syntax = Syntax::new(source.slice(..), Arc::new(config), Arc::new(loader)).unwrap(); + let syntax = Syntax::new( + source.slice(..), + Arc::new(config), + Arc::new(ArcSwap::from_pointee(loader)), + ) + .unwrap(); let tree = syntax.tree(); let root = tree.root_node(); assert_eq!(root.kind(), "source_file"); @@ -2829,7 +2840,12 @@ mod test { let language = get_language(language_name).unwrap(); let config = HighlightConfiguration::new(language, "", "", "").unwrap(); - let syntax = Syntax::new(source.slice(..), Arc::new(config), Arc::new(loader)).unwrap(); + let syntax = Syntax::new( + source.slice(..), + Arc::new(config), + Arc::new(ArcSwap::from_pointee(loader)), + ) + .unwrap(); let root = syntax .tree() diff --git a/helix-core/tests/indent.rs b/helix-core/tests/indent.rs index de1434f72..53265e0b1 100644 --- a/helix-core/tests/indent.rs +++ b/helix-core/tests/indent.rs @@ -1,10 +1,11 @@ +use arc_swap::ArcSwap; use helix_core::{ indent::{indent_level_for_line, treesitter_indent_for_pos, IndentStyle}, syntax::{Configuration, Loader}, Syntax, }; use ropey::Rope; -use std::{ops::Range, path::PathBuf, process::Command}; +use std::{ops::Range, path::PathBuf, process::Command, sync::Arc}; #[test] fn test_treesitter_indent_rust() { @@ -197,7 +198,12 @@ fn test_treesitter_indent( let indent_style = IndentStyle::from_str(&language_config.indent.as_ref().unwrap().unit); let highlight_config = language_config.highlight_config(&[]).unwrap(); let text = doc.slice(..); - let syntax = Syntax::new(text, highlight_config, std::sync::Arc::new(loader)).unwrap(); + let syntax = Syntax::new( + text, + highlight_config, + Arc::new(ArcSwap::from_pointee(loader)), + ) + .unwrap(); let indent_query = language_config.indent_query().unwrap(); for i in 0..doc.len_lines() { diff --git a/helix-lsp/Cargo.toml b/helix-lsp/Cargo.toml index 2bdd5cfce..4284b0520 100644 --- a/helix-lsp/Cargo.toml +++ b/helix-lsp/Cargo.toml @@ -30,3 +30,4 @@ thiserror = "1.0" tokio = { version = "1.36", features = ["rt", "rt-multi-thread", "io-util", "io-std", "time", "process", "macros", "fs", "parking_lot", "sync"] } tokio-stream = "0.1.14" parking_lot = "0.12.1" +arc-swap = "1" diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs index 05764418f..c58d967b6 100644 --- a/helix-lsp/src/lib.rs +++ b/helix-lsp/src/lib.rs @@ -5,6 +5,7 @@ pub mod jsonrpc; pub mod snippet; mod transport; +use arc_swap::ArcSwap; pub use client::Client; pub use futures_executor::block_on; pub use jsonrpc::Call; @@ -640,14 +641,14 @@ impl Notification { #[derive(Debug)] pub struct Registry { inner: HashMap>>, - syn_loader: Arc, + syn_loader: Arc>, counter: usize, pub incoming: SelectAll>, pub file_event_handler: file_event::Handler, } impl Registry { - pub fn new(syn_loader: Arc) -> Self { + pub fn new(syn_loader: Arc>) -> Self { Self { inner: HashMap::new(), syn_loader, @@ -681,8 +682,8 @@ impl Registry { root_dirs: &[PathBuf], enable_snippets: bool, ) -> Result>> { - let config = self - .syn_loader + let syn_loader = self.syn_loader.load(); + let config = syn_loader .language_server_configs() .get(&name) .ok_or_else(|| anyhow::anyhow!("Language server '{name}' not defined"))?; diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index b844b5f05..30df3981c 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -66,7 +66,7 @@ pub struct Application { #[allow(dead_code)] theme_loader: Arc, #[allow(dead_code)] - syn_loader: Arc, + syn_loader: Arc>, signals: Signals, jobs: Jobs, @@ -122,7 +122,7 @@ impl Application { }) .unwrap_or_else(|| theme_loader.default_theme(true_color)); - let syn_loader = std::sync::Arc::new(lang_loader); + let syn_loader = Arc::new(ArcSwap::from_pointee(lang_loader)); #[cfg(not(feature = "integration"))] let backend = CrosstermBackend::new(stdout(), &config.editor); @@ -391,7 +391,8 @@ impl Application { /// refresh language config after config change fn refresh_language_config(&mut self) -> Result<(), Error> { let lang_loader = helix_core::config::user_lang_loader()?; - self.syn_loader = std::sync::Arc::new(lang_loader); + + self.syn_loader.store(Arc::new(lang_loader)); self.editor.syn_loader = self.syn_loader.clone(); for document in self.editor.documents.values_mut() { document.detect_language(self.syn_loader.clone()); diff --git a/helix-term/src/ui/lsp.rs b/helix-term/src/ui/lsp.rs index 7037b1552..879f963e7 100644 --- a/helix-term/src/ui/lsp.rs +++ b/helix-term/src/ui/lsp.rs @@ -1,5 +1,6 @@ use std::sync::Arc; +use arc_swap::ArcSwap; use helix_core::syntax; use helix_view::graphics::{Margin, Rect, Style}; use tui::buffer::Buffer; @@ -18,13 +19,17 @@ pub struct SignatureHelp { active_param_range: Option<(usize, usize)>, language: String, - config_loader: Arc, + config_loader: Arc>, } impl SignatureHelp { pub const ID: &'static str = "signature-help"; - pub fn new(signature: String, language: String, config_loader: Arc) -> Self { + pub fn new( + signature: String, + language: String, + config_loader: Arc>, + ) -> Self { Self { signature, signature_doc: None, diff --git a/helix-term/src/ui/markdown.rs b/helix-term/src/ui/markdown.rs index 5cf530ad8..749d58508 100644 --- a/helix-term/src/ui/markdown.rs +++ b/helix-term/src/ui/markdown.rs @@ -1,4 +1,5 @@ use crate::compositor::{Component, Context}; +use arc_swap::ArcSwap; use tui::{ buffer::Buffer as Surface, text::{Span, Spans, Text}, @@ -31,7 +32,7 @@ pub fn highlighted_code_block<'a>( text: &str, language: &str, theme: Option<&Theme>, - config_loader: Arc, + config_loader: Arc>, additional_highlight_spans: Option)>>, ) -> Text<'a> { let mut spans = Vec::new(); @@ -48,6 +49,7 @@ pub fn highlighted_code_block<'a>( let ropeslice = RopeSlice::from(text); let syntax = config_loader + .load() .language_configuration_for_injection_string(&InjectionLanguageMarker::Name( language.into(), )) @@ -121,7 +123,7 @@ pub fn highlighted_code_block<'a>( pub struct Markdown { contents: String, - config_loader: Arc, + config_loader: Arc>, } // TODO: pre-render and self reference via Pin @@ -140,7 +142,7 @@ impl Markdown { ]; const INDENT: &'static str = " "; - pub fn new(contents: String, config_loader: Arc) -> Self { + pub fn new(contents: String, config_loader: Arc>) -> Self { Self { contents, config_loader, diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index efa2473e0..d27e83553 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -336,8 +336,8 @@ pub mod completers { pub fn language(editor: &Editor, input: &str) -> Vec { let text: String = "text".into(); - let language_ids = editor - .syn_loader + let loader = editor.syn_loader.load(); + let language_ids = loader .language_configs() .map(|config| &config.language_id) .chain(std::iter::once(&text)); diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index 4be5a11ef..c2728888a 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -461,14 +461,17 @@ impl Picker { // Then attempt to highlight it if it has no language set if doc.language_config().is_none() { - if let Some(language_config) = doc.detect_language_config(&cx.editor.syn_loader) { + if let Some(language_config) = doc.detect_language_config(&cx.editor.syn_loader.load()) + { doc.language = Some(language_config.clone()); let text = doc.text().clone(); let loader = cx.editor.syn_loader.clone(); let job = tokio::task::spawn_blocking(move || { - let syntax = language_config.highlight_config(&loader.scopes()).and_then( - |highlight_config| Syntax::new(text.slice(..), highlight_config, loader), - ); + let syntax = language_config + .highlight_config(&loader.load().scopes()) + .and_then(|highlight_config| { + Syntax::new(text.slice(..), highlight_config, loader) + }); let callback = move |editor: &mut Editor, compositor: &mut Compositor| { let Some(syntax) = syntax else { log::info!("highlighting picker item failed"); diff --git a/helix-term/src/ui/prompt.rs b/helix-term/src/ui/prompt.rs index 3764bba60..a6ee7f05d 100644 --- a/helix-term/src/ui/prompt.rs +++ b/helix-term/src/ui/prompt.rs @@ -1,5 +1,6 @@ use crate::compositor::{Component, Compositor, Context, Event, EventResult}; use crate::{alt, ctrl, key, shift, ui}; +use arc_swap::ArcSwap; use helix_core::syntax; use helix_view::input::KeyEvent; use helix_view::keyboard::KeyCode; @@ -34,7 +35,7 @@ pub struct Prompt { callback_fn: CallbackFn, pub doc_fn: DocFn, next_char_handler: Option, - language: Option<(&'static str, Arc)>, + language: Option<(&'static str, Arc>)>, } #[derive(Clone, Copy, PartialEq, Eq)] @@ -98,7 +99,11 @@ impl Prompt { self } - pub fn with_language(mut self, language: &'static str, loader: Arc) -> Self { + pub fn with_language( + mut self, + language: &'static str, + loader: Arc>, + ) -> Self { self.language = Some((language, loader)); self } diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 33137c6c9..4e7b1de9f 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -681,7 +681,7 @@ impl Document { pub fn open( path: &Path, encoding: Option<&'static Encoding>, - config_loader: Option>, + config_loader: Option>>, config: Arc>, ) -> Result { // Open the file if it exists, otherwise assume it is a new file (and thus empty). @@ -922,10 +922,11 @@ impl Document { } /// Detect the programming language based on the file type. - pub fn detect_language(&mut self, config_loader: Arc) { + pub fn detect_language(&mut self, config_loader: Arc>) { + let loader = config_loader.load(); self.set_language( - self.detect_language_config(&config_loader), - Some(config_loader), + self.detect_language_config(&loader), + Some(Arc::clone(&config_loader)), ); } @@ -1059,10 +1060,12 @@ impl Document { pub fn set_language( &mut self, language_config: Option>, - loader: Option>, + loader: Option>>, ) { if let (Some(language_config), Some(loader)) = (language_config, loader) { - if let Some(highlight_config) = language_config.highlight_config(&loader.scopes()) { + if let Some(highlight_config) = + language_config.highlight_config(&(*loader).load().scopes()) + { self.syntax = Syntax::new(self.text.slice(..), highlight_config, loader); } @@ -1078,9 +1081,10 @@ impl Document { pub fn set_language_by_language_id( &mut self, language_id: &str, - config_loader: Arc, + config_loader: Arc>, ) -> anyhow::Result<()> { - let language_config = config_loader + let language_config = (*config_loader) + .load() .language_config_for_language_id(language_id) .ok_or_else(|| anyhow!("invalid language id: {}", language_id))?; self.set_language(Some(language_config), Some(config_loader)); diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 0fa6d67c9..68b74cf00 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -50,7 +50,10 @@ use helix_stdx::path::canonicalize; use serde::{ser::SerializeMap, Deserialize, Deserializer, Serialize, Serializer}; -use arc_swap::access::{DynAccess, DynGuard}; +use arc_swap::{ + access::{DynAccess, DynGuard}, + ArcSwap, +}; fn deserialize_duration_millis<'de, D>(deserializer: D) -> Result where @@ -918,7 +921,7 @@ pub struct Editor { pub debugger_events: SelectAll>, pub breakpoints: HashMap>, - pub syn_loader: Arc, + pub syn_loader: Arc>, pub theme_loader: Arc, /// last_theme is used for theme previews. We store the current theme here, /// and if previewing is cancelled, we can return to it. @@ -1029,7 +1032,7 @@ impl Editor { pub fn new( mut area: Rect, theme_loader: Arc, - syn_loader: Arc, + syn_loader: Arc>, config: Arc>, handlers: Handlers, ) -> Self { @@ -1190,7 +1193,7 @@ impl Editor { } let scopes = theme.scopes(); - self.syn_loader.set_scopes(scopes.to_vec()); + (*self.syn_loader).load().set_scopes(scopes.to_vec()); match preview { ThemeAction::Preview => {