feat: let users turn off or reschedule the update check - #220
Conversation
Closes lzhgus#219 Preferences > General now has an "Automatically Check for Updates" switch and a Daily/Weekly/Monthly frequency picker, and the Updates section moved up under Startup. Sparkle already persists both values in user defaults, so there are no new AppSettings keys. Turning checks off greys out "Automatically Install Updates" (via Sparkle's own allowsAutomaticUpdates) without discarding the stored preference, and the manual check keeps working. UpdateManager now talks to a small SoftwareUpdating protocol so it can be unit tested with a stand-in instead of a live Sparkle updater.
Sparkle gates both accessors for the auto-install preference on allowsAutomaticUpdates, which follows automaticallyChecksForUpdates: the getter reports false while background checks are off, and SPUUpdaterSettings.setAutomaticallyDownloadsUpdates: drops the write entirely. Reading the masked getter made the install switch flip itself off when checks were switched off, and the KVO handler then wrote that false back, destroying the stored preference. Capso now reads and writes SUAutomaticallyUpdate directly, so the two settings move independently and the install row stays usable with checks off. Sparkle observes that key and stays in sync. Also splits the manual-probe outcomes out of the SPUUpdaterDelegate callbacks, whose signatures need a live SPUUpdater, so the manual check flow is covered: a user-initiated check always escalates to Sparkle's interactive flow and never touches either preference.
|
@lzhgus looks like we both took #219 within a few minutes of each other. Sorry for the pile on. If you go with your version, grab this bit: Mine also adds a Daily/Weekly/Monthly picker since the issue asked for an interval too. |
Daily was the old hardcoded interval. The issue behind this work is that people don't want update notifications more often than they open the app, so weekly is the friendlier starting point. Users who want daily can still pick it. Pins the shipped Info.plist defaults with tests so they can't drift unnoticed.
This reverts commit 3554a7b. Existing installs have nothing stored for the interval, so they read straight from Info.plist and changing it there would move everyone who upgrades, not just new installs. Keeping daily means nobody's behaviour changes and weekly is still two clicks away in the new picker. Keeps the tests that pin the shipped defaults, now asserting daily, so this can't drift again unnoticed.
lzhgus
left a comment
There was a problem hiding this comment.
Thanks for the note — and no worries about the overlap. These things happen. I really appreciate you digging into the masked Sparkle auto-install accessors; that was a subtle and useful catch. Also, good call reverting the weekly Info.plist default once you noticed it would change behavior for existing installs.
I had two small product/UX questions. I do not mean these as hard blockers — I would genuinely like your take:
-
When automatic checks are off, would you be open to preserving the stored auto-install preference but disabling the toggle visually (for example, .disabled(!updateManager.automaticallyChecksForUpdates))? Since a background install cannot happen without a scheduled check, leaving the control interactive may suggest it has an immediate effect. It would also make the comment in UpdatesSettingsSection match the behavior again.
-
Would you be open to keeping the Updates section in its previous location after Sound? The issue did not require reordering General preferences, although if you had a particular UX reason for placing it directly under Startup, I would be interested to hear it.
Two very minor housekeeping notes: the PR description still has the “Screenshot coming” placeholder, and “33 new tests” looks like 37 now (31 app tests plus 6 SharedKit tests).
Thanks again for the thoughtful implementation and strong test coverage. Curious what you think.
Review feedback from @lzhgus. A background install can't happen without a scheduled check, so leaving the switch interactive suggested it had an immediate effect. It's disabled while automatic checks are off now, which the stored-preference fix already makes safe: greying it doesn't touch the value, so switching checks back on restores it. Also puts the Updates section back after Sound, where it was, since reordering General wasn't part of the issue.
|
@lzhgus - Thanks, both fair, pushed. Greyed the auto-install row out. Since Capso writes Updates section is back under Sound. I'd put it up top figuring update settings should be above the fold, but you're right it wasn't in the issue, my bad. Count is 37, not 33. Screenshot shortly. |
Closes #219
Adds a way to turn off the daily update check, plus a frequency picker for people who want the check but not every day.
What's new in Preferences > General
Greying the install row doesn't cost you the setting
Switching checks off greys out "Automatically Install Updates" but leaves the stored preference exactly where you left it, so switching checks back on restores what you had.
That needed a workaround. Sparkle gates both accessors for the install preference on
allowsAutomaticUpdates, which just followsautomaticallyChecksForUpdates:falsewhile background checks are offSPUUpdaterSettings.setAutomaticallyDownloadsUpdates:drops the write entirelySo going through
SPUUpdatermeant the install switch flipped itself off when you switched checks off, and the KVO handler wrote that maskedfalsestraight back, wiping the real preference. Capso now reads and writesSUAutomaticallyUpdatedirectly. Sparkle observes that key, so it stays in sync.A manual check still works with automatic checks off. Sparkle routes user-initiated checks through
SPUUserInitiatedUpdateDriver, which never consults the install preference, so a found update is always presented rather than installed silently.Notes
Sparkle already persists all of this in user defaults and its header says not to keep a second copy, so there are no new
AppSettingskeys. Info.plist still supplies the starting values, unchanged: checks on, install off, daily interval. I did try defaulting to weekly, but existing installs have nothing stored for the interval and read straight from the plist, so it would have moved everyone who upgrades rather than just new installs. Weekly is two clicks away for anyone who wants it.The Updates section stays where it was, after Sound.
Tests
Built test first, 37 new tests.
UpdateCheckFrequency, a new pure enum in SharedKit, maps picker choices to Sparkle intervals and snaps unknown values to the closest one (6 tests)UpdateManagertalks to a smallSoftwareUpdatingprotocol, so it runs against a stand-in instead of a live Sparkle updater (24 tests): write-through, no writes during init, the install preference surviving a checks toggle in both directions, frequency persisting while checks are off, and the manual check escalating to the interactive flow without touching either preferenceSPUUpdaterDelegatecallbacks, whose signatures need a liveSPUUpdater, which is what makes that last group testableAutomaticInstallPreferencestorage is covered directly against a scratch defaults suite, since no stand-in updater can catch Sparkle silently dropping a writeswift test --package-path Packages/SharedKit(174 tests) andxcodebuild test -scheme Capso(109 tests) both pass.New strings are localized for ja, ko, and zh-Hans.