Have gArgs handle printing help #13190

achow101 · 2018-05-08T18:27:28Z

Instead of each binary generating and printing it's own help string, have gArgs know what the args are and generate the help string.

This is the first commit of #13112 pulled out.

jnewbery · 2018-05-08T19:04:26Z

Concept ACK

maflcko · 2018-05-08T19:10:06Z

I guess you'd have to cherry-pick the fixup to contrib/check-doc as well.

maflcko

Tested ACK 3333c7ef48cfde42619b7f6ae62893de192ed574:

Bumped the man pages in a commit on current master, cherry-picked 3333c7ef48cfde42619b7f6ae62893de192ed574, Generated the man pages and checked that git diff only shows differences to to sort-by-name.
Additionally checked that the strings are move-only by hand
Did an utACK on the code changes.

maflcko · 2018-05-08T19:24:16Z

src/init.cpp

+    gArgs.AddArg("-banscore=<n>", strprintf(_("Threshold for disconnecting misbehaving peers (default: %u)"), DEFAULT_BANSCORE_THRESHOLD), false, OptionsCategory::CONNECTION);
+    gArgs.AddArg("-bantime=<n>", strprintf(_("Number of seconds to keep misbehaving peers from reconnecting (default: %u)"), DEFAULT_MISBEHAVING_BANTIME), false, OptionsCategory::CONNECTION);
+    gArgs.AddArg("-bind=<addr>", _("Bind to given address and always listen on it. Use [host]:port notation for IPv6"), false, OptionsCategory::CONNECTION);
+    gArgs.AddArg("-connect=<ip>", _("Connect only to the specified node(s, false, OptionsCategory::OPTIONS)); -connect=0 disables automatic connections (the rules for this peer are the same as for -addnode)"), false, OptionsCategory::CONNECTION);


Rebase issue? (Translation string was changed)

Oops, fixed

achow101 · 2018-05-08T19:49:05Z

I added a commit for the check-doc fix.

maflcko · 2018-05-08T19:52:56Z

re-ACK 3ea4d9080d615e09017c23e9678f47c9edddb869 (only changes were suggested fixups)

promag · 2018-05-08T22:08:39Z

utACK 3ea4d90.

Please squash otherwise check-doc.py fails for 1st commit.

achow101 · 2018-05-08T22:26:00Z

Squashed

sipa · 2018-05-09T01:50:18Z

utACK dadad0d559f62ff7d9752655dc4bbb922e97423d

I did not review all the command line help/AddArg statements, but I did compare the output of -help and -help -help-debug before and after.

meshcollider · 2018-05-09T06:16:17Z

Concept ACK, makes sense to split this out

maflcko · 2018-05-09T13:53:21Z

Needs rebase

gArgs knows what the available arguments are and their help. Getting the help message is moved to gArgs and HelpMessage() is removed

achow101 · 2018-05-09T16:21:14Z

Rebased

sipa

reutACK 4d4185a

Two nits below, but feel free to ignore. This is probably a pain to keep rebasing.

sipa · 2018-05-09T17:05:56Z

src/util.h

+    /**
+     * Get the help string
+     */
+    std::string GetHelpMessage();


I believe this can be marked const.

sipa · 2018-05-09T17:06:38Z

src/util.cpp

+    for (auto& arg : m_available_args) {
+        if (arg.first.first != last_cat) {
+            last_cat = arg.first.first;
+            if (last_cat == OptionsCategory::CONNECTION)


Code style nit: use braces when putting then/else branches on separate lines.

laanwj · 2018-05-09T17:23:16Z

Code review and lightly tested ACK 4d4185a

maflcko · 2018-05-09T18:33:45Z

re-ACK 4d4185a (only change was rebase)

4d4185a Make gArgs aware of the arguments (Andrew Chow) Pull request description: Instead of each binary generating and printing it's own help string, have gArgs know what the args are and generate the help string. This is the first commit of #13112 pulled out. Tree-SHA512: d794c872b834bc56c78d947549f9cf046a8174984ab0c7b4ba06bc51d36dca11a4ed88afafe76bb4f776cdba042e17e30b9c2ed7b195bef7df77a1327823f989

9030557 Test gArgs erroring on unknown args (Andrew Chow) 4f8704d Give an error and exit if there are unknown parameters (Andrew Chow) 174f7c8 Use a struct for arguments and nested map for categories (Andrew Chow) Pull request description: Following bitcoin#13190, gArgs is aware of all of the command line arguments. This PR has gArgs check whether the arguments provided are actually valid arguments. When an unknown argument is encountered, an error is printed to stderr and the program exist. Since gArgs is used for everything that has command line arguments, `bitcoind`, `bitcoin-cli`, `bitcoin-qt`, `bitcoin-tx`, and `bench_bitcoin` are all effected by this change and all now have the same argument checking behavior. Closes bitcoin#1044 Tree-SHA512: 388201319a7d6493204bb5433da47e8e6c8266882e809f6df45f86d925f1f320f2fd13edb3e57ffc6a37415dfdfc689f83929452bca224229783accb367032e7

This commit contains 2 refactors. 1. mark "const" on ArgsManager::GetHelpMessage and IsArgKnown. 2. remove unused "error" argument from ArgsManager::IsArgKnown. Firstly, I mark "const" on where it is possible to. It is mentioned before (e.g. bitcoin#13190 (review)). And about 2nd change, ArgsManager::IsArgKnown was added at commit #4f8704d which was merged at PR bitcoin#13112. But from its beggining, "error" argument never be used. I think it should be refactored.

This commit contains 2 refactors. 1. mark "const" on ArgsManager::GetHelpMessage and IsArgKnown. 2. remove unused "error" argument from ArgsManager::IsArgKnown. Firstly, I mark "const" on where it is possible to. It is mentioned before (e.g. bitcoin/bitcoin#13190 (review)). And about 2nd change, ArgsManager::IsArgKnown was added at commit #4f8704d which was merged at PR #13112. But from its beggining, "error" argument never be used. I think it should be refactored.

This commit contains 2 refactors. 1. mark "const" on ArgsManager::GetHelpMessage and IsArgKnown. 2. remove unused "error" argument from ArgsManager::IsArgKnown. Firstly, I mark "const" on where it is possible to. It is mentioned before (e.g. bitcoin#13190 (review)). And about 2nd change, ArgsManager::IsArgKnown was added at commit #4f8704d which was merged at PR bitcoin#13112. But from its beggining, "error" argument never be used. I think it should be refactored.

4be3b76 refactor: Cleanup walletinitinterface.h (Hennadii Stepanov) Pull request description: Forward declarations of `CScheduler` and `CRPCTable` classes are no longer needed after ea961c3 (#14437) commit. Including `<string>` is no longer needed after 4d4185a (#13190) commit. ACKs for top commit: theStack: ACK 4be3b76 promag: ACK 4be3b76. kristapsk: ACK 4be3b76 (tested that it builds) Tree-SHA512: 5ed72e3deda3d7c7fb698a1a11db76199727e6c570dfc78422690dbda9a92af32e1913920062dd3c9f618095e7498c219ff9c145a4c151486865ebeaa20a1d3c

Summary: This commit contains 2 refactors. 1. mark "const" on ArgsManager::GetHelpMessage and IsArgKnown. 2. remove unused "error" argument from ArgsManager::IsArgKnown. Firstly, I mark "const" on where it is possible to. It is mentioned before (e.g. bitcoin/bitcoin#13190 (review)). And about 2nd change, ArgsManager::IsArgKnown was added at commit #4f8704d which was merged at PR #13112. But from its beggining, "error" argument never be used. I think it should be refactored. This is a backport of Core PR13733 Test Plan: make check Reviewers: #bitcoin_abc, jasonbcox Reviewed By: #bitcoin_abc, jasonbcox Differential Revision: https://reviews.bitcoinabc.org/D4168

9030557 Test gArgs erroring on unknown args (Andrew Chow) 4f8704d Give an error and exit if there are unknown parameters (Andrew Chow) 174f7c8 Use a struct for arguments and nested map for categories (Andrew Chow) Pull request description: Following bitcoin#13190, gArgs is aware of all of the command line arguments. This PR has gArgs check whether the arguments provided are actually valid arguments. When an unknown argument is encountered, an error is printed to stderr and the program exist. Since gArgs is used for everything that has command line arguments, `bitcoind`, `bitcoin-cli`, `bitcoin-qt`, `bitcoin-tx`, and `bench_bitcoin` are all effected by this change and all now have the same argument checking behavior. Closes dashpay#1044 Tree-SHA512: 388201319a7d6493204bb5433da47e8e6c8266882e809f6df45f86d925f1f320f2fd13edb3e57ffc6a37415dfdfc689f83929452bca224229783accb367032e7

achow101 mentioned this pull request May 8, 2018

Throw an error for unknown args #13112

Merged

maflcko reviewed May 8, 2018

View reviewed changes

achow101 force-pushed the gargs-help branch from 3333c7e to 3ea4d90 Compare May 8, 2018 19:48

achow101 force-pushed the gargs-help branch from 3ea4d90 to dadad0d Compare May 8, 2018 22:26

meshcollider added the Refactoring label May 9, 2018

Make gArgs aware of the arguments

4d4185a

gArgs knows what the available arguments are and their help. Getting the help message is moved to gArgs and HelpMessage() is removed

achow101 force-pushed the gargs-help branch from dadad0d to 4d4185a Compare May 9, 2018 16:21

sipa reviewed May 9, 2018

View reviewed changes

maflcko merged commit 4d4185a into bitcoin:master May 9, 2018

Bushstar mentioned this pull request May 10, 2018

commits from bitcoin/master FeatherCoin/Feathercoin#328

Merged

ken2812221 mentioned this pull request May 20, 2018

Help message does not get translated after #13190 #13287

Closed

achow101 mentioned this pull request May 21, 2018

[Qt] Re-setup args after translator setup to translate help text #13289

Closed

ryanofsky mentioned this pull request Sep 5, 2018

Multiprocess bitcoin #10102

Draft

HashUnlimited mentioned this pull request Sep 10, 2018

Throw an error for unknown args chaincoin/chaincoin#376

Merged

HashUnlimited mentioned this pull request Sep 12, 2018

tiny refactor for ArgsManager chaincoin/chaincoin#563

Merged

hebasto mentioned this pull request Sep 8, 2019

refactor: wallet: Cleanup walletinitinterface.h #16830

Merged

UdjinM6 added a commit to UdjinM6/dash that referenced this pull request Mar 19, 2021

backport bitcoin#13190: Have gArgs handle printing help

6bcc1bf

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Have gArgs handle printing help #13190

Have gArgs handle printing help #13190

achow101 commented May 8, 2018

jnewbery commented May 8, 2018

maflcko commented May 8, 2018

maflcko left a comment

maflcko May 8, 2018 •

edited

achow101 May 8, 2018

achow101 commented May 8, 2018

maflcko commented May 8, 2018

promag commented May 8, 2018 •

edited

achow101 commented May 8, 2018

sipa commented May 9, 2018

meshcollider commented May 9, 2018

maflcko commented May 9, 2018

achow101 commented May 9, 2018

sipa left a comment •

edited

sipa May 9, 2018

sipa May 9, 2018

laanwj commented May 9, 2018

maflcko commented May 9, 2018

Have gArgs handle printing help #13190

Have gArgs handle printing help #13190

Conversation

achow101 commented May 8, 2018

jnewbery commented May 8, 2018

maflcko commented May 8, 2018

maflcko left a comment

Choose a reason for hiding this comment

maflcko May 8, 2018 • edited

Choose a reason for hiding this comment

achow101 May 8, 2018

Choose a reason for hiding this comment

achow101 commented May 8, 2018

maflcko commented May 8, 2018

promag commented May 8, 2018 • edited

achow101 commented May 8, 2018

sipa commented May 9, 2018

meshcollider commented May 9, 2018

maflcko commented May 9, 2018

achow101 commented May 9, 2018

sipa left a comment • edited

Choose a reason for hiding this comment

sipa May 9, 2018

Choose a reason for hiding this comment

sipa May 9, 2018

Choose a reason for hiding this comment

laanwj commented May 9, 2018

maflcko commented May 9, 2018

maflcko May 8, 2018 •

edited

promag commented May 8, 2018 •

edited

sipa left a comment •

edited