Note that a tag's { } block binds to the last dotted segment#329
Open
mlennie wants to merge 1 commit into
Open
Note that a tag's { } block binds to the last dotted segment#329mlennie wants to merge 1 commit into
mlennie wants to merge 1 commit into
Conversation
`# bar_chart.stack { y=total_sales }` parses cleanly and sets `y` on `stack`,
where nothing reads it, so the chart silently drops the field and renders
wrong. The same shape catches `# duration.terse { number="0.0" }`.
Both properties tables invite it: they name every property in dotted form,
then show brace-form examples a few rows down, so combining the two reads as
the natural way to set both at once. The rule is in the tag language reference
under advanced property syntax, but only for the `tName.p1=value { ... }` form,
and not where someone reading a chart's properties would find it.
State it at the point of confusion, in the two tables that pair a bare dotted
example with a brace example for the same tag.
Signed-off-by: Monty Lennie <montylennie@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
# bar_chart.stack { y=total_sales }parses cleanly, setsyonstackwhere nothing reads it, and renders a chart that silently ignores the field you asked for. A user hit this in production last week and had to open the network tab to find out why their chart was empty.The docs aren't wrong about any of this, which is worth saying plainly: there's no example of that form anywhere, and
bar_charts.malloynbcorrectly shows# bar_chart.stackbare. The gap is that nothing states the binding rule where a chart author would meet it.Two properties tables invite the mistake by naming every property in dotted form and then showing brace-form examples a few rows down, so welding the two reads as the natural way to set both at once:
bar_charts.malloynb:.stack→# bar_chart.stackat :10, and.y→# bar_chart { y=total_sales }three rows later. This is the one the user hit.numbers.malloynb: the# durationtable puts.tersedirectly above.number→# duration { number="0.0" }, adjacent lines.This adds one line to each.
Why not just link to the tag reference
language/tags.malloynbdoes document the shape, under advanced property syntax, but only fortName.p1=value { pp1=v1 pp2=v2 }— the form with a value. The trap is the value-less form,tName.p1 { ... }, which isn't shown. It also never says "last segment", and it's in the language reference rather than where someone reading a chart's properties is looking. So the note states the rule at the point of confusion and links out for the rest.Verified
Both forms run through the real
@malloydata/malloy-tagparser, because "looks right" isn't enough here:# bar_chart.stack { y=total_sales }bar_chart.stack.y— y buried under stack# bar_chart { stack y=total_sales }bar_chart.stack+bar_chart.y# duration.terse { number="0.0" }duration.terse.number— number buried under terse# duration { terse number="0.0" }duration.terse+duration.numberBoth trap forms parse without error, which is exactly what makes them a trap. The fix form's tree is the exact union of
# duration.terseand# duration { number="0.0" }, so the two are provably equivalent rather than plausibly so.npm run build-prodpasses locally.Scope
Only tables that pair a bare dotted example with a brace example for the same tag can produce the inference, which is these two.
charts_line_chart.malloynbis all-brace with no bare-dotted example.overview.malloynblists.stackand.terse, but its Key Properties column is a name list rather than usage examples, and its neighbouring brace examples are different tag roots, so there's nothing to weld — it links here anyway.