-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
feat(derive): derive clap::Args
for enum types
#5700
base: master
Are you sure you want to change the base?
Changes from 2 commits
f11570c
3ecaa53
3249e4d
75eb4ee
bd54e23
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
{ | ||
inputs.fenix = { | ||
url = "github:nix-community/fenix"; | ||
inputs.nixpkgs.follows = "nixpkgs"; | ||
}; | ||
|
||
outputs = | ||
{ nixpkgs, fenix, ... }: | ||
let | ||
pkgs = nixpkgs.legacyPackages.aarch64-darwin; | ||
in | ||
{ | ||
devShells.aarch64-darwin.default = pkgs.mkShell { | ||
shellHook = '' | ||
export RUST_SRC_PATH="${pkgs.rustPlatform.rustLibSrc}"; | ||
''; | ||
packages = [ | ||
pkgs.lldb | ||
pkgs.libiconv | ||
pkgs.darwin.apple_sdk.frameworks.Security | ||
pkgs.darwin.apple_sdk.frameworks.SystemConfiguration | ||
(fenix.packages.aarch64-darwin.combine [ | ||
fenix.packages.aarch64-darwin.stable.cargo | ||
fenix.packages.aarch64-darwin.stable.rust | ||
fenix.packages.aarch64-darwin.targets.wasm32-wasi.stable.rust-std | ||
fenix.packages.aarch64-darwin.targets.wasm32-wasip1.stable.rust-std | ||
fenix.packages.aarch64-darwin.targets.aarch64-apple-darwin.stable.rust-std | ||
]) | ||
]; | ||
}; | ||
|
||
formatter.aarch64-darwin = pkgs.nixfmt-rfc-style; | ||
|
||
}; | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -239,3 +239,57 @@ For more information, try '--help'. | |
"; | ||
assert_output::<Opt>("test", OUTPUT, true); | ||
} | ||
|
||
#[test] | ||
fn enum_groups_1() { | ||
#[derive(Parser, Debug, PartialEq, Eq)] | ||
struct Opt { | ||
#[command(flatten)] | ||
source: Source, | ||
} | ||
|
||
#[derive(clap::Args, Clone, Debug, PartialEq, Eq)] | ||
enum Source { | ||
A { | ||
#[arg(short)] | ||
a: bool, | ||
#[arg(long)] | ||
aaa: bool, | ||
}, | ||
B { | ||
#[arg(short)] | ||
b: bool, | ||
}, | ||
} | ||
|
||
assert_eq!( | ||
Opt { | ||
source: Source::A { | ||
a: true, | ||
aaa: false, | ||
} | ||
}, | ||
Opt::try_parse_from(["test", "-a"]).unwrap() | ||
); | ||
assert_eq!( | ||
Opt { | ||
source: Source::A { a: true, aaa: true } | ||
}, | ||
Opt::try_parse_from(["test", "-a", "--aaa"]).unwrap() | ||
); | ||
assert_eq!( | ||
Opt { | ||
source: Source::B { b: true } | ||
}, | ||
Opt::try_parse_from(["test", "-b"]).unwrap() | ||
); | ||
|
||
assert_eq!( | ||
clap::error::ErrorKind::ArgumentConflict, | ||
Opt::try_parse_from(["test", "-b", "-a"]) | ||
.unwrap_err() | ||
.kind(), | ||
); | ||
Comment on lines
+287
to
+292
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When adding a test commit separate from a feature, it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would i add a test that does compile for a thing that doesn't yet compile, or where the subject of the PR is allowing these constructs to compile in the first place? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "it depends" Let's step back to the goals of why we encourage tests in a commit before the feature
All of this is trivial when its a bug fix to help output. You show the existing behavior in the tests, then change the test when you change the behavior. When the behavior doesn't exist yet, your options are
The ideal is the last but sometimes there isn't always a close enough parallel and it can take the most work. As an example of the last, take https://github.com/rust-lang/cargo/pull/14435/commits. This adds a new feature to Cargo. Originally, the test commit used the new features which didn't exist and errored out quickly. The commit that implemented the feature then made them not error. This was still more helpful than having them in the same commit because I only had to review the test output and not all of the test. However, I found a parallel feature and suggested it to the contributor. The tests are now much easier to see what the intended behavior is. Thats easy when its all textual output and you aren't dealing with compile errors. One option is to write the tests with structs, rater than enums. The commit that adds the feature could also switch the structs to enums, causing some cases to still work while other cases will error. That might or might not be worth it. If that seems like it will be too much or not show enough, feel free to say so, squash the commits, and move on. |
||
|
||
// assert_eq!( ) | ||
} |
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.
don't mind this it just helps me get a rust compiler using nix...
will be removed before this is ready to go anywhere