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

update clap to stable release #64

Closed
tzemanovic opened this issue Jan 28, 2022 · 5 comments · Fixed by #1580
Closed

update clap to stable release #64

tzemanovic opened this issue Jan 28, 2022 · 5 comments · Fixed by #1580
Assignees

Comments

@tzemanovic
Copy link
Member

tzemanovic commented Jan 28, 2022

we're using the beta version of clap 0.3, which now has stable release: https://github.com/clap-rs/clap/releases

┆Issue is synchronized with this Asana task by Unito

@tzemanovic
Copy link
Member Author

this might be a "good first issue" depending how many breaking changes there are

@tzemanovic tzemanovic added the good first issue Good for newcomers label Feb 3, 2022
@sync-by-unito sync-by-unito bot closed this as completed Feb 3, 2022
@juped juped reopened this Feb 3, 2022
@tzemanovic tzemanovic transferred this issue from anoma/anoma Jul 7, 2022
@cwgoes
Copy link
Contributor

cwgoes commented Jan 12, 2023

@juped is this still applicable?

@karbyshev
Copy link
Contributor

I have found that it is possible to update clap to v3.0.13 with minimal changes. See #1537

When building with versions >= 3.0.14, the following issue emerges: entries for sub-commands in the help message appear twice. For example, ./target/debug/namadaw --help prints

Namada v0.17.0-1-gc04d113-dirty
Namada wallet command line interface.

USAGE:
    namadaw [OPTIONS] <SUBCOMMAND>

OPTIONS:
        --base-dir <base-dir>    The base directory is where the nodes, client and wallet
                                 configuration and state is stored. This value can also be set via
                                 `NAMADA_BASE_DIR` environment variable, but the argument takes
                                 precedence, if specified. Defaults to `$XDG_DATA_HOME/namada`
                                 (`$HOME/.local/share/namada` where `XDG_DATA_HOME` is unset) on
                                 Unix,`$HOME/Library/Application Support/Namada` on Mac,and
                                 `%AppData%\Namada` on Windows.
        --chain-id <chain-id>    The chain ID.
    -h, --help                   Print help information
        --mode <mode>            The mode in which to run Namada. Options are
                                 	 * Validator (default)
                                 	 * Full
                                 	 * Seed
    -V, --version                Print version information
        --wasm-dir <wasm-dir>    Directory with built WASM validity predicates, transactions. This
                                 value can also be set via `NAMADA_WASM_DIR` environment variable,
                                 but the argument takes precedence, if specified.

SUBCOMMANDS:
    address    Address management, including methods to generate and look-up addresses.
    address    Address management, including methods to generate and look-up addresses.
    help       Print this message or the help of the given subcommand(s)
    key        Keypair management, including methods to generate and look-up keys.
    key        Keypair management, including methods to generate and look-up keys.
    masp       Multi-asset shielded pool address and keypair management including methods to
               generate and look-up addresses and keys.
    masp       Multi-asset shielded pool address and keypair management including methods to
               generate and look-up addresses and keys.

I could not figure out what changes in clap-rs/clap@v3.0.13...v3.0.14 are causing the problem.

@tzemanovic
Copy link
Member Author

Ah, looks like it's from a change in fn write_subcommands inside src/output/help.rs - it was previously using BTreeMap which was de-duplicating subcommands. It looks like in v4 they added some assertions to catch these, but there are bunch of breaking changes.

I found where we're inserting dups in our code in apps/src/lib/cli.rs:

  • in fn namada_node/client/wallet_app we're calling cmds::NamadaNode/Client/Wallet::add_sub
  • in fn namada_node/wallet_cli we're calling cmds::NamadaNode/Wallet::parse_or_print_help which again calls Self::add_sub(app) on the first line - I think we should remove this call
  • in fn namada_client_cli we're calling cmds::NamadaClient::add_sub(app) - let's also remove this one

@karbyshev
Copy link
Contributor

karbyshev commented Jun 14, 2023

@tzemanovic Thanks! The suggested changes have helped. It compiles now with the latest v3 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Tested in Devnet
4 participants