diff options
| -rw-r--r-- | src/test/settings_tests.cpp | 13 | ||||
| -rw-r--r-- | src/util/settings.cpp | 16 |
2 files changed, 22 insertions, 7 deletions
diff --git a/src/test/settings_tests.cpp b/src/test/settings_tests.cpp index b0ee76ea6b..7e30fbcf68 100644 --- a/src/test/settings_tests.cpp +++ b/src/test/settings_tests.cpp @@ -45,6 +45,19 @@ BOOST_AUTO_TEST_CASE(Simple) CheckValues(settings2, R"("val2")", R"(["val2","val3"])"); } +// Confirm that a high priority setting overrides a lower priority setting even +// if the high priority setting is null. This behavior is useful for a high +// priority setting source to be able to effectively reset any setting back to +// its default value. +BOOST_AUTO_TEST_CASE(NullOverride) +{ + util::Settings settings; + settings.command_line_options["name"].push_back("value"); + BOOST_CHECK_EQUAL(R"("value")", GetSetting(settings, "section", "name", false, false).write().c_str()); + settings.forced_settings["name"] = {}; + BOOST_CHECK_EQUAL(R"(null)", GetSetting(settings, "section", "name", false, false).write().c_str()); +} + // Test different ways settings can be merged, and verify results. This test can // be used to confirm that updates to settings code don't change behavior // unintentionally. diff --git a/src/util/settings.cpp b/src/util/settings.cpp index d20f509e24..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 @@ -79,6 +80,8 @@ 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) { @@ -88,13 +91,12 @@ SettingsValue GetSetting(const Settings& settings, // 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; @@ -106,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 @@ -125,7 +127,7 @@ std::vector<SettingsValue> GetSettingsList(const Settings& settings, // 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()); @@ -136,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(); |