ruff server: Closing an untitled, unsaved notebook document no longer throws an error (#11942)

## Summary

Fixes #11651.
Fixes #11851.

We were double-closing a notebook document from the index, once in
`textDocument/didClose` and then in the `notebookDocument/didClose`
handler. The second time this happens, taking a snapshot fails.

I've rewritten how we handle snapshots for closing notebooks / notebook
cells so that any failure is simply logged instead of propagating
upwards. This implementation works consistently even if we don't receive
`textDocument/didClose` notifications for each specific cell, since they
get closed (and the diagnostics get cleared) in the notebook document
removal process.

## Test Plan

1. Open an untitled, unsaved notebook with the `Create: New Jupyter
Notebook` command from the VS Code command palette (`Ctrl/Cmd + Shift +
P`)
2. Without saving the document, close it.
3. No error popup should appear.
4. Run the debug command (`Ruff: print debug information`) to confirm
that there are no open documents
This commit is contained in:
Jane Lewis 2024-06-21 10:53:30 -07:00 committed by GitHub
parent 3d0230f469
commit 791f6a1820
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 11 additions and 20 deletions

View file

@ -21,15 +21,16 @@ impl super::SyncNotificationHandler for DidClose {
text_document: types::TextDocumentIdentifier { uri },
}: types::DidCloseTextDocumentParams,
) -> Result<()> {
let key = session.key_from_url(uri);
// Publish an empty diagnostic report for the document. This will de-register any existing diagnostics.
let snapshot = session
.take_snapshot(uri.clone())
.ok_or_else(|| anyhow::anyhow!("Unable to take snapshot for document with URL {uri}"))
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
let Some(snapshot) = session.take_snapshot(key.clone().into_url()) else {
tracing::debug!(
"Unable to close document with key {key} - the snapshot was unavailable"
);
return Ok(());
};
clear_diagnostics_for_document(snapshot.query(), &notifier)?;
let key = snapshot.query().make_key();
session
.close_document(&key)
.with_failure_code(lsp_server::ErrorCode::InternalError)

View file

@ -2,9 +2,8 @@ use crate::server::api::LSPResult;
use crate::server::client::{Notifier, Requester};
use crate::server::Result;
use crate::session::Session;
use lsp_server::ErrorCode;
use lsp_types as types;
use lsp_types::notification as notif;
use lsp_types::{self as types, NotebookDocumentIdentifier};
pub(crate) struct DidCloseNotebook;
@ -18,16 +17,14 @@ impl super::SyncNotificationHandler for DidCloseNotebook {
_notifier: Notifier,
_requester: &mut Requester,
types::DidCloseNotebookDocumentParams {
notebook_document: types::NotebookDocumentIdentifier { uri },
notebook_document: NotebookDocumentIdentifier { uri },
..
}: types::DidCloseNotebookDocumentParams,
) -> Result<()> {
let key = session.key_from_url(uri);
session
.close_document(&key)
.with_failure_code(ErrorCode::InternalError)?;
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
Ok(())
}
}

View file

@ -352,16 +352,9 @@ impl Index {
anyhow::bail!("Tried to close unavailable document `{key}`");
};
let Some(controller) = self.documents.remove(&url) else {
let Some(_) = self.documents.remove(&url) else {
anyhow::bail!("tried to close document that didn't exist at {}", url)
};
if let Some(notebook) = controller.as_notebook() {
for url in notebook.urls() {
self.notebook_cells.remove(url).ok_or_else(|| {
anyhow!("tried to de-register notebook cell with URL {url} that didn't exist")
})?;
}
}
Ok(())
}