Diffusion Bitcoin ABC 619943a5c9c6

refactor: Settings code cleanups
619943a5c9c6
Actions

Description

refactor: Settings code cleanups

Summary:

refactor: Clean up includeconf comments

Suggested by Antoine Riard <ariard@student.42.fr>
https://github.com/bitcoin/bitcoin/pull/15934#discussion_r344291875

and John Newbery <john@johnnewbery.com>
https://github.com/bitcoin/bitcoin/pull/15934#discussion_r344271224

This commit does not change behavior.

refactor: Replace FlagsOfKnownArg with GetArgFlags

Rename suggested by João Barbosa <joao.paulo.barbosa@gmail.com>
https://github.com/bitcoin/bitcoin/pull/16545#issuecomment-519048000

This also gets rid of ArgsManager::NONE constant, which was an implementation
detail not meant to be used by ArgsManager callers.

Finally this reverts a change from 7f40528cd50fc43ac0bd3e785de24d661adddb7a
https://github.com/bitcoin/bitcoin/pull/15934 adding "-" characters to argument
names. Better for GetArgFlags to require "-" prefixes for consistency with
other ArgsManager methods, and to be more efficient later when GetArg functions
need to call GetArgFlags (https://github.com/bitcoin/bitcoin/pull/16545)

This commit does not change behavior.

refactor: Get rid of ArgsManagerHelper class

Suggested by John Newbery <john@johnnewbery.com>
https://github.com/bitcoin/bitcoin/pull/15934#issuecomment-551969778

This commit does not change behavior.

refactor: Add ArgsManager::GetSettingsList method

Add for consistency with ArgsManager::GetSetting method and to make setting
types accessible to ArgsManager callers and tests (test added next commit).

This commit does not change behavior.

refactor: Add util_CheckValue test

Test GetSetting and GetArg type coercion, negation, and default value handling.
Test is expanded later to cover other flags besides ALLOW_ANY when they are
implemented in https://github.com/bitcoin/bitcoin/pull/16545

This commit does not change behavior.

scripted-diff: Remove unused ArgsManager type flags in tests

The bool/int/string flags were added speculatively in #16097 and trigger errors
when type checking is actually implemented in
https://github.com/bitcoin/bitcoin/pull/16545

-BEGIN VERIFY SCRIPT-
sed -i 's/ALLOW_\(BOOL\|INT\|STRING\)/ALLOW_ANY/g' src/test/util_tests.cpp src/test/getarg_tests.cpp
-END VERIFY SCRIPT-

This commit does not change behavior.

refactor: Remove null setting check in GetSetting()

Also rename the "result_complete" variable in GetSettingsList() to "done" to be
more consistent with GetSetting().

This change doesn't affect current behavior but could be useful in the future
to support dynamically changing settings at runtime and adding new settings
sources, because it lets high priority sources reset settings back to default
(see test).

By removing a special case for null, this change also helps merge code treat
settings values more like black boxes, and interfere less with settings parsing
and retrieval.

This is a backport of Core PR17473

Depends on D5904

Test Plan: