[ios, macos] Add support for subscripting in expressions. - #11770
Conversation
1ec5
left a comment
There was a problem hiding this comment.
Besides the comment below, please add a blurb to the iOS and macOS changelogs. Thanks!
| if ([value isEqualToString:@"FIRST"]) { | ||
| index = [NSExpression expressionForConstantValue:@0].mgl_jsonExpressionObject; | ||
| } else if ([value isEqualToString:@"LAST"]) { | ||
| NSArray *array = self.arguments[0].constantValue; |
There was a problem hiding this comment.
The first argument isn’t necessarily a constant value; it could be an aggregate expression or perhaps a conditional expression. Instead, synthesize an expression of the form count(%@) - 1, where %@ is arguments[0]. Also add a unit test to exercise this case.
There was a problem hiding this comment.
There are tests for each case:
mapbox-gl-native/platform/darwin/test/MGLExpressionTests.mm
Lines 843 to 844 in 8de1490
a5cfe58 to
8de1490
Compare
| index = [NSExpression expressionForConstantValue:@0].mgl_jsonExpressionObject; | ||
| } else if ([value isEqualToString:@"LAST"]) { | ||
| NSExpression *count = [NSExpression expressionWithFormat:@"count(%@) - 1", self.arguments[0]]; | ||
| id last = [count expressionValueWithObject:nil context:nil]; |
There was a problem hiding this comment.
What if the first argument is an expression like CAST(color, 'NSArray')? Casting a color to an array is unimplemented by default in NSExpression, so evaluating the expression will raise an exception. Instead, get the mgl_jsonExpressionObject of count itself, similar to how the SIZE case is implemented.
| index = [NSExpression expressionForConstantValue:@0].mgl_jsonExpressionObject; | ||
| } else if ([value isEqualToString:@"LAST"]) { | ||
| NSExpression *count = [NSExpression expressionWithFormat:@"count(%@) - 1", self.arguments[0]]; | ||
| index = count.mgl_jsonExpressionObject; |
There was a problem hiding this comment.
Consider inlining count here, because the variable name count is a bit misleading. (It’s the last index, not the count.)
1572edf to
827998a
Compare
Fixes #11757