-
Notifications
You must be signed in to change notification settings - Fork 481
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
Bump Clap 3 -> 4 #991
Bump Clap 3 -> 4 #991
Conversation
🦋 Changeset detectedLatest commit: e37dd32 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all
value and ship?
"fish" => Ok(Box::from(super::fish::Fish)), | ||
"powershell" => Ok(Box::from(super::powershell::PowerShell)), | ||
shell_type => Err(format!("I don't know the shell type of {shell_type:?}",)), | ||
impl From<Shells> for Box<dyn Shell> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a first good step, in a later PR we can remove Box<dyn Shell>
entirely by implementing Shell
for Shells
and forward the calls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So kinda like this?
7781b5d
This reverts commit 7781b5d.
as far as i can tell, this was broken by the clap upgrade (Schniz#991).
as far as i can tell, this was broken by the clap upgrade (Schniz#991).
* fix(#1006): panic on completions as far as i can tell, this was broken by the clap upgrade (#991). * fix: add smoke test for completions * skip test on windows * Create dull-bottles-try.md --------- Co-authored-by: Gal Schlezinger <[email protected]>
Based on #823, and adjusted breaking-changes.
Mostly drop
possible_values
in favor of usingenum
s derivingValueEnum
.