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

Base sequences, dictionaries, and records on Infra types #317

Merged
merged 3 commits into from
Feb 22, 2017

Conversation

domenic
Copy link
Member

@domenic domenic commented Feb 22, 2017

This closes whatwg/infra#4 and whatwg/infra#65. It also helps a bit with #242.

/cc @jyasskin @annevk


Preview | Diff

Copy link
Collaborator

@tobie tobie left a comment

Choose a reason for hiding this comment

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

LGTM. Do keep #242 open though.

index.bs Outdated
@@ -4104,6 +4107,10 @@ the value to use for the dictionary member when passing a value to a
not have a specified value. Dictionary members with default values are
always considered to be present.

An [=ordered map=] with string [=map/keys=] can be implicitly treated as a dictionary value of a
specific dictionary |D| if all of its [=map/entries=] correspond to dictionary members, in the
Copy link
Member

Choose a reason for hiding this comment

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

Do you want to also enforce required fields here?

@@ -5662,7 +5669,10 @@ identifies a [=dictionary=] is used to refer to
a type that corresponds to the set of all dictionaries that adhere to
the dictionary definition.

There is no way to represent a constant dictionary value in IDL.
The literal syntax for [=ordered maps=] may also be used to represent dictionaries, when it is
Copy link
Member

Choose a reason for hiding this comment

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

\o/

index.bs Outdated
@@ -5792,22 +5804,20 @@ The [=type name=] of a sequence type
is the concatenation of the type name for |T| and
the string “Sequence”.

Any [=list=] can be implicitly treated as a sequence, as long as it contains only [=list/items=]
Copy link
Member

Choose a reason for hiding this comment

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

Maybe "… as a sequence<T>, … that are of type T."?

index.bs Outdated
However, there is no way to represent a constant record value in IDL.
A <dfn export>record type</dfn> is a parameterized type whose values are [=ordered maps=] with
[=map/keys=] that are instances of |K| and [=map/values=] that are instances of |V|. |K| must be one
of {{DOMString}}, {{USVString}}, or {{ByteString}}. The order of a record's [=map/entries=] is
Copy link
Member

Choose a reason for hiding this comment

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

We can probably drop the "The order of a record's entries ..." sentence now. I feel like that's obvious from it being an ordered map.

index.bs Outdated
@@ -5822,6 +5832,11 @@ Records must not be used as the type of an [=attribute=] or
The [=type name=] of a record type is the concatenation of the type
name for |K|, the type name for |V| and the string “Record”.

Any [=ordered map=] can be implicitly treated as a record, as long as it contains only
Copy link
Member

Choose a reason for hiding this comment

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

Similar to sequence, as a record<K, V>` might be clearer, and might let you omit the "Additionally..." sentence since the restriction on K was already stated above.

index.bs Outdated
@@ -7415,17 +7430,17 @@ ECMAScript <emu-val>Object</emu-val> values.
[=ECMAScript/throw=] a <emu-val>TypeError</emu-val>.
1. Let |result| be a new empty instance of <code>[=record=]&lt;|K|, |V|></code>.
1. Let |keys| be [=?=] |O|.\[[OwnPropertyKeys]]().
1. Repeat, for each element |key| of |keys| in [=List=] order:
1. Repeat, [=list/for each=] element |key| of |keys|:
Copy link
Member

Choose a reason for hiding this comment

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

s/Repeat, // I think.

@domenic
Copy link
Member Author

domenic commented Feb 22, 2017

@jyasskin addressed your feedback. Great call on the sequence<T> and record<K, V> idea, that is much cleaner.

index.bs Outdated
@@ -7415,17 +7429,17 @@ ECMAScript <emu-val>Object</emu-val> values.
[=ECMAScript/throw=] a <emu-val>TypeError</emu-val>.
1. Let |result| be a new empty instance of <code>[=record=]&lt;|K|, |V|></code>.
1. Let |keys| be [=?=] |O|.\[[OwnPropertyKeys]]().
1. Repeat, for each element |key| of |keys| in [=List=] order:
1. [=list/For each=] element |key| of |keys|:
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. [=list/For each=] |key| of |keys|:

@tobie
Copy link
Collaborator

tobie commented Feb 22, 2017

Best commit message ever. :)

Can I merge this or are you waiting for @annevk's feedback?

@domenic
Copy link
Member Author

domenic commented Feb 22, 2017

I think we should be good to merge :)

@tobie tobie merged commit 26aa830 into master Feb 22, 2017
@tobie tobie deleted the infra-integration branch February 22, 2017 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Data structures section
3 participants