Skip to content

Commit

Permalink
fix lsp config reload (helix-editor#9415)
Browse files Browse the repository at this point in the history
`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 <kyfan@email>
  • Loading branch information
2 people authored and cosmikwolf committed Feb 26, 2024
1 parent ad08d70 commit 150b567
Show file tree
Hide file tree
Showing 13 changed files with 89 additions and 41 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 23 additions & 7 deletions helix-core/src/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1000,7 +1000,7 @@ thread_local! {
pub struct Syntax {
layers: HopSlotMap<LayerId, LanguageLayer>,
root: LayerId,
loader: Arc<Loader>,
loader: Arc<ArcSwap<Loader>>,
}

fn byte_range_to_str(range: std::ops::Range<usize>, source: RopeSlice) -> Cow<str> {
Expand All @@ -1011,7 +1011,7 @@ impl Syntax {
pub fn new(
source: RopeSlice,
config: Arc<HighlightConfiguration>,
loader: Arc<Loader>,
loader: Arc<ArcSwap<Loader>>,
) -> Option<Self> {
let root_layer = LanguageLayer {
tree: None,
Expand Down Expand Up @@ -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))
};
Expand Down Expand Up @@ -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| {
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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()
Expand Down
10 changes: 8 additions & 2 deletions helix-core/tests/indent.rs
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down
1 change: 1 addition & 0 deletions helix-lsp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
9 changes: 5 additions & 4 deletions helix-lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -640,14 +641,14 @@ impl Notification {
#[derive(Debug)]
pub struct Registry {
inner: HashMap<LanguageServerName, Vec<Arc<Client>>>,
syn_loader: Arc<helix_core::syntax::Loader>,
syn_loader: Arc<ArcSwap<helix_core::syntax::Loader>>,
counter: usize,
pub incoming: SelectAll<UnboundedReceiverStream<(usize, Call)>>,
pub file_event_handler: file_event::Handler,
}

impl Registry {
pub fn new(syn_loader: Arc<helix_core::syntax::Loader>) -> Self {
pub fn new(syn_loader: Arc<ArcSwap<helix_core::syntax::Loader>>) -> Self {
Self {
inner: HashMap::new(),
syn_loader,
Expand Down Expand Up @@ -681,8 +682,8 @@ impl Registry {
root_dirs: &[PathBuf],
enable_snippets: bool,
) -> Result<Option<Arc<Client>>> {
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"))?;
Expand Down
7 changes: 4 additions & 3 deletions helix-term/src/application.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ pub struct Application {
#[allow(dead_code)]
theme_loader: Arc<theme::Loader>,
#[allow(dead_code)]
syn_loader: Arc<syntax::Loader>,
syn_loader: Arc<ArcSwap<syntax::Loader>>,

signals: Signals,
jobs: Jobs,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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());
Expand Down
9 changes: 7 additions & 2 deletions helix-term/src/ui/lsp.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -18,13 +19,17 @@ pub struct SignatureHelp {
active_param_range: Option<(usize, usize)>,

language: String,
config_loader: Arc<syntax::Loader>,
config_loader: Arc<ArcSwap<syntax::Loader>>,
}

impl SignatureHelp {
pub const ID: &'static str = "signature-help";

pub fn new(signature: String, language: String, config_loader: Arc<syntax::Loader>) -> Self {
pub fn new(
signature: String,
language: String,
config_loader: Arc<ArcSwap<syntax::Loader>>,
) -> Self {
Self {
signature,
signature_doc: None,
Expand Down
8 changes: 5 additions & 3 deletions helix-term/src/ui/markdown.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::compositor::{Component, Context};
use arc_swap::ArcSwap;
use tui::{
buffer::Buffer as Surface,
text::{Span, Spans, Text},
Expand Down Expand Up @@ -31,7 +32,7 @@ pub fn highlighted_code_block<'a>(
text: &str,
language: &str,
theme: Option<&Theme>,
config_loader: Arc<syntax::Loader>,
config_loader: Arc<ArcSwap<syntax::Loader>>,
additional_highlight_spans: Option<Vec<(usize, std::ops::Range<usize>)>>,
) -> Text<'a> {
let mut spans = Vec::new();
Expand All @@ -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(),
))
Expand Down Expand Up @@ -121,7 +123,7 @@ pub fn highlighted_code_block<'a>(
pub struct Markdown {
contents: String,

config_loader: Arc<syntax::Loader>,
config_loader: Arc<ArcSwap<syntax::Loader>>,
}

// TODO: pre-render and self reference via Pin
Expand All @@ -140,7 +142,7 @@ impl Markdown {
];
const INDENT: &'static str = " ";

pub fn new(contents: String, config_loader: Arc<syntax::Loader>) -> Self {
pub fn new(contents: String, config_loader: Arc<ArcSwap<syntax::Loader>>) -> Self {
Self {
contents,
config_loader,
Expand Down
4 changes: 2 additions & 2 deletions helix-term/src/ui/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,8 @@ pub mod completers {
pub fn language(editor: &Editor, input: &str) -> Vec<Completion> {
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));
Expand Down
11 changes: 7 additions & 4 deletions helix-term/src/ui/picker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,14 +461,17 @@ impl<T: Item + 'static> Picker<T> {

// 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");
Expand Down
9 changes: 7 additions & 2 deletions helix-term/src/ui/prompt.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -34,7 +35,7 @@ pub struct Prompt {
callback_fn: CallbackFn,
pub doc_fn: DocFn,
next_char_handler: Option<PromptCharHandler>,
language: Option<(&'static str, Arc<syntax::Loader>)>,
language: Option<(&'static str, Arc<ArcSwap<syntax::Loader>>)>,
}

#[derive(Clone, Copy, PartialEq, Eq)]
Expand Down Expand Up @@ -98,7 +99,11 @@ impl Prompt {
self
}

pub fn with_language(mut self, language: &'static str, loader: Arc<syntax::Loader>) -> Self {
pub fn with_language(
mut self,
language: &'static str,
loader: Arc<ArcSwap<syntax::Loader>>,
) -> Self {
self.language = Some((language, loader));
self
}
Expand Down
20 changes: 12 additions & 8 deletions helix-view/src/document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ impl Document {
pub fn open(
path: &Path,
encoding: Option<&'static Encoding>,
config_loader: Option<Arc<syntax::Loader>>,
config_loader: Option<Arc<ArcSwap<syntax::Loader>>>,
config: Arc<dyn DynAccess<Config>>,
) -> Result<Self, Error> {
// Open the file if it exists, otherwise assume it is a new file (and thus empty).
Expand Down Expand Up @@ -922,10 +922,11 @@ impl Document {
}

/// Detect the programming language based on the file type.
pub fn detect_language(&mut self, config_loader: Arc<syntax::Loader>) {
pub fn detect_language(&mut self, config_loader: Arc<ArcSwap<syntax::Loader>>) {
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)),
);
}

Expand Down Expand Up @@ -1059,10 +1060,12 @@ impl Document {
pub fn set_language(
&mut self,
language_config: Option<Arc<helix_core::syntax::LanguageConfiguration>>,
loader: Option<Arc<helix_core::syntax::Loader>>,
loader: Option<Arc<ArcSwap<helix_core::syntax::Loader>>>,
) {
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);
}

Expand All @@ -1078,9 +1081,10 @@ impl Document {
pub fn set_language_by_language_id(
&mut self,
language_id: &str,
config_loader: Arc<syntax::Loader>,
config_loader: Arc<ArcSwap<syntax::Loader>>,
) -> 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));
Expand Down
11 changes: 7 additions & 4 deletions helix-view/src/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Duration, D::Error>
where
Expand Down Expand Up @@ -918,7 +921,7 @@ pub struct Editor {
pub debugger_events: SelectAll<UnboundedReceiverStream<dap::Payload>>,
pub breakpoints: HashMap<PathBuf, Vec<Breakpoint>>,

pub syn_loader: Arc<syntax::Loader>,
pub syn_loader: Arc<ArcSwap<syntax::Loader>>,
pub theme_loader: Arc<theme::Loader>,
/// last_theme is used for theme previews. We store the current theme here,
/// and if previewing is cancelled, we can return to it.
Expand Down Expand Up @@ -1029,7 +1032,7 @@ impl Editor {
pub fn new(
mut area: Rect,
theme_loader: Arc<theme::Loader>,
syn_loader: Arc<syntax::Loader>,
syn_loader: Arc<ArcSwap<syntax::Loader>>,
config: Arc<dyn DynAccess<Config>>,
handlers: Handlers,
) -> Self {
Expand Down Expand Up @@ -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 => {
Expand Down

0 comments on commit 150b567

Please sign in to comment.