Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] Support two-argument shorthand for "match" expression #12415

Closed
wants to merge 5 commits into from

Conversation

jfirebaugh
Copy link
Contributor

Companion to mapbox/mapbox-gl-js#6964.

Dependent on getting some ignores or fixes for some new render tests (cc @ChrisLoer):

@1ec5 Does this require any NSExpression-related work?

It looks like it should fit into this Android API naturally. I think it should have had a @Size(min = 4) annotation, but after this change @Size(min = 2) is correct. cc @tobrun

@1ec5
Copy link
Contributor

1ec5 commented Jul 18, 2018

Does this require any NSExpression-related work?

If a property getter in mbgl can return a match expression that uses the shorthand, then +[NSExpression expressionWithMGLJSONObject:] should detect that there’s only one argument and synthesize TRUE and FALSE output values.

} else if ([op isEqualToString:@"match"]) {
NSMutableArray *optionsArray = [NSMutableArray array];
for (NSUInteger index = 0; index < argumentObjects.count; index++) {
NSExpression *option = [NSExpression expressionWithMGLJSONObject:argumentObjects[index]];
// match operators with arrays as matching values should not parse arrays as generic functions.
if (index > 0 && index < argumentObjects.count - 1 && !(index % 2 == 0) && [argumentObjects[index] isKindOfClass:[NSArray class]]) {
option = [NSExpression expressionForAggregate:MGLSubexpressionsWithJSONObjects(argumentObjects[index])];
}
[optionsArray addObject:option];
}
return [NSExpression expressionForFunction:@"MGL_MATCH"
arguments:optionsArray];

The usual case for this shorthand would be as part of a predicate rather than an expression set on a property. That’ll already work OK, thanks to this fallback that stuffs the expression in an == TRUE expression:

NSExpression *expression = [NSExpression expressionWithMGLJSONObject:object];
return [NSComparisonPredicate predicateWithLeftExpression:expression
rightExpression:[NSExpression expressionForConstantValue:@YES]
modifier:NSDirectPredicateModifier
type:NSEqualToPredicateOperatorType
options:0];

It would be nice for ["match", ["get", "type"], ["a", "b", "c"]] to translate to type IN {'a', 'b', 'c'} rather than MGL_MATCH(type, {'a', 'b', 'c'}, TRUE, FALSE) == TRUE, but that can be tail work if necessary.

Conversely, the latter two arguments to this match expression can be omitted, though there shouldn’t be any functional difference:

NSExpression *matchExpression = [NSExpression expressionForFunction:@"MGL_MATCH"
arguments:@[self.leftExpression,
self.rightExpression,
[NSExpression expressionForConstantValue:@YES],
[NSExpression expressionForConstantValue:@NO]]];

/cc @fabian-guerra

@jfirebaugh
Copy link
Contributor Author

If a property getter in mbgl can return a match expression that uses the shorthand, then +[NSExpression expressionWithMGLJSONObject:] should detect that there’s only one argument and synthesize TRUE and FALSE output values.

The shorthand is converted to longhand at parse time, so property getters will return the longhand form.

@kkaefer kkaefer added the Core The cross-platform C++ core, aka mbgl label Jul 31, 2018
@stale stale bot added the archived Archived because of inactivity label Nov 10, 2018
@stale
Copy link

stale bot commented Nov 11, 2018

This pull request has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this Nov 11, 2018
@tobrun
Copy link
Member

tobrun commented Nov 13, 2018

Closing since no one seems to be a strong yes on this feature

From mapbox/mapbox-gl-js#6964 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived Archived because of inactivity Core The cross-platform C++ core, aka mbgl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants