Skip to content

No-op: UpdateUserConfig already handles SaveIni internally#523

Closed
Copilot wants to merge 1 commit intoupdate_config_logfrom
copilot/sub-pr-522
Closed

No-op: UpdateUserConfig already handles SaveIni internally#523
Copilot wants to merge 1 commit intoupdate_config_logfrom
copilot/sub-pr-522

Conversation

Copy link
Copy Markdown

Copilot AI commented Feb 26, 2026

A bot-generated review suggestion on #522 proposed adding an explicit SaveIni() call after UpdateUserConfig() in EnsureLoaded, and removing the outer ini.LoadFile(path). This was incorrect.

UpdateUserConfig already calls SaveIni() internally when the config is modified:

if (modified)
{
    SaveIni();  // already persists merged config to disk
}

The existing flow in EnsureLoaded is correct as-is:

  • File missing: writes embedded default to disk → outer ini.LoadFile loads it
  • File exists: ini.LoadFile loads user config → UpdateUserConfig merges new keys and saves if modified → outer ini.LoadFile reloads from disk (already up to date)

No code changes are required. This PR documents the analysis and closes out the review thread.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI assigned Copilot and x87 Feb 26, 2026
Copilot AI changed the title [WIP] Update CLEO config loading to handle SaveIni calls No-op: UpdateUserConfig already handles SaveIni internally Feb 26, 2026
@x87
Copy link
Copy Markdown

x87 commented Feb 26, 2026

🤦‍♂️

@x87 x87 closed this Feb 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants