util: fix argsman dupe key error #27236

1. 
   willcl-ark commented at 9:57 am on March 10, 2023: contributor

   fixes #22638

   Make GUI “Settings file could not be read. Do you want to reset settings to default values?” dialog actually clear all settings instead of partially keeping them when settings.json contains duplicate keys. This change has no effect on bitcoind because it treats a corrupt settings.json file as a hard error and doesn’t attempt to modify it.

   If we find a duplicate key and error, clear values before returning so that WriteSettings() will write an empty file, therefore clearing it.

   This aligns with GUI behaviour added in 1ee6d0b.

   The test added only checks that values is empty after a duplicate key is detected. This paves the way for the abort option in the GUI to properly clear settings.json, if the user selects the option, but the test does not currently check this entire mechanism (e.g. the file contents).

2. util: fix argsman dupe key error

   fixes #22638
   
   If we find a duplicate key and error, clear `values` before returning so that
   WriteSettings will write an empty file, therefore clearing it.
   
   This aligns with GUI behaviour added in 1ee6d0b.

   8fcbdadfad
3. 
   DrahtBot commented at 9:57 am on March 10, 2023: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	ryanofsky, TheCharlatan

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

4. DrahtBot added the label Utils/log/libs on Mar 10, 2023
5. fanquake requested review from ryanofsky on Mar 10, 2023
6. ryanofsky approved
7. 
   ryanofsky commented at 3:44 pm on March 10, 2023: contributor

   Code review ACK 8fcbdadfad8a9b3143527141ff37e5fe1e87f3b3. Thanks for the fix! I would maybe update the PR description or add to it to describe behavior change of this PR:

   ---

   bugfix: Fix broken GUI “Reset” button when settings.json contains duplicate keys

   Make GUI “Settings file could not be read. Do you want to reset settings to default values?” dialog actually clear all settings instead of partially keeping them when settings.json contains duplicate keys. This change has no effect on bitcoind because it treats a corrupt settings.json file as a hard error and doesn’t attempt to modify it.

8. 
   willcl-ark commented at 4:35 pm on March 10, 2023: contributor

   Thanks @ryanofsky . I updated the PR description with your recommendation. Did you think I should update the commit title to “bugfix: Fix broken GUI “Reset” button when settings.json contains duplicate keys” also? I’m happy to do that if so.
9. 
   ryanofsky commented at 4:42 pm on March 10, 2023: contributor

   Thanks @ryanofsky . I updated the PR description with your recommendation. Did you think I should update the commit title to “bugfix: Fix broken GUI “Reset” button when settings.json contains duplicate keys” also? I’m happy to do that if so.

   Either way seems fine, it’s just a choice of whether you think it’s more useful to describe the goal of the PR or the implemenation of the PR. Another alternative to changing the title could be changing the first line “fixes #22638” to “fixes #22638, broken gui settings reset button when settings.json contains duplicate keys”

10. 
    TheCharlatan commented at 8:04 pm on March 10, 2023: contributor

    Code review ACK 8fcbdadfad8a9b3143527141ff37e5fe1e87f3b3
11. fanquake merged this on Mar 11, 2023
12. fanquake closed this on Mar 11, 2023

    ---

13. sidhujag referenced this in commit 7ef87fa38c on Mar 12, 2023
14. bitcoin locked this on Mar 10, 2024

Contributors
willcl-ark DrahtBot ryanofsky TheCharlatan

Labels
Utils/log/libs