-
Notifications
You must be signed in to change notification settings - Fork 682
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use bracket range notation in syntax grammar [css-values-3][css-values-4][css-backgrounds-3][css-backgrounds-4][css-multicol-1][css-multicol-2][css-flexbox-1][css-counter-styles-3] #3894
Conversation
- Fix incorrect heading level & a typo from 8ea8d7d - Add examples converting the notation to prose.
Added bracketed range notation for non-negative values ( [0,∞] ) in these properties & productions: - <bg-size> - <line-width> - border-radius and longhands - border-image-slice - border-image-width - border-image-outset - <shadow> NOTE: required re-writing the syntax (since ony one of the 4 possible lengths has a restriction) - border-limit and border-clip in level 4 NOTE: these didn't have prose restrictions, but they also didn't explain how negative values could work. I added a changes note to level 3 (change since CR), but not level 4 (not published yet).
Added bracketed range notation for non-negative values ( [0,∞] ) or strictly positive integers ( [1,∞] ) in these properties: - column-width - column-count - column-gap - column-span (level 2) For column-rule-width, the syntax change was made for the `<line-width>` production, in 933d7dd
Added bracketed range notation for non-negative values ( [0,∞] ) in these properties: - flex-grow - flex-shrink Also use it in the prose descriptions for `flex`, and to replace the made up `<postive-number>` variable in the examples of common patterns. Created a new "Changes since the 19 November 2018 CR" section to include a mention of the update. The syntax for `flex-basis` will be updated by reference when `width` is updated.
Added bracketed range notation for non-negative values ( [0,∞] ) in the following descriptors: - pad - additive-symbols Note: I didn't add any restriction to the integer in the `system: fixed <integer>` descriptor, since there was no prose constraint. But I'm not sure that negative values make sense here.
Reviewed multicol 1&2. The edits are correct, but I wonder: you both added the bracket range notation in the grammar and left the equivalent restrictions in prose. Shouldn't we drop the prose now that the grammar expresses the same thing unambiguously? |
@frivoal Because the bracket syntax is new, I thought it best to leave in the prose for clarification. But maybe I should tidy up the prose as I go along, too? There are a lot of different phrases used in different specs to describe these restrictions. I could switch them all to the simple "Negative values are not allowed." And maybe minimize duplication (e.g., only include that phrase once per property) when its unambiguous that it applies to multiple data types. |
My take would be to drop it from the prose as soon as it's added to the grammar, and write new tests to make sure it isn't ignored. But that's just me. Your approach isn't wrong either. Can we have a third opinion? |
I'm happy with redundancy - explaining the range in prose is another way to learn the new grammar. |
Ok. I'll rewrite the prose to be more consistent / less verbose, but will leave some prose in place. |
For me, I'm less worried about the new grammar being ignored by implementations & more worried about it being inscrutable to authors. I'm assuming that all these restrictions are already tested for any spec that's at stable CR, though I haven't been checking. |
…ter-styles-3][css-flexbox-1][css-multicol-1][css-values-3][css-values-4] For all the specs covered by PR w3c#3894. <<type [0,∞]>> → “Negative values are not allowed.” <<integer [1, ∞]>> → “Values must be greater than 0.” And remove some duplicate phrases. Added these phrases & the other most commonly used one (“are invalid”) to the note in CSS Values about prose restrictions.
Pushed a tidy-up of the prose restrictions for these specs. Also expanded the note in CSS Values:
|
Cherry-picked out (and tweaked) the css-values changes, so you'll want to drop those from the PR. Don't merge until we republish on /TR. :) Shouldn't take more than a week or two. |
Thanks @fantasai. I've merged your changes into the PR, so they won't get messed up when this PR gets merged into master. |
@AmeliaBR Where you're tweaking the prose... I think using the term "invalid" is better because it more cleanly hooks into the parsing implications. |
@AmeliaBR Would you mind updating to use "invalid" per #3894 (comment) ? css-values-3 has been republished with the range notation, so after that's fixed we should be OK to merge |
@AmeliaBR could you take a look at the three conflicts? It would be good to merge this PR. |
Thanks @tabatkins (it was on my to-do, but I'm still sick & with a backlog across many projects) |
…#3894) * [css-values-3] Tidy bracket range definition - Fix incorrect heading level & a typo from 8ea8d7d - Add examples converting the notation to prose. * [css-values-4] Copy over bracket range definition These changes were added to Level 3 in 8ea8d7d and 1265976 (And fix a tabs-vs-spaces error that bikeshed was choking on.) * [css-backgrounds-3][css-backgrounds-4] Use bracketed range notation Added bracketed range notation for non-negative values ( [0,∞] ) in these properties & productions: - <bg-size> - <line-width> - border-radius and longhands - border-image-slice - border-image-width - border-image-outset - <shadow> NOTE: required re-writing the syntax (since ony one of the 4 possible lengths has a restriction) - border-limit and border-clip in level 4 NOTE: these didn't have prose restrictions, but they also didn't explain how negative values could work. I added a changes note to level 3 (change since CR), but not level 4 (not published yet). * [css-values-3][css-values-4] Export dfn for bracketed range notation * [css-multicol-1][css-multicol-2] Use bracketed range notation Added bracketed range notation for non-negative values ( [0,∞] ) or strictly positive integers ( [1,∞] ) in these properties: - column-width - column-count - column-gap - column-span (level 2) For column-rule-width, the syntax change was made for the `<line-width>` production, in 933d7dd * [css-flexbox-1] Use bracketed range notation Added bracketed range notation for non-negative values ( [0,∞] ) in these properties: - flex-grow - flex-shrink Also use it in the prose descriptions for `flex`, and to replace the made up `<postive-number>` variable in the examples of common patterns. Created a new "Changes since the 19 November 2018 CR" section to include a mention of the update. The syntax for `flex-basis` will be updated by reference when `width` is updated. * [css-counter-styles-3] Use bracketed range notation Added bracketed range notation for non-negative values ( [0,∞] ) in the following descriptors: - pad - additive-symbols Note: I didn't add any restriction to the integer in the `system: fixed <integer>` descriptor, since there was no prose constraint. But I'm not sure that negative values make sense here. * Standardize prose for constrained values [css-backgrounds-3][css-counter-styles-3][css-flexbox-1][css-multicol-1][css-values-3][css-values-4] For all the specs covered by PR w3c#3894. <<type [0,∞]>> → “Negative values are not allowed.” <<integer [1, ∞]>> → “Values must be greater than 0.” And remove some duplicate phrases. Added these phrases & the other most commonly used one (“are invalid”) to the note in CSS Values about prose restrictions. * [css-values-3][css-values-4] Fix broken entity Co-authored-by: Tab Atkins Jr <[email protected]>
Partially addresses #355
Includes some minor fixes to the definition of the notation @fantasai added to css-values-3.
The definition is then copied over to css-values-4 (which is what you get for just /css-values/ now).
The term "bracketed range notation" is exported for use in other specs, primarily to explain why I'm changing things.
This initial PR includes syntax edits to the specs listed as Stable CR in the Current Work web page, and later levels of those same modules:
See the individual commit messages for details of the changes. I've tried to match code style to each document.
Note that this currently creates some non-fatal bikeshed errors, see speced/bikeshed#1467
If that's going to take a while to get fixed, @tabatkins, it might be worth a separate PR just to merge the changes to CSS Values.
I'll be continuing to work through the remaining CR and WD specs, but that may be spread out over a few weeks, so no need to delay merging these edits waiting for an omnibus pull request. (That'll just increase the risk of merge conflicts!)