fix(settings): stop wiping user settings on data re-extraction (DasherCore v0.2.10) - #29
Merged
Merged
Conversation
…rCore v0.2.10) User report: 'the input filter doesn't remember your choice' (Android side). The bundled data dir was passed as BOTH data_dir and user_dir to the engine, and DataInstaller.deleteRecursively()s that dir on every DATA_VERSION bump - which accompanies every DasherCore pin bump. Every app update destroyed dasher_settings.xml: input filter, speed, alphabet, everything. Fix: dedicated filesDir/dasher_user user dir that re-extraction never touches, matching DasherCore's CAPI contract of separating read-only data from user-writable state. One-time migration copies dasher_settings.xml and root-level user training files out of the old shared dir. Context-free migration is unit-tested without Robolectric. Also bumps DasherCore to v0.2.10, picking up the paragraph-newline alphabet fix. Known follow-up (not fixed here): the Activity and IME service each hold an engine instance against the same settings file with whole-file immediate saves - whichever saves last wins with its possibly-stale values. Filed for a follow-up. Signed-off-by: will wade <willwade@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the Android side of the user report: "the input filter doesn't remember your choice — every time I start Dasher I have to switch to stylus mode."
Root cause
The bundled data directory was passed as both
data_diranduser_dirto the engine, andDataInstallerdeleteRecursively()s that directory on everyDATA_VERSIONbump — which accompanies every DasherCore pin bump. So every app update destroyeddasher_settings.xml: input filter, speed, alphabet, everything. This violates DasherCore's CAPI contract, which explicitly keeps the read-only data directory and the user-writable directory (settings, training deltas) distinct.Changes
DataInstaller.userDir(context)— dedicatedfilesDir/dasher_userthat re-extraction can never touch. Both engine creation sites (MainActivity,DasherImeService) now pass it as the user dir.migrateUserArtifactscopiesdasher_settings.xmland root-level usertraining_*.txtfiles out of the old shared dir before any wipe. Context-free (pure file operations) so it's unit-testable without Robolectric.DataInstallerMigrationTest: migration moves settings + user training but not bundled training-subdir files; no-op cases for missing/empty state.Verification
:app:testDebugUnitTestgreen (3 new migration tests):app:assembleDebuggreenKnown follow-up (not fixed here)
The Activity and IME service each hold an engine instance against the same settings file with whole-file immediate saves — whichever saves last wins with its possibly-stale values. A stale-instance clobber window remains when both are alive simultaneously. Needs either a
dasher_reload_settingsCAPI or single-engine routing; tracked separately.Type of change
Definition of Done