fix(weather-editor): no-override weather file deletion (#1777)

[SkrubbySkrubInAShrub]

Fixed a bug where the saving system would delete the whole weather file if there were no more feature overrides left, even if there are other weather settings. Now checks if there's any data left at all and only then deletes the file. I accidentally introduced this bug myself.

Summary by CodeRabbit

• Bug Fixes
  • Prevented unintended deletion of weather files that only contained configuration settings.
  • Improved handling and logging when removing or updating weather data so files are preserved or safely rewritten if deletion fails.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR narrows WeatherManager's save-to-disk deletion logic: the weather file is deleted only when featureSettings is empty AND the JSON contains exclusively that object; on successful removal it logs "no data remain" and returns early to avoid further writes.

Changes

Cohort / File(s)	Summary
WeatherManager save-to-disk deletion refinement src/WeatherManager.cpp	Delete weather file only when featureSettings is empty AND weatherData.size() == 1 and weatherData.contains("featureSettings"); log "no data remain" on successful removal and return early; on removal failure, return if file missing, else warn and fall back to writing updated JSON.

Sequence Diagram(s)

(Skipped — change is a localized deletion-condition logic tweak and does not introduce a multi-component sequential flow.)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• fix(weather overrides): ensure json format for features #1748: Introduced the nested featureSettings structure and earlier deletion logic that this change tightens.

Suggested reviewers

• davo0411
• doodlum

Poem

🐰
I hopped through code with gentle care,
Found a file that vanished in thin air.
Now I check twice before I say goodbye,
Only when lone will the old file fly.
A cautious carrot wink and a soft sigh.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(weather-editor): no-override weather file deletion' clearly identifies the main change—preventing unintended weather file deletion when feature settings are removed.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

🧪 Unit Test Generation v2 is now available!

We have significantly improved our unit test generation capabilities.

To enable: Add this to your .coderabbit.yaml configuration:

reviews:
  finishing_touches:
    unit_tests:
      enabled: true

Try it out by using the @coderabbitai generate unit tests command on your code files or under ✨ Finishing Touches on the walkthrough!

Have feedback? Share your thoughts on our Discord thread!

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Using provided base ref: 423541f
Using base ref: 423541f
Base commit date: 2026-01-28T16:52:52Z (Wednesday, January 28, 2026 04:52 PM)
No actionable suggestions for changed features.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/WeatherManager.cpp (1)

178-187: Avoid losing updates when delete fails.
If std::filesystem::remove fails (e.g., permissions/lock), the early return skips writing the updated JSON, leaving stale settings on disk that can reappear on next load. Only return on successful delete (or ENOENT), and fall through to write the updated JSON on other errors.

🔧 Proposed fix

if (featureSettings.empty() && weatherData.size() == 1 && weatherData.contains("featureSettings")) {
  std::error_code ec;
- if (std::filesystem::remove(filePath, ec)) {
-   logger::info("Removed weather settings file (no data remain): {}", filePath);
- } else if (ec) {
-   logger::warn("Failed to remove empty weather settings file ({}): {}", filePath, ec.message());
- }
- return;
+ if (std::filesystem::remove(filePath, ec)) {
+   logger::info("Removed weather settings file (no data remain): {}", filePath);
+   return;
+ }
+ if (ec == std::errc::no_such_file_or_directory) {
+   return;
+ }
+ logger::warn("Failed to remove empty weather settings file ({}): {}", filePath, ec.message());
+ // fall through to write updated JSON as a best-effort fallback
}

[github-actions]

✅ A pre-release build is available for this PR:
Download