refactor: command identified by name instead of span content (#15471)

This should be a more robust method.

# Description

Previously, `export use` with double-space in between will fail to be
recognized as command `export use`.

# User-Facing Changes

minor bug fix

# Tests + Formatting

test cases made harder

# After Submitting
This commit is contained in:
zc he 2025-04-02 19:12:38 +08:00 committed by GitHub
parent af6c4bdc9c
commit df74a0c961
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 24 additions and 23 deletions

View File

@ -134,7 +134,7 @@ struct Context<'a> {
/// For argument completion /// For argument completion
struct PositionalArguments<'a> { struct PositionalArguments<'a> {
/// command name /// command name
command_head: &'a [u8], command_head: &'a str,
/// indices of positional arguments /// indices of positional arguments
positional_arg_indices: Vec<usize>, positional_arg_indices: Vec<usize>,
/// argument list /// argument list
@ -395,7 +395,7 @@ impl NuCompleter {
Argument::Positional(_) if prefix == b"-" => flag_completion_helper(), Argument::Positional(_) if prefix == b"-" => flag_completion_helper(),
// complete according to expression type and command head // complete according to expression type and command head
Argument::Positional(expr) => { Argument::Positional(expr) => {
let command_head = working_set.get_span_contents(call.head); let command_head = working_set.get_decl(call.decl_id).name();
positional_arg_indices.push(arg_idx); positional_arg_indices.push(arg_idx);
self.argument_completion_helper( self.argument_completion_helper(
PositionalArguments { PositionalArguments {
@ -537,19 +537,19 @@ impl NuCompleter {
// special commands // special commands
match command_head { match command_head {
// complete module file/directory // complete module file/directory
b"use" | b"export use" | b"overlay use" | b"source-env" "use" | "export use" | "overlay use" | "source-env"
if positional_arg_indices.len() == 1 => if positional_arg_indices.len() == 1 =>
{ {
return self.process_completion( return self.process_completion(
&mut DotNuCompletion { &mut DotNuCompletion {
std_virtual_path: command_head != b"source-env", std_virtual_path: command_head != "source-env",
}, },
ctx, ctx,
); );
} }
// NOTE: if module file already specified, // NOTE: if module file already specified,
// should parse it to get modules/commands/consts to complete // should parse it to get modules/commands/consts to complete
b"use" | b"export use" => { "use" | "export use" => {
let Some(Argument::Positional(Expression { let Some(Argument::Positional(Expression {
expr: Expr::String(module_name), expr: Expr::String(module_name),
span, span,
@ -613,7 +613,7 @@ impl NuCompleter {
_ => return vec![], _ => return vec![],
} }
} }
b"which" => { "which" => {
let mut completer = CommandCompletion { let mut completer = CommandCompletion {
internals: true, internals: true,
externals: true, externals: true,

View File

@ -586,6 +586,7 @@ fn dotnu_stdlib_completions() {
assert!(load_standard_library(&mut engine).is_ok()); assert!(load_standard_library(&mut engine).is_ok());
let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack)); let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack));
// `export use` should be recognized as command `export use`
let completion_str = "export use std/ass"; let completion_str = "export use std/ass";
let suggestions = completer.complete(completion_str, completion_str.len()); let suggestions = completer.complete(completion_str, completion_str.len());
match_suggestions(&vec!["assert"], &suggestions); match_suggestions(&vec!["assert"], &suggestions);

View File

@ -25,14 +25,14 @@ fn try_find_id_in_misc(
location: Option<&usize>, location: Option<&usize>,
id_ref: Option<&Id>, id_ref: Option<&Id>,
) -> Option<(Id, Span)> { ) -> Option<(Id, Span)> {
let call_name = working_set.get_span_contents(call.head); let call_name = working_set.get_decl(call.decl_id).name();
match call_name { match call_name {
b"def" | b"export def" => try_find_id_in_def(call, working_set, location, id_ref), "def" | "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), "module" | "export module" => try_find_id_in_mod(call, working_set, location, id_ref),
b"use" | b"export use" | b"hide" => { "use" | "export use" | "hide" => {
try_find_id_in_use(call, working_set, location, id_ref, call_name) try_find_id_in_use(call, working_set, location, id_ref, call_name)
} }
b"overlay use" | b"overlay hide" => { "overlay use" | "overlay hide" => {
try_find_id_in_overlay(call, working_set, location, id_ref) try_find_id_in_overlay(call, working_set, location, id_ref)
} }
_ => None, _ => None,
@ -141,7 +141,7 @@ fn try_find_id_in_use(
working_set: &StateWorkingSet, working_set: &StateWorkingSet,
location: Option<&usize>, location: Option<&usize>,
id: Option<&Id>, id: Option<&Id>,
call_name: &[u8], call_name: &str,
) -> Option<(Id, Span)> { ) -> Option<(Id, Span)> {
// TODO: for keyword `hide`, the decl/var is already hidden in working_set, // TODO: for keyword `hide`, the decl/var is already hidden in working_set,
// this function will always return None. // this function will always return None.
@ -176,7 +176,7 @@ fn try_find_id_in_use(
if let Some(pos) = location { if let Some(pos) = location {
// first argument of `use` should always be module name // first argument of `use` should always be module name
// while it is optional in `hide` // while it is optional in `hide`
if span.contains(*pos) && call_name != b"hide" { if span.contains(*pos) && call_name != "hide" {
return get_matched_module_id(working_set, span, id); return get_matched_module_id(working_set, span, id);
} }
} }
@ -196,7 +196,7 @@ fn try_find_id_in_use(
}) })
}; };
let arguments = if call_name != b"hide" { let arguments = if call_name != "hide" {
call.arguments.get(1..)? call.arguments.get(1..)?
} else { } else {
call.arguments.as_slice() call.arguments.as_slice()

View File

@ -658,7 +658,7 @@ mod tests {
let message_num = 5; let message_num = 5;
let messages = let messages =
send_reference_request(&client_connection, script.clone(), 6, 11, message_num); send_reference_request(&client_connection, script.clone(), 6, 12, message_num);
assert_eq!(messages.len(), message_num); assert_eq!(messages.len(), message_num);
for message in messages { for message in messages {
match message { match message {
@ -676,7 +676,7 @@ mod tests {
assert!(array.contains(&serde_json::json!( assert!(array.contains(&serde_json::json!(
{ {
"uri": script.to_string(), "uri": script.to_string(),
"range": { "start": { "line": 6, "character": 12 }, "end": { "line": 6, "character": 19 } } "range": { "start": { "line": 6, "character": 13 }, "end": { "line": 6, "character": 20 } }
} }
) )
)); ));
@ -712,7 +712,7 @@ mod tests {
&client_connection, &client_connection,
script.clone(), script.clone(),
6, 6,
11, 12,
message_num, message_num,
false, false,
); );
@ -723,8 +723,8 @@ mod tests {
Message::Response(r) => assert_json_eq!( Message::Response(r) => assert_json_eq!(
r.result, r.result,
serde_json::json!({ serde_json::json!({
"start": { "line": 6, "character": 12 }, "start": { "line": 6, "character": 13 },
"end": { "line": 6, "character": 19 } "end": { "line": 6, "character": 20 }
}), }),
), ),
_ => panic!("unexpected message type"), _ => panic!("unexpected message type"),
@ -738,7 +738,7 @@ mod tests {
changes[script.to_string()], changes[script.to_string()],
serde_json::json!([ serde_json::json!([
{ {
"range": { "start": { "line": 6, "character": 12 }, "end": { "line": 6, "character": 19 } }, "range": { "start": { "line": 6, "character": 13 }, "end": { "line": 6, "character": 20 } },
"newText": "new" "newText": "new"
} }
]) ])
@ -860,7 +860,7 @@ mod tests {
&client_connection, &client_connection,
script.clone(), script.clone(),
6, 6,
11, 12,
message_num, message_num,
true, true,
); );