refactor: implicit match_indices assignment in matcher::add_semantic_suggestion

Author: blindFS

crates/nu-cli/src/completions/attribute_completions.rs

@@ -25,13 +25,13 @@ impl Completer for AttributeCompletion {
         let mut matcher = NuMatcher::new(prefix, options);
 
         let attr_commands =
-            working_set.find_commands_by_predicate(|s| s.starts_with(b"attr "), true);
+            working_set.find_commands_by_predicate(|s| s.starts_with("attr "), true);
 
         for (decl_id, name, desc, ty) in attr_commands {
-            let name = name.strip_prefix(b"attr ").unwrap_or(&name);
+            let name = name.strip_prefix("attr ").unwrap_or(&name);
             matcher.add_semantic_suggestion(SemanticSuggestion {
                 suggestion: Suggestion {
-                    value: String::from_utf8_lossy(name).into_owned(),
+                    value: name.to_string(),
                     description: desc,
                     span: reedline::Span {
                         start: span.start - offset,
@@ -44,7 +44,7 @@ impl Completer for AttributeCompletion {
             });
         }
 
-        matcher.suggestion_results()
+        matcher.results()
     }
 }
 
@@ -80,6 +80,6 @@ impl Completer for AttributableCompletion {
             });
         }
 
-        matcher.suggestion_results()
+        matcher.results()
     }
 }

crates/nu-cli/src/completions/cell_path_completions.rs

@@ -72,7 +72,7 @@ impl Completer for CellPathCompletion<'_> {
         for suggestion in get_suggestions_by_value(&value, current_span) {
             matcher.add_semantic_suggestion(suggestion);
         }
-        matcher.suggestion_results()
+        matcher.results()
     }
 }
 

crates/nu-cli/src/completions/command_completions.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use std::collections::HashSet;
 
 use crate::{
     SuggestionKind,
@@ -26,9 +26,9 @@ impl CommandCompletion {
         working_set: &StateWorkingSet,
         sugg_span: reedline::Span,
         matched_internal: impl Fn(&str) -> bool,
-        matcher: &mut NuMatcher<String>,
-    ) -> HashMap<String, SemanticSuggestion> {
-        let mut suggs = HashMap::new();
+        matcher: &mut NuMatcher<SemanticSuggestion>,
+    ) {
+        let mut external_commands = HashSet::new();
 
         let paths = working_set.permanent_state.get_env_var_insensitive("path");
 
@@ -46,50 +46,43 @@ impl CommandCompletion {
                             .completions
                             .external
                             .max_results
-                            <= suggs.len() as i64
+                            <= external_commands.len() as i64
                         {
                             break;
                         }
                         let Ok(name) = item.file_name().into_string() else {
                             continue;
                         };
+                        // If there's an internal command with the same name, adds ^cmd to the
+                        // matcher so that both the internal and external command are included
                         let value = if matched_internal(&name) {
                             format!("^{name}")
                         } else {
                             name.clone()
                         };
-                        if suggs.contains_key(&value) {
+                        // If a command of the same name already exists, skip
+                        if external_commands.contains(&value) {
                             continue;
                         }
                         // TODO: check name matching before a relative heavy IO involved
                         // `is_executable` for performance consideration, should avoid
                         // duplicated `match_aux` call for matched items in the future
-                        if matcher.matches(&name) && is_executable::is_executable(item.path()) {
-                            // If there's an internal command with the same name, adds ^cmd to the
-                            // matcher so that both the internal and external command are included
-                            matcher.add(&name, value.clone());
-                            suggs.insert(
-                                value.clone(),
-                                SemanticSuggestion {
-                                    suggestion: Suggestion {
-                                        value,
-                                        span: sugg_span,
-                                        append_whitespace: true,
-                                        ..Default::default()
-                                    },
-                                    kind: Some(SuggestionKind::Command(
-                                        CommandType::External,
-                                        None,
-                                    )),
+                        if matcher.matches(&name).0 && is_executable::is_executable(item.path()) {
+                            external_commands.insert(value.clone());
+                            matcher.add_semantic_suggestion(SemanticSuggestion {
+                                suggestion: Suggestion {
+                                    value,
+                                    span: sugg_span,
+                                    append_whitespace: true,
+                                    ..Default::default()
                                 },
-                            );
+                                kind: Some(SuggestionKind::Command(CommandType::External, None)),
+                            });
                         }
                     }
                 }
             }
         }
-
-        suggs
     }
 }
 
@@ -103,58 +96,42 @@ impl Completer for CommandCompletion {
         offset: usize,
         options: &CompletionOptions,
     ) -> Vec<SemanticSuggestion> {
-        let mut matcher = NuMatcher::new(prefix, options);
+        let mut matcher = NuMatcher::new(prefix.as_ref(), options);
 
         let sugg_span = reedline::Span::new(span.start - offset, span.end - offset);
 
-        let mut internal_suggs = HashMap::new();
+        let mut internal_commands = HashSet::new();
         if self.internals {
-            let filtered_commands = working_set.find_commands_by_predicate(
-                |name| {
-                    let name = String::from_utf8_lossy(name);
-                    matcher.add(&name, name.to_string())
-                },
-                true,
-            );
+            let filtered_commands =
+                working_set.find_commands_by_predicate(|name| matcher.matches(name).0, true);
             for (decl_id, name, description, typ) in filtered_commands {
-                let name = String::from_utf8_lossy(&name);
-                internal_suggs.insert(
-                    name.to_string(),
-                    SemanticSuggestion {
-                        suggestion: Suggestion {
-                            value: name.to_string(),
-                            description,
-                            span: sugg_span,
-                            append_whitespace: true,
-                            ..Suggestion::default()
-                        },
-                        kind: Some(SuggestionKind::Command(typ, Some(decl_id))),
+                matcher.add_semantic_suggestion(SemanticSuggestion {
+                    suggestion: Suggestion {
+                        value: name.to_string(),
+                        description,
+                        span: sugg_span,
+                        append_whitespace: true,
+                        ..Suggestion::default()
                     },
-                );
+                    kind: Some(SuggestionKind::Command(typ, Some(decl_id))),
+                });
+                internal_commands.insert(name);
             }
         }
 
-        let mut external_suggs = if self.externals {
+        let mut ans = matcher.results();
+        if self.externals {
+            // Create another matcher so that externals always come after internals
+            let mut external_matcher =
+                NuMatcher::new_with_customized_trimming(prefix.as_ref(), options, &['^']);
             self.external_command_completion(
                 working_set,
                 sugg_span,
-                |name| internal_suggs.contains_key(name),
-                &mut matcher,
-            )
-        } else {
-            HashMap::new()
-        };
-
-        let mut res = Vec::new();
-        for (cmd_name, indices) in matcher.results() {
-            if let Some(mut sugg) = internal_suggs
-                .remove(&cmd_name)
-                .or_else(|| external_suggs.remove(&cmd_name))
-            {
-                sugg.suggestion.match_indices = indices;
-                res.push(sugg);
-            }
+                |name| internal_commands.contains(name),
+                &mut external_matcher,
+            );
+            ans.extend(external_matcher.results());
         }
-        res
+        ans
     }
 }

crates/nu-cli/src/completions/completion_common.rs

@@ -104,6 +104,7 @@ fn complete_rec(
                     }
                 }
 
+                // TODO: accurate `match_indices` for `substring`/`fuzzy` matching, #15887
                 matcher.add(entry_name, built);
             }
         }
@@ -123,7 +124,7 @@ fn complete_rec(
 
     if has_more {
         let mut completions = vec![];
-        for (built, _) in matcher.results() {
+        for built in matcher.results() {
             completions.extend(complete_rec(
                 &partial[1..],
                 &[built],
@@ -135,11 +136,7 @@ fn complete_rec(
         }
         completions
     } else {
-        matcher
-            .results()
-            .into_iter()
-            .map(|(built, _)| built)
-            .collect()
+        matcher.results()
     }
 }