Skip to content
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

Merged
merged 11 commits into from
Mar 10, 2020

Conversation

AmeliaBR
Copy link
Contributor

@AmeliaBR AmeliaBR commented May 6, 2019

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:

  • CSS Backgrounds and Borders Level 3 & 4
  • CSS Conditional Rules Level — no edits required
  • CSS Multi-column Layout Level 1 & 2
  • CSS Values and Units Level 3 & 4— edits described above
  • CSS Flexible Box Layout Level 1
  • CSS Cascading and Inheritance Level 3 — no edits required
  • CSS Writing Modes Level 3 — no edits required, although see [css-values] Automatic parsing of value definitions #2921 (comment)
  • CSS Counter Styles Level 3

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!)

- Fix incorrect heading level & a typo from
  8ea8d7d
- Add examples converting the notation to prose.
These changes were added to Level 3 in
8ea8d7d
and
1265976

(And fix a tabs-vs-spaces error that bikeshed was choking on.)
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.
@frivoal
Copy link
Collaborator

frivoal commented May 6, 2019

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?

@AmeliaBR
Copy link
Contributor Author

AmeliaBR commented May 6, 2019

@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.

@frivoal
Copy link
Collaborator

frivoal commented May 8, 2019

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?

@astearns
Copy link
Member

astearns commented May 8, 2019

I'm happy with redundancy - explaining the range in prose is another way to learn the new grammar.

@AmeliaBR
Copy link
Contributor Author

AmeliaBR commented May 8, 2019

Ok. I'll rewrite the prose to be more consistent / less verbose, but will leave some prose in place.

@AmeliaBR
Copy link
Contributor Author

AmeliaBR commented May 8, 2019

write new tests to make sure it isn't ignored.

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,&infin;]>>
→ “Negative values are not allowed.”

<<integer [1, &infin;]>>
→ “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.
@AmeliaBR
Copy link
Contributor Author

AmeliaBR commented May 8, 2019

Pushed a tidy-up of the prose restrictions for these specs. Also expanded the note in CSS Values:

Note: At the time of writing, the bracketed range notation is new; thus in most CSS specifications any range limitations are described only in prose. This does not make them any less binding. Common prose restrictions include “Negative values are not allowed” or “Negative values are invalid” to indicate a [0,∞] range. For an <integer>, “Values must be greater than 0” is equivalent to <integer [1, ∞]>.

@fantasai
Copy link
Collaborator

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.

@AmeliaBR
Copy link
Contributor Author

Thanks @fantasai. I've merged your changes into the PR, so they won't get messed up when this PR gets merged into master.

@fantasai
Copy link
Collaborator

@AmeliaBR Where you're tweaking the prose... I think using the term "invalid" is better because it more cleanly hooks into the parsing implications.

@fantasai
Copy link
Collaborator

@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

@svgeesus
Copy link
Contributor

@AmeliaBR could you take a look at the three conflicts? It would be good to merge this PR.

@tabatkins tabatkins merged commit 69197fd into w3c:master Mar 10, 2020
@AmeliaBR
Copy link
Contributor Author

Thanks @tabatkins (it was on my to-do, but I'm still sick & with a backlog across many projects)

JTensai pushed a commit to JTensai/csswg-drafts that referenced this pull request May 13, 2020
…#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,&infin;]>>
→ “Negative values are not allowed.”

<<integer [1, &infin;]>>
→ “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 <jackalmage@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment