Skip to content

Conversation

0xferrous
Copy link
Contributor

Motivation

Getting command completions in nushell.

Solution

  • clap_complete doesn't come with nushell completions built-in but they have another crate clap_complete_nushell that does. Sucks it isn't supported by default, see Why are the nushell completions not part of the completions crate? clap-rs/clap#5880. This PR implements a wrapper around the enum with nushell included to support nushell completions too.
  • moved clap_complete to workspace dependency to prevent conflicting versions between foundry-common and cast/anvil/forge.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

- `clap_complete` doesn't come with nushell completions built-in but they
  have another crate `clap_complete_nushell` that does. Sucks it isn't
  supported by default, see clap-rs/clap#5880.
  This PR implements a wrapper around the enum with nushell included to
  support nushell completions too.
- moved `clap_complete` to workspace dependency to prevent conflicting
  versions between foundry-common and cast/anvil/forge.
Copy link
Contributor

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

lgtm, smol ask

@0xferrous 0xferrous requested a review from onbjerg August 18, 2025 00:17
@zerosnacks
Copy link
Member

Implementation makes sense though I'm a bit concerned about leaking these internals into Foundry

@0xferrous
Copy link
Contributor Author

0xferrous commented Aug 18, 2025

What is the concern? I guess I can publish a mini crate that wraps this and exposes this functionality so the code stays the same as before? But I think security wise this is probably better to have the code in foundry and trust only the official clap crates.

@zerosnacks
Copy link
Member

That as you stated this logic is a concern of the library and not the implementation. That said, I am OK with merging this considering this is a small ask and the discussion upstream looks stale.

Copy link
Member

@zerosnacks zerosnacks left a comment

Choose a reason for hiding this comment

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

lgtm, pending @grandizzy

Copy link
Contributor

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

lgtm

@onbjerg onbjerg merged commit 4d8631d into foundry-rs:master Aug 21, 2025
22 checks passed
@github-project-automation github-project-automation bot moved this to Done in Foundry Aug 21, 2025
MerkleBoy pushed a commit to MerkleBoy/foundry that referenced this pull request Sep 17, 2025
* feat: add nushell completions

- `clap_complete` doesn't come with nushell completions built-in but they
  have another crate `clap_complete_nushell` that does. Sucks it isn't
  supported by default, see clap-rs/clap#5880.
  This PR implements a wrapper around the enum with nushell included to
  support nushell completions too.
- moved `clap_complete` to workspace dependency to prevent conflicting
  versions between foundry-common and cast/anvil/forge.

* chore: fix typo and clippy error

* chore: move clap_complete_nushell to workspace dependencies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants