Add commandline complete to invoke nushell completions#17941
Conversation
|
This sounds very cool! |
|
There should be dedicated flags for the default completions like |
6c59acc to
7533646
Compare
7533646 to
592b44f
Compare
e72b9d6 to
e415611
Compare
|
Github CI is just puking. I kept re-running it and it finally all passed. |
Awesome, thank you for looking into that! I noticed it failed last night and decided to go to bed instead of investigating further 😄 With that, I think this PR is finally in a state where it's ready for review — some info about the change since I first pushed:
I tried to add as much testing around the span logic as I could but found it a little tricky, so if you see ways to improve those tests please let me know! To some extend I suspect this command needs a bit of manual testing in the wild to see what issues crop up. |
|
Thanks. Let's see what copilot thinks. Can you take a look at what copilot is saying and see if it makes since to implement it's changes? |
There was a problem hiding this comment.
Pull request overview
Adds a new commandline complete command that exposes Nushell's built-in completer to scripts and custom completers. It accepts the current REPL buffer or a piped string, optionally restricts results by completion type (directory/path/glob), and can return either a list of strings or detailed records (via --detailed) compatible with the custom-completion record format.
Changes:
- New
commandline completebuiltin innu-cli, including registration in the default CLI context. - New
IntoValueconversion forSemanticSuggestion(and a derivedIntoValueonNuStyle) so completion results can round-trip into Nu records. - Tests for the new command (REPL + completion integration tests covering wrapping the builtin from custom and external completers).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/nu-cli/src/commands/commandline/complete.rs | New commandline complete command implementation. |
| crates/nu-cli/src/commands/commandline/mod.rs | Re-exports the new CommandlineComplete command. |
| crates/nu-cli/src/commands/mod.rs | Re-exports CommandlineComplete from the commands module. |
| crates/nu-cli/src/commands/default_context.rs | Registers CommandlineComplete in the CLI context. |
| crates/nu-cli/src/completions/base.rs | Adds IntoValue impl for SemanticSuggestion and helper for span records. |
| crates/nu-cli/src/completions/mod.rs | Re-exports Context for crate-internal use by the new command. |
| crates/nu-color-config/src/nu_style.rs | Derives IntoValue on NuStyle; minor TODO note for Color::Fixed. |
| crates/nu-cli/tests/completions/mod.rs | Adds tests wrapping commandline complete from custom/external completers; sets REPL buffer in helper. |
| crates/nu-cli/tests/completions/support/completions_helpers.rs | Adds CLI context to test engine; #[track_caller] on suggestion matchers. |
| tests/repl/test_commandline.rs | Adds REPL tests for commandline complete (input, flags, detailed, errors). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let repl = engine_state.repl_state.lock().expect("repl state mutex"); | ||
| let (buffer, cursor_pos) = match &input { | ||
| PipelineData::Empty => (repl.buffer.as_str(), repl.cursor_pos), | ||
| PipelineData::Value(Value::String { val, .. }, _) => (val.as_str(), val.len()), |
There was a problem hiding this comment.
Explicit --cursor positioning feels like a pretty niche use case, and should be possible to add later if there's demand for it. For now I think keeping this simpler makes sense, so I'll add a sentence about this to the extra_description.
Most of these were pretty reasonable suggestions, and one was a pretty clear potential deadlock which is now covered by an additional test case.
|
Thanks. Let's see how it goes. |
<!-- Thanks for contributing to Nushell! Before submitting, please read the contributing guide: https://github.com/nushell/nushell/blob/main/CONTRIBUTING.md This template helps reviewers understand your changes and allows us to generate high-quality release notes. --> ## Description <!-- Explain what this PR does and why. This section is intentionally flexible: - Describe the problem - Explain your approach - Include technical details if relevant Good examples: - "In this PR, I fixed..." - "In this PR, I added support for..." - "This change improves X by..." Write as much or as little as needed for reviewers to understand your changes. --> Follow-up to #17941 to make color lookups more complete. Notably, file and directory completions from `commandline complete -d` now include the same `LS_COLORS` values used by default. For example: ``` ~/Documents/Development/nushell> '%ls ' | commandline complete -d | first 5 ╭───┬────────────┬───────────────┬────────────────────────────┬───────────╮ │ # │ value │ span │ style │ kind │ ├───┼────────────┼───────────────┼────────────────────────────┼───────────┤ │ 0 │ AGENTS.md │ ╭───────┬───╮ │ ╭────┬──────────────╮ │ file │ │ │ │ │ start │ 4 │ │ │ fg │ xterm_khaki3 │ │ │ │ │ │ │ end │ 4 │ │ ╰────┴──────────────╯ │ │ │ │ │ ╰───────┴───╯ │ │ │ │ 1 │ assets/ │ ╭───────┬───╮ │ ╭────┬───────────────────╮ │ directory │ │ │ │ │ start │ 4 │ │ │ fg │ xterm_steelblue1b │ │ │ │ │ │ │ end │ 4 │ │ ╰────┴───────────────────╯ │ │ │ │ │ ╰───────┴───╯ │ │ │ │ 2 │ ast-grep/ │ ╭───────┬───╮ │ ╭────┬───────────────────╮ │ directory │ │ │ │ │ start │ 4 │ │ │ fg │ xterm_steelblue1b │ │ │ │ │ │ │ end │ 4 │ │ ╰────┴───────────────────╯ │ │ │ │ │ ╰───────┴───╯ │ │ │ │ 3 │ benches/ │ ╭───────┬───╮ │ ╭────┬───────────────────╮ │ directory │ │ │ │ │ start │ 4 │ │ │ fg │ xterm_steelblue1b │ │ │ │ │ │ │ end │ 4 │ │ ╰────┴───────────────────╯ │ │ │ │ │ ╰───────┴───╯ │ │ │ │ 4 │ Cargo.lock │ ╭───────┬───╮ │ ╭────┬──────────────╮ │ file │ │ │ │ │ start │ 4 │ │ │ fg │ xterm_grey46 │ │ │ │ │ │ │ end │ 4 │ │ ╰────┴──────────────╯ │ │ │ │ │ ╰───────┴───╯ │ │ │ ╰───┴────────────┴───────────────┴────────────────────────────┴───────────╯ ``` ## User-facing changes (Release notes) <!-- This section is used (mostly as-is) for https://www.nushell.sh/blog/ Describe how Nushell behavior changes from a user's perspective. Do NOT describe internal Rust changes here. Write in a release note style, for example: - "Added support for..." - "Fixed an issue where..." - "Nushell now supports..." - "Improved performance of..." If your changes do NOT affect users (internal refactors, cleanup, etc.), just write: - "n/a" - "nan" - or similar This tells us the change should not appear in the changelog. Tips: - Focus on observable behavior - Include examples if helpful - Keep it concise You can: - Write a short paragraph (will appear as a bullet point), OR - Use headings (###) if your change needs more structure Avoid writing things like: - "In this PR, I refactored..." - "This updates internal code..." You may leave this blank until the PR is ready. --> Improve `LS_COLORS` support to `commandline complete --detailed`. ## Additional notes <!-- Optional. Examples: - fixes #123 - closes #456 - related #789 Anything else reviewers should know. Remove this section if not needed. --> As my comment alludes to, it would be nice to have a single source of truth for all these color lookups between `nu-color-config` and the `ansi` commands. My attempts to write a macro that can be used by all three [à la X macros](https://danilafe.com/blog/chapel_x_macros/) were not very successful, but maybe there's a simpler way?
<!-- Thank you for improving Nushell! Please, read our contributing guide: https://github.com/nushell/nushell/blob/main/CONTRIBUTING.md --> Closes nushell#16951 There are a couple things that would be nice to have, but I think this PR stands on its own without them. I can probably do some followup PRs if they seem desirable: - This line should probably not clone the world just to run this command: ```rs let completer = NuCompleter::new(Arc::new(engine_state.clone()), Arc::new(stack.clone())); ``` It definitely is possible to make a `NuCompleterRef<'_>` type or something to avoid this, but it would be a bigger refactor and this PR already feels big, so I intend to do that as a followup. - Color conversion back from `Style` -> `NuStyle` is incomplete; the default `LS_COLORS` are `Fixed(_)` so they don't convert cleanly back to strings yet. This means file completions won't roundtrip properly when passed into a completer, but it should be easy to make that work. Questions: - ~Am I using spans appropriately here? Particularly converting between reedline span + nushell span, I'm not sure what the "source" for these completions really should be (or if it needs to be offset according to the cursor position).~ - After some more thorough testing, I think I got this right now? - ~adding `impl TryIntoValue for SuggestionKind` could be a breaking change, and I'm not sure it's the right semantics either... should I mvoe this into a custom wrapper in the command itself or `nu-cli` instead?~ - I moved this logic into `SemanticSuggestion::into_value`, avoids locking in any specific JSON representation. Still not 100% sure about my choices for the table format though - bikeshed on the name for `--detailed(-d)` (mirroring `describe`)? This felt more descriptive than `--long` (from `ps`, `ls`) to me, not sure if there are other builtins with similar flags for examples ## Release notes summary - What our users need to know <!-- This section will be included as part of our release notes. See the contributing guide for more details. Please include only details relevant for users here. Motivation and technical details can be added above or below this section. You may leave this section blank until your PR is finalized. Ask a core team member if you need help filling this section. --> Add `commandline complete` command. This can be used to obtain the suggestions that nushell itself would normally suggest. Some examples: - `commandline complete` — return completions based on the current commandline contents (i.e. what would be suggested when pressing `Tab`) - `'./a' | commandline complete --type directory` — return directory paths relative to the current directory that start with `a` - `'%ls -' | commandline complete --detailed` — return all flags the builtin `ls` command accepts, including their descriptions. These completions can be used in custom command completions, for example to wrap a builtin command with additional options, or to obtain a list of suggested paths for further filtering or processing. ## Tasks after submitting <!-- Remove any tasks which aren't relevant for your PR, or add your own --> - [ ] Update the [documentation](https://github.com/nushell/nushell.github.io) --------- Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
<!-- Thanks for contributing to Nushell! Before submitting, please read the contributing guide: https://github.com/nushell/nushell/blob/main/CONTRIBUTING.md This template helps reviewers understand your changes and allows us to generate high-quality release notes. --> ## Description <!-- Explain what this PR does and why. This section is intentionally flexible: - Describe the problem - Explain your approach - Include technical details if relevant Good examples: - "In this PR, I fixed..." - "In this PR, I added support for..." - "This change improves X by..." Write as much or as little as needed for reviewers to understand your changes. --> Follow-up to nushell#17941 to make color lookups more complete. Notably, file and directory completions from `commandline complete -d` now include the same `LS_COLORS` values used by default. For example: ``` ~/Documents/Development/nushell> '%ls ' | commandline complete -d | first 5 ╭───┬────────────┬───────────────┬────────────────────────────┬───────────╮ │ # │ value │ span │ style │ kind │ ├───┼────────────┼───────────────┼────────────────────────────┼───────────┤ │ 0 │ AGENTS.md │ ╭───────┬───╮ │ ╭────┬──────────────╮ │ file │ │ │ │ │ start │ 4 │ │ │ fg │ xterm_khaki3 │ │ │ │ │ │ │ end │ 4 │ │ ╰────┴──────────────╯ │ │ │ │ │ ╰───────┴───╯ │ │ │ │ 1 │ assets/ │ ╭───────┬───╮ │ ╭────┬───────────────────╮ │ directory │ │ │ │ │ start │ 4 │ │ │ fg │ xterm_steelblue1b │ │ │ │ │ │ │ end │ 4 │ │ ╰────┴───────────────────╯ │ │ │ │ │ ╰───────┴───╯ │ │ │ │ 2 │ ast-grep/ │ ╭───────┬───╮ │ ╭────┬───────────────────╮ │ directory │ │ │ │ │ start │ 4 │ │ │ fg │ xterm_steelblue1b │ │ │ │ │ │ │ end │ 4 │ │ ╰────┴───────────────────╯ │ │ │ │ │ ╰───────┴───╯ │ │ │ │ 3 │ benches/ │ ╭───────┬───╮ │ ╭────┬───────────────────╮ │ directory │ │ │ │ │ start │ 4 │ │ │ fg │ xterm_steelblue1b │ │ │ │ │ │ │ end │ 4 │ │ ╰────┴───────────────────╯ │ │ │ │ │ ╰───────┴───╯ │ │ │ │ 4 │ Cargo.lock │ ╭───────┬───╮ │ ╭────┬──────────────╮ │ file │ │ │ │ │ start │ 4 │ │ │ fg │ xterm_grey46 │ │ │ │ │ │ │ end │ 4 │ │ ╰────┴──────────────╯ │ │ │ │ │ ╰───────┴───╯ │ │ │ ╰───┴────────────┴───────────────┴────────────────────────────┴───────────╯ ``` ## User-facing changes (Release notes) <!-- This section is used (mostly as-is) for https://www.nushell.sh/blog/ Describe how Nushell behavior changes from a user's perspective. Do NOT describe internal Rust changes here. Write in a release note style, for example: - "Added support for..." - "Fixed an issue where..." - "Nushell now supports..." - "Improved performance of..." If your changes do NOT affect users (internal refactors, cleanup, etc.), just write: - "n/a" - "nan" - or similar This tells us the change should not appear in the changelog. Tips: - Focus on observable behavior - Include examples if helpful - Keep it concise You can: - Write a short paragraph (will appear as a bullet point), OR - Use headings (###) if your change needs more structure Avoid writing things like: - "In this PR, I refactored..." - "This updates internal code..." You may leave this blank until the PR is ready. --> Improve `LS_COLORS` support to `commandline complete --detailed`. ## Additional notes <!-- Optional. Examples: - fixes nushell#123 - closes nushell#456 - related nushell#789 Anything else reviewers should know. Remove this section if not needed. --> As my comment alludes to, it would be nice to have a single source of truth for all these color lookups between `nu-color-config` and the `ansi` commands. My attempts to write a macro that can be used by all three [à la X macros](https://danilafe.com/blog/chapel_x_macros/) were not very successful, but maybe there's a simpler way?
Closes #16951
There are a couple things that would be nice to have, but I think this PR stands on its own without them. I can probably do some followup PRs if they seem desirable:
This line should probably not clone the world just to run this command:
It definitely is possible to make a
NuCompleterRef<'_>type or something to avoid this, but it would be a bigger refactor and this PR already feels big, so I intend to do that as a followup.Color conversion back from
Style->NuStyleis incomplete; the defaultLS_COLORSareFixed(_)so they don't convert cleanly back to strings yet. This means file completions won't roundtrip properly when passed into a completer, but it should be easy to make that work.Questions:
Am I using spans appropriately here? Particularly converting between reedline span + nushell span, I'm not sure what the "source" for these completions really should be (or if it needs to be offset according to the cursor position).addingimpl TryIntoValue for SuggestionKindcould be a breaking change, and I'm not sure it's the right semantics either... should I mvoe this into a custom wrapper in the command itself ornu-cliinstead?SemanticSuggestion::into_value, avoids locking in any specific JSON representation. Still not 100% sure about my choices for the table format thoughbikeshed on the name for
--detailed(-d)(mirroringdescribe)? This felt more descriptive than--long(fromps,ls) to me, not sure if there are other builtins with similar flags for examplesRelease notes summary - What our users need to know
Add
commandline completecommand. This can be used to obtain the suggestions that nushell itself would normally suggest. Some examples:commandline complete— return completions based on the current commandline contents (i.e. what would be suggested when pressingTab)'./a' | commandline complete --type directory— return directory paths relative to the current directory that start witha'%ls -' | commandline complete --detailed— return all flags the builtinlscommand accepts, including their descriptions.These completions can be used in custom command completions, for example to wrap a builtin command with additional options, or to obtain a list of suggested paths for further filtering or processing.
Tasks after submitting