Skip to content

Leave the config alone when only its generated comments differ - #184

Merged
ben-dev-au merged 4 commits into
mainfrom
fix/config-comment-churn
Oct 6, 2026
Merged

ben-dev-au merged 4 commits into
mainfrom
fix/config-comment-churn

Conversation

@ben-dev-au

Copy link
Copy Markdown
Owner

Problem

  • Every CLI start runs ensure_current, which compared its render of the config against the file byte for byte.
  • The file carries generated help comments, and recent PRs reworded several of them.
  • So two builds on different commits (main and a worktree, say) each saw the other's wording as stale and rewrote the file on every switch.
  • Each rewrite printed fnd: config updated, adopt the canonical layout and left another timestamped backup, though no setting had changed.

Fix

  • ensure_current returns early when the file and the render parse to the same TOML values.
  • Real changes still rewrite: a migration, a missing config_version, or a value in a non-canonical form such as an absolute home path.
  • Trade-off: after an upgrade the help comments keep their old wording until the next Settings save regenerates them.

Tests

  • New: reworded generated comments alone leave the file untouched with no backup (fails without the fix).
  • New: a home path written absolute is still rewritten to ~.
  • make batch-close: ruff, pyright, 4911 passed / 3 skipped, all tmux harness scenarios pass.

The startup migration compared its render against the file byte for byte,
so two builds that word a settings hint differently rewrote each other's
config on every switch, each time printing "config updated, adopt the
canonical layout" and leaving another backup. It now rewrites only when the
parsed values differ.
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 38d1c269-12be-4abb-ba23-e1ac58582987
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Comment @coderabbitai help to get the list of available commands.

The test reached its stale-map state through `Screen._refresh_layout(scroll=True)`,
which takes Textual's visible-only fast path only while no widget has a layout
pending. On a slow runner one did, the screen reflowed in full, and the test's
own precondition failed on Windows. It now calls `Compositor.reflow_visible`,
the fast path itself. With a pending layout injected, the old setup fails with
the CI message and this one passes; it still catches a single derived offset.
@ben-dev-au
ben-dev-au merged commit fc4707a into main Oct 6, 2026
22 checks passed
@ben-dev-au
ben-dev-au deleted the fix/config-comment-churn branch October 6, 2026 11:53
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.

1 participant