refactor: simplify LSP implementation
All checks were successful
Continuous Integration / Lint, Check & Test (push) Successful in 1m34s
Continuous Integration / Build Package (push) Successful in 2m17s

- 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)
This commit is contained in:
Konstantin Fickel 2026-04-13 21:52:41 +02:00
parent b224620212
commit d0e4dccd1a
Signed by: kfickel
GPG key ID: A793722F9933C1A5

View file

@ -22,9 +22,7 @@ use crate::timesheet::{
BasicTimesheetConfiguration, BasicTimesheetConfiguration,
}; };
// --------------------------------------------------------------------------- const MD_EXT: &str = "md";
// Constants and helpers
// ---------------------------------------------------------------------------
/// R15 file name validation: YYYYMMDD[-HHMMSS][_type][...].md /// R15 file name validation: YYYYMMDD[-HHMMSS][_type][...].md
/// The extraction regex requires at least one non-dot char after date/time. /// The extraction regex requires at least one non-dot char after date/time.
@ -274,9 +272,31 @@ fn collect_workspace_symbols(
} }
} }
// --------------------------------------------------------------------------- /// Find all word-boundary-respecting occurrences of `pattern` in `lines`.
// LSP state /// 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 { struct LspState {
config: RepositoryConfiguration, config: RepositoryConfiguration,
@ -301,8 +321,11 @@ impl LspState {
} }
/// Parse a file from disk on demand (for cross-file features). /// Parse a file from disk on demand (for cross-file features).
fn parse_file_from_disk(&self, path: &std::path::Path) -> Option<LocalizedShard> { /// 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<LocalizedShard> {
let text = fs::read_to_string(path).ok()?; let text = fs::read_to_string(path).ok()?;
let lines: Vec<String> = text.lines().map(String::from).collect();
self.file_lines.insert(uri.clone(), lines);
let path_str = path.to_string_lossy(); let path_str = path.to_string_lossy();
let stream_file = parse_markdown_file(&path_str, &text); let stream_file = parse_markdown_file(&path_str, &text);
localize_stream_file(&stream_file, &self.config, self.tz).ok() localize_stream_file(&stream_file, &self.config, self.tz).ok()
@ -387,10 +410,6 @@ impl LspState {
} }
} }
// ---------------------------------------------------------------------------
// Backend
// ---------------------------------------------------------------------------
struct Backend { struct Backend {
client: Client, client: Client,
/// None → passive mode (no .streamd.toml found) /// None → passive mode (no .streamd.toml found)
@ -430,10 +449,6 @@ impl Backend {
} }
} }
// ---------------------------------------------------------------------------
// LanguageServer implementation
// ---------------------------------------------------------------------------
#[tower_lsp::async_trait] #[tower_lsp::async_trait]
impl LanguageServer for Backend { impl LanguageServer for Backend {
async fn initialize(&self, params: InitializeParams) -> Result<InitializeResult> { async fn initialize(&self, params: InitializeParams) -> Result<InitializeResult> {
@ -518,10 +533,6 @@ impl LanguageServer for Backend {
Ok(()) Ok(())
} }
// -----------------------------------------------------------------------
// Config watching
// -----------------------------------------------------------------------
async fn did_change_watched_files(&self, params: DidChangeWatchedFilesParams) { async fn did_change_watched_files(&self, params: DidChangeWatchedFilesParams) {
let has_config_change = params let has_config_change = params
.changes .changes
@ -557,10 +568,6 @@ impl LanguageServer for Backend {
} }
} }
// -----------------------------------------------------------------------
// Document lifecycle
// -----------------------------------------------------------------------
async fn did_open(&self, params: DidOpenTextDocumentParams) { async fn did_open(&self, params: DidOpenTextDocumentParams) {
let guard = self.state.read().await; let guard = self.state.read().await;
if let Some(state) = guard.as_ref() { if let Some(state) = guard.as_ref() {
@ -605,10 +612,6 @@ impl LanguageServer for Backend {
} }
} }
// -----------------------------------------------------------------------
// Completion
// -----------------------------------------------------------------------
async fn completion(&self, params: CompletionParams) -> Result<Option<CompletionResponse>> { async fn completion(&self, params: CompletionParams) -> Result<Option<CompletionResponse>> {
let guard = self.state.read().await; let guard = self.state.read().await;
let state = match guard.as_ref() { let state = match guard.as_ref() {
@ -635,10 +638,6 @@ impl LanguageServer for Backend {
)))) ))))
} }
// -----------------------------------------------------------------------
// Document symbols
// -----------------------------------------------------------------------
async fn document_symbol( async fn document_symbol(
&self, &self,
params: DocumentSymbolParams, params: DocumentSymbolParams,
@ -664,18 +663,7 @@ impl LanguageServer for Backend {
]))) ])))
} }
// -----------------------------------------------------------------------
// Code actions
// -----------------------------------------------------------------------
async fn code_action(&self, params: CodeActionParams) -> Result<Option<CodeActionResponse>> { async fn code_action(&self, params: CodeActionParams) -> Result<Option<CodeActionResponse>> {
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 guard = self.state.read().await;
let state = match guard.as_ref() { let state = match guard.as_ref() {
Some(s) => s, Some(s) => s,
@ -728,10 +716,6 @@ impl LanguageServer for Backend {
Ok(Some(actions)) Ok(Some(actions))
} }
// -----------------------------------------------------------------------
// Workspace symbols
// -----------------------------------------------------------------------
async fn symbol( async fn symbol(
&self, &self,
params: WorkspaceSymbolParams, params: WorkspaceSymbolParams,
@ -751,7 +735,7 @@ impl LanguageServer for Backend {
.filter_map(|e| e.ok()) .filter_map(|e| e.ok())
{ {
let path = entry.path(); let path = entry.path();
if path.extension().is_none_or(|e| e != "md") { if path.extension().is_none_or(|e| e != MD_EXT) {
continue; continue;
} }
let uri = match Url::from_file_path(path) { 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) { let shard = if let Some(cached) = state.file_cache.get(&uri) {
cached.clone() cached.clone()
} else { } 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()); state.file_cache.insert(uri.clone(), parsed.clone());
parsed parsed
}; };
@ -775,10 +759,6 @@ impl LanguageServer for Backend {
Ok(Some(symbols)) Ok(Some(symbols))
} }
// -----------------------------------------------------------------------
// References
// -----------------------------------------------------------------------
async fn references(&self, params: ReferenceParams) -> Result<Option<Vec<Location>>> { async fn references(&self, params: ReferenceParams) -> Result<Option<Vec<Location>>> {
let guard = self.state.read().await; let guard = self.state.read().await;
let state = match guard.as_ref() { let state = match guard.as_ref() {
@ -815,7 +795,7 @@ impl LanguageServer for Backend {
.filter_map(|e| e.ok()) .filter_map(|e| e.ok())
{ {
let path = entry.path(); let path = entry.path();
if path.extension().is_none_or(|e| e != "md") { if path.extension().is_none_or(|e| e != MD_EXT) {
continue; continue;
} }
let file_uri = match Url::from_file_path(path) { let file_uri = match Url::from_file_path(path) {
@ -827,15 +807,7 @@ impl LanguageServer for Backend {
None => continue, None => continue,
}; };
for (line_idx, line) in lines.iter().enumerate() { for (line_idx, abs, after) in find_pattern_occurrences_in_lines(&lines, &pattern) {
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 { locations.push(Location {
uri: file_uri.clone(), uri: file_uri.clone(),
range: Range { range: Range {
@ -844,21 +816,11 @@ impl LanguageServer for Backend {
}, },
}); });
} }
start = abs + pattern.len();
if start >= line.len() {
break;
}
}
}
} }
Ok(Some(locations)) Ok(Some(locations))
} }
// -----------------------------------------------------------------------
// Rename
// -----------------------------------------------------------------------
async fn rename(&self, params: RenameParams) -> Result<Option<WorkspaceEdit>> { async fn rename(&self, params: RenameParams) -> Result<Option<WorkspaceEdit>> {
let guard = self.state.read().await; let guard = self.state.read().await;
let state = match guard.as_ref() { let state = match guard.as_ref() {
@ -899,7 +861,7 @@ impl LanguageServer for Backend {
.filter_map(|e| e.ok()) .filter_map(|e| e.ok())
{ {
let path = entry.path(); let path = entry.path();
if path.extension().is_none_or(|e| e != "md") { if path.extension().is_none_or(|e| e != MD_EXT) {
continue; continue;
} }
let file_uri = match Url::from_file_path(path) { let file_uri = match Url::from_file_path(path) {
@ -911,29 +873,16 @@ impl LanguageServer for Backend {
None => continue, None => continue,
}; };
let mut file_edits: Vec<TextEdit> = Vec::new(); let file_edits: Vec<TextEdit> = find_pattern_occurrences_in_lines(&lines, &pattern)
for (line_idx, line) in lines.iter().enumerate() { .into_iter()
let mut start = 0; .map(|(line_idx, abs, after)| TextEdit {
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 { range: Range {
start: Position::new(line_idx as u32, abs as u32), start: Position::new(line_idx as u32, abs as u32),
end: Position::new(line_idx as u32, after as u32), end: Position::new(line_idx as u32, after as u32),
}, },
new_text: replacement.clone(), new_text: replacement.clone(),
}); })
} .collect();
start = abs + pattern.len();
if start >= line.len() {
break;
}
}
}
if !file_edits.is_empty() { if !file_edits.is_empty() {
changes.insert(file_uri, file_edits); changes.insert(file_uri, file_edits);
} }
@ -946,10 +895,6 @@ impl LanguageServer for Backend {
} }
} }
// ---------------------------------------------------------------------------
// Entry point
// ---------------------------------------------------------------------------
/// Run the LSP server over stdin/stdout. /// Run the LSP server over stdin/stdout.
pub fn run() -> std::result::Result<(), StreamdError> { pub fn run() -> std::result::Result<(), StreamdError> {
let rt = tokio::runtime::Runtime::new().map_err(StreamdError::IoError)?; let rt = tokio::runtime::Runtime::new().map_err(StreamdError::IoError)?;
@ -962,10 +907,6 @@ pub fn run() -> std::result::Result<(), StreamdError> {
Ok(()) Ok(())
} }
// ---------------------------------------------------------------------------
// Tests
// ---------------------------------------------------------------------------
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
@ -991,8 +932,6 @@ mod tests {
.with_marker("Break", Marker::new("Break").with_placements(vec![])) .with_marker("Break", Marker::new("Break").with_placements(vec![]))
} }
// --- File name diagnostic ---
#[test] #[test]
fn test_valid_file_name_no_diagnostic() { fn test_valid_file_name_no_diagnostic() {
assert!(compute_file_name_diagnostic("20260413-120000_daily.md").is_none()); 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()); assert!(compute_file_name_diagnostic("notes.txt").is_none());
} }
// --- Marker extraction ---
#[test] #[test]
fn test_extract_markers_from_line() { fn test_extract_markers_from_line() {
let markers = extract_markers_from_line("@Task @Done some content @Tag"); 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()); assert!(extract_markers_from_line("no markers here").is_empty());
} }
// --- Marker at position ---
#[test] #[test]
fn test_extract_marker_at_position_on_marker() { fn test_extract_marker_at_position_on_marker() {
let line = "some @Task content"; let line = "some @Task content";
@ -1055,8 +990,6 @@ mod tests {
assert!(extract_marker_at_position(line, 11).is_none()); assert!(extract_marker_at_position(line, 11).is_none());
} }
// --- Completions ---
#[test] #[test]
fn test_completions_no_at_sign_returns_empty() { fn test_completions_no_at_sign_returns_empty() {
let config = make_config(); let config = make_config();