Skip to content

feat(cli): add completion generation support for nushell - #838

Merged
woodruffw merged 4 commits into
zizmorcore:mainfrom
jcgruenhage:more-completions
May 21, 2025
Merged

feat(cli): add completion generation support for nushell#838
woodruffw merged 4 commits into
zizmorcore:mainfrom
jcgruenhage:more-completions

Conversation

@jcgruenhage

@jcgruenhage jcgruenhage commented May 21, 2025

Copy link
Copy Markdown
Contributor

I'm packaging zizmor for Chimera Linux right now (see chimera-linux/cports#4255), and Chimera Linux is shipping nushell completions where available. As a nushell user, I'd love to see nushell completions for zizmor in addition to the ones already provided. While I was at it, I also added carapace, which is a multi-shell completion engine.

The way I'm doing it here is not great, because it requires replicating clap_complete::Shell and extending it to support the additional shells. An alternative that requires less code on the zizmor side is to use something like clap_allgen, but that is not intended to be used with a Shell enum, it's intended use is to generate all completions with one function call. Going for that would be a breaking change of the CLI interface, which I'd assume would need to wait for a v2, if it's wanted at all?

A completely different alternative I'd like to bring up is generating completions during build time using a build.rs file. This would allow cross compiling zizmor and still shipping completion packages without resorting to emulation or compiling twice. That requires bigger changes though, so didn't want to spend time on this without discussing it first.

@woodruffw

Copy link
Copy Markdown
Member

Hey @jcgruenhage, thanks for opening a PR!

As a nushell user, I'd love to see nushell completions for zizmor in addition to the ones already provided.

Cool, thank you!

While I was at it, I also added carapace, which is a multi-shell completion engine.

All things being equal, I'd prefer to defer this until there's a end-user request for it -- that keeps things simpler for me in terms of dependency review. Does that work for you?

For the rest: I'll do a review now 🙂

@woodruffw woodruffw added cli enhancement New feature or request labels May 21, 2025
Comment thread crates/zizmor/src/main.rs Outdated
Comment thread crates/zizmor/src/main.rs Outdated
Comment thread crates/zizmor/src/main.rs Outdated
@jcgruenhage

jcgruenhage commented May 21, 2025

Copy link
Copy Markdown
Contributor Author

I've removed carapace_spec_clap again. For a good user experience in the CLI I think completions are one of the things that should just automatically be there. If I install a tool using my distro package manager the completions should work out of the box. Carapace only loads specs from ~/.config/carapace/specs though, not from something like /usr/share/carapace/specs, so this is not possible anyway. Considering that, I'm okay with waiting for user demand with the carapace spec generation.

I've also addressed the rest of your review comments, thanks for the feedback :)

@woodruffw

woodruffw commented May 21, 2025

Copy link
Copy Markdown
Member

Thanks a ton @jcgruenhage, this looks great to me!

Two small logistical things:

  • Could you add an appropriate entry to docs/release-notes.md?
  • Could you run make snippets to regenerate the documentation's --help render?

Other than those, I think this is good to go!

Edit: I've run the CI preliminarily, but I expect it'll fail on the above make snippets change.

@jcgruenhage jcgruenhage changed the title feat(cli): add completion generation support for nushell and carapace feat(cli): add completion generation support for nushell May 21, 2025
Comment thread crates/zizmor/src/models.rs
Comment thread crates/zizmor/src/main.rs Outdated
Co-authored-by: William Woodruff <william@yossarian.net>
@woodruffw

Copy link
Copy Markdown
Member

BTW, don't worry about force-pushing -- I use a squash-merge workflow so having the commit history here is fine.

Signed-off-by: William Woodruff <william@yossarian.net>
@woodruffw
woodruffw merged commit ee81e89 into zizmorcore:main May 21, 2025
@woodruffw

Copy link
Copy Markdown
Member

Thanks a ton @jcgruenhage!

@jcgruenhage
jcgruenhage deleted the more-completions branch May 23, 2025 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants