Skip to content

Commit

Permalink
Improve Client CLI help readability (#2073)
Browse files Browse the repository at this point in the history
Currently the CLI `-h/--help` commad output is almost unreadable as (for
some commands) it:
- doesn't provide a short brief of what the command does.
- doesn't separate the options description in smaller paragraphs.
- doesn't use a smart wrap strategy for lines longer than the number of
columns in the terminal.

Follow some pics taken with a 100 cols wide term

## Short help (./node -h)

### Before


![20231028-174531-grim](https://github.com/paritytech/polkadot-sdk/assets/8143589/11b62c3c-dcd5-43f4-ac58-f1b299e3f4b9)

### After


![20231028-175041-grim](https://github.com/paritytech/polkadot-sdk/assets/8143589/dc08f6fd-b287-40fb-8b33-71a185922104)


## Long help (./node --help)

### Before


![20231028-175257-grim](https://github.com/paritytech/polkadot-sdk/assets/8143589/9ebdc0ae-54ee-4760-b873-a7e813523cb6)

### After


![20231028-175155-grim](https://github.com/paritytech/polkadot-sdk/assets/8143589/69cbe5cb-eb2f-46a5-8ebf-76c0cf8c4bad)

---------

Co-authored-by: command-bot <>
  • Loading branch information
davxy authored Oct 29, 2023
1 parent 8ce16ee commit 7035034
Show file tree
Hide file tree
Showing 16 changed files with 190 additions and 97 deletions.
11 changes: 11 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 18 additions & 15 deletions polkadot/cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,29 +84,29 @@ pub struct RunCmd {

/// Setup a GRANDPA scheduled voting pause.
///
/// This parameter takes two values, namely a block number and a delay (in
/// blocks). After the given block number is finalized the GRANDPA voter
/// will temporarily stop voting for new blocks until the given delay has
/// elapsed (i.e. until a block at height `pause_block + delay` is imported).
/// This parameter takes two values, namely a block number and a delay (in blocks).
///
/// After the given block number is finalized the GRANDPA voter will temporarily
/// stop voting for new blocks until the given delay has elapsed (i.e. until a
/// block at height `pause_block + delay` is imported).
#[arg(long = "grandpa-pause", num_args = 2)]
pub grandpa_pause: Vec<u32>,

/// Disable the BEEFY gadget
/// (currently enabled by default on Rococo, Wococo and Versi).
/// Disable the BEEFY gadget.
///
/// Currently enabled by default on 'Rococo', 'Wococo' and 'Versi'.
#[arg(long)]
pub no_beefy: bool,

/// Add the destination address to the jaeger agent.
/// Add the destination address to the 'Jaeger' agent.
///
/// Must be valid socket address, of format `IP:Port`
/// commonly `127.0.0.1:6831`.
/// Must be valid socket address, of format `IP:Port` (commonly `127.0.0.1:6831`).
#[arg(long)]
pub jaeger_agent: Option<String>,

/// Add the destination address to the `pyroscope` agent.
///
/// Must be valid socket address, of format `IP:Port`
/// commonly `127.0.0.1:4040`.
/// Must be valid socket address, of format `IP:Port` (commonly `127.0.0.1:4040`).
#[arg(long)]
pub pyroscope_server: Option<String>,

Expand All @@ -126,10 +126,13 @@ pub struct RunCmd {
#[arg(long)]
pub overseer_channel_capacity_override: Option<usize>,

/// Path to the directory where auxiliary worker binaries reside. If not specified, the main
/// binary's directory is searched first, then `/usr/lib/polkadot` is searched. TESTING ONLY:
/// if the path points to an executable rather then directory, that executable is used both as
/// preparation and execution worker.
/// Path to the directory where auxiliary worker binaries reside.
///
/// If not specified, the main binary's directory is searched first, then
/// `/usr/lib/polkadot` is searched.
///
/// TESTING ONLY: if the path points to an executable rather then directory,
/// that executable is used both as preparation and execution worker.
#[arg(long, value_name = "PATH")]
pub workers_path: Option<PathBuf>,

Expand Down
2 changes: 1 addition & 1 deletion substrate/client/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ targets = ["x86_64-unknown-linux-gnu"]
[dependencies]
array-bytes = "6.1"
chrono = "0.4.27"
clap = { version = "4.4.6", features = ["derive", "string"] }
clap = { version = "4.4.6", features = ["derive", "string", "wrap_help"] }
fdlimit = "0.2.1"
futures = "0.3.21"
libp2p-identity = { version = "0.1.3", features = ["peerid", "ed25519"]}
Expand Down
69 changes: 46 additions & 23 deletions substrate/client/cli/src/commands/run_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,35 +40,38 @@ use std::net::{IpAddr, Ipv4Addr, SocketAddr};
#[derive(Debug, Clone, Parser)]
pub struct RunCmd {
/// Enable validator mode.
///
/// The node will be started with the authority role and actively
/// participate in any consensus task that it can (e.g. depending on
/// availability of local keys).
#[arg(long)]
pub validator: bool,

/// Disable GRANDPA voter when running in validator mode, otherwise disable the GRANDPA
/// Disable GRANDPA.
///
/// Disables voter when running in validator mode, otherwise disable the GRANDPA
/// observer.
#[arg(long)]
pub no_grandpa: bool,

/// Listen to all RPC interfaces.
/// Default is local. Note: not all RPC methods are safe to be exposed publicly. Use an RPC
/// proxy server to filter out dangerous methods. More details:
/// Listen to all RPC interfaces (default: local).
///
/// Not all RPC methods are safe to be exposed publicly.
///
/// Use an RPC proxy server to filter out dangerous methods. More details:
/// <https://docs.substrate.io/main-docs/build/custom-rpc/#public-rpcs>.
///
/// Use `--unsafe-rpc-external` to suppress the warning if you understand the risks.
#[arg(long)]
pub rpc_external: bool,

/// Listen to all RPC interfaces.
///
/// Same as `--rpc-external`.
#[arg(long)]
pub unsafe_rpc_external: bool,

/// RPC methods to expose.
/// - `unsafe`: Exposes every RPC method.
/// - `safe`: Exposes only a safe subset of RPC methods, denying unsafe RPC methods.
/// - `auto`: Acts as `safe` if RPC is served externally, e.g. when `--rpc--external` is
/// passed, otherwise acts as `unsafe`.
#[arg(
long,
value_name = "METHOD SET",
Expand All @@ -79,15 +82,15 @@ pub struct RunCmd {
)]
pub rpc_methods: RpcMethods,

/// Set the the maximum RPC request payload size for both HTTP and WS in megabytes.
/// Set the maximum RPC request payload size for both HTTP and WS in megabytes.
#[arg(long, default_value_t = RPC_DEFAULT_MAX_REQUEST_SIZE_MB)]
pub rpc_max_request_size: u32,

/// Set the the maximum RPC response payload size for both HTTP and WS in megabytes.
/// Set the maximum RPC response payload size for both HTTP and WS in megabytes.
#[arg(long, default_value_t = RPC_DEFAULT_MAX_RESPONSE_SIZE_MB)]
pub rpc_max_response_size: u32,

/// Set the the maximum concurrent subscriptions per connection.
/// Set the maximum concurrent subscriptions per connection.
#[arg(long, default_value_t = RPC_DEFAULT_MAX_SUBS_PER_CONN)]
pub rpc_max_subscriptions_per_connection: u32,

Expand All @@ -99,15 +102,17 @@ pub struct RunCmd {
#[arg(long, value_name = "COUNT", default_value_t = RPC_DEFAULT_MAX_CONNECTIONS)]
pub rpc_max_connections: u32,

/// Specify browser Origins allowed to access the HTTP & WS RPC servers.
/// A comma-separated list of origins (protocol://domain or special `null`
/// Specify browser *origins* allowed to access the HTTP and WS RPC servers.
///
/// A comma-separated list of origins (`protocol://domain` or special `null`
/// value). Value of `all` will disable origin validation. Default is to
/// allow localhost and <https://polkadot.js.org> origins. When running in
/// --dev mode the default is to allow all origins.
/// `--dev` mode the default is to allow all origins.
#[arg(long, value_name = "ORIGINS", value_parser = parse_cors)]
pub rpc_cors: Option<Cors>,

/// The human-readable name for this node.
///
/// It's used as network node name.
#[arg(long, value_name = "NAME")]
pub name: Option<String>,
Expand Down Expand Up @@ -148,36 +153,51 @@ pub struct RunCmd {
#[clap(flatten)]
pub keystore_params: KeystoreParams,

/// Shortcut for `--name Alice --validator` with session keys for `Alice` added to keystore.
/// Shortcut for `--name Alice --validator`.
///
/// Session keys for `Alice` are added to keystore.
#[arg(long, conflicts_with_all = &["bob", "charlie", "dave", "eve", "ferdie", "one", "two"])]
pub alice: bool,

/// Shortcut for `--name Bob --validator` with session keys for `Bob` added to keystore.
/// Shortcut for `--name Bob --validator`.
///
/// Session keys for `Bob` are added to keystore.
#[arg(long, conflicts_with_all = &["alice", "charlie", "dave", "eve", "ferdie", "one", "two"])]
pub bob: bool,

/// Shortcut for `--name Charlie --validator` with session keys for `Charlie` added to
/// keystore.
/// Shortcut for `--name Charlie --validator`.
///
/// Session keys for `Charlie` are added to keystore.
#[arg(long, conflicts_with_all = &["alice", "bob", "dave", "eve", "ferdie", "one", "two"])]
pub charlie: bool,

/// Shortcut for `--name Dave --validator` with session keys for `Dave` added to keystore.
/// Shortcut for `--name Dave --validator`.
///
/// Session keys for `Dave` are added to keystore.
#[arg(long, conflicts_with_all = &["alice", "bob", "charlie", "eve", "ferdie", "one", "two"])]
pub dave: bool,

/// Shortcut for `--name Eve --validator` with session keys for `Eve` added to keystore.
/// Shortcut for `--name Eve --validator`.
///
/// Session keys for `Eve` are added to keystore.
#[arg(long, conflicts_with_all = &["alice", "bob", "charlie", "dave", "ferdie", "one", "two"])]
pub eve: bool,

/// Shortcut for `--name Ferdie --validator` with session keys for `Ferdie` added to keystore.
/// Shortcut for `--name Ferdie --validator`.
///
/// Session keys for `Ferdie` are added to keystore.
#[arg(long, conflicts_with_all = &["alice", "bob", "charlie", "dave", "eve", "one", "two"])]
pub ferdie: bool,

/// Shortcut for `--name One --validator` with session keys for `One` added to keystore.
/// Shortcut for `--name One --validator`.
///
/// Session keys for `One` are added to keystore.
#[arg(long, conflicts_with_all = &["alice", "bob", "charlie", "dave", "eve", "ferdie", "two"])]
pub one: bool,

/// Shortcut for `--name Two --validator` with session keys for `Two` added to keystore.
/// Shortcut for `--name Two --validator`.
///
/// Session keys for `Two` are added to keystore.
#[arg(long, conflicts_with_all = &["alice", "bob", "charlie", "dave", "eve", "ferdie", "one"])]
pub two: bool,

Expand All @@ -186,10 +206,13 @@ pub struct RunCmd {
pub force_authoring: bool,

/// Run a temporary node.
///
/// A temporary directory will be created to store the configuration and will be deleted
/// at the end of the process.
///
/// Note: the directory is random per process execution. This directory is used as base path
/// which includes: database, node key and keystore.
///
/// When `--dev` is given and no explicit `--base-path`, this option is implied.
#[arg(long, conflicts_with = "base_path")]
pub tmp: bool,
Expand Down
19 changes: 9 additions & 10 deletions substrate/client/cli/src/params/import_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ pub struct ImportParams {
pub wasm_method: WasmExecutionMethod,

/// The WASM instantiation method to use.
///
/// Only has an effect when `wasm-execution` is set to `compiled`.
/// The copy-on-write strategies are only supported on Linux.
/// If the copy-on-write variant of a strategy is unsupported
Expand All @@ -65,6 +66,7 @@ pub struct ImportParams {
pub wasmtime_instantiation_strategy: WasmtimeInstantiationStrategy,

/// Specify the path where local WASM runtimes are stored.
///
/// These runtimes will override on-chain runtimes when the version matches.
#[arg(long, value_name = "PATH")]
pub wasm_runtime_overrides: Option<PathBuf>,
Expand All @@ -74,12 +76,12 @@ pub struct ImportParams {
pub execution_strategies: ExecutionStrategiesParams,

/// Specify the state cache size.
///
/// Providing `0` will disable the cache.
#[arg(long, value_name = "Bytes", default_value_t = 67108864)]
pub trie_cache_size: usize,

/// DEPRECATED
/// Switch to `--trie-cache-size`.
/// DEPRECATED: switch to `--trie-cache-size`.
#[arg(long)]
state_cache_size: Option<usize>,
}
Expand Down Expand Up @@ -115,26 +117,23 @@ impl ImportParams {
/// Execution strategies parameters.
#[derive(Debug, Clone, Args)]
pub struct ExecutionStrategiesParams {
/// The means of execution used when calling into the runtime for importing blocks as
/// part of an initial sync.
/// Runtime execution strategy for importing blocks during initial sync.
#[arg(long, value_name = "STRATEGY", value_enum, ignore_case = true)]
pub execution_syncing: Option<ExecutionStrategy>,

/// The means of execution used when calling into the runtime for general block import
/// (including locally authored blocks).
/// Runtime execution strategy for general block import (including locally authored blocks).
#[arg(long, value_name = "STRATEGY", value_enum, ignore_case = true)]
pub execution_import_block: Option<ExecutionStrategy>,

/// The means of execution used when calling into the runtime while constructing blocks.
/// Runtime execution strategy for constructing blocks.
#[arg(long, value_name = "STRATEGY", value_enum, ignore_case = true)]
pub execution_block_construction: Option<ExecutionStrategy>,

/// The means of execution used when calling into the runtime while using an off-chain worker.
/// Runtime execution strategy for offchain workers.
#[arg(long, value_name = "STRATEGY", value_enum, ignore_case = true)]
pub execution_offchain_worker: Option<ExecutionStrategy>,

/// The means of execution used when calling into the runtime while not syncing, importing or
/// constructing blocks.
/// Runtime execution strategy when not syncing, importing or constructing blocks.
#[arg(long, value_name = "STRATEGY", value_enum, ignore_case = true)]
pub execution_other: Option<ExecutionStrategy>,

Expand Down
5 changes: 3 additions & 2 deletions substrate/client/cli/src/params/keystore_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@ pub struct KeystoreParams {
#[arg(long, conflicts_with_all = &["password", "password_filename"])]
pub password_interactive: bool,

/// Password used by the keystore. This allows appending an extra user-defined secret to the
/// seed.
/// Password used by the keystore.
///
/// This allows appending an extra user-defined secret to the seed.
#[arg(
long,
value_parser = secret_string_from_str,
Expand Down
Loading

0 comments on commit 7035034

Please sign in to comment.