Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 87 additions & 7 deletions Sources/KeyboardShortcuts/KeyboardShortcuts.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,31 @@ public enum KeyboardShortcuts {

private static var openMenuObserver: NSObjectProtocol?
private static var closeMenuObserver: NSObjectProtocol?
private static var userDefaultsObservers = [UserDefaultsObservation]()

/**
The `UserDefaults` instance used to store and retrieve keyboard shortcut configurations.

By default, this uses the standard `UserDefaults` instance. You can customize this to use a different `UserDefaults` instance, for example, to store shortcuts in a specific app group or to use a custom `UserDefaults` instance for testing.

```swift
// Example: Using a custom UserDefaults instance
KeyboardShortcuts.userDefaults = UserDefaults(suiteName: "com.example.suite")!

// Example: Using a custom UserDefaults instance for testing
KeyboardShortcuts.userDefaults = UserDefaults(suiteName: "test")!
```

- Important: Changing this property will not migrate existing shortcuts from the previous `UserDefaults` instance.
- Note: All keyboard shortcut configurations are stored with the prefix `KeyboardShortcuts_` to avoid conflicts with other app data.
*/
public static var userDefaults = UserDefaults.standard {
didSet {
for observer in userDefaultsObservers {
observer.update(suite: userDefaults)
}
}
}

/**
When `true`, event handlers will not be called for registered keyboard shortcuts.
Expand All @@ -59,7 +84,7 @@ public enum KeyboardShortcuts {
}

static var allNames: Set<Name> {
UserDefaults.standard.dictionaryRepresentation()
Self.userDefaults.dictionaryRepresentation()
Comment on lines -62 to +87
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self. is not needed.

.compactMap { key, _ in
guard key.hasPrefix(userDefaultsPrefix) else {
return nil
Expand Down Expand Up @@ -179,6 +204,12 @@ public enum KeyboardShortcuts {

legacyKeyDownHandlers = [:]
legacyKeyUpHandlers = [:]

// invalidate and remove all elements of userDefaultsObservers
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should comment that it's "defensive" since it's already done when calling removeAll() (deinit).

for observer in userDefaultsObservers {
observer.invalidate()
}
userDefaultsObservers.removeAll()
}

// TODO: Also add `.isEnabled(_ name: Name)`.
Expand Down Expand Up @@ -326,7 +357,7 @@ public enum KeyboardShortcuts {
*/
public static func getShortcut(for name: Name) -> Shortcut? {
guard
let data = UserDefaults.standard.string(forKey: userDefaultsKey(for: name))?.data(using: .utf8),
let data = Self.userDefaults.string(forKey: userDefaultsKey(for: name))?.data(using: .utf8),
let decoded = try? JSONDecoder().decode(Shortcut.self, from: data)
else {
return nil
Expand Down Expand Up @@ -411,6 +442,9 @@ public enum KeyboardShortcuts {
public static func onKeyDown(for name: Name, action: @escaping () -> Void) {
legacyKeyDownHandlers[name, default: []].append(action)
registerShortcutIfNeeded(for: name)

// observe changes to the UserDefaults instance for the given shortcut name
startObservingShortcutIfNeeded(for: name)
}

/**
Expand All @@ -437,11 +471,54 @@ public enum KeyboardShortcuts {
public static func onKeyUp(for name: Name, action: @escaping () -> Void) {
legacyKeyUpHandlers[name, default: []].append(action)
registerShortcutIfNeeded(for: name)

// observe changes to the UserDefaults instance for the given shortcut name
startObservingShortcutIfNeeded(for: name)
}

private static let userDefaultsPrefix = "KeyboardShortcuts_"

private static func userDefaultsKey(for shortcutName: Name) -> String { "\(userDefaultsPrefix)\(shortcutName.rawValue)"
private static func userDefaultsKey(for shortcutName: Name) -> String {
"\(userDefaultsPrefix)\(shortcutName.rawValue)"
}

/**
Start observing changes to the `UserDefaults` instance for a specific shortcut name.

This function manages the lifecycle of observations for keyboard shortcuts in the given `UserDefaults` instance (set by `userDefaults` property):
- Checks if the shortcut is already being observed
- If already observed, restarts the observation
- If not observed, creates a new observation and adds it to the observers list

The observation handles changes to the shortcut configuration in the suite:
- When the shortcut is removed (value becomes nil), unregisters the shortcut
- When the shortcut is added or modified, registers the new shortcut

- Parameter name: The name of the shortcut to observe
*/
private static func startObservingShortcutIfNeeded(for name: Name) {
let key = userDefaultsKey(for: name)

// check userDefaultsObservers to see if we are already observing this key
if let observer = userDefaultsObservers.first(where: { $0.key == key }) {
observer.start()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

return
}

let observer = UserDefaultsObservation(
suite: userDefaults,
key: key
) { value in
if value == nil {
self.unregisterShortcutIfNeeded(for: name)
} else {
self.registerShortcutIfNeeded(for: name)
}
}

observer.start()

userDefaultsObservers.append(observer)
}

static func userDefaultsDidChange(name: Name) {
Expand All @@ -459,7 +536,7 @@ public enum KeyboardShortcuts {
}

register(shortcut)
UserDefaults.standard.set(encoded, forKey: userDefaultsKey(for: name))
Self.userDefaults.set(encoded, forKey: userDefaultsKey(for: name))
userDefaultsDidChange(name: name)
}

Expand All @@ -468,7 +545,7 @@ public enum KeyboardShortcuts {
return
}

UserDefaults.standard.set(false, forKey: userDefaultsKey(for: name))
Self.userDefaults.set(false, forKey: userDefaultsKey(for: name))
unregister(shortcut)
userDefaultsDidChange(name: name)
}
Expand All @@ -478,13 +555,13 @@ public enum KeyboardShortcuts {
return
}

UserDefaults.standard.removeObject(forKey: userDefaultsKey(for: name))
Self.userDefaults.removeObject(forKey: userDefaultsKey(for: name))
unregister(shortcut)
userDefaultsDidChange(name: name)
}

static func userDefaultsContains(name: Name) -> Bool {
UserDefaults.standard.object(forKey: userDefaultsKey(for: name)) != nil
Self.userDefaults.object(forKey: userDefaultsKey(for: name)) != nil
}
}

Expand Down Expand Up @@ -537,6 +614,9 @@ extension KeyboardShortcuts {
}

registerShortcutIfNeeded(for: name)

// observe changes to the UserDefaults instance for the given shortcut name
startObservingShortcutIfNeeded(for: name)
}

continuation.onTermination = { _ in
Expand Down
102 changes: 102 additions & 0 deletions Sources/KeyboardShortcuts/Utilities.swift
Original file line number Diff line number Diff line change
Expand Up @@ -559,3 +559,105 @@ extension Character {
self = Character(content)
}
}

final class UserDefaultsObservation: NSObject {
typealias Callback = (_ newKeyValue: String?) -> Void

let key: String

static var observationContext = 0
private weak var suite: UserDefaults?
private var isObserving = false
private let callback: Callback
private var lock = NSLock()

init(
suite: UserDefaults,
key: String,
_ callback: @escaping Callback
) {
self.suite = suite
self.key = key
self.callback = callback
}

deinit {
invalidate()
}

func start() {
lock.withLock {
guard !isObserving else {
return
}

suite?.addObserver(
self,
forKeyPath: key,
options: [.new],
context: &Self.observationContext
)
isObserving = true
}
}

func invalidate() {
lock.withLock {
guard isObserving else {
return
}

suite?.removeObserver(
self,
forKeyPath: key
)
isObserving = false
suite = nil
}
}

func update(suite new: UserDefaults) {
invalidate()
suite = new
start()
}
Comment on lines +619 to +623
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
		}
	}
}


// swiftlint:disable:next block_based_kvo
override func observeValue(
forKeyPath keyPath: String?,
of object: Any?,
change: [NSKeyValueChangeKey: Any]?, // swiftlint:disable:this discouraged_optional_collection
context: UnsafeMutableRawPointer?
) {
guard
context == &Self.observationContext
else {
super.observeValue(
forKeyPath: keyPath,
of: object,
change: change,
context: context
)
return
}

guard let selfSuite = suite else {
invalidate()
return
}

guard
selfSuite == (object as? UserDefaults),
let change
else {
return
}

guard keyPath == key else {
return
}

let encodedString = change[.newKey] as? String
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not assume String. It could be a Bool too:

Self.userDefaults.set(false, forKey: userDefaultsKey(for: name))

callback(encodedString)
}
}