From 34339fbcd9854723f085bfdeab6a97d75796add3 Mon Sep 17 00:00:00 2001 From: Klaas Pieter Annema Date: Tue, 23 Nov 2010 14:18:22 +0100 Subject: [PATCH] several bindings / KVC fixes - setValue:forKeyPath will now get call setValue:forKeyPath again on the value of the first part of the key path - override valueForKeyPath: and valueForKey: in CPObjectController's CPControllerSelectionProxy to return controller markers when appropriate - override setValue:forKeyPath: and setValue:forKey: in CPObjectController's CPControllerSelectionProxy to bypass possible controller markers This commit also removes previous hacks for compound paths and adds test cases for the errors. All this is related to issue #967. --- AppKit/CPObjectController.j | 63 +++++++++++++---------- Foundation/CPKeyValueCoding.j | 25 +++------ Tests/AppKit/CPArrayControllerTest.j | 68 ++++++++++++++++++++----- Tests/Foundation/CPKeyValueCodingTest.j | 7 --- 4 files changed, 98 insertions(+), 65 deletions(-) diff --git a/AppKit/CPObjectController.j b/AppKit/CPObjectController.j index 5cc86d8b5..30a720848 100644 --- a/AppKit/CPObjectController.j +++ b/AppKit/CPObjectController.j @@ -530,50 +530,64 @@ var CPObjectControllerContentKey = @"CPObjectControllerCo return self; } -/* - @ignore - Can be used to get the actual value for key in stead of the controller markers -*/ -- (id)_valueForKey:(CPString)aKey +- (id)_controllerMarkerForValues:(CPArray)theValues { - return [[_controller selectedObjects] valueForKey:aKey]; -} - -- (id)valueForKey:(CPString)aKey -{ - var value = [_cachedValues objectForKey:aKey]; - - if (value !== undefined && value !== nil) - return value; - - var allValues = [[_controller selectedObjects] valueForKeyPath:aKey], - count = [allValues count]; + var count = [theValues count]; if (!count) value = CPNoSelectionMarker; else if (count === 1) - value = [allValues objectAtIndex:0]; + value = [theValues objectAtIndex:0]; else { if ([_controller alwaysUsesMultipleValuesMarker]) value = CPMultipleValuesMarker; else { - value = [allValues objectAtIndex:0]; + value = [theValues objectAtIndex:0]; - for (var i = 0, count= [allValues count]; i < count && value != CPMultipleValuesMarker; i++) + for (var i = 0, count= [theValues count]; i < count && value != CPMultipleValuesMarker; i++) { - if (![value isEqual:[allValues objectAtIndex:i]]) + if (![value isEqual:[theValues objectAtIndex:i]]) value = CPMultipleValuesMarker; } } } - [_cachedValues setValue:value forKey:aKey]; + return value; +} + +- (id)valueForKeyPath:(CPString)theKeyPath +{ + var value = [_cachedValues objectForKey:theKeyPath]; + + if (value !== undefined && value !== nil) + return value; + + var values = [[_controller selectedObjects] valueForKeyPath:theKeyPath]; + value = [self _controllerMarkerForValues:values]; + + [_cachedValues setObject:value forKey:theKeyPath]; return value; } +- (id)valueForKey:(CPString)theKeyPath +{ + return [self valueForKeyPath:theKeyPath]; +} + +- (void)setValue:(id)theValue forKeyPath:(CPString)theKeyPath +{ + [[_controller selectedObjects] setValue:theValue forKeyPath:theKeyPath]; + [_cachedValues removeObjectForKey:theKeyPath]; +} + +- (void)setValue:(id)theValue forKey:(CPString)theKeyPath +{ + [self setValue:theKeyPath forKeyPath:theKeyPath]; +} + - (unsigned)count { return [_cachedValues count]; @@ -584,11 +598,6 @@ var CPObjectControllerContentKey = @"CPObjectControllerCo return [_cachedValues keyEnumerator]; } -- (void)setValue:(id)aValue forKey:(CPString)aKey -{ - [[_controller selectedObjects] setValue:aValue forKey:aKey]; -} - - (void)controllerWillChange { _keys = [_cachedValues allKeys]; diff --git a/Foundation/CPKeyValueCoding.j b/Foundation/CPKeyValueCoding.j index 404146287..771d3194a 100644 --- a/Foundation/CPKeyValueCoding.j +++ b/Foundation/CPKeyValueCoding.j @@ -178,9 +178,6 @@ var CPObjectAccessorsForClassKey = @"$CPObjectAccessorsForClassKey", remainingKeyPath = aKeyPath.substring(firstDotIndex + 1), value = [self valueForKey:firstKeyComponent]; - if (CPIsControllerMarker(value)) - return value; - return [value valueForKeyPath:remainingKeyPath]; } @@ -213,24 +210,18 @@ var CPObjectAccessorsForClassKey = @"$CPObjectAccessorsForClassKey", - (void)setValue:(id)aValue forKeyPath:(CPString)aKeyPath { - if (!aKeyPath) aKeyPath = "self"; + if (!aKeyPath) aKeyPath = @"self"; - var i = 0, - keys = aKeyPath.split("."), - count = keys.length - 1, - owner = self; + var firstDotIndex = aKeyPath.indexOf("."); - for (; i < count; ++i) - { - var newOwner = [owner valueForKey:keys[i]]; + if (firstDotIndex === -1) + return [self setValue:aValue forKey:aKeyPath]; - if (CPIsControllerMarker(newOwner)) - newOwner = [owner _valueForKey:keys[i]]; + var firstKeyComponent = aKeyPath.substring(0, firstDotIndex), + remainingKeyPath = aKeyPath.substring(firstDotIndex + 1), + value = [self valueForKey:firstKeyComponent]; - owner = newOwner; - } - - [owner setValue:aValue forKey:keys[i]]; + return [value setValue:aValue forKeyPath:remainingKeyPath]; } - (void)setValue:(id)aValue forKey:(CPString)aKey diff --git a/Tests/AppKit/CPArrayControllerTest.j b/Tests/AppKit/CPArrayControllerTest.j index e07bce3d6..9ebf65227 100644 --- a/Tests/AppKit/CPArrayControllerTest.j +++ b/Tests/AppKit/CPArrayControllerTest.j @@ -13,9 +13,9 @@ { _contentArray = []; - [_contentArray addObject:[Person personWithName:@"Francisco" age:21]]; - [_contentArray addObject:[Person personWithName:@"Ross" age:30]]; - [_contentArray addObject:[Person personWithName:@"Tom" age:15]]; + [_contentArray addObject:[Employee employeeWithName:@"Francisco" department:[Department departmentWithName:@"Cappuccino"]]]; + [_contentArray addObject:[Employee employeeWithName:@"Ross" department:[Department departmentWithName:@"Cappuccino"]]]; + [_contentArray addObject:[Employee employeeWithName:@"Tom" department:[Department departmentWithName:@"CommonJS"]]]; // Copy the array since we'll reuse the original array later. Also see issue #795. _arrayController = [[CPArrayController alloc] initWithContent:[[self contentArray] copy]]; @@ -46,10 +46,10 @@ - (void)testInsertObjectAtArrangedObjectIndex { - var object = [Person personWithName:@"Klaas Pieter" age:24], + var object = [Employee employeeWithName:@"Klaas Pieter" department:[Department departmentWithName:@"Theming"]], arrayController = [self arrayController]; - [arrayController setSortDescriptors:[[CPSortDescriptor sortDescriptorWithKey:@"age" ascending:YES]]]; + [arrayController setSortDescriptors:[[CPSortDescriptor sortDescriptorWithKey:@"name" ascending:YES]]]; [arrayController insertObject:object atArrangedObjectIndex:1]; [self assert:object equals:[[arrayController arrangedObjects] objectAtIndex:1]]; @@ -296,6 +296,24 @@ [self assert:newSelection equals:[arrayController selectionIndexes] message:@"selection was not set properly"]; } +- (void)testCompoundKeyPaths +{ + var departmentNameField = [[CPTextField alloc] init]; + [departmentNameField bind:@"value" toObject:[self arrayController] withKeyPath:@"selection.department.name" options:nil]; + + // This should be 'No Selection' + [self assert:@"" equals:[departmentNameField stringValue]]; + + [[self arrayController] setSelectionIndexes:[CPIndexSet indexSetWithIndex:1]]; + [self assert:@"Cappuccino" equals:[departmentNameField stringValue]]; + + [[self arrayController] setSelectionIndexes:[CPIndexSet indexSetWithIndexesInRange:CPMakeRange(0, 2)]]; + [self assert:@"Cappuccino" equals:[departmentNameField stringValue] message:@"key path values should be equal"]; + + [[self arrayController] setValue:@"280North" forKeyPath:@"selection.department.name"]; + [self assert:@"280North" equals:[departmentNameField stringValue]]; +} + - (void)observeValueForKeyPath:keyPath ofObject:anActivity change:change @@ -313,23 +331,23 @@ @end -@implementation Person : CPObject +@implementation Employee : CPObject { - CPString _name @accessors(property=name); - int _age @accessors(property=age); + CPString _name @accessors(property=name); + Department _department @accessors(property=department); } -+ (id)personWithName:(CPString)aName age:(int)anAge ++ (id)employeeWithName:(CPString)theName department:(Department)theDepartment { - return [[self alloc] initWithName:aName age:anAge]; + return [[self alloc] initWithName:theName department:theDepartment]; } -- (id)initWithName:(CPString)aName age:(int)anAge +- (id)initWithName:(CPString)theName department:(Department)theDepartment { if (self = [super init]) { - _name = aName; - _age = anAge; + _name = theName; + _department = theDepartment; } return self; @@ -337,7 +355,29 @@ - (CPString)description { - return [CPString stringWithFormat:@"", [self name], [self age]]; + return [CPString stringWithFormat:@"", [self name]]; +} + +@end + +@implementation Department : CPObject +{ + CPString _name @accessors(property=name); +} + ++ (id)departmentWithName:(CPString)theName +{ + return [[self alloc] initWithName:theName]; +} + +- (id)initWithName:(CPString)theName +{ + if (self = [super init]) + { + _name = theName; + } + + return self; } @end \ No newline at end of file diff --git a/Tests/Foundation/CPKeyValueCodingTest.j b/Tests/Foundation/CPKeyValueCodingTest.j index 7cf5678fa..cd3cb61f7 100644 --- a/Tests/Foundation/CPKeyValueCodingTest.j +++ b/Tests/Foundation/CPKeyValueCodingTest.j @@ -37,13 +37,6 @@ [self assert:department equals:[employee valueForKey:@"department"]]; [self assert:@"Engineering" equals:[employee valueForKeyPath:@"department.name"]]; - - // When using selection bindings, part of a keypath might contain a controller marker - // (e.g. the selection.employees part of selection.employees.name might return the CPMultipleValuesMarker) - // This test makes sure that in such a case we return the controller marker in stead of trying to ask the controller - // marker for it's value for key. - [employee setDepartment:CPMultipleValuesMarker]; - [self assert:CPMultipleValuesMarker equals:[employee valueForKeyPath:@"department.name"]]; } @end