Folder selection in smart folders is fragile

[barijaona]

To improve legibility, the smart folder editor indents folder names when it lists the existing folders to choose from:

This is done through the -fillFolderValueField:atIndent: method.

vienna-rss/Vienna/Sources/Alerts/SmartFolder.m

Lines 262 to 301 in c516ee1

	// folder is / is not
	NSArray<NSExpression *> *folders = [self fillFolderValueField:VNAFolderTypeRoot atIndent:0];
	NSPredicateEditorRowTemplate *folderTemplate = [[NSPredicateEditorRowTemplate alloc]
	initWithLeftExpressions:@[[NSExpression expressionForConstantValue:MA_Field_Folder]]
	rightExpressions:folders
	modifier:NSDirectPredicateModifier
	operators:@[@(NSEqualToPredicateOperatorType), @(NSNotEqualToPredicateOperatorType)]
	options:0];
	[rowTemplates addObject:folderTemplate];
	self.predicateEditor.rowTemplates = rowTemplates;
	}
	/// Fill the folder value field popup menu with a list of all RSS and group
	/// folders in the database under the specified folder ID. The indentation value
	/// is used to indent the items in the menu when they are part of a group. I've
	/// used an increment of 2 which looks clearer than 1 in the UI.
	- (NSArray<NSExpression *> *)fillFolderValueField:(NSInteger)fromId
	atIndent:(NSInteger)indentation
	{
	NSMutableArray<NSExpression *> *expressions = [NSMutableArray array];
	NSArray *folders = [self.database arrayOfFolders:fromId];
	NSArray *sortedFolders = [folders sortedArrayUsingSelector:@selector(folderNameCompare:)];
	for (Folder *folder in sortedFolders) {
	if (folder.type == VNAFolderTypeRSS ||
	folder.type == VNAFolderTypeOpenReader ||
	folder.type == VNAFolderTypeGroup) {
	NSString *indentSpaces = [@"" stringByPaddingToLength:indentation * 2
	withString:@" "
	startingAtIndex:0];
	NSString *constantValue = [indentSpaces stringByAppendingString:folder.name];
	[expressions addObject:[NSExpression expressionForConstantValue:constantValue]];
	if (folder.type == VNAFolderTypeGroup) {
	[expressions addObjectsFromArray:[self fillFolderValueField:folder.itemId
	atIndent:indentation + 1]];
	}
	}
	}
	return expressions;
	}

I have identified two issues caused by this:

1. If I select a feed which is located inside a subfolder, the smart folder does not work as intended. The extra spaces in the expression prevent correct identification of the feed in the folderSqlString function of Criteria+SQL.swift :
   

   vienna-rss/Vienna/Sources/Criteria/Criteria+SQL.swift

   Lines 151 to 152 in c516ee1

   	func folderSqlString(sqlFieldName: String, database: Database) -> String {
   	let folder = database.folder(fromName: value)

2. If I move a feed (or a folder) into a group folder or out from it, the editor gets confused when I attempt to edit a preexisting smart folder involving this feed/folder, because the number of indentation spaces has changed.

[Eitot]

It might be worth adding a custom predicate row template and using NSMenuItem's indentationLevel property or using NSMenuItem submenus for folders to separate the presentation from the model.

I can have a look, I have some draft code somewhere that I might be able to repurpose.

[TAKeanice]

I think another approach could be to strip leading spaces when converting the predicate to xml (which is the canonical representation and gets converted to SQL).

[Eitot]

I actually cannot get indentationLevel or submenus to work. It seems that NSPredicateEditor ignores most of NSPopUpButton/NSMenuItem/NSMenu properties in row templates, including submenus and indentationLevel. I also tried setting a menu-item icon, which did not work either.

@TAKeanice: What about loading predicates back into NSPredicateEditor? If the saved predicate does not match any of the templates, it won't select the option.

[TAKeanice]

I started a solution which converts from folders in criteria to indented folder names in predicates, and from indented folder names in predicates to folders in criteria. To make it rename resistant, I will use folder IDs in criteria and the XML representation. The last piece of the puzzle I still need to add is a way to convert all predicates on app startup, if something like the folder name change to folder id must be performed.

[TAKeanice]

Actually, a custom presentation taking in folder IDs and showing them as indented folder names would be cleaner, but it works this way.