From 403130f81f1ee6f42c250abd641ab1d9c7f5e02f Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Thu, 12 Sep 2024 03:41:39 +0100 Subject: [PATCH] implement cell formatting --- cli/lsp/language_server.rs | 97 +++++++++++++++++---------- cli/lsp/urls.rs | 131 ++++++++++++++++++++++++++----------- 2 files changed, 157 insertions(+), 71 deletions(-) diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index d4ee2df446..e4f926cf23 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -104,8 +104,9 @@ use crate::http_util::HttpClientProvider; use crate::lsp::config::ConfigWatchedFileType; use crate::lsp::logging::init_log_file; use crate::lsp::tsc::file_text_changes_to_workspace_edit; +use crate::lsp::urls::file_like_to_file_specifier; use crate::lsp::urls::LspUrlKind; -use crate::lsp::urls::NotebookCellInfo; +use crate::lsp::urls::NotebookScriptCellInfo; use crate::tools::fmt::format_file; use crate::tools::fmt::format_parsed_source; use crate::tools::upgrade::check_for_upgrades_for_lsp; @@ -1445,26 +1446,25 @@ impl Inner { item .collect_document_symbols(line_index.clone(), &mut document_symbols); } - if let Some(notebook_cell_info) = mapped_specifier.notebook_cell_info() { + if let Some(cell_info) = mapped_specifier.notebook_script_cell_info() { fn map_to_cell_ranges( mut symbol: DocumentSymbol, - notebook_cell_info: &NotebookCellInfo, + cell_info: &NotebookScriptCellInfo, ) -> Option { - symbol.range = - notebook_cell_info.range_server_to_client(symbol.range)?; - symbol.selection_range = notebook_cell_info - .range_server_to_client(symbol.selection_range)?; + symbol.range = cell_info.range_server_to_client(symbol.range)?; + symbol.selection_range = + cell_info.range_server_to_client(symbol.selection_range)?; symbol.children = symbol.children.map(|children| { children .into_iter() - .filter_map(|s| map_to_cell_ranges(s, notebook_cell_info)) + .filter_map(|s| map_to_cell_ranges(s, cell_info)) .collect() }); Some(symbol) } document_symbols = document_symbols .into_iter() - .filter_map(|s| map_to_cell_ranges(s, notebook_cell_info)) + .filter_map(|s| map_to_cell_ranges(s, cell_info)) .collect(); } Some(DocumentSymbolResponse::Nested(document_symbols)) @@ -1479,12 +1479,48 @@ impl Inner { &self, params: DocumentFormattingParams, ) -> LspResult>> { - let file_referrer = Some(uri_to_url(¶ms.text_document.uri)) - .filter(|s| self.documents.is_valid_file_referrer(s)); - let mut specifier = self - .url_map - .uri_to_specifier(¶ms.text_document.uri, LspUrlKind::File); - // skip formatting any files ignored by the config file + dbg!(params.text_document.uri.as_str()); + let raw_url = uri_to_url(¶ms.text_document.uri); + let mut specifier; + let language_id; + let content; + let parsed_source; + let line_index; + let is_notebook_cell; + if let Some(notebook_cell) = + self.url_map.notebook_cell(¶ms.text_document.uri) + { + match file_like_to_file_specifier(&raw_url) { + Some(file_specifier) => { + specifier = file_specifier; + } + None => { + specifier = raw_url.clone(); + } + } + language_id = notebook_cell.language_id.parse().ok(); + content = notebook_cell.text.clone(); + parsed_source = None; + line_index = notebook_cell.line_index.clone(); + is_notebook_cell = true; + } else { + let file_referrer = + Some(&raw_url).filter(|s| self.documents.is_valid_file_referrer(s)); + specifier = self + .url_map + .uri_to_specifier2(¶ms.text_document.uri, LspUrlKind::File) + .into_specifier(); + let Some(document) = + self.documents.get_or_load(&specifier, file_referrer) + else { + return Ok(None); + }; + content = document.content().clone(); + language_id = document.maybe_language_id(); + parsed_source = document.maybe_parsed_source().cloned(); + line_index = document.line_index(); + is_notebook_cell = false; + } if !self .config .tree @@ -1494,19 +1530,13 @@ impl Inner { { return Ok(None); } - let document = self - .documents - .get_or_load(&specifier, file_referrer.as_ref()); - let Some(document) = document else { - return Ok(None); - }; // Detect vendored paths. Vendor file URLs will normalize to their remote // counterparts, but for formatting we want to favour the file URL. // TODO(nayeemrmn): Implement `Document::file_resource_path()` or similar. if specifier.scheme() != "file" && params.text_document.uri.scheme().map(|s| s.as_str()) == Some("file") { - specifier = uri_to_url(¶ms.text_document.uri); + specifier = raw_url; } let file_path = specifier_to_file_path(&specifier).map_err(|err| { error!("{:#}", err); @@ -1543,37 +1573,38 @@ impl Inner { .map(|w| w.has_unstable("fmt-yaml")) .unwrap_or(false), }; - let document = document.clone(); move || { - let format_result = match document.maybe_parsed_source() { + let format_result = match parsed_source { Some(Ok(parsed_source)) => { - format_parsed_source(parsed_source, &fmt_options) + format_parsed_source(&parsed_source, &fmt_options) } Some(Err(err)) => Err(anyhow!("{:#}", err)), None => { // the file path is only used to determine what formatter should // be used to format the file, so give the filepath an extension // that matches what the user selected as the language - let file_path = document - .maybe_language_id() + let file_path = language_id .and_then(|id| id.as_extension()) .map(|ext| file_path.with_extension(ext)) .unwrap_or(file_path); // it's not a js/ts file, so attempt to format its contents format_file( &file_path, - document.content(), + content.as_ref(), &fmt_options, &unstable_options, ) } }; match format_result { - Ok(Some(new_text)) => Some(text::get_edits( - document.content(), - &new_text, - document.line_index().as_ref(), - )), + Ok(Some(new_text)) => { + let text = if is_notebook_cell { + new_text.trim_end_matches('\n') + } else { + new_text.as_str() + }; + Some(text::get_edits(content.as_ref(), text, line_index.as_ref())) + } Ok(None) => Some(Vec::new()), Err(err) => { lsp_warn!("Format error: {:#}", err); diff --git a/cli/lsp/urls.rs b/cli/lsp/urls.rs index d6e0099f87..0a1d34b00a 100644 --- a/cli/lsp/urls.rs +++ b/cli/lsp/urls.rs @@ -126,15 +126,32 @@ fn from_deno_url(url: &Url) -> Option { Url::parse(&string).ok() } -#[derive(Debug)] -struct NotebookCell { - item: lsp_types::TextDocumentItem, +#[derive(Debug, Clone)] +pub struct NotebookCell { + pub uri: Uri, + pub language_id: String, + pub version: i32, + pub text: Arc, + pub line_index: Arc, +} + +impl NotebookCell { + fn new(item: lsp_types::TextDocumentItem) -> Self { + let line_index = Arc::new(LineIndex::new(&item.text)); + Self { + uri: item.uri, + language_id: item.language_id, + version: item.version, + text: item.text.into(), + line_index, + } + } } #[derive(Debug)] struct Notebook { uri: Uri, - cells: IndexMap, + cells: IndexMap>, version: i32, script_language_id: Option, script_cells: IndexMap, @@ -143,7 +160,7 @@ struct Notebook { impl Notebook { fn new( uri: Uri, - cells: IndexMap, + cells: IndexMap>, version: i32, ) -> Self { static SCRIPT_LANGUAGE_IDS: &[&str] = &[ @@ -154,26 +171,26 @@ impl Notebook { "typescriptreact", "tsx", ]; - let script_language_id = cells.values().find_map(|i| { + let script_language_id = cells.values().find_map(|c| { SCRIPT_LANGUAGE_IDS - .contains(&i.language_id.as_str()) - .then(|| i.language_id.clone()) + .contains(&c.language_id.as_str()) + .then(|| c.language_id.clone()) }); let mut script_cells = IndexMap::new(); let mut script_line_offset = 0; if let Some(language_id) = &script_language_id { - for item in cells.values() { - if &item.language_id != language_id { + for cell in cells.values() { + if &cell.language_id != language_id { continue; } let line_count = - item.text.chars().filter(|c| *c == '\n').count() as u32; + cell.text.chars().filter(|c| *c == '\n').count() as u32; let cell_info = NotebookScriptCellInfo { line_offset: script_line_offset, line_count, }; script_line_offset += line_count; - script_cells.insert(item.uri.clone(), cell_info); + script_cells.insert(cell.uri.clone(), cell_info); } } Self { @@ -198,36 +215,50 @@ impl Notebook { cells.retain(|i, _| !closed_cells.contains(i)); } if let Some(did_open) = structure.did_open { - cells.extend(did_open.into_iter().map(|i| (i.uri.clone(), i))); + cells.extend( + did_open + .into_iter() + .map(|i| (i.uri.clone(), Arc::new(NotebookCell::new(i)))), + ); } } if let Some(changes) = cell_change.text_content { for change in changes { - let Some(item) = - cells.values_mut().find(|i| &i.uri == &change.document.uri) + let Some(cell) = + cells.values_mut().find(|c| c.uri == change.document.uri) else { continue; }; - item.version = change.document.version; - let mut content = item.text.clone(); - let mut line_index = LineIndex::new(&item.text); + let mut text = cell.text.to_string(); + let mut line_index = cell.line_index.clone(); let mut index_valid = IndexValid::All; for change in change.changes { if let Some(range) = change.range { if !index_valid.covers(range.start.line) { - line_index = LineIndex::new(&content); + line_index = Arc::new(LineIndex::new(&text)); } index_valid = IndexValid::UpTo(range.start.line); let Ok(range) = line_index.get_text_range(range) else { continue; }; - content.replace_range(Range::::from(range), &change.text); + text.replace_range(Range::::from(range), &change.text); } else { - content = change.text; + text = change.text; index_valid = IndexValid::UpTo(0); } } - item.text = content; + let line_index = if index_valid == IndexValid::All { + line_index + } else { + Arc::new(LineIndex::new(text.as_str())) + }; + *cell = Arc::new(NotebookCell { + uri: cell.uri.clone(), + language_id: cell.language_id.clone(), + version: cell.version, + text: text.into(), + line_index, + }); } } } @@ -238,8 +269,7 @@ impl Notebook { let text = self .script_cells .iter() - .map(|(u, _)| [self.cells.get(u).unwrap().text.as_str(), "\n"]) - .flatten() + .flat_map(|(u, _)| [self.cells.get(u).unwrap().text.as_ref(), "\n"]) .collect::>() .join(""); dbg!(self.uri.as_str(), &text); @@ -313,7 +343,7 @@ pub fn uri_to_url(uri: &Uri) -> Url { Url::parse(uri.as_str()).unwrap() } -#[derive(Debug, Copy)] +#[derive(Debug, Copy, Clone)] pub struct NotebookScriptCellInfo { pub line_offset: u32, pub line_count: u32, @@ -352,7 +382,7 @@ impl MappedSpecifier { } } - pub fn notebook_cell_info(&self) -> Option<&NotebookScriptCellInfo> { + pub fn notebook_script_cell_info(&self) -> Option<&NotebookScriptCellInfo> { match self { Self::NotebookScript(_, i) => Some(i), _ => None, @@ -470,7 +500,7 @@ impl LspUrlMap { kind: LspUrlKind, ) -> MappedSpecifier { let notebook_script = (|| { - let mut inner = self.inner.lock(); + let inner = self.inner.lock(); let notebook_uri = inner.script_cell_to_notebook_uri.get(uri)?; let notebook = inner.notebooks.get(notebook_uri)?; let cell_info = *notebook.script_cells.get(uri)?; @@ -486,13 +516,35 @@ impl LspUrlMap { pub fn specifier_to_uri2( &self, specifier: &ModuleSpecifier, - _line: Option, + line: Option, file_referrer: Option<&ModuleSpecifier>, ) -> Result<(Uri, Option), AnyError> { - // TODO(nayeemrmn): Implement! - self - .specifier_to_uri(specifier, file_referrer) - .map(|s| (s, None)) + let uri = self.specifier_to_uri(specifier, file_referrer)?; + let cell_entry = (|| { + let line = line?; + let inner = self.inner.lock(); + let notebook = inner.notebooks.get(&uri)?; + let (uri, cell_info) = notebook + .script_cells + .iter() + .rev() + .find(|(_, i)| i.line_offset <= line)?; + if cell_info.line_offset + cell_info.line_count <= line { + return None; + } + Some((uri.clone(), *cell_info)) + })(); + if let Some((uri, cell_info)) = cell_entry { + return Ok((uri, Some(cell_info))); + } + Ok((uri, None)) + } + + pub fn notebook_cell(&self, uri: &Uri) -> Option> { + let inner = self.inner.lock(); + let notebook_uri = inner.script_cell_to_notebook_uri.get(uri)?; + let notebook = inner.notebooks.get(notebook_uri)?; + notebook.cells.get(uri).cloned() } pub fn notebook_did_open( @@ -505,7 +557,7 @@ impl LspUrlMap { params .cell_text_documents .into_iter() - .map(|i| (i.uri.clone(), i)) + .map(|i| (i.uri.clone(), Arc::new(NotebookCell::new(i)))) .collect(), params.notebook_document.version, ); @@ -534,11 +586,14 @@ impl LspUrlMap { ), )); }; - let structure_changed = - params.change.cells.is_some_and(|c| c.structure.is_some()); + let structure_changed = params + .change + .cells + .as_ref() + .is_some_and(|c| c.structure.is_some()); if structure_changed { for script_cell_uri in notebook.script_cells.keys() { - inner.script_cell_to_notebook_uri.remove(&script_cell_uri); + inner.script_cell_to_notebook_uri.remove(script_cell_uri); } } let notebook = notebook.with_change(params); @@ -563,7 +618,7 @@ impl LspUrlMap { inner.notebooks.remove(¶ms.notebook_document.uri) { for script_cell_uri in notebook.script_cells.keys() { - inner.script_cell_to_notebook_uri.remove(&script_cell_uri); + inner.script_cell_to_notebook_uri.remove(script_cell_uri); } } lsp_types::TextDocumentIdentifier { @@ -580,7 +635,7 @@ impl LspUrlMap { /// ), /// Some(Url::parse("file:///path/to/file.ipynb.ts?scheme=deno-notebook-cell#abc").unwrap()), /// ); -fn file_like_to_file_specifier(specifier: &Url) -> Option { +pub fn file_like_to_file_specifier(specifier: &Url) -> Option { if matches!(specifier.scheme(), "untitled" | "deno-notebook-cell") { if let Ok(mut s) = ModuleSpecifier::parse(&format!( "file://{}",