-
Notifications
You must be signed in to change notification settings - Fork 98
Grouped flags #2766
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
Grouped flags #2766
Conversation
This is what it looked like previously: (74 flags!) compared to 48 grouped!
|
@@ -126,6 +127,35 @@ func AddNetworkFlagsToCmd(cmd *cobra.Command, networkFlags *NetworkFlags, addEnd | |||
} | |||
} | |||
|
|||
func GetNetworkFlagsGroup(cmd *cobra.Command, networkFlags *NetworkFlags, addEndpoint bool, supportedNetworkOptions []NetworkOption) flags.GroupedFlags { | |||
return flags.RegisterFlagGroup(cmd, "Network Flags (Select One)", "show-network-flags", true, func(set *pflag.FlagSet) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
network group is always visible
@@ -126,6 +128,35 @@ func AddNetworkFlagsToCmd(cmd *cobra.Command, networkFlags *NetworkFlags, addEnd | |||
} | |||
} | |||
|
|||
func GetNetworkFlagsGroup(cmd *cobra.Command, networkFlags *NetworkFlags, addEndpoint bool, supportedNetworkOptions []NetworkOption) flags.GroupedFlags { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should replace the existing AddNetworkFlagsToCmd command that is used by other commands right now
|
||
func flagExists(target string, args []string) bool { | ||
for _, arg := range args { | ||
if arg == target { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how do we handle option casing (upper case\lower case) in CLI in general?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good question, we always put lowercase for flags in CLI. Not sure if cobra (the CLI SDK that we use) automatically does this for us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
found this https://github.com/spf13/pflag/blob/master/flag.go#L224 so i assume if we don't do anything special, the pflag
pkg will honor the casing we passed in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - leaving to @felipemadero for the final stamp
cmd/blockchaincmd/deploy.go
Outdated
requiredArgCount := 1 | ||
if len(args) != requiredArgCount { | ||
_ = cmd.Help() // show full help with flag grouping | ||
return utils.ErrWrongArgCount(requiredArgCount, len(args)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also probably this error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean move utils.ErrWrongArgCount into cobrautils.ErrWrongArgCount
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is done in #2803
} | ||
return err | ||
return nil | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remaining functions will be changed as we refactor more of the codebase
err := cobra.ExactArgs(n)(cmd, args) | ||
if err != nil { | ||
err = NewUsageError(cmd, err) | ||
if len(args) != n { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change is required, because otherwise this is what it looks like
AVL-7H2W7V:avalanche-cli raymondsukanto$ ./bin/avalanche blockchain deploy
Usage:
avalanche blockchain deploy [blockchainName] [flags]
Flags:
--aggregator-log-level string log level to use with signature aggregator (default "Debug")
--aggregator-log-to-stdout use stdout for signature aggregator logs
--auth-keys strings control keys that will be used to authenticate chain creation
--avalanchego-path string use this avalanchego binary path
--avalanchego-version string use this version of avalanchego (ex: v1.17.12) (default "latest")
--balance float set the AVAX balance of each bootstrap validator that will be used for continuous fee on P-Chain (default 0.1)
--bootstrap-endpoints strings take validator node info from the given endpoints
--bootstrap-filepath string JSON file path that provides details about bootstrap validators, leave Node-ID and BLS values empty if using --generate-node-id=true
--change-owner-address string address that will receive change if node is no longer L1 validator
--control-keys strings addresses that may make blockchain changes
--convert-only avoid node track, restart and poa manager setup
-e, --ewoq use ewoq key [local/devnet deploy only]
--generate-node-id whether to create new node id for bootstrap validators (Node-ID and BLS values in bootstrap JSON file will be overridden if --bootstrap-filepath flag is used)
-h, --help help for deploy
--http-port uints http port for node(s) (default [])
-k, --key string select the key to use [fuji/devnet deploy only]
-g, --ledger use ledger instead of key (always true on mainnet, defaults to false on fuji/devnet)
--ledger-addrs strings use the given ledger addresses
--mainnet-chain-id uint32 use different ChainID for mainnet deployment
--num-bootstrap-validators int (only if --generate-node-id is true) number of bootstrap validators to set up in sovereign L1 validator)
--num-local-nodes int number of nodes to be created on local machine
--num-nodes uint32 number of nodes to be created on local network deploy (default 2)
--output-tx-path string file path of the blockchain creation tx
--partial-sync set primary network partial sync for new validators (default true)
-s, --same-control-key use the fee-paying key as control key
--staking-port uints staking port for node(s) (default [])
-u, --subnet-id string do not create a subnet, deploy the blockchain into the given subnet id
--subnet-only only create a subnet
--threshold uint32 required number of control key signatures to make blockchain changes
--use-local-machine use local machine as a blockchain validator
Global Flags:
--config string config file (default is $HOME/.avalanche-cli/config.json)
--log-level string log level for the application (default "ERROR")
--skip-update-check skip check for new versions
Network Flags (Select One):
--cluster string operate on the given cluster
--devnet operate on a devnet network
--endpoint string use the given endpoint for network operations
--fuji operate on fuji (alias to `testnet`)
--local operate on a local network
--mainnet operate on mainnet
--testnet operate on testnet (alias to `fuji`)
ICM Flags:
(hidden) Use --show-icm-flags to show these options
Proof Of Stake Flags:
(hidden) Use --show-pos-flags to show these options
Usage:
avalanche blockchain deploy [blockchainName] [flags]
Flags:
--aggregator-log-level string log level to use with signature aggregator (default "Debug")
--aggregator-log-to-stdout use stdout for signature aggregator logs
--auth-keys strings control keys that will be used to authenticate chain creation
--avalanchego-path string use this avalanchego binary path
--avalanchego-version string use this version of avalanchego (ex: v1.17.12) (default "latest")
--balance float set the AVAX balance of each bootstrap validator that will be used for continuous fee on P-Chain (default 0.1)
--bootstrap-endpoints strings take validator node info from the given endpoints
--bootstrap-filepath string JSON file path that provides details about bootstrap validators, leave Node-ID and BLS values empty if using --generate-node-id=true
--change-owner-address string address that will receive change if node is no longer L1 validator
--control-keys strings addresses that may make blockchain changes
--convert-only avoid node track, restart and poa manager setup
-e, --ewoq use ewoq key [local/devnet deploy only]
--generate-node-id whether to create new node id for bootstrap validators (Node-ID and BLS values in bootstrap JSON file will be overridden if --bootstrap-filepath flag is used)
-h, --help help for deploy
--http-port uints http port for node(s) (default [])
-k, --key string select the key to use [fuji/devnet deploy only]
-g, --ledger use ledger instead of key (always true on mainnet, defaults to false on fuji/devnet)
--ledger-addrs strings use the given ledger addresses
--mainnet-chain-id uint32 use different ChainID for mainnet deployment
--num-bootstrap-validators int (only if --generate-node-id is true) number of bootstrap validators to set up in sovereign L1 validator)
--num-local-nodes int number of nodes to be created on local machine
--num-nodes uint32 number of nodes to be created on local network deploy (default 2)
--output-tx-path string file path of the blockchain creation tx
--partial-sync set primary network partial sync for new validators (default true)
-s, --same-control-key use the fee-paying key as control key
--staking-port uints staking port for node(s) (default [])
-u, --subnet-id string do not create a subnet, deploy the blockchain into the given subnet id
--subnet-only only create a subnet
--threshold uint32 required number of control key signatures to make blockchain changes
--use-local-machine use local machine as a blockchain validator
Global Flags:
--config string config file (default is $HOME/.avalanche-cli/config.json)
--log-level string log level for the application (default "ERROR")
--skip-update-check skip check for new versions
Usage error: accepts 1 arg(s), received 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add an empty line between the help and the error message?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed
* remove duplicate flags in deploy * lint
Grouped flags for blockchain deploy cmd, which will be extended to other commands.
Grouping flags cuts down on the number of flags displayed to users and enables users to discover which functionality (ICM / POS config) a flag serves.
This PR currently only groups ICM flags and POS flags. We can group more flags together: network flags, control & auth flags (ledger, keys, control keys, etc), non sov flags, etc. This flags will only be grouped, but never hidden.
This is what it looks like now: (we cut it down from 74 flags to 48 flags!)
To show hidden grouped flags: