fix(lsp): completion label descriptions for cell_path and external values (#15226)

# Description

The type shown in the completion description is 1 level higher than the
actual entry.
Also cleans some TODOs for `SuggetionKind`.

# User-Facing Changes

## Before

<img width="409" alt="image"
src="https://github.com/user-attachments/assets/c7d7df02-aed9-4ea9-892a-0bca707352eb"
/>

<img width="491" alt="image"
src="https://github.com/user-attachments/assets/9b9394d4-62ee-4924-9840-402f00d88a8a"
/>

## After

<img width="425" alt="image"
src="https://github.com/user-attachments/assets/d8f41059-2c68-4902-9c32-d789f91b6d77"
/>

<img width="425" alt="image"
src="https://github.com/user-attachments/assets/ce03afb9-6c1f-4a65-a1cc-cbba4655abb3"
/>

# Tests + Formatting

Adjusted accordingly

# After Submitting
This commit is contained in:
zc he
2025-03-03 06:17:12 +08:00
committed by GitHub
parent 95f89a093a
commit 8e1385417e
9 changed files with 88 additions and 54 deletions

View File

@ -29,9 +29,14 @@ pub struct SemanticSuggestion {
#[derive(Clone, Debug, PartialEq)] #[derive(Clone, Debug, PartialEq)]
pub enum SuggestionKind { pub enum SuggestionKind {
Command(nu_protocol::engine::CommandType), Command(nu_protocol::engine::CommandType),
Type(nu_protocol::Type), Value(nu_protocol::Type),
CellPath,
Directory,
File,
Flag,
Module, Module,
Operator, Operator,
Variable,
} }
impl From<Suggestion> for SemanticSuggestion { impl From<Suggestion> for SemanticSuggestion {

View File

@ -108,23 +108,29 @@ fn get_suggestions_by_value(
value: &Value, value: &Value,
current_span: reedline::Span, current_span: reedline::Span,
) -> Vec<SemanticSuggestion> { ) -> Vec<SemanticSuggestion> {
let kind = SuggestionKind::Type(value.get_type()); let to_suggestion = |s: String, v: Option<&Value>| SemanticSuggestion {
let str_to_suggestion = |s: String| SemanticSuggestion {
suggestion: Suggestion { suggestion: Suggestion {
value: s, value: s,
span: current_span, span: current_span,
description: v.map(|v| v.get_type().to_string()),
..Suggestion::default() ..Suggestion::default()
}, },
kind: Some(kind.to_owned()), kind: Some(SuggestionKind::CellPath),
}; };
match value { match value {
Value::Record { val, .. } => val Value::Record { val, .. } => val
.columns() .columns()
.map(|s| str_to_suggestion(s.to_string())) .map(|s| to_suggestion(s.to_string(), val.get(s)))
.collect(), .collect(),
Value::List { vals, .. } => get_columns(vals.as_slice()) Value::List { vals, .. } => get_columns(vals.as_slice())
.into_iter() .into_iter()
.map(str_to_suggestion) .map(|s| {
let sub_val = vals
.first()
.and_then(|v| v.as_record().ok())
.and_then(|rv| rv.get(&s));
to_suggestion(s, sub_val)
})
.collect(), .collect(),
_ => vec![], _ => vec![],
} }

View File

@ -10,7 +10,7 @@ use nu_protocol::{
ast::{Argument, Block, Expr, Expression, FindMapResult, Traverse}, ast::{Argument, Block, Expr, Expression, FindMapResult, Traverse},
debugger::WithoutDebug, debugger::WithoutDebug,
engine::{Closure, EngineState, Stack, StateWorkingSet}, engine::{Closure, EngineState, Stack, StateWorkingSet},
PipelineData, Span, Value, PipelineData, Span, Type, Value,
}; };
use reedline::{Completer as ReedlineCompleter, Suggestion}; use reedline::{Completer as ReedlineCompleter, Suggestion};
use std::{str, sync::Arc}; use std::{str, sync::Arc};
@ -639,7 +639,7 @@ pub fn map_value_completions<'a>(
}, },
..Suggestion::default() ..Suggestion::default()
}, },
kind: Some(SuggestionKind::Type(x.get_type())), kind: Some(SuggestionKind::Value(x.get_type())),
}); });
} }
@ -653,41 +653,41 @@ pub fn map_value_completions<'a>(
}, },
..Suggestion::default() ..Suggestion::default()
}; };
let mut value_type = Type::String;
// Iterate the cols looking for `value` and `description` // Iterate the cols looking for `value` and `description`
record.iter().for_each(|it| { record.iter().for_each(|(key, value)| {
// Match `value` column match key.as_str() {
if it.0 == "value" { "value" => {
value_type = value.get_type();
// Convert the value to string // Convert the value to string
if let Ok(val_str) = it.1.coerce_string() { if let Ok(val_str) = value.coerce_string() {
// Update the suggestion value // Update the suggestion value
suggestion.value = val_str; suggestion.value = val_str;
} }
} }
"description" => {
// Match `description` column
if it.0 == "description" {
// Convert the value to string // Convert the value to string
if let Ok(desc_str) = it.1.coerce_string() { if let Ok(desc_str) = value.coerce_string() {
// Update the suggestion value // Update the suggestion value
suggestion.description = Some(desc_str); suggestion.description = Some(desc_str);
} }
} }
"style" => {
// Match `style` column
if it.0 == "style" {
// Convert the value to string // Convert the value to string
suggestion.style = match it.1 { suggestion.style = match value {
Value::String { val, .. } => Some(lookup_ansi_color_style(val)), Value::String { val, .. } => Some(lookup_ansi_color_style(val)),
Value::Record { .. } => Some(color_record_to_nustyle(it.1)), Value::Record { .. } => Some(color_record_to_nustyle(value)),
_ => None, _ => None,
}; };
} }
_ => (),
}
}); });
return Some(SemanticSuggestion { return Some(SemanticSuggestion {
suggestion, suggestion,
kind: Some(SuggestionKind::Type(x.get_type())), kind: Some(SuggestionKind::Value(value_type)),
}); });
} }

View File

@ -157,6 +157,7 @@ pub struct FileSuggestion {
pub span: nu_protocol::Span, pub span: nu_protocol::Span,
pub path: String, pub path: String,
pub style: Option<Style>, pub style: Option<Style>,
pub is_dir: bool,
} }
/// # Parameters /// # Parameters
@ -260,6 +261,7 @@ pub fn complete_item(
if should_collapse_dots { if should_collapse_dots {
p = collapse_ndots(p); p = collapse_ndots(p);
} }
let is_dir = p.isdir;
let path = original_cwd.apply(p, path_separator); let path = original_cwd.apply(p, path_separator);
let style = ls_colors.as_ref().map(|lsc| { let style = ls_colors.as_ref().map(|lsc| {
lsc.style_for_path_with_metadata( lsc.style_for_path_with_metadata(
@ -275,6 +277,7 @@ pub fn complete_item(
span, span,
path: escape_path(path, want_directory), path: escape_path(path, want_directory),
style, style,
is_dir,
} }
}) })
.collect() .collect()

View File

@ -9,7 +9,7 @@ use nu_protocol::{
use reedline::Suggestion; use reedline::Suggestion;
use std::path::Path; use std::path::Path;
use super::{completion_common::FileSuggestion, SemanticSuggestion}; use super::{completion_common::FileSuggestion, SemanticSuggestion, SuggestionKind};
pub struct DirectoryCompletion; pub struct DirectoryCompletion;
@ -47,8 +47,7 @@ impl Completer for DirectoryCompletion {
}, },
..Suggestion::default() ..Suggestion::default()
}, },
// TODO???? kind: Some(SuggestionKind::Directory),
kind: None,
}) })
.collect(); .collect();

View File

@ -9,7 +9,7 @@ use nu_protocol::{
use reedline::Suggestion; use reedline::Suggestion;
use std::path::Path; use std::path::Path;
use super::{completion_common::FileSuggestion, SemanticSuggestion}; use super::{completion_common::FileSuggestion, SemanticSuggestion, SuggestionKind};
pub struct FileCompletion; pub struct FileCompletion;
@ -50,8 +50,11 @@ impl Completer for FileCompletion {
}, },
..Suggestion::default() ..Suggestion::default()
}, },
// TODO???? kind: Some(if x.is_dir {
kind: None, SuggestionKind::Directory
} else {
SuggestionKind::File
}),
}) })
.collect(); .collect();

View File

@ -5,7 +5,7 @@ use nu_protocol::{
}; };
use reedline::Suggestion; use reedline::Suggestion;
use super::SemanticSuggestion; use super::{SemanticSuggestion, SuggestionKind};
#[derive(Clone)] #[derive(Clone)]
pub struct FlagCompletion { pub struct FlagCompletion {
@ -35,8 +35,7 @@ impl Completer for FlagCompletion {
append_whitespace: true, append_whitespace: true,
..Suggestion::default() ..Suggestion::default()
}, },
// TODO???? kind: Some(SuggestionKind::Flag),
kind: None,
}); });
}; };

View File

@ -32,10 +32,10 @@ impl Completer for VariableCompletion {
suggestion: Suggestion { suggestion: Suggestion {
value: builtin.to_string(), value: builtin.to_string(),
span: current_span, span: current_span,
description: Some("reserved".into()),
..Suggestion::default() ..Suggestion::default()
}, },
// TODO is there a way to get the VarId to get the type??? kind: Some(SuggestionKind::Variable),
kind: None,
}); });
} }
@ -44,11 +44,10 @@ impl Completer for VariableCompletion {
suggestion: Suggestion { suggestion: Suggestion {
value: String::from_utf8_lossy(name).to_string(), value: String::from_utf8_lossy(name).to_string(),
span: current_span, span: current_span,
description: Some(working_set.get_variable(*var_id).ty.to_string()),
..Suggestion::default() ..Suggestion::default()
}, },
kind: Some(SuggestionKind::Type( kind: Some(SuggestionKind::Variable),
working_set.get_variable(*var_id).ty.clone(),
)),
}) })
}; };

View File

@ -61,10 +61,13 @@ impl LanguageServer {
.kind .kind
.clone() .clone()
.map(|kind| match kind { .map(|kind| match kind {
SuggestionKind::Type(t) => t.to_string(), SuggestionKind::Value(t) => t.to_string(),
SuggestionKind::Command(cmd) => cmd.to_string(), SuggestionKind::Command(cmd) => cmd.to_string(),
SuggestionKind::Module => "module".to_string(), SuggestionKind::Module => "module".to_string(),
SuggestionKind::Operator => "operator".to_string(), SuggestionKind::Operator => "operator".to_string(),
SuggestionKind::Variable => "variable".to_string(),
SuggestionKind::Flag => "flag".to_string(),
_ => String::new(),
}) })
.map(|s| CompletionItemLabelDetails { .map(|s| CompletionItemLabelDetails {
detail: None, detail: None,
@ -103,18 +106,23 @@ impl LanguageServer {
suggestion_kind: Option<SuggestionKind>, suggestion_kind: Option<SuggestionKind>,
) -> Option<CompletionItemKind> { ) -> Option<CompletionItemKind> {
suggestion_kind.and_then(|suggestion_kind| match suggestion_kind { suggestion_kind.and_then(|suggestion_kind| match suggestion_kind {
SuggestionKind::Type(t) => match t { SuggestionKind::Value(t) => match t {
nu_protocol::Type::String => Some(CompletionItemKind::VARIABLE), nu_protocol::Type::String => Some(CompletionItemKind::VALUE),
_ => None, _ => None,
}, },
SuggestionKind::CellPath => Some(CompletionItemKind::PROPERTY),
SuggestionKind::Command(c) => match c { SuggestionKind::Command(c) => match c {
nu_protocol::engine::CommandType::Keyword => Some(CompletionItemKind::KEYWORD), nu_protocol::engine::CommandType::Keyword => Some(CompletionItemKind::KEYWORD),
nu_protocol::engine::CommandType::Builtin => Some(CompletionItemKind::FUNCTION), nu_protocol::engine::CommandType::Builtin => Some(CompletionItemKind::FUNCTION),
nu_protocol::engine::CommandType::External => Some(CompletionItemKind::INTERFACE), nu_protocol::engine::CommandType::External => Some(CompletionItemKind::INTERFACE),
_ => None, _ => None,
}, },
SuggestionKind::Directory => Some(CompletionItemKind::FOLDER),
SuggestionKind::File => Some(CompletionItemKind::FILE),
SuggestionKind::Flag => Some(CompletionItemKind::FIELD),
SuggestionKind::Module => Some(CompletionItemKind::MODULE), SuggestionKind::Module => Some(CompletionItemKind::MODULE),
SuggestionKind::Operator => Some(CompletionItemKind::OPERATOR), SuggestionKind::Operator => Some(CompletionItemKind::OPERATOR),
SuggestionKind::Variable => Some(CompletionItemKind::VARIABLE),
}) })
} }
} }
@ -180,7 +188,7 @@ mod tests {
expected: serde_json::json!([ expected: serde_json::json!([
{ {
"label": "$greeting", "label": "$greeting",
"labelDetails": { "description": "string" }, "labelDetails": { "description": "variable" },
"textEdit": { "textEdit": {
"newText": "$greeting", "newText": "$greeting",
"range": { "start": { "character": 5, "line": 2 }, "end": { "character": 9, "line": 2 } } "range": { "start": { "character": 5, "line": 2 }, "end": { "character": 9, "line": 2 } }
@ -236,9 +244,11 @@ mod tests {
&serde_json::json!({ &serde_json::json!({
"label": "-s", "label": "-s",
"detail": "test flag", "detail": "test flag",
"labelDetails": { "description": "flag" },
"textEdit": { "range": { "start": { "line": 1, "character": 17 }, "end": { "line": 1, "character": 18 }, }, "textEdit": { "range": { "start": { "line": 1, "character": 17 }, "end": { "line": 1, "character": 18 }, },
"newText": "-s" "newText": "-s"
}, },
"kind": 5
}) })
)); ));
@ -248,9 +258,11 @@ mod tests {
&serde_json::json!({ &serde_json::json!({
"label": "--long", "label": "--long",
"detail": "test flag", "detail": "test flag",
"labelDetails": { "description": "flag" },
"textEdit": { "range": { "start": { "line": 2, "character": 19 }, "end": { "line": 2, "character": 22 }, }, "textEdit": { "range": { "start": { "line": 2, "character": 19 }, "end": { "line": 2, "character": 22 }, },
"newText": "--long" "newText": "--long"
}, },
"kind": 5
}) })
)); ));
@ -259,9 +271,11 @@ mod tests {
assert!(result_from_message(resp).as_array().unwrap().contains( assert!(result_from_message(resp).as_array().unwrap().contains(
&serde_json::json!({ &serde_json::json!({
"label": "LICENSE", "label": "LICENSE",
"labelDetails": { "description": "" },
"textEdit": { "range": { "start": { "line": 2, "character": 17 }, "end": { "line": 2, "character": 18 }, }, "textEdit": { "range": { "start": { "line": 2, "character": 17 }, "end": { "line": 2, "character": 18 }, },
"newText": "LICENSE" "newText": "LICENSE"
}, },
"kind": 17
}) })
)); ));
@ -271,9 +285,11 @@ mod tests {
&serde_json::json!({ &serde_json::json!({
"label": "-g", "label": "-g",
"detail": "count indexes and split using grapheme clusters (all visible chars have length 1)", "detail": "count indexes and split using grapheme clusters (all visible chars have length 1)",
"labelDetails": { "description": "flag" },
"textEdit": { "range": { "start": { "line": 10, "character": 33 }, "end": { "line": 10, "character": 34 }, }, "textEdit": { "range": { "start": { "line": 10, "character": 33 }, "end": { "line": 10, "character": 34 }, },
"newText": "-g" "newText": "-g"
}, },
"kind": 5
}) })
)); ));
} }
@ -328,9 +344,11 @@ mod tests {
assert!(result_from_message(resp).as_array().unwrap().contains( assert!(result_from_message(resp).as_array().unwrap().contains(
&serde_json::json!({ &serde_json::json!({
"label": "LICENSE", "label": "LICENSE",
"labelDetails": { "description": "" },
"textEdit": { "range": { "start": { "line": 5, "character": 3 }, "end": { "line": 5, "character": 4 }, }, "textEdit": { "range": { "start": { "line": 5, "character": 3 }, "end": { "line": 5, "character": 4 }, },
"newText": "LICENSE" "newText": "LICENSE"
}, },
"kind": 17
}) })
)); ));
} }
@ -411,11 +429,12 @@ mod tests {
expected: serde_json::json!([ expected: serde_json::json!([
{ {
"label": "1", "label": "1",
"labelDetails": { "description": "record<1: string, 🤔🐘: table<baz: int>>" }, "detail": "string",
"textEdit": { "textEdit": {
"newText": "1", "newText": "1",
"range": { "start": { "line": 1, "character": 5 }, "end": { "line": 1, "character": 5 } } "range": { "start": { "line": 1, "character": 5 }, "end": { "line": 1, "character": 5 } }
}, },
"kind": 10
}, },
]) ])
); );
@ -426,11 +445,12 @@ mod tests {
expected: serde_json::json!([ expected: serde_json::json!([
{ {
"label": "baz", "label": "baz",
"labelDetails": { "description": "table<baz: int>" }, "detail": "int",
"textEdit": { "textEdit": {
"newText": "baz", "newText": "baz",
"range": { "start": { "line": 1, "character": 10 }, "end": { "line": 1, "character": 10 } } "range": { "start": { "line": 1, "character": 10 }, "end": { "line": 1, "character": 10 } }
}, },
"kind": 10
}, },
]) ])
); );