Settings: host in main window, sync changes live across windows - #59
Conversation
… live across windows TermFlow supports multiple windows, each with its own Redux store. Settings used to open (and only ever apply edits) in whichever window triggered it. Now: - Settings always opens/activates in the current main window (the boot window, or whichever window is promoted after it closes), regardless of which window "Open Settings" was triggered from, and that window is focused/restored. - A settings change (font, color schema, ...) broadcasts to every open window via a new config:changed event, so all windows' terminals apply it live instead of only the window Settings happened to be open in.
agy review (PR #59) found a genuine infinite broadcast/persist loop: every settings reducer persists on every dispatch, so a window that applied a config:changed broadcast would re-persist the same value, which re-broadcast to every window again, forever — including from the very first boot, before any user touches Settings. merge_many_locked now reports whether a merge actually changed anything (and skips the write entirely when it didn't); merge_config only broadcasts when it did. An echoed write compares equal to what's already stored and is a true no-op, breaking the cycle after one extra round-trip per window.
… promotion agy review (PR #59) round 1 follow-up: - F-03: shellProfiles/defaultProfile now sync live to other windows too, via the config:changed listener directly (NOT folded into applyConfigSettings, which also runs at boot alongside initializeShellProfiles — that function re-queries live system profiles and merges saved cwd overrides, so applying the raw saved snapshot there too would race it and could overwrite the correctly-merged list with a stale one). - F-04: extracted the active_window/main_window promotion algorithm out of resolve_window_label_excluding into a pure pick_window_label(chosen, exclude, live_labels) function, decoupled from AppHandle, so the actual selection logic is unit-testable without Tauri's test feature (which crashes the Windows test binary). Added 6 tests covering promotion, boot-window fallback, drag-preview exclusion, and the destroy-handler race (exclude beats map membership).
External review (agy) — 2 rounds, converged, merge-readyRound 1 found one genuine blocker: Fixed in Round 2 (confirming re-review, independently verifying each fix against the current code rather than the commit messages) found all 4 resolved and zero new issues. Full writeup: Still outstanding: the two manual test items in the PR description (multi-window promotion, live settings sync) — these need a real multi-window run, which hasn't happened yet. |
2 participants
Summary
config:changedevent, so all windows' terminals apply it live instead of only the window Settings happened to be open in.Implementation
state.rs: newAppState.main_window(mirrorsactive_window's shape but kept separate —active_windowis a user-toggleable API/MCP routing target with its own titlebar control; coupling Settings' location to that toggle would relocate Settings as a side effect of an unrelated setting). Refactored the shared "current choice → boot window → first live window" fallback intoresolve_window_label_excluding, reused by both.lib.rs: the existing window-Destroyedhandler (already promotesactive_windowon close) now does the same formain_window.commands.rs: newopen_settings_in_main_windowcommand broadcastssettings:opennaming the resolved main window as target, and calls the existingwebview_power::restore_and_focushelper to bring it forward.merge_confignow also emitsconfig:changedwith the merged keys to every window.openSettings.ts/App.tsx:openSettingsTab()routes through the new command instead of dispatching locally; every window listens viainstallSettingsRouting()and only the targeted window acts. Boot-time config-applying logic (loadConfigSettings) was split so the reusable per-key part (applyConfigSettings) excludes the EULA-hydration line, which must stay boot-only/unconditional — reusing it as-is for a partial broadcast would reset every other window's EULA acceptance on any unrelated settings change.Test plan
tsc --noEmitcargo check(default,--features integration-tests,--tests)cargo testforstate::andwebview_power::(incl. the guard test that no other file calls.unminimize()directly)