Skip to content

Conversation

@MilesCranmer
Copy link
Member

@MilesCranmer MilesCranmer commented Aug 24, 2025

cherry picked from #1230

Also I added #[arg(value_enum, value_name = "SHELL")] which makes the help message print the available shells.

This also bumps the Cargo.lock

@IanButterworth IanButterworth requested a review from Copilot August 26, 2025 13:10
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds Nushell completion support to juliaup by introducing a custom completion shell enum and generic completion generator. It cherry-picks changes from #1230 and enhances the help message to display available shell options.

  • Adds Nushell as a supported shell for tab completions
  • Replaces direct clap::Shell usage with a custom CompletionShell enum
  • Implements a generic completion generator that handles both standard shells and Nushell

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/command_completions.rs Implements generic completion generator with Nushell support
src/cli.rs Adds CompletionShell enum and updates Completions command
Cargo.toml Adds clap_complete_nushell dependency

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@MilesCranmer
Copy link
Member Author

Actually I think there is a way to simplify this... One sec

@IanButterworth
Copy link
Member

I guess new tests are also needed?

@MilesCranmer
Copy link
Member Author

Ok the new refactor avoids the string conversion, it's much cleaner

@MilesCranmer
Copy link
Member Author

There's no tests of the completions utility but i can add some

@MilesCranmer
Copy link
Member Author

Just pushed some tests; everything should be ready now I think

This was referenced Aug 26, 2025
Copy link
Member

@IanButterworth IanButterworth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@IanButterworth IanButterworth merged commit af9e14b into JuliaLang:main Aug 26, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants