Conversation
Replace clap derive macros with facet-args, a lightweight alternative from the facet ecosystem. This uses git dependencies for now as the published version doesn't export the help module yet. Closes #13
There was a problem hiding this comment.
Pull request overview
This PR migrates the CLI argument parsing from clap to facet-args, replacing derive macros and adding manual help text handling. The changes address issue #13 by switching to the facet ecosystem for argument parsing.
Key changes:
- Replaced
clap::Parserderive withfacet::Facetand updated all argument attributes - Added manual
--helphandling since facet-args doesn't intercept it automatically - Updated dependencies to use git sources for facet packages
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| fontcull-cli/src/main.rs | Replaced clap imports and derive macros with facet equivalents, converted all argument attributes to facet syntax, and added manual help handling logic |
| fontcull-cli/Cargo.toml | Removed clap dependency and added git-based facet and facet-args dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # CLI dependencies | ||
| clap = { version = "4", features = ["derive"] } | ||
| facet = { git = "https://github.com/facet-rs/facet" } | ||
| facet-args = { git = "https://github.com/facet-rs/facet" } |
There was a problem hiding this comment.
Git dependencies lack version pinning. Consider adding rev, tag, or branch specifiers to ensure reproducible builds and avoid unexpected breaking changes.
| facet-args = { git = "https://github.com/facet-rs/facet" } | |
| facet-args = { git = "https://github.com/facet-rs/facet", rev = "a1b2c3d4e5f6g7h8i9j0k" } |
| let raw_args: Vec<String> = std::env::args().skip(1).collect(); | ||
| if raw_args.iter().any(|a| a == "--help" || a == "-h") { |
There was a problem hiding this comment.
The raw_args vector is collected but only used for the help check. Consider using std::env::args().skip(1).any(|a| a == \"--help\" || a == \"-h\") to avoid the unnecessary allocation.
| let raw_args: Vec<String> = std::env::args().skip(1).collect(); | |
| if raw_args.iter().any(|a| a == "--help" || a == "-h") { | |
| if std::env::args().skip(1).any(|a| a == "--help" || a == "-h") { |
Summary
Closes #13
Test plan
cargo build --package fontcull-clicompiles successfullyfontcull --helpdisplays help textfontcull --unknownshows error for unknown flags