Skip to content
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

[token-cli] upgrade to clap-v3 #4511

Closed
samkim-crypto opened this issue Jun 8, 2023 · 4 comments
Closed

[token-cli] upgrade to clap-v3 #4511

samkim-crypto opened this issue Jun 8, 2023 · 4 comments
Labels
stale [bot only] Added to stale content; will be closed soon

Comments

@samkim-crypto
Copy link
Contributor

Problem

There has been useful updates to solana-clap-v3-utils in the monorepo, but the token-cli still uses clap-v2. For example:

Proposed changes

Upgrade token-cli to use clap v3 and solana-clap-v3-utils in a sequence of steps suggested in the clap change logs.

Migrating
From clap v2

  1. Add CLI tests, -h and --help output at a minimum (recommendation: trycmd for snapshot testing)
  2. Update your dependency
    i. If you use no-default-features: add the std feature
  3. Resolve compiler errors
  4. Resolve behavior changes
    i. Refactor your App creation to a function and add a test similar to the one below, resolving any of its assertions
    ii. Look over the "subtle changes" under BREAKING CHANGES
    iii. If using builder: test your application under various circumstances to see if ArgMatches asserts regarding AllowInvalidUtf8.
  5. At your leisure: resolve deprecation notices

Status:

@samkim-crypto
Copy link
Contributor Author

samkim-crypto commented Jun 8, 2023

From clap change logs v3.0.0:

Subtle changes (i.e. compiler won't catch):

@samkim-crypto
Copy link
Contributor Author

samkim-crypto commented Jun 8, 2023

From clap change logs v3.0.0:

Easier to catch changes:

  • When using no-default-features, you now have to specify the std feature (reserved for future work)
  • Gated env support behind env feature flag
  • Gated crate information behind cargo feature flag
    • Impacts crate_name!, crate_version!, crate_authors!, crate_description!, app_from_crate!
  • AppSettings::StrictUtf8 is now default behaviour and asserts if AppSettings::AllowInvalidUtf8ForExternalSubcommands and ArgSettings::AllowInvalidUtf8 and ArgMatches::value_of_os aren't used together
  • Arg::short and Arg::value_delimiter now take a char instead of a &str
  • ArgMatches panics on unknown arguments
  • Removed VersionlessSubcommands, making it the default (see Please default to DisableVersionForSubcommands clap-rs/clap#2812)
  • Completion generation has been split out into clap_complete.
  • Removed ArgSettings::EmptyValues in favor of ArgSettings::ForbidEmptyValues
  • Validator signatures have been loosed:
    • Arg::validator now takes first argument as Fn(&str) -> Result<O, E: ToString> instead of Fn(String) -> Result<(), String>
    • Arg::validator_os now takes first argument as Fn(&OsStr) -> Result<O, OsString> instead of Fn(&OsStr) -> Result<(), OsString>
    • COMPLETED in [token-cli] upgrade to clap-v3 #4506 commit: 3171490
  • Arg::value_name now sets, rather than appends (see value_name appends, rather than overrides clap-rs/clap#2634)
  • Upgrade yaml-rust from 0.3 to 0.4
  • Replaced ArgGroup::from(BTreeMap) to ArgGroup::from(yaml)
  • Replaced ArgMatches::usage with App::generate_usage
  • Replaced Arg::settings with Arg::setting(Setting1 | Setting2)
  • App and Arg now need only one lifetime
  • Removed deprecated App::with_defaults, replaced with app_from_crate
  • Removed deprecated AppSettings::PropagateGlobalValuesDown (now the default)
  • Some App functions, like App::write_help now take &mut self instead of &self
  • Error::message is now private, use Error::to_string
  • Arg::default_value_if, Arg::default_value_if_os, Arg::default_value_ifs, Arg::default_value_ifs_os now takes the default value parameter as an option (unset default value if some other Arg is present (or has a specific value) clap-rs/clap#1406)
  • Changed App::print_help & App::print_long_help to now return std::io::Result
  • Changed App::write_help & App::write_long_help to now return std::io::Result
  • Changed Arg::index, Arg::number_of_values, Arg::min_values, Arg::max_values to taking usize instead of u64
  • Changed Error::info to type Vec<String> instead of Option<Vec<String>>
  • Changed ArgMatches::subcommand to now return Option<(&str, &ArgMatches)>
  • Renamed ErrorKind::MissingArgumentOrSubcommand to ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand
  • Renamed ErrorKind::HelpDisplayed to ErrorKind::DisplayHelp
  • Renamed ErrorKind::VersionDisplayed to ErrorKind::DisplayVersion
  • Added #[non_exhaustive] to clap::{ValueHint, ErrorKind, AppSettings, ArgSettings} (fix!: Allow adding new enum variants clap-rs/clap#3167)

@samkim-crypto
Copy link
Contributor Author

From clap change logs v3.1.0:

Compatibility:
Changes in behavior of note that are not guaranteed to be compatible across releases:

Deprecations:

@samkim-crypto
Copy link
Contributor Author

From clap change logs v3.2.0:

Compatibility
MSRV is now 1.56.0 (#3732)

Behavior

Moving (old location deprecated)

Replaced

@github-actions github-actions bot added the stale [bot only] Added to stale content; will be closed soon label Jun 10, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale [bot only] Added to stale content; will be closed soon
Projects
None yet
Development

No branches or pull requests

1 participant