Skip to content

Change ABNF array definition to permit single value types only #663

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

Change ABNF array definition to permit single value types only #663

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d4657be
Change ABNF array def to permit single types only
eksortso Sep 3, 2019
4a72cb0
Clarification: make explicit all value types
dave-ostroske-at-microbac Sep 5, 2019
37f8c1f
Replace date-time with 4 separate types in arrays
dave-ostroske-at-microbac Sep 5, 2019
bd8b641
Clarify: arrays contain values like those in KVPs
dave-ostroske-at-microbac Sep 6, 2019
1ce4a40
Merge remote-tracking branch 'upstream/master' into abnf-single-type-…
eksortso Sep 10, 2019
94bfb5f
Make array def in ABNF more readable for all types
eksortso Sep 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,20 @@ on the same line (though some values can be broken over multiple lines).
key = "value"
```

Values must be of the following types: String, Integer, Float, Boolean,
Datetime, Array, or Inline Table. Unspecified values are invalid.
Values must have one of the following types.

- [String](#user-content-string)
- [Integer](#user-content-integer)
- [Float](#user-content-float)
- [Boolean](#user-content-boolean)
- [Offset Date-Time](#user-content-offset-date-time)
- [Local Date-Time](#user-content-local-date-time)
- [Local Date](#user-content-local-date)
- [Local Time](#user-content-local-time)
- [Array](#user-content-array)
- [Inline Table](#user-content-inline-table)

Unspecified values are invalid.

```toml
key = # INVALID
Expand Down Expand Up @@ -579,9 +591,13 @@ Array
-----

Arrays are square brackets with values inside. Whitespace is ignored. Elements
are separated by commas. Data types may not be mixed (different ways to define
strings should be considered the same type, and so should arrays with different
element types).
are separated by commas.

All values that are allowed in [key/value pairs](#user-content-keyvalue-pair)
Copy link
Member

Choose a reason for hiding this comment

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

That's means... all values are allowed?

I'd prefer that we rephrase this paragraph as something along the lines of:

<prev-paragraph> All elements within an array must have the same data type.

All string values are considered the same type. All arrays (regardless of types of elements in them) are considered the same type. All inline tables (regardless of the key-value pairs in them) are considered the same type.

Copy link
Contributor

Choose a reason for hiding this comment

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

@pradyunsg "All values that are allowed in key/value pairs are also allowed in arrays..." – That (partial) sentence or something similar is essential because we have to explain which types are allowed in arrays (String, Integer, Float, Boolean, Offset Date-Time, Local Date-Time, Local Date, Local Time, Array, Inline Table) and which are not (Table, Array of Tables), and currently we don't.

It might be possible to rephrase it a bit, say: "An array collects values of any of the types that are allowed in key/value pairs..." But we must be explicit about this, and a backreference to the key/value pairs it much better than having to list all (currently ten) allowed types again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about we provide a separate section for Value Types, so that we can pinpoint those types in the spec? Then we can include a backreference to that section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Optionally, we could move the statements about string, array, and inline table types up to the proposed Value Types section. We'd simplify it, though, if we did that.

All string values are considered the same type. All arrays are considered the same type, regardless of content. All inline tables are considered the same type, regardless of content.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having said all that, let me propose the following synthesis of both of your suggestions. We'd also include a backreference from "value type".

Arrays are square brackets with values inside. Whitespace is ignored. Elements
are separated by commas.

All elements within an array must have the same value type.

(Examples go here.)

Copy link
Member

Choose a reason for hiding this comment

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

Arrays are square brackets with values inside. Whitespace is ignored. Elements
are separated by commas.

All elements within an array must have the same value type.

LGTM.

are also allowed in arrays, but all values within an array must have the same
data type. Different ways to define strings are considered the same type, and so
are arrays with different element types, and inline tables regardless of their
contents.

```toml
arr1 = [ 1, 2, 3 ]
Expand Down
44 changes: 41 additions & 3 deletions toml.abnf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,51 @@ local-time = partial-time

;; Array

array = array-open [ array-values ] ws-comment-newline array-close
array = array-open ws-comment-newline [ array-values ] ws-comment-newline array-close

array-open = %x5B ; [
array-close = %x5D ; ]

array-values = ws-comment-newline val ws array-sep array-values
array-values =/ ws-comment-newline val ws [ array-sep ]
array-values = array-strings
array-values =/ array-booleans
array-values =/ array-arrays
array-values =/ array-inline-tables
array-values =/ array-offset-dts
array-values =/ array-local-dts
array-values =/ array-local-dates
array-values =/ array-local-times
array-values =/ array-floats
array-values =/ array-integers

array-strings = string ws array-sep ws-comment-newline array-strings
Copy link
Member

Choose a reason for hiding this comment

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

array-value-sep = ws array-sep ws-comment-newline
array-value-ending = ws [ array-sep ]

array-strings =  string array-value-sep array-strings
array-strings =/ string array-value-ending

array-strings =/ string ws [ array-sep ]

array-booleans = boolean ws array-sep ws-comment-newline array-booleans
array-booleans =/ boolean ws [ array-sep ]

array-arrays = array ws array-sep ws-comment-newline array-arrays
array-arrays =/ array ws [ array-sep ]

array-inline-tables = inline-table ws array-sep ws-comment-newline array-inline-tables
array-inline-tables =/ inline-table ws [ array-sep ]

array-offset-dts = offset-date-time ws array-sep ws-comment-newline array-offset-dts
array-offset-dts =/ offset-date-time ws [ array-sep ]

array-local-dts = local-date-time ws array-sep ws-comment-newline array-local-dts
array-local-dts =/ local-date-time ws [ array-sep ]

array-local-dates = local-date ws array-sep ws-comment-newline array-local-dates
array-local-dates =/ local-date ws [ array-sep ]

array-local-times = local-time ws array-sep ws-comment-newline array-local-times
array-local-times =/ local-time ws [ array-sep ]

array-floats = float ws array-sep ws-comment-newline array-floats
array-floats =/ float ws [ array-sep ]

array-integers = integer ws array-sep ws-comment-newline array-integers
array-integers =/ integer ws [ array-sep ]

array-sep = %x2C ; , Comma

Expand Down