From 7e1139de730675e0199f47e0ca05815464f480a7 Mon Sep 17 00:00:00 2001 From: Jonatan Waern Date: Wed, 9 Jul 2025 14:36:03 +0200 Subject: [PATCH 1/4] Make registrations respect config changes Signed-off-by: Jonatan Waern --- src/actions/mod.rs | 57 ++++++++++++++++++++++++++++++++++-- src/actions/notifications.rs | 14 +-------- src/actions/requests.rs | 10 +++++++ 3 files changed, 66 insertions(+), 15 deletions(-) diff --git a/src/actions/mod.rs b/src/actions/mod.rs index cd9180e..8d20519 100644 --- a/src/actions/mod.rs +++ b/src/actions/mod.rs @@ -16,6 +16,10 @@ use std::fs; use std::sync::atomic::AtomicBool; use std::sync::{Arc, Mutex}; +use lsp_types::notification::{DidChangeWatchedFiles}; +use lsp_types::request::{RegisterCapability, UnregisterCapability}; +use lsp_types::Unregistration; + use crate::actions::analysis_storage::AnalysisStorage; use crate::actions::analysis_queue::AnalysisQueue; use crate::actions::progress::{AnalysisProgressNotifier, @@ -32,6 +36,7 @@ use crate::lint::{LintCfg, maybe_parse_lint_cfg}; use crate::lsp_data; use crate::lsp_data::*; use crate::lsp_data::ls_util::{dls_to_range, dls_to_location}; + use crate::server::{Output, ServerToHandle, error_message, Request, RequestId, SentRequest}; use crate::server::message::RawResponse; @@ -255,6 +260,7 @@ pub struct InitActionContext { pub config: Arc>, pub lint_config: Arc>, + pub active_watch: Arc>>, pub sent_warnings: Arc>>, jobs: Arc>, pub client_capabilities: Arc, @@ -358,6 +364,7 @@ impl InitActionContext { quiescent: Arc::new(AtomicBool::new(false)), prev_changes: Arc::default(), client_capabilities: Arc::new(client_capabilities), + active_watch: Arc::default(), has_notified_missing_builtins: false, //client_supports_cmd_run, active_waits: Arc::default(), @@ -689,6 +696,49 @@ impl InitActionContext { self.report_errors(out); }, } + self.update_file_watchers(out); + } + + + const WATCH_ID: &str = "dls-watch"; + pub fn update_file_watchers(&self, out: &O) { + if self.active_watch.lock().unwrap().take().is_some() { + self.send_request::( + UnregistrationParams { + unregisterations: vec![Unregistration { + id: Self::WATCH_ID.to_string(), + method: ::METHOD + .to_string(), + }] + }, + out); + } else { + self.register_new_watchers(out); + } + } + + pub fn register_new_watchers(&self, out: &O) { + let mut watchers = self.active_watch.lock().unwrap(); + if let Some(previous) = watchers.take() { + error!("Wanted to register new watchers, but the previous ones \ + were not cleared. (were: {:?})", previous); + } + *watchers = FileWatch::new(self); + if let Some(watchers_spec) = watchers.as_ref() { + let reg_params = RegistrationParams { + registrations: vec![Registration { + id: Self::WATCH_ID.to_string(), + method: + ::METHOD.to_string(), + register_options: Some(watchers_spec.watchers_config()), + }], + }; + self.send_request::(reg_params, out); + } else { + error!("Failed to register file watchers with config: {:?}", + self.config.lock().unwrap()); + } } // Call before adding new analysis @@ -1323,6 +1373,7 @@ fn find_word_at_pos(line: &str, pos: Column) -> (Column, Column) { } // /// Client file-watching request / filtering logic +#[derive(Debug, Clone)] pub struct FileWatch { file_path: PathBuf, } @@ -1381,7 +1432,9 @@ impl FileWatch { let watchers = vec![watcher( self.file_path.to_string_lossy().to_string())]; - - json!({ "watchers": watchers }) + let watchers = DidChangeWatchedFilesRegistrationOptions { + watchers, + }; + json!(watchers) } } diff --git a/src/actions/notifications.rs b/src/actions/notifications.rs index 7dcb4e8..a924a8b 100644 --- a/src/actions/notifications.rs +++ b/src/actions/notifications.rs @@ -54,19 +54,7 @@ impl BlockingNotificationAction for Initialized { }; ctx.send_request::(reg_params, &out); } - - // Register files we watch for changes based on config - const WATCH_ID: &str = "dls-watch"; - let reg_params = RegistrationParams { - registrations: vec![Registration { - id: WATCH_ID.to_owned(), - method: - ::METHOD.to_owned(), - register_options: FileWatch::new(ctx).map( - |fw|fw.watchers_config()), - }], - }; - ctx.send_request::(reg_params, &out); + ctx.update_file_watchers(&out); Ok(()) } } diff --git a/src/actions/requests.rs b/src/actions/requests.rs index ff699cb..76c2695 100644 --- a/src/actions/requests.rs +++ b/src/actions/requests.rs @@ -36,6 +36,7 @@ pub use crate::lsp_data::request::{ RangeFormatting, References, RegisterCapability, + UnregisterCapability, Rename, ResolveCompletionItem as ResolveCompletion, WorkspaceConfiguration, @@ -999,6 +1000,15 @@ impl SentRequest for RegisterCapability { } } +impl SentRequest for UnregisterCapability { + type Response = ::Result; + fn on_response + (ctx: &InitActionContext, _response: Self::Response, out: &O) { + ctx.register_new_watchers(out) + } +} + + impl SentRequest for WorkspaceConfiguration { type Response = ::Result; fn on_response From 1c36564535b4c47c227642bfeede7f44d4f6065a Mon Sep 17 00:00:00 2001 From: Jonatan Waern Date: Wed, 9 Jul 2025 14:41:41 +0200 Subject: [PATCH 2/4] Add lint config to watched files Signed-off-by: Jonatan Waern --- src/actions/mod.rs | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/actions/mod.rs b/src/actions/mod.rs index 8d20519..3935293 100644 --- a/src/actions/mod.rs +++ b/src/actions/mod.rs @@ -1375,18 +1375,24 @@ fn find_word_at_pos(line: &str, pos: Column) -> (Column, Column) { // /// Client file-watching request / filtering logic #[derive(Debug, Clone)] pub struct FileWatch { - file_path: PathBuf, + file_paths: Vec, } impl FileWatch { /// Construct a new `FileWatch`. pub fn new(ctx: &InitActionContext) -> Option { + let mut file_paths = vec![]; match ctx.config.lock() { Ok(config) => { - config.compile_info_path.as_ref().map( - |c| FileWatch { - file_path: c.clone() - }) + if let Some(compile_info) = config.compile_info_path.as_ref() { + file_paths.push(compile_info.clone()); + } + if let Some(lint_cfg_path) = config.lint_cfg_path.as_ref() { + file_paths.push(lint_cfg_path.clone()); + } + Some(FileWatch { + file_paths, + }) }, Err(e) => { error!("Unable to access configuration: {:?}", e); @@ -1403,7 +1409,9 @@ impl FileWatch { fn relevant_change_kind(&self, change_uri: &Uri, _kind: FileChangeType) -> bool { let path = change_uri.as_str(); - self.file_path.to_str().map_or(false, |fp|fp == path) + self.file_paths.iter() + .filter_map(|p|p.to_str()) + .any(|our_path|our_path == path) } #[inline] @@ -1430,8 +1438,10 @@ impl FileWatch { kind: Some(kind) } } - let watchers = vec![watcher( - self.file_path.to_string_lossy().to_string())]; + let watchers: Vec<_> = self.file_paths.iter() + .map(|p|p.to_string_lossy().to_string()) + .map(watcher) + .collect(); let watchers = DidChangeWatchedFilesRegistrationOptions { watchers, }; From 09a197490577c6139c5036c7063504502dc4f961 Mon Sep 17 00:00:00 2001 From: Jonatan Waern Date: Wed, 9 Jul 2025 15:30:26 +0200 Subject: [PATCH 3/4] Watch files relative to working directory Signed-off-by: Jonatan Waern --- src/actions/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/actions/mod.rs b/src/actions/mod.rs index 3935293..6c9987e 100644 --- a/src/actions/mod.rs +++ b/src/actions/mod.rs @@ -1429,12 +1429,12 @@ impl FileWatch { /// Returns json config for desired file watches pub fn watchers_config(&self) -> serde_json::Value { fn watcher(pat: String) -> FileSystemWatcher { - FileSystemWatcher { glob_pattern: GlobPattern::String(pat), + FileSystemWatcher { glob_pattern: GlobPattern::Relative(pat), kind: None } } fn _watcher_with_kind(pat: String, kind: WatchKind) -> FileSystemWatcher { - FileSystemWatcher { glob_pattern: GlobPattern::String(pat), + FileSystemWatcher { glob_pattern: GlobPattern::Relative(pat), kind: Some(kind) } } From 6448d90e68cbfb86aed0f6e61807249c0690475c Mon Sep 17 00:00:00 2001 From: Jonatan Waern Date: Thu, 10 Jul 2025 11:29:50 +0200 Subject: [PATCH 4/4] Canonize watch paths before watching them The note on implementation details for relevant_change_kind are kind of outdated, we will only really be calling this on change notifications for specific files that rarely change. Signed-off-by: Jonatan Waern --- src/actions/mod.rs | 99 ++++++++++++++++++++++++++++++++++-------- src/file_management.rs | 3 ++ 2 files changed, 84 insertions(+), 18 deletions(-) diff --git a/src/actions/mod.rs b/src/actions/mod.rs index 6c9987e..b7ea51a 100644 --- a/src/actions/mod.rs +++ b/src/actions/mod.rs @@ -1372,26 +1372,90 @@ fn find_word_at_pos(line: &str, pos: Column) -> (Column, Column) { (span::Column::new_zero_indexed(start), span::Column::new_zero_indexed(end)) } +#[derive(Debug, Clone)] +pub struct FileWatchSpec { + full: CanonPath, + base: WorkspaceFolder, + relative: String, +} + // /// Client file-watching request / filtering logic #[derive(Debug, Clone)] pub struct FileWatch { - file_paths: Vec, + file_paths: Vec, } impl FileWatch { /// Construct a new `FileWatch`. pub fn new(ctx: &InitActionContext) -> Option { - let mut file_paths = vec![]; + let mut file_paths: HashMap = HashMap::default(); match ctx.config.lock() { Ok(config) => { if let Some(compile_info) = config.compile_info_path.as_ref() { - file_paths.push(compile_info.clone()); + if let Some(canon_path) = + CanonPath::from_path_buf(compile_info.clone()) { + file_paths.insert(canon_path, false); + } else { + error!("Could not watch compilation info {:?}, \ + not a canonizable path", compile_info); + } } if let Some(lint_cfg_path) = config.lint_cfg_path.as_ref() { - file_paths.push(lint_cfg_path.clone()); + if let Some(canon_path) = + CanonPath::from_path_buf(lint_cfg_path.clone()) { + file_paths.insert(canon_path, false); + } else { + error!("Could not watch lint config path {:?}, \ + not a canonizable path", lint_cfg_path); + } + } + fn path_to_relative(path: CanonPath, + roots: &Vec, + hit_paths: &mut HashMap) + -> Option> { + let mut globs = vec![]; + for root in roots { + let root_path = + parse_file_path!(&root.uri, "workspace").ok()?; + let root_canon_path = + CanonPath::from_path_buf(root_path)?; + info!("watch {:?} under {:?}", path, root); + if let Ok(relative_path) = path + .strip_prefix(root_canon_path.as_path()) { + hit_paths.insert(path.clone(), true); + globs.push( + FileWatchSpec { + full: root_canon_path, + base: root.clone(), + relative: relative_path + .to_string_lossy().to_string(), + } + ); + } + } + Some(globs) + } + + let watch_paths: Vec<_> = { + let lock_workspaces = ctx.workspace_roots.lock().unwrap(); + file_paths.keys().cloned() + .collect::>().into_iter() + .flat_map(|post_path|path_to_relative( + post_path, + &lock_workspaces, + &mut file_paths)) + .flatten() + .collect() + }; + for (path, watched) in &file_paths { + if !watched { + error!("Could not watch {:?}, not under any \ + workspace root", path); + } } Some(FileWatch { - file_paths, + file_paths: watch_paths, }) }, Err(e) => { @@ -1403,14 +1467,12 @@ impl FileWatch { /// Returns if a file change is relevant to the files we /// actually wanted to watch - /// Implementation note: This is expected to be called a - /// large number of times in a loop so should be fast / avoid allocation. #[inline] fn relevant_change_kind(&self, change_uri: &Uri, _kind: FileChangeType) -> bool { let path = change_uri.as_str(); self.file_paths.iter() - .filter_map(|p|p.to_str()) + .filter_map(|ws|ws.full.to_str()) .any(|our_path|our_path == path) } @@ -1428,19 +1490,20 @@ impl FileWatch { /// Returns json config for desired file watches pub fn watchers_config(&self) -> serde_json::Value { - fn watcher(pat: String) -> FileSystemWatcher { - FileSystemWatcher { glob_pattern: GlobPattern::Relative(pat), - kind: None } - } - fn _watcher_with_kind(pat: String, kind: WatchKind) - -> FileSystemWatcher { - FileSystemWatcher { glob_pattern: GlobPattern::Relative(pat), - kind: Some(kind) } + fn watcher(base: WorkspaceFolder, pat: String) -> FileSystemWatcher { + FileSystemWatcher { + glob_pattern: GlobPattern::Relative( + RelativePattern { + base_uri: OneOf::Left(base), + pattern: "./".to_string() + &pat, + }), + kind: Some(WatchKind::all()), + } } let watchers: Vec<_> = self.file_paths.iter() - .map(|p|p.to_string_lossy().to_string()) - .map(watcher) + .map(|ws|watcher(ws.base.clone(), + ws.relative.clone())) .collect(); let watchers = DidChangeWatchedFilesRegistrationOptions { watchers, diff --git a/src/file_management.rs b/src/file_management.rs index a8c52e1..05d3f5d 100644 --- a/src/file_management.rs +++ b/src/file_management.rs @@ -56,6 +56,9 @@ impl CanonPath { pub fn as_path(&self) -> &Path { self.0.as_path() } + pub fn as_path_buf(self) -> PathBuf { + self.0 + } } /// This is how we resolve relative paths to in-workspace full paths