Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

boulder/moss: Support shell completions generation #239

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ReillyBrogan
Copy link
Contributor

@ReillyBrogan ReillyBrogan commented May 9, 2024

This wires up clap_complete so that the completions subcommand of boulder and moss will now automatically generate shell completions when provided a shell as the single argument.

The second commit replaces the alias implementation in moss with one that works better with how Clap works with shell completions. Now commands like moss ur correctly show further completions as though the user had used moss repo update

Closes #220

This wires up clap_complete so that the `completions` subcommand of boulder and moss will now automatically generate shell completions when provided a shell as the single argument.

Closes #220

Signed-off-by: Reilly Brogan <[email protected]>
@ikeycode
Copy link
Member

OK I'm liking in principle but I don't actually know how I feel about matching on None as a magic value for completions?

@ReillyBrogan
Copy link
Contributor Author

OK I'm liking in principle but I don't actually know how I feel about matching on None as a magic value for completions?

The reason I'm doing that is in order to support moss li as an alias for moss list installed we have to attach the command for installed to the root command object as li (but hidden so it doesn't appear in --help). Ordinarily when moss list installed is called we unpack the clap return as a subcommand list which has a further child installed. When we call it as moss li instead we just get the equivalent of the final installed object. So that's why the pattern matcher matches on None currently, if moss li is called then there's no further subcommand to extract out and route with so we need to use the additional context that's passed to the install handler.

An alternative would be to split up the "figure out what subcommand was called" logic from the "route to appropriate handler" logic which would probably make it look a bit cleaner.

@ermo ermo added status: help wanted Need an extra set of eyes on this. type: enhancement New feature or request labels May 14, 2024
@ermo ermo added this to the oxide-prealpha1 milestone May 14, 2024
@ermo
Copy link
Member

ermo commented May 20, 2024

An alternative would be to split up the "figure out what subcommand was called" logic from the "route to appropriate handler" logic which would probably make it look a bit cleaner.

Per Ikey's comments, this is probably a good idea, assuming you're up for it?

@ReillyBrogan ReillyBrogan marked this pull request as draft July 9, 2024 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help wanted Need an extra set of eyes on this. type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

boulder/moss: Shell completions support
3 participants