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

Commit

Permalink
Use appropriate part of speech for properties (#7457)
Browse files Browse the repository at this point in the history
* [ios, macos] Use appropriate part of speech for properties

Fixed overridden property references in requirements lists. Boolean-typed properties can now have getters beginning with “is”.

Renamed a number of layout properties according to the following rules: Boolean-typed properties should include a verb; other properties must be noun phrases; all properties must be grammatical.

* [ios, macos] Added style spec names as unavailable aliases

Renamed properties now have aliases based on their style specification names, marked unavailable, for wayfinding purposes.

* [ios, macos] Fixed autosynthesis warnings

* [ios, macos] Moved style layer test template to test folder

* [ios, macos] Customized iconOptional getter

* [ios, macos] Avoid autosynthesis of aliases

* [ios, macos] Test that property names are grammatical

Run property getter names through a basic battery of tests to see if they’re grammatical. Most part-of-speech tagging tests are guarded by a compile-time flag, off by default, because NSLinguisticTagger does a poor job of telling nouns from verbs, and we’ve intentionally kept many words in property names that could be read as either verbs or nouns (like “transform” or “scale”).
  • Loading branch information
1ec5 authored Dec 16, 2016
1 parent b846694 commit 3f26e88
Show file tree
Hide file tree
Showing 29 changed files with 751 additions and 243 deletions.
45 changes: 30 additions & 15 deletions platform/darwin/scripts/generate-style-code.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,33 @@ const suffix = 'StyleLayer';
// Rename properties and keep `original` for use with setters and getters
_.forOwn(cocoaConventions, function (properties, kind) {
_.forOwn(properties, function (newName, oldName) {
spec[kind][newName] = spec[kind][oldName];
spec[kind][newName].original = oldName;
let property = spec[kind][oldName];
if (newName.startsWith('is-')) {
property.getter = newName;
newName = newName.substr(3);
}
if (newName !== oldName) {
property.original = oldName;
}
delete spec[kind][oldName];
spec[kind][newName] = property;

// Update requirements in other properties.
let updateRequirements = function (property, name) {
let requires = property.requires || [];
for (let i = 0; i < requires.length; i++) {
if (requires[i] in cocoaConventions[kind]) {
property.requires[i] = cocoaConventions[kind][requires[i]];
if (requires[i] === oldName) {
property.requires[i] = newName;
}
if (typeof requires[i] !== 'string') {
let prop = name;
_.forOwn(requires[i], function (values, name, require) {
if (name === oldName) {
require[newName] = values;
delete require[name];
}
});
}
_.forOwn(requires[i], function (values, name, require) {
if (require in cocoaConventions[kind]) {
require[cocoaConventions[kind][name]] = values;
}
});
}
};
_.forOwn(spec[kind.replace(/^layout_/, 'paint_')], updateRequirements);
Expand All @@ -51,6 +62,10 @@ global.objCName = function (property) {
return camelizeWithLeadingLowercase(property.name);
}

global.objCGetter = function (property) {
return camelizeWithLeadingLowercase(property.getter || property.name);
}

global.objCType = function (layerType, propertyName) {
return `${prefix}${camelize(propertyName)}`;
}
Expand Down Expand Up @@ -149,15 +164,15 @@ global.propertyDoc = function (propertyName, property, layerType) {
return doc.replace(/(p)ixel/gi, '$1oint').replace(/(\d)px\b/g, '$1pt');
};

global.propertyReqs = function (property, layoutPropertiesByName, type) {
global.propertyReqs = function (property, propertiesByName, type) {
return 'This property is only applied to the style if ' + property.requires.map(function (req) {
if (typeof req === 'string') {
return '`' + camelizeWithLeadingLowercase(req) + '` is non-`nil`';
} else if ('!' in req) {
return '`' + camelizeWithLeadingLowercase(req['!']) + '` is set to `nil`';
} else {
let name = Object.keys(req)[0];
return '`' + camelizeWithLeadingLowercase(name) + '` is set to an `MGLStyleValue` object containing ' + describeValue(req[name], layoutPropertiesByName[name], type);
return '`' + camelizeWithLeadingLowercase(name) + '` is set to an `MGLStyleValue` object containing ' + describeValue(req[name], propertiesByName[name], type);
}
}).join(', and ') + '. Otherwise, it is ignored.';
};
Expand Down Expand Up @@ -318,19 +333,19 @@ global.setSourceLayer = function() {
}

global.mbglType = function(property) {
let mbglType = camelize(property.name) + 'Type';
if (/-translate-anchor$/.test(property.name)) {
let mbglType = camelize(originalPropertyName(property)) + 'Type';
if (/-translate-anchor$/.test(originalPropertyName(property))) {
mbglType = 'TranslateAnchorType';
}
if (/-(rotation|pitch)-alignment$/.test(property.name)) {
if (/-(rotation|pitch)-alignment$/.test(originalPropertyName(property))) {
mbglType = 'AlignmentType';
}
return mbglType;
}

const layerH = ejs.compile(fs.readFileSync('platform/darwin/src/MGLStyleLayer.h.ejs', 'utf8'), { strict: true });
const layerM = ejs.compile(fs.readFileSync('platform/darwin/src/MGLStyleLayer.mm.ejs', 'utf8'), { strict: true});
const testLayers = ejs.compile(fs.readFileSync('platform/darwin/src/MGLRuntimeStylingTests.m.ejs', 'utf8'), { strict: true});
const testLayers = ejs.compile(fs.readFileSync('platform/darwin/test/MGLStyleLayerTests.m.ejs', 'utf8'), { strict: true});
const categoryH = ejs.compile(fs.readFileSync('platform/darwin/src/NSValue+MGLStyleEnumAttributeAdditions.h.ejs', 'utf8'), { strict: true});
const categoryM = ejs.compile(fs.readFileSync('platform/darwin/src/NSValue+MGLStyleEnumAttributeAdditions.mm.ejs', 'utf8'), { strict: true});

Expand Down
19 changes: 18 additions & 1 deletion platform/darwin/scripts/style-spec-cocoa-conventions-v8.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,21 @@
{
"layout_symbol": {
"icon-allow-overlap": "icon-allows-overlap",
"icon-image": "icon-image-name",
"icon-size": "icon-scale"
"icon-ignore-placement": "icon-ignores-placement",
"icon-keep-upright": "keeps-icon-upright",
"icon-optional": "is-icon-optional",
"icon-rotate": "icon-rotation",
"icon-size": "icon-scale",
"symbol-avoid-edges": "symbol-avoids-edges",
"text-allow-overlap": "text-allows-overlap",
"text-ignore-placement": "text-ignores-placement",
"text-justify": "text-justification",
"text-keep-upright": "keeps-text-upright",
"text-max-angle": "maximum-text-angle",
"text-max-width": "maximum-text-width",
"text-optional": "is-text-optional",
"text-rotate": "text-rotation"
},
"paint_raster": {
"raster-brightness-min": "minimum-raster-brightness",
Expand All @@ -10,5 +24,8 @@
},
"paint_line": {
"line-dasharray": "line-dash-pattern"
},
"paint_fill": {
"fill-antialias": "is-fill-antialiased"
}
}
2 changes: 1 addition & 1 deletion platform/darwin/src/MGLBackgroundStyleLayer.mm
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ - (void)setRawLayer:(mbgl::style::BackgroundLayer *)rawLayer
super.rawLayer = rawLayer;
}

#pragma mark - Adding to and removing from a map view
#pragma mark - Adding to and removing from a map view

- (void)addToMapView:(MGLMapView *)mapView belowLayer:(MGLStyleLayer *)otherLayer
{
Expand Down
2 changes: 1 addition & 1 deletion platform/darwin/src/MGLCircleStyleLayer.mm
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ - (NSPredicate *)predicate
return [NSPredicate mgl_predicateWithFilter:self.rawLayer->getFilter()];
}

#pragma mark - Adding to and removing from a map view
#pragma mark - Adding to and removing from a map view

- (void)addToMapView:(MGLMapView *)mapView belowLayer:(MGLStyleLayer *)otherLayer
{
Expand Down
9 changes: 7 additions & 2 deletions platform/darwin/src/MGLFillStyleLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,13 @@ typedef NS_ENUM(NSUInteger, MGLFillTranslateAnchor) {
Whether or not the fill should be antialiased.
The default value of this property is an `MGLStyleValue` object containing an `NSNumber` object containing `YES`. Set this property to `nil` to reset it to the default value.
This attribute corresponds to the <a href="https://www.mapbox.com/mapbox-gl-style-spec/#paint-fill-antialias"><code>fill-antialias</code></a> paint property in the Mapbox Style Specification.
*/
@property (nonatomic, null_resettable) MGLStyleValue<NSNumber *> *fillAntialias;
@property (nonatomic, null_resettable, getter=isFillAntialiased) MGLStyleValue<NSNumber *> *fillAntialiased;


@property (nonatomic, null_resettable) MGLStyleValue<NSNumber *> *fillAntialias __attribute__((unavailable("Use fillAntialiased instead.")));

#if TARGET_OS_IPHONE
/**
Expand Down Expand Up @@ -69,7 +74,7 @@ typedef NS_ENUM(NSUInteger, MGLFillTranslateAnchor) {
/**
The outline color of the fill. Matches the value of `fillColor` if unspecified.
This property is only applied to the style if `fillPattern` is set to `nil`, and `fillAntialias` is set to an `MGLStyleValue` object containing an `NSNumber` object containing `YES`. Otherwise, it is ignored.
This property is only applied to the style if `fillPattern` is set to `nil`, and `fillAntialiased` is set to an `MGLStyleValue` object containing an `NSNumber` object containing `YES`. Otherwise, it is ignored.
*/
@property (nonatomic, null_resettable) MGLStyleValue<MGLColor *> *fillOutlineColor;

Expand Down
13 changes: 9 additions & 4 deletions platform/darwin/src/MGLFillStyleLayer.mm
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ - (NSPredicate *)predicate
return [NSPredicate mgl_predicateWithFilter:self.rawLayer->getFilter()];
}

#pragma mark - Adding to and removing from a map view
#pragma mark - Adding to and removing from a map view

- (void)addToMapView:(MGLMapView *)mapView belowLayer:(MGLStyleLayer *)otherLayer
{
Expand Down Expand Up @@ -119,20 +119,25 @@ - (void)removeFromMapView:(MGLMapView *)mapView

#pragma mark - Accessing the Paint Attributes

- (void)setFillAntialias:(MGLStyleValue<NSNumber *> *)fillAntialias {
- (void)setFillAntialiased:(MGLStyleValue<NSNumber *> *)fillAntialiased {
MGLAssertStyleLayerIsValid();

auto mbglValue = MGLStyleValueTransformer<bool, NSNumber *>().toPropertyValue(fillAntialias);
auto mbglValue = MGLStyleValueTransformer<bool, NSNumber *>().toPropertyValue(fillAntialiased);
self.rawLayer->setFillAntialias(mbglValue);
}

- (MGLStyleValue<NSNumber *> *)fillAntialias {
- (MGLStyleValue<NSNumber *> *)isFillAntialiased {
MGLAssertStyleLayerIsValid();

auto propertyValue = self.rawLayer->getFillAntialias() ?: self.rawLayer->getDefaultFillAntialias();
return MGLStyleValueTransformer<bool, NSNumber *>().toStyleValue(propertyValue);
}


- (void)setFillAntialias:(MGLStyleValue<NSNumber *> *)fillAntialias {
NSAssert(NO, @"Use -setFillAntialiased: instead.");
}

- (void)setFillColor:(MGLStyleValue<MGLColor *> *)fillColor {
MGLAssertStyleLayerIsValid();

Expand Down
3 changes: 3 additions & 0 deletions platform/darwin/src/MGLLineStyleLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ typedef NS_ENUM(NSUInteger, MGLLineTranslateAnchor) {
*/
@property (nonatomic, null_resettable) MGLStyleValue<NSArray<NSNumber *> *> *lineDashPattern;


@property (nonatomic, null_resettable) MGLStyleValue<NSArray<NSNumber *> *> *lineDasharray __attribute__((unavailable("Use lineDashPattern instead.")));

/**
Draws a line casing outside of a line's actual path. Value indicates the width of the inner gap.
Expand Down
7 changes: 6 additions & 1 deletion platform/darwin/src/MGLLineStyleLayer.mm
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ - (NSPredicate *)predicate
return [NSPredicate mgl_predicateWithFilter:self.rawLayer->getFilter()];
}

#pragma mark - Adding to and removing from a map view
#pragma mark - Adding to and removing from a map view

- (void)addToMapView:(MGLMapView *)mapView belowLayer:(MGLStyleLayer *)otherLayer
{
Expand Down Expand Up @@ -231,6 +231,11 @@ - (void)setLineDashPattern:(MGLStyleValue<NSArray<NSNumber *> *> *)lineDashPatte
return MGLStyleValueTransformer<std::vector<float>, NSArray<NSNumber *> *, float>().toStyleValue(propertyValue);
}


- (void)setLineDasharray:(MGLStyleValue<NSArray<NSNumber *> *> *)lineDasharray {
NSAssert(NO, @"Use -setLineDashPattern: instead.");
}

- (void)setLineGapWidth:(MGLStyleValue<NSNumber *> *)lineGapWidth {
MGLAssertStyleLayerIsValid();

Expand Down
9 changes: 9 additions & 0 deletions platform/darwin/src/MGLRasterStyleLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ NS_ASSUME_NONNULL_BEGIN
*/
@property (nonatomic, null_resettable) MGLStyleValue<NSNumber *> *maximumRasterBrightness;


@property (nonatomic, null_resettable) MGLStyleValue<NSNumber *> *rasterBrightnessMax __attribute__((unavailable("Use maximumRasterBrightness instead.")));

/**
Increase or reduce the brightness of the image. The value is the minimum brightness.
Expand All @@ -34,6 +37,9 @@ NS_ASSUME_NONNULL_BEGIN
*/
@property (nonatomic, null_resettable) MGLStyleValue<NSNumber *> *minimumRasterBrightness;


@property (nonatomic, null_resettable) MGLStyleValue<NSNumber *> *rasterBrightnessMin __attribute__((unavailable("Use minimumRasterBrightness instead.")));

/**
Increase or reduce the contrast of the image.
Expand Down Expand Up @@ -61,6 +67,9 @@ NS_ASSUME_NONNULL_BEGIN
*/
@property (nonatomic, null_resettable) MGLStyleValue<NSNumber *> *rasterHueRotation;


@property (nonatomic, null_resettable) MGLStyleValue<NSNumber *> *rasterHueRotate __attribute__((unavailable("Use rasterHueRotation instead.")));

/**
The opacity at which the image will be drawn.
Expand Down
17 changes: 16 additions & 1 deletion platform/darwin/src/MGLRasterStyleLayer.mm
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ - (void)setRawLayer:(mbgl::style::RasterLayer *)rawLayer
super.rawLayer = rawLayer;
}

#pragma mark - Adding to and removing from a map view
#pragma mark - Adding to and removing from a map view

- (void)addToMapView:(MGLMapView *)mapView belowLayer:(MGLStyleLayer *)otherLayer
{
Expand Down Expand Up @@ -96,6 +96,11 @@ - (void)setMaximumRasterBrightness:(MGLStyleValue<NSNumber *> *)maximumRasterBri
return MGLStyleValueTransformer<float, NSNumber *>().toStyleValue(propertyValue);
}


- (void)setRasterBrightnessMax:(MGLStyleValue<NSNumber *> *)rasterBrightnessMax {
NSAssert(NO, @"Use -setMaximumRasterBrightness: instead.");
}

- (void)setMinimumRasterBrightness:(MGLStyleValue<NSNumber *> *)minimumRasterBrightness {
MGLAssertStyleLayerIsValid();

Expand All @@ -110,6 +115,11 @@ - (void)setMinimumRasterBrightness:(MGLStyleValue<NSNumber *> *)minimumRasterBri
return MGLStyleValueTransformer<float, NSNumber *>().toStyleValue(propertyValue);
}


- (void)setRasterBrightnessMin:(MGLStyleValue<NSNumber *> *)rasterBrightnessMin {
NSAssert(NO, @"Use -setMinimumRasterBrightness: instead.");
}

- (void)setRasterContrast:(MGLStyleValue<NSNumber *> *)rasterContrast {
MGLAssertStyleLayerIsValid();

Expand Down Expand Up @@ -152,6 +162,11 @@ - (void)setRasterHueRotation:(MGLStyleValue<NSNumber *> *)rasterHueRotation {
return MGLStyleValueTransformer<float, NSNumber *>().toStyleValue(propertyValue);
}


- (void)setRasterHueRotate:(MGLStyleValue<NSNumber *> *)rasterHueRotate {
NSAssert(NO, @"Use -setRasterHueRotation: instead.");
}

- (void)setRasterOpacity:(MGLStyleValue<NSNumber *> *)rasterOpacity {
MGLAssertStyleLayerIsValid();

Expand Down
12 changes: 10 additions & 2 deletions platform/darwin/src/MGLStyleLayer.h.ejs
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,12 @@ typedef NS_ENUM(NSUInteger, MGL<%- camelize(property.name) %>) {
This attribute corresponds to the <a href="https://www.mapbox.com/mapbox-gl-style-spec/#layout-<%- type -%>-<%- property.original -%>"><code><%- property.original -%></code></a> layout property in the Mapbox Style Specification.
<% } -%>
*/
@property (nonatomic<% if (!property.required) { %>, null_resettable<% } %>) MGLStyleValue<<%- propertyType(property, true) %>> *<%- camelizeWithLeadingLowercase(property.name) %>;
@property (nonatomic<% if (!property.required) { %>, null_resettable<% } if (property.getter) { %>, getter=<%- objCGetter(property) -%><% } %>) MGLStyleValue<<%- propertyType(property, true) %>> *<%- camelizeWithLeadingLowercase(property.name) %>;
<% if (property.original) { %>
@property (nonatomic<% if (!property.required) { %>, null_resettable<% } %>) MGLStyleValue<<%- propertyType(property, true) %>> *<%- camelizeWithLeadingLowercase(originalPropertyName(property)) %> __attribute__((unavailable("Use <%- camelizeWithLeadingLowercase(property.name) %> instead.")));
<% } -%>
<% } -%>
<% } -%>
<% if (paintProperties.length) { -%>
Expand All @@ -118,8 +122,12 @@ typedef NS_ENUM(NSUInteger, MGL<%- camelize(property.name) %>) {
This attribute corresponds to the <a href="https://www.mapbox.com/mapbox-gl-style-spec/#paint-<%- property.original -%>"><code><%- property.original -%></code></a> paint property in the Mapbox Style Specification.
<% } -%>
*/
@property (nonatomic<% if (!property.required) { %>, null_resettable<% } %>) MGLStyleValue<<%- propertyType(property, true) %>> *<%- camelizeWithLeadingLowercase(property.name) %>;
@property (nonatomic<% if (!property.required) { %>, null_resettable<% } if (property.getter) { %>, getter=<%- objCGetter(property) -%><% } %>) MGLStyleValue<<%- propertyType(property, true) %>> *<%- camelizeWithLeadingLowercase(property.name) %>;
<% if (property.original) { %>
@property (nonatomic<% if (!property.required) { %>, null_resettable<% } %>) MGLStyleValue<<%- propertyType(property, true) %>> *<%- camelizeWithLeadingLowercase(originalPropertyName(property)) %> __attribute__((unavailable("Use <%- camelizeWithLeadingLowercase(property.name) %> instead.")));
<% } -%>
<% } -%>
<% } -%>
@end
Expand Down
Loading

0 comments on commit 3f26e88

Please sign in to comment.