From dd0a47860cf51774bcd20788766d9f55900a4636 Mon Sep 17 00:00:00 2001 From: Andrew Kitchen Date: Mon, 15 Jul 2024 11:44:34 -0700 Subject: [PATCH] NSUserDefault keys cannot use dot namespacing if you need KVO The NSNotification-based observation is far too chatty, which can add complexity and impact performance/power consumption. We will probably want to migrate the other keys away from dot notation in the future since it now looks like technical debt. The Svelte visualizer will also need to migrate away from NSNotification-based NSUserDefaults updates. --- keycastr/KCDefaultVisualizer.m | 2 +- keycastr/KCEventTransformer.m | 35 ++++++++++++------- keycastr/KCUserDefaultsMigration.h | 6 ++-- keycastr/KCUserDefaultsMigration.m | 2 ++ .../KCKeystrokeConversionTests.m | 14 ++++---- keycastr/SvelteVisualizer.m | 7 ++-- 6 files changed, 41 insertions(+), 25 deletions(-) diff --git a/keycastr/KCDefaultVisualizer.m b/keycastr/KCDefaultVisualizer.m index cfd3dff..32614cc 100644 --- a/keycastr/KCDefaultVisualizer.m +++ b/keycastr/KCDefaultVisualizer.m @@ -192,7 +192,7 @@ static const CGFloat kKCDefaultBezelPadding = 10.0; @"default.textColor": [NSKeyedArchiver archivedDataWithRootObject:[NSColor colorWithCalibratedWhite:1 alpha:1] requiringSecureCoding:NO error:NULL], - @"default.displayModifiedCharacters": @NO, + @"default_displayModifiedCharacters": @NO, }; } diff --git a/keycastr/KCEventTransformer.m b/keycastr/KCEventTransformer.m index 9e90a4f..6b7de26 100644 --- a/keycastr/KCEventTransformer.m +++ b/keycastr/KCEventTransformer.m @@ -35,12 +35,12 @@ #import "KCMouseEvent.h" @interface KCEventTransformer () -@property (nonatomic, readonly) struct __TISInputSource *keyboardLayout; +@property (nonatomic, readonly) TISInputSourceRef keyboardLayout; @property (nonatomic, assign) BOOL displayModifiedCharacters; @end @implementation KCEventTransformer { - TISInputSourceRef _keyboardLayout; + NSUserDefaults *_userDefaults; const UCKeyboardLayout *_uchrData; } @@ -81,28 +81,35 @@ static NSString* kLeftTabString = @"\xe2\x87\xa4"; - (instancetype)initWithKeyboardLayout:(TISInputSourceRef)keyboardLayout userDefaults:(NSUserDefaults *)userDefaults { if (self = [super init]) { + CFRetain(keyboardLayout); _keyboardLayout = keyboardLayout; - CFRetain(_keyboardLayout); + _userDefaults = userDefaults; CFDataRef uchr = TISGetInputSourceProperty(_keyboardLayout , kTISPropertyUnicodeKeyLayoutData); _uchrData = ( const UCKeyboardLayout* )CFDataGetBytePtr(uchr); - _displayModifiedCharacters = [userDefaults boolForKey:@"default.displayModifiedCharacters"]; - - __weak typeof(self) weakSelf = self; - [[NSNotificationCenter defaultCenter] addObserverForName:NSUserDefaultsDidChangeNotification - object:nil - queue:nil - usingBlock:^(NSNotification * _Nonnull notification) { - weakSelf.displayModifiedCharacters = [notification.object boolForKey:@"default.displayModifiedCharacters"]; - }]; + _displayModifiedCharacters = [_userDefaults boolForKey:@"default_displayModifiedCharacters"]; + [_userDefaults addObserver:self + forKeyPath:@"default_displayModifiedCharacters" + options:NSKeyValueObservingOptionNew + context:NULL]; } return self; } +- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context { + if ([keyPath isEqualToString:@"default_displayModifiedCharacters"]) { + id newValue = change[NSKeyValueChangeNewKey]; + if ([newValue respondsToSelector:@selector(boolValue)]) { + self.displayModifiedCharacters = [newValue boolValue]; + } + } +} + - (void)dealloc { + [_userDefaults removeObserver:self forKeyPath:@"default_displayModifiedCharacters"]; CFRelease(_keyboardLayout); } @@ -232,6 +239,10 @@ static NSString* kLeftTabString = @"\xe2\x87\xa4"; NSString *specialKeyString = [[self _specialKeys] objectForKey:@(_keyCode)]; if (specialKeyString) { +// if (hasOptionModifier && !keystroke.isCommand) { +// [mutableResponse appendString:kOptionKeyString]; +// } + [mutableResponse appendString:specialKeyString]; return mutableResponse; } diff --git a/keycastr/KCUserDefaultsMigration.h b/keycastr/KCUserDefaultsMigration.h index 60371b8..4f55767 100644 --- a/keycastr/KCUserDefaultsMigration.h +++ b/keycastr/KCUserDefaultsMigration.h @@ -33,9 +33,11 @@ NS_ASSUME_NONNULL_BEGIN @interface KCUserDefaultsMigration : NSObject /// -/// Migrates the default visualizer's NSColor data stored in NSUserDefaults from the deprecated NSArchiver/NSUnarchiver format to the format used by the NSKeyedArchiver/NSKeyedUnarchiver. Also removes a default which was written to but otherwise unused by the svelte visualizer. +/// Migrates the default visualizer's NSColor data stored in NSUserDefaults from the deprecated NSArchiver/NSUnarchiver format to the format used by the NSKeyedArchiver/NSKeyedUnarchiver. Also removes a default which was written to but otherwise unused by the Svelte visualizer. /// -/// This method should be the only place in the codebase that references the deprecated classes. It can be removed in any release after the next release after a reasonable period of time. +/// This method should be the only place in the codebase that references the deprecated NSArchiver/NSUnarchiver classes. It can be removed in any release after the next release after a reasonable period of time. +/// +/// TODO: A new migration may be needed for keys with namespacing, e.g. default.bezelColor. This namespacing breaks KVO, although it seems that KVC works OK /// + (void)performMigration:(NSUserDefaults *)userDefaults; + (NSArray *)colorKeyNames; diff --git a/keycastr/KCUserDefaultsMigration.m b/keycastr/KCUserDefaultsMigration.m index 130cafa..e507ae3 100644 --- a/keycastr/KCUserDefaultsMigration.m +++ b/keycastr/KCUserDefaultsMigration.m @@ -29,6 +29,8 @@ @implementation KCUserDefaultsMigration +// TODO: migrate legacy keys from dot/namespaces to underscores, and audit for observation (KVC?) + + (void)performMigration:(NSUserDefaults *)userDefaults { NSArray *colorKeyNames = [self colorKeyNames]; for (NSString *colorKey in colorKeyNames) { diff --git a/keycastr/KCVisualizerTests/KCKeystrokeConversionTests.m b/keycastr/KCVisualizerTests/KCKeystrokeConversionTests.m index 0c9aa89..b449cde 100644 --- a/keycastr/KCVisualizerTests/KCKeystrokeConversionTests.m +++ b/keycastr/KCVisualizerTests/KCKeystrokeConversionTests.m @@ -191,15 +191,15 @@ // shift-opt-7 KCKeystroke *keystroke = [self keystrokeWithKeyCode:26 modifiers:655650 characters:@"»" charactersIgnoringModifiers:@"7"]; - // shift-opt-7 transforms to "»" when default.displayModifiedCharacters is set to YES - [userDefaults setBool:YES forKey:@"default.displayModifiedCharacters"]; + // shift-opt-7 transforms to "»" when default_displayModifiedCharacters is set to YES + [userDefaults setBool:YES forKey:@"default_displayModifiedCharacters"]; XCTAssertEqualObjects([eventTransformer transformedValue:keystroke], @"»"); - // shift-opt-7 transforms to "⌥⇧7" when default.displayModifiedCharacters is set to NO - [userDefaults setBool:NO forKey:@"default.displayModifiedCharacters"]; + // shift-opt-7 transforms to "⌥⇧7" when default_displayModifiedCharacters is set to NO + [userDefaults setBool:NO forKey:@"default_displayModifiedCharacters"]; XCTAssertEqualObjects([eventTransformer transformedValue:keystroke], @"⌥⇧7"); - [userDefaults removeObjectForKey:@"default.displayModifiedCharacters"]; + [userDefaults removeObjectForKey:@"default_displayModifiedCharacters"]; } - (void)test_displayingCorrectlyWhenInputSourceKeyboardLayoutChanges { @@ -211,7 +211,7 @@ - (void)test_tabKey { // tab characters and charactersIgnoringModifiers fields are UTF8 "\t" KCKeystroke *keystroke = [self keystrokeWithKeyCode:48 modifiers:256 characters:@"\t" charactersIgnoringModifiers:@"\t"]; - XCTAssertTrue([[keystroke convertToString] isEqualToString:@"\xe2\x87\xa5"]); + XCTAssertEqualObjects([keystroke convertToString], @"⇥"); } - (void)test_shiftTab { @@ -219,7 +219,7 @@ // it is not possble use Objective-C @ literals with \U000000xx syntax for many 2 byte ASCII characters // https://stackoverflow.com/a/27697100 KCKeystroke *keystroke = [self keystrokeWithKeyCode:48 modifiers:131330 characters:[NSString stringWithFormat:@"%C", 0x00000019] charactersIgnoringModifiers:[NSString stringWithFormat:@"%C", 0x00000019]]; - XCTAssertTrue([[keystroke convertToString] isEqualToString:@"\xe2\x87\xa4"]); + XCTAssertEqualObjects([keystroke convertToString], @"⇤"); } @end diff --git a/keycastr/SvelteVisualizer.m b/keycastr/SvelteVisualizer.m index 71973ea..e0ba1a6 100644 --- a/keycastr/SvelteVisualizer.m +++ b/keycastr/SvelteVisualizer.m @@ -1,5 +1,5 @@ // Copyright (c) 2009 Stephen Deken -// Copyright (c) 2014-2023 Andrew Kitchen +// Copyright (c) 2014-2024 Andrew Kitchen // // All rights reserved. // @@ -204,11 +204,12 @@ _displayAll = [[[NSUserDefaults standardUserDefaults] valueForKey:@"svelte.displayAll"] boolValue]; + // TODO: migrate away from using NSNotificationCenter for this, as it is far too chatty [[NSNotificationCenter defaultCenter] addObserverForName:NSUserDefaultsDidChangeNotification object:nil queue:nil - usingBlock:^(NSNotification * _Nonnull note) { - _displayAll = [[[NSUserDefaults standardUserDefaults] valueForKey:@"svelte.displayAll"] boolValue]; + usingBlock:^(NSNotification * _Nonnull notification) { + _displayAll = [notification.object boolForKey:@"svelte.displayAll"]; }]; return self;