Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

Commit

Permalink
fix(diagnostics): run upon change, with a hack for throttling
Browse files Browse the repository at this point in the history
  • Loading branch information
jokeyrhyme committed Oct 7, 2023
1 parent bb0cd98 commit e554717
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 8 deletions.
7 changes: 1 addition & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,8 @@ Language Server Protocol implementation for nushell
- [x] [textDocument/didChange](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_didChange),
[textDocument/didClose](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_didClose),
and [textDocument/didOpen](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_didOpen)
- [x] [textDocument/publishDiagnostics](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_publishDiagnostics) -> `nu --ide-check`
- [ ] [textDocument/inlayHint](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_inlayHint) -> `nu --ide-check`
- [ ] [textDocument/publishDiagnostics](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_publishDiagnostics) -> `nu --ide-check`

- [x] triggered once per [textDocument/didOpen](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_synchronization)
- [ ] triggered every [textDocument/didChange](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_didChange)
- [ ] debounced/throttled to avoid performance issues

- [ ] [textDocument/diagnostic](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_pullDiagnostics) -> `nu --ide-check`
- [ ] [textDocument/formatting](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_formatting) -> [`nufmt`](https://github.com/nushell/nufmt)
- [ ] [workspace/configuration](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#workspace_configuration)
Expand Down
8 changes: 6 additions & 2 deletions src/backend/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,17 @@ use tower_lsp::LanguageServer;
#[tower_lsp::async_trait]
impl LanguageServer for Backend {
async fn did_change(&self, params: DidChangeTextDocumentParams) {
let uri = params.text_document.uri.clone();
if let Err(e) = self.try_did_change(params) {
self.client
.log_message(MessageType::ERROR, format!("{e:?}"))
.await;
}
// TODO: trigger debounced `nu --ide-check`
if let Err(e) = self.throttled_validate_document(&uri).await {
self.client
.log_message(MessageType::ERROR, format!("{e:?}"))
.await;
};
}

async fn did_change_workspace_folders(&self, params: DidChangeWorkspaceFoldersParams) {
Expand Down Expand Up @@ -49,7 +54,6 @@ impl LanguageServer for Backend {
.log_message(MessageType::ERROR, format!("{e:?}"))
.await;
}
// TODO: trigger debounced `nu --ide-check` instead
if let Err(e) = self.validate_document(&uri).await {
self.client
.log_message(MessageType::ERROR, format!("{e:?}"))
Expand Down
24 changes: 24 additions & 0 deletions src/backend/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::sync::OnceLock;
use std::time::{Duration, Instant};
use std::{ffi::OsStr, sync::RwLock};

pub(crate) mod language_server;
Expand All @@ -21,6 +22,7 @@ pub(crate) struct Backend {
client: Client,
documents: RwLock<TextDocuments>,
ide_settings: RwLock<IdeSettings>,
last_validated: RwLock<Instant>,
}

impl Backend {
Expand Down Expand Up @@ -53,9 +55,31 @@ impl Backend {
client,
documents: RwLock::new(TextDocuments::new()),
ide_settings: RwLock::new(IdeSettings::default()),
last_validated: RwLock::new(Instant::now()),
}
}

async fn throttled_validate_document(&self, uri: &Url) -> Result<()> {
// TODO: this is a quick imperfect hack, but eventually we probably want a thorough solution using threads/channels?
// TODO: ensure that we validate at least once after the most recent throttling (i.e. debounce instead of throttle)
let then = {
*self.last_validated.read().map_err(|e| {
map_err_to_internal_error(&e, format!("cannot read throttling marker: {e:?}"))
})?
};
if then.elapsed() < Duration::from_millis(500) {
return Ok(());
}

self.validate_document(uri).await?;

let mut then = self.last_validated.write().map_err(|e| {
map_err_to_internal_error(&e, format!("cannot write throttling marker: {e:?}"))
})?;
*then = Instant::now();
Ok(())
}

fn try_did_change(&self, params: DidChangeTextDocumentParams) -> Result<()> {
let mut documents = self.documents.write().map_err(|e| {
map_err_to_internal_error(&e, format!("cannot write to document cache: {e:?}"))
Expand Down

0 comments on commit e554717

Please sign in to comment.