mirror of
https://github.com/nushell/nushell.git
synced 2025-08-10 10:08:21 +02:00
feat(lsp): use lsp-textdocument to handle utf16 position (#14742)
<!-- if this PR closes one or more issues, you can automatically link the PR with them by using one of the [*linking keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword), e.g. - this PR should close #xxxx - fixes #xxxx you can also mention related issues, PRs or discussions! --> # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> This PR replaces `ropey` with `lsp-textdocument` for easier utf16 position handling. As a side effect, if fixes the following crashing bug: 1. create a `foo.nu` file with errors in it 2. in `bar.nu`, add code `use foo.nu *` # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> * <s>Diagnostics are now triggered only with document open/save, that's my personal preference. Changing back to previous behavior is easy if you guys have other concerns.</s> * UTF-8 position encoding is not supported by lsp-textdocument, but that's not an issue, since the previous utf-8 ropey implementation is buggy when used in real scenarios in a text editor. # Tests + Formatting <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use toolkit.nu; toolkit test stdlib"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> No new tests added, removed some utf-8 related ones. # After Submitting <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. -->
This commit is contained in:
@ -1,41 +1,24 @@
|
||||
use crate::LanguageServer;
|
||||
use lsp_types::{
|
||||
notification::{Notification, PublishDiagnostics},
|
||||
Diagnostic, DiagnosticSeverity, PublishDiagnosticsParams, Url,
|
||||
Diagnostic, DiagnosticSeverity, PublishDiagnosticsParams, Uri,
|
||||
};
|
||||
use miette::{IntoDiagnostic, Result};
|
||||
use nu_parser::parse;
|
||||
use nu_protocol::{
|
||||
engine::{EngineState, StateWorkingSet},
|
||||
Span, Value,
|
||||
};
|
||||
use nu_protocol::Value;
|
||||
|
||||
impl LanguageServer {
|
||||
pub(crate) fn publish_diagnostics_for_file(
|
||||
&self,
|
||||
uri: Url,
|
||||
engine_state: &mut EngineState,
|
||||
) -> Result<()> {
|
||||
pub(crate) fn publish_diagnostics_for_file(&mut self, uri: Uri) -> Result<()> {
|
||||
let mut engine_state = self.engine_state.clone();
|
||||
let cwd = std::env::current_dir().expect("Could not get current working directory.");
|
||||
engine_state.add_env_var("PWD".into(), Value::test_string(cwd.to_string_lossy()));
|
||||
engine_state.generate_nu_constant();
|
||||
|
||||
let mut working_set = StateWorkingSet::new(engine_state);
|
||||
|
||||
let Some((rope_of_file, file_path)) = self.rope(&uri) else {
|
||||
let Some((_, offset, working_set, file)) =
|
||||
self.update_engine_state(&mut engine_state, &uri)
|
||||
else {
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
let contents = rope_of_file.bytes().collect::<Vec<u8>>();
|
||||
let offset = working_set.next_span_start();
|
||||
working_set.files.push(file_path.into(), Span::unknown())?;
|
||||
parse(
|
||||
&mut working_set,
|
||||
Some(&file_path.to_string_lossy()),
|
||||
&contents,
|
||||
false,
|
||||
);
|
||||
|
||||
let mut diagnostics = PublishDiagnosticsParams {
|
||||
uri,
|
||||
diagnostics: Vec::new(),
|
||||
@ -46,12 +29,7 @@ impl LanguageServer {
|
||||
let message = err.to_string();
|
||||
|
||||
diagnostics.diagnostics.push(Diagnostic {
|
||||
range: Self::span_to_range(
|
||||
&err.span(),
|
||||
rope_of_file,
|
||||
offset,
|
||||
&self.position_encoding,
|
||||
),
|
||||
range: Self::span_to_range(&err.span(), file, offset),
|
||||
severity: Some(DiagnosticSeverity::ERROR),
|
||||
message,
|
||||
..Default::default()
|
||||
@ -70,20 +48,20 @@ impl LanguageServer {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use assert_json_diff::assert_json_eq;
|
||||
use lsp_types::Url;
|
||||
use nu_test_support::fs::fixtures;
|
||||
|
||||
use crate::path_to_uri;
|
||||
use crate::tests::{initialize_language_server, open_unchecked, update};
|
||||
|
||||
#[test]
|
||||
fn publish_diagnostics_variable_does_not_exists() {
|
||||
let (client_connection, _recv) = initialize_language_server(None);
|
||||
let (client_connection, _recv) = initialize_language_server();
|
||||
|
||||
let mut script = fixtures();
|
||||
script.push("lsp");
|
||||
script.push("diagnostics");
|
||||
script.push("var.nu");
|
||||
let script = Url::from_file_path(script).unwrap();
|
||||
let script = path_to_uri(&script);
|
||||
|
||||
let notification = open_unchecked(&client_connection, script.clone());
|
||||
|
||||
@ -108,13 +86,13 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn publish_diagnostics_fixed_unknown_variable() {
|
||||
let (client_connection, _recv) = initialize_language_server(None);
|
||||
let (client_connection, _recv) = initialize_language_server();
|
||||
|
||||
let mut script = fixtures();
|
||||
script.push("lsp");
|
||||
script.push("diagnostics");
|
||||
script.push("var.nu");
|
||||
let script = Url::from_file_path(script).unwrap();
|
||||
let script = path_to_uri(&script);
|
||||
|
||||
open_unchecked(&client_connection, script.clone());
|
||||
let notification = update(
|
||||
|
File diff suppressed because it is too large
Load Diff
@ -1,10 +1,7 @@
|
||||
use lsp_types::{
|
||||
notification::{
|
||||
DidChangeTextDocument, DidCloseTextDocument, DidOpenTextDocument, Notification,
|
||||
},
|
||||
DidChangeTextDocumentParams, DidCloseTextDocumentParams, DidOpenTextDocumentParams, Url,
|
||||
notification::{DidChangeTextDocument, DidOpenTextDocument, DidSaveTextDocument, Notification},
|
||||
DidChangeTextDocumentParams, DidOpenTextDocumentParams, DidSaveTextDocumentParams, Uri,
|
||||
};
|
||||
use ropey::Rope;
|
||||
|
||||
use crate::LanguageServer;
|
||||
|
||||
@ -12,107 +9,52 @@ impl LanguageServer {
|
||||
pub(crate) fn handle_lsp_notification(
|
||||
&mut self,
|
||||
notification: lsp_server::Notification,
|
||||
) -> Option<Url> {
|
||||
) -> Option<Uri> {
|
||||
self.docs
|
||||
.listen(notification.method.as_str(), ¬ification.params);
|
||||
match notification.method.as_str() {
|
||||
DidOpenTextDocument::METHOD => Self::handle_notification_payload::<
|
||||
DidOpenTextDocumentParams,
|
||||
_,
|
||||
>(notification, |param| {
|
||||
if let Ok(file_path) = param.text_document.uri.to_file_path() {
|
||||
let rope = Rope::from_str(¶m.text_document.text);
|
||||
self.ropes.insert(file_path, rope);
|
||||
Some(param.text_document.uri)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}),
|
||||
DidOpenTextDocument::METHOD => {
|
||||
let params: DidOpenTextDocumentParams =
|
||||
serde_json::from_value(notification.params.clone())
|
||||
.expect("Expect receive DidOpenTextDocumentParams");
|
||||
Some(params.text_document.uri)
|
||||
}
|
||||
DidSaveTextDocument::METHOD => {
|
||||
let params: DidSaveTextDocumentParams =
|
||||
serde_json::from_value(notification.params.clone())
|
||||
.expect("Expect receive DidSaveTextDocumentParams");
|
||||
Some(params.text_document.uri)
|
||||
}
|
||||
DidChangeTextDocument::METHOD => {
|
||||
Self::handle_notification_payload::<DidChangeTextDocumentParams, _>(
|
||||
notification,
|
||||
|params| self.update_rope(params),
|
||||
)
|
||||
let params: DidChangeTextDocumentParams =
|
||||
serde_json::from_value(notification.params.clone())
|
||||
.expect("Expect receive DidChangeTextDocumentParams");
|
||||
Some(params.text_document.uri)
|
||||
}
|
||||
DidCloseTextDocument::METHOD => Self::handle_notification_payload::<
|
||||
DidCloseTextDocumentParams,
|
||||
_,
|
||||
>(notification, |param| {
|
||||
if let Ok(file_path) = param.text_document.uri.to_file_path() {
|
||||
self.ropes.remove(&file_path);
|
||||
}
|
||||
None
|
||||
}),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn handle_notification_payload<P, H>(
|
||||
notification: lsp_server::Notification,
|
||||
mut param_handler: H,
|
||||
) -> Option<Url>
|
||||
where
|
||||
P: serde::de::DeserializeOwned,
|
||||
H: FnMut(P) -> Option<Url>,
|
||||
{
|
||||
if let Ok(params) = serde_json::from_value::<P>(notification.params) {
|
||||
param_handler(params)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
fn update_rope(&mut self, params: DidChangeTextDocumentParams) -> Option<Url> {
|
||||
if let Ok(file_path) = params.text_document.uri.to_file_path() {
|
||||
for content_change in params.content_changes.into_iter() {
|
||||
let entry = self.ropes.entry(file_path.clone());
|
||||
match content_change.range {
|
||||
Some(range) => {
|
||||
entry.and_modify(|rope| {
|
||||
let start = Self::lsp_position_to_location(
|
||||
&range.start,
|
||||
rope,
|
||||
&self.position_encoding,
|
||||
);
|
||||
let end = Self::lsp_position_to_location(
|
||||
&range.end,
|
||||
rope,
|
||||
&self.position_encoding,
|
||||
);
|
||||
|
||||
rope.remove(start..end);
|
||||
rope.insert(start, &content_change.text);
|
||||
});
|
||||
}
|
||||
None => {
|
||||
entry.and_modify(|r| *r = Rope::from_str(&content_change.text));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Some(params.text_document.uri)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use assert_json_diff::assert_json_eq;
|
||||
use lsp_server::Message;
|
||||
use lsp_types::{Range, Url};
|
||||
use lsp_types::Range;
|
||||
use nu_test_support::fs::fixtures;
|
||||
|
||||
use crate::path_to_uri;
|
||||
use crate::tests::{hover, initialize_language_server, open, open_unchecked, update};
|
||||
|
||||
#[test]
|
||||
fn hover_correct_documentation_on_let() {
|
||||
let (client_connection, _recv) = initialize_language_server(None);
|
||||
let (client_connection, _recv) = initialize_language_server();
|
||||
|
||||
let mut script = fixtures();
|
||||
script.push("lsp");
|
||||
script.push("hover");
|
||||
script.push("var.nu");
|
||||
let script = Url::from_file_path(script).unwrap();
|
||||
let script = path_to_uri(&script);
|
||||
|
||||
open_unchecked(&client_connection, script.clone());
|
||||
|
||||
@ -136,13 +78,13 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn hover_on_command_after_full_content_change() {
|
||||
let (client_connection, _recv) = initialize_language_server(None);
|
||||
let (client_connection, _recv) = initialize_language_server();
|
||||
|
||||
let mut script = fixtures();
|
||||
script.push("lsp");
|
||||
script.push("hover");
|
||||
script.push("command.nu");
|
||||
let script = Url::from_file_path(script).unwrap();
|
||||
let script = path_to_uri(&script);
|
||||
|
||||
open_unchecked(&client_connection, script.clone());
|
||||
update(
|
||||
@ -177,13 +119,13 @@ hello"#,
|
||||
|
||||
#[test]
|
||||
fn hover_on_command_after_partial_content_change() {
|
||||
let (client_connection, _recv) = initialize_language_server(None);
|
||||
let (client_connection, _recv) = initialize_language_server();
|
||||
|
||||
let mut script = fixtures();
|
||||
script.push("lsp");
|
||||
script.push("hover");
|
||||
script.push("command.nu");
|
||||
let script = Url::from_file_path(script).unwrap();
|
||||
let script = path_to_uri(&script);
|
||||
|
||||
open_unchecked(&client_connection, script.clone());
|
||||
update(
|
||||
@ -222,13 +164,13 @@ hello"#,
|
||||
|
||||
#[test]
|
||||
fn open_document_with_utf_char() {
|
||||
let (client_connection, _recv) = initialize_language_server(None);
|
||||
let (client_connection, _recv) = initialize_language_server();
|
||||
|
||||
let mut script = fixtures();
|
||||
script.push("lsp");
|
||||
script.push("notifications");
|
||||
script.push("issue_11522.nu");
|
||||
let script = Url::from_file_path(script).unwrap();
|
||||
let script = path_to_uri(&script);
|
||||
|
||||
let result = open(&client_connection, script);
|
||||
|
||||
|
Reference in New Issue
Block a user