[Adium-devl] [PATCH] CL filtering expanding fix (want review)

[Colin Barrett]

I've been seeing a number of problems with CL filtering causing groups  
to expand and contract. I have what I think is a pretty good patch to  
fix this issue. Quoting patch's commit message:

> Fix many problems with the contact list filtering not properly  
> restoring group expanding, and not properly expanding groups.
>
> Instead of trying to twiddle the view proxies directly, we set a  
> special display property on each expanded group's model object. The  
> list controller checks this property when deciding if an item is  
> expanded or not.

I have another patch that fixes this in a way more similar to the old  
method of poking the state of the model objects directly (but this  
time saving things properly by stashing the old values in, again,  
display properties), but this way seems cleaner to me: expanding  
during filtering is a temporary view expansion and shouldn't actually  
change the state of the object. Additionally, actually flipping the  
model object's state means that if we crash during a filtering  
operation, there's a chance user's groups will all be expanded when  
she starts up next.

What do others think of this? I've copy pasted the patch below, and  
attached a bundle. Personally, I'm a little wary of [contactListView  
reloadData], it seems too straightforwad for Adium code ;)

Suggestions welcome!



=====================================================================

diff --git a/Frameworks/Adium Framework/Source/ 
AIAbstractListController.m b/Frameworks/Adium Framework/Source/ 
AIAbstractListController.m
--- a/Frameworks/Adium Framework/Source/AIAbstractListController.m	
+++ b/Frameworks/Adium Framework/Source/AIAbstractListController.m	
@@ -673,7 +673,7 @@
  //
  - (BOOL)outlineView:(NSOutlineView *)outlineView expandStateOfItem: 
(AIProxyListObject *)item
  {
-    return !item || ((id<AIContainingObject>) 
(item.listObject)).expanded;
+    return !item || ((id<AIContainingObject>) 
(item.listObject)).expanded || [item.listObject  
boolValueForProperty:@"ExpandedByFiltering"];
  }

  /*!
diff --git a/Source/AIListWindowController.m b/Source/ 
AIListWindowController.m
--- a/Source/AIListWindowController.m
+++ b/Source/AIListWindowController.m
@@ -1666,54 +1666,34 @@
  		return;
  	
  	if (!filterBarExpandedGroups && ![[sender stringValue]  
isEqualToString:@""]) {
-		// Temporarily expand all groups when performing a search.
-		NSMutableArray *groupsToExpand = [NSMutableArray array];
-		NSUInteger rows = [contactListView numberOfRows];
-		for (int i = 0; i < rows; i++) {
-			AIProxyListObject *proxyObject = [contactListView itemAtRow:i];
-			if ([proxyObject isKindOfClass:[AIListGroup class]] &&  
((AIListGroup *)proxyObject).expanded == NO)
-				[groupsToExpand addObject:proxyObject];
-		}
-		
-		if (groupsToExpand.count) {
-			for (AIProxyListObject *proxyObject in groupsToExpand) {
-				// Force the listgroup to save its expanded status
-				AIListGroup *proxiedObject = (AIListGroup *)proxyObject;
-				
-				[proxiedObject setPreference:[NSNumber  
numberWithBool:proxiedObject.expanded]
-									  forKey:@"IsExpanded"
-									   group:@"Contact List"];
-				
-				// Set the group as expanded
-				[contactListView expandItem:proxyObject];
-			}			
-		}
-		
-		filterBarExpandedGroups = YES;
-		
-	} else if (filterBarExpandedGroups && [[sender stringValue]  
isEqualToString:@""]) {
-		// Restore saved expansion status when returning to no search.
-		
-		// Temporarily expand all groups when performing a search.
-		NSMutableArray *groupsToCollapse = [NSMutableArray array];
-		NSUInteger rows = [contactListView numberOfRows];
-		for (int i = 0; i < rows; i++) {
-			AIProxyListObject *proxyObject = [contactListView itemAtRow:i];
-			
-			if ([proxyObject isKindOfClass:[AIListGroup class]] &&
-				![[(AIListGroup *)proxyObject preferenceForKey:@"IsExpanded"  
group:@"Contact List"] boolValue]) {
-				[groupsToCollapse addObject:proxyObject];
-			}
-		}
-		
-		if (groupsToCollapse.count) {
-			for (AIProxyListObject *proxyObject in groupsToCollapse) {
-				[contactListView collapseItem:proxyObject];
-			}			
-		}
-		
-		filterBarExpandedGroups = NO;
-	}
+        BOOL modified = NO;
+        for (AIListObject *listObject in [self.contactList  
containedObjects]) {
+            if ([listObject isKindOfClass:[AIListGroup class]] &&  
[(AIListGroup *)listObject isExpanded] == NO) {
+                [listObject setValue:[NSNumber numberWithBool:YES]  
forProperty:@"ExpandedByFiltering" notify:NotifyNever];
+                modified = YES;
+            }
+        }
+
+        filterBarExpandedGroups = YES;
+
+        if (modified) {
+            [contactListView reloadData];
+        }
+    } else if (filterBarExpandedGroups && [[sender stringValue]  
isEqualToString:@""]) {
+        BOOL modified = NO;
+        for (AIListObject *listObject in [self.contactList  
containedObjects]) {
+            if ([listObject isKindOfClass:[AIListGroup class]] &&  
[listObject boolValueForProperty:@"ExpandedByFiltering"]) {
+                [listObject setValue:[NSNumber numberWithBool:NO]  
forProperty:@"ExpandedByFiltering" notify:NotifyNever];
+                modified = YES;
+            }
+        }
+
+        filterBarExpandedGroups = NO;
+
+        if (modified) {
+            [contactListView reloadData];
+        }
+    }
  	
  	if ([[AIContactHidingController sharedController] filterContacts: 
[sender stringValue]]) {
  		// Select the first contact; we're guaranteed at least one visible  
contact.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://adium.im/pipermail/devel_adium.im/attachments/20090424/09ca2311/attachment-0002.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: expandfix.hgbundle
Type: application/octet-stream
Size: 1351 bytes
Desc: not available
URL: <http://adium.im/pipermail/devel_adium.im/attachments/20090424/09ca2311/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://adium.im/pipermail/devel_adium.im/attachments/20090424/09ca2311/attachment-0003.html>