Merge #17473: refactor: Settings code cleanups

e9fd366044e271632dc0e4f96e1c14f8e87213ae refactor: Remove null setting check in GetSetting() (Russell Yanofsky)
cba2710220d76bbe790b04088839cbbd410436de scripted-diff: Remove unused ArgsManager type flags in tests (Russell Yanofsky)
425bb307252cf4dec9b3ef6426e6548b2be7a303 refactor: Add util_CheckValue test (Russell Yanofsky)
0fa54358b06b58f4d17073bcc8a959eb9498aadc refactor: Add ArgsManager::GetSettingsList method (Russell Yanofsky)
3e185522ace1678e0a25b9cf8a5553a4bc279bea refactor: Get rid of ArgsManagerHelper class (Russell Yanofsky)
dc0f1480746b34aa3ca2d9c0f1ec764083026b40 refactor: Replace FlagsOfKnownArg with GetArgFlags (Russell Yanofsky)
57e8b7a7273567aa4a4aee87cce18e9bff8f3196 refactor: Clean up includeconf comments (Russell Yanofsky)
3f7dc9b808316c1e5d677af8d9a99112568c8ccb refactor: Clean up long lines in settings code (Russell Yanofsky)

Pull request description:

  This PR doesn't change behavior. It just implements some suggestions from #15934 and #16545 and few other small cleanups.

ACKs for top commit:
  jnewbery:
    Code review ACK e9fd366044e271632dc0e4f96e1c14f8e87213ae
  MarcoFalke:
    ACK e9fd366044 🚟

Tree-SHA512: 6e100d92c72f72bc39567187ab97a3547b3c06e5fcf1a1b74023358b8bca552124ca6a53c0ab53179b7f1329c03d9a73faaef6d73d2cd1a2321568a0286525e2

1 files changed, 19 insertions, 10 deletions

diff --git a/src/util/settings.cpp b/src/util/settings.cpp
index af75fef310..e4979df755 100644
--- a/src/util/settings.cpp
+++ b/src/util/settings.cpp
@@ -56,6 +56,7 @@ SettingsValue GetSetting(const Settings& settings,
     bool get_chain_name)
 {
     SettingsValue result;
+    bool done = false; // Done merging any more settings sources.
     MergeSettings(settings, section, name, [&](SettingsSpan span, Source source) {
         // Weird behavior preserved for backwards compatibility: Apply negated
         // setting even if non-negated setting would be ignored. A negated
@@ -68,7 +69,9 @@ SettingsValue GetSetting(const Settings& settings,
         // precedence over early settings, but for backwards compatibility in
         // the config file the precedence is reversed for all settings except
         // chain name settings.
-        const bool reverse_precedence = (source == Source::CONFIG_FILE_NETWORK_SECTION || source == Source::CONFIG_FILE_DEFAULT_SECTION) && !get_chain_name;
+        const bool reverse_precedence =
+            (source == Source::CONFIG_FILE_NETWORK_SECTION || source == Source::CONFIG_FILE_DEFAULT_SECTION) &&
+            !get_chain_name;
 
         // Weird behavior preserved for backwards compatibility: Negated
         // -regtest and -testnet arguments which you would expect to override
@@ -77,19 +80,23 @@ SettingsValue GetSetting(const Settings& settings,
         // negated values, or at least warn they are ignored.
         const bool skip_negated_command_line = get_chain_name;
 
+        if (done) return;
+
         // Ignore settings in default config section if requested.
-        if (ignore_default_section_config && source == Source::CONFIG_FILE_DEFAULT_SECTION && !never_ignore_negated_setting) return;
+        if (ignore_default_section_config && source == Source::CONFIG_FILE_DEFAULT_SECTION &&
+            !never_ignore_negated_setting) {
+            return;
+        }
 
         // Skip negated command line settings.
         if (skip_negated_command_line && span.last_negated()) return;
 
-        // Stick with highest priority value, keeping result if already set.
-        if (!result.isNull()) return;
-
         if (!span.empty()) {
             result = reverse_precedence ? span.begin()[0] : span.end()[-1];
+            done = true;
         } else if (span.last_negated()) {
             result = false;
+            done = true;
         }
     });
     return result;
@@ -101,7 +108,7 @@ std::vector<SettingsValue> GetSettingsList(const Settings& settings,
     bool ignore_default_section_config)
 {
     std::vector<SettingsValue> result;
-    bool result_complete = false;
+    bool done = false; // Done merging any more settings sources.
     bool prev_negated_empty = false;
     MergeSettings(settings, section, name, [&](SettingsSpan span, Source source) {
         // Weird behavior preserved for backwards compatibility: Apply config
@@ -111,14 +118,16 @@ std::vector<SettingsValue> GetSettingsList(const Settings& settings,
         // value is followed by non-negated value, in which case config file
         // settings will be brought back from the dead (but earlier command
         // line settings will still be ignored).
-        const bool add_zombie_config_values = (source == Source::CONFIG_FILE_NETWORK_SECTION || source == Source::CONFIG_FILE_DEFAULT_SECTION) && !prev_negated_empty;
+        const bool add_zombie_config_values =
+            (source == Source::CONFIG_FILE_NETWORK_SECTION || source == Source::CONFIG_FILE_DEFAULT_SECTION) &&
+            !prev_negated_empty;
 
         // Ignore settings in default config section if requested.
         if (ignore_default_section_config && source == Source::CONFIG_FILE_DEFAULT_SECTION) return;
 
         // Add new settings to the result if isn't already complete, or if the
         // values are zombies.
-        if (!result_complete || add_zombie_config_values) {
+        if (!done || add_zombie_config_values) {
             for (const auto& value : span) {
                 if (value.isArray()) {
                     result.insert(result.end(), value.getValues().begin(), value.getValues().end());
@@ -129,8 +138,8 @@ std::vector<SettingsValue> GetSettingsList(const Settings& settings,
         }
 
         // If a setting was negated, or if a setting was forced, set
-        // result_complete to true to ignore any later lower priority settings.
-        result_complete |= span.negated() > 0 || source == Source::FORCED;
+        // done to true to ignore any later lower priority settings.
+        done |= span.negated() > 0 || source == Source::FORCED;
 
         // Update the negated and empty state used for the zombie values check.
         prev_negated_empty |= span.last_negated() && result.empty();