From da98c23ab3639ab78cadf2fe71ffbca42048b168 Mon Sep 17 00:00:00 2001
From: Yash Thakur <45539777+ysthakur@users.noreply.github.com>
Date: Mon, 26 Aug 2024 13:14:57 -0400
Subject: [PATCH] Use right options in custom completions (#13698)

<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.
- this PR should close #xxxx
- fixes #xxxx

you can also mention related issues, PRs or discussions!
-->

# Description
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->

This issue was reported by kira in
[Discord](https://discord.com/channels/601130461678272522/1276981416307069019).
In https://github.com/nushell/nushell/pull/13311, I accidentally made it
so that custom completions are filtered according to the user's
configured completion options (`$env.config.completions`) rather than
the completion options provided as part of the custom completions. This
PR is a quick fix for that.

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

It should once again be possible to override match algorithm, case
sensitivity, and substring matching (`positional`) in custom
completions.

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use toolkit.nu; toolkit test stdlib"` to run the
tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

Added a couple tests.

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->

fdncred and I discussed this in Discord a bit and we thought it might be
better to not allow custom completions to override the user's config.
However, `positional` can't currently be set inside one's config, so you
can only do strict prefix matching, no substring matching. Another PR
could do one of the following:
- Document the fact that you can provide completion options inside
custom completions
- Remove the ability to provide completion options with custom
completions and add a `$env.config.completions.positional: true` option
- Remove the ability to provide completion options with custom
completions and add a new match algorithm `substring` (this is the one I
like most, since `positional` only applies to prefix matching anyway)

Separately from these options, we could also allow completers to specify
that they don't Nushell to do any filtering and sorting on the provided
custom completions.
---
 .../src/completions/custom_completions.rs     |  2 +-
 crates/nu-cli/tests/completions/mod.rs        | 40 +++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/crates/nu-cli/src/completions/custom_completions.rs b/crates/nu-cli/src/completions/custom_completions.rs
index f21f942501..8d5512a6a9 100644
--- a/crates/nu-cli/src/completions/custom_completions.rs
+++ b/crates/nu-cli/src/completions/custom_completions.rs
@@ -126,7 +126,7 @@ impl Completer for CustomCompletion {
         let options = custom_completion_options
             .as_ref()
             .unwrap_or(completion_options);
-        let suggestions = filter(&prefix, suggestions, completion_options);
+        let suggestions = filter(&prefix, suggestions, options);
         sort_suggestions(&String::from_utf8_lossy(&prefix), suggestions, options)
     }
 }
diff --git a/crates/nu-cli/tests/completions/mod.rs b/crates/nu-cli/tests/completions/mod.rs
index e1714f78e2..70ea1ea99f 100644
--- a/crates/nu-cli/tests/completions/mod.rs
+++ b/crates/nu-cli/tests/completions/mod.rs
@@ -61,6 +61,32 @@ fn extern_completer() -> NuCompleter {
     NuCompleter::new(Arc::new(engine), Arc::new(stack))
 }
 
+#[fixture]
+fn completer_strings_with_options() -> NuCompleter {
+    // Create a new engine
+    let (dir, _, mut engine, mut stack) = new_engine();
+    // Add record value as example
+    let record = r#"
+        # To test that the config setting has no effect on the custom completions
+        $env.config.completions.algorithm = "fuzzy"
+        def animals [] {
+            {
+                # Very rare and totally real animals
+                completions: ["Abcdef", "Foo Abcdef", "Acd Bar" ],
+                options: {
+                    completion_algorithm: "prefix",
+                    positional: false,
+                    case_sensitive: false,
+                }
+            }
+        }
+        def my-command [animal: string@animals] { print $animal }"#;
+    assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack, dir).is_ok());
+
+    // Instantiate a new completer
+    NuCompleter::new(Arc::new(engine), Arc::new(stack))
+}
+
 #[fixture]
 fn custom_completer() -> NuCompleter {
     // Create a new engine
@@ -169,6 +195,20 @@ fn variables_customcompletion_subcommands_with_customcompletion_2(
     match_suggestions(&expected, &suggestions);
 }
 
+#[rstest]
+fn customcompletions_substring_matching(mut completer_strings_with_options: NuCompleter) {
+    let suggestions = completer_strings_with_options.complete("my-command Abcd", 15);
+    let expected: Vec<String> = vec!["Abcdef".into(), "Foo Abcdef".into()];
+    match_suggestions(&expected, &suggestions);
+}
+
+#[rstest]
+fn customcompletions_case_insensitive(mut completer_strings_with_options: NuCompleter) {
+    let suggestions = completer_strings_with_options.complete("my-command foo", 14);
+    let expected: Vec<String> = vec!["Foo Abcdef".into()];
+    match_suggestions(&expected, &suggestions);
+}
+
 #[test]
 fn dotnu_completions() {
     // Create a new engine