[keygen] Remove deprecated functions from keygen cli (minus grind command)
#438
Conversation
keygen/src/keygen.rs
Outdated
| let source = if matches.try_contains_id("keypair")? { | ||
| Cow::Borrowed(matches.get_one::<SignerSource>("keypair").unwrap()) | ||
| } else if !config.keypair_path.is_empty() { | ||
| &config.keypair_path | ||
| Cow::Owned(SignerSource::parse(&config.keypair_path)?) | ||
| } else { | ||
| let mut path = dirs_next::home_dir().expect("home directory"); | ||
| path.extend([".config", "solana", "id.json"]); | ||
| path.to_str().unwrap() | ||
| Cow::Owned(SignerSource::parse(path.to_str().unwrap())?) | ||
| }; |
There was a problem hiding this comment.
Not obvious to me: why is the Cow wrapping necessary? If this will be a common need, it's unfortunate ergonomics.
There was a problem hiding this comment.
Yeah, this is unfortunate ergonomics. The issue is that matches.get_one::<SignerSource>("keypair").unwrap() gives a reference &SignerSource while SignerSource::parse(...).unwrap() gives a fully owned SignerSource. Returning &SignerSource::parse(...).unwrap() is not possible since the owned type will be freed out of scope.
The alternative would be to define an owned variable outside of the if statement, set it to SignerSource::parse(...).unwrap(), and return a reference to it. I thought this was less elegant than using Cow at first, but it seems like it could be cleaner, so I made the change. Please take a look.
There was a problem hiding this comment.
Thanks, I agree the current version is a little cleaner.
It's actually odd to me that ArgMatches::get_one returns a reference instead of an owned object. I suppose I should read the clap code to see why.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #438 +/- ##
=========================================
- Coverage 81.9% 81.8% -0.1%
=========================================
Files 840 841 +1
Lines 228107 228265 +158
=========================================
+ Hits 186851 186931 +80
- Misses 41256 41334 +78 |
Problem
The keygen cli uses clap-v3 and solana-clap-v3-utils, but still uses deprecated functions from clap-v2.
Summary of Changes
Replaced deprecated clap-v2 functions with clap-v3 functions. I organized the changes so that each commit is associated with different commands.
This PR removes all the deprecated functions for all commands except for the
grindcommand. Thegrindcommand seems like it will need a bit more structural changes, so I will make the changes in a follow-up PR.The first and last commits here adds and removes the
deprecatedfeature from clap-v3. If this feature is set, then clap-v3 warns on the use of deprecated functions. I removed this feature since thegrindcommand is not yet updated.Fixes #