mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-06-11 23:30:05 +02:00
Merge bitcoin/bitcoin#28802: ArgsManager: support command-specific options
89af67d79ftests: Add some fuzz test coverage for command-specific args (Anthony Towns)92df785859tests: Add some test coverage for ArgsManager::AddCommand (Anthony Towns)33c8090be9ArgsManager: automate checking for correct command options (Anthony Towns)186354a0d8bitcoin-wallet: use command-specific options (Anthony Towns)d21e82b7d6ArgsManager: support command-specific options (Anthony Towns) Pull request description: Adds the ability to link particular options to one or more (`OptionsCategory::COMMANDS`) commands, and uses this feature in `bitcoin-wallet`. Separates out the help information for these command-specific options (duplicating it if an option applies to multiple commands), and provides a function for checking at runtime if some options have been specified by the user that only apply to other commands. #### Motivation Currently, `ArgsManager` supports commands like `bitcoin-wallet dump` but while some of the options are command-specific (like `-dumpfile`), `ArgsManager` itself doesn't know that. As a result, `-dumpfile` is listed in the global help rather than under the relevant commands, and if you use `-dumpfile` with a different command that doesn't support it, `ArgsManager` cannot automatically report that as an error, resulting in the commands that don't support the option having to have error-handling specific to all the options they don't support. #### Changes Help output moves command-specific options under their associated commands: Before: ``` Options: -dumpfile=<file name> When used with 'dump', writes out the records to this file. When used with 'createfromdump', loads the records into a new wallet. ... Commands: createfromdump Create new wallet file from dumped records dump Print out all of the wallet key-value records ``` After: ``` Commands: createfromdump Create new wallet file from dumped records -dumpfile=<file name> When used with 'dump', writes out the records to this file. When used with 'createfromdump', loads the records into a new wallet. dump Print out all of the wallet key-value records -dumpfile=<file name> When used with 'dump', writes out the records to this file. When used with 'createfromdump', loads the records into a new wallet. ``` Error messages are now generated automatically by `ArgsManager` rather than ad-hoc wallet code for each option: Before: ```c++ if (args.IsArgSet("-dumpfile") && command != "dump" && command != "createfromdump") { tfm::format(std::cerr, "The -dumpfile option can only be used with the \"dump\" and \"createfromdump\" commands.\n"); return false; } ``` After: ```c++ std::vector<std::string> details; if (!args.CheckCommandOptions(command, &details)) { tfm::format(std::cerr, "Error: Invalid arguments provided:\n%s\n", util::MakeUnorderedList(details)); return false; } ``` #### Limitations - If an option applies to multiple commands, it shares the same help text. There's no way to provide per-command descriptions. - Option parsing rules are unchanged — options still cannot appear after the command. ACKs for top commit: achow101: ACK89af67d79fsedited: Re-ACK89af67d79fryanofsky: Code review ACK89af67d79f. Since last review: rebase, integration with ClearArgs and fuzz test, and Assume -> Assert switch Tree-SHA512: 7ae7c3b74d0c8c4db8459e9f0b9c7498b2fa4758954ec49983decbba177877b039779f0f7b55e60c3a0ed74c5e9e4ac4734ba9e049bf3a7743280ef8300869fa
This commit is contained in:
@@ -421,6 +421,7 @@ class ToolWalletTest(BitcoinTestFramework):
|
||||
assert not (self.nodes[0].wallets_path / "legacy").exists()
|
||||
self.assert_raises_tool_error("Invalid parameter -descriptors", "-wallet=legacy", "-descriptors=false", "create")
|
||||
assert not (self.nodes[0].wallets_path / "legacy").exists()
|
||||
self.assert_raises_tool_error("The -dumpfile option cannot be used with the 'create' command.", "-wallet=legacy", "-dumpfile=wallet.dump", "create")
|
||||
|
||||
def test_no_create_unnamed(self):
|
||||
self.log.info("Test that unnamed (default) wallets cannot be created")
|
||||
|
||||
Reference in New Issue
Block a user