From fb7b0a8c1188282f8d4a43bcbb10e9964a200b07 Mon Sep 17 00:00:00 2001 From: zc he Date: Sat, 15 Feb 2025 00:59:46 +0800 Subject: [PATCH] feat(lsp): hover on external command shows manpage (#15115) # Description image Not particularly useful, but better than showing nothing I guess. #14464 Also fixed a markdown syntax issue for mutable variable hovering # User-Facing Changes # Tests + Formatting +1 # After Submitting --------- Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com> --- crates/nu-lsp/src/ast.rs | 61 +++++++++++++++-------------- crates/nu-lsp/src/hints.rs | 2 +- crates/nu-lsp/src/lib.rs | 57 ++++++++++++++++++++++----- crates/nu-lsp/src/symbols.rs | 6 +-- tests/fixtures/lsp/hover/command.nu | 3 +- 5 files changed, 85 insertions(+), 44 deletions(-) diff --git a/crates/nu-lsp/src/ast.rs b/crates/nu-lsp/src/ast.rs index 775093c133..91f1114060 100644 --- a/crates/nu-lsp/src/ast.rs +++ b/crates/nu-lsp/src/ast.rs @@ -19,6 +19,26 @@ fn strip_quotes(span: Span, working_set: &StateWorkingSet) -> Span { } } +fn try_find_id_in_misc( + call: &Call, + working_set: &StateWorkingSet, + location: Option<&usize>, + id_ref: Option<&Id>, +) -> Option<(Id, Span)> { + let call_name = working_set.get_span_contents(call.head); + match call_name { + b"def" | b"export def" => try_find_id_in_def(call, working_set, location, id_ref), + b"module" | b"export module" => try_find_id_in_mod(call, working_set, location, id_ref), + b"use" | b"export use" | b"hide" => { + try_find_id_in_use(call, working_set, location, id_ref, call_name) + } + b"overlay use" | b"overlay hide" => { + try_find_id_in_overlay(call, working_set, location, id_ref) + } + _ => None, + } +} + /// For situations like /// ```nushell /// def foo [] {} @@ -35,10 +55,6 @@ fn try_find_id_in_def( location: Option<&usize>, id_ref: Option<&Id>, ) -> Option<(Id, Span)> { - let call_name = working_set.get_span_contents(call.head); - if call_name != b"def" && call_name != b"export def" { - return None; - }; let mut span = None; for arg in call.arguments.iter() { if location.map_or(true, |pos| arg.span().contains(*pos)) { @@ -80,10 +96,6 @@ fn try_find_id_in_mod( location: Option<&usize>, id_ref: Option<&Id>, ) -> Option<(Id, Span)> { - let call_name = working_set.get_span_contents(call.head); - if call_name != b"module" && call_name != b"export module" { - return None; - }; let check_location = |span: &Span| location.map_or(true, |pos| span.contains(*pos)); call.arguments.first().and_then(|arg| { @@ -119,11 +131,8 @@ fn try_find_id_in_use( working_set: &StateWorkingSet, location: Option<&usize>, id: Option<&Id>, + call_name: &[u8], ) -> Option<(Id, Span)> { - let call_name = working_set.get_span_contents(call.head); - if call_name != b"use" && call_name != b"export use" && call_name != b"hide" { - return None; - } // TODO: for keyword `hide`, the decl/var is already hidden in working_set, // this function will always return None. let find_by_name = |name: &[u8]| match id { @@ -223,10 +232,6 @@ fn try_find_id_in_overlay( location: Option<&usize>, id: Option<&Id>, ) -> Option<(Id, Span)> { - let call_name = working_set.get_span_contents(call.head); - if call_name != b"overlay use" && call_name != b"overlay hide" { - return None; - } let check_location = |span: &Span| location.map_or(true, |pos| span.contains(*pos)); let module_from_overlay_name = |name: &str, span: Span| { let found_id = Id::Module(working_set.find_overlay(name.as_bytes())?.origin); @@ -292,19 +297,19 @@ fn find_id_in_expr( if call.head.contains(*location) { FindMapResult::Found((Id::Declaration(call.decl_id), call.head)) } else { - try_find_id_in_def(call, working_set, Some(location), None) - .or(try_find_id_in_mod(call, working_set, Some(location), None)) - .or(try_find_id_in_use(call, working_set, Some(location), None)) - .or(try_find_id_in_overlay( - call, - working_set, - Some(location), - None, - )) + try_find_id_in_misc(call, working_set, Some(location), None) .map(FindMapResult::Found) .unwrap_or_default() } } + Expr::ExternalCall(head, _) => { + if head.span.contains(*location) { + if let Expr::GlobPattern(cmd, _) = &head.expr { + return FindMapResult::Found((Id::External(cmd.clone()), head.span)); + } + } + FindMapResult::Continue + } Expr::FullCellPath(fcp) => { if fcp.head.span.contains(*location) { FindMapResult::Continue @@ -383,11 +388,7 @@ fn find_reference_by_id_in_expr( occurs.push(call.head); return Some(occurs); } - if let Some((_, span_found)) = try_find_id_in_def(call, working_set, None, Some(id)) - .or(try_find_id_in_mod(call, working_set, None, Some(id))) - .or(try_find_id_in_use(call, working_set, None, Some(id))) - .or(try_find_id_in_overlay(call, working_set, None, Some(id))) - { + if let Some((_, span_found)) = try_find_id_in_misc(call, working_set, None, Some(id)) { occurs.push(span_found); } Some(occurs) diff --git a/crates/nu-lsp/src/hints.rs b/crates/nu-lsp/src/hints.rs index 564430bad2..807206a9f5 100644 --- a/crates/nu-lsp/src/hints.rs +++ b/crates/nu-lsp/src/hints.rs @@ -145,7 +145,7 @@ fn extract_inlay_hints_from_expression( impl LanguageServer { pub(crate) fn get_inlay_hints(&mut self, params: &InlayHintParams) -> Option> { - Some(self.inlay_hints.get(¶ms.text_document.uri)?.clone()) + self.inlay_hints.get(¶ms.text_document.uri).cloned() } pub(crate) fn extract_inlay_hints( diff --git a/crates/nu-lsp/src/lib.rs b/crates/nu-lsp/src/lib.rs index 4762cc6ed8..6d75ab0e3a 100644 --- a/crates/nu-lsp/src/lib.rs +++ b/crates/nu-lsp/src/lib.rs @@ -43,6 +43,7 @@ pub(crate) enum Id { Value(Type), Module(ModuleId), CellPath(VarId, Vec), + External(String), } pub struct LanguageServer { @@ -594,14 +595,12 @@ impl LanguageServer { .const_val .clone() .and_then(|v| v.coerce_into_string().ok()) - .map(|s| format!("\n---\n{}", s)) - .unwrap_or_default(); - let contents = format!( - "{} ```\n{}\n``` {}", - if var.mutable { "mutable " } else { "" }, - var.ty, - value - ); + .unwrap_or(String::from(if var.mutable { + "mutable" + } else { + "immutable" + })); + let contents = format!("```\n{}\n``` \n---\n{}", var.ty, value); markdown_hover(contents) } Id::CellPath(var_id, cell_path) => { @@ -636,6 +635,20 @@ impl LanguageServer { markdown_hover(description) } Id::Value(t) => markdown_hover(format!("`{}`", t)), + Id::External(cmd) => { + let command_output = if cfg!(windows) { + std::process::Command::new("powershell.exe") + .args(["-NoProfile", "-Command", "help", &cmd]) + .output() + } else { + std::process::Command::new("man").arg(&cmd).output() + }; + let manpage_str = match command_output { + Ok(output) => String::from_utf8_lossy(&output.stdout).to_string(), + Err(_) => format!("No command help found for {}", &cmd), + }; + markdown_hover(manpage_str) + } } } @@ -929,7 +942,7 @@ mod tests { assert_json_eq!( result_from_message(resp), - serde_json::json!({ "contents": { "kind": "markdown", "value": " ```\ntable\n``` " } }) + serde_json::json!({ "contents": { "kind": "markdown", "value": "```\ntable\n``` \n---\nimmutable" } }) ); } @@ -991,6 +1004,32 @@ mod tests { ); } + #[test] + fn hover_on_external_command() { + let (client_connection, _recv) = initialize_language_server(None); + + let mut script = fixtures(); + script.push("lsp"); + script.push("hover"); + script.push("command.nu"); + let script = path_to_uri(&script); + + open_unchecked(&client_connection, script.clone()); + let resp = send_hover_request(&client_connection, script.clone(), 6, 2); + + let hover_text = result_from_message(resp) + .pointer("/contents/value") + .unwrap() + .as_str() + .unwrap() + .to_string(); + + #[cfg(not(windows))] + assert!(hover_text.starts_with("SLEEP(1)")); + #[cfg(windows)] + assert!(hover_text.starts_with("NAME\r\n Start-Sleep")); + } + #[test] fn hover_on_str_join() { let (client_connection, _recv) = initialize_language_server(None); diff --git a/crates/nu-lsp/src/symbols.rs b/crates/nu-lsp/src/symbols.rs index 4383e33738..13f48b51c4 100644 --- a/crates/nu-lsp/src/symbols.rs +++ b/crates/nu-lsp/src/symbols.rs @@ -274,9 +274,9 @@ impl LanguageServer { let uri = params.text_document.uri.to_owned(); let docs = self.docs.lock().ok()?; self.symbol_cache.update(&uri, &engine_state, &docs); - Some(DocumentSymbolResponse::Flat( - self.symbol_cache.get_symbols_by_uri(&uri)?, - )) + self.symbol_cache + .get_symbols_by_uri(&uri) + .map(DocumentSymbolResponse::Flat) } pub(crate) fn workspace_symbol( diff --git a/tests/fixtures/lsp/hover/command.nu b/tests/fixtures/lsp/hover/command.nu index 688eaf5f17..d9df49ba4d 100644 --- a/tests/fixtures/lsp/hover/command.nu +++ b/tests/fixtures/lsp/hover/command.nu @@ -1,6 +1,7 @@ # Renders some greeting message -def hello [] {} +def hello [] { } hello [""] | str join +^sleep