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

Doc gen handles arrays as required property values#6448

Merged
davidtheclark merged 1 commit into
masterfrom
icon-text-fit-padding-requires
Sep 24, 2016
Merged

Doc gen handles arrays as required property values#6448
davidtheclark merged 1 commit into
masterfrom
icon-text-fit-padding-requires

Conversation

@davidtheclark
Copy link
Copy Markdown
Contributor

Ref mapbox/mapbox-gl-style-spec#504.

Modifies the style documentation generation code to allow for an array as the required property value. An array means that the property must have one of multiple possible values.

I tried to match the sentence-style syntax and punctuation I saw elsewhere. Here's what we end up with in a few scenarios:

"requires": [ 
  {
    "icon-text-fit": ["both", "width", "height"]
  },
],

This property is only applied to the style if iconTextFit is set to either an NSValue object containing MGLSymbolStyleLayerIconTextFitBoth, or an NSValue object containing MGLSymbolStyleLayerIconTextFitWidth, or an NSValue object containing MGLSymbolStyleLayerIconTextFitHeight. Otherwise, it is ignored.

"requires": [
  {
    "icon-text-fit": [
      "both",
      "width"
    ]
  }
],

This property is only applied to the style if iconTextFit is set to either an NSValue object containing MGLSymbolStyleLayerIconTextFitBoth or an NSValue object containing MGLSymbolStyleLayerIconTextFitWidth. Otherwise, it is ignored.

"requires": [
  {
    "icon-text-fit": "both"
  }
],

This property is only applied to the style if iconTextFit is set to an NSValue object containing MGLSymbolStyleLayerIconTextFitBoth. Otherwise, it is ignored.

(Looks like I also stripped some superfluous end-of-line whitespace.)

cc @1ec5

@mention-bot
Copy link
Copy Markdown

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

@davidtheclark davidtheclark force-pushed the icon-text-fit-padding-requires branch 2 times, most recently from de75c68 to 3a19da6 Compare September 23, 2016 23:13
@davidtheclark
Copy link
Copy Markdown
Contributor Author

davidtheclark commented Sep 23, 2016

Modified the text based on chat feedback from @1ec5. Added semicolons between required properties and removed "either" if there are more than 2 possibilities. Now the doc for icon-text-fit-padding would look like this ...

... with 2 possible values:

This property is only applied to the style if iconImage is non-nil; and textField is non-nil; and iconTextFit is set to either an NSValue object containing MGLSymbolStyleLayerIconTextFitBoth or an NSValue object containing MGLSymbolStyleLayerIconTextFitWidth. Otherwise, it is ignored.

... with 3 possible values (the real thing):

This property is only applied to the style if iconImage is non-nil; and textField is non-nil; and iconTextFit is set to an NSValue object containing MGLSymbolStyleLayerIconTextFitBoth, an NSValue object containing MGLSymbolStyleLayerIconTextFitWidth, or an NSValue object containing MGLSymbolStyleLayerIconTextFitHeight. Otherwise, it is ignored.

@davidtheclark
Copy link
Copy Markdown
Contributor Author

Had another idea for reducing verbosity, by cutting the repetition of "an NSValue object containing". After latest commit, here's the example output ...

... with 2 possible values:

This property is only applied to the style if iconImage is non-nil; and textField is non-nil; and iconTextFit is set to an NSValue object containing either MGLSymbolStyleLayerIconTextFitBoth or MGLSymbolStyleLayerIconTextFitHeight. Otherwise, it is ignored.

... with 3 possible values:

This property is only applied to the style if iconImage is non-nil; and textField is non-nil; and iconTextFit is set to an NSValue object containing MGLSymbolStyleLayerIconTextFitBoth, MGLSymbolStyleLayerIconTextFitWidth, or MGLSymbolStyleLayerIconTextFitHeight. Otherwise, it is ignored.

@davidtheclark davidtheclark force-pushed the icon-text-fit-padding-requires branch from 819b340 to 45b97e8 Compare September 23, 2016 23:39
Copy link
Copy Markdown
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is tricky stuff, but it looks pretty good! You'll need to run make style-code-darwin again to regenerate the headers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This semicolon might be excessive if none of the requirements includes a comma.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can remove that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, a semicolon is definitely helpful if a requirement does include a comma, though. It isn’t wrong either way, since you’ve simplified the verbiage a bit, just a stylistic choice.

@davidtheclark davidtheclark force-pushed the icon-text-fit-padding-requires branch from 45b97e8 to 8ddbae2 Compare September 24, 2016 00:42
@davidtheclark
Copy link
Copy Markdown
Contributor Author

@1ec5 I reverted the semicolon to a comma and rebased & squashed into a single commit.

There is and should be no changes to the generated documentation yet — not until the a new spec version is released with the change discussed in mapbox/mapbox-gl-style-spec#504.

@1ec5
Copy link
Copy Markdown
Contributor

1ec5 commented Sep 24, 2016

Oh, one more thing: if you don’t mind rebasing again, can you prepend [ios, macos] to the commit message? Thanks!

@1ec5 1ec5 added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS documentation runtime styling labels Sep 24, 2016
@1ec5 1ec5 added this to the ios-v3.4.0 milestone Sep 24, 2016
@friedbunny
Copy link
Copy Markdown
Contributor

(Or you could add that to the commit message when you squash merge.)

@davidtheclark davidtheclark force-pushed the icon-text-fit-padding-requires branch from 8ddbae2 to f60e2eb Compare September 24, 2016 14:06
@davidtheclark davidtheclark merged commit bf18c33 into master Sep 24, 2016
@davidtheclark davidtheclark deleted the icon-text-fit-padding-requires branch September 24, 2016 14:39
@1ec5
Copy link
Copy Markdown
Contributor

1ec5 commented Sep 29, 2016

There is and should be no changes to the generated documentation yet — not until the a new spec version is released with the change discussed in mapbox/mapbox-gl-style-spec#504.

We update the gl-native project frequently between official specification releases, so the corresponding changes are being handled in #6508.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

documentation iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants