Custom UserDefaults suite #210
Conversation
|
Being able to substitute UserDefaults suites is valuable in other respects. In an app where I use the package, for unit tests and integration tests we use a custom suite for prefs, and it would be useful to be able to use the same suite with KeyboardShortcuts. |
Maschina
changed the title
….Name +++ Fix swiftlint warnings
…otentially existing observer +++ Update observer if UserDefaults being changed
|
I am just realizing that event streams also require observations. Will make another commit for that. |
…nor readability improvements
| func update(suite new: UserDefaults) { | ||
| invalidate() | ||
| suite = new | ||
| start() | ||
| } |
There was a problem hiding this comment.
Unless I'm missing something, I think it would be better to just recreate observations than this update method.
Something like:
public static var userDefaults = UserDefaults.standard {
didSet {
for observer in userDefaultsObservers {
observer.invalidate()
}
userDefaultsObservers.removeAll()
for name in allNames {
startObservingShortcutIfNeeded(for: name)
}
}
}| return | ||
| } | ||
|
|
||
| let encodedString = change[.newKey] as? String |
There was a problem hiding this comment.
This should not assume String. It could be a Bool too:
|
|
||
| // check userDefaultsObservers to see if we are already observing this key | ||
| if let observer = userDefaultsObservers.first(where: { $0.key == key }) { | ||
| observer.start() |
There was a problem hiding this comment.
I think this could use a better code comment because just looking at it, it's not immediate obvious why we may want to call start() on an already existing observation.
| @@ -179,6 +204,12 @@ public enum KeyboardShortcuts { | |||
|
|
|||
| legacyKeyDownHandlers = [:] | |||
| legacyKeyUpHandlers = [:] | |||
|
|
|||
| // invalidate and remove all elements of userDefaultsObservers | |||
There was a problem hiding this comment.
This should comment that it's "defensive" since it's already done when calling removeAll() (deinit).
| UserDefaults.standard.dictionaryRepresentation() | ||
| Self.userDefaults.dictionaryRepresentation() |
|
Make sure you manually test it in the example app and also a real app to ensure it doesn't introduce any new bugs. I'm a little bit nervous about large changes like this as we have no real tests. |
This PR adds the capability to define a custom UserDefaults to support App Groups and access group containers.
I am using KeyboardShortcuts in an environment with App Groups. The shared UserDefaults shall also include the defined shortcuts.