From 5c6a88a2117ef8f2db150ebfd1eea7aab4d38f73 Mon Sep 17 00:00:00 2001 From: Konstantin Fickel Date: Mon, 13 Apr 2026 21:52:41 +0200 Subject: [PATCH] refactor: simplify LSP implementation - Extract find_pattern_occurrences_in_lines helper; use in both references and rename handlers (deduplicates word-boundary search logic) - Add MD_EXT constant to replace three repeated "md" string literals - Fix double lock acquire/drop/re-acquire in code_action (use single guard) - Fix symbol() to populate file_lines cache when parsing from disk (avoids redundant re-reads on subsequent requests) - Remove section separator comment blocks (code structure is self-evident from Rust syntax) --- src/cli/commands/lsp.rs | 171 ++++++++++++---------------------------- 1 file changed, 52 insertions(+), 119 deletions(-) diff --git a/src/cli/commands/lsp.rs b/src/cli/commands/lsp.rs index 378e7f9..99e221d 100644 --- a/src/cli/commands/lsp.rs +++ b/src/cli/commands/lsp.rs @@ -22,9 +22,7 @@ use crate::timesheet::{ BasicTimesheetConfiguration, }; -// --------------------------------------------------------------------------- -// Constants and helpers -// --------------------------------------------------------------------------- +const MD_EXT: &str = "md"; /// R15 file name validation: YYYYMMDD[-HHMMSS][_type][...].md /// The extraction regex requires at least one non-dot char after date/time. @@ -274,9 +272,31 @@ fn collect_workspace_symbols( } } -// --------------------------------------------------------------------------- -// LSP state -// --------------------------------------------------------------------------- +/// Find all word-boundary-respecting occurrences of `pattern` in `lines`. +/// Returns `(line_idx, start_col, end_col)` for each match. +fn find_pattern_occurrences_in_lines( + lines: &[String], + pattern: &str, +) -> Vec<(usize, usize, usize)> { + let mut results = Vec::new(); + for (line_idx, line) in lines.iter().enumerate() { + let mut start = 0; + while let Some(pos) = line[start..].find(pattern) { + let abs = start + pos; + let after = abs + pattern.len(); + let is_word_boundary = + after >= line.len() || !line.as_bytes()[after].is_ascii_alphanumeric(); + if is_word_boundary { + results.push((line_idx, abs, after)); + } + start = abs + pattern.len(); + if start >= line.len() { + break; + } + } + } + results +} struct LspState { config: RepositoryConfiguration, @@ -301,8 +321,11 @@ impl LspState { } /// Parse a file from disk on demand (for cross-file features). - fn parse_file_from_disk(&self, path: &std::path::Path) -> Option { + /// Also populates `file_lines` so subsequent line-based requests avoid a re-read. + fn parse_file_from_disk(&self, uri: &Url, path: &std::path::Path) -> Option { let text = fs::read_to_string(path).ok()?; + let lines: Vec = text.lines().map(String::from).collect(); + self.file_lines.insert(uri.clone(), lines); let path_str = path.to_string_lossy(); let stream_file = parse_markdown_file(&path_str, &text); localize_stream_file(&stream_file, &self.config, self.tz).ok() @@ -387,10 +410,6 @@ impl LspState { } } -// --------------------------------------------------------------------------- -// Backend -// --------------------------------------------------------------------------- - struct Backend { client: Client, /// None → passive mode (no .streamd.toml found) @@ -430,10 +449,6 @@ impl Backend { } } -// --------------------------------------------------------------------------- -// LanguageServer implementation -// --------------------------------------------------------------------------- - #[tower_lsp::async_trait] impl LanguageServer for Backend { async fn initialize(&self, params: InitializeParams) -> Result { @@ -518,10 +533,6 @@ impl LanguageServer for Backend { Ok(()) } - // ----------------------------------------------------------------------- - // Config watching - // ----------------------------------------------------------------------- - async fn did_change_watched_files(&self, params: DidChangeWatchedFilesParams) { let has_config_change = params .changes @@ -557,10 +568,6 @@ impl LanguageServer for Backend { } } - // ----------------------------------------------------------------------- - // Document lifecycle - // ----------------------------------------------------------------------- - async fn did_open(&self, params: DidOpenTextDocumentParams) { let guard = self.state.read().await; if let Some(state) = guard.as_ref() { @@ -605,10 +612,6 @@ impl LanguageServer for Backend { } } - // ----------------------------------------------------------------------- - // Completion - // ----------------------------------------------------------------------- - async fn completion(&self, params: CompletionParams) -> Result> { let guard = self.state.read().await; let state = match guard.as_ref() { @@ -635,10 +638,6 @@ impl LanguageServer for Backend { )))) } - // ----------------------------------------------------------------------- - // Document symbols - // ----------------------------------------------------------------------- - async fn document_symbol( &self, params: DocumentSymbolParams, @@ -664,18 +663,7 @@ impl LanguageServer for Backend { ]))) } - // ----------------------------------------------------------------------- - // Code actions - // ----------------------------------------------------------------------- - async fn code_action(&self, params: CodeActionParams) -> Result> { - let guard = self.state.read().await; - if guard.is_none() { - return Ok(None); - } - // Drop the guard before the borrow ends — we only needed it to check active mode. - drop(guard); - let guard = self.state.read().await; let state = match guard.as_ref() { Some(s) => s, @@ -728,10 +716,6 @@ impl LanguageServer for Backend { Ok(Some(actions)) } - // ----------------------------------------------------------------------- - // Workspace symbols - // ----------------------------------------------------------------------- - async fn symbol( &self, params: WorkspaceSymbolParams, @@ -751,7 +735,7 @@ impl LanguageServer for Backend { .filter_map(|e| e.ok()) { let path = entry.path(); - if path.extension().is_none_or(|e| e != "md") { + if path.extension().is_none_or(|e| e != MD_EXT) { continue; } let uri = match Url::from_file_path(path) { @@ -762,7 +746,7 @@ impl LanguageServer for Backend { let shard = if let Some(cached) = state.file_cache.get(&uri) { cached.clone() } else { - let parsed = state.parse_file_from_disk(path); + let parsed = state.parse_file_from_disk(&uri, path); state.file_cache.insert(uri.clone(), parsed.clone()); parsed }; @@ -775,10 +759,6 @@ impl LanguageServer for Backend { Ok(Some(symbols)) } - // ----------------------------------------------------------------------- - // References - // ----------------------------------------------------------------------- - async fn references(&self, params: ReferenceParams) -> Result>> { let guard = self.state.read().await; let state = match guard.as_ref() { @@ -815,7 +795,7 @@ impl LanguageServer for Backend { .filter_map(|e| e.ok()) { let path = entry.path(); - if path.extension().is_none_or(|e| e != "md") { + if path.extension().is_none_or(|e| e != MD_EXT) { continue; } let file_uri = match Url::from_file_path(path) { @@ -827,38 +807,20 @@ impl LanguageServer for Backend { None => continue, }; - for (line_idx, line) in lines.iter().enumerate() { - let mut start = 0; - while let Some(pos) = line[start..].find(&pattern) { - let abs = start + pos; - let after = abs + pattern.len(); - // Ensure not part of a longer marker name. - let is_word_boundary = - after >= line.len() || !line.as_bytes()[after].is_ascii_alphanumeric(); - if is_word_boundary { - locations.push(Location { - uri: file_uri.clone(), - range: Range { - start: Position::new(line_idx as u32, abs as u32), - end: Position::new(line_idx as u32, after as u32), - }, - }); - } - start = abs + pattern.len(); - if start >= line.len() { - break; - } - } + for (line_idx, abs, after) in find_pattern_occurrences_in_lines(&lines, &pattern) { + locations.push(Location { + uri: file_uri.clone(), + range: Range { + start: Position::new(line_idx as u32, abs as u32), + end: Position::new(line_idx as u32, after as u32), + }, + }); } } Ok(Some(locations)) } - // ----------------------------------------------------------------------- - // Rename - // ----------------------------------------------------------------------- - async fn rename(&self, params: RenameParams) -> Result> { let guard = self.state.read().await; let state = match guard.as_ref() { @@ -899,7 +861,7 @@ impl LanguageServer for Backend { .filter_map(|e| e.ok()) { let path = entry.path(); - if path.extension().is_none_or(|e| e != "md") { + if path.extension().is_none_or(|e| e != MD_EXT) { continue; } let file_uri = match Url::from_file_path(path) { @@ -911,29 +873,16 @@ impl LanguageServer for Backend { None => continue, }; - let mut file_edits: Vec = Vec::new(); - for (line_idx, line) in lines.iter().enumerate() { - let mut start = 0; - while let Some(pos) = line[start..].find(&pattern) { - let abs = start + pos; - let after = abs + pattern.len(); - let is_word_boundary = - after >= line.len() || !line.as_bytes()[after].is_ascii_alphanumeric(); - if is_word_boundary { - file_edits.push(TextEdit { - range: Range { - start: Position::new(line_idx as u32, abs as u32), - end: Position::new(line_idx as u32, after as u32), - }, - new_text: replacement.clone(), - }); - } - start = abs + pattern.len(); - if start >= line.len() { - break; - } - } - } + let file_edits: Vec = find_pattern_occurrences_in_lines(&lines, &pattern) + .into_iter() + .map(|(line_idx, abs, after)| TextEdit { + range: Range { + start: Position::new(line_idx as u32, abs as u32), + end: Position::new(line_idx as u32, after as u32), + }, + new_text: replacement.clone(), + }) + .collect(); if !file_edits.is_empty() { changes.insert(file_uri, file_edits); } @@ -946,10 +895,6 @@ impl LanguageServer for Backend { } } -// --------------------------------------------------------------------------- -// Entry point -// --------------------------------------------------------------------------- - /// Run the LSP server over stdin/stdout. pub fn run() -> std::result::Result<(), StreamdError> { let rt = tokio::runtime::Runtime::new().map_err(StreamdError::IoError)?; @@ -962,10 +907,6 @@ pub fn run() -> std::result::Result<(), StreamdError> { Ok(()) } -// --------------------------------------------------------------------------- -// Tests -// --------------------------------------------------------------------------- - #[cfg(test)] mod tests { use super::*; @@ -991,8 +932,6 @@ mod tests { .with_marker("Break", Marker::new("Break").with_placements(vec![])) } - // --- File name diagnostic --- - #[test] fn test_valid_file_name_no_diagnostic() { assert!(compute_file_name_diagnostic("20260413-120000_daily.md").is_none()); @@ -1013,8 +952,6 @@ mod tests { assert!(compute_file_name_diagnostic("notes.txt").is_none()); } - // --- Marker extraction --- - #[test] fn test_extract_markers_from_line() { let markers = extract_markers_from_line("@Task @Done some content @Tag"); @@ -1026,8 +963,6 @@ mod tests { assert!(extract_markers_from_line("no markers here").is_empty()); } - // --- Marker at position --- - #[test] fn test_extract_marker_at_position_on_marker() { let line = "some @Task content"; @@ -1055,8 +990,6 @@ mod tests { assert!(extract_marker_at_position(line, 11).is_none()); } - // --- Completions --- - #[test] fn test_completions_no_at_sign_returns_empty() { let config = make_config();