[ios] Add inline examples in documentation #7337

ericrwolfe · 2016-12-07T23:36:05Z

Examples for:

Sources
- MGLGeoJSONSource
- MGLVectorSource
- MGLRasterSource
Layers
- MGLCircleStyleLayer
- MGLFillStyleLayer
- MGLLineStyleLayer
- MGLSymbolStyleLayer
MGLVectorStyleLayer.predicate

Swift 3 for now. We can add Objective-C inline or a link to the respective examples on http://mapbox.com/ios-sdk/examples later.

mention-bot · 2016-12-07T23:36:07Z

@ericrwolfe, thanks for your PR! By analyzing this pull request, we identified @incanus, @1ec5 and @boundsj to be potential reviewers.

1ec5

Thanks for this PR. Examples in our documentation are long overdue.

Most of the comments below relate to changes that have taken place since beta 4 that break these examples. Have you considered other approaches to ensuring maintainability, like keeping these examples in a Swift playground and pulling them into the header in a build step?

1ec5 · 2016-12-08T09:23:43Z

platform/darwin/src/MGLCircleStyleLayer.h

+ ])
+ layer.circleOpacity = MGLStyleValue(rawValue: 0.7)
+ layer.predicate = NSPredicate(format: "%K == %@", "ethnicity", "ewok")
+ mapView.style().add(layer)


#6097 replaced MGLMapView.style() with an MGLMapView.style property.

1ec5 · 2016-12-08T09:27:43Z

platform/darwin/src/MGLCircleStyleLayer.h

+    22: MGLStyleValue(rawValue: 180)
+ ])
+ layer.circleOpacity = MGLStyleValue(rawValue: 0.7)
+ layer.predicate = NSPredicate(format: "%K == %@", "ethnicity", "ewok")


Nit: ethnicity == 'ewok' would have clearer intent. And lol on the choice of key-value pair.

1ec5 · 2016-12-08T09:32:43Z

platform/darwin/src/MGLGeoJSONSource.h

+ var coordinates: [CLLocationCoordinate2D] = []
+ let polyline = MGLPolylineFeature(coordinates: &coordinates, count: UInt(coordinates.count))
+ let source = MGLGeoJSONSource(identifier: "lines", features: [polyline], options: nil)
+ self.mapView.style().add(source)


Nit: other examples omit self., presumably because mapView could be a local variable.

1ec5 · 2016-12-08T09:35:40Z

platform/darwin/src/MGLGeoJSONSource.h

+ ```swift
+ var coordinates: [CLLocationCoordinate2D] = []
+ let polyline = MGLPolylineFeature(coordinates: &coordinates, count: UInt(coordinates.count))
+ let source = MGLGeoJSONSource(identifier: "lines", features: [polyline], options: nil)


#7145 renamed this initializer to MGLGeoJSONSource(identifier:shape:options:), and #7334 would go a step further, renaming the class to MGLShapeSource. It still makes sense to demonstrate creating the source with a feature rather than an unadorned shape.

1ec5 · 2016-12-08T09:41:43Z

platform/darwin/src/MGLVectorStyleLayer.h

+ ```swift
+ let meters = 1500.0
+ let layer = MGLLineStyleLayer(identifier: "contour", source: contours)
+ layer.predicate = NSCompoundPredicate(andPredicateWithSubpredicates: [


This is a lot of typing. Is there a point you want to make about the individual classes within the NSPredicate class cluster, or would a single predicate format string such as (index == 10 || index == 5) && ele >= 200 be sufficient?

friedbunny · 2016-12-09T21:47:40Z

Most of the comments below relate to changes that have taken place since beta 4 that break these examples. Have you considered other approaches to ensuring maintainability, like keeping these examples in a Swift playground and pulling them into the header in a build step?

Maintainability and testability are important considerations that have kept us from expanding our examples repertoire in the past, so I’m agreed that we should tackle those problems in tandem with the actual example-writing process.

Using a Swift playground is an interesting idea that opens the possibility of users being able to fiddle with live examples themselves, which would be an obvious win. I still think that we give Obj-C examples the same prominence, though, which means finding a solution that works for both (or a second Obj-C solution).

boundsj · 2016-12-10T00:10:22Z

MGLGeoJSONSource was renamed to MGLShapeSource in #7334

ericrwolfe · 2016-12-13T21:50:33Z

Added unit tests for each of the inline examples. Should help catch any breaking changes to the examples.

friedbunny · 2016-12-13T21:51:01Z

platform/darwin/src/MGLLineStyleLayer.h

+ let layer = MGLLineStyleLayer(identifier: "trails-path", source: trails)
+ layer.sourceLayerIdentifier = "trails"
+ layer.lineWidth = MGLStyleValue(base: 1.5, stops: [
+ 14: MGLStyleValue(rawValue: 2),


Missed the indentation here.

Bah, copy and paste fail.

friedbunny · 2016-12-13T22:01:15Z

@mayagao is reworking the jazzy theme elsewhere, but here’s how these examples look now:

👍

1ec5 · 2016-12-14T02:47:48Z

platform/darwin/src/MGLRasterSource.h

+
+ ```swift
+ // Define tileset
+ let tileset = MGLTileSet(tileURLTemplates: ["https://example.com/raster-tiles/{z}/{x}/{y}.png"])


#7377 removed MGLTileSet in favor of direct source initializers with option dictionaries. Sorry to keep shifting the goalposts on you. There’s still an attribution option that takes an HTML string, but now there’s also an option that takes structured MGLAttributionInfo objects.

1ec5 · 2016-12-14T02:50:05Z

platform/darwin/src/MGLShapeSource.h

+ mapView.style.addSource(source)
+ ```
+
+ @see <a href="https://www.mapbox.com/mapbox-gl-style-spec/#sources-geojson">The


These links become redundant once you merge in (or rebase atop) #7377.

boundsj · 2016-12-14T06:55:43Z

platform/darwin/src/MGLCircleStyleLayer.h

- layer.predicate = NSPredicate(format: "%K == %@", "ethnicity", "ewok")
- mapView.style().add(layer)
+ layer.predicate = NSPredicate(format: "%K == %@", "marital-status", "married")
+ mapView.style.addLayer(layer)


@1ec5 it's interesting that making the style a property instead of returning it in a method had this effect. Is this a case for NS_SWIFT_NAME?

You’re referring to the change from add(_:) to addLayer(_:)? I was curious about that too. It’s unlikely to be due to the change from MGLMapView.style() to MGLMapView.style. The more likely cause is that #6097 added layers and sources properties, or that we started using Xcode 8.2 instead 8.1, but I’m not really sure exactly what introduced the change.

As for whether we should use NS_SWIFT_NAME, I’m ambivalent. If we do, we should make sure to refine all the layer and source methods on this class.

Using a reduced project, I found that indeed the addition of a sources property changed add(_:) to addSource(_:), and likewise the addition of a layers property changed add(_:) to addLayer(_:). This appears to be consistent with Cocoa APIs such as -[UIView addConstraint:], which, due to the presence of UIView.constraints, is bridged as addConstraint(_:).

ericrwolfe · 2016-12-19T20:58:45Z

Last commit should cover all of the outstanding changes needed. Lmk if you see anything else that could use an inline example.

/cc @1ec5 @friedbunny

1ec5 · 2016-12-19T22:30:52Z

platform/darwin/src/MGLCircleStyleLayer.h

+
+ ### Example ###
+
+ ```swift


These style layer headers are generated by platform/darwin/scripts/generate-style-code.js, so the next time someone runs make style-code-darwin, these changes will get blown away. The approach I took in #7475 was to override a property on each layer’s entry in the style specification JSON to provide documentation. Unfortunately, it’d be quite inconvenient to embed a multiline code example inside a JSON string.

Let’s have the overridden style specification JSON name the relevant function within MGLDocumentationExampleTests. generate-style-code.js can then extract that function using a regular expression and insert it into the generated header.

ericrwolfe · 2016-12-21T01:34:44Z

Last set of commits after rebasing on release-3.4.0 adds a run script that will extract the examples from MGLDocumentationExampleTests and replaces the corresponding comment token in the header docs.

/cc @1ec5 @friedbunny

ericrwolfe · 2016-12-21T01:41:58Z

platform/ios/scripts/document.sh

 jazzy \
    --config platform/ios/jazzy.yml \
    --sdk iphonesimulator \
    --github-file-prefix https://github.com/mapbox/mapbox-gl-native/tree/${BRANCH} \
    --module-version ${SHORT_VERSION} \
+    --framework-root ${FRAMEWORK_PATH} \
+    --umbrella-header "${FRAMEWORK_PATH}/Headers/Mapbox.h" \


By default, Jazzy generates its docs from the project source, which excludes any changes by the run script, so I've added an optional FRAMEWORK_PATH env variable that by default searches for the dynamic framework output by make iframework.

If someone runs make idocument after building only the static framework, this will probably error out. It’s a workflow I don’t personally use, but I suppose someone might end up doing that after reading DEVELOPING.md. I wonder if we could check for the existence of the dynamic framework and, if it doesn’t exist, fall back to the static framework’s Headers folder. But that’s all fodder for other PRs.

1ec5

Beautiful!

1ec5 · 2016-12-21T02:07:24Z

platform/darwin/src/MGLVectorSource.h

@@ -24,6 +24,9 @@ NS_ASSUME_NONNULL_BEGIN
 (<var>extent</var>&nbsp;×&nbsp;2)&nbsp;−&nbsp;1, inclusive. Any vector style
 layer initialized with a vector source must have a non-`nil` value in its
 `sourceLayerIdentifier` property.
+
+ <!--EXAMPLE: MGLVectorSource-->
+ ```


Stray backticks.

1ec5 · 2016-12-21T02:22:15Z

platform/ios/scripts/document.sh

 jazzy \
    --config platform/ios/jazzy.yml \
    --sdk iphonesimulator \
    --github-file-prefix https://github.com/mapbox/mapbox-gl-native/tree/${BRANCH} \
    --module-version ${SHORT_VERSION} \
+    --framework-root ${FRAMEWORK_PATH} \
+    --umbrella-header "${FRAMEWORK_PATH}/Headers/Mapbox.h" \


If someone runs make idocument after building only the static framework, this will probably error out. It’s a workflow I don’t personally use, but I suppose someone might end up doing that after reading DEVELOPING.md. I wonder if we could check for the existence of the dynamic framework and, if it doesn’t exist, fall back to the static framework’s Headers folder. But that’s all fodder for other PRs.

1ec5 · 2016-12-21T02:27:05Z

platform/ios/ios.xcodeproj/project.pbxproj

+			);
+			runOnlyForDeploymentPostprocessing = 0;
+			shellPath = /bin/sh;
+			shellScript = "node \"${SRCROOT}/scripts/add-examples-to-docs.js\"";


If you declare add-examples-to-docs.js and MGLDocumentationExampleTests.swift as input files to this build phase and the modified headers as output files, Xcode will avoid running this script when none of those files have changed. But since that’d be one more place that would have to get updated every time we add an example, it’s fine to leave the input and output files out.

ericrwolfe added documentation iOS Mapbox Maps SDK for iOS runtime styling labels Dec 7, 2016

ericrwolfe added this to the ios-v3.4.0 milestone Dec 8, 2016

1ec5 suggested changes Dec 8, 2016

View reviewed changes

1ec5 mentioned this pull request Dec 8, 2016

Expand source documentation #7346

Merged

friedbunny reviewed Dec 13, 2016

View reviewed changes

1ec5 reviewed Dec 14, 2016

View reviewed changes

boundsj reviewed Dec 14, 2016

View reviewed changes

ericrwolfe force-pushed the erw_inline_examples branch from 6f5a8bc to 920b0b8 Compare December 19, 2016 20:56

1ec5 suggested changes Dec 19, 2016

View reviewed changes

1ec5 self-assigned this Dec 19, 2016

vivienlegrand mentioned this pull request Dec 20, 2016

MGLRasterStyleLayer not fully implemented #7490

Closed

ericrwolfe added 7 commits December 20, 2016 17:07

Add inline code examples for sources, layers, and layer.predicate

33b3fc3

Autolink out to concrete source classes

be8b3b0

Futureproof inline examples with test cases

bcce1e7

Update examples and corresponding tests

bfbf31d

Run script to extract example code into headers

036bc75

Fix missing examples

c58f64d

Add console logging

2b7863d

ericrwolfe force-pushed the erw_inline_examples branch from fd2fa3b to 2b7863d Compare December 21, 2016 01:31

Optional FRAMEWORK_PATH env variable

d4d6542

ericrwolfe commented Dec 21, 2016

View reviewed changes

1ec5 approved these changes Dec 21, 2016

View reviewed changes

Remove stray backticks

28949c5

ericrwolfe merged commit 9faa4af into release-ios-v3.4.0 Dec 21, 2016

ericrwolfe deleted the erw_inline_examples branch December 21, 2016 18:22

1ec5 mentioned this pull request Dec 25, 2016

Guides for style and GeoJSON authors #7542

Merged

This was referenced Jan 1, 2017

“View Source on GitHub” links in documentation are broken #7565

Closed

Insert example code in original headers #7569

Merged

ericrwolfe mentioned this pull request Mar 17, 2017

[WIP] update iOS SDK guides #8354

Merged

2 tasks

friedbunny mentioned this pull request Mar 17, 2017

[ios, macos] Add testing for code in guides #8461

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ios] Add inline examples in documentation #7337

[ios] Add inline examples in documentation #7337

ericrwolfe commented Dec 7, 2016

mention-bot commented Dec 7, 2016

1ec5 left a comment

1ec5 Dec 8, 2016

1ec5 Dec 8, 2016

1ec5 Dec 8, 2016

1ec5 Dec 8, 2016

1ec5 Dec 8, 2016

friedbunny commented Dec 9, 2016

boundsj commented Dec 10, 2016

ericrwolfe commented Dec 13, 2016

friedbunny Dec 13, 2016

ericrwolfe Dec 14, 2016

friedbunny commented Dec 13, 2016 •

edited

Loading

1ec5 Dec 14, 2016

1ec5 Dec 14, 2016

boundsj Dec 14, 2016

1ec5 Dec 14, 2016

1ec5 Dec 15, 2016

ericrwolfe commented Dec 19, 2016

1ec5 Dec 19, 2016 •

edited

Loading

ericrwolfe commented Dec 21, 2016 •

edited

Loading

ericrwolfe Dec 21, 2016

1ec5 Dec 21, 2016

1ec5 left a comment

1ec5 Dec 21, 2016

1ec5 Dec 21, 2016

1ec5 Dec 21, 2016

[ios] Add inline examples in documentation #7337

[ios] Add inline examples in documentation #7337

Conversation

ericrwolfe commented Dec 7, 2016

mention-bot commented Dec 7, 2016

1ec5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

friedbunny commented Dec 9, 2016

boundsj commented Dec 10, 2016

ericrwolfe commented Dec 13, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

friedbunny commented Dec 13, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericrwolfe commented Dec 19, 2016

1ec5 Dec 19, 2016 • edited Loading

Choose a reason for hiding this comment

ericrwolfe commented Dec 21, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

friedbunny commented Dec 13, 2016 •

edited

Loading

1ec5 Dec 19, 2016 •

edited

Loading

ericrwolfe commented Dec 21, 2016 •

edited

Loading