Add `all_layers: true` wildcard for data source layer matching

[bcamper]

NOTE: description updated to reflect PR discussion.

This PR introduces a "wildcard" syntax for the source layer in the data block in layers. When the parameter all_layers: true is included, it will match ALL layers in the data source.

This is useful for easily creating wireframe-like views, without knowing or needing to specify all the layers in the data source:

layers:
  wireframe:
    data: { source: tilezen, all_layers: true }
      draw:
        lines:
          color: red
          width: 1.5px
          order: 1
        points:
          color: yellow
          size: 4px
          collide: false

This parameter takes precedence over layer. If both all_layers: true and a value for layers are specified, then a warning is logged, all_layers: true is used, and the value of layer is ignored. Note that other values of all_layers (all_layers: false, all_layers: null, etc.) are not treated as a conflict, and in these cases, the value of layer will be used as usual.

[bcamper]

cc @matteblair @meetar @tallytalwar @nvkelso @burritojustice

What do you think of this? It's not crucial, but could be a nice to have, especially for exploring data sets. Seemed like a natural extension since we already allow you to specify multiple layer names as an array. See the gotcha with YAML syntax and * though... alternatives?

[nvkelso]

I dig it, I could have used it a few times already ;)

[matteblair]

Seems useful and simple to implement.

I feel like the most intuitive syntax would be to treat null or undefined layer values as a wildcard. If no specific layers are named then it's reasonable to assume that any layer applies. However, this isn't compatible with the current behavior of using the scene layer name as the data layer in this scenario.

layer: '*' could be fine I think, as long as it doesn't lead people to assume that this field supports pattern-matching.

layer: _ also seems reasonable to me (it isn't as ubiquitous as * but _ acts as a sort of catch-all placeholder in Rust and Swift).

[bcamper]

@matteblair yeah I can definitely see the logic for using an undefined layer to mean all... but that would be a pretty huge breaking change given the current scene layer matching behavior! Next time around... ;)

[pnorman]

How do you then match a layer named *?

[matteblair]

You could still match a layer named * with layer: ['*']

[matteblair]

We could also use a different key, e.g.

layers:
  wireframe:
    data: { source: tilezen, all_layers: true }

[tallytalwar]

👍 for using an explicit key. all_layers. Makes it readable and intent very explicit.

[bcamper]

Any other votes on syntax here? While all_layers is good because it's more explicit, it feels a little less Tangram-y vs another layer variant (not saying that's necessarily a great thing :), and also means you have potential conflict if both layer and all_layers are specified. I'm totally fine with it though if that's the consensus. I'm also fine with layer: _ and like that it doesn't require quotes in YAML.

[nvkelso]

My preference:

2. all_layers: true (most favorite, but tossup with the next one)
3. layers: '*' (the quoted * is slightly confusing, and it's a magic symbol)
4. layers: _ (least favorite, I'm not familiar with that syntax)

[matteblair]

If we really wanted to make the options mutually exclusive we could do something like layers: true, but that seems even more opaque than '*' or _ 😀

My favorite option is a separate key like all_layers - I think it's the least likely to be misinterpreted. We'll just specify in the documentation that this key takes precedence over layers, I don't think that logic would be surprising.

[meetar]

One nice thing about a separate key is that if you wanted to toggle between a specific layer/layers and "all", you could just toggle the value of "all_layers" and wouldn't have to remember which specific layers you'd specified previously.

On Apr 16, 2019, at 10:44 AM, Matt Blair notifications@github.com wrote:

If we really wanted to make the options mutually exclusive we could do something like layers: true, but that seems even more opaque than '*' or _ 😀

My favorite option is a separate key like all_layers - I think it's the least likely to be misinterpreted. We'll just specify in the documentation that this key takes precedence over layers, I don't think that logic would be surprising.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[bcamper]

all_layers: true it is! I'll update this branch.

[bcamper]

Updated the branch and description to reflect the syntax described here. When all_layers: true AND layer are both specified, all_layers: true takes precedence, and a warning is logged. Note that I didn't treat other values of all_layers (e.g. all_layers: false) as a conflict, and allowed layer to be used in these cases (to @meetar's point), but this is also up for interpretation as to correct behavior.