refactor(lsp): align markdown doc string with output of --help (#15508)

#15499 reminds me of the discrepancies between lsp hover docs and
`--help` outputs.

# Description

# User-Facing Changes

Before:

<img width="610" alt="image"
src="https://github.com/user-attachments/assets/f73f7ace-5c1b-4380-9921-fb4783bdb187"
/>

After:

<img width="610" alt="image"
src="https://github.com/user-attachments/assets/96de3ffe-e37b-41b1-88bb-123eeb72ced2"
/>

Output of `if -h` as a reference:

```
Usage:
  > if <cond> <then_block> (else <else_expression>)

Flags:
  -h, --help: Display the help message for this command

Parameters:
  cond <variable>: Condition to check.
  then_block <block>: Block to run if check succeeds.
  "else" + <one_of(block, expression)>: Expression or block to run when the condition is false. (optional)

```

# Tests + Formatting

Refined

# After Submitting

crates/nu-lsp/src/hover.rs

@@ -1,7 +1,10 @@
 use lsp_types::{Hover, HoverContents, HoverParams, MarkupContent, MarkupKind};
-use nu_protocol::engine::Command;
+use nu_protocol::{engine::Command, PositionalArg};
 
-use crate::{Id, LanguageServer};
+use crate::{
+    signature::{display_flag, doc_for_arg, get_signature_label},
+    Id, LanguageServer,
+};
 
 impl LanguageServer {
     pub(crate) fn get_decl_description(decl: &dyn Command, skip_description: bool) -> String {
@@ -19,35 +22,27 @@ impl LanguageServer {
         // Usage
         description.push_str("---\n### Usage \n```nu\n");
         let signature = decl.signature();
-        description.push_str(&Self::get_signature_label(&signature));
+        description.push_str(&get_signature_label(&signature, true));
         description.push_str("\n```\n");
 
         // Flags
         if !signature.named.is_empty() {
             description.push_str("\n### Flags\n\n");
             let mut first = true;
-            for named in &signature.named {
+            for named in signature.named {
                 if first {
                     first = false;
                 } else {
                     description.push('\n');
                 }
                 description.push_str("  ");
-                if let Some(short_flag) = &named.short {
+                description.push_str(&display_flag(&named, true));
-                    description.push_str(&format!("`-{short_flag}`"));
+                description.push_str(&doc_for_arg(
-                }
+                    named.arg,
-                if !named.long.is_empty() {
+                    named.desc,
-                    if named.short.is_some() {
+                    named.default_value,
-                        description.push_str(", ");
+                    false,
-                    }
+                ));
-                    description.push_str(&format!("`--{}`", named.long));
-                }
-                if let Some(arg) = &named.arg {
-                    description.push_str(&format!(" `<{}>`", arg.to_type()));
-                }
-                if !named.desc.is_empty() {
-                    description.push_str(&format!(" - {}", named.desc));
-                }
                 description.push('\n');
             }
             description.push('\n');
@@ -60,46 +55,38 @@ impl LanguageServer {
         {
             description.push_str("\n### Parameters\n\n");
             let mut first = true;
-            for required_arg in &signature.required_positional {
+            let mut write_arg = |arg: PositionalArg, optional: bool| {
                 if first {
                     first = false;
                 } else {
                     description.push('\n');
                 }
-                description.push_str(&format!(
+                description.push_str(&format!("  `{}`", arg.name));
-                    "  `{}: {}`",
+                description.push_str(&doc_for_arg(
-                    required_arg.name,
+                    Some(arg.shape),
-                    required_arg.shape.to_type()
+                    arg.desc,
+                    arg.default_value,
+                    optional,
                 ));
-                if !required_arg.desc.is_empty() {
-                    description.push_str(&format!(" - {}", required_arg.desc));
-                }
                 description.push('\n');
+            };
+            for required_arg in signature.required_positional {
+                write_arg(required_arg, false);
             }
-            for optional_arg in &signature.optional_positional {
+            for optional_arg in signature.optional_positional {
-                if first {
+                write_arg(optional_arg, true);
-                    first = false;
-                } else {
-                    description.push('\n');
-                }
-                description.push_str(&format!(
-                    "  `{}: {}`",
-                    optional_arg.name,
-                    optional_arg.shape.to_type()
-                ));
-                if !optional_arg.desc.is_empty() {
-                    description.push_str(&format!(" - {}", optional_arg.desc));
-                }
-                description.push('\n');
             }
-            if let Some(arg) = &signature.rest_positional {
+            if let Some(arg) = signature.rest_positional {
                 if !first {
                     description.push('\n');
                 }
-                description.push_str(&format!(" `...{}: {}`", arg.name, arg.shape.to_type()));
+                description.push_str(&format!(" `...{}`", arg.name));
-                if !arg.desc.is_empty() {
+                description.push_str(&doc_for_arg(
-                    description.push_str(&format!(" - {}", arg.desc));
+                    Some(arg.shape),
-                }
+                    arg.desc,
+                    arg.default_value,
+                    false,
+                ));
                 description.push('\n');
             }
             description.push('\n');
@@ -378,7 +365,7 @@ mod hover_tests {
             serde_json::json!({
                     "contents": {
                     "kind": "markdown",
-                    "value": "Concatenate multiple strings into a single string, with an optional separator between each.\n---\n### Usage \n```nu\n  str join {flags} <separator?>\n```\n\n### Flags\n\n  `-h`, `--help` - Display the help message for this command\n\n\n### Parameters\n\n  `separator: string` - Optional separator to use when creating string.\n\n\n### Input/output types\n\n```nu\n list<any> | string\n string | string\n\n```\n### Example(s)\n  Create a string from input\n```nu\n  ['nu', 'shell'] | str join\n```\n  Create a string from input with a separator\n```nu\n  ['nu', 'shell'] | str join '-'\n```\n"
+                    "value": "Concatenate multiple strings into a single string, with an optional separator between each.\n---\n### Usage \n```nu\n  str join {flags} (separator)\n```\n\n### Flags\n\n  `-h`, `--help` - Display the help message for this command\n\n\n### Parameters\n\n  `separator`: `<string>` - Optional separator to use when creating string. (optional)\n\n\n### Input/output types\n\n```nu\n list<any> | string\n string | string\n\n```\n### Example(s)\n  Create a string from input\n```nu\n  ['nu', 'shell'] | str join\n```\n  Create a string from input with a separator\n```nu\n  ['nu', 'shell'] | str join '-'\n```\n"
                 }
             })
         );

crates/nu-lsp/src/notification.rs

@@ -162,7 +162,7 @@ mod tests {
             serde_json::json!({
                 "contents": {
                     "kind": "markdown",
-                    "value": "Create a variable and give it a value.\n\nThis command is a parser keyword. For details, check:\n  https://www.nushell.sh/book/thinking_in_nu.html\n---\n### Usage \n```nu\n  let {flags} <var_name> <initial_value>\n```\n\n### Flags\n\n  `-h`, `--help` - Display the help message for this command\n\n\n### Parameters\n\n  `var_name: any` - Variable name.\n\n  `initial_value: any` - Equals sign followed by value.\n\n\n### Input/output types\n\n```nu\n any | nothing\n\n```\n### Example(s)\n  Set a variable to a value\n```nu\n  let x = 10\n```\n  Set a variable to the result of an expression\n```nu\n  let x = 10 + 100\n```\n  Set a variable based on the condition\n```nu\n  let x = if false { -1 } else { 1 }\n```\n"
+                    "value": "Create a variable and give it a value.\n\nThis command is a parser keyword. For details, check:\n  https://www.nushell.sh/book/thinking_in_nu.html\n---\n### Usage \n```nu\n  let {flags} <var_name> = <initial_value>\n```\n\n### Flags\n\n  `-h`, `--help` - Display the help message for this command\n\n\n### Parameters\n\n  `var_name`: `<vardecl>` - Variable name.\n\n  `initial_value`: `<variable>` - Equals sign followed by value.\n\n\n### Input/output types\n\n```nu\n any | nothing\n\n```\n### Example(s)\n  Set a variable to a value\n```nu\n  let x = 10\n```\n  Set a variable to the result of an expression\n```nu\n  let x = 10 + 100\n```\n  Set a variable based on the condition\n```nu\n  let x = if false { -1 } else { 1 }\n```\n"
                 }
             })
         );

crates/nu-lsp/src/signature.rs

@@ -5,7 +5,7 @@ use lsp_types::{
 use nu_protocol::{
     ast::{Argument, Call, Expr, Expression, FindMapResult, Traverse},
     engine::StateWorkingSet,
-    PositionalArg, Signature,
+    Flag, PositionalArg, Signature, SyntaxShape, Value,
 };
 
 use crate::{uri_to_path, LanguageServer};
@@ -35,34 +35,85 @@ fn find_active_internal_call<'a>(
     }
 }
 
-impl LanguageServer {
+pub(crate) fn display_flag(flag: &Flag, verbitam: bool) -> String {
-    pub(crate) fn get_signature_label(signature: &Signature) -> String {
+    let md_backtick = if verbitam { "`" } else { "" };
-        let mut label = String::new();
+    let mut text = String::new();
-        label.push_str(&format!("  {}", signature.name));
+    if let Some(short_flag) = flag.short {
-        if !signature.named.is_empty() {
+        text.push_str(&format!("{md_backtick}-{short_flag}{md_backtick}"));
-            label.push_str(" {flags}");
-        }
-        for required_arg in &signature.required_positional {
-            label.push_str(&format!(" <{}>", required_arg.name));
-        }
-        for optional_arg in &signature.optional_positional {
-            let value_info = if let Some(value) = optional_arg
-                .default_value
-                .as_ref()
-                .and_then(|v| v.coerce_str().ok())
-            {
-                format!("={}", value)
-            } else {
-                String::new()
-            };
-            label.push_str(&format!(" <{}?{}>", optional_arg.name, value_info));
-        }
-        if let Some(arg) = &signature.rest_positional {
-            label.push_str(&format!(" <...{}>", arg.name));
-        }
-        label
     }
+    if !flag.long.is_empty() {
+        if flag.short.is_some() {
+            text.push_str(", ");
+        }
+        text.push_str(&format!("{md_backtick}--{}{md_backtick}", flag.long));
+    }
+    text
+}
 
+pub(crate) fn doc_for_arg(
+    syntax_shape: Option<SyntaxShape>,
+    desc: String,
+    default_value: Option<Value>,
+    optional: bool,
+) -> String {
+    let mut text = String::new();
+    if let Some(mut shape) = syntax_shape {
+        if let SyntaxShape::Keyword(_, inner_shape) = shape {
+            shape = *inner_shape;
+        }
+        text.push_str(&format!(": `<{}>`", shape));
+    }
+    if !(desc.is_empty() && default_value.is_none()) || optional {
+        text.push_str(" -")
+    };
+    if !desc.is_empty() {
+        text.push_str(&format!(" {}", desc));
+    };
+    if let Some(value) = default_value.as_ref().and_then(|v| v.coerce_str().ok()) {
+        text.push_str(&format!(
+            " ({}default: `{value}`)",
+            if optional { "optional, " } else { "" }
+        ));
+    } else if optional {
+        text.push_str(" (optional)");
+    }
+    text
+}
+
+pub(crate) fn get_signature_label(signature: &Signature, indent: bool) -> String {
+    let expand_keyword = |arg: &PositionalArg, optional: bool| match &arg.shape {
+        SyntaxShape::Keyword(kwd, _) => {
+            format!("{} <{}>", String::from_utf8_lossy(kwd), arg.name)
+        }
+        _ => {
+            if optional {
+                arg.name.clone()
+            } else {
+                format!("<{}>", arg.name)
+            }
+        }
+    };
+    let mut label = String::new();
+    if indent {
+        label.push_str("  ");
+    }
+    label.push_str(&signature.name);
+    if !signature.named.is_empty() {
+        label.push_str(" {flags}");
+    }
+    for required_arg in &signature.required_positional {
+        label.push_str(&format!(" {}", expand_keyword(required_arg, false)));
+    }
+    for optional_arg in &signature.optional_positional {
+        label.push_str(&format!(" ({})", expand_keyword(optional_arg, true)));
+    }
+    if let Some(arg) = &signature.rest_positional {
+        label.push_str(&format!(" ...({})", arg.name));
+    }
+    label
+}
+
+impl LanguageServer {
     pub(crate) fn get_signature_help(
         &mut self,
         params: &SignatureHelpParams,
@@ -120,6 +171,7 @@ impl LanguageServer {
             find_active_internal_call(expr, &working_set, pos_to_search)
         })?;
         let active_signature = working_set.get_decl(active_call.decl_id).signature();
+        let label = get_signature_label(&active_signature, false);
 
         let mut param_num_before_pos = 0;
         for arg in active_call.arguments.iter() {
@@ -133,39 +185,51 @@ impl LanguageServer {
                 break;
             }
         }
+
         let str_to_doc = |s: String| {
             Some(Documentation::MarkupContent(MarkupContent {
                 kind: MarkupKind::Markdown,
                 value: s,
             }))
         };
-        let arg_to_param_info = |arg: &PositionalArg| ParameterInformation {
+        let arg_to_param_info = |arg: PositionalArg, optional: bool| ParameterInformation {
-            label: lsp_types::ParameterLabel::Simple(arg.name.to_owned()),
+            label: lsp_types::ParameterLabel::Simple(arg.name),
-            documentation: str_to_doc(format!(
+            documentation: str_to_doc(doc_for_arg(
-                ": `<{}>` - {}",
+                Some(arg.shape),
-                arg.shape.to_type(),
+                arg.desc,
-                arg.desc.to_owned()
+                arg.default_value,
+                optional,
             )),
         };
+        let flag_to_param_info = |flag: Flag| ParameterInformation {
+            label: lsp_types::ParameterLabel::Simple(display_flag(&flag, false)),
+            documentation: str_to_doc(doc_for_arg(flag.arg, flag.desc, flag.default_value, false)),
+        };
+
+        // positional args
         let mut parameters: Vec<ParameterInformation> = active_signature
             .required_positional
-            .iter()
+            .into_iter()
-            .map(arg_to_param_info)
+            .map(|arg| arg_to_param_info(arg, false))
             .chain(
                 active_signature
                     .optional_positional
-                    .iter()
+                    .into_iter()
-                    .map(arg_to_param_info),
+                    .map(|arg| arg_to_param_info(arg, true)),
             )
             .collect();
-        if let Some(rest_arg) = &active_signature.rest_positional {
+        if let Some(rest_arg) = active_signature.rest_positional {
-            parameters.push(arg_to_param_info(rest_arg));
+            parameters.push(arg_to_param_info(rest_arg, false));
         }
+
         let max_idx = parameters.len().saturating_sub(1) as u32;
         let active_parameter = Some(param_num_before_pos.min(max_idx));
+        // also include flags in the end, just for documentation
+        parameters.extend(active_signature.named.into_iter().map(flag_to_param_info));
+
         Some(SignatureHelp {
             signatures: vec![SignatureInformation {
-                label: Self::get_signature_label(&active_signature),
+                label,
                 documentation: str_to_doc(active_signature.description),
                 parameters: Some(parameters),
                 active_parameter,
@@ -233,7 +297,7 @@ mod tests {
             actual: result_from_message(resp),
             expected: serde_json::json!({
                 "signatures": [{
-                    "label": "  str substring {flags} <range> <...rest>",
+                    "label": "str substring {flags} <range> ...(rest)",
                     "parameters": [ ],
                     "activeParameter": 0
                 }],
@@ -263,7 +327,7 @@ mod tests {
         assert_json_include!(
             actual: result_from_message(resp),
             expected: serde_json::json!({ "signatures": [{
-                "label": "  str substring {flags} <range> <...rest>",
+                "label": "str substring {flags} <range> ...(rest)",
                 "activeParameter": 1
             }]})
         );
@@ -272,7 +336,7 @@ mod tests {
         assert_json_include!(
             actual: result_from_message(resp),
             expected: serde_json::json!({ "signatures": [{
-                "label": "  str substring {flags} <range> <...rest>",
+                "label": "str substring {flags} <range> ...(rest)",
                 "activeParameter": 0
             }]})
         );
@@ -281,7 +345,7 @@ mod tests {
         assert_json_include!(
             actual: result_from_message(resp),
             expected: serde_json::json!({ "signatures": [{
-                "label": "  echo {flags} <...rest>",
+                "label": "echo {flags} ...(rest)",
                 "activeParameter": 0
             }]})
         );
@@ -291,8 +355,8 @@ mod tests {
     fn signature_help_on_custom_commands() {
         let config_str = r#"export def "foo bar" [
     p1: int
-    p2: string,
+    p2: string, # doc
-    p3?: int = 1 # doc
+    p3?: int = 1
 ] {}"#;
         let (client_connection, _recv) = initialize_language_server(Some(config_str), None);
 
@@ -308,11 +372,11 @@ mod tests {
             actual: result_from_message(resp),
             expected: serde_json::json!({
                 "signatures": [{
-                    "label": "  foo bar {flags} <p1> <p2> <p3?=1>",
+                    "label": "foo bar {flags} <p1> <p2> (p3)",
                     "parameters": [
-                        {"label": "p1", "documentation": {"value": ": `<int>` - "}},
+                        {"label": "p1", "documentation": {"value": ": `<int>`"}},
-                        {"label": "p2", "documentation": {"value": ": `<string>` - "}},
+                        {"label": "p2", "documentation": {"value": ": `<string>` - doc"}},
-                        {"label": "p3", "documentation": {"value": ": `<int>` - doc"}},
+                        {"label": "p3", "documentation": {"value": ": `<int>` - (optional, default: `1`)"}},
                     ],
                     "activeParameter": 1
                 }],
@@ -326,11 +390,12 @@ mod tests {
             actual: result_from_message(resp),
             expected: serde_json::json!({
                 "signatures": [{
-                    "label": "  foo baz {flags} <p1> <p2> <p3?=1>",
+                    "label": "foo baz {flags} <p1> <p2> (p3)",
                     "parameters": [
-                        {"label": "p1", "documentation": {"value": ": `<int>` - "}},
+                        {"label": "p1", "documentation": {"value": ": `<int>`"}},
-                        {"label": "p2", "documentation": {"value": ": `<string>` - "}},
+                        {"label": "p2", "documentation": {"value": ": `<string>` - doc"}},
-                        {"label": "p3", "documentation": {"value": ": `<int>` - doc"}},
+                        {"label": "p3", "documentation": {"value": ": `<int>` - (optional, default: `1`)"}},
+                        {"label": "-h, --help", "documentation": {"value": " - Display the help message for this command"}},
                     ],
                     "activeParameter": 2
                 }],

tests/fixtures/lsp/hints/signature.nu

@@ -11,7 +11,7 @@ foo bar 1 2 3
 foo baz 1 2 3  
 def "foo baz" [
     p1: int
-    p2: string,
+    p2: string, # doc
-    p3?: int = 1 # doc
+    p3?: int = 1
 ] {}
 echo